From 4a8273a3ed1d9db48920433471f87090423d1b5e Mon Sep 17 00:00:00 2001 From: John Keeping Date: Fri, 25 Jan 2013 01:43:48 -0800 Subject: [PATCH 1/8] git-mergetool: move show_tool_help to mergetool--lib This is the first step in unifying "git difftool --tool-help" and "git mergetool --tool-help". Signed-off-by: John Keeping Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 37 +++++++++++++++++++++++++++++++++++++ git-mergetool.sh | 37 ------------------------------------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index f013a0350..89a857f50 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -174,6 +174,43 @@ list_merge_tool_candidates () { esac } +show_tool_help () { + TOOL_MODE=merge + list_merge_tool_candidates + unavailable= available= LF=' +' + for i in $tools + do + merge_tool_path=$(translate_merge_tool_path "$i") + if type "$merge_tool_path" >/dev/null 2>&1 + then + available="$available$i$LF" + else + unavailable="$unavailable$i$LF" + fi + done + if test -n "$available" + then + echo "'git mergetool --tool=' may be set to one of the following:" + echo "$available" | sort | sed -e 's/^/ /' + else + echo "No suitable tool for 'git mergetool --tool=' found." + fi + if test -n "$unavailable" + then + echo + echo 'The following tools are valid, but not currently available:' + echo "$unavailable" | sort | sed -e 's/^/ /' + fi + if test -n "$unavailable$available" + then + echo + echo "Some of the tools listed above only work in a windowed" + echo "environment. If run in a terminal-only session, they will fail." + fi + exit 0 +} + guess_merge_tool () { list_merge_tool_candidates echo >&2 "merge tool candidates: $tools" diff --git a/git-mergetool.sh b/git-mergetool.sh index c50e18a89..c0ee9aaf8 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -315,43 +315,6 @@ merge_file () { return 0 } -show_tool_help () { - TOOL_MODE=merge - list_merge_tool_candidates - unavailable= available= LF=' -' - for i in $tools - do - merge_tool_path=$(translate_merge_tool_path "$i") - if type "$merge_tool_path" >/dev/null 2>&1 - then - available="$available$i$LF" - else - unavailable="$unavailable$i$LF" - fi - done - if test -n "$available" - then - echo "'git mergetool --tool=' may be set to one of the following:" - echo "$available" | sort | sed -e 's/^/ /' - else - echo "No suitable tool for 'git mergetool --tool=' found." - fi - if test -n "$unavailable" - then - echo - echo 'The following tools are valid, but not currently available:' - echo "$unavailable" | sort | sed -e 's/^/ /' - fi - if test -n "$unavailable$available" - then - echo - echo "Some of the tools listed above only work in a windowed" - echo "environment. If run in a terminal-only session, they will fail." - fi - exit 0 -} - prompt=$(git config --bool mergetool.prompt || echo true) while test $# != 0 From 26daa842dc35b2fe522ef18a1404c72421b3894c Mon Sep 17 00:00:00 2001 From: John Keeping Date: Fri, 25 Jan 2013 01:43:49 -0800 Subject: [PATCH 2/8] git-mergetool: remove redundant assignment TOOL_MODE is set at the top of git-mergetool.sh so there is no need to set it again in show_tool_help. Removing this lets us re-use show_tool_help in git-difftool. Signed-off-by: John Keeping Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 89a857f50..1748315bc 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -175,7 +175,6 @@ list_merge_tool_candidates () { } show_tool_help () { - TOOL_MODE=merge list_merge_tool_candidates unavailable= available= LF=' ' From 62b6f7e021cd5e7a7c3ee8d46ec45fe39d1bf562 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Fri, 25 Jan 2013 01:43:50 -0800 Subject: [PATCH 3/8] git-mergetool: don't hardcode 'mergetool' in show_tool_help When using show_tool_help from git-difftool we will want it to print "git difftool" not "git mergetool" so use "git ${TOOL_MODE}tool". Signed-off-by: John Keeping Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 1748315bc..4c1e1292a 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -188,12 +188,14 @@ show_tool_help () { unavailable="$unavailable$i$LF" fi done + + cmd_name=${TOOL_MODE}tool if test -n "$available" then - echo "'git mergetool --tool=' may be set to one of the following:" + echo "'git $cmd_name --tool=' may be set to one of the following:" echo "$available" | sort | sed -e 's/^/ /' else - echo "No suitable tool for 'git mergetool --tool=' found." + echo "No suitable tool for 'git $cmd_name --tool=' found." fi if test -n "$unavailable" then From abaf175cdf85ab0eb8bedd7d84ea8b42b6b3dd01 Mon Sep 17 00:00:00 2001 From: John Keeping Date: Fri, 25 Jan 2013 01:43:51 -0800 Subject: [PATCH 4/8] git-difftool: use git-mergetool--lib for "--tool-help" The "--tool-help" option to git-difftool currently displays incorrect output since it uses the names of the files in "$GIT_EXEC_PATH/mergetools/" rather than the list of command names in git-mergetool--lib. Fix this by simply delegating the "--tool-help" argument to the show_tool_help function in git-mergetool--lib. Signed-off-by: John Keeping Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-difftool.perl | 55 ++++++----------------------------------------- 1 file changed, 7 insertions(+), 48 deletions(-) diff --git a/git-difftool.perl b/git-difftool.perl index edd0493a0..0a90de414 100755 --- a/git-difftool.perl +++ b/git-difftool.perl @@ -59,57 +59,16 @@ sub find_worktree return $worktree; } -sub filter_tool_scripts -{ - my ($tools) = @_; - if (-d $_) { - if ($_ ne ".") { - # Ignore files in subdirectories - $File::Find::prune = 1; - } - } else { - if ((-f $_) && ($_ ne "defaults")) { - push(@$tools, $_); - } - } -} - sub print_tool_help { - my ($cmd, @found, @notfound, @tools); - my $gitpath = Git::exec_path(); - - find(sub { filter_tool_scripts(\@tools) }, "$gitpath/mergetools"); - - foreach my $tool (@tools) { - $cmd = "TOOL_MODE=diff"; - $cmd .= ' && . "$(git --exec-path)/git-mergetool--lib"'; - $cmd .= " && get_merge_tool_path $tool >/dev/null 2>&1"; - $cmd .= " && can_diff >/dev/null 2>&1"; - if (system('sh', '-c', $cmd) == 0) { - push(@found, $tool); - } else { - push(@notfound, $tool); - } - } - - print << 'EOF'; -'git difftool --tool=' may be set to one of the following: -EOF - print "\t$_\n" for (sort(@found)); + my $cmd = 'TOOL_MODE=diff'; + $cmd .= ' && . "$(git --exec-path)/git-mergetool--lib"'; + $cmd .= ' && show_tool_help'; - print << 'EOF'; - -The following tools are valid, but not currently available: -EOF - print "\t$_\n" for (sort(@notfound)); - - print << 'EOF'; - -NOTE: Some of the tools listed above only work in a windowed -environment. If run in a terminal-only session, they will fail. -EOF - exit(0); + # See the comment at the bottom of file_diff() for the reason behind + # using system() followed by exit() instead of exec(). + my $rc = system('sh', '-c', $cmd); + exit($rc | ($rc >> 8)); } sub exit_cleanup From b2a6b7122e66d4882ef5ac31e3a03969b5b6a199 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Fri, 25 Jan 2013 01:43:52 -0800 Subject: [PATCH 5/8] mergetools/vim: remove redundant diff command vimdiff and vimdiff2 differ only by their merge command so remove the logic in the diff command since it's not actually needed. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- mergetools/vim | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/mergetools/vim b/mergetools/vim index 619594ae4..39d032771 100644 --- a/mergetools/vim +++ b/mergetools/vim @@ -1,14 +1,6 @@ diff_cmd () { - case "$1" in - gvimdiff|vimdiff) - "$merge_tool_path" -R -f -d \ - -c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE" - ;; - gvimdiff2|vimdiff2) - "$merge_tool_path" -R -f -d \ - -c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE" - ;; - esac + "$merge_tool_path" -R -f -d \ + -c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE" } merge_cmd () { From 88d3406ad77dbab4d8ea76756822531228332b1b Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Fri, 25 Jan 2013 01:43:54 -0800 Subject: [PATCH 6/8] mergetool--lib: improve show_tool_help() output Check the can_diff and can_merge functions before deciding whether to add the tool to the available/unavailable lists. This makes "--tool-help" context-sensitive so that "git mergetool --tool-help" displays merge tools only and "git difftool --tool-help" displays diff tools only. Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 4c1e1292a..aa38bd18b 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -175,17 +175,33 @@ list_merge_tool_candidates () { } show_tool_help () { - list_merge_tool_candidates unavailable= available= LF=' ' - for i in $tools + + scriptlets="$(git --exec-path)"/mergetools + for i in "$scriptlets"/* do - merge_tool_path=$(translate_merge_tool_path "$i") + . "$scriptlets"/defaults + . "$i" + + tool="$(basename "$i")" + if test "$tool" = "defaults" + then + continue + elif merge_mode && ! can_merge + then + continue + elif diff_mode && ! can_diff + then + continue + fi + + merge_tool_path=$(translate_merge_tool_path "$tool") if type "$merge_tool_path" >/dev/null 2>&1 then - available="$available$i$LF" + available="$available$tool$LF" else - unavailable="$unavailable$i$LF" + unavailable="$unavailable$tool$LF" fi done From 62957bea0c7637ae1db4452fc165c59d9408585b Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sat, 26 Jan 2013 16:40:06 -0800 Subject: [PATCH 7/8] mergetool--lib: don't call "exit" in setup_tool This will make it easier to use setup_tool in places where we expect that the selected tool will not support the current mode. We need to introduce a new return code for setup_tool to differentiate between the case of "the selected tool is invalid" and "the selected tool is not a built-in" since we must call setup_tool when a custom 'merge..path' is configured for a built-in tool but avoid failing when the configured tool is not a built-in. Signed-off-by: John Keeping Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index aa38bd18b..f1bb372b6 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -58,7 +58,11 @@ setup_tool () { . "$mergetools/defaults" if ! test -f "$mergetools/$tool" then - return 1 + # Use a special return code for this case since we want to + # source "defaults" even when an explicit tool path is + # configured since the user can use that to override the + # default path in the scriptlet. + return 2 fi # Load the redefined functions @@ -67,11 +71,11 @@ setup_tool () { if merge_mode && ! can_merge then echo "error: '$tool' can not be used to resolve merges" >&2 - exit 1 + return 1 elif diff_mode && ! can_diff then echo "error: '$tool' can only be used to resolve merges" >&2 - exit 1 + return 1 fi return 0 } @@ -101,6 +105,19 @@ run_merge_tool () { # Bring tool-specific functions into scope setup_tool "$1" + exitcode=$? + case $exitcode in + 0) + : + ;; + 2) + # The configured tool is not a built-in tool. + test -n "$merge_tool_path" || return 1 + ;; + *) + return $exitcode + ;; + esac if merge_mode then From 073678b8e6324a155fa99f40eee0637941a70a34 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Sat, 26 Jan 2013 16:46:12 -0800 Subject: [PATCH 8/8] mergetools: simplify how we handle "vim" and "defaults" Remove the exceptions for "vim" and "defaults" in the mergetool library so that every filename in mergetools/ matches 1:1 with the name of a valid built-in tool. Define the trivial fallback definition of shell functions in-line in git-mergetool-lib script, instead of dot-sourcing them from another file. The result is much easier to follow. [jc: squashed in an update from John Keeping as well] Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- git-mergetool--lib.sh | 61 +++++++++++++++++++------------------ mergetools/defaults | 22 ------------- mergetools/gvimdiff | 1 + mergetools/gvimdiff2 | 1 + mergetools/{vim => vimdiff} | 0 mergetools/vimdiff2 | 1 + 6 files changed, 34 insertions(+), 52 deletions(-) delete mode 100644 mergetools/defaults create mode 100644 mergetools/gvimdiff create mode 100644 mergetools/gvimdiff2 rename mergetools/{vim => vimdiff} (100%) create mode 100644 mergetools/vimdiff2 diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index f1bb372b6..211ffe5d3 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -1,5 +1,7 @@ #!/bin/sh # git-mergetool--lib is a library for common merge tool functions +MERGE_TOOLS_DIR=$(git --exec-path)/mergetools + diff_mode() { test "$TOOL_MODE" = diff } @@ -44,19 +46,32 @@ valid_tool () { } setup_tool () { - case "$1" in - vim*|gvim*) - tool=vim - ;; - *) - tool="$1" - ;; - esac - mergetools="$(git --exec-path)/mergetools" + tool="$1" + + # Fallback definitions, to be overriden by tools. + can_merge () { + return 0 + } + + can_diff () { + return 0 + } + + diff_cmd () { + status=1 + return $status + } + + merge_cmd () { + status=1 + return $status + } - # Load the default definitions - . "$mergetools/defaults" - if ! test -f "$mergetools/$tool" + translate_merge_tool_path () { + echo "$1" + } + + if ! test -f "$MERGE_TOOLS_DIR/$tool" then # Use a special return code for this case since we want to # source "defaults" even when an explicit tool path is @@ -66,7 +81,7 @@ setup_tool () { fi # Load the redefined functions - . "$mergetools/$tool" + . "$MERGE_TOOLS_DIR/$tool" if merge_mode && ! can_merge then @@ -194,24 +209,10 @@ list_merge_tool_candidates () { show_tool_help () { unavailable= available= LF=' ' - - scriptlets="$(git --exec-path)"/mergetools - for i in "$scriptlets"/* + for i in "$MERGE_TOOLS_DIR"/* do - . "$scriptlets"/defaults - . "$i" - - tool="$(basename "$i")" - if test "$tool" = "defaults" - then - continue - elif merge_mode && ! can_merge - then - continue - elif diff_mode && ! can_diff - then - continue - fi + tool=$(basename "$i") + setup_tool "$tool" 2>/dev/null || continue merge_tool_path=$(translate_merge_tool_path "$tool") if type "$merge_tool_path" >/dev/null 2>&1 diff --git a/mergetools/defaults b/mergetools/defaults deleted file mode 100644 index 21e63ecc3..000000000 --- a/mergetools/defaults +++ /dev/null @@ -1,22 +0,0 @@ -# Redefined by builtin tools -can_merge () { - return 0 -} - -can_diff () { - return 0 -} - -diff_cmd () { - status=1 - return $status -} - -merge_cmd () { - status=1 - return $status -} - -translate_merge_tool_path () { - echo "$1" -} diff --git a/mergetools/gvimdiff b/mergetools/gvimdiff new file mode 100644 index 000000000..04a5bb0ea --- /dev/null +++ b/mergetools/gvimdiff @@ -0,0 +1 @@ +. "$MERGE_TOOLS_DIR/vimdiff" diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2 new file mode 100644 index 000000000..04a5bb0ea --- /dev/null +++ b/mergetools/gvimdiff2 @@ -0,0 +1 @@ +. "$MERGE_TOOLS_DIR/vimdiff" diff --git a/mergetools/vim b/mergetools/vimdiff similarity index 100% rename from mergetools/vim rename to mergetools/vimdiff diff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2 new file mode 100644 index 000000000..04a5bb0ea --- /dev/null +++ b/mergetools/vimdiff2 @@ -0,0 +1 @@ +. "$MERGE_TOOLS_DIR/vimdiff"