{"thread":{"id":"10333","subject":"[PATCH] When renaming config sections delete conflicting sections","startedAt":"2007-10-17T00:34:18Z","lastAt":"2007-10-17T10:37:26Z","messageCount":3,"participants":["Jonas Fonseca","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"56160","messageId":"20071017003418.GA11013@diku.dk","threadId":"10333","inReplyTo":null,"subject":"[PATCH] When renaming config sections delete conflicting sections","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2007-10-17T00:34:18Z","receivedAt":"2007-10-17T00:34:18Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"The old behavior of keeping config sections matching the new name caused\nproblems leading to warnings being emitted by git-remote when renaming\nbranches where information about tracked remote branches differed. To\nfix this any config sections that will conflict with the new name are\nremoved from the config file. Update test to check for this.\n\nSigned-off-by: Jonas Fonseca \n---\n config.c | 9 ++++++++-\n t/t1300-repo-config.sh | 17 +++++++++++++++++\n 2 files changed, 25 insertions(+), 1 deletions(-)\n\n This command sequence was causing problems for me:\n\n\tgit checkout -b test madcoder/next\n\tgit checkout -b test2 spearce/next\n\tgit branch -M test\n\n On top of spearce/next ...\n\ndiff --git a/config.c b/config.c\nindex dc3148d..578849a 100644\n--- a/config.c\n+++ b/config.c\n@@ -1013,6 +1013,14 @@ int git_config_rename_section(const char *old_name, const char *new_name)\n \t\t\t; /* do nothing */\n \t\tif (buf[i] == '[') {\n \t\t\t/* it's a section */\n+\t\t\tremove = 0;\n+\t\t\tif (new_name != NULL\n+\t\t\t && section_name_match (&buf[i+1], new_name)) {\n+\t\t\t\t/* Remove any existing occurences of the\n+\t\t\t\t * new section. */\n+\t\t\t\tremove = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (section_name_match (&buf[i+1], old_name)) {\n \t\t\t\tret++;\n \t\t\t\tif (new_name == NULL) {\n@@ -1026,7 +1034,6 @@ int git_config_rename_section(const char *old_name, const char *new_name)\n \t\t\t\t}\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tremove = 0;\n \t\t}\n \t\tif (remove)\n \t\t\tcontinue;\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 1d2bf2c..63b969e 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -419,6 +419,23 @@ EOF\n test_expect_success \"section was removed properly\" \\\n \t\"git diff -u expect .git/config\"\n \n+cat > .git/config << EOF\n+[branch \"new-name\"]\n+\tx = 1\n+[branch \"old-name\"]\n+\ty = 1\n+EOF\n+\n+test_expect_success \"rename and remove old section\" \\\n+\t'git config --rename-section branch.old-name branch.new-name'\n+\n+cat > expect << EOF\n+[branch \"new-name\"]\n+\ty = 1\n+EOF\n+\n+test_expect_success \"rename and remove succeeded\" \"git diff expect .git/config\"\n+\n rm .git/config\n \n cat > expect << EOF\n-- \n1.5.3.4.1206.g5f96-dirty\n\n-- \nJonas Fonseca\n"},{"id":"56164","messageId":"20071017005517.GJ13801@spearce.org","threadId":"10333","inReplyTo":"20071017003418.GA11013@diku.dk","subject":"Re: [PATCH] When renaming config sections delete conflicting sections","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-10-17T00:55:17Z","receivedAt":"2007-10-17T00:55:17Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jonas Fonseca wrote:\n> The old behavior of keeping config sections matching the new name caused\n> problems leading to warnings being emitted by git-remote when renaming\n> branches where information about tracked remote branches differed. To\n> fix this any config sections that will conflict with the new name are\n> removed from the config file. Update test to check for this.\n...\n> This command sequence was causing problems for me:\n> \n> \tgit checkout -b test madcoder/next\n> \tgit checkout -b test2 spearce/next\n> \tgit branch -M test\n\nOuch. But this may cause the user to lose what they might consider\nimportant settings relative to the old section named branch.test.\n\nI think in the case you mention above where you are doing a\n`branch -M` the user really does want the basic branch properties\nto be forced over (branch.$name.remote, branch.$name.merge) but\nthey probably do not want other branch properties to be removed\nor deleted. Or maybe they do.\n\nIts really hard to second guess the user's intent here. I think\nits too broad to whack an entire section when renaming. For example\ntoday lets say I do:\n\n\tcat >.git/config < wrote Tue, Oct 16, 2007:\n> Jonas Fonseca wrote:\n> > The old behavior of keeping config sections matching the new name caused\n> > problems leading to warnings being emitted by git-remote when renaming\n> > branches where information about tracked remote branches differed. To\n> > fix this any config sections that will conflict with the new name are\n> > removed from the config file. Update test to check for this.\n> ...\n> > This command sequence was causing problems for me:\n> > \n> > \tgit checkout -b test madcoder/next\n> > \tgit checkout -b test2 spearce/next\n> > \tgit branch -M test\n> \n> Ouch. But this may cause the user to lose what they might consider\n> important settings relative to the old section named branch.test.\n\nTrue, but to me the meaning of -M is \"I know what I am doing\".\n\n> I think in the case you mention above where you are doing a\n> `branch -M` the user really does want the basic branch properties\n> to be forced over (branch.$name.remote, branch.$name.merge) but\n> they probably do not want other branch properties to be removed\n> or deleted. Or maybe they do.\n\nYou never know, and sure if there is an option to gracefully avoid\nlossing this information that is the right approach, but I don't see how\nthis can be done in this situation. Besides currently only\nbranch.$name.mergoptions will be lost, hardly a problem.\n\n> Its really hard to second guess the user's intent here. I think\n> its too broad to whack an entire section when renaming. [...]\n>\n> So we don't blindly replace multi-valued keys just because the\n> user asked us to. I don't really see a section as being that much\n> different to warrant a potentially lossy behavior by default.\n\nBecause it makes sense in this situation and erroring out is a good\nchoice, but we are running out of git-branch options based on the letter\n'm'. And to me, -m is the default for renaming branches, and -M is a\nshortcut for doing a lot of other stuff with well-defined implications.\n\nPerhaps we can enable only this \"lossy behavior\" only for git-branch by\nadding an extra argument to git_config_rename_section? Then we can later\nadd a new option to git-config along the lines of --overwrite-section.\n\n-- \nJonas Fonseca\n"}]}