{"thread":{"id":"27682","subject":"[PATCH 2/2] mergetool: Don't assume paths are unmerged","startedAt":"2011-06-22T02:46:28Z","lastAt":"2011-06-22T21:33:12Z","messageCount":3,"participants":["Jonathon Mah","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"170444","messageId":"92B6FB42-FE0D-48DC-ABD0-BA1903D842D2@JonathonMah.com","threadId":"27682","inReplyTo":null,"subject":"[PATCH 2/2] mergetool: Don't assume paths are unmerged","fromName":"Jonathon Mah","fromEmail":"me@jonathonmah.com","sentAt":"2011-06-22T02:46:28Z","receivedAt":"2011-06-22T02:46:28Z","isPatch":true,"sender":{"key":"me@jonathonmah.com","avatar":"https://avatars.githubusercontent.com/u/2748?v=4"},"body":"Like commit, mergetool now treats its path arguments as restricting\noperation to the listed paths. Running \"git mergetool subdir\" will\nprompt to resolve all conflicted blobs under subdir.\n\nPreviously mergetool would assume each path was in an unresolved state,\nand get confused when it couldn't check out their other stages.\n\nSigned-off-by: Jonathon Mah <me@JonathonMah.com>\n---\nApologies, having some issues with my mail agent. This should be in-reply-to: 734F376D-0CF5-4417-8DC2-8A46AA05D995@JonathonMah.com\n\n Documentation/git-mergetool.txt |    7 ++-\n git-mergetool.sh                |   83 +++++++++++++++++---------------------\n t/t7610-mergetool.sh            |   28 +++++++++++--\n 3 files changed, 64 insertions(+), 54 deletions(-)\n\ndiff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt\nindex 8c79ae8..f1f4e7a 100644\n--- a/Documentation/git-mergetool.txt\n+++ b/Documentation/git-mergetool.txt\n@@ -16,9 +16,10 @@ Use `git mergetool` to run one of several merge utilities to resolve\n merge conflicts.  It is typically run after 'git merge'.\n \n If one or more <file> parameters are given, the merge tool program will\n-be run to resolve differences on each file.  If no <file> names are\n-specified, 'git mergetool' will run the merge tool program on every file\n-with merge conflicts.\n+be run to resolve differences on each file (skipping those without\n+conflicts).  Specifying a directory will include all unresolved files in\n+that path.  If no <file> names are specified, 'git mergetool' will run\n+the merge tool program on every file with merge conflicts.\n \n OPTIONS\n -------\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 3aab5aa..81cf2cb 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -342,64 +342,55 @@ merge_keep_temporaries=\"$(git config --bool mergetool.keepTemporaries || echo fa\n \n last_status=0\n rollup_status=0\n-rerere=false\n-\n-files_to_merge() {\n-    if test \"$rerere\" = true\n-    then\n-\tgit rerere remaining\n-    else\n-\tgit ls-files -u | sed -e 's/^[^\t]*\t//' | sort -u\n-    fi\n-}\n-\n+files=\n \n if test $# -eq 0 ; then\n     cd_to_toplevel\n \n     if test -e \"$GIT_DIR/MERGE_RR\"\n     then\n-\trerere=true\n-    fi\n-\n-    files=$(files_to_merge)\n-    if test -z \"$files\" ; then\n-\techo \"No files need merging\"\n-\texit 0\n+\tfiles=$(git rerere remaining)\n+    else\n+\tfiles=$(git ls-files -u | sed -e 's/^[^\t]*\t//' | sort -u)\n     fi\n-\n-    # Save original stdin\n-    exec 3<&0\n-\n-    printf \"Merging:\\n\"\n-    printf \"$files\\n\"\n-\n-    files_to_merge |\n-    while IFS= read i\n-    do\n-\tif test $last_status -ne 0; then\n-\t    prompt_after_failed_merge <&3 || exit 1\n-\tfi\n-\tprintf \"\\n\"\n-\tmerge_file \"$i\" <&3\n-\tlast_status=$?\n-\tif test $last_status -ne 0; then\n-\t    rollup_status=1\n-\tfi\n-    done\n else\n     while test $# -gt 0; do\n-\tif test $last_status -ne 0; then\n-\t    prompt_after_failed_merge || exit 1\n-\tfi\n-\tprintf \"\\n\"\n-\tmerge_file \"$1\"\n-\tlast_status=$?\n-\tif test $last_status -ne 0; then\n-\t    rollup_status=1\n+\tmatches=$(git ls-files -u -- \"$1\" | sed -e 's/^[^\t]*\t//' | sort -u)\n+\tif test -n \"$matches\"; then\n+\t    if test -z \"$files\"; then\n+\t\tfiles=$matches\n+\t    else\n+\t\tfiles=$(printf \"%s\\n%s\" \"$files\" \"$matches\")\n+\t    fi\n \tfi\n \tshift\n     done\n+    files=$(printf \"%s\" \"$files\" | sort -u)\n fi\n \n+if test -z \"$files\" ; then\n+    echo \"No files need merging\"\n+    exit 0\n+fi\n+\n+# Save original stdin\n+exec 3<&0\n+\n+printf \"Merging:\\n\"\n+printf \"$files\\n\"\n+\n+IFS='\n+'; for i in $files\n+do\n+    if test $last_status -ne 0; then\n+\tprompt_after_failed_merge <&3 || exit 1\n+    fi\n+    printf \"\\n\"\n+    merge_file \"$i\" <&3\n+    last_status=$?\n+    if test $last_status -ne 0; then\n+\trollup_status=1\n+    fi\n+done\n+\n exit $rollup_status\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex f00caa3..4aab2a7 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -81,7 +81,7 @@ test_expect_success 'custom mergetool' '\n     git checkout -b test1 branch1 &&\n     git submodule update -N &&\n     test_must_fail git merge master >/dev/null 2>&1 &&\n-    ( yes \"\" | git mergetool file1 >/dev/null 2>&1 ) &&\n+    ( yes \"\" | git mergetool file1 file1 ) &&\n     ( yes \"\" | git mergetool file2 \"spaced name\" >/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@@ -184,6 +184,24 @@ test_expect_success 'mergetool skips resolved paths when rerere is active' '\n     git reset --hard\n '\n \n+test_expect_success 'mergetool takes partial path' '\n+    git config rerere.enabled false &&\n+    git checkout -b test12 branch1 &&\n+    git submodule update -N &&\n+    test_must_fail git merge master &&\n+\n+    #shouldnt need these lines\n+    #( yes \"d\" | git mergetool file11 >/dev/null 2>&1 ) &&\n+    #( yes \"d\" | git mergetool file12 >/dev/null 2>&1 ) &&\n+    #( yes \"l\" | git mergetool submod >/dev/null 2>&1 ) &&\n+    #( yes \"\" | git mergetool file1 file2 >/dev/null 2>&1 ) &&\n+\n+    ( yes \"\" | git mergetool subdir ) &&\n+\n+    test \"$(cat subdir/file3)\" = \"master new sub\" &&\n+    git reset --hard\n+'\n+\n test_expect_success 'deleted vs modified submodule' '\n     git checkout -b test6 branch1 &&\n     git submodule update -N &&\n@@ -392,7 +410,7 @@ test_expect_success 'directory vs modified submodule' '\n     test \"$(cat submod/file16)\" = \"not a submodule\" &&\n     rm -rf submod.orig &&\n \n-    git reset --hard &&\n+    git reset --hard >/dev/null 2>&1 &&\n     test_must_fail git merge master &&\n     test -n \"$(git ls-files -u)\" &&\n     test ! -e submod.orig &&\n@@ -404,7 +422,7 @@ test_expect_success 'directory vs modified submodule' '\n     ( cd submod && git clean -f && git reset --hard ) &&\n     git submodule update -N &&\n     test \"$(cat submod/bar)\" = \"master submodule\" &&\n-    git reset --hard && rm -rf submod-movedaside &&\n+    git reset --hard >/dev/null 2>&1 && rm -rf submod-movedaside &&\n \n     git checkout -b test11.c master &&\n     git submodule update -N &&\n@@ -414,7 +432,7 @@ test_expect_success 'directory vs modified submodule' '\n     git submodule update -N &&\n     test \"$(cat submod/bar)\" = \"master submodule\" &&\n \n-    git reset --hard &&\n+    git reset --hard >/dev/null 2>&1 &&\n     git submodule update -N &&\n     test_must_fail git merge test11 &&\n     test -n \"$(git ls-files -u)\" &&\n@@ -422,7 +440,7 @@ test_expect_success 'directory vs modified submodule' '\n     ( yes \"r\" | git mergetool submod ) &&\n     test \"$(cat submod/file16)\" = \"not a submodule\" &&\n \n-    git reset --hard master &&\n+    git reset --hard master >/dev/null 2>&1 &&\n     ( cd submod && git clean -f && git reset --hard ) &&\n     git submodule update -N\n '\n-- \n1.7.4.4\n"},{"id":"170480","messageId":"7v62nx5zhk.fsf@alter.siamese.dyndns.org","threadId":"27682","inReplyTo":"92B6FB42-FE0D-48DC-ABD0-BA1903D842D2@JonathonMah.com","subject":"Re: [PATCH 2/2] mergetool: Don't assume paths are unmerged","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-22T21:26:47Z","receivedAt":"2011-06-22T21:26:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathon Mah <me@JonathonMah.com> writes:\n\n> Like commit, mergetool now treats its path arguments as restricting\n> operation to the listed paths. Running \"git mergetool subdir\" will\n> prompt to resolve all conflicted blobs under subdir.\n>\n> Previously mergetool would assume each path was in an unresolved state,\n> and get confused when it couldn't check out their other stages.\n>\n> Signed-off-by: Jonathon Mah <me@JonathonMah.com>\n> ---\n\nThis does two unrelated things, no?\n\n - Given the name of a file from the command line, current mergetool gets\n   confused if the given file is not in a conflicted state (bugfix).\n\n - Instead of taking command line arguments as literal filenames, treat\n   them as pathspec like any sane git subcommand does, allowing the users\n   more flexibility (new feature).\n\nAnd the title of the patch advertises only the first point.  Using the\ncommand line argument as pathspec may make a lot of sense, and code\nreduction in git-mergetool.sh with this patch may also be a good change.\n\n> diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt\n> index 8c79ae8..f1f4e7a 100644\n> --- a/Documentation/git-mergetool.txt\n> +++ b/Documentation/git-mergetool.txt\n> @@ -16,9 +16,10 @@ Use `git mergetool` to run one of several merge utilities to resolve\n>  merge conflicts.  It is typically run after 'git merge'.\n>  \n>  If one or more <file> parameters are given, the merge tool program will\n> -be run to resolve differences on each file.  If no <file> names are\n> -specified, 'git mergetool' will run the merge tool program on every file\n> -with merge conflicts.\n> +be run to resolve differences on each file (skipping those without\n> +conflicts).  Specifying a directory will include all unresolved files in\n> +that path.  If no <file> names are specified, 'git mergetool' will run\n> +the merge tool program on every file with merge conflicts.\n\nThe documentation may also have to reword <file> written back in the days\nwhen they were literal filenames, not pathspecs.\n\n> +\tmatches=$(git ls-files -u -- \"$1\" | sed -e 's/^[^\t]*\t//' | sort -u)\n\nWould we want to catch and signal a typo like this?\n\n\tgit mergetool Mkaefile\n\tgit mergetool Documentaiton/\n\nwhen there is a conflict in Makefile and some file in Documentation/\ndirectory, and user obviously wanted to name them but botched typing? I\nthink you are just letting these typos silently go.\n\n> @@ -392,7 +410,7 @@ test_expect_success 'directory vs modified submodule' '\n>      test \"$(cat submod/file16)\" = \"not a submodule\" &&\n>      rm -rf submod.orig &&\n>  \n> -    git reset --hard &&\n> +    git reset --hard >/dev/null 2>&1 &&\n\nWhat is the justification for this change?\n\nPlease do not make \"sh txxx.sh -v\" less useful.\n"},{"id":"170481","messageId":"7v1uyl5z6v.fsf@alter.siamese.dyndns.org","threadId":"27682","inReplyTo":"92B6FB42-FE0D-48DC-ABD0-BA1903D842D2@JonathonMah.com","subject":"Re: [PATCH 2/2] mergetool: Don't assume paths are unmerged","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-22T21:33:12Z","receivedAt":"2011-06-22T21:33:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathon Mah <me@JonathonMah.com> writes:\n\n>  if test $# -eq 0 ; then\n>      cd_to_toplevel\n>  \n>      if test -e \"$GIT_DIR/MERGE_RR\"\n>      then\n> +\tfiles=$(git rerere remaining)\n> +    else\n> +\tfiles=$(git ls-files -u | sed -e 's/^[^\t]*\t//' | sort -u)\n>      fi\n>  else\n>      while test $# -gt 0; do\n> +\tmatches=$(git ls-files -u -- \"$1\" | sed -e 's/^[^\t]*\t//' | sort -u)\n> +\tif test -n \"$matches\"; then\n> +\t    if test -z \"$files\"; then\n> +\t\tfiles=$matches\n> +\t    else\n> +\t\tfiles=$(printf \"%s\\n%s\" \"$files\" \"$matches\")\n> +\t    fi\n>  \tfi\n>  \tshift\n>      done\n> +    files=$(printf \"%s\" \"$files\" | sort -u)\n>  fi\n\nWhy do you need a loop here in the else clause, instead of just a single:\n\n\tfiles=$(git ls-files -u -- \"$@\" |...)\n"}]}