{"thread":{"id":"32736","subject":"[PATCH 1/2] mergetool--lib: don't call \"exit\" in setup_tool","startedAt":"2013-01-27T00:40:06Z","lastAt":"2013-01-27T00:40:07Z","messageCount":2,"participants":["David Aguilar"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"207899","messageId":"1359247207-71819-1-git-send-email-davvid@gmail.com","threadId":"32736","inReplyTo":null,"subject":"[PATCH 1/2] mergetool--lib: don't call \"exit\" in setup_tool","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-01-27T00:40:06Z","receivedAt":"2013-01-27T00:40:06Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"From: John Keeping <john@keeping.me.uk>\n\nThis will make it easier to use setup_tool in places where we expect\nthat the selected tool will not support the current mode.\n\nWe need to introduce a new return code for setup_tool to differentiate\nbetween the case of \"the selected tool is invalid\" and \"the selected\ntool is not a built-in\" since we must call setup_tool when a custom\n'merge.<tool>.path' is configured for a built-in tool but avoid failing\nwhen the configured tool is not a built-in.\n\nSigned-off-by: John Keeping <john@keeping.me.uk>\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\nThis series is based on jk/mergetool in \"pu\".\nThis patch is unchanged from $gmane/214624.\n\n git-mergetool--lib.sh | 23 ++++++++++++++++++++---\n 1 file changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex aa38bd1..f1bb372 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -58,7 +58,11 @@ setup_tool () {\n \t. \"$mergetools/defaults\"\n \tif ! test -f \"$mergetools/$tool\"\n \tthen\n-\t\treturn 1\n+\t\t# Use a special return code for this case since we want to\n+\t\t# source \"defaults\" even when an explicit tool path is\n+\t\t# configured since the user can use that to override the\n+\t\t# default path in the scriptlet.\n+\t\treturn 2\n \tfi\n \n \t# Load the redefined functions\n@@ -67,11 +71,11 @@ setup_tool () {\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+\t\treturn 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+\t\treturn 1\n \tfi\n \treturn 0\n }\n@@ -101,6 +105,19 @@ run_merge_tool () {\n \n \t# Bring tool-specific functions into scope\n \tsetup_tool \"$1\"\n+\texitcode=$?\n+\tcase $exitcode in\n+\t0)\n+\t\t:\n+\t\t;;\n+\t2)\n+\t\t# The configured tool is not a built-in tool.\n+\t\ttest -n \"$merge_tool_path\" || return 1\n+\t\t;;\n+\t*)\n+\t\treturn $exitcode\n+\t\t;;\n+\tesac\n \n \tif merge_mode\n \tthen\n-- \n1.8.0.8.g9bc9422\n"},{"id":"207900","messageId":"1359247207-71819-2-git-send-email-davvid@gmail.com","threadId":"32736","inReplyTo":"1359247207-71819-1-git-send-email-davvid@gmail.com","subject":"[PATCH v2 2/2] mergetools: Simplify how we handle \"vim\" and \"defaults\"","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-01-27T00:40:07Z","receivedAt":"2013-01-27T00:40:07Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Remove the exceptions for \"vim\" and \"defaults\" in the mergetool library\nso that every filename in mergetools/ matches 1:1 with the name of a\nvalid built-in tool.\n\nMake common functions available in $MERGE_TOOLS_DIR/include/.\n\nSigned-off-by: David Aguilar <davvid@gmail.com>\n---\nThis diffstat is much nicer now thanks to John's setup_tool rework in 1/1.\n\n Makefile                                     |  6 +++-\n git-mergetool--lib.sh                        | 41 ++++++++--------------------\n mergetools/gvimdiff                          |  1 +\n mergetools/gvimdiff2                         |  1 +\n mergetools/{defaults => include/defaults.sh} |  0\n mergetools/{vim => vimdiff}                  |  0\n mergetools/vimdiff2                          |  1 +\n 7 files changed, 19 insertions(+), 31 deletions(-)\n create mode 100644 mergetools/gvimdiff\n create mode 100644 mergetools/gvimdiff2\n rename mergetools/{defaults => include/defaults.sh} (100%)\n rename mergetools/{vim => vimdiff} (100%)\n create mode 100644 mergetools/vimdiff2\n\ndiff --git a/Makefile b/Makefile\nindex f69979e..0f89032 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2724,7 +2724,11 @@ 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$(INSTALL) -m 644 mergetools/* '$(DESTDIR_SQ)$(mergetools_instdir_SQ)'\n+\t$(INSTALL) -m 644 $(filter-out include,$(wildcard mergetools/*)) \\\n+\t\t'$(DESTDIR_SQ)$(mergetools_instdir_SQ)'\n+\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'\n+\t$(INSTALL) -m 644 mergetools/include/* \\\n+\t\t'$(DESTDIR_SQ)$(mergetools_instdir_SQ)/include'\n ifndef NO_GETTEXT\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(localedir_SQ)'\n \t(cd po/build/locale && $(TAR) cf - .) | \\\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex f1bb372..7ea7510 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -1,5 +1,7 @@\n #!/bin/sh\n # git-mergetool--lib is a library for common merge tool functions\n+MERGE_TOOLS_DIR=$(git --exec-path)/mergetools\n+\n diff_mode() {\n \ttest \"$TOOL_MODE\" = diff\n }\n@@ -44,19 +46,9 @@ valid_tool () {\n }\n \n setup_tool () {\n-\tcase \"$1\" in\n-\tvim*|gvim*)\n-\t\ttool=vim\n-\t\t;;\n-\t*)\n-\t\ttool=\"$1\"\n-\t\t;;\n-\tesac\n-\tmergetools=\"$(git --exec-path)/mergetools\"\n+\ttool=\"$1\"\n \n-\t# Load the default definitions\n-\t. \"$mergetools/defaults\"\n-\tif ! test -f \"$mergetools/$tool\"\n+\tif ! test -f \"$MERGE_TOOLS_DIR/$tool\"\n \tthen\n \t\t# Use a special return code for this case since we want to\n \t\t# source \"defaults\" even when an explicit tool path is\n@@ -65,8 +57,11 @@ setup_tool () {\n \t\treturn 2\n \tfi\n \n+\t# Load the default functions\n+\t. \"$MERGE_TOOLS_DIR/include/defaults.sh\"\n+\n \t# Load the redefined functions\n-\t. \"$mergetools/$tool\"\n+\t. \"$MERGE_TOOLS_DIR/$tool\"\n \n \tif merge_mode && ! can_merge\n \tthen\n@@ -194,24 +189,10 @@ list_merge_tool_candidates () {\n show_tool_help () {\n \tunavailable= available= LF='\n '\n-\n-\tscriptlets=\"$(git --exec-path)\"/mergetools\n-\tfor i in \"$scriptlets\"/*\n+\tfor i in \"$MERGE_TOOLS_DIR\"/*\n \tdo\n-\t\t. \"$scriptlets\"/defaults\n-\t\t. \"$i\"\n-\n-\t\ttool=\"$(basename \"$i\")\"\n-\t\tif test \"$tool\" = \"defaults\"\n-\t\tthen\n-\t\t\tcontinue\n-\t\telif merge_mode && ! can_merge\n-\t\tthen\n-\t\t\tcontinue\n-\t\telif diff_mode && ! can_diff\n-\t\tthen\n-\t\t\tcontinue\n-\t\tfi\n+\t\ttool=$(basename \"$i\")\n+\t\tsetup_tool \"$tool\" 2>/dev/null || continue\n \n \t\tmerge_tool_path=$(translate_merge_tool_path \"$tool\")\n \t\tif type \"$merge_tool_path\" >/dev/null 2>&1\ndiff --git a/mergetools/gvimdiff b/mergetools/gvimdiff\nnew file mode 100644\nindex 0000000..04a5bb0\n--- /dev/null\n+++ b/mergetools/gvimdiff\n@@ -0,0 +1 @@\n+. \"$MERGE_TOOLS_DIR/vimdiff\"\ndiff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2\nnew file mode 100644\nindex 0000000..04a5bb0\n--- /dev/null\n+++ b/mergetools/gvimdiff2\n@@ -0,0 +1 @@\n+. \"$MERGE_TOOLS_DIR/vimdiff\"\ndiff --git a/mergetools/defaults b/mergetools/include/defaults.sh\nsimilarity index 100%\nrename from mergetools/defaults\nrename to mergetools/include/defaults.sh\ndiff --git a/mergetools/vim b/mergetools/vimdiff\nsimilarity index 100%\nrename from mergetools/vim\nrename to mergetools/vimdiff\ndiff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2\nnew file mode 100644\nindex 0000000..04a5bb0\n--- /dev/null\n+++ b/mergetools/vimdiff2\n@@ -0,0 +1 @@\n+. \"$MERGE_TOOLS_DIR/vimdiff\"\n-- \n1.8.0.8.g9bc9422\n"}]}