Merge branch 'jk/mergetool'

Cleans up mergetool/difftool combo.

* jk/mergetool:
  mergetools: simplify how we handle "vim" and "defaults"
  mergetool--lib: don't call "exit" in setup_tool
  mergetool--lib: improve show_tool_help() output
  mergetools/vim: remove redundant diff command
  git-difftool: use git-mergetool--lib for "--tool-help"
  git-mergetool: don't hardcode 'mergetool' in show_tool_help
  git-mergetool: remove redundant assignment
  git-mergetool: move show_tool_help to mergetool--lib

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 $cmd = 'TOOL_MODE=diff';
-	my $gitpath = Git::exec_path();
+	$cmd .= ' && . "$(git --exec-path)/git-mergetool--lib"';
+	$cmd .= ' && show_tool_help';
 
-	find(sub { filter_tool_scripts(\@tools) }, "$gitpath/mergetools");
+	# See the comment at the bottom of file_diff() for the reason behind
-
+	# using system() followed by exit() instead of exec().
-	foreach my $tool (@tools) {
+	my $rc = system('sh', '-c', $cmd);
-		$cmd  = "TOOL_MODE=diff";
+	exit($rc | ($rc >> 8));
-		$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=<tool>' may be set to one of the following:
-EOF
-	print "\t$_\n" for (sort(@found));
-
-	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);
 }
 
 sub exit_cleanup

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,34 +46,51 @@ valid_tool () {
 }
 
 setup_tool () {
-	case "$1" in
+	tool="$1"
-	vim*|gvim*)
-		tool=vim
-		;;
-	*)
-		tool="$1"
-		;;
-	esac
-	mergetools="$(git --exec-path)/mergetools"
 
-	# Load the default definitions
+	# Fallback definitions, to be overriden by tools.
-	. "$mergetools/defaults"
+	can_merge () {
-	if ! test -f "$mergetools/$tool"
+		return 0
+	}
+
+	can_diff () {
+		return 0
+	}
+
+	diff_cmd () {
+		status=1
+		return $status
+	}
+
+	merge_cmd () {
+		status=1
+		return $status
+	}
+
+	translate_merge_tool_path () {
+		echo "$1"
+	}
+
+	if ! test -f "$MERGE_TOOLS_DIR/$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
-	. "$mergetools/$tool"
+	. "$MERGE_TOOLS_DIR/$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 +120,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
@@ -174,6 +206,46 @@ list_merge_tool_candidates () {
 	esac
 }
 
+show_tool_help () {
+	unavailable= available= LF='
+'
+	for i in "$MERGE_TOOLS_DIR"/*
+	do
+		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
+		then
+			available="$available$tool$LF"
+		else
+			unavailable="$unavailable$tool$LF"
+		fi
+	done
+
+	cmd_name=${TOOL_MODE}tool
+	if test -n "$available"
+	then
+		echo "'git $cmd_name --tool=<tool>' may be set to one of the following:"
+		echo "$available" | sort | sed -e 's/^/	/'
+	else
+		echo "No suitable tool for 'git $cmd_name --tool=<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"

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=<tool>' may be set to one of the following:"
-		echo "$available" | sort | sed -e 's/^/	/'
-	else
-		echo "No suitable tool for 'git mergetool --tool=<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

mergetools/defaults

@@ -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"
-}

mergetools/gvimdiff

@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"

mergetools/gvimdiff2

@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"

mergetools/vimdiff

@@ -1,14 +1,6 @@
 diff_cmd () {
-	case "$1" in
+	"$merge_tool_path" -R -f -d \
-	gvimdiff|vimdiff)
+		-c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE"
-		"$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_cmd () {

mergetools/vimdiff2

@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"