{"thread":{"id":"18681","subject":"git-{diff,merge} refactor round 2","startedAt":"2009-04-01T12:55:04Z","lastAt":"2009-04-06T00:33:55Z","messageCount":26,"participants":["David Aguilar","Markus Heidelberg","Charles Bailey","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"110129","messageId":"1238590514-41893-1-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":null,"subject":"git-{diff,merge} refactor round 2","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:04Z","receivedAt":"2009-04-01T12:55:04Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Here's the 2nd round of refactoring.\n\nThis is based on Junio's pu branch.\n\nI'm including the difftool.prompt patch in this series\nbecause it is a dependency and including it here makes\nthat obvious.\n\nI tried to keep the dependencies untangled while still being\nable to manage the various command-line flags all in one\nplace.  Alas, this is shell so it can only be so elegant.\n\nI went ahead and rolled in the \"remove -o $MERGED\" from\ntkdiff for the diff mode case.\n\n\nStill TODO:\nincorporate the \"add diffuse as a merge tool\" patch.\n\nIs there more that can be refactored?\nProbably the part that sets up candidate mergetools,\nreplacing that with a function might be useful.\n\nHere's what I've got so far.\n"},{"id":"110136","messageId":"1238590514-41893-2-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":"1238590514-41893-1-git-send-email-davvid@gmail.com","subject":"[PATCH 01/10] difftool: add support for a difftool.prompt config variable","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:05Z","receivedAt":"2009-04-01T12:55:05Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"difftool now supports difftool.prompt so that users do not have to\npass --no-prompt or hit enter each time a diff tool is launched.\nThe --prompt flag overrides the configuration variable.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n Documentation/config.txt       |    3 ++\n Documentation/git-difftool.txt |   10 +++++-\n git-difftool-helper.sh         |   10 +++++-\n git-difftool.perl              |   15 +++++++--\n t/t7800-difftool.sh            |   64 ++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 96 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 185a9ef..79c8b66 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -684,6 +684,9 @@ difftool.<tool>.cmd::\n \tis set to the name of the temporary file containing the contents\n \tof the diff post-image.\n \n+difftool.prompt::\n+\tPrompt before each invocation of the diff tool.\n+\n diff.wordRegex::\n \tA POSIX Extended Regular Expression used to determine what is a \"word\"\n \twhen performing word-by-word difference calculations.  Character\ndiff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt\nindex a00e943..73d4782 100644\n--- a/Documentation/git-difftool.txt\n+++ b/Documentation/git-difftool.txt\n@@ -7,7 +7,7 @@ git-difftool - Show changes using common diff tools\n \n SYNOPSIS\n --------\n-'git difftool' [--tool=<tool>] [-y|--no-prompt] [<'git diff' options>]\n+'git difftool' [--tool=<tool>] [-y|--no-prompt|--prompt] [<'git diff' options>]\n \n DESCRIPTION\n -----------\n@@ -21,6 +21,11 @@ OPTIONS\n --no-prompt::\n \tDo not prompt before launching a diff tool.\n \n+--prompt::\n+\tPrompt before each invocation of the diff tool.\n+\tThis is the default behaviour; the option is provided to\n+\toverride any configuration settings.\n+\n -t <tool>::\n --tool=<tool>::\n \tUse the diff tool specified by <tool>.\n@@ -72,6 +77,9 @@ difftool.<tool>.cmd::\n +\n See the `--tool=<tool>` option above for more details.\n \n+difftool.prompt::\n+\tPrompt before each invocation of the diff tool.\n+\n SEE ALSO\n --------\n linkgit:git-diff[1]::\ndiff --git a/git-difftool-helper.sh b/git-difftool-helper.sh\nindex b91002b..02bb135 100755\n--- a/git-difftool-helper.sh\n+++ b/git-difftool-helper.sh\n@@ -7,9 +7,15 @@\n #\n # Copyright (c) 2009 David Aguilar\n \n-# Set GIT_DIFFTOOL_NO_PROMPT to bypass the per-file prompt.\n+# difftool.prompt controls the default prompt/no-prompt behavior\n+# and is overridden with $GIT_DIFFTOOL*_PROMPT.\n should_prompt () {\n-\ttest -z \"$GIT_DIFFTOOL_NO_PROMPT\"\n+\tprompt=$(git config --bool difftool.prompt || echo true)\n+\tif test \"$prompt\" = true; then\n+\t\ttest -z \"$GIT_DIFFTOOL_NO_PROMPT\"\n+\telse\n+\t\ttest -n \"$GIT_DIFFTOOL_PROMPT\"\n+\tfi\n }\n \n # This function prepares temporary files and launches the appropriate\ndiff --git a/git-difftool.perl b/git-difftool.perl\nindex 8c160e5..985dfe0 100755\n--- a/git-difftool.perl\n+++ b/git-difftool.perl\n@@ -2,9 +2,12 @@\n # Copyright (c) 2009 David Aguilar\n #\n # This is a wrapper around the GIT_EXTERNAL_DIFF-compatible\n-# git-difftool-helper script.  This script exports\n-# GIT_EXTERNAL_DIFF and GIT_PAGER for use by git, and\n-# GIT_DIFFTOOL_NO_PROMPT and GIT_DIFF_TOOL for use by git-difftool-helper.\n+# git-difftool-helper script.\n+#\n+# This script exports GIT_EXTERNAL_DIFF and GIT_PAGER for use by git.\n+# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, and GIT_DIFF_TOOL\n+# are exported for use by git-difftool-helper.\n+#\n # Any arguments that are unknown to this script are forwarded to 'git diff'.\n \n use strict;\n@@ -62,6 +65,12 @@ sub generate_command\n \t\t}\n \t\tif ($arg eq '-y' || $arg eq '--no-prompt') {\n \t\t\t$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';\n+\t\t\tdelete $ENV{GIT_DIFFTOOL_PROMPT};\n+\t\t\tnext;\n+\t\t}\n+\t\tif ($arg eq '--prompt') {\n+\t\t\t$ENV{GIT_DIFFTOOL_PROMPT} = 'true';\n+\t\t\tdelete $ENV{GIT_DIFFTOOL_NO_PROMPT};\n \t\t\tnext;\n \t\t}\n \t\tif ($arg eq '-h' || $arg eq '--help') {\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex ceef84b..33d07e6 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -15,6 +15,7 @@ remove_config_vars()\n \t# Unset all config variables used by git-difftool\n \tgit config --unset diff.tool\n \tgit config --unset difftool.test-tool.cmd\n+\tgit config --unset difftool.prompt\n \tgit config --unset merge.tool\n \tgit config --unset mergetool.test-tool.cmd\n \treturn 0\n@@ -26,11 +27,18 @@ restore_test_defaults()\n \tremove_config_vars\n \tunset GIT_DIFF_TOOL\n \tunset GIT_MERGE_TOOL\n+\tunset GIT_DIFFTOOL_PROMPT\n \tunset GIT_DIFFTOOL_NO_PROMPT\n \tgit config diff.tool test-tool &&\n \tgit config difftool.test-tool.cmd 'cat $LOCAL'\n }\n \n+prompt_given()\n+{\n+\tprompt=\"$1\"\n+\ttest \"$prompt\" = \"Hit return to launch 'test-tool': branch\"\n+}\n+\n # Create a file on master and change it on branch\n test_expect_success 'setup' '\n \techo master >file &&\n@@ -116,6 +124,62 @@ test_expect_success 'GIT_DIFFTOOL_NO_PROMPT variable' '\n \trestore_test_defaults\n '\n \n+# git-difftool supports the difftool.prompt variable.\n+# Test that GIT_DIFFTOOL_PROMPT can override difftool.prompt = false\n+test_expect_success 'GIT_DIFFTOOL_PROMPT variable' '\n+\tgit config difftool.prompt false &&\n+\tGIT_DIFFTOOL_PROMPT=true &&\n+\texport GIT_DIFFTOOL_PROMPT &&\n+\n+\tprompt=$(echo | git difftool --prompt branch | tail -1) &&\n+\tprompt_given \"$prompt\" &&\n+\n+\trestore_test_defaults\n+'\n+\n+# Test that we don't have to pass --no-prompt when difftool.prompt is false\n+test_expect_success 'difftool.prompt config variable is false' '\n+\tgit config difftool.prompt false &&\n+\n+\tdiff=$(git difftool branch) &&\n+\ttest \"$diff\" = \"branch\" &&\n+\n+\trestore_test_defaults\n+'\n+\n+# Test that the -y flag can override difftool.prompt = true\n+test_expect_success 'difftool.prompt can overridden with -y' '\n+\tgit config difftool.prompt true &&\n+\n+\tdiff=$(git difftool -y branch) &&\n+\ttest \"$diff\" = \"branch\" &&\n+\n+\trestore_test_defaults\n+'\n+\n+# Test that the --prompt flag can override difftool.prompt = false\n+test_expect_success 'difftool.prompt can overridden with --prompt' '\n+\tgit config difftool.prompt false &&\n+\n+\tprompt=$(echo | git difftool --prompt branch | tail -1) &&\n+\tprompt_given \"$prompt\" &&\n+\n+\trestore_test_defaults\n+'\n+\n+# Test that the last flag passed on the command-line wins\n+test_expect_success 'difftool last flag wins' '\n+\tdiff=$(git difftool --prompt --no-prompt branch) &&\n+\ttest \"$diff\" = \"branch\" &&\n+\n+\trestore_test_defaults &&\n+\n+\tprompt=$(echo | git difftool --no-prompt --prompt branch | tail -1) &&\n+\tprompt_given \"$prompt\" &&\n+\n+\trestore_test_defaults\n+'\n+\n # git-difftool falls back to git-mergetool config variables\n # so test that behavior here\n test_expect_success 'difftool + mergetool config variables' '\n-- \n1.6.1.3\n"},{"id":"110134","messageId":"1238590514-41893-3-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":"1238590514-41893-2-git-send-email-davvid@gmail.com","subject":"[PATCH 02/10] mergetool: use $( ... ) instead of `backticks`","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:06Z","receivedAt":"2009-04-01T12:55:06Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"This makes mergetool consistent with Documentation/CodingGuidelines.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-mergetool.sh |   16 ++++++++--------\n 1 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 87fa88a..7c04031 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -137,7 +137,7 @@ checkout_staged_file () {\n merge_file () {\n     MERGED=\"$1\"\n \n-    f=`git ls-files -u -- \"$MERGED\"`\n+    f=$(git ls-files -u -- \"$MERGED\")\n     if test -z \"$f\" ; then\n \tif test ! -f \"$MERGED\" ; then\n \t    echo \"$MERGED: file not found\"\n@@ -156,9 +156,9 @@ merge_file () {\n     mv -- \"$MERGED\" \"$BACKUP\"\n     cp -- \"$BACKUP\" \"$MERGED\"\n \n-    base_mode=`git ls-files -u -- \"$MERGED\" | awk '{if ($3==1) print $1;}'`\n-    local_mode=`git ls-files -u -- \"$MERGED\" | awk '{if ($3==2) print $1;}'`\n-    remote_mode=`git ls-files -u -- \"$MERGED\" | awk '{if ($3==3) print $1;}'`\n+    base_mode=$(git ls-files -u -- \"$MERGED\" | awk '{if ($3==1) print $1;}')\n+    local_mode=$(git ls-files -u -- \"$MERGED\" | awk '{if ($3==2) print $1;}')\n+    remote_mode=$(git ls-files -u -- \"$MERGED\" | awk '{if ($3==3) print $1;}')\n \n     base_present   && checkout_staged_file 1 \"$MERGED\" \"$BASE\"\n     local_present  && checkout_staged_file 2 \"$MERGED\" \"$LOCAL\"\n@@ -308,7 +308,7 @@ do\n \t-t|--tool*)\n \t    case \"$#,$1\" in\n \t\t*,*=*)\n-\t\t    merge_tool=`expr \"z$1\" : 'z-[^=]*=\\(.*\\)'`\n+\t\t    merge_tool=$(expr \"z$1\" : 'z-[^=]*=\\(.*\\)')\n \t\t    ;;\n \t\t1,*)\n \t\t    usage ;;\n@@ -356,7 +356,7 @@ valid_tool() {\n }\n \n init_merge_tool_path() {\n-\tmerge_tool_path=`git config mergetool.$1.path`\n+\tmerge_tool_path=$(git config mergetool.$1.path)\n \tif test -z \"$merge_tool_path\" ; then\n \t\tcase \"$1\" in\n \t\t\temerge)\n@@ -387,7 +387,7 @@ prompt_after_failed_merge() {\n }\n \n if test -z \"$merge_tool\"; then\n-    merge_tool=`git config merge.tool`\n+    merge_tool=$(git config merge.tool)\n     if test -n \"$merge_tool\" && ! valid_tool \"$merge_tool\"; then\n \t    echo >&2 \"git config option merge.tool set to unknown tool: $merge_tool\"\n \t    echo >&2 \"Resetting to default...\"\n@@ -447,7 +447,7 @@ last_status=0\n rollup_status=0\n \n if test $# -eq 0 ; then\n-    files=`git ls-files -u | sed -e 's/^[^\t]*\t//' | sort -u`\n+    files=$(git ls-files -u | sed -e 's/^[^\t]*\t//' | sort -u)\n     if test -z \"$files\" ; then\n \techo \"No files need merging\"\n \texit 0\n-- \n1.6.1.3\n"},{"id":"110135","messageId":"1238590514-41893-4-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":"1238590514-41893-3-git-send-email-davvid@gmail.com","subject":"[PATCH 03/10] Add a mergetool-lib scriptlet for holding common merge tool functions","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:07Z","receivedAt":"2009-04-01T12:55:07Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"git-mergetool-lib provides common merge tool functions.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n .gitignore                          |    1 +\n Documentation/git-mergetool-lib.txt |   42 +++++++++++++++++++++++++++++++++++\n Makefile                            |    1 +\n command-list.txt                    |    1 +\n git-mergetool-lib.sh                |   41 ++++++++++++++++++++++++++++++++++\n 5 files changed, 86 insertions(+), 0 deletions(-)\n create mode 100644 Documentation/git-mergetool-lib.txt\n create mode 100644 git-mergetool-lib.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex 966c886..75c154a 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -80,6 +80,7 @@ git-merge-recursive\n git-merge-resolve\n git-merge-subtree\n git-mergetool\n+git-mergetool-lib\n git-mktag\n git-mktree\n git-name-rev\ndiff --git a/Documentation/git-mergetool-lib.txt b/Documentation/git-mergetool-lib.txt\nnew file mode 100644\nindex 0000000..a8d62f5\n--- /dev/null\n+++ b/Documentation/git-mergetool-lib.txt\n@@ -0,0 +1,42 @@\n+git-mergetool-lib(1)\n+====================\n+\n+NAME\n+----\n+git-mergetool-lib - Common git merge tool shell scriptlets\n+\n+SYNOPSIS\n+--------\n+'. \"$(git --exec-path)/git-mergetool-lib\"'\n+\n+DESCRIPTION\n+-----------\n+\n+This is not a command the end user would want to run.  Ever.\n+This documentation is meant for people who are studying the\n+Porcelain-ish scripts and/or are writing new ones.\n+\n+The 'git-mergetool-lib' scriptlet is designed to be sourced (using\n+`.`) by other shell scripts to set up functions for working\n+with git merge tools.\n+\n+Before sourcing it, your script should set up a few variables;\n+`TOOL_MODE` is used to define the operation mode for various\n+functions.  'diff' and 'merge' are valid values.\n+\n+FUNCTIONS\n+---------\n+get_merge_tool_path::\n+\treturns the `merge_tool_path` for a `merge_tool`.\n+\n+Author\n+------\n+Written by David Aguilar <davvid@gmail.com>\n+\n+Documentation\n+--------------\n+Documentation by David Aguilar and the git-list <git@vger.kernel.org>.\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex d77fd71..086f9e7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -284,6 +284,7 @@ SCRIPT_SH += git-merge-octopus.sh\n SCRIPT_SH += git-merge-one-file.sh\n SCRIPT_SH += git-merge-resolve.sh\n SCRIPT_SH += git-mergetool.sh\n+SCRIPT_SH += git-mergetool-lib.sh\n SCRIPT_SH += git-parse-remote.sh\n SCRIPT_SH += git-pull.sh\n SCRIPT_SH += git-quiltimport.sh\ndiff --git a/command-list.txt b/command-list.txt\nindex fb03a2e..922c815 100644\n--- a/command-list.txt\n+++ b/command-list.txt\n@@ -69,6 +69,7 @@ git-merge-file                          plumbingmanipulators\n git-merge-index                         plumbingmanipulators\n git-merge-one-file                      purehelpers\n git-mergetool                           ancillarymanipulators\n+git-mergetool-lib                       purehelpers\n git-merge-tree                          ancillaryinterrogators\n git-mktag                               plumbingmanipulators\n git-mktree                              plumbingmanipulators\ndiff --git a/git-mergetool-lib.sh b/git-mergetool-lib.sh\nnew file mode 100644\nindex 0000000..c307a5d\n--- /dev/null\n+++ b/git-mergetool-lib.sh\n@@ -0,0 +1,41 @@\n+# git-mergetool-lib is a library for common merge tool functions\n+diff_mode() {\n+\ttest $TOOL_MODE = \"diff\"\n+}\n+\n+get_merge_tool_path () {\n+\tif test -z \"$2\"; then\n+\t\tcase \"$1\" in\n+\t\temerge)\n+\t\t\tpath=emacs\n+\t\t\t;;\n+\t\t*)\n+\t\t\tpath=\"$1\"\n+\t\t\t;;\n+\t\tesac\n+\tfi\n+\techo \"$path\"\n+}\n+\n+valid_tool () {\n+\tcase \"$1\" in\n+\tkdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)\n+\t\tif test \"$1\" = \"kompare\" && ! diff_mode; then\n+\t\t\treturn 1\n+\t\tfi\n+\t\t;; # happy\n+\t*)\n+\t\tif ! test -n \"$(get_custom_cmd \"$1\")\"; then\n+\t\t\treturn 1\n+\t\tfi ;;\n+\tesac\n+}\n+\n+get_custom_cmd () {\n+\tdiff_mode &&\n+\tcustom_cmd=\"$(git config difftool.$1.cmd)\"\n+\ttest -z \"$custom_cmd\" &&\n+\tcustom_cmd=\"$(git config mergetool.$1.cmd)\"\n+\ttest -n \"$custom_cmd\" &&\n+\techo \"$custom_cmd\"\n+}\n-- \n1.6.1.3\n"},{"id":"110137","messageId":"1238590514-41893-5-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":"1238590514-41893-4-git-send-email-davvid@gmail.com","subject":"[PATCH 04/10] mergetool: use get_mergetool_path from git-mergetool-lib","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:08Z","receivedAt":"2009-04-01T12:55:08Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"This refactors git-mergetool to use get_mergetool_path().\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-mergetool.sh |   20 +++-----------------\n 1 files changed, 3 insertions(+), 17 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 7c04031..732a5b7 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -12,6 +12,7 @@ USAGE='[--tool=tool] [-y|--no-prompt|--prompt] [file to merge] ...'\n SUBDIRECTORY_OK=Yes\n OPTIONS_SPEC=\n . git-sh-setup\n+. git-mergetool-lib\n require_work_tree\n \n # Returns true if the mode reflects a symlink\n@@ -355,20 +356,6 @@ valid_tool() {\n \tesac\n }\n \n-init_merge_tool_path() {\n-\tmerge_tool_path=$(git config mergetool.$1.path)\n-\tif test -z \"$merge_tool_path\" ; then\n-\t\tcase \"$1\" in\n-\t\t\temerge)\n-\t\t\t\tmerge_tool_path=emacs\n-\t\t\t\t;;\n-\t\t\t*)\n-\t\t\t\tmerge_tool_path=$1\n-\t\t\t\t;;\n-\t\tesac\n-\tfi\n-}\n-\n prompt_after_failed_merge() {\n     while true; do\n \tprintf \"Continue merging other unresolved paths (y/n) ? \"\n@@ -412,7 +399,7 @@ if test -z \"$merge_tool\" ; then\n     fi\n     echo \"merge tool candidates: $merge_tool_candidates\"\n     for i in $merge_tool_candidates; do\n-        init_merge_tool_path $i\n+        merge_tool_path=\"$(get_merge_tool_path \"$i\" \"$merge_tool_path\")\"\n         if type \"$merge_tool_path\" > /dev/null 2>&1; then\n             merge_tool=$i\n             break\n@@ -428,8 +415,7 @@ else\n         exit 1\n     fi\n \n-    init_merge_tool_path \"$merge_tool\"\n-\n+    merge_tool_path=\"$(get_merge_tool_path \"$merge_tool\")\"\n     merge_keep_backup=\"$(git config --bool merge.keepBackup || echo true)\"\n     merge_keep_temporaries=\"$(git config --bool mergetool.keepTemporaries || echo false)\"\n \n-- \n1.6.1.3\n"},{"id":"110130","messageId":"1238590514-41893-6-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":"1238590514-41893-5-git-send-email-davvid@gmail.com","subject":"[PATCH 05/10] difftool: use get_mergetool_path from git-mergetool-lib","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:09Z","receivedAt":"2009-04-01T12:55:09Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"This refactors git-difftool-helper to use get_mergetool_path().\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-difftool-helper.sh |   33 ++++++++++++---------------------\n 1 files changed, 12 insertions(+), 21 deletions(-)\n\ndiff --git a/git-difftool-helper.sh b/git-difftool-helper.sh\nindex 02bb135..d1bea18 100755\n--- a/git-difftool-helper.sh\n+++ b/git-difftool-helper.sh\n@@ -7,6 +7,9 @@\n #\n # Copyright (c) 2009 David Aguilar\n \n+# Load common functions from git-mergetool-lib\n+. git-mergetool-lib\n+\n # difftool.prompt controls the default prompt/no-prompt behavior\n # and is overridden with $GIT_DIFFTOOL*_PROMPT.\n should_prompt () {\n@@ -125,25 +128,6 @@ valid_tool() {\n \tesac\n }\n \n-# Sets up the merge_tool_path variable.\n-# This handles the difftool.<tool>.path configuration.\n-# This also falls back to mergetool defaults.\n-init_merge_tool_path() {\n-\tmerge_tool_path=$(git config difftool.\"$1\".path)\n-\ttest -z \"$merge_tool_path\" &&\n-\tmerge_tool_path=$(git config mergetool.\"$1\".path)\n-\tif test -z \"$merge_tool_path\"; then\n-\t\tcase \"$1\" in\n-\t\temerge)\n-\t\t\tmerge_tool_path=emacs\n-\t\t\t;;\n-\t\t*)\n-\t\t\tmerge_tool_path=\"$1\"\n-\t\t\t;;\n-\t\tesac\n-\tfi\n-}\n-\n # Allow GIT_DIFF_TOOL and GIT_MERGE_TOOL to provide default values\n test -n \"$GIT_MERGE_TOOL\" && merge_tool=\"$GIT_MERGE_TOOL\"\n test -n \"$GIT_DIFF_TOOL\" && merge_tool=\"$GIT_DIFF_TOOL\"\n@@ -187,7 +171,7 @@ if test -z \"$merge_tool\"; then\n \t# Loop over each candidate and stop when a valid merge tool is found.\n \tfor i in $merge_tool_candidates\n \tdo\n-\t\tinit_merge_tool_path $i\n+\t\tmerge_tool_path=\"$(get_merge_tool_path \"$i\")\"\n \t\tif type \"$merge_tool_path\" > /dev/null 2>&1; then\n \t\t\tmerge_tool=$i\n \t\t\tbreak\n@@ -206,7 +190,14 @@ else\n \t\texit 1\n \tfi\n \n-\tinit_merge_tool_path \"$merge_tool\"\n+\t# Sets up the merge_tool_path variable.\n+\t# This handles the difftool.<tool>.path configuration variable\n+\t# and falls back to mergetool defaults.\n+\tmerge_tool_path=$(git config difftool.\"$1\".path)\n+\ttest -z \"$merge_tool_path\" &&\n+\tmerge_tool_path=$(git config mergetool.\"$1\".path)\n+\tmerge_tool_path=\"$(get_merge_tool_path \"$merge_tool\" \\\n+\t                                       \"$merge_tool_path\")\"\n \n \tif test -z \"$merge_tool_cmd\" && ! type \"$merge_tool_path\" > /dev/null 2>&1; then\n \t\techo \"The merge tool $merge_tool is not available as '$merge_tool_path'\"\n-- \n1.6.1.3\n"},{"id":"110131","messageId":"1238590514-41893-7-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":"1238590514-41893-6-git-send-email-davvid@gmail.com","subject":"[PATCH 06/10] mergetool: use valid_tool from git-mergetool-lib","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:10Z","receivedAt":"2009-04-01T12:55:10Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"This refactors git-mergetool to use valid_tool from git-mergetool-lib.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-mergetool.sh |   21 +++------------------\n 1 files changed, 3 insertions(+), 18 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 732a5b7..957993c 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -11,6 +11,7 @@\n USAGE='[--tool=tool] [-y|--no-prompt|--prompt] [file to merge] ...'\n SUBDIRECTORY_OK=Yes\n OPTIONS_SPEC=\n+TOOL_MODE=merge\n . git-sh-setup\n . git-mergetool-lib\n require_work_tree\n@@ -338,24 +339,6 @@ do\n     shift\n done\n \n-valid_custom_tool()\n-{\n-    merge_tool_cmd=\"$(git config mergetool.$1.cmd)\"\n-    test -n \"$merge_tool_cmd\"\n-}\n-\n-valid_tool() {\n-\tcase \"$1\" in\n-\t\tkdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)\n-\t\t\t;; # happy\n-\t\t*)\n-\t\t\tif ! valid_custom_tool \"$1\"; then\n-\t\t\t\treturn 1\n-\t\t\tfi\n-\t\t\t;;\n-\tesac\n-}\n-\n prompt_after_failed_merge() {\n     while true; do\n \tprintf \"Continue merging other unresolved paths (y/n) ? \"\n@@ -409,12 +392,14 @@ if test -z \"$merge_tool\" ; then\n \techo \"No known merge resolution program available.\"\n \texit 1\n     fi\n+    merge_tool_cmd=\"$(git config mergetool.\"$merge_tool\".cmd)\"\n else\n     if ! valid_tool \"$merge_tool\"; then\n         echo >&2 \"Unknown merge_tool $merge_tool\"\n         exit 1\n     fi\n \n+    merge_tool_cmd=\"$(git config mergetool.$merge_tool.cmd)\"\n     merge_tool_path=\"$(get_merge_tool_path \"$merge_tool\")\"\n     merge_keep_backup=\"$(git config --bool merge.keepBackup || echo true)\"\n     merge_keep_temporaries=\"$(git config --bool mergetool.keepTemporaries || echo false)\"\n-- \n1.6.1.3\n"},{"id":"110133","messageId":"1238590514-41893-8-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":"1238590514-41893-7-git-send-email-davvid@gmail.com","subject":"[PATCH 07/10] difftool: use valid_tool from git-mergetool-lib","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:11Z","receivedAt":"2009-04-01T12:55:11Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"This refactors difftool to use valid_tool fro git-mergetool-lib\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-difftool-helper.sh |   29 +++--------------------------\n 1 files changed, 3 insertions(+), 26 deletions(-)\n\ndiff --git a/git-difftool-helper.sh b/git-difftool-helper.sh\nindex d1bea18..6cc5ab5 100755\n--- a/git-difftool-helper.sh\n+++ b/git-difftool-helper.sh\n@@ -8,6 +8,7 @@\n # Copyright (c) 2009 David Aguilar\n \n # Load common functions from git-mergetool-lib\n+TOOL_MODE=diff\n . git-mergetool-lib\n \n # difftool.prompt controls the default prompt/no-prompt behavior\n@@ -105,29 +106,6 @@ launch_merge_tool () {\n \tesac\n }\n \n-# Verifies that (difftool|mergetool).<tool>.cmd exists\n-valid_custom_tool() {\n-\tmerge_tool_cmd=\"$(git config difftool.$1.cmd)\"\n-\ttest -z \"$merge_tool_cmd\" &&\n-\tmerge_tool_cmd=\"$(git config mergetool.$1.cmd)\"\n-\ttest -n \"$merge_tool_cmd\"\n-}\n-\n-# Verifies that the chosen merge tool is properly setup.\n-# Built-in merge tools are always valid.\n-valid_tool() {\n-\tcase \"$1\" in\n-\tkdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)\n-\t\t;; # happy\n-\t*)\n-\t\tif ! valid_custom_tool \"$1\"\n-\t\tthen\n-\t\t\treturn 1\n-\t\tfi\n-\t\t;;\n-\tesac\n-}\n-\n # Allow GIT_DIFF_TOOL and GIT_MERGE_TOOL to provide default values\n test -n \"$GIT_MERGE_TOOL\" && merge_tool=\"$GIT_MERGE_TOOL\"\n test -n \"$GIT_DIFF_TOOL\" && merge_tool=\"$GIT_DIFF_TOOL\"\n@@ -183,6 +161,7 @@ if test -z \"$merge_tool\"; then\n \t\texit 1\n \tfi\n \n+\tmerge_tool_cmd=$(get_custom_cmd \"$merge_tool\")\n else\n \t# A merge tool has been set, so verify that it's valid.\n \tif ! valid_tool \"$merge_tool\"; then\n@@ -190,9 +169,7 @@ else\n \t\texit 1\n \tfi\n \n-\t# Sets up the merge_tool_path variable.\n-\t# This handles the difftool.<tool>.path configuration variable\n-\t# and falls back to mergetool defaults.\n+\tmerge_tool_cmd=$(get_custom_cmd \"$merge_tool\")\n \tmerge_tool_path=$(git config difftool.\"$1\".path)\n \ttest -z \"$merge_tool_path\" &&\n \tmerge_tool_path=$(git config mergetool.\"$1\".path)\n-- \n1.6.1.3\n"},{"id":"110138","messageId":"1238590514-41893-9-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":"1238590514-41893-8-git-send-email-davvid@gmail.com","subject":"[PATCH 08/10] mergetool-lib: introduce run_mergetool","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:12Z","receivedAt":"2009-04-01T12:55:12Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"run_mergetool contains the common functionality for launching\nmergetools.  There's some context-specific behavior but overall\nit's better to have all of this stuff in one place versus multiple\nplaces.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n Documentation/git-mergetool-lib.txt |    4 +\n git-mergetool-lib.sh                |  141 +++++++++++++++++++++++++++++++++++\n 2 files changed, 145 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-mergetool-lib.txt b/Documentation/git-mergetool-lib.txt\nindex a8d62f5..3060673 100644\n--- a/Documentation/git-mergetool-lib.txt\n+++ b/Documentation/git-mergetool-lib.txt\n@@ -29,6 +29,10 @@ FUNCTIONS\n get_merge_tool_path::\n \treturns the `merge_tool_path` for a `merge_tool`.\n \n+run_mergetool::\n+\tlaunches a merge tool given the tool name and a true/false\n+\tflag to indicate whether a merge base is present\n+\n Author\n ------\n Written by David Aguilar <davvid@gmail.com>\ndiff --git a/git-mergetool-lib.sh b/git-mergetool-lib.sh\nindex c307a5d..9c26b5f 100644\n--- a/git-mergetool-lib.sh\n+++ b/git-mergetool-lib.sh\n@@ -3,7 +3,12 @@ diff_mode() {\n \ttest $TOOL_MODE = \"diff\"\n }\n \n+merge_mode() {\n+\ttest $TOOL_MODE = \"merge\"\n+}\n+\n get_merge_tool_path () {\n+\tpath=\"$1\"\n \tif test -z \"$2\"; then\n \t\tcase \"$1\" in\n \t\temerge)\n@@ -17,6 +22,11 @@ get_merge_tool_path () {\n \techo \"$path\"\n }\n \n+# Overridden in git-mergetool\n+check_unchanged () {\n+\ttrue\n+}\n+\n valid_tool () {\n \tcase \"$1\" in\n \tkdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)\n@@ -39,3 +49,134 @@ get_custom_cmd () {\n \ttest -n \"$custom_cmd\" &&\n \techo \"$custom_cmd\"\n }\n+\n+run_mergetool () {\n+\tbase_present=\"$2\"\n+\tif diff_mode; then\n+\t\tbase_present=\"false\"\n+\tfi\n+\tif test -z \"$base_present\"; then\n+\t\tbase_present=\"true\"\n+\tfi\n+\n+\tcase \"$1\" in\n+\tkdiff3)\n+\t\tif $base_present; then\n+\t\t\t(\"$merge_tool_path\" --auto \\\n+\t\t\t --L1 \"$MERGED (Base)\" --L2 \"$MERGED (Local)\" --L3 \"$MERGED (Remote)\" \\\n+\t\t\t -o \"$MERGED\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" > /dev/null 2>&1)\n+\t\telse\n+\t\t\t(\"$merge_tool_path\" --auto \\\n+\t\t\t --L1 \"$MERGED (Local)\" --L2 \"$MERGED (Remote)\" \\\n+\t\t\t -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\" > /dev/null 2>&1)\n+\t\tfi\n+\t\tstatus=$?\n+\t\t;;\n+\tkompare)\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+\t\tstatus=$?\n+\t\t;;\n+\ttkdiff)\n+\t\tif $base_present; then\n+\t\t\t\"$merge_tool_path\" -a \"$BASE\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n+\t\telse\n+\t\t\tif diff_mode; then\n+\t\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+\t\t\telse\n+\t\t\t\t\"$merge_tool_path\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n+\t\t\tfi\n+\t\tfi\n+\t\tstatus=$?\n+\t\t;;\n+\tmeld)\n+\t\tmerge_mode && touch \"$BACKUP\"\n+\t\tif merge_mode; then\n+\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\t\telse\n+\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+\t\tfi\n+\t\tcheck_unchanged\n+\t\t;;\n+\tvimdiff)\n+\t\tif merge_mode; then\n+\t\t\ttouch \"$BACKUP\"\n+\t\t\t\"$merge_tool_path\" -c \"wincmd l\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\t\t\tcheck_unchanged\n+\t\telse\n+\t\t\t\"$merge_tool_path\" -c \"wincmd l\" \"$LOCAL\" \"$REMOTE\"\n+\t\tfi\n+\t\t;;\n+\tgvimdiff)\n+\t\tif merge_mode; then\n+\t\t\ttouch \"$BACKUP\"\n+\t\t\t\"$merge_tool_path\" -c \"wincmd l\" -f \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\t\t\tcheck_unchanged\n+\t\telse\n+\t\t\t\"$merge_tool_path\" -c \"wincmd l\" -f \"$LOCAL\" \"$REMOTE\"\n+\t\tfi\n+\t\t;;\n+\txxdiff)\n+\t\tmerge_mode && touch \"$BACKUP\"\n+\t\tif $base_present; then\n+\t\t\t\"$merge_tool_path\" -X --show-merged-pane \\\n+\t\t\t    -R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n+\t\t\t    -R 'Accel.Search: \"Ctrl+F\"' \\\n+\t\t\t    -R 'Accel.SearchForward: \"Ctrl-G\"' \\\n+\t\t\t    --merged-file \"$MERGED\" \"$LOCAL\" \"$BASE\" \"$REMOTE\"\n+\t\telse\n+\t\t\tmerge_mode && extra=--show-merged-pane\n+\t\t\t\"$merge_tool_path\" -X $extra \\\n+\t\t\t\t-R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n+\t\t\t\t-R 'Accel.Search: \"Ctrl+F\"' \\\n+\t\t\t\t-R 'Accel.SearchForward: \"Ctrl-G\"' \\\n+\t\t\t\t--merged-file \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n+\t\tfi\n+\t\tcheck_unchanged\n+\t\t;;\n+\topendiff)\n+\t\tmerge_mode && touch \"$BACKUP\"\n+\t\tif $base_present; then\n+\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t\t-ancestor \"$BASE\" -merge \"$MERGED\" | cat\n+\t\telse\n+\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t\t-merge \"$MERGED\" | cat\n+\t\tfi\n+\t\tcheck_unchanged\n+\t\t;;\n+\tecmerge)\n+\t\tmerge_mode && touch \"$BACKUP\"\n+\t\tif $base_present; then\n+\t\t\t\"$merge_tool_path\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t\t--default --mode=merge3 --to=\"$MERGED\"\n+\t\telse\n+\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t\t--default --mode=merge2 --to=\"$MERGED\"\n+\t\tfi\n+\t\tcheck_unchanged\n+\t\t;;\n+\temerge)\n+\t\tif $base_present; then\n+\t\t\t\"$merge_tool_path\" -f emerge-files-with-ancestor-command \\\n+\t\t\t\t\"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$(basename \"$MERGED\")\"\n+\t\telse\n+\t\t\t\"$merge_tool_path\" -f emerge-files-command \\\n+\t\t\t\t\"$LOCAL\" \"$REMOTE\" \"$(basename \"$MERGED\")\"\n+\t\tfi\n+\t\tstatus=$?\n+\t\t;;\n+\t*)\n+\t\tif test -n \"$merge_tool_cmd\"; then\n+\t\t\tif merge_mode &&\n+\t\t\ttest \"$merge_tool_trust_exit_code\" = \"false\"; then\n+\t\t\t\ttouch \"$BACKUP\"\n+\t\t\t\t( eval $merge_tool_cmd )\n+\t\t\t\tcheck_unchanged\n+\t\t\telse\n+\t\t\t\t( eval $merge_tool_cmd )\n+\t\t\t\tstatus=$?\n+\t\t\tfi\n+\t\tfi\n+\t\t;;\n+\tesac\n+}\n-- \n1.6.1.3\n"},{"id":"110132","messageId":"1238590514-41893-10-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":"1238590514-41893-9-git-send-email-davvid@gmail.com","subject":"[PATCH 09/10] difftool: use run_mergetool from git-mergetool-lib","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:13Z","receivedAt":"2009-04-01T12:55:13Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"This refactors git-mergetool-helper to use run_mergetool.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-difftool-helper.sh |   62 +-----------------------------------------------\n 1 files changed, 1 insertions(+), 61 deletions(-)\n\ndiff --git a/git-difftool-helper.sh b/git-difftool-helper.sh\nindex 6cc5ab5..2dfc870 100755\n--- a/git-difftool-helper.sh\n+++ b/git-difftool-helper.sh\n@@ -43,67 +43,7 @@ launch_merge_tool () {\n \tfi\n \n \t# Run the appropriate merge tool command\n-\tcase \"$merge_tool\" in\n-\tkdiff3)\n-\t\tbasename=$(basename \"$MERGED\")\n-\t\t\"$merge_tool_path\" --auto \\\n-\t\t\t--L1 \"$basename (A)\" \\\n-\t\t\t--L2 \"$basename (B)\" \\\n-\t\t\t-o \"$MERGED\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t> /dev/null 2>&1\n-\t\t;;\n-\n-\tkompare)\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n-\t\t;;\n-\n-\ttkdiff)\n-\t\t\"$merge_tool_path\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n-\t\t;;\n-\n-\tmeld)\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n-\t\t;;\n-\n-\tvimdiff)\n-\t\t\"$merge_tool_path\" -c \"wincmd l\" \"$LOCAL\" \"$REMOTE\"\n-\t\t;;\n-\n-\tgvimdiff)\n-\t\t\"$merge_tool_path\" -c \"wincmd l\" -f \"$LOCAL\" \"$REMOTE\"\n-\t\t;;\n-\n-\txxdiff)\n-\t\t\"$merge_tool_path\" \\\n-\t\t\t-X \\\n-\t\t\t-R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n-\t\t\t-R 'Accel.Search: \"Ctrl+F\"' \\\n-\t\t\t-R 'Accel.SearchForward: \"Ctrl-G\"' \\\n-\t\t\t--merged-file \"$MERGED\" \\\n-\t\t\t\"$LOCAL\" \"$REMOTE\"\n-\t\t;;\n-\n-\topendiff)\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t-merge \"$MERGED\" | cat\n-\t\t;;\n-\n-\tecmerge)\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t--default --mode=merge2 --to=\"$MERGED\"\n-\t\t;;\n-\n-\temerge)\n-\t\t\"$merge_tool_path\" -f emerge-files-command \\\n-\t\t\t\"$LOCAL\" \"$REMOTE\" \"$(basename \"$MERGED\")\"\n-\t\t;;\n-\n-\t*)\n-\t\tif test -n \"$merge_tool_cmd\"; then\n-\t\t\t( eval $merge_tool_cmd )\n-\t\tfi\n-\t\t;;\n-\tesac\n+\trun_mergetool \"$merge_tool\";\n }\n \n # Allow GIT_DIFF_TOOL and GIT_MERGE_TOOL to provide default values\n-- \n1.6.1.3\n"},{"id":"110139","messageId":"1238590514-41893-11-git-send-email-davvid@gmail.com","threadId":"18681","inReplyTo":"1238590514-41893-10-git-send-email-davvid@gmail.com","subject":"[PATCH 10/10] mergetool: use run_mergetool from git-mergetool-lib","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-01T12:55:14Z","receivedAt":"2009-04-01T12:55:14Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"This refactors git-mergetool to use run_mergetool.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-mergetool.sh |   96 +++--------------------------------------------------\n 1 files changed, 6 insertions(+), 90 deletions(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 957993c..2c6b325 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -190,96 +190,12 @@ merge_file () {\n \tread ans\n     fi\n \n-    case \"$merge_tool\" in\n-\tkdiff3)\n-\t    if base_present ; then\n-\t\t(\"$merge_tool_path\" --auto --L1 \"$MERGED (Base)\" --L2 \"$MERGED (Local)\" --L3 \"$MERGED (Remote)\" \\\n-\t\t    -o \"$MERGED\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" > /dev/null 2>&1)\n-\t    else\n-\t\t(\"$merge_tool_path\" --auto --L1 \"$MERGED (Local)\" --L2 \"$MERGED (Remote)\" \\\n-\t\t    -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\" > /dev/null 2>&1)\n-\t    fi\n-\t    status=$?\n-\t    ;;\n-\ttkdiff)\n-\t    if base_present ; then\n-\t\t\"$merge_tool_path\" -a \"$BASE\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n-\t    else\n-\t\t\"$merge_tool_path\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n-\t    fi\n-\t    status=$?\n-\t    ;;\n-\tmeld)\n-\t    touch \"$BACKUP\"\n-\t    \"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n-\t    check_unchanged\n-\t    ;;\n-\tvimdiff)\n-\t    touch \"$BACKUP\"\n-\t    \"$merge_tool_path\" -c \"wincmd l\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n-\t    check_unchanged\n-\t    ;;\n-\tgvimdiff)\n-\t    touch \"$BACKUP\"\n-\t    \"$merge_tool_path\" -c \"wincmd l\" -f \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n-\t    check_unchanged\n-\t    ;;\n-\txxdiff)\n-\t    touch \"$BACKUP\"\n-\t    if base_present ; then\n-\t\t\"$merge_tool_path\" -X --show-merged-pane \\\n-\t\t    -R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n-\t\t    -R 'Accel.Search: \"Ctrl+F\"' \\\n-\t\t    -R 'Accel.SearchForward: \"Ctrl-G\"' \\\n-\t\t    --merged-file \"$MERGED\" \"$LOCAL\" \"$BASE\" \"$REMOTE\"\n-\t    else\n-\t\t\"$merge_tool_path\" -X --show-merged-pane \\\n-\t\t    -R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n-\t\t    -R 'Accel.Search: \"Ctrl+F\"' \\\n-\t\t    -R 'Accel.SearchForward: \"Ctrl-G\"' \\\n-\t\t    --merged-file \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n-\t    fi\n-\t    check_unchanged\n-\t    ;;\n-\topendiff)\n-\t    touch \"$BACKUP\"\n-\t    if base_present; then\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" -ancestor \"$BASE\" -merge \"$MERGED\" | cat\n-\t    else\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" -merge \"$MERGED\" | cat\n-\t    fi\n-\t    check_unchanged\n-\t    ;;\n-\tecmerge)\n-\t    touch \"$BACKUP\"\n-\t    if base_present; then\n-\t\t\"$merge_tool_path\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" --default --mode=merge3 --to=\"$MERGED\"\n-\t    else\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" --default --mode=merge2 --to=\"$MERGED\"\n-\t    fi\n-\t    check_unchanged\n-\t    ;;\n-\temerge)\n-\t    if base_present ; then\n-\t\t\"$merge_tool_path\" -f emerge-files-with-ancestor-command \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$(basename \"$MERGED\")\"\n-\t    else\n-\t\t\"$merge_tool_path\" -f emerge-files-command \"$LOCAL\" \"$REMOTE\" \"$(basename \"$MERGED\")\"\n-\t    fi\n-\t    status=$?\n-\t    ;;\n-\t*)\n-\t    if test -n \"$merge_tool_cmd\"; then\n-\t\tif test \"$merge_tool_trust_exit_code\" = \"false\"; then\n-\t\t    touch \"$BACKUP\"\n-\t\t    ( eval $merge_tool_cmd )\n-\t\t    check_unchanged\n-\t\telse\n-\t\t    ( eval $merge_tool_cmd )\n-\t\t    status=$?\n-\t\tfi\n-\t    fi\n-\t    ;;\n-    esac\n+    present=false\n+    base_present &&\n+    present=true\n+\n+    run_mergetool \"$merge_tool\" \"$present\"\n+    status=$?\n     if test \"$status\" -ne 0; then\n \techo \"merge of $MERGED failed\" 1>&2\n \tmv -- \"$BACKUP\" \"$MERGED\"\n-- \n1.6.1.3\n"},{"id":"110171","messageId":"200904020039.41894.markus.heidelberg@web.de","threadId":"18681","inReplyTo":"1238590514-41893-4-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 03/10] Add a mergetool-lib scriptlet for holding common merge tool functions","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-04-01T22:39:41Z","receivedAt":"2009-04-01T22:39:41Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"David Aguilar, 01.04.2009:\n> \n> diff --git a/git-mergetool-lib.sh b/git-mergetool-lib.sh\n> +valid_tool () {\n> +\tcase \"$1\" in\n> +\tkdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)\n> +\t\tif test \"$1\" = \"kompare\" && ! diff_mode; then\n> +\t\t\treturn 1\n> +\t\tfi\n> +\t\t;; # happy\n> +\t*)\n> +\t\tif ! test -n \"$(get_custom_cmd \"$1\")\"; then\n\nBetter this?\n\t\tif test -z \"$(get_custom_cmd \"$1\")\"; then\n\n> +\t\t\treturn 1\n> +\t\tfi ;;\n\nFor consistency:\n\t\tfi\n\t\t;;\n"},{"id":"110172","messageId":"200904020047.41561.markus.heidelberg@web.de","threadId":"18681","inReplyTo":"1238590514-41893-9-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 08/10] mergetool-lib: introduce run_mergetool","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-04-01T22:47:40Z","receivedAt":"2009-04-01T22:47:40Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"David Aguilar, 01.04.2009:\n> diff --git a/git-mergetool-lib.sh b/git-mergetool-lib.sh\n> +run_mergetool () {\n> +\tbase_present=\"$2\"\n> +\tif diff_mode; then\n> +\t\tbase_present=\"false\"\n> +\tfi\n> +\tif test -z \"$base_present\"; then\n> +\t\tbase_present=\"true\"\n> +\tfi\n> +\n> +\tcase \"$1\" in\n> +\tkdiff3)\n> +\t\tif $base_present; then\n> +\t\t\t(\"$merge_tool_path\" --auto \\\n> +\t\t\t --L1 \"$MERGED (Base)\" --L2 \"$MERGED (Local)\" --L3 \"$MERGED (Remote)\" \\\n> +\t\t\t -o \"$MERGED\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" > /dev/null 2>&1)\n> +\t\telse\n> +\t\t\t(\"$merge_tool_path\" --auto \\\n> +\t\t\t --L1 \"$MERGED (Local)\" --L2 \"$MERGED (Remote)\" \\\n> +\t\t\t -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\" > /dev/null 2>&1)\n> +\t\tfi\n> +\t\tstatus=$?\n> +\t\t;;\n> +\tkompare)\n> +\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n> +\t\tstatus=$?\n> +\t\t;;\n> +\ttkdiff)\n> +\t\tif $base_present; then\n> +\t\t\t\"$merge_tool_path\" -a \"$BASE\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n> +\t\telse\n> +\t\t\tif diff_mode; then\n\nQuery merge_mode instead of diff_mode as in all the other places.\n\n> +\t\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n> +\t\t\telse\n> +\t\t\t\t\"$merge_tool_path\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n> +\t\t\tfi\n> +\t\tfi\n> +\t\tstatus=$?\n> +\t\t;;\n> +\tmeld)\n> +\t\tmerge_mode && touch \"$BACKUP\"\n> +\t\tif merge_mode; then\n> +\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n\nPut the touch command into the if then block as in the other places.\n\n> +\t\telse\n> +\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n> +\t\tfi\n> +\t\tcheck_unchanged\n> +\t\t;;\n> +\tvimdiff)\n> +\t\tif merge_mode; then\n> +\t\t\ttouch \"$BACKUP\"\n> +\t\t\t\"$merge_tool_path\" -c \"wincmd l\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n> +\t\t\tcheck_unchanged\n> +\t\telse\n> +\t\t\t\"$merge_tool_path\" -c \"wincmd l\" \"$LOCAL\" \"$REMOTE\"\n> +\t\tfi\n> +\t\t;;\n> +\tgvimdiff)\n> +\t\tif merge_mode; then\n> +\t\t\ttouch \"$BACKUP\"\n> +\t\t\t\"$merge_tool_path\" -c \"wincmd l\" -f \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n> +\t\t\tcheck_unchanged\n> +\t\telse\n> +\t\t\t\"$merge_tool_path\" -c \"wincmd l\" -f \"$LOCAL\" \"$REMOTE\"\n> +\t\tfi\n> +\t\t;;\n> +\txxdiff)\n> +\t\tmerge_mode && touch \"$BACKUP\"\n> +\t\tif $base_present; then\n> +\t\t\t\"$merge_tool_path\" -X --show-merged-pane \\\n> +\t\t\t    -R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n> +\t\t\t    -R 'Accel.Search: \"Ctrl+F\"' \\\n> +\t\t\t    -R 'Accel.SearchForward: \"Ctrl-G\"' \\\n> +\t\t\t    --merged-file \"$MERGED\" \"$LOCAL\" \"$BASE\" \"$REMOTE\"\n> +\t\telse\n> +\t\t\tmerge_mode && extra=--show-merged-pane\n> +\t\t\t\"$merge_tool_path\" -X $extra \\\n> +\t\t\t\t-R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n> +\t\t\t\t-R 'Accel.Search: \"Ctrl+F\"' \\\n> +\t\t\t\t-R 'Accel.SearchForward: \"Ctrl-G\"' \\\n> +\t\t\t\t--merged-file \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n> +\t\tfi\n> +\t\tcheck_unchanged\n> +\t\t;;\n> +\topendiff)\n> +\t\tmerge_mode && touch \"$BACKUP\"\n> +\t\tif $base_present; then\n> +\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n> +\t\t\t\t-ancestor \"$BASE\" -merge \"$MERGED\" | cat\n> +\t\telse\n> +\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n> +\t\t\t\t-merge \"$MERGED\" | cat\n> +\t\tfi\n> +\t\tcheck_unchanged\n> +\t\t;;\n> +\tecmerge)\n> +\t\tmerge_mode && touch \"$BACKUP\"\n> +\t\tif $base_present; then\n> +\t\t\t\"$merge_tool_path\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" \\\n> +\t\t\t\t--default --mode=merge3 --to=\"$MERGED\"\n> +\t\telse\n> +\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n> +\t\t\t\t--default --mode=merge2 --to=\"$MERGED\"\n> +\t\tfi\n> +\t\tcheck_unchanged\n> +\t\t;;\n> +\temerge)\n> +\t\tif $base_present; then\n> +\t\t\t\"$merge_tool_path\" -f emerge-files-with-ancestor-command \\\n> +\t\t\t\t\"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$(basename \"$MERGED\")\"\n> +\t\telse\n> +\t\t\t\"$merge_tool_path\" -f emerge-files-command \\\n> +\t\t\t\t\"$LOCAL\" \"$REMOTE\" \"$(basename \"$MERGED\")\"\n> +\t\tfi\n> +\t\tstatus=$?\n> +\t\t;;\n> +\t*)\n> +\t\tif test -n \"$merge_tool_cmd\"; then\n> +\t\t\tif merge_mode &&\n> +\t\t\ttest \"$merge_tool_trust_exit_code\" = \"false\"; then\n> +\t\t\t\ttouch \"$BACKUP\"\n> +\t\t\t\t( eval $merge_tool_cmd )\n> +\t\t\t\tcheck_unchanged\n> +\t\t\telse\n> +\t\t\t\t( eval $merge_tool_cmd )\n> +\t\t\t\tstatus=$?\n> +\t\t\tfi\n> +\t\tfi\n> +\t\t;;\n> +\tesac\n> +}\n"},{"id":"110174","messageId":"200904020054.23147.markus.heidelberg@web.de","threadId":"18681","inReplyTo":"1238590514-41893-11-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 10/10] mergetool: use run_mergetool from git-mergetool-lib","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-04-01T22:54:22Z","receivedAt":"2009-04-01T22:54:22Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"David Aguilar, 01.04.2009:\n> This refactors git-mergetool to use run_mergetool.\n> \n> Signed-off-by: David Aguilar <davvid@gmail.com>\n> ---\n>  git-mergetool.sh |   96 +++--------------------------------------------------\n>  1 files changed, 6 insertions(+), 90 deletions(-)\n> \n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 957993c..2c6b325 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -190,96 +190,12 @@ merge_file () {\n>  \tread ans\n>      fi\n>  \n> -    case \"$merge_tool\" in\n> -\tkdiff3)\n> -\t    if base_present ; then\n> -\t\t(\"$merge_tool_path\" --auto --L1 \"$MERGED (Base)\" --L2 \"$MERGED (Local)\" --L3 \"$MERGED (Remote)\" \\\n> -\t\t    -o \"$MERGED\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" > /dev/null 2>&1)\n> -\t    else\n> -\t\t(\"$merge_tool_path\" --auto --L1 \"$MERGED (Local)\" --L2 \"$MERGED (Remote)\" \\\n> -\t\t    -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\" > /dev/null 2>&1)\n> -\t    fi\n> -\t    status=$?\n> -\t    ;;\n> -\ttkdiff)\n> -\t    if base_present ; then\n> -\t\t\"$merge_tool_path\" -a \"$BASE\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n> -\t    else\n> -\t\t\"$merge_tool_path\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n> -\t    fi\n> -\t    status=$?\n> -\t    ;;\n> -\tmeld)\n> -\t    touch \"$BACKUP\"\n> -\t    \"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n> -\t    check_unchanged\n> -\t    ;;\n> -\tvimdiff)\n> -\t    touch \"$BACKUP\"\n> -\t    \"$merge_tool_path\" -c \"wincmd l\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n> -\t    check_unchanged\n> -\t    ;;\n> -\tgvimdiff)\n> -\t    touch \"$BACKUP\"\n> -\t    \"$merge_tool_path\" -c \"wincmd l\" -f \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n> -\t    check_unchanged\n> -\t    ;;\n> -\txxdiff)\n> -\t    touch \"$BACKUP\"\n> -\t    if base_present ; then\n> -\t\t\"$merge_tool_path\" -X --show-merged-pane \\\n> -\t\t    -R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n> -\t\t    -R 'Accel.Search: \"Ctrl+F\"' \\\n> -\t\t    -R 'Accel.SearchForward: \"Ctrl-G\"' \\\n> -\t\t    --merged-file \"$MERGED\" \"$LOCAL\" \"$BASE\" \"$REMOTE\"\n> -\t    else\n> -\t\t\"$merge_tool_path\" -X --show-merged-pane \\\n> -\t\t    -R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n> -\t\t    -R 'Accel.Search: \"Ctrl+F\"' \\\n> -\t\t    -R 'Accel.SearchForward: \"Ctrl-G\"' \\\n> -\t\t    --merged-file \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n> -\t    fi\n> -\t    check_unchanged\n> -\t    ;;\n> -\topendiff)\n> -\t    touch \"$BACKUP\"\n> -\t    if base_present; then\n> -\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" -ancestor \"$BASE\" -merge \"$MERGED\" | cat\n> -\t    else\n> -\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" -merge \"$MERGED\" | cat\n> -\t    fi\n> -\t    check_unchanged\n> -\t    ;;\n> -\tecmerge)\n> -\t    touch \"$BACKUP\"\n> -\t    if base_present; then\n> -\t\t\"$merge_tool_path\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" --default --mode=merge3 --to=\"$MERGED\"\n> -\t    else\n> -\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" --default --mode=merge2 --to=\"$MERGED\"\n> -\t    fi\n> -\t    check_unchanged\n> -\t    ;;\n> -\temerge)\n> -\t    if base_present ; then\n> -\t\t\"$merge_tool_path\" -f emerge-files-with-ancestor-command \"$LOCAL\" \"$REMOTE\" \"$BASE\" \"$(basename \"$MERGED\")\"\n> -\t    else\n> -\t\t\"$merge_tool_path\" -f emerge-files-command \"$LOCAL\" \"$REMOTE\" \"$(basename \"$MERGED\")\"\n> -\t    fi\n> -\t    status=$?\n> -\t    ;;\n> -\t*)\n> -\t    if test -n \"$merge_tool_cmd\"; then\n> -\t\tif test \"$merge_tool_trust_exit_code\" = \"false\"; then\n> -\t\t    touch \"$BACKUP\"\n> -\t\t    ( eval $merge_tool_cmd )\n> -\t\t    check_unchanged\n> -\t\telse\n> -\t\t    ( eval $merge_tool_cmd )\n> -\t\t    status=$?\n> -\t\tfi\n> -\t    fi\n> -\t    ;;\n> -    esac\n> +    present=false\n> +    base_present &&\n> +    present=true\n> +\n> +    run_mergetool \"$merge_tool\" \"$present\"\n> +    status=$?\n\nThis last line has to be deleted, because the variable 'status' set in\nrun_mergetool would be overwritten then. In this case the merge will\nsucceed even if it didn't and the file will be staged.\n\n>      if test \"$status\" -ne 0; then\n>  \techo \"merge of $MERGED failed\" 1>&2\n>  \tmv -- \"$BACKUP\" \"$MERGED\"\n"},{"id":"110184","messageId":"20090402035839.GA43852@gmail.com","threadId":"18681","inReplyTo":"200904020039.41894.markus.heidelberg@web.de","subject":"Re: [PATCH 03/10] Add a mergetool-lib scriptlet for holding common merge tool functions","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-02T03:58:40Z","receivedAt":"2009-04-02T03:58:40Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On  0, Markus Heidelberg <markus.heidelberg@web.de> wrote:\n> David Aguilar, 01.04.2009:\n> > \n> > diff --git a/git-mergetool-lib.sh b/git-mergetool-lib.sh\n> > +valid_tool () {\n> > +\tcase \"$1\" in\n> > +\tkdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)\n> > +\t\tif test \"$1\" = \"kompare\" && ! diff_mode; then\n> > +\t\t\treturn 1\n> > +\t\tfi\n> > +\t\t;; # happy\n> > +\t*)\n> > +\t\tif ! test -n \"$(get_custom_cmd \"$1\")\"; then\n> \n> Better this?\n> \t\tif test -z \"$(get_custom_cmd \"$1\")\"; then\n> \n> > +\t\t\treturn 1\n> > +\t\tfi ;;\n> \n> For consistency:\n> \t\tfi\n> \t\t;;\n> \n\n\nI'll reroll tonight and include you on the CC when I resent the\npatches in question.\n\nThanks Markus.\n\n-- \n\t\tDavid\n"},{"id":"110240","messageId":"20090402195918.GA13817@hashpling.org","threadId":"18681","inReplyTo":"1238590514-41893-1-git-send-email-davvid@gmail.com","subject":"Re: git-{diff,merge} refactor round 2","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-04-02T19:59:18Z","receivedAt":"2009-04-02T19:59:18Z","isPatch":false,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Wed, Apr 01, 2009 at 05:55:04AM -0700, David Aguilar wrote:\n> Here's the 2nd round of refactoring.\n \nAgain, I reply in summary form...\n\n[02/10] - I ack this.\n\n[03-07 / 10] - fine. I'm not sure, but should they be one commit?\nThey're really about consolidating blocks from one place to another\nand the change might be more 'obvious' as a single commit.\n\n[08/10] Now that we've isolated the run_mergetool, can we get it to\nreturn the value of '$status' (either the result of the tool or of\ncheck_unchanged) as its exit code so that we know whether the merge\nsucceeded?\n\n[10/10] Then this chunk:\n\n+    run_mergetool \"$merge_tool\" \"$present\"\n+    status=$?\n     if test \"$status\" -ne 0; then\n\ncan validly become:\n\n     if ! run_mergtool \"$merge_tool\" \"$present\"; then\n\nwhich seems clearer to me.\n\n-- \nCharles Bailey\nhttp://ccgi.hashpling.plus.com/blog/\n"},{"id":"110241","messageId":"20090402200235.GB13817@hashpling.org","threadId":"18681","inReplyTo":"200904020054.23147.markus.heidelberg@web.de","subject":"Re: [PATCH 10/10] mergetool: use run_mergetool from git-mergetool-lib","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2009-04-02T20:02:35Z","receivedAt":"2009-04-02T20:02:35Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On Thu, Apr 02, 2009 at 12:54:22AM +0200, Markus Heidelberg wrote:\n> David Aguilar, 01.04.2009:\n> > +    present=false\n> > +    base_present &&\n> > +    present=true\n> > +\n> > +    run_mergetool \"$merge_tool\" \"$present\"\n> > +    status=$?\n> \n> This last line has to be deleted, because the variable 'status' set in\n> run_mergetool would be overwritten then. In this case the merge will\n> succeed even if it didn't and the file will be staged.\n\nI think that it would be better if $status stayed as local as\npossible. If run_mergetool returned the value of $status as its exit\ncode then the function will properly return whether the merge\nsucceeded or not.\n\nThis way run_mergetool's clients can just use its exit code, without\nhaving to know about a magic $status global variable.\n\n-- \nCharles Bailey\nhttp://ccgi.hashpling.plus.com/blog/\n"},{"id":"110243","messageId":"200904022213.15173.markus.heidelberg@web.de","threadId":"18681","inReplyTo":"20090402200235.GB13817@hashpling.org","subject":"Re: [PATCH 10/10] mergetool: use run_mergetool from git-mergetool-lib","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-04-02T20:13:14Z","receivedAt":"2009-04-02T20:13:14Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Charles Bailey, 02.04.2009:\n> On Thu, Apr 02, 2009 at 12:54:22AM +0200, Markus Heidelberg wrote:\n> > David Aguilar, 01.04.2009:\n> > > +    present=false\n> > > +    base_present &&\n> > > +    present=true\n> > > +\n> > > +    run_mergetool \"$merge_tool\" \"$present\"\n> > > +    status=$?\n> > \n> > This last line has to be deleted, because the variable 'status' set in\n> > run_mergetool would be overwritten then. In this case the merge will\n> > succeed even if it didn't and the file will be staged.\n> \n> I think that it would be better if $status stayed as local as\n> possible. If run_mergetool returned the value of $status as its exit\n> code then the function will properly return whether the merge\n> succeeded or not.\n> \n> This way run_mergetool's clients can just use its exit code, without\n> having to know about a magic $status global variable.\n\nSure. Next time I have to think about the right solution before\nspreading workarounds.\n\nAnd shouldn't also check_unchanged() move from git-mergetool.sh to\ngit-mergetool--lib.sh, since it is only used there?\nHuh, it seems as if some editors for difftool cannot work correctly\ncurrently, because this function is used, but defined in\ngit-mergetool.sh.\n\nMarkus\n"},{"id":"110244","messageId":"200904022216.51159.markus.heidelberg@web.de","threadId":"18681","inReplyTo":"200904022213.15173.markus.heidelberg@web.de","subject":"Re: [PATCH 10/10] mergetool: use run_mergetool from git-mergetool-lib","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-04-02T20:16:50Z","receivedAt":"2009-04-02T20:16:50Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Markus Heidelberg, 02.04.2009:\n> Huh, it seems as if some editors for difftool cannot work correctly\n> currently, because this function is used, but defined in\n> git-mergetool.sh.\n\nYes, it works...\n\n# Overridden in git-mergetool\ncheck_unchanged () {\n\ttrue\n}\n\nBut that's not really nice, is it? Maybe we'd be better with a\n\n    if merge_mode; then\n\ninside this function and only define it in git-mergetool--lib.sh.\n\nMarkus\n"},{"id":"110257","messageId":"20090403015402.GA52576@gmail.com","threadId":"18681","inReplyTo":"200904022213.15173.markus.heidelberg@web.de","subject":"Re: [PATCH 10/10] mergetool: use run_mergetool from git-mergetool-lib","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-03T01:54:03Z","receivedAt":"2009-04-03T01:54:03Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On  0, Markus Heidelberg <markus.heidelberg@web.de> wrote:\n> Charles Bailey, 02.04.2009:\n> > \n> > I think that it would be better if $status stayed as local as\n> > possible. If run_mergetool returned the value of $status as its exit\n> > code then the function will properly return whether the merge\n> > succeeded or not.\n> > \n> > This way run_mergetool's clients can just use its exit code, without\n> > having to know about a magic $status global variable.\n> \n> And shouldn't also check_unchanged() move from git-mergetool.sh to\n> git-mergetool--lib.sh, since it is only used there?\n> Huh, it seems as if some editors for difftool cannot work correctly\n> currently, because this function is used, but defined in\n> git-mergetool.sh.\n\nSounds good.\nA patch is on the way:\n\n- moves check_unchanged into mergetool--lib\n- returns $status in run_mergetool\n- updates mergetool to not rely on the global $status\n\n-- \n\t\tDavid\n"},{"id":"110371","messageId":"200904050458.17708.markus.heidelberg@web.de","threadId":"18681","inReplyTo":"1238590514-41893-1-git-send-email-davvid@gmail.com","subject":"Re: git-{diff,merge} refactor round 2","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-04-05T02:58:17Z","receivedAt":"2009-04-05T02:58:17Z","isPatch":false,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"David Aguilar, 01.04.2009:\n> Here's the 2nd round of refactoring.\n\nI just noticed that mergetool.<mergetool>.path doesn't work anymore.\ngit grep mergetool.*path only hits one line in git-difftool--helper.sh\nNeither does it seem to work with difftool, but I'm gonna go to bed now.\n\nMarkus\n"},{"id":"110376","messageId":"20090405033443.GA16219@gmail.com","threadId":"18681","inReplyTo":"200904050458.17708.markus.heidelberg@web.de","subject":"Re: git-{diff,merge} refactor round 2","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-05T03:34:44Z","receivedAt":"2009-04-05T03:34:44Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On  0, Markus Heidelberg <markus.heidelberg@web.de> wrote:\n> David Aguilar, 01.04.2009:\n> > Here's the 2nd round of refactoring.\n> \n> I just noticed that mergetool.<mergetool>.path doesn't work anymore.\n> git grep mergetool.*path only hits one line in git-difftool--helper.sh\n> Neither does it seem to work with difftool, but I'm gonna go to bed now.\n> \n> Markus\n> \n\nOops.  Well, I have one final patch that removes the last bit of\nredundant code.  It also fixed this problem so I'll go ahead\nand send it (it's based on top of da/difftool mentioned in\npu).\n\nSince the test cases didn't catch that breakage I added a test\nfor it. \n\nLook for a patch called:\n\nmergetool--lib: consolidate the last redundant bits in {diff,merge}tool\n\n\n-- \n\n\tDavid\n"},{"id":"110394","messageId":"7vzlevv3fy.fsf@gitster.siamese.dyndns.org","threadId":"18681","inReplyTo":"20090405033443.GA16219@gmail.com","subject":"Re: git-{diff,merge} refactor round 2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-05T09:45:53Z","receivedAt":"2009-04-05T09:45:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> On  0, Markus Heidelberg <markus.heidelberg@web.de> wrote:\n>> David Aguilar, 01.04.2009:\n>> > Here's the 2nd round of refactoring.\n>> \n>> I just noticed that mergetool.<mergetool>.path doesn't work anymore.\n>> git grep mergetool.*path only hits one line in git-difftool--helper.sh\n>> Neither does it seem to work with difftool, but I'm gonna go to bed now.\n>> \n>> Markus\n>> \n>\n> Oops.  Well, I have one final patch that removes the last bit of\n> redundant code.  It also fixed this problem so I'll go ahead\n> and send it (it's based on top of da/difftool mentioned in\n> pu).\n>\n> Since the test cases didn't catch that breakage I added a test\n> for it. \n>\n> Look for a patch called:\n>\n> mergetool--lib: consolidate the last redundant bits in {diff,merge}tool\n\nI'll try to queue all the outstanding da/difftool patches tonight, but I\nthink the patches in the series are getting to the point of needing a\nfresh redoing.  Patches like \"oops, these non-user scripts should have\nbeen named with double-dash\" can and should disappear.\n\nCurrently they are:\n\n$ git log --oneline next..da/difftool\n736e6b6 mergetool--lib: add new merge tool TortoiseMerge\nb3ef7cc mergetool--lib: make (g)vimdiff workable under Windows\nc4d690e mergetool--lib: consolidate the last redundant bits in {diff,merge}tool\ndef88c8 mergetool-lib: specialize opendiff options when in diff mode\nbd52fab mergetool-lib: refactor run_mergetool and check_unchanged\ne87266c bash completion: add git-difftool\n04c3b54 {diff,merge}tool: rename helpers to remove them from tab-completion\n2a83022 mergetool-lib: add diffuse as merge and diff tool\n73c59d9 mergetool-lib: specialize xxdiff options when in diff mode\n273e7a2 mergetool-lib: specialize kdiff3 options when in diff mode\n99511d8 mergetool: use run_mergetool from git-mergetool-lib\n37c48c7 difftool: use run_mergetool from git-mergetool-lib\n4e314b5 mergetool-lib: introduce run_mergetool\n588954e difftool: use valid_tool from git-mergetool-lib\n8af4556 mergetool: use valid_tool from git-mergetool-lib\n72286b5 difftool: use get_mergetool_path from git-mergetool-lib\nd03b97f mergetool: use get_mergetool_path from git-mergetool-lib\nc6afc72 Add a mergetool-lib scriptlet for holding common merge tool functions\n6108b75 mergetool: use $( ... ) instead of `backticks`\n73786e2 difftool: add support for a difftool.prompt config variable\n472ff62 difftool: add a -y shortcut for --no-prompt\nde2b85d difftool: use perl built-ins when testing for msys\n9df990e difftool: add various git-difftool tests\n8ac77f2 difftool: add git-difftool to the list of commands\n"},{"id":"110460","messageId":"20090405211533.GA1393@gmail.com","threadId":"18681","inReplyTo":"7vzlevv3fy.fsf@gitster.siamese.dyndns.org","subject":"Re: git-{diff,merge} refactor round 2","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-04-05T21:15:34Z","receivedAt":"2009-04-05T21:15:34Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On  0, Junio C Hamano <gitster@pobox.com> wrote:\n> David Aguilar <davvid@gmail.com> writes:\n> \n> I'll try to queue all the outstanding da/difftool patches tonight, but I\n> think the patches in the series are getting to the point of needing a\n> fresh redoing.  Patches like \"oops, these non-user scripts should have\n> been named with double-dash\" can and should disappear.\n> \n> Currently they are:\n> \n> $ git log --oneline next..da/difftool\n> 736e6b6 mergetool--lib: add new merge tool TortoiseMerge\n> b3ef7cc mergetool--lib: make (g)vimdiff workable under Windows\n> c4d690e mergetool--lib: consolidate the last redundant bits in {diff,merge}tool\n> def88c8 mergetool-lib: specialize opendiff options when in diff mode\n> bd52fab mergetool-lib: refactor run_mergetool and check_unchanged\n> e87266c bash completion: add git-difftool\n> 04c3b54 {diff,merge}tool: rename helpers to remove them from tab-completion\n> 2a83022 mergetool-lib: add diffuse as merge and diff tool\n> 73c59d9 mergetool-lib: specialize xxdiff options when in diff mode\n> 273e7a2 mergetool-lib: specialize kdiff3 options when in diff mode\n> 99511d8 mergetool: use run_mergetool from git-mergetool-lib\n> 37c48c7 difftool: use run_mergetool from git-mergetool-lib\n> 4e314b5 mergetool-lib: introduce run_mergetool\n> 588954e difftool: use valid_tool from git-mergetool-lib\n> 8af4556 mergetool: use valid_tool from git-mergetool-lib\n> 72286b5 difftool: use get_mergetool_path from git-mergetool-lib\n> d03b97f mergetool: use get_mergetool_path from git-mergetool-lib\n> c6afc72 Add a mergetool-lib scriptlet for holding common merge tool functions\n> 6108b75 mergetool: use $( ... ) instead of `backticks`\n> 73786e2 difftool: add support for a difftool.prompt config variable\n> 472ff62 difftool: add a -y shortcut for --no-prompt\n> de2b85d difftool: use perl built-ins when testing for msys\n> 9df990e difftool: add various git-difftool tests\n> 8ac77f2 difftool: add git-difftool to the list of commands\n\nIt goes back even farther...\n\nd3b8cec difftool: move 'git-difftool' out of contrib\n\nd3db8cec is currently sitting in 'next' and is where the\n\"oops, I should have used double-dash\" lack of foresight\nbegan.\n\nI like the *result* of what's currently sitting in\nda/difftool, so rewriting history is easy now that we know\nexactly where we're going.\n\nI think we have a couple of options here.  I won't be able to\nget to it until sometime tonight, but I'll throw my plan out\nhere to get a feel for what's the better way to do it.\nI think we have a couple of options.  I'm open to any of\nthese:\n\n1. Base it on the current master, completely throwing away\nthe existing da/difftool branch.  That would include throwing\naway the commit that's in next if we really want to be clean\nabout the history.  In the process, move Markus' mergetool\nfixes for windows to the top so that they can be applied\nindependently if necessary.  This series would then depend\non them.\n\nThis would probably mean a merge conflict with next at some\npoint.\n\n\n2. Base the series on next, keeping the 'move out of contrib'\npatch intact.  That'll minimize conflicts but will have an\nextra commit that renames difftool-helper.  I can still push\nthe fixes-for-windows patches up to the top so that it makes\nit easier on msysgit since we already have those two patches\nin the msysgit 'tortoisemerge' branch.\n\n3. Any others?\n\n\nRegardless of which it's based on, it's obvious that there'll\nbe some squashing going on.  Tentatively,\n\nWill be squashed:\n588954e difftool: use valid_tool from git-mergetool-lib\n8af4556 mergetool: use valid_tool from git-mergetool-lib\n\nWill be squashed:\n72286b5 difftool: use get_mergetool_path from git-mergetool-lib\nd03b97f mergetool: use get_mergetool_path from git-mergetool-lib\nc6afc72 Add a mergetool-lib scriptlet for holding common merge tool functions\n\nWill be squashed:\n99511d8 mergetool: use run_mergetool from git-mergetool-lib\n37c48c7 difftool: use run_mergetool from git-mergetool-lib\n4e314b5 mergetool-lib: introduce run_mergetool\n\nWill be squashed:\ndef88c8 mergetool-lib: specialize opendiff options when in diff mode\n73c59d9 mergetool-lib: specialize xxdiff options when in diff mode\n273e7a2 mergetool-lib: specialize kdiff3 options when in diff mode\n\n\nI'll go with whatever you guys think makes the series easiest to manage.\n\nI think my preference would be to resend the entires series\nbased on 'master', but I don't want to make it any harder to\nmanage 'next'.\n\nThoughts?\n\n-- \n\n\tDavid\n"},{"id":"110475","messageId":"200904060015.17145.markus.heidelberg@web.de","threadId":"18681","inReplyTo":"20090405211533.GA1393@gmail.com","subject":"Re: git-{diff,merge} refactor round 2","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-04-05T22:15:16Z","receivedAt":"2009-04-05T22:15:16Z","isPatch":false,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"\nDavid Aguilar, 05.04.2009:\n> On  0, Junio C Hamano <gitster@pobox.com> wrote:\n> > David Aguilar <davvid@gmail.com> writes:\n> > \n> > I'll try to queue all the outstanding da/difftool patches tonight, but I\n> > think the patches in the series are getting to the point of needing a\n> > fresh redoing.  Patches like \"oops, these non-user scripts should have\n> > been named with double-dash\" can and should disappear.\n> > \n> > Currently they are:\n> > \n> > $ git log --oneline next..da/difftool\n> > [..]\n> \n> It goes back even farther...\n> \n> d3b8cec difftool: move 'git-difftool' out of contrib\n> \n> d3db8cec is currently sitting in 'next' and is where the\n> \"oops, I should have used double-dash\" lack of foresight\n> began.\n\n> 1. Base it on the current master, completely throwing away\n> the existing da/difftool branch.  That would include throwing\n> away the commit that's in next if we really want to be clean\n> about the history.  In the process, move Markus' mergetool\n> fixes for windows to the top so that they can be applied\n> independently if necessary.  This series would then depend\n> on them.\n\nThis is my favourite, too.\n\nAdditionally, what about basing these on master as well? They also are\nunrelated to the refactoring:\n\ndef88c8 mergetool-lib: specialize opendiff options when in diff mode\n2a83022 mergetool-lib: add diffuse as merge and diff tool\n73c59d9 mergetool-lib: specialize xxdiff options when in diff mode\n273e7a2 mergetool-lib: specialize kdiff3 options when in diff mode\n\nAnd start refactoring on top of these?\nThen these could go into master or next, since they are mostly bugfixes\nand the refactoring can start in pu.\n\n> This would probably mean a merge conflict with next at some\n> point.\n\nRevert \"d3b8cec difftool: move 'git-difftool' out of contrib\" from next?\n\n> Regardless of which it's based on, it's obvious that there'll\n> be some squashing going on.  Tentatively,\n> \n> Will be squashed:\n> 588954e difftool: use valid_tool from git-mergetool-lib\n> 8af4556 mergetool: use valid_tool from git-mergetool-lib\n> \n> Will be squashed:\n> 72286b5 difftool: use get_mergetool_path from git-mergetool-lib\n> d03b97f mergetool: use get_mergetool_path from git-mergetool-lib\n> c6afc72 Add a mergetool-lib scriptlet for holding common merge tool functions\n> \n> Will be squashed:\n> 99511d8 mergetool: use run_mergetool from git-mergetool-lib\n> 37c48c7 difftool: use run_mergetool from git-mergetool-lib\n> 4e314b5 mergetool-lib: introduce run_mergetool\n> \n> Will be squashed:\n> def88c8 mergetool-lib: specialize opendiff options when in diff mode\n> 73c59d9 mergetool-lib: specialize xxdiff options when in diff mode\n> 273e7a2 mergetool-lib: specialize kdiff3 options when in diff mode\n\nYes, some squashing would be nice. Similar commit messages are confusing\nwhen reading the history.\n\nMarkus\n"},{"id":"110496","messageId":"7vocvaoc24.fsf@gitster.siamese.dyndns.org","threadId":"18681","inReplyTo":"200904060015.17145.markus.heidelberg@web.de","subject":"Re: git-{diff,merge} refactor round 2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-06T00:33:55Z","receivedAt":"2009-04-06T00:33:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Markus Heidelberg <markus.heidelberg@web.de> writes:\n\n> David Aguilar, 05.04.2009:\n> ...\n>> 1. Base it on the current master, completely throwing away\n>> the existing da/difftool branch.  That would include throwing\n>> away the commit that's in next if we really want to be clean\n>> about the history.  In the process, move Markus' mergetool\n>> fixes for windows to the top so that they can be applied\n>> independently if necessary.  This series would then depend\n>> on them.\n>\n> This is my favourite, too.\n\nOk.  You two forgot another obvious option of not doing anything, but\nsince both of you seem to be Ok with the rewrite, let's take that\napproach.  It is easy to revert the one premature merge out of next and\nthen merge the cleaned-up version.\n\n> Yes, some squashing would be nice. Similar commit messages are confusing\n> when reading the history.\n\nAgreed.\n"}]}