{"thread":{"id":"55206","subject":"[PATCH 1/2] remote: add camel-cased *.tagOpt key, like clone","startedAt":"2021-02-25T01:22:15Z","lastAt":"2021-03-18T11:23:45Z","messageCount":8,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Bert Wesarg"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"417769","messageId":"20210225012117.17331-1-avarab@gmail.com","threadId":"55206","inReplyTo":null,"subject":"[PATCH 1/2] remote: add camel-cased *.tagOpt key, like clone","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-25T01:21:16Z","receivedAt":"2021-02-25T01:22:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change \"git remote add\" so that it adds a *.tagOpt key, and not the\nlower-cased *.tagopt on \"git remote add --no-tags\", just as \"git clone\n--no-tags\" would do.\n\nThis doesn't matter for anything that reads the config. It's just\nprettier if we write config keys in their documented camelCase form to\nuser-readable config files.\n\nWhen I added support for \"clone -no-tags\" in 0dab2468ee5 (clone: add a\n--no-tags option to clone without tags, 2017-04-26) I made it use\nthe *.tagOpt form, but the older \"git remote add\" added in\n111fb858654 (remote add: add a --[no-]tags option, 2010-04-20) has\nbeen using *.tagopt all this time.\n\nIt's easy enough to add a test for this, so let's do that. We can't\nuse \"git config -l\" there, because it'll normalize the keys to their\nlower-cased form. Let's add the test for \"git clone\" too for good\nmeasure, not just to the \"git remote\" codepath we're fixing.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nI also noticed that we write e.g. init.objectformat instead of\ninit.objectFormat, and core.logallrefupdates etc. If anyone's got an\neven even worse case of OCD there's an interesting #leftoverbits\nproject there of scouring the code for more cases of this sort of\nthing...\n\n builtin/remote.c         | 2 +-\n t/t5505-remote.sh        | 1 +\n t/t5612-clone-refspec.sh | 1 +\n 3 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex d11a5589e49..f286ae97538 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -221,7 +221,7 @@ static int add(int argc, const char **argv)\n \n \tif (fetch_tags != TAGS_DEFAULT) {\n \t\tstrbuf_reset(&buf);\n-\t\tstrbuf_addf(&buf, \"remote.%s.tagopt\", name);\n+\t\tstrbuf_addf(&buf, \"remote.%s.tagOpt\", name);\n \t\tgit_config_set(buf.buf,\n \t\t\t       fetch_tags == TAGS_SET ? \"--tags\" : \"--no-tags\");\n \t}\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 045398b94e6..2a7b5cd00a0 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -594,6 +594,7 @@ test_expect_success 'add --no-tags' '\n \t\tcd add-no-tags &&\n \t\tgit init &&\n \t\tgit remote add -f --no-tags origin ../one &&\n+\t\tgrep tagOpt .git/config &&\n \t\tgit tag -l some-tag >../test/output &&\n \t\tgit tag -l foobar-tag >../test/output &&\n \t\tgit config remote.origin.tagopt >>../test/output\ndiff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh\nindex 6a6af7449ca..3126cfd7e9d 100755\n--- a/t/t5612-clone-refspec.sh\n+++ b/t/t5612-clone-refspec.sh\n@@ -97,6 +97,7 @@ test_expect_success 'by default no tags will be kept updated' '\n test_expect_success 'clone with --no-tags' '\n \t(\n \t\tcd dir_all_no_tags &&\n+\t\tgrep tagOpt .git/config &&\n \t\tgit fetch &&\n \t\tgit for-each-ref refs/tags >../actual\n \t) &&\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"417770","messageId":"20210225012117.17331-2-avarab@gmail.com","threadId":"55206","inReplyTo":"20210225012117.17331-1-avarab@gmail.com","subject":"[PATCH 2/2] remote: write camel-cased *.pushRemote on rename","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-25T01:21:17Z","receivedAt":"2021-02-25T01:22:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"When a remote is renamed don't change the canonical \"*.pushRemote\"\nform to \"*.pushremote\". Fixes and tests for a minor bug in\n923d4a5ca4f (remote rename/remove: handle branch.<name>.pushRemote\nconfig values, 2020-01-27). See the preceding commit for why this does\n& doesn't matter.\n\nWhile we're at it let's also test that we handle the \"*.pushDefault\"\nkey correctly. The code to handle that was added in\nb3fd6cbf294 (remote rename/remove: gently handle remote.pushDefault\nconfig, 2020-02-01) and does the right thing, but nothing tested that\nwe wrote out the canonical camel-cased form.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/remote.c  | 2 +-\n t/t5505-remote.sh | 2 ++\n 2 files changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex f286ae97538..717b662d455 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -746,7 +746,7 @@ static int mv(int argc, const char **argv)\n \t\t}\n \t\tif (info->push_remote_name && !strcmp(info->push_remote_name, rename.old_name)) {\n \t\t\tstrbuf_reset(&buf);\n-\t\t\tstrbuf_addf(&buf, \"branch.%s.pushremote\", item->string);\n+\t\t\tstrbuf_addf(&buf, \"branch.%s.pushRemote\", item->string);\n \t\t\tgit_config_set(buf.buf, rename.new_name);\n \t\t}\n \t}\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 2a7b5cd00a0..34fc3fa421f 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -757,6 +757,7 @@ test_expect_success 'rename a remote' '\n \t\tcd four &&\n \t\tgit config branch.main.pushRemote origin &&\n \t\tgit remote rename origin upstream &&\n+\t\tgrep \"pushRemote\" .git/config &&\n \t\ttest -z \"$(git for-each-ref refs/remotes/origin)\" &&\n \t\ttest \"$(git symbolic-ref refs/remotes/upstream/HEAD)\" = \"refs/remotes/upstream/main\" &&\n \t\ttest \"$(git rev-parse upstream/main)\" = \"$(git rev-parse main)\" &&\n@@ -773,6 +774,7 @@ test_expect_success 'rename a remote renames repo remote.pushDefault' '\n \t\tcd four.1 &&\n \t\tgit config remote.pushDefault origin &&\n \t\tgit remote rename origin upstream &&\n+\t\tgrep pushDefault .git/config &&\n \t\ttest \"$(git config --local remote.pushDefault)\" = \"upstream\"\n \t)\n '\n-- \n2.30.0.284.gd98b1dd5eaa7\n\n"},{"id":"417779","messageId":"xmqq1rd4yhms.fsf@gitster.g","threadId":"55206","inReplyTo":"20210225012117.17331-1-avarab@gmail.com","subject":"Re: [PATCH 1/2] remote: add camel-cased *.tagOpt key, like clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-25T03:02:19Z","receivedAt":"2021-02-25T03:03:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> Change \"git remote add\" so that it adds a *.tagOpt key, and not the\n> lower-cased *.tagopt on \"git remote add --no-tags\", just as \"git clone\n> --no-tags\" would do.\n>\n> This doesn't matter for anything that reads the config. It's just\n> prettier if we write config keys in their documented camelCase form to\n> user-readable config files.\n>\n> When I added support for \"clone -no-tags\" in 0dab2468ee5 (clone: add a\n> --no-tags option to clone without tags, 2017-04-26) I made it use\n> the *.tagOpt form, but the older \"git remote add\" added in\n> 111fb858654 (remote add: add a --[no-]tags option, 2010-04-20) has\n> been using *.tagopt all this time.\n>\n> It's easy enough to add a test for this, so let's do that. We can't\n> use \"git config -l\" there, because it'll normalize the keys to their\n> lower-cased form. Let's add the test for \"git clone\" too for good\n> measure, not just to the \"git remote\" codepath we're fixing.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>\n> I also noticed that we write e.g. init.objectformat instead of\n> init.objectFormat, and core.logallrefupdates etc. If anyone's got an\n> even even worse case of OCD there's an interesting #leftoverbits\n> project there of scouring the code for more cases of this sort of\n> thing...\n>\n>  builtin/remote.c         | 2 +-\n>  t/t5505-remote.sh        | 1 +\n>  t/t5612-clone-refspec.sh | 1 +\n>  3 files changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/remote.c b/builtin/remote.c\n> index d11a5589e49..f286ae97538 100644\n> --- a/builtin/remote.c\n> +++ b/builtin/remote.c\n> @@ -221,7 +221,7 @@ static int add(int argc, const char **argv)\n>  \n>  \tif (fetch_tags != TAGS_DEFAULT) {\n>  \t\tstrbuf_reset(&buf);\n> -\t\tstrbuf_addf(&buf, \"remote.%s.tagopt\", name);\n> +\t\tstrbuf_addf(&buf, \"remote.%s.tagOpt\", name);\n\nGood find.\n\nA general rule for a name used to refer to a configuration variable\nthe C code ought to be\n\n - if it is used to match what the system gave us, make sure we use\n   all lowercase for the first and the last component and match with\n   strcmp(), not with strcasecmp().\n\n - if it is used to update, make sure we use the canonical spelling,\n   if only for the documentation value.\n\n> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\n> index 045398b94e6..2a7b5cd00a0 100755\n> --- a/t/t5505-remote.sh\n> +++ b/t/t5505-remote.sh\n> @@ -594,6 +594,7 @@ test_expect_success 'add --no-tags' '\n>  \t\tcd add-no-tags &&\n>  \t\tgit init &&\n>  \t\tgit remote add -f --no-tags origin ../one &&\n> +\t\tgrep tagOpt .git/config &&\n>  \t\tgit tag -l some-tag >../test/output &&\n>  \t\tgit tag -l foobar-tag >../test/output &&\n>  \t\tgit config remote.origin.tagopt >>../test/output\n> diff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh\n> index 6a6af7449ca..3126cfd7e9d 100755\n> --- a/t/t5612-clone-refspec.sh\n> +++ b/t/t5612-clone-refspec.sh\n> @@ -97,6 +97,7 @@ test_expect_success 'by default no tags will be kept updated' '\n>  test_expect_success 'clone with --no-tags' '\n>  \t(\n>  \t\tcd dir_all_no_tags &&\n> +\t\tgrep tagOpt .git/config &&\n>  \t\tgit fetch &&\n>  \t\tgit for-each-ref refs/tags >../actual\n>  \t) &&\n"},{"id":"417780","messageId":"xmqqwnuwx2ea.fsf@gitster.g","threadId":"55206","inReplyTo":"20210225012117.17331-1-avarab@gmail.com","subject":"Re: [PATCH 1/2] remote: add camel-cased *.tagOpt key, like clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-25T03:16:45Z","receivedAt":"2021-02-25T03:17:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> It's easy enough to add a test for this, so let's do that. We can't\n> use \"git config -l\" there, because it'll normalize the keys to their\n> lower-cased form.\n\nI wondered if we want \"git config -l --preserve-case\" or something\nlike that, but an extra grep for \"tagOpt\" would be sufficient in a\nsimple test like these that are unlikely to have unrelated tagOpt\ndefined in the file.  More importantly, I am starting to doubt if\nthis should even be tested.\n\nIf there were existing \"section.varname\" variable definition and we\nask\n\n\tgit_config_set(\"section.varName\", \"newvalue\");\n\nwe may end up with \"[section] varname = newvalue\", and that is\nperfectly OK, I would think, because the first and the last\ncomponent of the configuration variable names are defined to be case\ninsensitive, and here may be \"[Section] varname = oldvalue\" in the\nconfiguration file before we try to set it, and the implementation\nis free to replace \"oldvalue\" with \"newvalue\", instead of first\nremoving \"[Section] varname = oldvalue\" and then adding a new\n\"[section] varName = newvalue\" (after all, there may be variables\nother than \"varname\" in the section, and the existing \"[Section]\"\nheader may need to be kept for the remaining variables while we futz\nwith the varname or varName).\n\nWhich means that while we do want to spell the names in our source\ncode correctly (i.e. \"tagOpt\", not \"tagopt\") when we tell which\nvariable we want to get modified to the git_config_set() function,\nwe should not care how exactly git_config_set() chooses to spell the\nvariable in the resulting configuration file, no?\n\nSo, ...\n\n> diff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh\n> index 6a6af7449ca..3126cfd7e9d 100755\n> --- a/t/t5612-clone-refspec.sh\n> +++ b/t/t5612-clone-refspec.sh\n> @@ -97,6 +97,7 @@ test_expect_success 'by default no tags will be kept updated' '\n>  test_expect_success 'clone with --no-tags' '\n>  \t(\n>  \t\tcd dir_all_no_tags &&\n> +\t\tgrep tagOpt .git/config &&\n>  \t\tgit fetch &&\n>  \t\tgit for-each-ref refs/tags >../actual\n\n...as long as \"git config remote.origin.tagopt\" yields what we\nexpect, we should be OK, I would think.  Insisting that the variable\nname is kept by git_config_set() API may be expecting too much.\n\n>  \t) &&\n"},{"id":"417781","messageId":"xmqqsg5kx2ci.fsf@gitster.g","threadId":"55206","inReplyTo":"20210225012117.17331-2-avarab@gmail.com","subject":"Re: [PATCH 2/2] remote: write camel-cased *.pushRemote on rename","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-25T03:17:49Z","receivedAt":"2021-02-25T03:18:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> When a remote is renamed don't change the canonical \"*.pushRemote\"\n> form to \"*.pushremote\". Fixes and tests for a minor bug in\n> 923d4a5ca4f (remote rename/remove: handle branch.<name>.pushRemote\n> config values, 2020-01-27). See the preceding commit for why this does\n> & doesn't matter.\n>\n> While we're at it let's also test that we handle the \"*.pushDefault\"\n> key correctly. The code to handle that was added in\n> b3fd6cbf294 (remote rename/remove: gently handle remote.pushDefault\n> config, 2020-02-01) and does the right thing, but nothing tested that\n> we wrote out the canonical camel-cased form.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  builtin/remote.c  | 2 +-\n>  t/t5505-remote.sh | 2 ++\n>  2 files changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/remote.c b/builtin/remote.c\n> index f286ae97538..717b662d455 100644\n> --- a/builtin/remote.c\n> +++ b/builtin/remote.c\n> @@ -746,7 +746,7 @@ static int mv(int argc, const char **argv)\n>  \t\t}\n>  \t\tif (info->push_remote_name && !strcmp(info->push_remote_name, rename.old_name)) {\n>  \t\t\tstrbuf_reset(&buf);\n> -\t\t\tstrbuf_addf(&buf, \"branch.%s.pushremote\", item->string);\n> +\t\t\tstrbuf_addf(&buf, \"branch.%s.pushRemote\", item->string);\n>  \t\t\tgit_config_set(buf.buf, rename.new_name);\n>  \t\t}\n>  \t}\n> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\n> index 2a7b5cd00a0..34fc3fa421f 100755\n> --- a/t/t5505-remote.sh\n> +++ b/t/t5505-remote.sh\n> @@ -757,6 +757,7 @@ test_expect_success 'rename a remote' '\n>  \t\tcd four &&\n>  \t\tgit config branch.main.pushRemote origin &&\n>  \t\tgit remote rename origin upstream &&\n> +\t\tgrep \"pushRemote\" .git/config &&\n>  \t\ttest -z \"$(git for-each-ref refs/remotes/origin)\" &&\n>  \t\ttest \"$(git symbolic-ref refs/remotes/upstream/HEAD)\" = \"refs/remotes/upstream/main\" &&\n>  \t\ttest \"$(git rev-parse upstream/main)\" = \"$(git rev-parse main)\" &&\n> @@ -773,6 +774,7 @@ test_expect_success 'rename a remote renames repo remote.pushDefault' '\n>  \t\tcd four.1 &&\n>  \t\tgit config remote.pushDefault origin &&\n>  \t\tgit remote rename origin upstream &&\n> +\t\tgrep pushDefault .git/config &&\n>  \t\ttest \"$(git config --local remote.pushDefault)\" = \"upstream\"\n>  \t)\n>  '\n\nAgain, good find, but I am not sure if we want the test to be so\nstrict.  Besides, quoting one and not quoting the other in the same\npatch looks the test is also being inconsistent ;-).\n\nThanks.\n"},{"id":"417839","messageId":"87wnuw6iaw.fsf@evledraar.gmail.com","threadId":"55206","inReplyTo":"xmqqwnuwx2ea.fsf@gitster.g","subject":"Re: [PATCH 1/2] remote: add camel-cased *.tagOpt key, like clone","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-25T19:47:35Z","receivedAt":"2021-02-25T19:50:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Feb 25 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n>\n>> It's easy enough to add a test for this, so let's do that. We can't\n>> use \"git config -l\" there, because it'll normalize the keys to their\n>> lower-cased form.\n>\n> I wondered if we want \"git config -l --preserve-case\" or something\n> like that, but an extra grep for \"tagOpt\" would be sufficient in a\n> simple test like these that are unlikely to have unrelated tagOpt\n> defined in the file.  More importantly, I am starting to doubt if\n> this should even be tested.\n>\n> If there were existing \"section.varname\" variable definition and we\n> ask\n>\n> \tgit_config_set(\"section.varName\", \"newvalue\");\n>\n> we may end up with \"[section] varname = newvalue\", and that is\n> perfectly OK, I would think, because the first and the last\n> component of the configuration variable names are defined to be case\n> insensitive, and here may be \"[Section] varname = oldvalue\" in the\n> configuration file before we try to set it, and the implementation\n> is free to replace \"oldvalue\" with \"newvalue\", instead of first\n> removing \"[Section] varname = oldvalue\" and then adding a new\n> \"[section] varName = newvalue\" (after all, there may be variables\n> other than \"varname\" in the section, and the existing \"[Section]\"\n> header may need to be kept for the remaining variables while we futz\n> with the varname or varName).\n>\n> Which means that while we do want to spell the names in our source\n> code correctly (i.e. \"tagOpt\", not \"tagopt\") when we tell which\n> variable we want to get modified to the git_config_set() function,\n> we should not care how exactly git_config_set() chooses to spell the\n> variable in the resulting configuration file, no?\n>\n> So, ...\n\nYes, in general, but...\n\n>> diff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh\n>> index 6a6af7449ca..3126cfd7e9d 100755\n>> --- a/t/t5612-clone-refspec.sh\n>> +++ b/t/t5612-clone-refspec.sh\n>> @@ -97,6 +97,7 @@ test_expect_success 'by default no tags will be kept updated' '\n>>  test_expect_success 'clone with --no-tags' '\n>>  \t(\n>>  \t\tcd dir_all_no_tags &&\n>> +\t\tgrep tagOpt .git/config &&\n>>  \t\tgit fetch &&\n>>  \t\tgit for-each-ref refs/tags >../actual\n>\n> ...as long as \"git config remote.origin.tagopt\" yields what we\n> expect, we should be OK, I would think.  Insisting that the variable\n> name is kept by git_config_set() API may be expecting too much.\n>\n>>  \t) &&\n\n...the cases fixed in this series are not ones where we're possibly\nchanging an existing variable name, but where we're guaranteed to be\nwriting new values.\n\nWe are renaming a remote or otherwise moving variables around, if there\nwere existing values to contend with we'd have died earlier.\n\nI'm not quite sure what to make of this feedback in general. That you'd\nlike the bugfix but we shouldn't bother with a regression test, or that\nwe shouldn't bother with the fix at all?\n\nI admit this is getting to diminishing returns in testing, we're\nunlikely to break this, and if we do it's not such a big deal.\n\nBut I don't agree that we should feel free to munge user config files\nwithin the bound of valid config syntax when we edit these files for\nusers.\n\nWe already go out of our way to add values to existing sections, not add\nnew empty headings etc. (I believe the last major effort on that front\nwas from Johannes S. a while back).\n\nlikewise here, even though cmd.averylongvariablename is perfectly valid,\nit's much more user friendly if we write/edit it as\ncmd.AVeryLongVariableName.\n"},{"id":"417842","messageId":"xmqqzgzrudcn.fsf@gitster.g","threadId":"55206","inReplyTo":"87wnuw6iaw.fsf@evledraar.gmail.com","subject":"Re: [PATCH 1/2] remote: add camel-cased *.tagOpt key, like clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-25T20:00:40Z","receivedAt":"2021-02-25T20:01:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> I'm not quite sure what to make of this feedback in general. That you'd\n> like the bugfix but we shouldn't bother with a regression test, or that\n> we shouldn't bother with the fix at all?\n\nI like the style update to make the callers use the canonical case\n(even though they do not have to), but the test that inspects the\ncases in the resulting configuration file may be too strict.\n\n> But I don't agree that we should feel free to munge user config files\n> within the bound of valid config syntax when we edit these files for\n> users.\n\nI agree with your sentiment in principle.  I just wanted to make\nsure that future test writers agree with the principle, and also\nthat they understand there are cases where end-user input may not\nmatch the output (e.g. when running \"git config Vari.Able value\" to\nan existing configuration file that has \"[vari] ous = true\", it may\nbe less desirable to add \"[Vari] Able = value\" than to add to the\nexisting \"[Vari] section a new line \"Able = value\").\n\n\n"},{"id":"419631","messageId":"CAKPyHN33Aj63FuSMPJfz6W3F6EUt6ZLZzWaYS5puiKa13uHctw@mail.gmail.com","threadId":"55206","inReplyTo":"20210225012117.17331-2-avarab@gmail.com","subject":"Re: [PATCH 2/2] remote: write camel-cased *.pushRemote on rename","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2021-03-18T11:22:36Z","receivedAt":"2021-03-18T11:23:45Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Thu, Feb 25, 2021 at 2:21 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n>\n> When a remote is renamed don't change the canonical \"*.pushRemote\"\n> form to \"*.pushremote\". Fixes and tests for a minor bug in\n> 923d4a5ca4f (remote rename/remove: handle branch.<name>.pushRemote\n> config values, 2020-01-27). See the preceding commit for why this does\n> & doesn't matter.\n>\n> While we're at it let's also test that we handle the \"*.pushDefault\"\n> key correctly. The code to handle that was added in\n> b3fd6cbf294 (remote rename/remove: gently handle remote.pushDefault\n> config, 2020-02-01) and does the right thing, but nothing tested that\n> we wrote out the canonical camel-cased form.\n>\n\nFine with me.\n\nThanks.\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  builtin/remote.c  | 2 +-\n>  t/t5505-remote.sh | 2 ++\n>  2 files changed, 3 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/remote.c b/builtin/remote.c\n> index f286ae97538..717b662d455 100644\n> --- a/builtin/remote.c\n> +++ b/builtin/remote.c\n> @@ -746,7 +746,7 @@ static int mv(int argc, const char **argv)\n>                 }\n>                 if (info->push_remote_name && !strcmp(info->push_remote_name, rename.old_name)) {\n>                         strbuf_reset(&buf);\n> -                       strbuf_addf(&buf, \"branch.%s.pushremote\", item->string);\n> +                       strbuf_addf(&buf, \"branch.%s.pushRemote\", item->string);\n>                         git_config_set(buf.buf, rename.new_name);\n>                 }\n>         }\n> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\n> index 2a7b5cd00a0..34fc3fa421f 100755\n> --- a/t/t5505-remote.sh\n> +++ b/t/t5505-remote.sh\n> @@ -757,6 +757,7 @@ test_expect_success 'rename a remote' '\n>                 cd four &&\n>                 git config branch.main.pushRemote origin &&\n>                 git remote rename origin upstream &&\n> +               grep \"pushRemote\" .git/config &&\n>                 test -z \"$(git for-each-ref refs/remotes/origin)\" &&\n>                 test \"$(git symbolic-ref refs/remotes/upstream/HEAD)\" = \"refs/remotes/upstream/main\" &&\n>                 test \"$(git rev-parse upstream/main)\" = \"$(git rev-parse main)\" &&\n> @@ -773,6 +774,7 @@ test_expect_success 'rename a remote renames repo remote.pushDefault' '\n>                 cd four.1 &&\n>                 git config remote.pushDefault origin &&\n>                 git remote rename origin upstream &&\n> +               grep pushDefault .git/config &&\n>                 test \"$(git config --local remote.pushDefault)\" = \"upstream\"\n>         )\n>  '\n> --\n> 2.30.0.284.gd98b1dd5eaa7\n>\n"}]}