{"thread":{"id":"26442","subject":"[PATCH] mergetool: don't skip modify/remove conflicts","startedAt":"2011-02-08T03:08:38Z","lastAt":"2011-02-16T21:14:20Z","messageCount":13,"participants":["Martin von Zweigbergk","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"160675","messageId":"1297134518-4387-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26442","inReplyTo":null,"subject":"[PATCH] mergetool: don't skip modify/remove conflicts","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-08T03:08:38Z","receivedAt":"2011-02-08T03:08:38Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Since bb0a484 (mergetool: Skip autoresolved paths, 2010-08-17),\nmergetool uses different ways of figuring out the list of files with\nmerge conflicts depending on whether rerere is active. If rerere is\nactive, mergetool will use 'git rerere status' to list the files with\nremaining conflicts. However, the output from that command does not\nlist conflicts of types that rerere does not handle, such as\nmodify/remove conflicts.\n\nAnother problem with solely relying on the output from 'git rerere\nstatus' is that, for new conflicts that are not yet known to rerere,\nthe output from the command will list the files even after adding them\nto the index. This means that if the conflicts in some files have been\nresolved and 'git mergetool' is run again, it will ask the user\nsomething like the following for each of those files.\n\n file1: file does not need merging\n Continue merging other unresolved paths (y/n) ?\n\nSolve the first of these problems by replacing the call to 'git rerere\nstatus' with a call to the new 'git rerere remaining' that was\nintroduced in the previous commit. Solve the second problem by\nmodifying 'git rerere remaining' not to output files that have already\nbeen staged.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\nThis applies on top of jc/rerere-remaining.\n\nThis is my first patch that touches any C code and I haven't written\nanything in C for about 10 years. I hope it doesn't look too bad.\n\nI also have another version of this patch that doesn't modify 'git\nrerere remaining', but instead combines the output with the output\nfrom 'git ls-files'. Tell me if you think that is better.\n\nIs it correct to call \"yes\" with multiple arguments in the test script\nthe way I did?\n\n\n builtin/rerere.c     |   10 +++++++---\n git-mergetool.sh     |    2 +-\n rerere.c             |   31 ++++++++++++++++++++-----------\n rerere.h             |    3 +++\n t/t7610-mergetool.sh |   40 ++++++++++++++++++++++++++++++++++------\n 5 files changed, 65 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/rerere.c b/builtin/rerere.c\nindex 081fccc..9cbab27 100644\n--- a/builtin/rerere.c\n+++ b/builtin/rerere.c\n@@ -147,7 +147,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[0], \"clear\")) {\n \t\tfor (i = 0; i < merge_rr.nr; i++) {\n \t\t\tconst char *name = (const char *)merge_rr.items[i].util;\n-\t\t\tif (!name)\n+\t\t\tif (name == RERERE_UTIL_PUNTED || name == RERERE_UTIL_STAGED)\n \t\t\t\tcontinue;\n \t\t\tif (!has_rerere_resolution(name))\n \t\t\t\tunlink_rr_item(name);\n@@ -162,13 +162,17 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n \t\t}\n \telse if (!strcmp(argv[0], \"remaining\"))\n-\t\tfor (i = 0; i < merge_rr.nr; i++)\n+\t\tfor (i = 0; i < merge_rr.nr; i++) {\n+\t\t\tconst char *name = (const char *)merge_rr.items[i].util;\n+\t\t\tif (name == RERERE_UTIL_STAGED)\n+\t\t\t\tcontinue;\n \t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n+\t\t}\n \telse if (!strcmp(argv[0], \"diff\"))\n \t\tfor (i = 0; i < merge_rr.nr; i++) {\n \t\t\tconst char *path = merge_rr.items[i].string;\n \t\t\tconst char *name = (const char *)merge_rr.items[i].util;\n-\t\t\tif (!name)\n+\t\t\tif (name == RERERE_UTIL_PUNTED || name == RERERE_UTIL_STAGED)\n \t\t\t\tcontinue;\n \t\t\tdiff_two(rerere_path(name, \"preimage\"), path, path, path);\n \t\t}\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 2f8dc44..bacbda2 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -269,7 +269,7 @@ rerere=false\n files_to_merge() {\n     if test \"$rerere\" = true\n     then\n-\tgit rerere status\n+\tgit rerere remaining\n     else\n \tgit ls-files -u | sed -e 's/^[^\t]*\t//' | sort -u\n     fi\ndiff --git a/rerere.c b/rerere.c\nindex eb47f97..61cac54 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -7,6 +7,10 @@\n #include \"ll-merge.h\"\n #include \"attr.h\"\n \n+#define RERERE_UTIL_ELIGIBLE NULL\n+void *RERERE_UTIL_PUNTED = &RERERE_UTIL_PUNTED;\n+void *RERERE_UTIL_STAGED = &RERERE_UTIL_STAGED;\n+\n /* if rerere_enabled == -1, fall back to detection of .git/rr-cache */\n static int rerere_enabled = -1;\n \n@@ -352,18 +356,20 @@ static int find_conflict(struct string_list *conflict)\n \t\treturn error(\"Could not read index\");\n \n \t/*\n-\t * Collect paths with conflict, mark them with NULL (punted) or\n-\t * !NULL (eligible) in their ->util field.\n+\t * Collect paths with conflict, mark them according to type in\n+\t * their ->util field.\n \t */\n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct cache_entry *e = active_cache[i];\n \t\tstruct string_list_item *it;\n \n-\t\tif (!ce_stage(e))\n+\t\tif (!ce_stage(e)) {\n \t\t\tcontinue;\n+\t\t}\n \t\tit = string_list_insert(conflict, (const char *)e->name);\n-\t\tit->util = NULL;\n+\t\tit->util = RERERE_UTIL_PUNTED;\n \t\tif (ce_stage(e) == 1) {\n+\t\t\tit->util = RERERE_UTIL_STAGED;\n \t\t\tif (active_nr <= ++i)\n \t\t\t\tbreak;\n \t\t}\n@@ -377,7 +383,7 @@ static int find_conflict(struct string_list *conflict)\n \t\t\t    ce_same_name(e, e3) &&\n \t\t\t    S_ISREG(e2->ce_mode) &&\n \t\t\t    S_ISREG(e3->ce_mode))\n-\t\t\t\tit->util = (void *) 1;\n+\t\t\t\tit->util = RERERE_UTIL_ELIGIBLE;\n \t\t}\n \n \t\t/* Skip the entries with the same name */\n@@ -395,9 +401,10 @@ static void add_punted(struct string_list *merge_rr)\n \n \tfind_conflict(&conflict);\n \tfor (i = 0; i < conflict.nr; i++) {\n-\t\tif (conflict.items[i].util)\n+\t\tif (conflict.items[i].util == RERERE_UTIL_ELIGIBLE)\n \t\t\tcontinue;\n-\t\tstring_list_insert(merge_rr, conflict.items[i].string);\n+\t\tstring_list_insert(merge_rr, conflict.items[i].string)->util =\n+\t\t\tconflict.items[i].util;\n \t}\n }\n \n@@ -487,8 +494,9 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \n \tfor (i = 0; i < conflict.nr; i++) {\n \t\tconst char *path = conflict.items[i].string;\n-\t\tif (!conflict.items[i].util)\n-\t\t\tcontinue; /* punted */\n+\t\tif (conflict.items[i].util == RERERE_UTIL_PUNTED ||\n+\t\t\tconflict.items[i].util == RERERE_UTIL_STAGED)\n+\t\t\tcontinue;\n \t\tif (!string_list_has_string(rr, path)) {\n \t\t\tunsigned char sha1[20];\n \t\t\tchar *hex;\n@@ -648,8 +656,9 @@ int rerere_forget(const char **pathspec)\n \tfind_conflict(&conflict);\n \tfor (i = 0; i < conflict.nr; i++) {\n \t\tstruct string_list_item *it = &conflict.items[i];\n-\t\tif (!conflict.items[i].util)\n-\t\t\tcontinue; /* punted */\n+\t\tif (conflict.items[i].util == RERERE_UTIL_PUNTED ||\n+\t\t\tconflict.items[i].util == RERERE_UTIL_STAGED)\n+\t\t\tcontinue;\n \t\tif (!match_pathspec(pathspec, it->string, strlen(it->string),\n \t\t\t\t    0, NULL))\n \t\t\tcontinue;\ndiff --git a/rerere.h b/rerere.h\nindex eaa9004..107a2bc 100644\n--- a/rerere.h\n+++ b/rerere.h\n@@ -6,6 +6,9 @@\n #define RERERE_AUTOUPDATE   01\n #define RERERE_NOAUTOUPDATE 02\n \n+extern void *RERERE_UTIL_PUNTED;\n+extern void *RERERE_UTIL_STAGED;\n+\n extern int setup_rerere(struct string_list *, int);\n extern int rerere(int);\n extern const char *rerere_path(const char *hex, const char *file);\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex d78bdec..dc838c9 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -16,23 +16,33 @@ Testing basic merge tool invocation'\n test_expect_success 'setup' '\n     git config rerere.enabled true &&\n     echo master >file1 &&\n+    echo master file11 >file11 &&\n+    echo master file12 >file12 &&\n+    echo master file13 >file13 &&\n+    echo master file14 >file14 &&\n     mkdir subdir &&\n     echo master sub >subdir/file3 &&\n-    git add file1 subdir/file3 &&\n-    git commit -m \"added file1\" &&\n+    git add file1 file1[1-4] subdir/file3 &&\n+    git commit -m \"add initial versions\" &&\n \n     git checkout -b branch1 master &&\n     echo branch1 change >file1 &&\n     echo branch1 newfile >file2 &&\n+    echo branch1 change file11 >file11 &&\n+    echo branch1 change file13 >file13 &&\n     echo branch1 sub >subdir/file3 &&\n-    git add file1 file2 subdir/file3 &&\n+    git add file1 file11 file13 file2 subdir/file3 &&\n+    git rm file12 &&\n     git commit -m \"branch1 changes\" &&\n \n     git checkout master &&\n     echo master updated >file1 &&\n     echo master new >file2 &&\n+    echo master updated file12 >file12 &&\n+    echo master updated file14 >file14 &&\n     echo master new sub >subdir/file3 &&\n-    git add file1 file2 subdir/file3 &&\n+    git add file1 file12 file14 file2 subdir/file3 &&\n+    git rm file11 &&\n     git commit -m \"master updates\" &&\n \n     git config merge.tool mytool &&\n@@ -46,6 +56,8 @@ test_expect_success 'custom mergetool' '\n     ( yes \"\" | git mergetool file1 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool file2 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file11 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n     test \"$(cat file1)\" = \"master updated\" &&\n     test \"$(cat file2)\" = \"master new\" &&\n     test \"$(cat subdir/file3)\" = \"master new sub\" &&\n@@ -59,6 +71,8 @@ test_expect_success 'mergetool crlf' '\n     ( yes \"\" | git mergetool file1 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool file2 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file11 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n     test \"$(printf x | cat file1 -)\" = \"$(printf \"master updated\\r\\nx\")\" &&\n     test \"$(printf x | cat file2 -)\" = \"$(printf \"master new\\r\\nx\")\" &&\n     test \"$(printf x | cat subdir/file3 -)\" = \"$(printf \"master new sub\\r\\nx\")\" &&\n@@ -82,6 +96,8 @@ test_expect_success 'mergetool on file in parent dir' '\n \tcd subdir &&\n \t( yes \"\" | git mergetool ../file1 >/dev/null 2>&1 ) &&\n \t( yes \"\" | git mergetool ../file2 >/dev/null 2>&1 ) &&\n+\t( yes \"d\" | git mergetool ../file11 >/dev/null 2>&1 ) &&\n+\t( yes \"d\" | git mergetool ../file12 >/dev/null 2>&1 ) &&\n \ttest \"$(cat ../file1)\" = \"master updated\" &&\n \ttest \"$(cat ../file2)\" = \"master new\" &&\n \tgit commit -m \"branch1 resolved with mergetool - subdir\"\n@@ -92,6 +108,8 @@ test_expect_success 'mergetool skips autoresolved' '\n     git checkout -b test4 branch1 &&\n     test_must_fail git merge master &&\n     test -n \"$(git ls-files -u)\" &&\n+    ( yes \"d\" | git mergetool file11 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n     output=\"$(git mergetool --no-prompt)\" &&\n     test \"$output\" = \"No files need merging\" &&\n     git reset --hard\n@@ -102,13 +120,23 @@ test_expect_success 'mergetool merges all from subdir' '\n \tcd subdir &&\n \tgit config rerere.enabled false &&\n \ttest_must_fail git merge master &&\n-\tgit mergetool --no-prompt &&\n+\t( yes \"d\" \"d\" | git mergetool --no-prompt ) &&\n \ttest \"$(cat ../file1)\" = \"master updated\" &&\n \ttest \"$(cat ../file2)\" = \"master new\" &&\n \ttest \"$(cat file3)\" = \"master new sub\" &&\n-\tgit add ../file1 ../file2 file3 &&\n \tgit commit -m \"branch2 resolved by mergetool from subdir\"\n     )\n '\n \n+test_expect_success 'mergetool skips resolved paths when rerere is active' '\n+    git config rerere.enabled true &&\n+    rm -rf .git/rr-cache &&\n+    git checkout -b test5 branch1\n+    test_must_fail git merge master >/dev/null 2>&1 &&\n+    ( yes \"d\" \"d\" | git mergetool --no-prompt >/dev/null 2>&1 ) &&\n+    output=\"$(yes \"n\" | git mergetool --no-prompt)\" &&\n+    test \"$output\" = \"No files need merging\" &&\n+    git reset --hard\n+'\n+\n test_done\n-- \n1.7.4.rc2.33.g8a14f\n"},{"id":"160785","messageId":"7vzkq4opaw.fsf@alter.siamese.dyndns.org","threadId":"26442","inReplyTo":"1297134518-4387-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH] mergetool: don't skip modify/remove conflicts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-09T21:45:11Z","receivedAt":"2011-02-09T21:45:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n\n> diff --git a/rerere.h b/rerere.h\n> index eaa9004..107a2bc 100644\n> --- a/rerere.h\n> +++ b/rerere.h\n> @@ -6,6 +6,9 @@\n>  #define RERERE_AUTOUPDATE   01\n>  #define RERERE_NOAUTOUPDATE 02\n>  \n> +extern void *RERERE_UTIL_PUNTED;\n\nThis is for paths without three-stages, i.e. ones rerere won't help. \nNeeds some comment for these two symbols.\n\nLeading \"RERERE_\" is necessary for clarifying the namespace, PUNTED is\nnecessary because it defines what the symbols means, but why do we need\nUTIL in the name?  Drop it.\n\n> +extern void *RERERE_UTIL_STAGED;\n\nThis is for what kind?  The contents on the filesystem is ready to go,\nadded to the index, but still in MERGE_RR (i.e. \"git rerere\" not run yet)?\n\nIs the real problem that git-mergetool is not running rerere when it\nshould, I wonder...\n\n> diff --git a/rerere.c b/rerere.c\n> index eb47f97..61cac54 100644\n> --- a/rerere.c\n> +++ b/rerere.c\n> @@ -7,6 +7,10 @@\n>  #include \"ll-merge.h\"\n>  #include \"attr.h\"\n>  \n> +#define RERERE_UTIL_ELIGIBLE NULL\n> +void *RERERE_UTIL_PUNTED = &RERERE_UTIL_PUNTED;\n> +void *RERERE_UTIL_STAGED = &RERERE_UTIL_STAGED;\n> +\n\nSame for \"ELIGIBLE\"; describe what it means.\n\n>  \tfor (i = 0; i < active_nr; i++) {\n>  \t\tstruct cache_entry *e = active_cache[i];\n>  \t\tstruct string_list_item *it;\n>  \n> -\t\tif (!ce_stage(e))\n> +\t\tif (!ce_stage(e)) {\n>  \t\t\tcontinue;\n> +\t\t}\n\nUnnecessary change.\n\n>  \t\tit = string_list_insert(conflict, (const char *)e->name);\n> -\t\tit->util = NULL;\n> +\t\tit->util = RERERE_UTIL_PUNTED;\n>  \t\tif (ce_stage(e) == 1) {\n> +\t\t\tit->util = RERERE_UTIL_STAGED;\n\nHmm, I thought that you were taling about paths that the user\nhand-resolved and then ran \"git add\" on.  Why is this marked \"STAGED\"?\n\nEither your logic is wrong, or the name of the symbol is.\n\nIn any case, the original code is letting rerere handle both two-way\nmerge (stage #2 and #3 exist without state #1) and three-way merge (all\nthree stages exist) case, and changing only the body of this if statement\nsmells there is extremely fishy going on.\n\n> @@ -487,8 +494,9 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n>  \n>  \tfor (i = 0; i < conflict.nr; i++) {\n>  \t\tconst char *path = conflict.items[i].string;\n> -\t\tif (!conflict.items[i].util)\n> -\t\t\tcontinue; /* punted */\n> +\t\tif (conflict.items[i].util == RERERE_UTIL_PUNTED ||\n> +\t\t\tconflict.items[i].util == RERERE_UTIL_STAGED)\n> +\t\t\tcontinue;\n>  \t\tif (!string_list_has_string(rr, path)) {\n>  \t\t\tunsigned char sha1[20];\n>  \t\t\tchar *hex;\n> @@ -648,8 +656,9 @@ int rerere_forget(const char **pathspec)\n>  \tfind_conflict(&conflict);\n>  \tfor (i = 0; i < conflict.nr; i++) {\n>  \t\tstruct string_list_item *it = &conflict.items[i];\n> -\t\tif (!conflict.items[i].util)\n> -\t\t\tcontinue; /* punted */\n> +\t\tif (conflict.items[i].util == RERERE_UTIL_PUNTED ||\n> +\t\t\tconflict.items[i].util == RERERE_UTIL_STAGED)\n> +\t\t\tcontinue;\n\nThere are a few repetition of \"if it is marked with PUNTED or STAGED\"; can\nyou make it into a small helper function and give it a _meaningful_ name?\nWhat does it mean for an entry to be marked with either of these marks?\n\nThanks.\n"},{"id":"160873","messageId":"alpine.DEB.2.00.1102102130040.29809@debian","threadId":"26442","inReplyTo":"7vzkq4opaw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] mergetool: don't skip modify/remove conflicts","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-11T02:46:12Z","receivedAt":"2011-02-11T02:46:12Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Wed, 9 Feb 2011, Junio C Hamano wrote:\n\n> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n> \n> > +extern void *RERERE_UTIL_STAGED;\n> \n> This is for what kind?  The contents on the filesystem is ready to go,\n> added to the index, but still in MERGE_RR (i.e. \"git rerere\" not run yet)?\n\nCorrect.\n\n> Is the real problem that git-mergetool is not running rerere when it\n> should, I wonder...\n\nYou mean that maybe it should call rerere for files that are added to\nthe index?\n\n> >  \t\tit = string_list_insert(conflict, (const char *)e->name);\n> > -\t\tit->util = NULL;\n> > +\t\tit->util = RERERE_UTIL_PUNTED;\n> >  \t\tif (ce_stage(e) == 1) {\n> > +\t\t\tit->util = RERERE_UTIL_STAGED;\n> \n> Hmm, I thought that you were taling about paths that the user\n> hand-resolved and then ran \"git add\" on.  Why is this marked \"STAGED\"?\n\nVery good question. It shouldn't.\n\n> Either your logic is wrong, or the name of the symbol is.\n\nThe former. What I really wanted to check is if the file has been\nadded to the index. I am currently struggling with how to do this\ncorrectly. My inexperience with the git internals makes it a very slow\nprocess with a lot of guesses and trial and error.\n\n> > -\t\tif (!conflict.items[i].util)\n> > -\t\t\tcontinue; /* punted */\n> > +\t\tif (conflict.items[i].util == RERERE_UTIL_PUNTED ||\n> > +\t\t\tconflict.items[i].util == RERERE_UTIL_STAGED)\n> > +\t\t\tcontinue;\n> \n> There are a few repetition of \"if it is marked with PUNTED or STAGED\"; can\n> you make it into a small helper function and give it a _meaningful_ name?\n> What does it mean for an entry to be marked with either of these marks?\n\nThere is another big problem here. I had not realized that the util\nfield would be freed later (being spoilt by GCing languages, I guess)\nand that will of course fail if it is one of the two constants I\ndefined. I stupidly did not run the commands manually, but relied only\non the test cases, so I didn't notice the segfault at the end. Somehow\nall the test cases still pass.\n\nI think this means that anyone intending to run rerere should not\nbuild pu at the moment, right? I'm really sorry about the\ninconvenience :-(. \n\n/Martin\n"},{"id":"160973","messageId":"1297570198-21103-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26442","inReplyTo":"1297134518-4387-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH v2 0/2] mergetool: don't skip modify/remove conflicts","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-13T04:09:56Z","receivedAt":"2011-02-13T04:09:56Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Trying again. I hope it looks better this time. The second patch could\nbe squashed with the first one.\n\nThis applies on top of jc/rerere-remaining.\n\nMartin von Zweigbergk (2):\n  mergetool: don't skip modify/remove conflicts\n  rerere: factor out common conflict search code\n\n builtin/rerere.c     |   21 +++++-----\n git-mergetool.sh     |    2 +-\n rerere.c             |  109 +++++++++++++++++++++++++++++---------------------\n rerere.h             |    8 ++++\n t/t7610-mergetool.sh |   40 +++++++++++++++---\n 5 files changed, 117 insertions(+), 63 deletions(-)\n\n-- \n1.7.4.rc2.33.g8a14f\n"},{"id":"160974","messageId":"1297570198-21103-2-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26442","inReplyTo":"1297570198-21103-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH v2 1/2] mergetool: don't skip modify/remove conflicts","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-13T04:09:57Z","receivedAt":"2011-02-13T04:09:57Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Since bb0a484 (mergetool: Skip autoresolved paths, 2010-08-17),\nmergetool uses different ways of figuring out the list of files with\nmerge conflicts depending on whether rerere is active. If rerere is\nactive, mergetool will use 'git rerere status' to list the files with\nremaining conflicts. However, the output from that command does not\nlist conflicts of types that rerere does not handle, such as\nmodify/remove conflicts.\n\nAnother problem with solely relying on the output from 'git rerere\nstatus' is that, for new conflicts that are not yet known to rerere,\nthe output from the command will list the files even after adding them\nto the index. This means that if the conflicts in some files have been\nresolved and 'git mergetool' is run again, it will ask the user\nsomething like the following for each of those files.\n\n file1: file does not need merging\n Continue merging other unresolved paths (y/n) ?\n\nSolve the first of these problems by replacing the call to 'git rerere\nstatus' with a call to the new 'git rerere remaining' that was\nintroduced in the previous commit. Solve the second problem by\nmodifying 'git rerere remaining' not to output files that have already\nbeen staged.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n\nIdeally, I would have liked to just remove the resolved elements from\nthe string_list, but since there is no function that drops an element,\nI marked it by pointing its util field to a special address. Is this a\ngood way of doing it or can/should I manually unlink the element from\nthe list instead?\n\n builtin/rerere.c     |   21 +++++++++++----------\n git-mergetool.sh     |    2 +-\n rerere.c             |   24 ++++++++++++++++++++----\n rerere.h             |    8 ++++++++\n t/t7610-mergetool.sh |   40 ++++++++++++++++++++++++++++++++++------\n 5 files changed, 74 insertions(+), 21 deletions(-)\n\ndiff --git a/builtin/rerere.c b/builtin/rerere.c\nindex 081fccc..7b9fe18 100644\n--- a/builtin/rerere.c\n+++ b/builtin/rerere.c\n@@ -147,8 +147,6 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[0], \"clear\")) {\n \t\tfor (i = 0; i < merge_rr.nr; i++) {\n \t\t\tconst char *name = (const char *)merge_rr.items[i].util;\n-\t\t\tif (!name)\n-\t\t\t\tcontinue;\n \t\t\tif (!has_rerere_resolution(name))\n \t\t\t\tunlink_rr_item(name);\n \t\t}\n@@ -157,19 +155,22 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \t\tgarbage_collect(&merge_rr);\n \telse if (!strcmp(argv[0], \"status\"))\n \t\tfor (i = 0; i < merge_rr.nr; i++) {\n-\t\t\tif (!merge_rr.items[i].util)\n-\t\t\t\tcontinue;\n \t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n \t\t}\n-\telse if (!strcmp(argv[0], \"remaining\"))\n-\t\tfor (i = 0; i < merge_rr.nr; i++)\n-\t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n-\telse if (!strcmp(argv[0], \"diff\"))\n+\telse if (!strcmp(argv[0], \"remaining\")) {\n+\t\trerere_remaining(&merge_rr);\n+\t\tfor (i = 0; i < merge_rr.nr; i++) {\n+\t\t\tif (merge_rr.items[i].util != RERERE_RESOLVED)\n+\t\t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n+\t\t\telse\n+\t\t\t\t/* prepare for later call to\n+\t\t\t\t * string_list_clear() */\n+\t\t\t\tmerge_rr.items[i].util = NULL;\n+\t\t}\n+\t} else if (!strcmp(argv[0], \"diff\"))\n \t\tfor (i = 0; i < merge_rr.nr; i++) {\n \t\t\tconst char *path = merge_rr.items[i].string;\n \t\t\tconst char *name = (const char *)merge_rr.items[i].util;\n-\t\t\tif (!name)\n-\t\t\t\tcontinue;\n \t\t\tdiff_two(rerere_path(name, \"preimage\"), path, path, path);\n \t\t}\n \telse\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 2f8dc44..bacbda2 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -269,7 +269,7 @@ rerere=false\n files_to_merge() {\n     if test \"$rerere\" = true\n     then\n-\tgit rerere status\n+\tgit rerere remaining\n     else\n \tgit ls-files -u | sed -e 's/^[^\t]*\t//' | sort -u\n     fi\ndiff --git a/rerere.c b/rerere.c\nindex eb47f97..e5bccd5 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -7,6 +7,8 @@\n #include \"ll-merge.h\"\n #include \"attr.h\"\n \n+void *RERERE_RESOLVED = &RERERE_RESOLVED;\n+\n /* if rerere_enabled == -1, fall back to detection of .git/rr-cache */\n static int rerere_enabled = -1;\n \n@@ -388,16 +390,31 @@ static int find_conflict(struct string_list *conflict)\n \treturn 0;\n }\n \n-static void add_punted(struct string_list *merge_rr)\n+void rerere_remaining(struct string_list *merge_rr)\n {\n \tint i;\n \tstruct string_list conflict = STRING_LIST_INIT_DUP;\n \n+\t/* Add punted paths */\n \tfind_conflict(&conflict);\n \tfor (i = 0; i < conflict.nr; i++) {\n-\t\tif (conflict.items[i].util)\n+\t\tif (!conflict.items[i].util)\n+\t\t\tstring_list_insert(merge_rr, conflict.items[i].string);\n+\t}\n+\n+\t/* Mark already resolved paths */\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tstruct cache_entry *e = active_cache[i];\n+\n+\t\tif (!ce_stage(e)) {\n+\t\t\tstruct string_list_item *it;\n+\t\t\tit = string_list_lookup(merge_rr, (const char *)e->name);\n+\t\t\tif (it != NULL) {\n+\t\t\t\tfree(it->util);\n+\t\t\t\tit->util = RERERE_RESOLVED;\n+\t\t\t}\n \t\t\tcontinue;\n-\t\tstring_list_insert(merge_rr, conflict.items[i].string);\n+\t\t}\n \t}\n }\n \n@@ -592,7 +609,6 @@ int setup_rerere(struct string_list *merge_rr, int flags)\n \tfd = hold_lock_file_for_update(&write_lock, merge_rr_path,\n \t\t\t\t       LOCK_DIE_ON_ERROR);\n \tread_rr(merge_rr);\n-\tadd_punted(merge_rr);\n \treturn fd;\n }\n \ndiff --git a/rerere.h b/rerere.h\nindex eaa9004..ea2618b 100644\n--- a/rerere.h\n+++ b/rerere.h\n@@ -6,11 +6,19 @@\n #define RERERE_AUTOUPDATE   01\n #define RERERE_NOAUTOUPDATE 02\n \n+/*\n+ * Marks paths that have been hand-resolved and added to the\n+ * index. Set in the util field of such paths after calling\n+ * rerere_remaining.\n+ */\n+extern void *RERERE_RESOLVED;\n+\n extern int setup_rerere(struct string_list *, int);\n extern int rerere(int);\n extern const char *rerere_path(const char *hex, const char *file);\n extern int has_rerere_resolution(const char *hex);\n extern int rerere_forget(const char **);\n+extern void rerere_remaining(struct string_list *);\n \n #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, \"rerere-autoupdate\", (v), \\\n \t\"update the index with reused conflict resolution if possible\")\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex d78bdec..dc838c9 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -16,23 +16,33 @@ Testing basic merge tool invocation'\n test_expect_success 'setup' '\n     git config rerere.enabled true &&\n     echo master >file1 &&\n+    echo master file11 >file11 &&\n+    echo master file12 >file12 &&\n+    echo master file13 >file13 &&\n+    echo master file14 >file14 &&\n     mkdir subdir &&\n     echo master sub >subdir/file3 &&\n-    git add file1 subdir/file3 &&\n-    git commit -m \"added file1\" &&\n+    git add file1 file1[1-4] subdir/file3 &&\n+    git commit -m \"add initial versions\" &&\n \n     git checkout -b branch1 master &&\n     echo branch1 change >file1 &&\n     echo branch1 newfile >file2 &&\n+    echo branch1 change file11 >file11 &&\n+    echo branch1 change file13 >file13 &&\n     echo branch1 sub >subdir/file3 &&\n-    git add file1 file2 subdir/file3 &&\n+    git add file1 file11 file13 file2 subdir/file3 &&\n+    git rm file12 &&\n     git commit -m \"branch1 changes\" &&\n \n     git checkout master &&\n     echo master updated >file1 &&\n     echo master new >file2 &&\n+    echo master updated file12 >file12 &&\n+    echo master updated file14 >file14 &&\n     echo master new sub >subdir/file3 &&\n-    git add file1 file2 subdir/file3 &&\n+    git add file1 file12 file14 file2 subdir/file3 &&\n+    git rm file11 &&\n     git commit -m \"master updates\" &&\n \n     git config merge.tool mytool &&\n@@ -46,6 +56,8 @@ test_expect_success 'custom mergetool' '\n     ( yes \"\" | git mergetool file1 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool file2 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file11 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n     test \"$(cat file1)\" = \"master updated\" &&\n     test \"$(cat file2)\" = \"master new\" &&\n     test \"$(cat subdir/file3)\" = \"master new sub\" &&\n@@ -59,6 +71,8 @@ test_expect_success 'mergetool crlf' '\n     ( yes \"\" | git mergetool file1 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool file2 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file11 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n     test \"$(printf x | cat file1 -)\" = \"$(printf \"master updated\\r\\nx\")\" &&\n     test \"$(printf x | cat file2 -)\" = \"$(printf \"master new\\r\\nx\")\" &&\n     test \"$(printf x | cat subdir/file3 -)\" = \"$(printf \"master new sub\\r\\nx\")\" &&\n@@ -82,6 +96,8 @@ test_expect_success 'mergetool on file in parent dir' '\n \tcd subdir &&\n \t( yes \"\" | git mergetool ../file1 >/dev/null 2>&1 ) &&\n \t( yes \"\" | git mergetool ../file2 >/dev/null 2>&1 ) &&\n+\t( yes \"d\" | git mergetool ../file11 >/dev/null 2>&1 ) &&\n+\t( yes \"d\" | git mergetool ../file12 >/dev/null 2>&1 ) &&\n \ttest \"$(cat ../file1)\" = \"master updated\" &&\n \ttest \"$(cat ../file2)\" = \"master new\" &&\n \tgit commit -m \"branch1 resolved with mergetool - subdir\"\n@@ -92,6 +108,8 @@ test_expect_success 'mergetool skips autoresolved' '\n     git checkout -b test4 branch1 &&\n     test_must_fail git merge master &&\n     test -n \"$(git ls-files -u)\" &&\n+    ( yes \"d\" | git mergetool file11 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n     output=\"$(git mergetool --no-prompt)\" &&\n     test \"$output\" = \"No files need merging\" &&\n     git reset --hard\n@@ -102,13 +120,23 @@ test_expect_success 'mergetool merges all from subdir' '\n \tcd subdir &&\n \tgit config rerere.enabled false &&\n \ttest_must_fail git merge master &&\n-\tgit mergetool --no-prompt &&\n+\t( yes \"d\" \"d\" | git mergetool --no-prompt ) &&\n \ttest \"$(cat ../file1)\" = \"master updated\" &&\n \ttest \"$(cat ../file2)\" = \"master new\" &&\n \ttest \"$(cat file3)\" = \"master new sub\" &&\n-\tgit add ../file1 ../file2 file3 &&\n \tgit commit -m \"branch2 resolved by mergetool from subdir\"\n     )\n '\n \n+test_expect_success 'mergetool skips resolved paths when rerere is active' '\n+    git config rerere.enabled true &&\n+    rm -rf .git/rr-cache &&\n+    git checkout -b test5 branch1\n+    test_must_fail git merge master >/dev/null 2>&1 &&\n+    ( yes \"d\" \"d\" | git mergetool --no-prompt >/dev/null 2>&1 ) &&\n+    output=\"$(yes \"n\" | git mergetool --no-prompt)\" &&\n+    test \"$output\" = \"No files need merging\" &&\n+    git reset --hard\n+'\n+\n test_done\n-- \n1.7.4.rc2.33.g8a14f\n"},{"id":"160975","messageId":"1297570198-21103-3-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26442","inReplyTo":"1297570198-21103-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH v2 2/2] rerere: factor out common conflict search code","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-13T04:09:58Z","receivedAt":"2011-02-13T04:09:58Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"A little bit of the code in the new rerere_remaining function is\nshared with find_conflicts. Factor this out into a new function that\nis called from the two mentioned functions. Apart from reducing code\nduplication, it also means that the callers of find_conflict do not\nneed to handle the case when the util pointer is NULL, because that is\nonly set in rerere_remaining now.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n rerere.c |  105 +++++++++++++++++++++++++++++++------------------------------\n rerere.h |    2 +-\n 2 files changed, 54 insertions(+), 53 deletions(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex e5bccd5..d14fd9a 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -7,6 +7,9 @@\n #include \"ll-merge.h\"\n #include \"attr.h\"\n \n+#define RESOLVED 0\n+#define PUNTED 1\n+#define THREE_STAGED 2\n void *RERERE_RESOLVED = &RERERE_RESOLVED;\n \n /* if rerere_enabled == -1, fall back to detection of .git/rr-cache */\n@@ -347,75 +350,79 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \treturn hunk_no;\n }\n \n+\n+static int check_one_conflict(int i, int *type)\n+{\n+\tstruct cache_entry *e = active_cache[i];\n+\tstruct string_list_item *it;\n+\n+\tif (!ce_stage(e)) {\n+\t\t*type = RESOLVED;\n+\t\treturn i + 1;\n+\t}\n+\n+\t*type = PUNTED;\n+\tif (ce_stage(e) == 1) {\n+\t\tif (active_nr <= ++i)\n+\t\t\treturn i + 1;\n+\t}\n+\n+\t/* Only handle regular files with both stages #2 and #3 */\n+\tif (i + 1 < active_nr) {\n+\t\tstruct cache_entry *e2 = active_cache[i];\n+\t\tstruct cache_entry *e3 = active_cache[i + 1];\n+\t\tif (ce_stage(e2) == 2 &&\n+\t\t    ce_stage(e3) == 3 &&\n+\t\t    ce_same_name(e, e3) &&\n+\t\t    S_ISREG(e2->ce_mode) &&\n+\t\t    S_ISREG(e3->ce_mode))\n+\t\t\t*type = THREE_STAGED;\n+\t}\n+\n+\t/* Skip the entries with the same name */\n+\twhile (i < active_nr && ce_same_name(e, active_cache[i]))\n+\t\ti++;\n+\treturn i;\n+}\n+\n static int find_conflict(struct string_list *conflict)\n {\n \tint i;\n \tif (read_cache() < 0)\n \t\treturn error(\"Could not read index\");\n \n-\t/*\n-\t * Collect paths with conflict, mark them with NULL (punted) or\n-\t * !NULL (eligible) in their ->util field.\n-\t */\n-\tfor (i = 0; i < active_nr; i++) {\n+\tfor (i = 0; i < active_nr;) {\n+\t\tint conflict_type;\n \t\tstruct cache_entry *e = active_cache[i];\n-\t\tstruct string_list_item *it;\n-\n-\t\tif (!ce_stage(e))\n-\t\t\tcontinue;\n-\t\tit = string_list_insert(conflict, (const char *)e->name);\n-\t\tit->util = NULL;\n-\t\tif (ce_stage(e) == 1) {\n-\t\t\tif (active_nr <= ++i)\n-\t\t\t\tbreak;\n-\t\t}\n-\n-\t\t/* Only handle regular files with both stages #2 and #3 */\n-\t\tif (i + 1 < active_nr) {\n-\t\t\tstruct cache_entry *e2 = active_cache[i];\n-\t\t\tstruct cache_entry *e3 = active_cache[i + 1];\n-\t\t\tif (ce_stage(e2) == 2 &&\n-\t\t\t    ce_stage(e3) == 3 &&\n-\t\t\t    ce_same_name(e, e3) &&\n-\t\t\t    S_ISREG(e2->ce_mode) &&\n-\t\t\t    S_ISREG(e3->ce_mode))\n-\t\t\t\tit->util = (void *) 1;\n-\t\t}\n-\n-\t\t/* Skip the entries with the same name */\n-\t\twhile (i < active_nr && ce_same_name(e, active_cache[i]))\n-\t\t\ti++;\n-\t\ti--; /* compensate for the outer loop */\n+\t\ti = check_one_conflict(i, &conflict_type);\n+\t\tif (conflict_type == THREE_STAGED)\n+\t\t\tstring_list_insert(conflict, (const char *)e->name);\n \t}\n \treturn 0;\n }\n \n-void rerere_remaining(struct string_list *merge_rr)\n+int rerere_remaining(struct string_list *merge_rr)\n {\n \tint i;\n-\tstruct string_list conflict = STRING_LIST_INIT_DUP;\n-\n-\t/* Add punted paths */\n-\tfind_conflict(&conflict);\n-\tfor (i = 0; i < conflict.nr; i++) {\n-\t\tif (!conflict.items[i].util)\n-\t\t\tstring_list_insert(merge_rr, conflict.items[i].string);\n-\t}\n+\tif (read_cache() < 0)\n+\t\treturn error(\"Could not read index\");\n \n-\t/* Mark already resolved paths */\n-\tfor (i = 0; i < active_nr; i++) {\n+\tfor (i = 0; i < active_nr;) {\n+\t\tint conflict_type;\n \t\tstruct cache_entry *e = active_cache[i];\n-\n-\t\tif (!ce_stage(e)) {\n+\t\ti = check_one_conflict(i, &conflict_type);\n+\t\tif (conflict_type == PUNTED)\n+\t\t\tstring_list_insert(merge_rr, (const char *)e->name);\n+\t\telse if (conflict_type == RESOLVED) {\n \t\t\tstruct string_list_item *it;\n \t\t\tit = string_list_lookup(merge_rr, (const char *)e->name);\n \t\t\tif (it != NULL) {\n \t\t\t\tfree(it->util);\n \t\t\t\tit->util = RERERE_RESOLVED;\n \t\t\t}\n-\t\t\tcontinue;\n \t\t}\n \t}\n+\treturn 0;\n }\n \n static int merge(const char *name, const char *path)\n@@ -504,8 +511,6 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \n \tfor (i = 0; i < conflict.nr; i++) {\n \t\tconst char *path = conflict.items[i].string;\n-\t\tif (!conflict.items[i].util)\n-\t\t\tcontinue; /* punted */\n \t\tif (!string_list_has_string(rr, path)) {\n \t\t\tunsigned char sha1[20];\n \t\t\tchar *hex;\n@@ -533,8 +538,6 @@ static int do_plain_rerere(struct string_list *rr, int fd)\n \t\tconst char *path = rr->items[i].string;\n \t\tconst char *name = (const char *)rr->items[i].util;\n \n-\t\tif (!name)\n-\t\t\tcontinue;\n \t\tif (has_rerere_resolution(name)) {\n \t\t\tif (!merge(name, path)) {\n \t\t\t\tif (rerere_autoupdate)\n@@ -664,8 +667,6 @@ int rerere_forget(const char **pathspec)\n \tfind_conflict(&conflict);\n \tfor (i = 0; i < conflict.nr; i++) {\n \t\tstruct string_list_item *it = &conflict.items[i];\n-\t\tif (!conflict.items[i].util)\n-\t\t\tcontinue; /* punted */\n \t\tif (!match_pathspec(pathspec, it->string, strlen(it->string),\n \t\t\t\t    0, NULL))\n \t\t\tcontinue;\ndiff --git a/rerere.h b/rerere.h\nindex ea2618b..595f49f 100644\n--- a/rerere.h\n+++ b/rerere.h\n@@ -18,7 +18,7 @@ extern int rerere(int);\n extern const char *rerere_path(const char *hex, const char *file);\n extern int has_rerere_resolution(const char *hex);\n extern int rerere_forget(const char **);\n-extern void rerere_remaining(struct string_list *);\n+extern int rerere_remaining(struct string_list *);\n \n #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, \"rerere-autoupdate\", (v), \\\n \t\"update the index with reused conflict resolution if possible\")\n-- \n1.7.4.rc2.33.g8a14f\n"},{"id":"161140","messageId":"7v1v3aqfqm.fsf@alter.siamese.dyndns.org","threadId":"26442","inReplyTo":"1297570198-21103-2-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH v2 1/2] mergetool: don't skip modify/remove conflicts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-15T00:54:41Z","receivedAt":"2011-02-15T00:54:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n\n> diff --git a/builtin/rerere.c b/builtin/rerere.c\n> index 081fccc..7b9fe18 100644\n> --- a/builtin/rerere.c\n> +++ b/builtin/rerere.c\n> @@ -147,8 +147,6 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n>  \tif (!strcmp(argv[0], \"clear\")) {\n>  \t\tfor (i = 0; i < merge_rr.nr; i++) {\n>  \t\t\tconst char *name = (const char *)merge_rr.items[i].util;\n> -\t\t\tif (!name)\n> -\t\t\t\tcontinue;\n>  \t\t\tif (!has_rerere_resolution(name))\n>  \t\t\t\tunlink_rr_item(name);\n>  \t\t}\n> @@ -157,19 +155,22 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n>  \t\tgarbage_collect(&merge_rr);\n>  \telse if (!strcmp(argv[0], \"status\"))\n>  \t\tfor (i = 0; i < merge_rr.nr; i++) {\n> -\t\t\tif (!merge_rr.items[i].util)\n> -\t\t\t\tcontinue;\n>  \t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n>  \t\t}\n> -\telse if (!strcmp(argv[0], \"remaining\"))\n> -\t\tfor (i = 0; i < merge_rr.nr; i++)\n> -\t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n> -\telse if (!strcmp(argv[0], \"diff\"))\n> +\telse if (!strcmp(argv[0], \"remaining\")) {\n> +\t\trerere_remaining(&merge_rr);\n> +\t\tfor (i = 0; i < merge_rr.nr; i++) {\n> +\t\t\tif (merge_rr.items[i].util != RERERE_RESOLVED)\n> +\t\t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n> +\t\t\telse\n> +\t\t\t\t/* prepare for later call to\n> +\t\t\t\t * string_list_clear() */\n> +\t\t\t\tmerge_rr.items[i].util = NULL;\n> +\t\t}\n> +\t} else if (!strcmp(argv[0], \"diff\"))\n>  \t\tfor (i = 0; i < merge_rr.nr; i++) {\n>  \t\t\tconst char *path = merge_rr.items[i].string;\n>  \t\t\tconst char *name = (const char *)merge_rr.items[i].util;\n> -\t\t\tif (!name)\n> -\t\t\t\tcontinue;\n>  \t\t\tdiff_two(rerere_path(name, \"preimage\"), path, path, path);\n>  \t\t}\n>  \telse\n\nBy looking at the diff between the parent of f322b35 (my earlier \"rerere\nremaining\" patch) and the result of applying this patch on top of the\npatch, I think the basic idea of this is to correct my stupid mistake of\ncontaminating merge_rr unconditionally, and instead adding the \"remaining\"\nentries only when handling the \"rerere remaining\" command (hence many\nremoval of special cases \"if (!name) continue\" introduced by my patch.\nThe result looks _much_ cleaner than the above patch shows and I am happy\nwith it.\n\nBut shouldn't you also revert the parts of my patch to do_plain_rerere()\nand rerere_forget() that have similar special cases?\n"},{"id":"161145","messageId":"alpine.DEB.2.00.1102142204240.7843@debian","threadId":"26442","inReplyTo":"7v1v3aqfqm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/2] mergetool: don't skip modify/remove conflicts","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-15T03:30:55Z","receivedAt":"2011-02-15T03:30:55Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"On Mon, 14 Feb 2011, Junio C Hamano wrote:\n\n> But shouldn't you also revert the parts of my patch to do_plain_rerere()\n> and rerere_forget() that have similar special cases?\n\nYep, done in patch 2/2.\n\nYour changes in find_conflicts() are still there after patch 1/2 and\nsince do_plain_rerere() and rerere_forget() call find_conflicts(), I\nthink they need to check for the punted paths. Only the callers of\nsetup_rerere() no longer need to care.\n\nIn patch 2/2, I refactored a bit more, so that find_conflicts()\nbehaves the way it did before your first patch, and then the checks\nfor punted paths are removed from the callers.\n\nIf you think the second one is good, it could easily be squashed with\nthe first one.  I left it separate because I wasn't quite sure. For\nexample, could the interface to check_one_conflict() be improved? As\nyou said, part of the changes from your patch are now undone, so I\nguess all three patches could even be squashed.\n\n\n/Martin\n"},{"id":"161209","messageId":"7vbp2dnjkt.fsf@alter.siamese.dyndns.org","threadId":"26442","inReplyTo":"alpine.DEB.2.00.1102142204240.7843@debian","subject":"Re: [PATCH v2 1/2] mergetool: don't skip modify/remove conflicts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-15T20:12:18Z","receivedAt":"2011-02-15T20:12:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n\n> On Mon, 14 Feb 2011, Junio C Hamano wrote:\n>\n>> But shouldn't you also revert the parts of my patch to do_plain_rerere()\n>> and rerere_forget() that have similar special cases?\n>\n> Yep, done in patch 2/2.\n>\n> Your changes in find_conflicts() are still there after patch 1/2 and\n> since do_plain_rerere() and rerere_forget() call find_conflicts(), I\n> think they need to check for the punted paths. Only the callers of\n> setup_rerere() no longer need to care.\n>\n> In patch 2/2, I refactored a bit more, so that find_conflicts()\n> behaves the way it did before your first patch, and then the checks\n> for punted paths are removed from the callers.\n>\n> If you think the second one is good, it could easily be squashed with\n> the first one.  I left it separate because I wasn't quite sure.\n\nProbably the changes to rerere proper from all three patches should be\nsquashed into one, and then the patch to the script to use the new feature\ncan be applied on top of it.\n"},{"id":"161290","messageId":"1297853265-18993-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26442","inReplyTo":"7vbp2dnjkt.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3 0/2] mergetool: don't skip modify/remove conflicts","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-16T10:47:43Z","receivedAt":"2011-02-16T10:47:43Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Third round. Reorganized the patches a bit as suggested by Junio. The\nfirst patch now contains all the changes to rerere and the second one\nonly makes mergetool call 'rerere remaining'.\n\nThe only tiny change to the end result is the removal of a blank line.\n\nThe series applies on top of master.\n\n\nJunio C Hamano (1):\n  rerere \"remaining\"\n\nMartin von Zweigbergk (1):\n  mergetool: don't skip modify/remove conflicts\n\n builtin/rerere.c     |   17 +++++++++--\n git-mergetool.sh     |    2 +-\n rerere.c             |   79 +++++++++++++++++++++++++++++++++++++++++++------\n rerere.h             |    8 +++++\n t/t7610-mergetool.sh |   40 +++++++++++++++++++++----\n 5 files changed, 126 insertions(+), 20 deletions(-)\n\n-- \n1.7.4.rc2.33.g8a14f\n"},{"id":"161291","messageId":"1297853265-18993-2-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26442","inReplyTo":"1297853265-18993-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH v3 1/2] rerere \"remaining\"","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-16T10:47:44Z","receivedAt":"2011-02-16T10:47:44Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"From: Junio C Hamano <gitster@pobox.com>\n\nAfter \"rerere\" resolves conflicts by reusing old resolution, there would\nbe three kinds of paths with conflict in the index:\n\n * paths that have been resolved in the working tree by rerere;\n * paths that need further work whose resolution could be recorded;\n * paths that need resolving that rerere won't help.\n\nWhen the user wants a list of paths that need hand-resolving, output from\n\"rerere status\" does not help, as it shows only the second category, but\nthe paths in the third category still needs work (rerere only makes sense\nfor regular files that have both our side and their side, and does not\nhelp other kinds of conflicts, e.g. \"we modified, they deleted\").\n\nThe new subcommand \"rerere remaining\" can be used to show both. As\nopposed to \"rerere status\", this subcommand also skips printing paths\nthat have been added to the index, since these paths are already\nresolved and are no longer \"remaining\".\n\nInitial patch provided by Junio. Refactored and modified to skip\nresolved paths by Martin. Commit message mostly by Junio.\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n\nJunio, I wasn't sure how to handle the sign-off etc, so feel free to\nmodify those as appropriate.\n\n builtin/rerere.c |   17 +++++++++--\n rerere.c         |   79 +++++++++++++++++++++++++++++++++++++++++++++++-------\n rerere.h         |    8 +++++\n 3 files changed, 91 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/rerere.c b/builtin/rerere.c\nindex 642bf35..7b9fe18 100644\n--- a/builtin/rerere.c\n+++ b/builtin/rerere.c\n@@ -8,7 +8,7 @@\n #include \"xdiff-interface.h\"\n \n static const char * const rerere_usage[] = {\n-\t\"git rerere [clear | status | diff | gc]\",\n+\t\"git rerere [clear | status | remaining | diff | gc]\",\n \tNULL,\n };\n \n@@ -154,9 +154,20 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n \t} else if (!strcmp(argv[0], \"gc\"))\n \t\tgarbage_collect(&merge_rr);\n \telse if (!strcmp(argv[0], \"status\"))\n-\t\tfor (i = 0; i < merge_rr.nr; i++)\n+\t\tfor (i = 0; i < merge_rr.nr; i++) {\n \t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n-\telse if (!strcmp(argv[0], \"diff\"))\n+\t\t}\n+\telse if (!strcmp(argv[0], \"remaining\")) {\n+\t\trerere_remaining(&merge_rr);\n+\t\tfor (i = 0; i < merge_rr.nr; i++) {\n+\t\t\tif (merge_rr.items[i].util != RERERE_RESOLVED)\n+\t\t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n+\t\t\telse\n+\t\t\t\t/* prepare for later call to\n+\t\t\t\t * string_list_clear() */\n+\t\t\t\tmerge_rr.items[i].util = NULL;\n+\t\t}\n+\t} else if (!strcmp(argv[0], \"diff\"))\n \t\tfor (i = 0; i < merge_rr.nr; i++) {\n \t\t\tconst char *path = merge_rr.items[i].string;\n \t\t\tconst char *name = (const char *)merge_rr.items[i].util;\ndiff --git a/rerere.c b/rerere.c\nindex d260843..1240d06 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -7,6 +7,11 @@\n #include \"ll-merge.h\"\n #include \"attr.h\"\n \n+#define RESOLVED 0\n+#define PUNTED 1\n+#define THREE_STAGED 2\n+void *RERERE_RESOLVED = &RERERE_RESOLVED;\n+\n /* if rerere_enabled == -1, fall back to detection of .git/rr-cache */\n static int rerere_enabled = -1;\n \n@@ -345,21 +350,75 @@ static int handle_cache(const char *path, unsigned char *sha1, const char *outpu\n \treturn hunk_no;\n }\n \n-static int find_conflict(struct string_list *conflict)\n+static int check_one_conflict(int i, int *type)\n {\n-\tint i;\n-\tif (read_cache() < 0)\n-\t\treturn error(\"Could not read index\");\n-\tfor (i = 0; i+1 < active_nr; i++) {\n+\tstruct cache_entry *e = active_cache[i];\n+\tstruct string_list_item *it;\n+\n+\tif (!ce_stage(e)) {\n+\t\t*type = RESOLVED;\n+\t\treturn i + 1;\n+\t}\n+\n+\t*type = PUNTED;\n+\tif (ce_stage(e) == 1) {\n+\t\tif (active_nr <= ++i)\n+\t\t\treturn i + 1;\n+\t}\n+\n+\t/* Only handle regular files with both stages #2 and #3 */\n+\tif (i + 1 < active_nr) {\n \t\tstruct cache_entry *e2 = active_cache[i];\n-\t\tstruct cache_entry *e3 = active_cache[i+1];\n+\t\tstruct cache_entry *e3 = active_cache[i + 1];\n \t\tif (ce_stage(e2) == 2 &&\n \t\t    ce_stage(e3) == 3 &&\n-\t\t    ce_same_name(e2, e3) &&\n+\t\t    ce_same_name(e, e3) &&\n \t\t    S_ISREG(e2->ce_mode) &&\n-\t\t    S_ISREG(e3->ce_mode)) {\n-\t\t\tstring_list_insert(conflict, (const char *)e2->name);\n-\t\t\ti++; /* skip over both #2 and #3 */\n+\t\t    S_ISREG(e3->ce_mode))\n+\t\t\t*type = THREE_STAGED;\n+\t}\n+\n+\t/* Skip the entries with the same name */\n+\twhile (i < active_nr && ce_same_name(e, active_cache[i]))\n+\t\ti++;\n+\treturn i;\n+}\n+\n+static int find_conflict(struct string_list *conflict)\n+{\n+\tint i;\n+\tif (read_cache() < 0)\n+\t\treturn error(\"Could not read index\");\n+\n+\tfor (i = 0; i < active_nr;) {\n+\t\tint conflict_type;\n+\t\tstruct cache_entry *e = active_cache[i];\n+\t\ti = check_one_conflict(i, &conflict_type);\n+\t\tif (conflict_type == THREE_STAGED)\n+\t\t\tstring_list_insert(conflict, (const char *)e->name);\n+\t}\n+\treturn 0;\n+}\n+\n+int rerere_remaining(struct string_list *merge_rr)\n+{\n+\tint i;\n+\tif (read_cache() < 0)\n+\t\treturn error(\"Could not read index\");\n+\n+\tfor (i = 0; i < active_nr;) {\n+\t\tint conflict_type;\n+\t\tstruct cache_entry *e = active_cache[i];\n+\t\ti = check_one_conflict(i, &conflict_type);\n+\t\tif (conflict_type == PUNTED)\n+\t\t\tstring_list_insert(merge_rr, (const char *)e->name);\n+\t\telse if (conflict_type == RESOLVED) {\n+\t\t\tstruct string_list_item *it;\n+\t\t\tit = string_list_lookup(merge_rr, (const char *)e->name);\n+\t\t\tif (it != NULL) {\n+\t\t\t\tfree(it->util);\n+\t\t\t\tit->util = RERERE_RESOLVED;\n+\t\t\t}\n \t\t}\n \t}\n \treturn 0;\ndiff --git a/rerere.h b/rerere.h\nindex eaa9004..595f49f 100644\n--- a/rerere.h\n+++ b/rerere.h\n@@ -6,11 +6,19 @@\n #define RERERE_AUTOUPDATE   01\n #define RERERE_NOAUTOUPDATE 02\n \n+/*\n+ * Marks paths that have been hand-resolved and added to the\n+ * index. Set in the util field of such paths after calling\n+ * rerere_remaining.\n+ */\n+extern void *RERERE_RESOLVED;\n+\n extern int setup_rerere(struct string_list *, int);\n extern int rerere(int);\n extern const char *rerere_path(const char *hex, const char *file);\n extern int has_rerere_resolution(const char *hex);\n extern int rerere_forget(const char **);\n+extern int rerere_remaining(struct string_list *);\n \n #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, \"rerere-autoupdate\", (v), \\\n \t\"update the index with reused conflict resolution if possible\")\n-- \n1.7.4.rc2.33.g8a14f\n"},{"id":"161292","messageId":"1297853265-18993-3-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26442","inReplyTo":"1297853265-18993-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH v3 2/2] mergetool: don't skip modify/remove conflicts","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-02-16T10:47:45Z","receivedAt":"2011-02-16T10:47:45Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Since bb0a484 (mergetool: Skip autoresolved paths, 2010-08-17),\nmergetool uses different ways of figuring out the list of files with\nmerge conflicts depending on whether rerere is active. If rerere is\nactive, mergetool will use 'git rerere status' to list the files with\nremaining conflicts. However, the output from that command does not\nlist conflicts of types that rerere does not handle, such as\nmodify/remove conflicts.\n\nAnother problem with solely relying on the output from 'git rerere\nstatus' is that, for new conflicts that are not yet known to rerere,\nthe output from the command will list the files even after adding them\nto the index. This means that if the conflicts in some files have been\nresolved and 'git mergetool' is run again, it will ask the user\nsomething like the following for each of those files.\n\n file1: file does not need merging\n Continue merging other unresolved paths (y/n) ?\n\nSolve both of these problems by replacing the call to 'git rerere\nstatus' with a call to the new 'git rerere remaining' that was\nintroduced in the previous commit.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n git-mergetool.sh     |    2 +-\n t/t7610-mergetool.sh |   40 ++++++++++++++++++++++++++++++++++------\n 2 files changed, 35 insertions(+), 7 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 2f8dc44..bacbda2 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -269,7 +269,7 @@ rerere=false\n files_to_merge() {\n     if test \"$rerere\" = true\n     then\n-\tgit rerere status\n+\tgit rerere remaining\n     else\n \tgit ls-files -u | sed -e 's/^[^\t]*\t//' | sort -u\n     fi\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex d78bdec..dc838c9 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -16,23 +16,33 @@ Testing basic merge tool invocation'\n test_expect_success 'setup' '\n     git config rerere.enabled true &&\n     echo master >file1 &&\n+    echo master file11 >file11 &&\n+    echo master file12 >file12 &&\n+    echo master file13 >file13 &&\n+    echo master file14 >file14 &&\n     mkdir subdir &&\n     echo master sub >subdir/file3 &&\n-    git add file1 subdir/file3 &&\n-    git commit -m \"added file1\" &&\n+    git add file1 file1[1-4] subdir/file3 &&\n+    git commit -m \"add initial versions\" &&\n \n     git checkout -b branch1 master &&\n     echo branch1 change >file1 &&\n     echo branch1 newfile >file2 &&\n+    echo branch1 change file11 >file11 &&\n+    echo branch1 change file13 >file13 &&\n     echo branch1 sub >subdir/file3 &&\n-    git add file1 file2 subdir/file3 &&\n+    git add file1 file11 file13 file2 subdir/file3 &&\n+    git rm file12 &&\n     git commit -m \"branch1 changes\" &&\n \n     git checkout master &&\n     echo master updated >file1 &&\n     echo master new >file2 &&\n+    echo master updated file12 >file12 &&\n+    echo master updated file14 >file14 &&\n     echo master new sub >subdir/file3 &&\n-    git add file1 file2 subdir/file3 &&\n+    git add file1 file12 file14 file2 subdir/file3 &&\n+    git rm file11 &&\n     git commit -m \"master updates\" &&\n \n     git config merge.tool mytool &&\n@@ -46,6 +56,8 @@ test_expect_success 'custom mergetool' '\n     ( yes \"\" | git mergetool file1 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool file2 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file11 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n     test \"$(cat file1)\" = \"master updated\" &&\n     test \"$(cat file2)\" = \"master new\" &&\n     test \"$(cat subdir/file3)\" = \"master new sub\" &&\n@@ -59,6 +71,8 @@ test_expect_success 'mergetool crlf' '\n     ( yes \"\" | git mergetool file1 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool file2 >/dev/null 2>&1 ) &&\n     ( yes \"\" | git mergetool subdir/file3 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file11 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n     test \"$(printf x | cat file1 -)\" = \"$(printf \"master updated\\r\\nx\")\" &&\n     test \"$(printf x | cat file2 -)\" = \"$(printf \"master new\\r\\nx\")\" &&\n     test \"$(printf x | cat subdir/file3 -)\" = \"$(printf \"master new sub\\r\\nx\")\" &&\n@@ -82,6 +96,8 @@ test_expect_success 'mergetool on file in parent dir' '\n \tcd subdir &&\n \t( yes \"\" | git mergetool ../file1 >/dev/null 2>&1 ) &&\n \t( yes \"\" | git mergetool ../file2 >/dev/null 2>&1 ) &&\n+\t( yes \"d\" | git mergetool ../file11 >/dev/null 2>&1 ) &&\n+\t( yes \"d\" | git mergetool ../file12 >/dev/null 2>&1 ) &&\n \ttest \"$(cat ../file1)\" = \"master updated\" &&\n \ttest \"$(cat ../file2)\" = \"master new\" &&\n \tgit commit -m \"branch1 resolved with mergetool - subdir\"\n@@ -92,6 +108,8 @@ test_expect_success 'mergetool skips autoresolved' '\n     git checkout -b test4 branch1 &&\n     test_must_fail git merge master &&\n     test -n \"$(git ls-files -u)\" &&\n+    ( yes \"d\" | git mergetool file11 >/dev/null 2>&1 ) &&\n+    ( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n     output=\"$(git mergetool --no-prompt)\" &&\n     test \"$output\" = \"No files need merging\" &&\n     git reset --hard\n@@ -102,13 +120,23 @@ test_expect_success 'mergetool merges all from subdir' '\n \tcd subdir &&\n \tgit config rerere.enabled false &&\n \ttest_must_fail git merge master &&\n-\tgit mergetool --no-prompt &&\n+\t( yes \"d\" \"d\" | git mergetool --no-prompt ) &&\n \ttest \"$(cat ../file1)\" = \"master updated\" &&\n \ttest \"$(cat ../file2)\" = \"master new\" &&\n \ttest \"$(cat file3)\" = \"master new sub\" &&\n-\tgit add ../file1 ../file2 file3 &&\n \tgit commit -m \"branch2 resolved by mergetool from subdir\"\n     )\n '\n \n+test_expect_success 'mergetool skips resolved paths when rerere is active' '\n+    git config rerere.enabled true &&\n+    rm -rf .git/rr-cache &&\n+    git checkout -b test5 branch1\n+    test_must_fail git merge master >/dev/null 2>&1 &&\n+    ( yes \"d\" \"d\" | git mergetool --no-prompt >/dev/null 2>&1 ) &&\n+    output=\"$(yes \"n\" | git mergetool --no-prompt)\" &&\n+    test \"$output\" = \"No files need merging\" &&\n+    git reset --hard\n+'\n+\n test_done\n-- \n1.7.4.rc2.33.g8a14f\n"},{"id":"161337","messageId":"7vk4gziswj.fsf@alter.siamese.dyndns.org","threadId":"26442","inReplyTo":"1297853265-18993-2-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH v3 1/2] rerere \"remaining\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-16T21:14:20Z","receivedAt":"2011-02-16T21:14:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:\n\n> Junio, I wasn't sure how to handle the sign-off etc, so feel free to\n> modify those as appropriate.\n\nAt this point I think the primary value of the change is your adding\n\"rerere_remaining()\" to only where it matters (i.e. the isolation of the\nchange) and you should get the credit as the primary author.  Mine was\nimpossible to read because I contaminated merge_rr in a wrong place and\nhad to work it around in unrelated codepaths all over the place.\n\nThis was much cleaner and easier to read.\n\n>  builtin/rerere.c |   17 +++++++++--\n>  rerere.c         |   79 +++++++++++++++++++++++++++++++++++++++++++++++-------\n>  rerere.h         |    8 +++++\n>  3 files changed, 91 insertions(+), 13 deletions(-)\n>\n> diff --git a/builtin/rerere.c b/builtin/rerere.c\n> index 642bf35..7b9fe18 100644\n> --- a/builtin/rerere.c\n> +++ b/builtin/rerere.c\n> @@ -8,7 +8,7 @@\n>  #include \"xdiff-interface.h\"\n>  \n>  static const char * const rerere_usage[] = {\n> -\t\"git rerere [clear | status | diff | gc]\",\n> +\t\"git rerere [clear | status | remaining | diff | gc]\",\n>  \tNULL,\n>  };\n>  \n> @@ -154,9 +154,20 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)\n>  \t} else if (!strcmp(argv[0], \"gc\"))\n>  \t\tgarbage_collect(&merge_rr);\n>  \telse if (!strcmp(argv[0], \"status\"))\n> -\t\tfor (i = 0; i < merge_rr.nr; i++)\n> +\t\tfor (i = 0; i < merge_rr.nr; i++) {\n>  \t\t\tprintf(\"%s\\n\", merge_rr.items[i].string);\n> -\telse if (!strcmp(argv[0], \"diff\"))\n> +\t\t}\n\nUnnecessary {} around a single printf().  Will clean-up.\n\nThanks.\n"}]}