{"thread":{"id":"28151","subject":"[PATCH 0/4] Refactor git-mergetool--lib.sh","startedAt":"2011-08-18T07:23:43Z","lastAt":"2011-10-11T00:00:48Z","messageCount":18,"participants":["David Aguilar","Sebastian Schuberth","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"173731","messageId":"1313652227-48545-1-git-send-email-davvid@gmail.com","threadId":"28151","inReplyTo":null,"subject":"[PATCH 0/4] Refactor git-mergetool--lib.sh","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-08-18T07:23:43Z","receivedAt":"2011-08-18T07:23:43Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"This series splits the git-mergetool--lib.sh file into separate\ntool scriptlets that define the commands for diff and merge.\n\nDavid Aguilar (4):\n  difftool--helper: Make style consistent with git\n  mergetool--lib: Make style consistent with git\n  mergetool--lib: Refactor tools into separate files\n  mergetools/meld: Use '--output' when available\n\n Makefile                 |   11 ++\n git-difftool--helper.sh  |   18 ++-\n git-mergetool--lib.sh    |  394 ++++++++++------------------------------------\n mergetools/araxis        |   21 +++\n mergetools/bc3           |   20 +++\n mergetools/defaults      |   46 ++++++\n mergetools/diffuse       |   17 ++\n mergetools/ecmerge       |   16 ++\n mergetools/emerge        |   23 +++\n mergetools/kdiff3        |   24 +++\n mergetools/kompare       |    7 +\n mergetools/meld          |   40 +++++\n mergetools/opendiff      |   16 ++\n mergetools/p4merge       |   10 ++\n mergetools/tkdiff        |   12 ++\n mergetools/tortoisemerge |   17 ++\n mergetools/vim           |   44 +++++\n mergetools/xxdiff        |   25 +++\n 18 files changed, 441 insertions(+), 320 deletions(-)\n create mode 100644 mergetools/araxis\n create mode 100644 mergetools/bc3\n create mode 100644 mergetools/defaults\n create mode 100644 mergetools/diffuse\n create mode 100644 mergetools/ecmerge\n create mode 100644 mergetools/emerge\n create mode 100644 mergetools/kdiff3\n create mode 100644 mergetools/kompare\n create mode 100644 mergetools/meld\n create mode 100644 mergetools/opendiff\n create mode 100644 mergetools/p4merge\n create mode 100644 mergetools/tkdiff\n create mode 100644 mergetools/tortoisemerge\n create mode 100644 mergetools/vim\n create mode 100644 mergetools/xxdiff\n\n$ git describe --dirty\nv1.7.6-476-g572925c\n$ git version\ngit version 1.7.6.476.g57292\n$ ./t7610-mergetool.sh \nok 1 - setup\nok 2 - custom mergetool\nok 3 - mergetool crlf\nok 4 - mergetool in subdir\nok 5 - mergetool on file in parent dir\nok 6 - mergetool skips autoresolved\nok 7 - mergetool merges all from subdir\nok 8 - mergetool skips resolved paths when rerere is active\nok 9 - deleted vs modified submodule\nok 10 - file vs modified submodule\nok 11 - submodule in subdirectory\nok 12 - directory vs modified submodule\n# passed all 12 test(s)\n1..12\n$ ./t7800-difftool.sh \nok 1 - setup\nok 2 - custom commands\nok 3 - difftool ignores bad --tool values\nok 4 - difftool honors --gui\nok 5 - difftool --gui works without configured diff.guitool\nok 6 - GIT_DIFF_TOOL variable\nok 7 - GIT_DIFF_TOOL overrides\nok 8 - GIT_DIFFTOOL_NO_PROMPT variable\nok 9 - GIT_DIFFTOOL_PROMPT variable\nok 10 - difftool.prompt config variable is false\nok 11 - difftool merge.prompt = false\nok 12 - difftool.prompt can overridden with -y\nok 13 - difftool.prompt can overridden with --prompt\nok 14 - difftool last flag wins\nok 15 - difftool + mergetool config variables\nok 16 - difftool.<tool>.path\nok 17 - difftool --extcmd=cat\nok 18 - difftool --extcmd cat\nok 19 - difftool -x cat\nok 20 - difftool --extcmd echo arg1\nok 21 - difftool --extcmd cat arg1\nok 22 - difftool --extcmd cat arg2\n# passed all 22 test(s)\n1..22\n\n-- \n1.7.6.476.g57292\n"},{"id":"173732","messageId":"1313652227-48545-2-git-send-email-davvid@gmail.com","threadId":"28151","inReplyTo":"1313652227-48545-1-git-send-email-davvid@gmail.com","subject":"[PATCH 1/4] difftool--helper: Make style consistent with git","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-08-18T07:23:44Z","receivedAt":"2011-08-18T07:23:44Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Use the predominant conditional style where \"then\" appears\nalone on the line after the test expression.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-difftool--helper.sh |   18 ++++++++++++------\n 1 files changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/git-difftool--helper.sh b/git-difftool--helper.sh\nindex 0594bf7..8452890 100755\n--- a/git-difftool--helper.sh\n+++ b/git-difftool--helper.sh\n@@ -13,7 +13,8 @@ TOOL_MODE=diff\n should_prompt () {\n \tprompt_merge=$(git config --bool mergetool.prompt || echo true)\n \tprompt=$(git config --bool difftool.prompt || echo $prompt_merge)\n-\tif test \"$prompt\" = true; then\n+\tif test \"$prompt\" = true\n+\tthen\n \t\ttest -z \"$GIT_DIFFTOOL_NO_PROMPT\"\n \telse\n \t\ttest -n \"$GIT_DIFFTOOL_PROMPT\"\n@@ -37,9 +38,11 @@ launch_merge_tool () {\n \n \t# $LOCAL and $REMOTE are temporary files so prompt\n \t# the user with the real $MERGED name before launching $merge_tool.\n-\tif should_prompt; then\n+\tif should_prompt\n+\tthen\n \t\tprintf \"\\nViewing: '$MERGED'\\n\"\n-\t\tif use_ext_cmd; then\n+\t\tif use_ext_cmd\n+\t\tthen\n \t\t\tprintf \"Hit return to launch '%s': \" \\\n \t\t\t\t\"$GIT_DIFFTOOL_EXTCMD\"\n \t\telse\n@@ -48,7 +51,8 @@ launch_merge_tool () {\n \t\tread ans\n \tfi\n \n-\tif use_ext_cmd; then\n+\tif use_ext_cmd\n+\tthen\n \t\texport BASE\n \t\teval $GIT_DIFFTOOL_EXTCMD '\"$LOCAL\"' '\"$REMOTE\"'\n \telse\n@@ -56,8 +60,10 @@ launch_merge_tool () {\n \tfi\n }\n \n-if ! use_ext_cmd; then\n-\tif test -n \"$GIT_DIFF_TOOL\"; then\n+if ! use_ext_cmd\n+then\n+\tif test -n \"$GIT_DIFF_TOOL\"\n+\tthen\n \t\tmerge_tool=\"$GIT_DIFF_TOOL\"\n \telse\n \t\tmerge_tool=\"$(get_merge_tool)\" || exit\n-- \n1.7.6.476.g57292\n"},{"id":"173733","messageId":"1313652227-48545-3-git-send-email-davvid@gmail.com","threadId":"28151","inReplyTo":"1313652227-48545-1-git-send-email-davvid@gmail.com","subject":"[PATCH 2/4] mergetool--lib: Make style consistent with git","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-08-18T07:23:45Z","receivedAt":"2011-08-18T07:23:45Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Use the predominant conditional style where \"then\" appears\nalone on the line after the test expression.\nRemove spaces after \">\" output redirections.\nRemove unnecessary parentheses around the kdiff3 commands.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n git-mergetool--lib.sh |  153 ++++++++++++++++++++++++++++++++-----------------\n 1 files changed, 100 insertions(+), 53 deletions(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 91f90ac..03cfb19 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -32,10 +32,12 @@ translate_merge_tool_path () {\n }\n \n check_unchanged () {\n-\tif test \"$MERGED\" -nt \"$BACKUP\"; then\n+\tif test \"$MERGED\" -nt \"$BACKUP\"\n+\tthen\n \t\tstatus=0\n \telse\n-\t\twhile true; do\n+\t\twhile true\n+\t\tdo\n \t\t\techo \"$MERGED seems unchanged.\"\n \t\t\tprintf \"Was the merge successful? [y/n] \"\n \t\t\tread answer\n@@ -53,17 +55,20 @@ valid_tool () {\n \tkdiff3 | meld | opendiff | p4merge | tkdiff | vimdiff | vimdiff2 | xxdiff)\n \t\t;; # happy\n \tkompare)\n-\t\tif ! diff_mode; then\n+\t\tif ! diff_mode\n+\t\tthen\n \t\t\treturn 1\n \t\tfi\n \t\t;;\n \ttortoisemerge)\n-\t\tif ! merge_mode; then\n+\t\tif ! merge_mode\n+\t\tthen\n \t\t\treturn 1\n \t\tfi\n \t\t;;\n \t*)\n-\t\tif test -z \"$(get_merge_tool_cmd \"$1\")\"; then\n+\t\tif test -z \"$(get_merge_tool_cmd \"$1\")\"\n+\t\tthen\n \t\t\treturn 1\n \t\tfi\n \t\t;;\n@@ -72,12 +77,14 @@ valid_tool () {\n \n get_merge_tool_cmd () {\n \t# Prints the custom command for a merge tool\n-\tif test -n \"$1\"; then\n+\tif test -n \"$1\"\n+\tthen\n \t\tmerge_tool=\"$1\"\n \telse\n \t\tmerge_tool=\"$(get_merge_tool)\"\n \tfi\n-\tif diff_mode; then\n+\tif diff_mode\n+\tthen\n \t\techo \"$(git config difftool.$merge_tool.cmd ||\n \t\t        git config mergetool.$merge_tool.cmd)\"\n \telse\n@@ -97,9 +104,11 @@ run_merge_tool () {\n \n \tcase \"$1\" in\n \taraxis)\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present; then\n+\t\t\tif $base_present\n+\t\t\tthen\n \t\t\t\t\"$merge_tool_path\" -wait -merge -3 -a1 \\\n \t\t\t\t\t\"$BASE\" \"$LOCAL\" \"$REMOTE\" \"$MERGED\" \\\n \t\t\t\t\t>/dev/null 2>&1\n@@ -115,9 +124,11 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \tbc3)\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present; then\n+\t\t\tif $base_present\n+\t\t\tthen\n \t\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n \t\t\t\t\t-mergeoutput=\"$MERGED\"\n \t\t\telse\n@@ -130,9 +141,11 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \tdiffuse)\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present; then\n+\t\t\tif $base_present\n+\t\t\tthen\n \t\t\t\t\"$merge_tool_path\" \\\n \t\t\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\" \\\n \t\t\t\t\t\"$BASE\" | cat\n@@ -146,9 +159,11 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \tecmerge)\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present; then\n+\t\t\tif $base_present\n+\t\t\tthen\n \t\t\t\t\"$merge_tool_path\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" \\\n \t\t\t\t\t--default --mode=merge3 --to=\"$MERGED\"\n \t\t\telse\n@@ -162,8 +177,10 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \temerge)\n-\t\tif merge_mode; then\n-\t\t\tif $base_present; then\n+\t\tif merge_mode\n+\t\tthen\n+\t\t\tif $base_present\n+\t\t\tthen\n \t\t\t\t\"$merge_tool_path\" \\\n \t\t\t\t\t-f emerge-files-with-ancestor-command \\\n \t\t\t\t\t\"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n@@ -181,9 +198,11 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \tgvimdiff|vimdiff)\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present; then\n+\t\t\tif $base_present\n+\t\t\tthen\n \t\t\t\t\"$merge_tool_path\" -f -d -c \"wincmd J\" \\\n \t\t\t\t\t\"$MERGED\" \"$LOCAL\" \"$BASE\" \"$REMOTE\"\n \t\t\telse\n@@ -198,7 +217,8 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \tgvimdiff2|vimdiff2)\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n \t\t\t\"$merge_tool_path\" -f -d -c \"wincmd l\" \\\n \t\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n@@ -210,36 +230,39 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \tkdiff3)\n-\t\tif merge_mode; then\n-\t\t\tif $base_present; then\n-\t\t\t\t(\"$merge_tool_path\" --auto \\\n+\t\tif merge_mode\n+\t\tthen\n+\t\t\tif $base_present\n+\t\t\tthen\n+\t\t\t\t\"$merge_tool_path\" --auto \\\n \t\t\t\t\t--L1 \"$MERGED (Base)\" \\\n \t\t\t\t\t--L2 \"$MERGED (Local)\" \\\n \t\t\t\t\t--L3 \"$MERGED (Remote)\" \\\n \t\t\t\t\t-o \"$MERGED\" \\\n \t\t\t\t\t\"$BASE\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t> /dev/null 2>&1)\n+\t\t\t\t>/dev/null 2>&1\n \t\t\telse\n-\t\t\t\t(\"$merge_tool_path\" --auto \\\n+\t\t\t\t\"$merge_tool_path\" --auto \\\n \t\t\t\t\t--L1 \"$MERGED (Local)\" \\\n \t\t\t\t\t--L2 \"$MERGED (Remote)\" \\\n \t\t\t\t\t-o \"$MERGED\" \\\n \t\t\t\t\t\"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t> /dev/null 2>&1)\n+\t\t\t\t>/dev/null 2>&1\n \t\t\tfi\n \t\t\tstatus=$?\n \t\telse\n-\t\t\t(\"$merge_tool_path\" --auto \\\n+\t\t\t\"$merge_tool_path\" --auto \\\n \t\t\t\t--L1 \"$MERGED (A)\" \\\n \t\t\t\t--L2 \"$MERGED (B)\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t> /dev/null 2>&1)\n+\t\t\t>/dev/null 2>&1\n \t\tfi\n \t\t;;\n \tkompare)\n \t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n \t\t;;\n \tmeld)\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n \t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n \t\t\tcheck_unchanged\n@@ -248,9 +271,11 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \topendiff)\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present; then\n+\t\t\tif $base_present\n+\t\t\tthen\n \t\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n \t\t\t\t\t-ancestor \"$BASE\" \\\n \t\t\t\t\t-merge \"$MERGED\" | cat\n@@ -264,7 +289,8 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \tp4merge)\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n \t\t\t$base_present || >\"$BASE\"\n \t\t\t\"$merge_tool_path\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" \"$MERGED\"\n@@ -274,8 +300,10 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \ttkdiff)\n-\t\tif merge_mode; then\n-\t\t\tif $base_present; then\n+\t\tif merge_mode\n+\t\tthen\n+\t\t\tif $base_present\n+\t\t\tthen\n \t\t\t\t\"$merge_tool_path\" -a \"$BASE\" \\\n \t\t\t\t\t-o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n \t\t\telse\n@@ -288,7 +316,8 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \ttortoisemerge)\n-\t\tif $base_present; then\n+\t\tif $base_present\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n \t\t\t\"$merge_tool_path\" \\\n \t\t\t\t-base:\"$BASE\" -mine:\"$LOCAL\" \\\n@@ -300,9 +329,11 @@ run_merge_tool () {\n \t\tfi\n \t\t;;\n \txxdiff)\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present; then\n+\t\t\tif $base_present\n+\t\t\tthen\n \t\t\t\t\"$merge_tool_path\" -X --show-merged-pane \\\n \t\t\t\t\t-R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n \t\t\t\t\t-R 'Accel.Search: \"Ctrl+F\"' \\\n@@ -327,16 +358,20 @@ run_merge_tool () {\n \t\t;;\n \t*)\n \t\tmerge_tool_cmd=\"$(get_merge_tool_cmd \"$1\")\"\n-\t\tif test -z \"$merge_tool_cmd\"; then\n-\t\t\tif merge_mode; then\n+\t\tif test -z \"$merge_tool_cmd\"\n+\t\tthen\n+\t\t\tif merge_mode\n+\t\t\tthen\n \t\t\t\tstatus=1\n \t\t\tfi\n \t\t\tbreak\n \t\tfi\n-\t\tif merge_mode; then\n+\t\tif merge_mode\n+\t\tthen\n \t\t\ttrust_exit_code=\"$(git config --bool \\\n \t\t\t\tmergetool.\"$1\".trustExitCode || echo false)\"\n-\t\t\tif test \"$trust_exit_code\" = \"false\"; then\n+\t\t\tif test \"$trust_exit_code\" = \"false\"\n+\t\t\tthen\n \t\t\t\ttouch \"$BACKUP\"\n \t\t\t\t( eval $merge_tool_cmd )\n \t\t\t\tcheck_unchanged\n@@ -353,13 +388,16 @@ run_merge_tool () {\n }\n \n guess_merge_tool () {\n-\tif merge_mode; then\n+\tif merge_mode\n+\tthen\n \t\ttools=\"tortoisemerge\"\n \telse\n \t\ttools=\"kompare\"\n \tfi\n-\tif test -n \"$DISPLAY\"; then\n-\t\tif test -n \"$GNOME_DESKTOP_SESSION_ID\" ; then\n+\tif test -n \"$DISPLAY\"\n+\tthen\n+\t\tif test -n \"$GNOME_DESKTOP_SESSION_ID\"\n+\t\tthen\n \t\t\ttools=\"meld opendiff kdiff3 tkdiff xxdiff $tools\"\n \t\telse\n \t\t\ttools=\"opendiff kdiff3 tkdiff xxdiff meld $tools\"\n@@ -380,7 +418,8 @@ guess_merge_tool () {\n \tfor i in $tools\n \tdo\n \t\tmerge_tool_path=\"$(translate_merge_tool_path \"$i\")\"\n-\t\tif type \"$merge_tool_path\" > /dev/null 2>&1; then\n+\t\tif type \"$merge_tool_path\" >/dev/null 2>&1\n+\t\tthen\n \t\t\techo \"$i\"\n \t\t\treturn 0\n \t\tfi\n@@ -393,12 +432,14 @@ guess_merge_tool () {\n get_configured_merge_tool () {\n \t# Diff mode first tries diff.tool and falls back to merge.tool.\n \t# Merge mode only checks merge.tool\n-\tif diff_mode; then\n+\tif diff_mode\n+\tthen\n \t\tmerge_tool=$(git config diff.tool || git config merge.tool)\n \telse\n \t\tmerge_tool=$(git config merge.tool)\n \tfi\n-\tif test -n \"$merge_tool\" && ! valid_tool \"$merge_tool\"; then\n+\tif test -n \"$merge_tool\" && ! valid_tool \"$merge_tool\"\n+\tthen\n \t\techo >&2 \"git config option $TOOL_MODE.tool set to unknown tool: $merge_tool\"\n \t\techo >&2 \"Resetting to default...\"\n \t\treturn 1\n@@ -408,26 +449,31 @@ get_configured_merge_tool () {\n \n get_merge_tool_path () {\n \t# A merge tool has been set, so verify that it's valid.\n-\tif test -n \"$1\"; then\n+\tif test -n \"$1\"\n+\tthen\n \t\tmerge_tool=\"$1\"\n \telse\n \t\tmerge_tool=\"$(get_merge_tool)\"\n \tfi\n-\tif ! valid_tool \"$merge_tool\"; then\n+\tif ! valid_tool \"$merge_tool\"\n+\tthen\n \t\techo >&2 \"Unknown merge tool $merge_tool\"\n \t\texit 1\n \tfi\n-\tif diff_mode; then\n+\tif diff_mode\n+\tthen\n \t\tmerge_tool_path=$(git config difftool.\"$merge_tool\".path ||\n \t\t                  git config mergetool.\"$merge_tool\".path)\n \telse\n \t\tmerge_tool_path=$(git config mergetool.\"$merge_tool\".path)\n \tfi\n-\tif test -z \"$merge_tool_path\"; then\n+\tif test -z \"$merge_tool_path\"\n+\tthen\n \t\tmerge_tool_path=\"$(translate_merge_tool_path \"$merge_tool\")\"\n \tfi\n \tif test -z \"$(get_merge_tool_cmd \"$merge_tool\")\" &&\n-\t! type \"$merge_tool_path\" > /dev/null 2>&1; then\n+\t\t! type \"$merge_tool_path\" >/dev/null 2>&1\n+\tthen\n \t\techo >&2 \"The $TOOL_MODE tool $merge_tool is not available as\"\\\n \t\t         \"'$merge_tool_path'\"\n \t\texit 1\n@@ -439,7 +485,8 @@ get_merge_tool () {\n \t# Check if a merge tool has been configured\n \tmerge_tool=$(get_configured_merge_tool)\n \t# Try to guess an appropriate merge tool if no tool has been set.\n-\tif test -z \"$merge_tool\"; then\n+\tif test -z \"$merge_tool\"\n+\tthen\n \t\tmerge_tool=\"$(guess_merge_tool)\" || exit\n \tfi\n \techo \"$merge_tool\"\n-- \n1.7.6.476.g57292\n"},{"id":"173735","messageId":"1313652227-48545-4-git-send-email-davvid@gmail.com","threadId":"28151","inReplyTo":"1313652227-48545-1-git-send-email-davvid@gmail.com","subject":"[PATCH 3/4] mergetool--lib: Refactor tools into separate files","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-08-18T07:23:46Z","receivedAt":"2011-08-18T07:23:46Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Individual merge tools are now defined in a mergetools/$tool\nfile which is sourced at runtime.\n\nThe individual files are installed into $(git --exec-path)/mergetools/.\nNew tools can be added by creating a new file instead of editing the\ngit-mergetool--lib.sh scriptlet.\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/134906/focus=135006\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n Makefile                 |   11 ++\n git-mergetool--lib.sh    |  385 ++++++---------------------------------------\n mergetools/araxis        |   21 +++\n mergetools/bc3           |   20 +++\n mergetools/defaults      |   46 ++++++\n mergetools/diffuse       |   17 ++\n mergetools/ecmerge       |   16 ++\n mergetools/emerge        |   23 +++\n mergetools/kdiff3        |   24 +++\n mergetools/kompare       |    7 +\n mergetools/meld          |    9 +\n mergetools/opendiff      |   16 ++\n mergetools/p4merge       |   10 ++\n mergetools/tkdiff        |   12 ++\n mergetools/tortoisemerge |   17 ++\n mergetools/vim           |   44 ++++++\n mergetools/xxdiff        |   25 +++\n 17 files changed, 370 insertions(+), 333 deletions(-)\n create mode 100644 mergetools/araxis\n create mode 100644 mergetools/bc3\n create mode 100644 mergetools/defaults\n create mode 100644 mergetools/diffuse\n create mode 100644 mergetools/ecmerge\n create mode 100644 mergetools/emerge\n create mode 100644 mergetools/kdiff3\n create mode 100644 mergetools/kompare\n create mode 100644 mergetools/meld\n create mode 100644 mergetools/opendiff\n create mode 100644 mergetools/p4merge\n create mode 100644 mergetools/tkdiff\n create mode 100644 mergetools/tortoisemerge\n create mode 100644 mergetools/vim\n create mode 100644 mergetools/xxdiff\n\ndiff --git a/Makefile b/Makefile\nindex 8dd782f..6a68e3a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -302,6 +302,7 @@ bindir = $(prefix)/$(bindir_relative)\n mandir = share/man\n infodir = share/info\n gitexecdir = libexec/git-core\n+mergetoolsdir = $(gitexecdir)/mergetools\n sharedir = $(prefix)/share\n gitwebdir = $(sharedir)/gitweb\n template_dir = share/git-core/templates\n@@ -2257,6 +2258,13 @@ endif\n gitexec_instdir_SQ = $(subst ','\\'',$(gitexec_instdir))\n export gitexec_instdir\n \n+ifneq ($(filter /%,$(firstword $(mergetoolsdir))),)\n+mergetools_instdir = $(mergetoolsdir)\n+else\n+mergetools_instdir = $(prefix)/$(mergetoolsdir)\n+endif\n+mergetools_instdir_SQ = $(subst ','\\'',$(mergetools_instdir))\n+\n install_bindir_programs := $(patsubst %,%$X,$(BINDIR_PROGRAMS_NEED_X)) $(BINDIR_PROGRAMS_NO_X)\n \n install: all\n@@ -2266,6 +2274,9 @@ install: all\n \t$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install\n+\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'\n+\t(cd mergetools && $(TAR) cf - .) | \\\n+\t(cd '$(DESTDIR_SQ)$(mergetools_instdir_SQ)' && umask 022 && $(TAR) xof -)\n ifndef NO_PERL\n \t$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install\n \t$(MAKE) -C gitweb install\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex 03cfb19..2421700 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -9,26 +9,7 @@ merge_mode() {\n }\n \n translate_merge_tool_path () {\n-\tcase \"$1\" in\n-\taraxis)\n-\t\techo compare\n-\t\t;;\n-\tbc3)\n-\t\techo bcompare\n-\t\t;;\n-\temerge)\n-\t\techo emacs\n-\t\t;;\n-\tgvimdiff|gvimdiff2)\n-\t\techo gvim\n-\t\t;;\n-\tvimdiff|vimdiff2)\n-\t\techo vim\n-\t\t;;\n-\t*)\n-\t\techo \"$1\"\n-\t\t;;\n-\tesac\n+\techo \"$1\"\n }\n \n check_unchanged () {\n@@ -49,40 +30,55 @@ check_unchanged () {\n \tfi\n }\n \n+valid_tool_config () {\n+\tif test -n \"$(get_merge_tool_cmd \"$1\")\"\n+\tthen\n+\t\treturn 0\n+\telse\n+\t\treturn 1\n+\tfi\n+}\n+\n valid_tool () {\n+\tsetup_tool \"$1\" || valid_tool_config \"$1\"\n+}\n+\n+setup_tool () {\n \tcase \"$1\" in\n-\taraxis | bc3 | diffuse | ecmerge | emerge | gvimdiff | gvimdiff2 | \\\n-\tkdiff3 | meld | opendiff | p4merge | tkdiff | vimdiff | vimdiff2 | xxdiff)\n-\t\t;; # happy\n-\tkompare)\n-\t\tif ! diff_mode\n-\t\tthen\n-\t\t\treturn 1\n-\t\tfi\n-\t\t;;\n-\ttortoisemerge)\n-\t\tif ! merge_mode\n-\t\tthen\n-\t\t\treturn 1\n-\t\tfi\n+\tvim*|gvim*)\n+\t\ttool=vim\n \t\t;;\n \t*)\n-\t\tif test -z \"$(get_merge_tool_cmd \"$1\")\"\n-\t\tthen\n-\t\t\treturn 1\n-\t\tfi\n+\t\ttool=\"$1\"\n \t\t;;\n \tesac\n+\tmergetools=\"$(git --exec-path)/mergetools\"\n+\n+\t# Load the default definitions\n+\t. \"$mergetools/defaults\"\n+\tif ! test -f \"$mergetools/$tool\"\n+\tthen\n+\t\treturn 1\n+\tfi\n+\n+\t# Load the redefined functions\n+\t. \"$mergetools/$tool\"\n+\n+\tif merge_mode && ! can_merge\n+\tthen\n+\t\techo \"error: '$tool' can not be used to resolve merges\" >&2\n+\t\texit 1\n+\telif diff_mode && ! can_diff\n+\tthen\n+\t\techo \"error: '$tool' can only be used to resolve merges\" >&2\n+\t\texit 1\n+\tfi\n+\treturn 0\n }\n \n get_merge_tool_cmd () {\n \t# Prints the custom command for a merge tool\n-\tif test -n \"$1\"\n-\tthen\n-\t\tmerge_tool=\"$1\"\n-\telse\n-\t\tmerge_tool=\"$(get_merge_tool)\"\n-\tfi\n+\tmerge_tool=\"$1\"\n \tif diff_mode\n \tthen\n \t\techo \"$(git config difftool.$merge_tool.cmd ||\n@@ -92,6 +88,7 @@ get_merge_tool_cmd () {\n \tfi\n }\n \n+# Entry point for running tools\n run_merge_tool () {\n \t# If GIT_PREFIX is empty then we cannot use it in tools\n \t# that expect to be able to chdir() to its value.\n@@ -102,288 +99,15 @@ run_merge_tool () {\n \tbase_present=\"$2\"\n \tstatus=0\n \n-\tcase \"$1\" in\n-\taraxis)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present\n-\t\t\tthen\n-\t\t\t\t\"$merge_tool_path\" -wait -merge -3 -a1 \\\n-\t\t\t\t\t\"$BASE\" \"$LOCAL\" \"$REMOTE\" \"$MERGED\" \\\n-\t\t\t\t\t>/dev/null 2>&1\n-\t\t\telse\n-\t\t\t\t\"$merge_tool_path\" -wait -2 \\\n-\t\t\t\t\t\"$LOCAL\" \"$REMOTE\" \"$MERGED\" \\\n-\t\t\t\t\t>/dev/null 2>&1\n-\t\t\tfi\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\t\"$merge_tool_path\" -wait -2 \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t>/dev/null 2>&1\n-\t\tfi\n-\t\t;;\n-\tbc3)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present\n-\t\t\tthen\n-\t\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n-\t\t\t\t\t-mergeoutput=\"$MERGED\"\n-\t\t\telse\n-\t\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t\t-mergeoutput=\"$MERGED\"\n-\t\t\tfi\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n-\t\tfi\n-\t\t;;\n-\tdiffuse)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present\n-\t\t\tthen\n-\t\t\t\t\"$merge_tool_path\" \\\n-\t\t\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\" \\\n-\t\t\t\t\t\"$BASE\" | cat\n-\t\t\telse\n-\t\t\t\t\"$merge_tool_path\" \\\n-\t\t\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\" | cat\n-\t\t\tfi\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" | cat\n-\t\tfi\n-\t\t;;\n-\tecmerge)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present\n-\t\t\tthen\n-\t\t\t\t\"$merge_tool_path\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t\t--default --mode=merge3 --to=\"$MERGED\"\n-\t\t\telse\n-\t\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t\t--default --mode=merge2 --to=\"$MERGED\"\n-\t\t\tfi\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\t\"$merge_tool_path\" --default --mode=diff2 \\\n-\t\t\t\t\"$LOCAL\" \"$REMOTE\"\n-\t\tfi\n-\t\t;;\n-\temerge)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\tif $base_present\n-\t\t\tthen\n-\t\t\t\t\"$merge_tool_path\" \\\n-\t\t\t\t\t-f emerge-files-with-ancestor-command \\\n-\t\t\t\t\t\"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n-\t\t\t\t\t\"$(basename \"$MERGED\")\"\n-\t\t\telse\n-\t\t\t\t\"$merge_tool_path\" \\\n-\t\t\t\t\t-f emerge-files-command \\\n-\t\t\t\t\t\"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t\t\"$(basename \"$MERGED\")\"\n-\t\t\tfi\n-\t\t\tstatus=$?\n-\t\telse\n-\t\t\t\"$merge_tool_path\" -f emerge-files-command \\\n-\t\t\t\t\"$LOCAL\" \"$REMOTE\"\n-\t\tfi\n-\t\t;;\n-\tgvimdiff|vimdiff)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present\n-\t\t\tthen\n-\t\t\t\t\"$merge_tool_path\" -f -d -c \"wincmd J\" \\\n-\t\t\t\t\t\"$MERGED\" \"$LOCAL\" \"$BASE\" \"$REMOTE\"\n-\t\t\telse\n-\t\t\t\t\"$merge_tool_path\" -f -d -c \"wincmd l\" \\\n-\t\t\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n-\t\t\tfi\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\t\"$merge_tool_path\" -R -f -d -c \"wincmd l\" \\\n-\t\t\t\t-c 'cd $GIT_PREFIX' \\\n-\t\t\t\t\"$LOCAL\" \"$REMOTE\"\n-\t\tfi\n-\t\t;;\n-\tgvimdiff2|vimdiff2)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\t\"$merge_tool_path\" -f -d -c \"wincmd l\" \\\n-\t\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\t\"$merge_tool_path\" -R -f -d -c \"wincmd l\" \\\n-\t\t\t\t-c 'cd $GIT_PREFIX' \\\n-\t\t\t\t\"$LOCAL\" \"$REMOTE\"\n-\t\tfi\n-\t\t;;\n-\tkdiff3)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\tif $base_present\n-\t\t\tthen\n-\t\t\t\t\"$merge_tool_path\" --auto \\\n-\t\t\t\t\t--L1 \"$MERGED (Base)\" \\\n-\t\t\t\t\t--L2 \"$MERGED (Local)\" \\\n-\t\t\t\t\t--L3 \"$MERGED (Remote)\" \\\n-\t\t\t\t\t-o \"$MERGED\" \\\n-\t\t\t\t\t\"$BASE\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t>/dev/null 2>&1\n-\t\t\telse\n-\t\t\t\t\"$merge_tool_path\" --auto \\\n-\t\t\t\t\t--L1 \"$MERGED (Local)\" \\\n-\t\t\t\t\t--L2 \"$MERGED (Remote)\" \\\n-\t\t\t\t\t-o \"$MERGED\" \\\n-\t\t\t\t\t\"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t>/dev/null 2>&1\n-\t\t\tfi\n-\t\t\tstatus=$?\n-\t\telse\n-\t\t\t\"$merge_tool_path\" --auto \\\n-\t\t\t\t--L1 \"$MERGED (A)\" \\\n-\t\t\t\t--L2 \"$MERGED (B)\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t>/dev/null 2>&1\n-\t\tfi\n-\t\t;;\n-\tkompare)\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n-\t\t;;\n-\tmeld)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n-\t\tfi\n-\t\t;;\n-\topendiff)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present\n-\t\t\tthen\n-\t\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t\t-ancestor \"$BASE\" \\\n-\t\t\t\t\t-merge \"$MERGED\" | cat\n-\t\t\telse\n-\t\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t\t-merge \"$MERGED\" | cat\n-\t\t\tfi\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" | cat\n-\t\tfi\n-\t\t;;\n-\tp4merge)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\t$base_present || >\"$BASE\"\n-\t\t\t\"$merge_tool_path\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" \"$MERGED\"\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n-\t\tfi\n-\t\t;;\n-\ttkdiff)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\tif $base_present\n-\t\t\tthen\n-\t\t\t\t\"$merge_tool_path\" -a \"$BASE\" \\\n-\t\t\t\t\t-o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n-\t\t\telse\n-\t\t\t\t\"$merge_tool_path\" \\\n-\t\t\t\t\t-o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n-\t\t\tfi\n-\t\t\tstatus=$?\n-\t\telse\n-\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n-\t\tfi\n-\t\t;;\n-\ttortoisemerge)\n-\t\tif $base_present\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\t\"$merge_tool_path\" \\\n-\t\t\t\t-base:\"$BASE\" -mine:\"$LOCAL\" \\\n-\t\t\t\t-theirs:\"$REMOTE\" -merged:\"$MERGED\"\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\techo \"TortoiseMerge cannot be used without a base\" 1>&2\n-\t\t\tstatus=1\n-\t\tfi\n-\t\t;;\n-\txxdiff)\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttouch \"$BACKUP\"\n-\t\t\tif $base_present\n-\t\t\tthen\n-\t\t\t\t\"$merge_tool_path\" -X --show-merged-pane \\\n-\t\t\t\t\t-R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n-\t\t\t\t\t-R 'Accel.Search: \"Ctrl+F\"' \\\n-\t\t\t\t\t-R 'Accel.SearchForward: \"Ctrl-G\"' \\\n-\t\t\t\t\t--merged-file \"$MERGED\" \\\n-\t\t\t\t\t\"$LOCAL\" \"$BASE\" \"$REMOTE\"\n-\t\t\telse\n-\t\t\t\t\"$merge_tool_path\" -X $extra \\\n-\t\t\t\t\t-R 'Accel.SaveAsMerged: \"Ctrl-S\"' \\\n-\t\t\t\t\t-R 'Accel.Search: \"Ctrl+F\"' \\\n-\t\t\t\t\t-R 'Accel.SearchForward: \"Ctrl-G\"' \\\n-\t\t\t\t\t--merged-file \"$MERGED\" \\\n-\t\t\t\t\t\"$LOCAL\" \"$REMOTE\"\n-\t\t\tfi\n-\t\t\tcheck_unchanged\n-\t\telse\n-\t\t\t\"$merge_tool_path\" \\\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\"$LOCAL\" \"$REMOTE\"\n-\t\tfi\n-\t\t;;\n-\t*)\n-\t\tmerge_tool_cmd=\"$(get_merge_tool_cmd \"$1\")\"\n-\t\tif test -z \"$merge_tool_cmd\"\n-\t\tthen\n-\t\t\tif merge_mode\n-\t\t\tthen\n-\t\t\t\tstatus=1\n-\t\t\tfi\n-\t\t\tbreak\n-\t\tfi\n-\t\tif merge_mode\n-\t\tthen\n-\t\t\ttrust_exit_code=\"$(git config --bool \\\n-\t\t\t\tmergetool.\"$1\".trustExitCode || echo false)\"\n-\t\t\tif test \"$trust_exit_code\" = \"false\"\n-\t\t\tthen\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\telse\n-\t\t\t( eval $merge_tool_cmd )\n-\t\tfi\n-\t\t;;\n-\tesac\n+\t# Bring tool-specific functions into scope\n+\tsetup_tool \"$1\"\n+\n+\tif merge_mode\n+\tthen\n+\t\tmerge_cmd \"$1\"\n+\telse\n+\t\tdiff_cmd \"$1\"\n+\tfi\n \treturn $status\n }\n \n@@ -449,12 +173,7 @@ get_configured_merge_tool () {\n \n get_merge_tool_path () {\n \t# A merge tool has been set, so verify that it's valid.\n-\tif test -n \"$1\"\n-\tthen\n-\t\tmerge_tool=\"$1\"\n-\telse\n-\t\tmerge_tool=\"$(get_merge_tool)\"\n-\tfi\n+\tmerge_tool=\"$1\"\n \tif ! valid_tool \"$merge_tool\"\n \tthen\n \t\techo >&2 \"Unknown merge tool $merge_tool\"\n@@ -483,7 +202,7 @@ get_merge_tool_path () {\n \n get_merge_tool () {\n \t# Check if a merge tool has been configured\n-\tmerge_tool=$(get_configured_merge_tool)\n+\tmerge_tool=\"$(get_configured_merge_tool)\"\n \t# Try to guess an appropriate merge tool if no tool has been set.\n \tif test -z \"$merge_tool\"\n \tthen\ndiff --git a/mergetools/araxis b/mergetools/araxis\nnew file mode 100644\nindex 0000000..69a9c8c\n--- /dev/null\n+++ b/mergetools/araxis\n@@ -0,0 +1,21 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" -wait -2 \"$LOCAL\" \"$REMOTE\" >/dev/null 2>&1\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\tif $base_present\n+\tthen\n+\t\t\"$merge_tool_path\" -wait -merge -3 -a1 \\\n+\t\t\t\"$BASE\" \"$LOCAL\" \"$REMOTE\" \"$MERGED\" >/dev/null 2>&1\n+\telse\n+\t\t\"$merge_tool_path\" -wait -2 \\\n+\t\t\t\"$LOCAL\" \"$REMOTE\" \"$MERGED\" >/dev/null 2>&1\n+\tfi\n+\tcheck_unchanged\n+}\n+\n+translate_merge_tool_path() {\n+\techo compare\n+}\n+\ndiff --git a/mergetools/bc3 b/mergetools/bc3\nnew file mode 100644\nindex 0000000..27b3dd4\n--- /dev/null\n+++ b/mergetools/bc3\n@@ -0,0 +1,20 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\tif $base_present\n+\tthen\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n+\t\t\t-mergeoutput=\"$MERGED\"\n+\telse\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t-mergeoutput=\"$MERGED\"\n+\tfi\n+\tcheck_unchanged\n+}\n+\n+translate_merge_tool_path() {\n+\techo bcompare\n+}\ndiff --git a/mergetools/defaults b/mergetools/defaults\nnew file mode 100644\nindex 0000000..1d8f2a3\n--- /dev/null\n+++ b/mergetools/defaults\n@@ -0,0 +1,46 @@\n+# Redefined by builtin tools\n+can_merge () {\n+\treturn 0\n+}\n+\n+can_diff () {\n+\treturn 0\n+}\n+\n+diff_cmd () {\n+\tmerge_tool_cmd=\"$(get_merge_tool_cmd \"$1\")\"\n+\tif test -z \"$merge_tool_cmd\"\n+\tthen\n+\t\tstatus=1\n+\t\tbreak\n+\tfi\n+\t( eval $merge_tool_cmd )\n+\tstatus=$?\n+\treturn $status\n+}\n+\n+merge_cmd () {\n+\tmerge_tool_cmd=\"$(get_merge_tool_cmd \"$1\")\"\n+\tif test -z \"$merge_tool_cmd\"\n+\tthen\n+\t\tstatus=1\n+\t\tbreak\n+\tfi\n+\ttrust_exit_code=\"$(git config --bool \\\n+\t\tmergetool.\"$1\".trustExitCode || echo false)\"\n+\tif test \"$trust_exit_code\" = \"false\"\n+\tthen\n+\t\ttouch \"$BACKUP\"\n+\t\t( eval $merge_tool_cmd )\n+\t\tstatus=$?\n+\t\tcheck_unchanged\n+\telse\n+\t\t( eval $merge_tool_cmd )\n+\t\tstatus=$?\n+\tfi\n+\treturn $status\n+}\n+\n+translate_merge_tool_path () {\n+\techo \"$1\"\n+}\ndiff --git a/mergetools/diffuse b/mergetools/diffuse\nnew file mode 100644\nindex 0000000..02e0843\n--- /dev/null\n+++ b/mergetools/diffuse\n@@ -0,0 +1,17 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" | cat\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\tif $base_present\n+\tthen\n+\t\t\"$merge_tool_path\" \\\n+\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\" \\\n+\t\t\t\"$BASE\" | cat\n+\telse\n+\t\t\"$merge_tool_path\" \\\n+\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\" | cat\n+\tfi\n+\tcheck_unchanged\n+}\ndiff --git a/mergetools/ecmerge b/mergetools/ecmerge\nnew file mode 100644\nindex 0000000..13c2e43\n--- /dev/null\n+++ b/mergetools/ecmerge\n@@ -0,0 +1,16 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" --default --mode=diff2 \"$LOCAL\" \"$REMOTE\"\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\tif $base_present\n+\tthen\n+\t\t\"$merge_tool_path\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t--default --mode=merge3 --to=\"$MERGED\"\n+\telse\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t--default --mode=merge2 --to=\"$MERGED\"\n+\tfi\n+\tcheck_unchanged\n+}\ndiff --git a/mergetools/emerge b/mergetools/emerge\nnew file mode 100644\nindex 0000000..f96d9e5\n--- /dev/null\n+++ b/mergetools/emerge\n@@ -0,0 +1,23 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" -f emerge-files-command \"$LOCAL\" \"$REMOTE\"\n+}\n+\n+merge_cmd () {\n+\tif $base_present\n+\tthen\n+\t\t\"$merge_tool_path\" \\\n+\t\t\t-f emerge-files-with-ancestor-command \\\n+\t\t\t\"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n+\t\t\t\"$(basename \"$MERGED\")\"\n+\telse\n+\t\t\"$merge_tool_path\" \\\n+\t\t\t-f emerge-files-command \\\n+\t\t\t\"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t\"$(basename \"$MERGED\")\"\n+\tfi\n+\tstatus=$?\n+}\n+\n+translate_merge_tool_path() {\n+\techo emacs\n+}\ndiff --git a/mergetools/kdiff3 b/mergetools/kdiff3\nnew file mode 100644\nindex 0000000..28fead4\n--- /dev/null\n+++ b/mergetools/kdiff3\n@@ -0,0 +1,24 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" --auto \\\n+\t\t--L1 \"$MERGED (A)\" --L2 \"$MERGED (B)\" \\\n+\t\t\"$LOCAL\" \"$REMOTE\" >/dev/null 2>&1\n+}\n+\n+merge_cmd () {\n+\tif $base_present\n+\tthen\n+\t\t\"$merge_tool_path\" --auto \\\n+\t\t\t--L1 \"$MERGED (Base)\" \\\n+\t\t\t--L2 \"$MERGED (Local)\" \\\n+\t\t\t--L3 \"$MERGED (Remote)\" \\\n+\t\t\t-o \"$MERGED\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t>/dev/null 2>&1\n+\telse\n+\t\t\"$merge_tool_path\" --auto \\\n+\t\t\t--L1 \"$MERGED (Local)\" \\\n+\t\t\t--L2 \"$MERGED (Remote)\" \\\n+\t\t\t-o \"$MERGED\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t>/dev/null 2>&1\n+\tfi\n+\tstatus=$?\n+}\ndiff --git a/mergetools/kompare b/mergetools/kompare\nnew file mode 100644\nindex 0000000..433686c\n--- /dev/null\n+++ b/mergetools/kompare\n@@ -0,0 +1,7 @@\n+can_merge () {\n+\treturn 1\n+}\n+\n+diff_cmd () {\n+\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+}\ndiff --git a/mergetools/meld b/mergetools/meld\nnew file mode 100644\nindex 0000000..73d70ae\n--- /dev/null\n+++ b/mergetools/meld\n@@ -0,0 +1,9 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\tcheck_unchanged\n+}\ndiff --git a/mergetools/opendiff b/mergetools/opendiff\nnew file mode 100644\nindex 0000000..0942b2a\n--- /dev/null\n+++ b/mergetools/opendiff\n@@ -0,0 +1,16 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" | cat\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\tif $base_present\n+\tthen\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t-ancestor \"$BASE\" -merge \"$MERGED\" | cat\n+\telse\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t-merge \"$MERGED\" | cat\n+\tfi\n+\tcheck_unchanged\n+}\ndiff --git a/mergetools/p4merge b/mergetools/p4merge\nnew file mode 100644\nindex 0000000..1a45c1b\n--- /dev/null\n+++ b/mergetools/p4merge\n@@ -0,0 +1,10 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\t$base_present || >\"$BASE\"\n+\t\"$merge_tool_path\" \"$BASE\" \"$LOCAL\" \"$REMOTE\" \"$MERGED\"\n+\tcheck_unchanged\n+}\ndiff --git a/mergetools/tkdiff b/mergetools/tkdiff\nnew file mode 100644\nindex 0000000..618c438\n--- /dev/null\n+++ b/mergetools/tkdiff\n@@ -0,0 +1,12 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+}\n+\n+merge_cmd () {\n+\tif $base_present\n+\tthen\n+\t\t\"$merge_tool_path\" -a \"$BASE\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n+\telse\n+\t\t\"$merge_tool_path\" -o \"$MERGED\" \"$LOCAL\" \"$REMOTE\"\n+\tfi\n+}\ndiff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge\nnew file mode 100644\nindex 0000000..ed7db49\n--- /dev/null\n+++ b/mergetools/tortoisemerge\n@@ -0,0 +1,17 @@\n+can_diff () {\n+\treturn 1\n+}\n+\n+merge_cmd () {\n+\tif $base_present\n+\tthen\n+\t\ttouch \"$BACKUP\"\n+\t\t\"$merge_tool_path\" \\\n+\t\t\t-base:\"$BASE\" -mine:\"$LOCAL\" \\\n+\t\t\t-theirs:\"$REMOTE\" -merged:\"$MERGED\"\n+\t\tcheck_unchanged\n+\telse\n+\t\techo \"TortoiseMerge cannot be used without a base\" 1>&2\n+\t\treturn 1\n+\tfi\n+}\ndiff --git a/mergetools/vim b/mergetools/vim\nnew file mode 100644\nindex 0000000..619594a\n--- /dev/null\n+++ b/mergetools/vim\n@@ -0,0 +1,44 @@\n+diff_cmd () {\n+\tcase \"$1\" in\n+\tgvimdiff|vimdiff)\n+\t\t\"$merge_tool_path\" -R -f -d \\\n+\t\t\t-c 'wincmd l' -c 'cd $GIT_PREFIX' \"$LOCAL\" \"$REMOTE\"\n+\t\t;;\n+\tgvimdiff2|vimdiff2)\n+\t\t\"$merge_tool_path\" -R -f -d \\\n+\t\t\t-c 'wincmd l' -c 'cd $GIT_PREFIX' \"$LOCAL\" \"$REMOTE\"\n+\t\t;;\n+\tesac\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\tcase \"$1\" in\n+\tgvimdiff|vimdiff)\n+\t\tif $base_present\n+\t\tthen\n+\t\t\t\"$merge_tool_path\" -f -d -c 'wincmd J' \\\n+\t\t\t\t\"$MERGED\" \"$LOCAL\" \"$BASE\" \"$REMOTE\"\n+\t\telse\n+\t\t\t\"$merge_tool_path\" -f -d -c 'wincmd l' \\\n+\t\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\t\tfi\n+\t\t;;\n+\tgvimdiff2|vimdiff2)\n+\t\t\"$merge_tool_path\" -f -d -c 'wincmd l' \\\n+\t\t\t\"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\t\t;;\n+\tesac\n+\tcheck_unchanged\n+}\n+\n+translate_merge_tool_path() {\n+\tcase \"$1\" in\n+\tgvimdiff|gvimdiff2)\n+\t\techo gvim\n+\t\t;;\n+\tvimdiff|vimdiff2)\n+\t\techo vim\n+\t\t;;\n+\tesac\n+}\ndiff --git a/mergetools/xxdiff b/mergetools/xxdiff\nnew file mode 100644\nindex 0000000..05b4433\n--- /dev/null\n+++ b/mergetools/xxdiff\n@@ -0,0 +1,25 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" \\\n+\t\t-R 'Accel.Search: \"Ctrl+F\"' \\\n+\t\t-R 'Accel.SearchForward: \"Ctrl-G\"' \\\n+\t\t\"$LOCAL\" \"$REMOTE\"\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\tif $base_present\n+\tthen\n+\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+\telse\n+\t\t\"$merge_tool_path\" -X $extra \\\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\" \"$REMOTE\"\n+\tfi\n+\tcheck_unchanged\n+}\n-- \n1.7.6.476.g57292\n"},{"id":"173734","messageId":"1313652227-48545-5-git-send-email-davvid@gmail.com","threadId":"28151","inReplyTo":"1313652227-48545-1-git-send-email-davvid@gmail.com","subject":"[PATCH 4/4] mergetools/meld: Use '--output' when available","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-08-18T07:23:47Z","receivedAt":"2011-08-18T07:23:47Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"meld 1.5.0 and newer allow the output file to be specified\nwhen merging multiple files.  Check the meld version and use\nthe '--output' option when available.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n mergetools/meld |   33 ++++++++++++++++++++++++++++++++-\n 1 files changed, 32 insertions(+), 1 deletions(-)\n\ndiff --git a/mergetools/meld b/mergetools/meld\nindex 73d70ae..1923797 100644\n--- a/mergetools/meld\n+++ b/mergetools/meld\n@@ -4,6 +4,37 @@ diff_cmd () {\n \n merge_cmd () {\n \ttouch \"$BACKUP\"\n-\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\tif test \"$meld_has_output_option\" = true\n+\tthen\n+\t\t\"$merge_tool_path\" --output \"$MERGED\" \\\n+\t\t\t\"$BASE\" \"$LOCAL\" \"$REMOTE\"\n+\telse\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\tfi\n \tcheck_unchanged\n }\n+\n+check_meld_version () {\n+\tif test -n \"$meld_version_checked\"\n+\tthen\n+\t\treturn\n+\tfi\n+\t# Whether 'meld --output <file>' is supported\n+\tmeld_has_output_option=false\n+\n+\t# Filter meld --version to contain numbers and dots only\n+\tmeld=\"$(meld --version 2>/dev/null | sed -e 's/[^0-9.]\\+//g')\"\n+\tmeld=\"${meld:-0.0.0}\"\n+\n+\tmeld_major=\"$(expr \"$meld\" : '\\([0-9]\\{1,\\}\\)' || echo 0)\"\n+\tmeld_minor=\"$(expr \"$meld\" : '[0-9]\\{1,\\}\\.\\([0-9]\\{1,\\}\\)' || echo 0)\"\n+\n+\t# 'meld --output <file>' was introduced in meld 1.5.0\n+\tif test \"$meld_major\" -gt 1 ||\n+\t\t(test \"$meld_major\" = 1 && test \"$meld_minor\" -ge 5)\n+\tthen\n+\t\tmeld_has_output_option=true\n+\tfi\n+\tmeld_version_checked=true\n+}\n+check_meld_version\n-- \n1.7.6.476.g57292\n"},{"id":"173736","messageId":"CAHGBnuPRsh0WDVSFr0X1Y+nb=94pJVv8=evFQCjdcd_y9ux97A@mail.gmail.com","threadId":"28151","inReplyTo":"1313652227-48545-2-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 1/4] difftool--helper: Make style consistent with git","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2011-08-18T07:49:33Z","receivedAt":"2011-08-18T07:49:33Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Thu, Aug 18, 2011 at 09:23, David Aguilar <davvid@gmail.com> wrote:\n\n> Use the predominant conditional style where \"then\" appears\n> alone on the line after the test expression.\n\nI support your effort to unify the style, but for my personal taste it\nhas gone the wrong way :-) Even if \"then\" on its own line was the\npredominant style in the merge scripts, I would have voted for putting\nall \"then\" in the same line as \"if\" as I find this easier to read and\ncomprehend. But then again it's obvious that such a change would\npolarize ;-)\n\n-- \nSebastian Schuberth\n"},{"id":"173738","messageId":"CAHGBnuOziicuWPhRhtmpva8pt5ybJTTDP-mU=1zcnNZBMbz3Pw@mail.gmail.com","threadId":"28151","inReplyTo":"1313652227-48545-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 0/4] Refactor git-mergetool--lib.sh","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2011-08-18T07:57:50Z","receivedAt":"2011-08-18T07:57:50Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Thu, Aug 18, 2011 at 09:23, David Aguilar <davvid@gmail.com> wrote:\n\n> This series splits the git-mergetool--lib.sh file into separate\n> tool scriptlets that define the commands for diff and merge.\n\nVery nice as it simplifies adding new merge tools to adding a single\nfile with little code inside. Thanks!\n\n-- \nSebastian Schuberth\n"},{"id":"173740","messageId":"20110818081309.GP31888@elie.gateway.2wire.net","threadId":"28151","inReplyTo":"1313652227-48545-5-git-send-email-davvid@gmail.com","subject":"Re: [PATCH 4/4] mergetools/meld: Use '--output' when available","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-08-18T08:13:10Z","receivedAt":"2011-08-18T08:13:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"David Aguilar wrote:\n\n> use the '--output' option when available.\n\nYay. :)\n\n> --- a/mergetools/meld\n> +++ b/mergetools/meld\n> @@ -4,6 +4,37 @@ diff_cmd () {\n>  \n>  merge_cmd () {\n>  \ttouch \"$BACKUP\"\n> -\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n> +\tif test \"$meld_has_output_option\" = true\n> +\tthen\n> +\t\t\"$merge_tool_path\" --output \"$MERGED\" \\\n> +\t\t\t\"$BASE\" \"$LOCAL\" \"$REMOTE\"\n\nShouldn't this be \"$LOCAL\" \"$BASE\" \"$REMOTE\"?\n\n> +\telse\n> +\t\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n> +\tfi\n>  \tcheck_unchanged\n\nI wonder if the version test could be made a little simpler (perhaps\nto cope better if future versions use a different numbering system):\n\n\tif \"$merge_tool_path\" --output /dev/null --help >/dev/null 2>&1\n\tthen\n\t\t\"$merge_tool_path\" --output ...\n\telse\n\t\t\"$merge_tool_path\" \"$LOCAL\" ...\n\tfi\n\nForgive my ignorance: is this function likely to be called in a loop?\nIf so, it makes sense to precompute or cache the result of detection,\nlike you already do.\n\n\tcheck_meld_for_output_option () {\n\t\tif ...\n\t\tthen\n\t\t\tmeld_has_output_option=true\n\t\telse\n\t\t\tmeld_has_output_option=false\n\t\tfi\n\t}\n\n\tmerge_cmd () {\n\t\tif test -z \"${meld_has_output_option:+set}\"\n\t\tthen\n\t\t\tcheck_meld_for_output_option\n\t\tfi\n\n\t\tif test \"$meld_has_output_option\" = true\n\t\tthen\n\t\t\t...\n\n\n[...]\n> +\t# Filter meld --version to contain numbers and dots only\n> +\tmeld=\"$(meld --version 2>/dev/null | sed -e 's/[^0-9.]\\+//g')\"\n\n\\+ is not a BRE.  If parsing version numbers seems like the right\nthing to do, maybe \"tr -cd 0-9.\"?\n\n> +\tmeld=\"${meld:-0.0.0}\"\n> +\n> +\tmeld_major=\"$(expr \"$meld\" : '\\([0-9]\\{1,\\}\\)' || echo 0)\"\n> +\tmeld_minor=\"$(expr \"$meld\" : '[0-9]\\{1,\\}\\.\\([0-9]\\{1,\\}\\)' || echo 0)\"\n\nI think git avoids \\{m,n\\} ranges where possible (for portability).\nThis could be:\n\n\tmeld_major=${meld%%.*}\n\tmeld_nonmajor=${meld#${meld_major}.}\n\tmeld_minor=${meld_nonmajor%%.*}\n\nor:\n\n\tcase $meld in\n\t[2-9].* | [1-9][0-9]* | 1.[5-9]* | 1.[1-9][0-9]*)\t# >= 1.5.0\n\t\tmeld_has_output_option=true ;;\n\t*)\n\t\tmeld_has_output_option=false ;;\n\tesac\n\nIt's nice how self-contained this can be now that it's in its own\nfile.  Thanks.\n"},{"id":"173742","messageId":"20110818090810.GA15416@gmail.com","threadId":"28151","inReplyTo":"CAHGBnuPRsh0WDVSFr0X1Y+nb=94pJVv8=evFQCjdcd_y9ux97A@mail.gmail.com","subject":"Re: [PATCH 1/4] difftool--helper: Make style consistent with git","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-08-18T09:08:12Z","receivedAt":"2011-08-18T09:08:12Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Aug 18, 2011 at 09:49:33AM +0200, Sebastian Schuberth wrote:\n> On Thu, Aug 18, 2011 at 09:23, David Aguilar <davvid@gmail.com> wrote:\n> \n> > Use the predominant conditional style where \"then\" appears\n> > alone on the line after the test expression.\n> \n> I support your effort to unify the style, but for my personal taste it\n> has gone the wrong way :-) Even if \"then\" on its own line was the\n> predominant style in the merge scripts, I would have voted for putting\n> all \"then\" in the same line as \"if\" as I find this easier to read and\n> comprehend. But then again it's obvious that such a change would\n> polarize ;-)\n> \n> -- \n> Sebastian Schuberth\n\nI am fine reworking it either way.\nThere's not a specific note about it in\nDocumentation/CodingStyle.\n\nIf someone wants to make the final call I can patch the docs\nand rework these patches as needed.\n\nThanks,\n-- \n\t\t\t\t\tDavid\n"},{"id":"173744","messageId":"20110818092403.GB15416@gmail.com","threadId":"28151","inReplyTo":"20110818081309.GP31888@elie.gateway.2wire.net","subject":"Re: [PATCH 4/4] mergetools/meld: Use '--output' when available","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-08-18T09:24:06Z","receivedAt":"2011-08-18T09:24:06Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, Aug 18, 2011 at 03:13:10AM -0500, Jonathan Nieder wrote:\n> David Aguilar wrote:\n> \n> > use the '--output' option when available.\n> \n> Yay. :)\n> \n> > --- a/mergetools/meld\n> > +++ b/mergetools/meld\n> > @@ -4,6 +4,37 @@ diff_cmd () {\n> >  \n> >  merge_cmd () {\n> >  \ttouch \"$BACKUP\"\n> > -\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n> > +\tif test \"$meld_has_output_option\" = true\n> > +\tthen\n> > +\t\t\"$merge_tool_path\" --output \"$MERGED\" \\\n> > +\t\t\t\"$BASE\" \"$LOCAL\" \"$REMOTE\"\n> \n> Shouldn't this be \"$LOCAL\" \"$BASE\" \"$REMOTE\"?\n\nYup, thanks.\n\n> \n> > +\telse\n> > +\t\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n> > +\tfi\n> >  \tcheck_unchanged\n> \n> I wonder if the version test could be made a little simpler (perhaps\n> to cope better if future versions use a different numbering system):\n> \n> \tif \"$merge_tool_path\" --output /dev/null --help >/dev/null 2>&1\n> \tthen\n> \t\t\"$merge_tool_path\" --output ...\n> \telse\n> \t\t\"$merge_tool_path\" \"$LOCAL\" ...\n> \tfi\n> \n> Forgive my ignorance: is this function likely to be called in a loop?\n> If so, it makes sense to precompute or cache the result of detection,\n> like you already do.\n\nYes, it is called in a loop...\n\n\n> \tcheck_meld_for_output_option () {\n> \t\tif ...\n> \t\tthen\n> \t\t\tmeld_has_output_option=true\n> \t\telse\n> \t\t\tmeld_has_output_option=false\n> \t\tfi\n> \t}\n> \n> \tmerge_cmd () {\n> \t\tif test -z \"${meld_has_output_option:+set}\"\n> \t\tthen\n> \t\t\tcheck_meld_for_output_option\n> \t\tfi\n> \n> \t\tif test \"$meld_has_output_option\" = true\n> \t\tthen\n> \t\t\t...\n> \n> \n> [...]\n> > +\t# Filter meld --version to contain numbers and dots only\n> > +\tmeld=\"$(meld --version 2>/dev/null | sed -e 's/[^0-9.]\\+//g')\"\n> \n> \\+ is not a BRE.  If parsing version numbers seems like the right\n> thing to do, maybe \"tr -cd 0-9.\"?\n> \n> > +\tmeld=\"${meld:-0.0.0}\"\n> > +\n> > +\tmeld_major=\"$(expr \"$meld\" : '\\([0-9]\\{1,\\}\\)' || echo 0)\"\n> > +\tmeld_minor=\"$(expr \"$meld\" : '[0-9]\\{1,\\}\\.\\([0-9]\\{1,\\}\\)' || echo 0)\"\n> \n> I think git avoids \\{m,n\\} ranges where possible (for portability).\n> This could be:\n> \n> \tmeld_major=${meld%%.*}\n> \tmeld_nonmajor=${meld#${meld_major}.}\n> \tmeld_minor=${meld_nonmajor%%.*}\n> \n> or:\n> \n> \tcase $meld in\n> \t[2-9].* | [1-9][0-9]* | 1.[5-9]* | 1.[1-9][0-9]*)\t# >= 1.5.0\n> \t\tmeld_has_output_option=true ;;\n> \t*)\n> \t\tmeld_has_output_option=false ;;\n> \tesac\n> \n> It's nice how self-contained this can be now that it's in its own\n> file.  Thanks.\n\nRight, I was using \\{1,\\} since that's what CodingStyle said to\nuse instead of \\+ (which I forgot to fixup above as you saw).\n\nThe case statement is nice and simple enough to understand.\nBy doing meld --output /dev/null --help we're relying on\nolder versions blowing up with --output and --help returning\nexit status 0.  That seems pretty reasonable.\nThe case statement does seem sufficient but just trying\n--output and seeing what happens is even simpler.\nYour example also uses $merge_tool_path, which I forgot to do,\nso I'll be sure to include that too.\n\nI'll wait until tomorrow to see if there are any more comments\nand reroll.\n\nThanks,\n-- \n\t\t\t\t\tDavid\n"},{"id":"173765","messageId":"7vvctubp80.fsf@alter.siamese.dyndns.org","threadId":"28151","inReplyTo":"20110818090810.GA15416@gmail.com","subject":"Re: [PATCH 1/4] difftool--helper: Make style consistent with git","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-18T17:31:59Z","receivedAt":"2011-08-18T17:31:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> I am fine reworking it either way.\n> There's not a specific note about it in\n> Documentation/CodingStyle.\n>\n> If someone wants to make the final call I can patch the docs\n> and rework these patches as needed.\n\nPlease stick to (and document if not):\n\n\tif test ...\n        then\n        \taction\n\telif condition that can become looong so more &&\n        \tcondition needs to be split to next line &&\n                even more condition\n\tthen\n\t\taction\n\telse\n\t\taction\n\tfi\n\nThanks.\n\nYou can think of it as \"war against semicolon\" if you want.\n"},{"id":"173843","messageId":"20110819091444.GB18054@gmail.com","threadId":"28151","inReplyTo":"1313652227-48545-5-git-send-email-davvid@gmail.com","subject":"[PATCH v2 4/4] mergetools/meld: Use '--output' when available","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-08-19T09:14:45Z","receivedAt":"2011-08-19T09:14:45Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"meld 1.5.0 and newer allow the output file to be specified\nwhen merging multiple files.  Check whether the meld command\nsupports '--output' and use it when available.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\n mergetools/meld |   25 ++++++++++++++++++++++++-\n 1 files changed, 24 insertions(+), 1 deletions(-)\n\nChanges since v1 with help from Jonathan Nieder:\n\nUse $LOCAL $BASE REMOTE when calling meld.\nAvoid shell portability issues by using the return status.\nof --output instead of parsing --version.\n\ndiff --git a/mergetools/meld b/mergetools/meld\nindex 73d70ae..eaa115c 100644\n--- a/mergetools/meld\n+++ b/mergetools/meld\n@@ -3,7 +3,30 @@ diff_cmd () {\n }\n \n merge_cmd () {\n+\tif test -z \"${meld_has_output_option:+set}\"\n+\tthen\n+\t\tcheck_meld_for_output_version\n+\tfi\n \ttouch \"$BACKUP\"\n-\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\tif test \"$meld_has_output_option\" = true\n+\tthen\n+\t\t\"$merge_tool_path\" --output \"$MERGED\" \\\n+\t\t\t\"$LOCAL\" \"$BASE\" \"$REMOTE\"\n+\telse\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\tfi\n \tcheck_unchanged\n }\n+\n+# Check whether 'meld --output <file>' is supported\n+check_meld_for_output_version () {\n+\tmeld_path=\"$(git config mergetool.meld.path)\"\n+\tmeld_path=\"${meld_path:-meld}\"\n+\n+\tif \"$meld_path\" --output /dev/null --help >/dev/null 2>&1\n+\tthen\n+\t\tmeld_has_output_option=true\n+\telse\n+\t\tmeld_has_output_option=false\n+\tfi\n+}\n-- \n1.7.6.553.g1899\n"},{"id":"173898","messageId":"7vd3g1100w.fsf@alter.siamese.dyndns.org","threadId":"28151","inReplyTo":"20110819091444.GB18054@gmail.com","subject":"Re: [PATCH v2 4/4] mergetools/meld: Use '--output' when available","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-19T22:58:39Z","receivedAt":"2011-08-19T22:58:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> meld 1.5.0 and newer allow the output file to be specified\n> when merging multiple files.  Check whether the meld command\n> supports '--output' and use it when available.\n>\n> Signed-off-by: David Aguilar <davvid@gmail.com>\n> ---\n>  mergetools/meld |   25 ++++++++++++++++++++++++-\n>  1 files changed, 24 insertions(+), 1 deletions(-)\n>\n> Changes since v1 with help from Jonathan Nieder:\n\nThanks, both of you. Queued all patches in the series.\n"},{"id":"177266","messageId":"20111009091617.GA29150@elie.hsd1.il.comcast.net","threadId":"28151","inReplyTo":"1313652227-48545-4-git-send-email-davvid@gmail.com","subject":"[PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-09T09:17:07Z","receivedAt":"2011-10-09T09:17:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ever since mergetool--lib was split into multiple files in\nv1.7.7-rc0~3^2~1 (2011-08-18), the Makefile takes care to reset umask\nand use tar --no-owner when installing merge tool definitions to\n$(gitexecdir)/mergetools/.  Unfortunately it does not take into\naccount the possibility that the permission bits of the files being\ncopied might already be wrong.\n\nRather than fixing the \"tar\" incantation and making it even more\ncomplicated, let's just use the \"install\" utility.  This only means\nlosing the ability to install executables and subdirectories of\nmergetools/, which wasn't used.\n\nNoticed by installing from a copy of git checked out with umask 002.\nCompare v1.6.0.3~81^2 (Fix permission bits on sources checked out with\nan overtight umask, 2008-08-21).\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nDavid Aguilar wrote:\n\n> +++ b/Makefile\n[...]\n> @@ -2266,6 +2274,9 @@ install: all\n>  \t$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n>  \t$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'\n>  \t$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install\n> +\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'\n> +\t(cd mergetools && $(TAR) cf - .) | \\\n> +\t(cd '$(DESTDIR_SQ)$(mergetools_instdir_SQ)' && umask 022 && $(TAR) xof -)\n\nLast month I tried this out and found that, strangely, my files under\n/usr/lib/git-core/mergetools/ had the g+w bit set.  Leading me to\nwonder: does the \"umask\" here have any effect at all?\n\nSince debian/rules install is run as root, the default is for tar to\nact as thought --preserve-permissions were passed, so the umask when\nrunning \"tar\" is not relevant.  Luckily I think \"tar\" is overkill\nhere, anyway.\n\nThoughts?  Sorry to have taken so long to send this out.\n\n Makefile |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 1e91b19c..e27755e7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2275,8 +2275,7 @@ install: all\n \t$(INSTALL) $(install_bindir_programs) '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'\n-\t(cd mergetools && $(TAR) cf - .) | \\\n-\t(cd '$(DESTDIR_SQ)$(mergetools_instdir_SQ)' && umask 022 && $(TAR) xof -)\n+\t$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'\n ifndef NO_PERL\n \t$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install\n \t$(MAKE) -C gitweb install\n-- \n1.7.7.rc1\n"},{"id":"177274","messageId":"20111009114333.GA29829@elie.hsd1.il.comcast.net","threadId":"28151","inReplyTo":"20111009091617.GA29150@elie.hsd1.il.comcast.net","subject":"Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-09T11:43:33Z","receivedAt":"2011-10-09T11:43:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> ---\n[...]\n> Since debian/rules install is run as root, the default is for tar to\n> act as thought --preserve-permissions were passed\n\nI should have said: \"when 'make install' is run as root, ...\".\n\nTypically people building git for private use would run \"make install\"\nas root when installing to /usr/local, but as an unprivileged user\nwhen installing to $HOME.  The RPM packaging runs \"make install\"\nwithout special privileges and the Debian packaging runs it as (fake)\nroot, iirc.\n\nSorry for the lack of clarity.\n"},{"id":"177281","messageId":"1C9F1683-4C6E-4D49-86D3-3A47B2843F23@gmail.com","threadId":"28151","inReplyTo":"20111009114333.GA29829@elie.hsd1.il.comcast.net","subject":"Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-10-09T18:26:30Z","receivedAt":"2011-10-09T18:26:30Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Oct 9, 2011, at 4:43 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n> Jonathan Nieder wrote:\n> \n>> ---\n> [...]\n>> Since debian/rules install is run as root, the default is for tar to\n>> act as thought --preserve-permissions were passed\n> \n> I should have said: \"when 'make install' is run as root, ...\".\n> \n> Typically people building git for private use would run \"make install\"\n> as root when installing to /usr/local, but as an unprivileged user\n> when installing to $HOME.  The RPM packaging runs \"make install\"\n> without special privileges and the Debian packaging runs it as (fake)\n> root, iirc.\n> \n> Sorry for the lack of clarity.\n\nthanks. I agree that the tar is overkill. I think I copied that snippet from templates/makefile. does that have the same bug?\n-- \n    David (mobile)"},{"id":"177286","messageId":"20111009214752.GB3006@elie","threadId":"28151","inReplyTo":"1C9F1683-4C6E-4D49-86D3-3A47B2843F23@gmail.com","subject":"Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-10-09T21:47:52Z","receivedAt":"2011-10-09T21:47:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"David Aguilar wrote:\n\n> thanks. I agree that the tar is overkill. I think I copied that\n> snippet from templates/makefile. does that have the same bug?\n\nIt did have a similar bug before, but it was fixed (in a different\nway) by v1.6.0.3~81^2 (Fix permission bits on sources checked out with\nan overtight umask, 2008-08-21).  The main difference from the\nmergetools/ case is that the blt/ directory is populated at build\ntime.\n"},{"id":"177347","messageId":"7v8vosiehb.fsf@alter.siamese.dyndns.org","threadId":"28151","inReplyTo":"1C9F1683-4C6E-4D49-86D3-3A47B2843F23@gmail.com","subject":"Re: [PATCH da/difftool-mergtool-refactor] Makefile: fix permissions of mergetools/ checked out with permissive umask","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-10-11T00:00:48Z","receivedAt":"2011-10-11T00:00:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar <davvid@gmail.com> writes:\n\n> thanks. I agree that the tar is overkill. I think I copied that snippet\n> from templates/makefile. does that have the same bug?\n\nLikely, as I recall that I wrote the installation of templates in an\nexpecially sloppy way, thinking somebody would fix it in short order\nanyway. Instead the sloppiness seems to have been copied and pasted to\nmake things worse...\n\nThanks for fixing the mess---all's well that ends well.\n"}]}