Updates to the latest {diff,merge}tool series

Updates to the latest {diff,merge}tool series

[David Aguilar]

Here they are.

07/14: Updated usage synopsis
09/14: Updated diff.tool config.txt documentation
10/14: The original broke the test suite on Mac OS...
       Testing {diff,merge}tool.<tool>.path is tricky.
11/14: This depends on 07/14, so the rebase requires a resend
14/14: Reworked logic as discussed.

[PATCH v2 07/14] difftool: add a -y shortcut for --no-prompt

[David Aguilar]

This is another consistency cleanup to make git-difftool's options
match git-mergetool.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

Includes suggestions from Markus

 contrib/difftool/git-difftool     |    6 +++---
 contrib/difftool/git-difftool.txt |   36 ++++++++++++++----------------------
 2 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/contrib/difftool/git-difftool b/contrib/difftool/git-difftool
index 207dd50..8c160e5 100755
--- a/contrib/difftool/git-difftool
+++ b/contrib/difftool/git-difftool
@@ -18,7 +18,7 @@ my $DIR = abs_path(dirname($0));
 sub usage
 {
 	print << 'USAGE';
-usage: git difftool [--tool=<tool>] [--no-prompt] ["git diff" options]
+usage: git difftool [--tool=<tool>] [-y|--no-prompt] ["git diff" options]
 USAGE
 	exit 1;
 }
@@ -60,11 +60,11 @@ sub generate_command
 			$ENV{GIT_DIFF_TOOL} = substr($arg, 7);
 			next;
 		}
-		if ($arg eq '--no-prompt') {
+		if ($arg eq '-y' || $arg eq '--no-prompt') {
 			$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
 			next;
 		}
-		if ($arg eq '-h' or $arg eq '--help') {
+		if ($arg eq '-h' || $arg eq '--help') {
 			usage();
 		}
 		push @command, $arg;
diff --git a/contrib/difftool/git-difftool.txt b/contrib/difftool/git-difftool.txt
index 2b7bc03..a00e943 100644
--- a/contrib/difftool/git-difftool.txt
+++ b/contrib/difftool/git-difftool.txt
@@ -3,35 +3,32 @@ git-difftool(1)
 
 NAME
 ----
-git-difftool - compare changes using common merge tools
+git-difftool - Show changes using common diff tools
 
 SYNOPSIS
 --------
-'git difftool' [--tool=<tool>] [--no-prompt] ['git diff' options]
+'git difftool' [--tool=<tool>] [-y|--no-prompt] [<'git diff' options>]
 
 DESCRIPTION
 -----------
 'git-difftool' is a git command that allows you to compare and edit files
-between revisions using common merge tools.  At its most basic level,
-'git-difftool' does what 'git-mergetool' does but its use is for non-merge
-situations such as when preparing commits or comparing changes against
-the index.
-
-'git difftool' is a frontend to 'git diff' and accepts the same
-arguments and options.
-
-See linkgit:git-diff[1] for the full list of supported options.
+between revisions using common diff tools.  'git difftool' is a frontend
+to 'git-diff' and accepts the same options and arguments.
 
 OPTIONS
 -------
+-y::
+--no-prompt::
+	Do not prompt before launching a diff tool.
+
 -t <tool>::
 --tool=<tool>::
-	Use the merge resolution program specified by <tool>.
+	Use the diff tool specified by <tool>.
 	Valid merge tools are:
 	kdiff3, kompare, tkdiff, meld, xxdiff, emerge,
 	vimdiff, gvimdiff, ecmerge, and opendiff
 +
-If a merge resolution program is not specified, 'git-difftool'
+If a diff tool is not specified, 'git-difftool'
 will use the configuration variable `diff.tool`.  If the
 configuration variable `diff.tool` is not set, 'git-difftool'
 will pick a suitable default.
@@ -42,7 +39,7 @@ can configure the absolute path to kdiff3 by setting
 `difftool.kdiff3.path`. Otherwise, 'git-difftool' assumes the
 tool is available in PATH.
 +
-Instead of running one of the known merge tool programs,
+Instead of running one of the known diff tools,
 'git-difftool' can be customized to run an alternative program
 by specifying the command line to invoke in a configuration
 variable `difftool.<tool>.cmd`.
@@ -56,8 +53,7 @@ is set to the name of the temporary file containing the contents
 of the diff post-image.  `$BASE` is provided for compatibility
 with custom merge tool commands and has the same value as `$LOCAL`.
 
---no-prompt::
-	Do not prompt before launching a diff tool.
+See linkgit:git-diff[1] for the full list of supported options.
 
 CONFIG VARIABLES
 ----------------
@@ -65,21 +61,17 @@ CONFIG VARIABLES
 difftool equivalents have not been defined.
 
 diff.tool::
-	The default merge tool to use.
+	The default diff tool to use.
 
 difftool.<tool>.path::
 	Override the path for the given tool.  This is useful in case
 	your tool is not in the PATH.
 
 difftool.<tool>.cmd::
-	Specify the command to invoke the specified merge tool.
+	Specify the command to invoke the specified diff tool.
 +
 See the `--tool=<tool>` option above for more details.
 
-merge.keepBackup::
-	The original, unedited file content can be saved to a file with
-	a `.orig` extension.  Defaults to `true` (i.e. keep the backup files).
-
 SEE ALSO
 --------
 linkgit:git-diff[1]::
-- 
1.6.2.2.414.g81aa9

[PATCH v2 09/14] difftool: move 'git-difftool' out of contrib

[David Aguilar]

This prepares 'git-difftool' and its documentation for
mainstream use.

'git-difftool-helper' became 'git-difftool--helper'
since users should not use it directly.

'git-difftool' was added to the list of commands as
an ancillaryinterrogator.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

Includes suggestions from Markus

 .gitignore                                         |    2 ++
 Documentation/config.txt                           |   18 ++++++++++++++++++
 .../difftool => Documentation}/git-difftool.txt    |    0
 Makefile                                           |    2 ++
 command-list.txt                                   |    1 +
 .../git-difftool-helper => git-difftool--helper.sh |    2 +-
 contrib/difftool/git-difftool => git-difftool.perl |    6 +++---
 7 files changed, 27 insertions(+), 4 deletions(-)
 rename {contrib/difftool => Documentation}/git-difftool.txt (100%)
 rename contrib/difftool/git-difftool-helper => git-difftool--helper.sh (98%)
 rename contrib/difftool/git-difftool => git-difftool.perl (92%)

diff --git a/.gitignore b/.gitignore
index 1c57d4c..a36da9d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -35,6 +35,8 @@ git-diff
 git-diff-files
 git-diff-index
 git-diff-tree
+git-difftool
+git-difftool--helper
 git-describe
 git-fast-export
 git-fast-import
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3afd124..94ef1a6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -667,6 +667,24 @@ diff.suppressBlankEmpty::
 	A boolean to inhibit the standard behavior of printing a space
 	before each empty output line. Defaults to false.
 
+diff.tool::
+	Controls which diff tool is used.  `diff.tool` overrides
+	`merge.tool` when used by linkgit:git-difftool[1] and has
+	the same valid values as `merge.tool` minus "tortoisemerge"
+	and plus "kompare".
+
+difftool.<tool>.path::
+	Override the path for the given tool.  This is useful in case
+	your tool is not in the PATH.
+
+difftool.<tool>.cmd::
+	Specify the command to invoke the specified diff tool.
+	The specified command is evaluated in shell with the following
+	variables available:  'LOCAL' is set to the name of the temporary
+	file containing the contents of the diff pre-image and 'REMOTE'
+	is set to the name of the temporary file containing the contents
+	of the diff post-image.
+
 diff.wordRegex::
 	A POSIX Extended Regular Expression used to determine what is a "word"
 	when performing word-by-word difference calculations.  Character
diff --git a/contrib/difftool/git-difftool.txt b/Documentation/git-difftool.txt
similarity index 100%
rename from contrib/difftool/git-difftool.txt
rename to Documentation/git-difftool.txt
diff --git a/Makefile b/Makefile
index 7867eac..a80055f 100644
--- a/Makefile
+++ b/Makefile
@@ -277,6 +277,7 @@ TEST_PROGRAMS =
 
 SCRIPT_SH += git-am.sh
 SCRIPT_SH += git-bisect.sh
+SCRIPT_SH += git-difftool--helper.sh
 SCRIPT_SH += git-filter-branch.sh
 SCRIPT_SH += git-lost-found.sh
 SCRIPT_SH += git-merge-octopus.sh
@@ -296,6 +297,7 @@ SCRIPT_SH += git-submodule.sh
 SCRIPT_SH += git-web--browse.sh
 
 SCRIPT_PERL += git-add--interactive.perl
+SCRIPT_PERL += git-difftool.perl
 SCRIPT_PERL += git-archimport.perl
 SCRIPT_PERL += git-cvsexportcommit.perl
 SCRIPT_PERL += git-cvsimport.perl
diff --git a/command-list.txt b/command-list.txt
index 3583a33..fb03a2e 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -33,6 +33,7 @@ git-diff                                mainporcelain common
 git-diff-files                          plumbinginterrogators
 git-diff-index                          plumbinginterrogators
 git-diff-tree                           plumbinginterrogators
+git-difftool                            ancillaryinterrogators
 git-fast-export				ancillarymanipulators
 git-fast-import				ancillarymanipulators
 git-fetch                               mainporcelain common
diff --git a/contrib/difftool/git-difftool-helper b/git-difftool--helper.sh
similarity index 98%
rename from contrib/difftool/git-difftool-helper
rename to git-difftool--helper.sh
index 4b0daec..fc61416 100755
--- a/contrib/difftool/git-difftool-helper
+++ b/git-difftool--helper.sh
@@ -1,5 +1,5 @@
 #!/bin/sh
-# git-difftool-helper is a GIT_EXTERNAL_DIFF-compatible diff tool launcher.
+# git-difftool--helper is a GIT_EXTERNAL_DIFF-compatible diff tool launcher.
 # This script is typically launched by using the 'git difftool'
 # convenience command.
 #
diff --git a/contrib/difftool/git-difftool b/git-difftool.perl
similarity index 92%
rename from contrib/difftool/git-difftool
rename to git-difftool.perl
index 8c160e5..8857ac8 100755
--- a/contrib/difftool/git-difftool
+++ b/git-difftool.perl
@@ -2,9 +2,9 @@
 # Copyright (c) 2009 David Aguilar
 #
 # This is a wrapper around the GIT_EXTERNAL_DIFF-compatible
-# git-difftool-helper script.  This script exports
+# git-difftool--helper script.  This script exports
 # GIT_EXTERNAL_DIFF and GIT_PAGER for use by git, and
-# GIT_DIFFTOOL_NO_PROMPT and GIT_DIFF_TOOL for use by git-difftool-helper.
+# GIT_DIFFTOOL_NO_PROMPT and GIT_DIFF_TOOL for use by git-difftool--helper.
 # Any arguments that are unknown to this script are forwarded to 'git diff'.
 
 use strict;
@@ -27,7 +27,7 @@ sub setup_environment
 {
 	$ENV{PATH} = "$DIR:$ENV{PATH}";
 	$ENV{GIT_PAGER} = '';
-	$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool-helper';
+	$ENV{GIT_EXTERNAL_DIFF} = 'git-difftool--helper';
 }
 
 sub exe
-- 
1.6.2.2.414.g81aa9

[PATCH v2 10/14] difftool: add various git-difftool tests

[David Aguilar]

t7800-difftool.sh tests the various command-line flags,
git-config variables, and environment settings supported by
git-difftool.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

The original broke the test suite on Mac OS.
Testing difftool.<path>.path is hard without internal support
for a fake tool.

 t/t7800-difftool.sh |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 139 insertions(+), 0 deletions(-)
 create mode 100755 t/t7800-difftool.sh

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
new file mode 100755
index 0000000..ceef84b
--- /dev/null
+++ b/t/t7800-difftool.sh
@@ -0,0 +1,139 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 David Aguilar
+#
+
+test_description='git-difftool
+
+Testing basic diff tool invocation
+'
+
+. ./test-lib.sh
+
+remove_config_vars()
+{
+	# Unset all config variables used by git-difftool
+	git config --unset diff.tool
+	git config --unset difftool.test-tool.cmd
+	git config --unset merge.tool
+	git config --unset mergetool.test-tool.cmd
+	return 0
+}
+
+restore_test_defaults()
+{
+	# Restores the test defaults used by several tests
+	remove_config_vars
+	unset GIT_DIFF_TOOL
+	unset GIT_MERGE_TOOL
+	unset GIT_DIFFTOOL_NO_PROMPT
+	git config diff.tool test-tool &&
+	git config difftool.test-tool.cmd 'cat $LOCAL'
+}
+
+# Create a file on master and change it on branch
+test_expect_success 'setup' '
+	echo master >file &&
+	git add file &&
+	git commit -m "added file" &&
+
+	git checkout -b branch master &&
+	echo branch >file &&
+	git commit -a -m "branch changed file" &&
+	git checkout master
+'
+
+# Configure a custom difftool.<tool>.cmd and use it
+test_expect_success 'custom commands' '
+	restore_test_defaults &&
+	git config difftool.test-tool.cmd "cat \$REMOTE" &&
+
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "master" &&
+
+	restore_test_defaults &&
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch"
+'
+
+# Ensures that git-difftool ignores bogus --tool values
+test_expect_success 'difftool ignores bad --tool values' '
+	diff=$(git difftool --no-prompt --tool=bogus-tool branch)
+	test "$?" = 1 &&
+	test "$diff" = ""
+'
+
+# Specify the diff tool using $GIT_DIFF_TOOL
+test_expect_success 'GIT_DIFF_TOOL variable' '
+	git config --unset diff.tool
+	GIT_DIFF_TOOL=test-tool &&
+	export GIT_DIFF_TOOL &&
+
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
+# Test the $GIT_*_TOOL variables and ensure
+# that $GIT_DIFF_TOOL always wins unless --tool is specified
+test_expect_success 'GIT_DIFF_TOOL overrides' '
+	git config diff.tool bogus-tool &&
+	git config merge.tool bogus-tool &&
+
+	GIT_MERGE_TOOL=test-tool &&
+	export GIT_MERGE_TOOL &&
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch" &&
+	unset GIT_MERGE_TOOL &&
+
+	GIT_MERGE_TOOL=bogus-tool &&
+	GIT_DIFF_TOOL=test-tool &&
+	export GIT_MERGE_TOOL &&
+	export GIT_DIFF_TOOL &&
+
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch" &&
+
+	GIT_DIFF_TOOL=bogus-tool &&
+	export GIT_DIFF_TOOL &&
+
+	diff=$(git difftool --no-prompt --tool=test-tool branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
+# Test that we don't have to pass --no-prompt to difftool
+# when $GIT_DIFFTOOL_NO_PROMPT is true
+test_expect_success 'GIT_DIFFTOOL_NO_PROMPT variable' '
+	GIT_DIFFTOOL_NO_PROMPT=true &&
+	export GIT_DIFFTOOL_NO_PROMPT &&
+
+	diff=$(git difftool branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
+# git-difftool falls back to git-mergetool config variables
+# so test that behavior here
+test_expect_success 'difftool + mergetool config variables' '
+	remove_config_vars
+	git config merge.tool test-tool &&
+	git config mergetool.test-tool.cmd "cat \$LOCAL" &&
+
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch" &&
+
+	# set merge.tool to something bogus, diff.tool to test-tool
+	git config merge.tool bogus-tool &&
+	git config diff.tool test-tool &&
+
+	diff=$(git difftool --no-prompt branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
+test_done
-- 
1.6.2.2.414.g81aa9

[PATCH v2 11/14] difftool: add support for a difftool.prompt config variable

[David Aguilar]

difftool now supports difftool.prompt so that users do not have to
pass --no-prompt or hit enter each time a diff tool is launched.
The --prompt flag overrides the configuration variable.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

This one had to be resent since rewriting #9 would
have caused a conflict in here.

 Documentation/config.txt       |    3 ++
 Documentation/git-difftool.txt |   10 +++++-
 git-difftool--helper.sh        |   10 +++++-
 git-difftool.perl              |   15 +++++++--
 t/t7800-difftool.sh            |   64 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 96 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index e8ff0be..6e29623 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -684,6 +684,9 @@ difftool.<tool>.cmd::
 	is set to the name of the temporary file containing the contents
 	of the diff post-image.
 
+difftool.prompt::
+	Prompt before each invocation of the diff tool.
+
 diff.wordRegex::
 	A POSIX Extended Regular Expression used to determine what is a "word"
 	when performing word-by-word difference calculations.  Character
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index af68466..15b247b 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -7,7 +7,7 @@ git-difftool - Show changes using common diff tools
 
 SYNOPSIS
 --------
-'git difftool' [--tool=<tool>] [-y|--no-prompt] [<'git diff' options>]
+'git difftool' [--tool=<tool>] [-y|--no-prompt|--prompt] [<'git diff' options>]
 
 DESCRIPTION
 -----------
@@ -21,6 +21,11 @@ OPTIONS
 --no-prompt::
 	Do not prompt before launching a diff tool.
 
+--prompt::
+	Prompt before each invocation of the diff tool.
+	This is the default behaviour; the option is provided to
+	override any configuration settings.
+
 -t <tool>::
 --tool=<tool>::
 	Use the diff tool specified by <tool>.
@@ -72,6 +77,9 @@ difftool.<tool>.cmd::
 +
 See the `--tool=<tool>` option above for more details.
 
+difftool.prompt::
+	Prompt before each invocation of the diff tool.
+
 SEE ALSO
 --------
 linkgit:git-diff[1]::
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index fc61416..f3c27d8 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -5,9 +5,15 @@
 #
 # Copyright (c) 2009 David Aguilar
 
-# Set GIT_DIFFTOOL_NO_PROMPT to bypass the per-file prompt.
+# difftool.prompt controls the default prompt/no-prompt behavior
+# and is overridden with $GIT_DIFFTOOL*_PROMPT.
 should_prompt () {
-	test -z "$GIT_DIFFTOOL_NO_PROMPT"
+	prompt=$(git config --bool difftool.prompt || echo true)
+	if test "$prompt" = true; then
+		test -z "$GIT_DIFFTOOL_NO_PROMPT"
+	else
+		test -n "$GIT_DIFFTOOL_PROMPT"
+	fi
 }
 
 # This function prepares temporary files and launches the appropriate
diff --git a/git-difftool.perl b/git-difftool.perl
index 8857ac8..948ff7f 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -2,9 +2,12 @@
 # Copyright (c) 2009 David Aguilar
 #
 # This is a wrapper around the GIT_EXTERNAL_DIFF-compatible
-# git-difftool--helper script.  This script exports
-# GIT_EXTERNAL_DIFF and GIT_PAGER for use by git, and
-# GIT_DIFFTOOL_NO_PROMPT and GIT_DIFF_TOOL for use by git-difftool--helper.
+# git-difftool--helper script.
+#
+# This script exports GIT_EXTERNAL_DIFF and GIT_PAGER for use by git.
+# GIT_DIFFTOOL_NO_PROMPT, GIT_DIFFTOOL_PROMPT, and GIT_DIFF_TOOL
+# are exported for use by git-difftool--helper.
+#
 # Any arguments that are unknown to this script are forwarded to 'git diff'.
 
 use strict;
@@ -62,6 +65,12 @@ sub generate_command
 		}
 		if ($arg eq '-y' || $arg eq '--no-prompt') {
 			$ENV{GIT_DIFFTOOL_NO_PROMPT} = 'true';
+			delete $ENV{GIT_DIFFTOOL_PROMPT};
+			next;
+		}
+		if ($arg eq '--prompt') {
+			$ENV{GIT_DIFFTOOL_PROMPT} = 'true';
+			delete $ENV{GIT_DIFFTOOL_NO_PROMPT};
 			next;
 		}
 		if ($arg eq '-h' || $arg eq '--help') {
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 85946d9..0b263ea 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -15,6 +15,7 @@ remove_config_vars()
 	# Unset all config variables used by git-difftool
 	git config --unset diff.tool
 	git config --unset difftool.test-tool.cmd
+	git config --unset difftool.prompt
 	git config --unset merge.tool
 	git config --unset mergetool.test-tool.cmd
 	return 0
@@ -26,11 +27,18 @@ restore_test_defaults()
 	remove_config_vars
 	unset GIT_DIFF_TOOL
 	unset GIT_MERGE_TOOL
+	unset GIT_DIFFTOOL_PROMPT
 	unset GIT_DIFFTOOL_NO_PROMPT
 	git config diff.tool test-tool &&
 	git config difftool.test-tool.cmd 'cat $LOCAL'
 }
 
+prompt_given()
+{
+	prompt="$1"
+	test "$prompt" = "Hit return to launch 'test-tool': branch"
+}
+
 # Create a file on master and change it on branch
 test_expect_success 'setup' '
 	echo master >file &&
@@ -116,6 +124,62 @@ test_expect_success 'GIT_DIFFTOOL_NO_PROMPT variable' '
 	restore_test_defaults
 '
 
+# git-difftool supports the difftool.prompt variable.
+# Test that GIT_DIFFTOOL_PROMPT can override difftool.prompt = false
+test_expect_success 'GIT_DIFFTOOL_PROMPT variable' '
+	git config difftool.prompt false &&
+	GIT_DIFFTOOL_PROMPT=true &&
+	export GIT_DIFFTOOL_PROMPT &&
+
+	prompt=$(echo | git difftool --prompt branch | tail -1) &&
+	prompt_given "$prompt" &&
+
+	restore_test_defaults
+'
+
+# Test that we don't have to pass --no-prompt when difftool.prompt is false
+test_expect_success 'difftool.prompt config variable is false' '
+	git config difftool.prompt false &&
+
+	diff=$(git difftool branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
+# Test that the -y flag can override difftool.prompt = true
+test_expect_success 'difftool.prompt can overridden with -y' '
+	git config difftool.prompt true &&
+
+	diff=$(git difftool -y branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults
+'
+
+# Test that the --prompt flag can override difftool.prompt = false
+test_expect_success 'difftool.prompt can overridden with --prompt' '
+	git config difftool.prompt false &&
+
+	prompt=$(echo | git difftool --prompt branch | tail -1) &&
+	prompt_given "$prompt" &&
+
+	restore_test_defaults
+'
+
+# Test that the last flag passed on the command-line wins
+test_expect_success 'difftool last flag wins' '
+	diff=$(git difftool --prompt --no-prompt branch) &&
+	test "$diff" = "branch" &&
+
+	restore_test_defaults &&
+
+	prompt=$(echo | git difftool --no-prompt --prompt branch | tail -1) &&
+	prompt_given "$prompt" &&
+
+	restore_test_defaults
+'
+
 # git-difftool falls back to git-mergetool config variables
 # so test that behavior here
 test_expect_success 'difftool + mergetool config variables' '
-- 
1.6.2.2.414.g81aa9

[PATCH v3 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib

[David Aguilar]

This consolidates the common functionality from git-mergetool and
git-difftool--helper into a single git-mergetool--lib scriptlet.

Signed-off-by: David Aguilar <davvid@gmail.com>
---

Includes suggestions from Markus.
We also moved opendiff up to the front so that it's
preferred when on Mac OS.

 .gitignore                           |    1 +
 Documentation/git-mergetool--lib.txt |   56 +++++
 Makefile                             |    1 +
 git-difftool--helper.sh              |  186 +----------------
 git-mergetool--lib.sh                |  387 ++++++++++++++++++++++++++++++++++
 git-mergetool.sh                     |  224 ++------------------
 6 files changed, 467 insertions(+), 388 deletions(-)
 create mode 100644 Documentation/git-mergetool--lib.txt
 create mode 100644 git-mergetool--lib.sh

diff --git a/.gitignore b/.gitignore
index a36da9d..757c7f0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -80,6 +80,7 @@ git-merge-recursive
 git-merge-resolve
 git-merge-subtree
 git-mergetool
+git-mergetool--lib
 git-mktag
 git-mktree
 git-name-rev
diff --git a/Documentation/git-mergetool--lib.txt b/Documentation/git-mergetool--lib.txt
new file mode 100644
index 0000000..3d57031
--- /dev/null
+++ b/Documentation/git-mergetool--lib.txt
@@ -0,0 +1,56 @@
+git-mergetool--lib(1)
+=====================
+
+NAME
+----
+git-mergetool--lib - Common git merge tool shell scriptlets
+
+SYNOPSIS
+--------
+'. "$(git --exec-path)/git-mergetool--lib"'
+
+DESCRIPTION
+-----------
+
+This is not a command the end user would want to run.  Ever.
+This documentation is meant for people who are studying the
+Porcelain-ish scripts and/or are writing new ones.
+
+The 'git-mergetool--lib' scriptlet is designed to be sourced (using
+`.`) by other shell scripts to set up functions for working
+with git merge tools.
+
+Before sourcing it, your script should set up a few variables;
+`TOOL_MODE` is used to define the operation mode for various
+functions.  'diff' and 'merge' are valid values.
+
+FUNCTIONS
+---------
+get_merge_tool::
+	returns a merge tool
+
+get_merge_tool_cmd::
+	returns the custom command for a merge tool.
+
+get_merge_tool_path::
+	returns the custom path for a merge tool.
+
+run_merge_tool::
+	launches a merge tool given the tool name and a true/false
+	flag to indicate whether a merge base is present.
+	'$merge_tool', '$merge_tool_path', and for custom commands,
+	'$merge_tool_cmd', must be defined prior to calling
+	run_merge_tool.  Additionally, '$MERGED', '$LOCAL', '$REMOTE',
+	and '$BASE' must be defined for use by the merge tool.
+
+Author
+------
+Written by David Aguilar <davvid@gmail.com>
+
+Documentation
+--------------
+Documentation by David Aguilar and the git-list <git@vger.kernel.org>.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index a80055f..3e56274 100644
--- a/Makefile
+++ b/Makefile
@@ -284,6 +284,7 @@ SCRIPT_SH += git-merge-octopus.sh
 SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
+SCRIPT_SH += git-mergetool--lib.sh
 SCRIPT_SH += git-parse-remote.sh
 SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
diff --git a/git-difftool--helper.sh b/git-difftool--helper.sh
index f3c27d8..b450036 100755
--- a/git-difftool--helper.sh
+++ b/git-difftool--helper.sh
@@ -5,6 +5,10 @@
 #
 # Copyright (c) 2009 David Aguilar
 
+# Load common functions from git-mergetool--lib
+TOOL_MODE=diff
+. git-mergetool--lib
+
 # difftool.prompt controls the default prompt/no-prompt behavior
 # and is overridden with $GIT_DIFFTOOL*_PROMPT.
 should_prompt () {
@@ -16,8 +20,7 @@ should_prompt () {
 	fi
 }
 
-# This function prepares temporary files and launches the appropriate
-# merge tool.
+# Sets up shell variables and runs a merge tool
 launch_merge_tool () {
 	# Merged is the filename as it appears in the work tree
 	# Local is the contents of a/filename
@@ -37,187 +40,16 @@ launch_merge_tool () {
 	fi
 
 	# Run the appropriate merge tool command
-	case "$merge_tool" in
-	kdiff3)
-		basename=$(basename "$MERGED")
-		"$merge_tool_path" --auto \
-			--L1 "$basename (A)" \
-			--L2 "$basename (B)" \
-			"$LOCAL" "$REMOTE" \
-			> /dev/null 2>&1
-		;;
-
-	kompare)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	tkdiff)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	meld)
-		"$merge_tool_path" "$LOCAL" "$REMOTE"
-		;;
-
-	diffuse)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
-		;;
-
-	vimdiff)
-		"$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$REMOTE"
-		;;
-
-	gvimdiff)
-		"$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$REMOTE"
-		;;
-
-	xxdiff)
-		"$merge_tool_path" \
-			-R 'Accel.Search: "Ctrl+F"' \
-			-R 'Accel.SearchForward: "Ctrl-G"' \
-			"$LOCAL" "$REMOTE"
-		;;
-
-	opendiff)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
-		;;
-
-	ecmerge)
-		"$merge_tool_path" "$LOCAL" "$REMOTE" \
-			--default --mode=merge2 --to="$MERGED"
-		;;
-
-	emerge)
-		"$merge_tool_path" -f emerge-files-command \
-			"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
-		;;
-
-	*)
-		if test -n "$merge_tool_cmd"; then
-			( eval $merge_tool_cmd )
-		fi
-		;;
-	esac
-}
-
-# Verifies that (difftool|mergetool).<tool>.cmd exists
-valid_custom_tool() {
-	merge_tool_cmd="$(git config difftool.$1.cmd)"
-	test -z "$merge_tool_cmd" &&
-	merge_tool_cmd="$(git config mergetool.$1.cmd)"
-	test -n "$merge_tool_cmd"
-}
-
-# Verifies that the chosen merge tool is properly setup.
-# Built-in merge tools are always valid.
-valid_tool() {
-	case "$1" in
-	kdiff3 | kompare | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge)
-		;; # happy
-	*)
-		if ! valid_custom_tool "$1"
-		then
-			return 1
-		fi
-		;;
-	esac
-}
-
-# Sets up the merge_tool_path variable.
-# This handles the difftool.<tool>.path configuration.
-# This also falls back to mergetool defaults.
-init_merge_tool_path() {
-	merge_tool_path=$(git config difftool."$1".path)
-	test -z "$merge_tool_path" &&
-	merge_tool_path=$(git config mergetool."$1".path)
-	if test -z "$merge_tool_path"; then
-		case "$1" in
-		vimdiff)
-			merge_tool_path=vim
-			;;
-		gvimdiff)
-			merge_tool_path=gvim
-			;;
-		emerge)
-			merge_tool_path=emacs
-			;;
-		*)
-			merge_tool_path="$1"
-			;;
-		esac
-	fi
+	run_merge_tool "$merge_tool"
 }
 
 # Allow GIT_DIFF_TOOL and GIT_MERGE_TOOL to provide default values
 test -n "$GIT_MERGE_TOOL" && merge_tool="$GIT_MERGE_TOOL"
 test -n "$GIT_DIFF_TOOL" && merge_tool="$GIT_DIFF_TOOL"
 
-# If merge tool was not specified then use the diff.tool
-# configuration variable.  If that's invalid then reset merge_tool.
-# Fallback to merge.tool.
-if test -z "$merge_tool"; then
-	merge_tool=$(git config diff.tool)
-	test -z "$merge_tool" &&
-	merge_tool=$(git config merge.tool)
-	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
-		echo >&2 "git config option diff.tool set to unknown tool: $merge_tool"
-		echo >&2 "Resetting to default..."
-		unset merge_tool
-	fi
-fi
-
-# Try to guess an appropriate merge tool if no tool has been set.
-if test -z "$merge_tool"; then
-	# We have a $DISPLAY so try some common UNIX merge tools
-	if test -n "$DISPLAY"; then
-		# If gnome then prefer meld, otherwise, prefer kdiff3 or kompare
-		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-			merge_tool_candidates="meld kdiff3 kompare tkdiff xxdiff gvimdiff diffuse"
-		else
-			merge_tool_candidates="kdiff3 kompare tkdiff xxdiff meld gvimdiff diffuse"
-		fi
-	fi
-	if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
-		# $EDITOR is emacs so add emerge as a candidate
-		merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
-	elif echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
-		# $EDITOR is vim so add vimdiff as a candidate
-		merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
-	else
-		merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
-	fi
-	echo "merge tool candidates: $merge_tool_candidates"
-
-	# Loop over each candidate and stop when a valid merge tool is found.
-	for i in $merge_tool_candidates
-	do
-		init_merge_tool_path $i
-		if type "$merge_tool_path" > /dev/null 2>&1; then
-			merge_tool=$i
-			break
-		fi
-	done
-
-	if test -z "$merge_tool" ; then
-		echo "No known merge resolution program available."
-		exit 1
-	fi
-
-else
-	# A merge tool has been set, so verify that it's valid.
-	if ! valid_tool "$merge_tool"; then
-		echo >&2 "Unknown merge tool $merge_tool"
-		exit 1
-	fi
-
-	init_merge_tool_path "$merge_tool"
-
-	if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-		echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
-		exit 1
-	fi
-fi
-
+merge_tool=$(get_merge_tool "$merge_tool") || exit
+merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
+merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
 
 # Launch the merge tool on each path provided by 'git diff'
 while test $# -gt 6
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
new file mode 100644
index 0000000..a2f45ee
--- /dev/null
+++ b/git-mergetool--lib.sh
@@ -0,0 +1,387 @@
+# git-mergetool--lib is a library for common merge tool functions
+diff_mode() {
+	test $TOOL_MODE = "diff"
+}
+
+merge_mode() {
+	test $TOOL_MODE = "merge"
+}
+
+translate_merge_tool_path () {
+	if test -n "$2"; then
+		echo "$2"
+	else
+		case "$1" in
+		vimdiff)
+			path=vim
+			;;
+		gvimdiff)
+			path=gvim
+			;;
+		emerge)
+			path=emacs
+			;;
+		*)
+			path="$1"
+			;;
+		esac
+		echo "$path"
+	fi
+}
+
+check_unchanged () {
+	if merge_mode; then
+		if test "$MERGED" -nt "$BACKUP"; then
+			status=0
+		else
+			while true; do
+				echo "$MERGED seems unchanged."
+				printf "Was the merge successful? [y/n] "
+				read answer < /dev/tty
+				case "$answer" in
+				y*|Y*) status=0; break ;;
+				n*|N*) status=1; break ;;
+				esac
+			done
+		fi
+	else
+		status=0
+	fi
+}
+
+valid_tool () {
+	case "$1" in
+	kdiff3 | tkdiff | xxdiff | meld | opendiff | \
+	emerge | vimdiff | gvimdiff | ecmerge | diffuse)
+		;; # happy
+	tortoisemerge)
+		if ! merge_mode; then
+			return 1
+		fi
+		;;
+	kompare)
+		if ! diff_mode; then
+			return 1
+		fi
+		;;
+	*)
+		if test -z "$(get_merge_tool_cmd "$1")"; then
+			return 1
+		fi
+		;;
+	esac
+}
+
+get_merge_tool_cmd () {
+	diff_mode &&
+	custom_cmd="$(git config difftool.$1.cmd)"
+	test -z "$custom_cmd" &&
+	custom_cmd="$(git config mergetool.$1.cmd)"
+	test -n "$custom_cmd" &&
+	echo "$custom_cmd"
+}
+
+run_merge_tool () {
+	base_present="$2"
+	if diff_mode; then
+		status=0
+		base_present="false"
+	fi
+
+	case "$1" in
+	kdiff3)
+		if merge_mode; then
+			if $base_present; then
+				("$merge_tool_path" --auto \
+					--L1 "$MERGED (Base)" \
+					--L2 "$MERGED (Local)" \
+					--L3 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$BASE" "$LOCAL" "$REMOTE" \
+				> /dev/null 2>&1)
+			else
+				("$merge_tool_path" --auto \
+					--L1 "$MERGED (Local)" \
+					--L2 "$MERGED (Remote)" \
+					-o "$MERGED" \
+					"$LOCAL" "$REMOTE" \
+				> /dev/null 2>&1)
+			fi
+			status=$?
+		else
+			("$merge_tool_path" --auto \
+			 --L1 "$MERGED (A)" \
+			 --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
+			 > /dev/null 2>&1)
+		fi
+		;;
+	kompare)
+		"$merge_tool_path" "$LOCAL" "$REMOTE"
+		;;
+	tkdiff)
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" -a "$BASE" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
+			else
+				"$merge_tool_path" \
+					-o "$MERGED" "$LOCAL" "$REMOTE"
+			fi
+			status=$?
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE"
+		fi
+		;;
+	meld)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE"
+		fi
+		;;
+	diffuse)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" \
+					"$BASE" | cat
+			else
+				"$merge_tool_path" \
+					"$LOCAL" "$MERGED" "$REMOTE" | cat
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+		fi
+		;;
+	vimdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" -d -c "wincmd l" \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	gvimdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$MERGED" "$REMOTE"
+			check_unchanged
+		else
+			"$merge_tool_path" -d -c "wincmd l" -f \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	xxdiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" -X --show-merged-pane \
+					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+					-R 'Accel.Search: "Ctrl+F"' \
+					-R 'Accel.SearchForward: "Ctrl-G"' \
+					--merged-file "$MERGED" \
+					"$LOCAL" "$BASE" "$REMOTE"
+			else
+				"$merge_tool_path" -X $extra \
+					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
+					-R 'Accel.Search: "Ctrl+F"' \
+					-R 'Accel.SearchForward: "Ctrl-G"' \
+					--merged-file "$MERGED" \
+					"$LOCAL" "$REMOTE"
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" \
+				-R 'Accel.Search: "Ctrl+F"' \
+				-R 'Accel.SearchForward: "Ctrl-G"' \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	opendiff)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-ancestor "$BASE" \
+					-merge "$MERGED" | cat
+			else
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					-merge "$MERGED" | cat
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
+		fi
+		;;
+	ecmerge)
+		if merge_mode; then
+			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
+					--default --mode=merge3 --to="$MERGED"
+			else
+				"$merge_tool_path" "$LOCAL" "$REMOTE" \
+					--default --mode=merge2 --to="$MERGED"
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" "$LOCAL" "$REMOTE" \
+				--default --mode=merge2 --to="$MERGED"
+		fi
+		;;
+	emerge)
+		if merge_mode; then
+			if $base_present; then
+				"$merge_tool_path" \
+					-f emerge-files-with-ancestor-command \
+					"$LOCAL" "$REMOTE" "$BASE" \
+					"$(basename "$MERGED")"
+			else
+				"$merge_tool_path" \
+					-f emerge-files-command \
+					"$LOCAL" "$REMOTE" \
+					"$(basename "$MERGED")"
+			fi
+			status=$?
+		else
+			"$merge_tool_path" -f emerge-files-command \
+				"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
+		fi
+		;;
+	tortoisemerge)
+		if $base_present; then
+			touch "$BACKUP"
+			"$merge_tool_path" \
+				-base:"$BASE" -mine:"$LOCAL" \
+				-theirs:"$REMOTE" -merged:"$MERGED"
+			check_unchanged
+		else
+			echo "TortoiseMerge cannot be used without a base" 1>&2
+			status=1
+		fi
+		;;
+	*)
+		if test -z "$merge_tool_cmd"; then
+			if merge_mode; then
+				status=1
+			fi
+			break
+		fi
+		if merge_mode; then
+			if test "$merge_tool_trust_exit_code" = "false"; then
+				touch "$BACKUP"
+				( eval $merge_tool_cmd )
+				check_unchanged
+			else
+				( eval $merge_tool_cmd )
+			fi
+			status=$?
+		else
+			( eval $merge_tool_cmd )
+		fi
+		;;
+	esac
+	return $status
+}
+
+guess_merge_tool () {
+	tools="ecmerge"
+	if merge_mode; then
+		tools="$tools tortoisemerge"
+	else
+		kompare=" kompare "
+	fi
+	if test -n "$DISPLAY"; then
+		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
+			tools="meld opendiff kdiff3""$kompare""tkdiff $tools"
+			tools="$tools xxdiff gvimdiff diffuse"
+		else
+			tools="opendiff kdiff3""$kompare""tkdiff xxdiff $tools"
+			tools="$tools meld gvimdiff diffuse"
+		fi
+	fi
+	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
+		# $EDITOR is emacs so add emerge as a candidate
+		tools="$tools emerge vimdiff"
+	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
+		# $EDITOR is vim so add vimdiff as a candidate
+		tools="$tools vimdiff emerge"
+	else
+		tools="$tools emerge vimdiff"
+	fi
+	echo >&2 "merge tool candidates: $tools"
+
+	# Loop over each candidate and stop when a valid merge tool is found.
+	for i in $tools
+	do
+		merge_tool_path="$(translate_merge_tool_path "$i")"
+		if type "$merge_tool_path" > /dev/null 2>&1; then
+			merge_tool="$i"
+			break
+		fi
+	done
+
+	if test -z "$merge_tool" ; then
+		echo >&2 "No known merge resolution program available."
+		return 1
+	fi
+	echo "$merge_tool"
+}
+
+get_configured_merge_tool () {
+	# Diff mode first tries diff.tool and falls back to merge.tool.
+	# Merge mode only checks merge.tool
+	if diff_mode; then
+		tool=$(git config diff.tool)
+	fi
+	if test -z "$tool"; then
+		tool=$(git config merge.tool)
+	fi
+	if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
+		echo >&2 "git config option $TOOL_MODE.tool set to unknown tool: $merge_tool"
+		echo >&2 "Resetting to default..."
+		return 1
+	fi
+	echo "$tool"
+}
+
+get_merge_tool_path () {
+	# A merge tool has been set, so verify that it's valid.
+	if ! valid_tool "$merge_tool"; then
+		echo >&2 "Unknown merge tool $merge_tool"
+		exit 1
+	fi
+	if diff_mode; then
+		merge_tool_path=$(git config difftool."$merge_tool".path)
+	fi
+	if test -z "$merge_tool_path"; then
+		merge_tool_path=$(git config mergetool."$merge_tool".path)
+	fi
+	merge_tool_path="$(translate_merge_tool_path "$merge_tool" "$merge_tool_path")"
+	if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
+		echo >&2 "The $TOOL_MODE tool $merge_tool is not available as '$merge_tool_path'"
+		exit 1
+	fi
+	echo "$merge_tool_path"
+}
+
+get_merge_tool () {
+	merge_tool="$1"
+	# Check if a merge tool has been configured
+	if test -z "$merge_tool"; then
+		merge_tool=$(get_configured_merge_tool)
+	fi
+	# Try to guess an appropriate merge tool if no tool has been set.
+	if test -z "$merge_tool"; then
+		merge_tool=$(guess_merge_tool) || exit
+	fi
+	echo "$merge_tool"
+}
diff --git a/git-mergetool.sh b/git-mergetool.sh
index cceebb7..efa31a2 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -11,7 +11,9 @@
 USAGE='[--tool=tool] [-y|--no-prompt|--prompt] [file to merge] ...'
 SUBDIRECTORY_OK=Yes
 OPTIONS_SPEC=
+TOOL_MODE=merge
 . git-sh-setup
+. git-mergetool--lib
 require_work_tree
 
 # Returns true if the mode reflects a symlink
@@ -110,22 +112,6 @@ resolve_deleted_merge () {
 	done
 }
 
-check_unchanged () {
-    if test "$MERGED" -nt "$BACKUP" ; then
-	status=0;
-    else
-	while true; do
-	    echo "$MERGED seems unchanged."
-	    printf "Was the merge successful? [y/n] "
-	    read answer < /dev/tty
-	    case "$answer" in
-		y*|Y*) status=0; break ;;
-		n*|N*) status=1; break ;;
-	    esac
-	done
-    fi
-}
-
 checkout_staged_file () {
     tmpfile=$(expr "$(git checkout-index --temp --stage="$1" "$2")" : '\([^	]*\)	')
 
@@ -188,107 +174,11 @@ merge_file () {
 	read ans
     fi
 
-    case "$merge_tool" in
-	kdiff3)
-	    if base_present ; then
-		("$merge_tool_path" --auto --L1 "$MERGED (Base)" --L2 "$MERGED (Local)" --L3 "$MERGED (Remote)" \
-		    -o "$MERGED" "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
-	    else
-		("$merge_tool_path" --auto --L1 "$MERGED (Local)" --L2 "$MERGED (Remote)" \
-		    -o "$MERGED" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
-	    fi
-	    status=$?
-	    ;;
-	tkdiff)
-	    if base_present ; then
-		"$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"
-	    else
-		"$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
-	    fi
-	    status=$?
-	    ;;
-	meld)
-	    touch "$BACKUP"
-	    "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	vimdiff)
-	    touch "$BACKUP"
-	    "$merge_tool_path" -d -c "wincmd l" "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	gvimdiff)
-	    touch "$BACKUP"
-	    "$merge_tool_path" -d -c "wincmd l" -f "$LOCAL" "$MERGED" "$REMOTE"
-	    check_unchanged
-	    ;;
-	xxdiff)
-	    touch "$BACKUP"
-	    if base_present ; then
-		"$merge_tool_path" -X --show-merged-pane \
-		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
-		    -R 'Accel.Search: "Ctrl+F"' \
-		    -R 'Accel.SearchForward: "Ctrl-G"' \
-		    --merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"
-	    else
-		"$merge_tool_path" -X --show-merged-pane \
-		    -R 'Accel.SaveAsMerged: "Ctrl-S"' \
-		    -R 'Accel.Search: "Ctrl+F"' \
-		    -R 'Accel.SearchForward: "Ctrl-G"' \
-		    --merged-file "$MERGED" "$LOCAL" "$REMOTE"
-	    fi
-	    check_unchanged
-	    ;;
-	opendiff)
-	    touch "$BACKUP"
-	    if base_present; then
-		"$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$MERGED" | cat
-	    else
-		"$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$MERGED" | cat
-	    fi
-	    check_unchanged
-	    ;;
-	ecmerge)
-	    touch "$BACKUP"
-	    if base_present; then
-		"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" --default --mode=merge3 --to="$MERGED"
-	    else
-		"$merge_tool_path" "$LOCAL" "$REMOTE" --default --mode=merge2 --to="$MERGED"
-	    fi
-	    check_unchanged
-	    ;;
-	emerge)
-	    if base_present ; then
-		"$merge_tool_path" -f emerge-files-with-ancestor-command "$LOCAL" "$REMOTE" "$BASE" "$(basename "$MERGED")"
-	    else
-		"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$MERGED")"
-	    fi
-	    status=$?
-	    ;;
-	tortoisemerge)
-	    if base_present ; then
-		touch "$BACKUP"
-		"$merge_tool_path" -base:"$BASE" -mine:"$LOCAL" -theirs:"$REMOTE" -merged:"$MERGED"
-		check_unchanged
-	    else
-		echo "TortoiseMerge cannot be used without a base" 1>&2
-		status=1
-	    fi
-	    ;;
-	*)
-	    if test -n "$merge_tool_cmd"; then
-		if test "$merge_tool_trust_exit_code" = "false"; then
-		    touch "$BACKUP"
-		    ( eval $merge_tool_cmd )
-		    check_unchanged
-		else
-		    ( eval $merge_tool_cmd )
-		    status=$?
-		fi
-	    fi
-	    ;;
-    esac
-    if test "$status" -ne 0; then
+    present=false
+    base_present &&
+    present=true
+
+    if ! run_merge_tool "$merge_tool" "$present"; then
 	echo "merge of $MERGED failed" 1>&2
 	mv -- "$BACKUP" "$MERGED"
 
@@ -347,44 +237,6 @@ do
     shift
 done
 
-valid_custom_tool()
-{
-    merge_tool_cmd="$(git config mergetool.$1.cmd)"
-    test -n "$merge_tool_cmd"
-}
-
-valid_tool() {
-	case "$1" in
-		kdiff3 | tkdiff | xxdiff | meld | opendiff | emerge | vimdiff | gvimdiff | ecmerge | tortoisemerge)
-			;; # happy
-		*)
-			if ! valid_custom_tool "$1"; then
-				return 1
-			fi
-			;;
-	esac
-}
-
-init_merge_tool_path() {
-	merge_tool_path=$(git config mergetool.$1.path)
-	if test -z "$merge_tool_path" ; then
-		case "$1" in
-			vimdiff)
-				merge_tool_path=vim
-				;;
-			gvimdiff)
-				merge_tool_path=gvim
-				;;
-			emerge)
-				merge_tool_path=emacs
-				;;
-			*)
-				merge_tool_path=$1
-				;;
-		esac
-	fi
-}
-
 prompt_after_failed_merge() {
     while true; do
 	printf "Continue merging other unresolved paths (y/n) ? "
@@ -402,62 +254,12 @@ prompt_after_failed_merge() {
     done
 }
 
-if test -z "$merge_tool"; then
-    merge_tool=$(git config merge.tool)
-    if test -n "$merge_tool" && ! valid_tool "$merge_tool"; then
-	    echo >&2 "git config option merge.tool set to unknown tool: $merge_tool"
-	    echo >&2 "Resetting to default..."
-	    unset merge_tool
-    fi
-fi
-
-if test -z "$merge_tool" ; then
-    if test -n "$DISPLAY"; then
-        if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
-            merge_tool_candidates="meld kdiff3 tkdiff xxdiff tortoisemerge gvimdiff diffuse"
-        else
-            merge_tool_candidates="kdiff3 tkdiff xxdiff meld tortoisemerge gvimdiff diffuse"
-        fi
-    fi
-    if echo "${VISUAL:-$EDITOR}" | grep 'emacs' > /dev/null 2>&1; then
-        merge_tool_candidates="$merge_tool_candidates emerge opendiff vimdiff"
-    elif echo "${VISUAL:-$EDITOR}" | grep 'vim' > /dev/null 2>&1; then
-        merge_tool_candidates="$merge_tool_candidates vimdiff opendiff emerge"
-    else
-        merge_tool_candidates="$merge_tool_candidates opendiff emerge vimdiff"
-    fi
-    echo "merge tool candidates: $merge_tool_candidates"
-    for i in $merge_tool_candidates; do
-        init_merge_tool_path $i
-        if type "$merge_tool_path" > /dev/null 2>&1; then
-            merge_tool=$i
-            break
-        fi
-    done
-    if test -z "$merge_tool" ; then
-	echo "No known merge resolution program available."
-	exit 1
-    fi
-else
-    if ! valid_tool "$merge_tool"; then
-        echo >&2 "Unknown merge_tool $merge_tool"
-        exit 1
-    fi
-
-    init_merge_tool_path "$merge_tool"
-
-    merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
-    merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
-
-    if test -z "$merge_tool_cmd" && ! type "$merge_tool_path" > /dev/null 2>&1; then
-        echo "The merge tool $merge_tool is not available as '$merge_tool_path'"
-        exit 1
-    fi
-
-    if ! test -z "$merge_tool_cmd"; then
-        merge_tool_trust_exit_code="$(git config --bool mergetool.$merge_tool.trustExitCode || echo false)"
-    fi
-fi
+merge_tool=$(get_merge_tool "$merge_tool") || exit
+merge_tool_cmd="$(get_merge_tool_cmd "$merge_tool")"
+merge_tool_path="$(get_merge_tool_path "$merge_tool")" || exit
+merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
+merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)"
+merge_tool_trust_exit_code="$(git config --bool mergetool."$merge_tool".trustExitCode || echo false)"
 
 last_status=0
 rollup_status=0
-- 
1.6.2.2.414.g81aa9

Re: [PATCH v3 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib

[Markus Heidelberg]

David Aguilar, 07.04.2009:
> This consolidates the common functionality from git-mergetool and
> git-difftool--helper into a single git-mergetool--lib scriptlet.
> 
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> 
> Includes suggestions from Markus.
> We also moved opendiff up to the front so that it's
> preferred when on Mac OS.
> 
>  .gitignore                           |    1 +
>  Documentation/git-mergetool--lib.txt |   56 +++++
>  Makefile                             |    1 +
>  git-difftool--helper.sh              |  186 +----------------
>  git-mergetool--lib.sh                |  387 ++++++++++++++++++++++++++++++++++
>  git-mergetool.sh                     |  224 ++------------------
>  6 files changed, 467 insertions(+), 388 deletions(-)
>  create mode 100644 Documentation/git-mergetool--lib.txt
>  create mode 100644 git-mergetool--lib.sh
> 
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh

> +check_unchanged () {
> +	if merge_mode; then
> +		if test "$MERGED" -nt "$BACKUP"; then
> +			status=0
> +		else
> +			while true; do
> +				echo "$MERGED seems unchanged."
> +				printf "Was the merge successful? [y/n] "
> +				read answer < /dev/tty
> +				case "$answer" in
> +				y*|Y*) status=0; break ;;
> +				n*|N*) status=1; break ;;
> +				esac
> +			done
> +		fi
> +	else
> +		status=0
> +	fi
> +}

This function is now only called when $merge_mode is set.

> +run_merge_tool () {
> +	base_present="$2"
> +	if diff_mode; then
> +		status=0
> +		base_present="false"
> +	fi

$base_present also is only used when $merge_mode is set. And I think we
don't need to set status=0 conditionally then.

> +	case "$1" in
> +	kdiff3)
> +		if merge_mode; then
> +			if $base_present; then
> +				("$merge_tool_path" --auto \
> +					--L1 "$MERGED (Base)" \
> +					--L2 "$MERGED (Local)" \
> +					--L3 "$MERGED (Remote)" \
> +					-o "$MERGED" \
> +					"$BASE" "$LOCAL" "$REMOTE" \
> +				> /dev/null 2>&1)
> +			else
> +				("$merge_tool_path" --auto \
> +					--L1 "$MERGED (Local)" \
> +					--L2 "$MERGED (Remote)" \
> +					-o "$MERGED" \
> +					"$LOCAL" "$REMOTE" \
> +				> /dev/null 2>&1)
> +			fi
> +			status=$?
> +		else
> +			("$merge_tool_path" --auto \
> +			 --L1 "$MERGED (A)" \
> +			 --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \
> +			 > /dev/null 2>&1)
> +		fi
> +		;;
> +	kompare)
> +		"$merge_tool_path" "$LOCAL" "$REMOTE"
> +		;;
> +	tkdiff)
> +		if merge_mode; then
> +			if $base_present; then
> +				"$merge_tool_path" -a "$BASE" \
> +					-o "$MERGED" "$LOCAL" "$REMOTE"
> +			else
> +				"$merge_tool_path" \
> +					-o "$MERGED" "$LOCAL" "$REMOTE"
> +			fi
> +			status=$?
> +		else
> +			"$merge_tool_path" "$LOCAL" "$REMOTE"
> +		fi
> +		;;
> +	meld)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			"$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
> +			check_unchanged
> +		else
> +			"$merge_tool_path" "$LOCAL" "$REMOTE"
> +		fi
> +		;;
> +	diffuse)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			if $base_present; then
> +				"$merge_tool_path" \
> +					"$LOCAL" "$MERGED" "$REMOTE" \
> +					"$BASE" | cat
> +			else
> +				"$merge_tool_path" \
> +					"$LOCAL" "$MERGED" "$REMOTE" | cat
> +			fi
> +			check_unchanged
> +		else
> +			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
> +		fi
> +		;;
> +	vimdiff)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			"$merge_tool_path" -d -c "wincmd l" \
> +				"$LOCAL" "$MERGED" "$REMOTE"
> +			check_unchanged
> +		else
> +			"$merge_tool_path" -d -c "wincmd l" \
> +				"$LOCAL" "$REMOTE"
> +		fi
> +		;;
> +	gvimdiff)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			"$merge_tool_path" -d -c "wincmd l" -f \
> +				"$LOCAL" "$MERGED" "$REMOTE"
> +			check_unchanged
> +		else
> +			"$merge_tool_path" -d -c "wincmd l" -f \
> +				"$LOCAL" "$REMOTE"
> +		fi
> +		;;
> +	xxdiff)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			if $base_present; then
> +				"$merge_tool_path" -X --show-merged-pane \
> +					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
> +					-R 'Accel.Search: "Ctrl+F"' \
> +					-R 'Accel.SearchForward: "Ctrl-G"' \
> +					--merged-file "$MERGED" \
> +					"$LOCAL" "$BASE" "$REMOTE"
> +			else
> +				"$merge_tool_path" -X $extra \
> +					-R 'Accel.SaveAsMerged: "Ctrl-S"' \
> +					-R 'Accel.Search: "Ctrl+F"' \
> +					-R 'Accel.SearchForward: "Ctrl-G"' \
> +					--merged-file "$MERGED" \
> +					"$LOCAL" "$REMOTE"
> +			fi
> +			check_unchanged
> +		else
> +			"$merge_tool_path" \
> +				-R 'Accel.Search: "Ctrl+F"' \
> +				-R 'Accel.SearchForward: "Ctrl-G"' \
> +				"$LOCAL" "$REMOTE"
> +		fi
> +		;;
> +	opendiff)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			if $base_present; then
> +				"$merge_tool_path" "$LOCAL" "$REMOTE" \
> +					-ancestor "$BASE" \
> +					-merge "$MERGED" | cat
> +			else
> +				"$merge_tool_path" "$LOCAL" "$REMOTE" \
> +					-merge "$MERGED" | cat
> +			fi
> +			check_unchanged
> +		else
> +			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
> +		fi
> +		;;
> +	ecmerge)
> +		if merge_mode; then
> +			touch "$BACKUP"
> +			if $base_present; then
> +				"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
> +					--default --mode=merge3 --to="$MERGED"
> +			else
> +				"$merge_tool_path" "$LOCAL" "$REMOTE" \
> +					--default --mode=merge2 --to="$MERGED"
> +			fi
> +			check_unchanged
> +		else
> +			"$merge_tool_path" "$LOCAL" "$REMOTE" \
> +				--default --mode=merge2 --to="$MERGED"
> +		fi
> +		;;
> +	emerge)
> +		if merge_mode; then
> +			if $base_present; then
> +				"$merge_tool_path" \
> +					-f emerge-files-with-ancestor-command \
> +					"$LOCAL" "$REMOTE" "$BASE" \
> +					"$(basename "$MERGED")"
> +			else
> +				"$merge_tool_path" \
> +					-f emerge-files-command \
> +					"$LOCAL" "$REMOTE" \
> +					"$(basename "$MERGED")"
> +			fi
> +			status=$?
> +		else
> +			"$merge_tool_path" -f emerge-files-command \
> +				"$LOCAL" "$REMOTE" "$(basename "$MERGED")"
> +		fi
> +		;;
> +	tortoisemerge)
> +		if $base_present; then
> +			touch "$BACKUP"
> +			"$merge_tool_path" \
> +				-base:"$BASE" -mine:"$LOCAL" \
> +				-theirs:"$REMOTE" -merged:"$MERGED"
> +			check_unchanged
> +		else
> +			echo "TortoiseMerge cannot be used without a base" 1>&2
> +			status=1
> +		fi
> +		;;

Looks nice.

> +	*)
> +		if test -z "$merge_tool_cmd"; then
> +			if merge_mode; then
> +				status=1
> +			fi
> +			break
> +		fi
> +		if merge_mode; then
> +			if test "$merge_tool_trust_exit_code" = "false"; then
> +				touch "$BACKUP"
> +				( eval $merge_tool_cmd )
> +				check_unchanged
> +			else
> +				( eval $merge_tool_cmd )
> +			fi
> +			status=$?

"fi" and "status=$?" have to be swapped, else it overwrites the value
set by check_unchanged():
+				status=$?
+			fi

> +		else
> +			( eval $merge_tool_cmd )
> +		fi
> +		;;
> +	esac
> +	return $status
> +}
> +
> +guess_merge_tool () {
> +	tools="ecmerge"
> +	if merge_mode; then
> +		tools="$tools tortoisemerge"
> +	else
> +		kompare=" kompare "
> +	fi
> +	if test -n "$DISPLAY"; then
> +		if test -n "$GNOME_DESKTOP_SESSION_ID" ; then
> +			tools="meld opendiff kdiff3""$kompare""tkdiff $tools"
> +			tools="$tools xxdiff gvimdiff diffuse"
> +		else
> +			tools="opendiff kdiff3""$kompare""tkdiff xxdiff $tools"
> +			tools="$tools meld gvimdiff diffuse"
> +		fi
> +	fi

The setting of $tools looks a bit weird. Maybe we can just ignore the
additional spaces here...

> +	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
> +		# $EDITOR is emacs so add emerge as a candidate
> +		tools="$tools emerge vimdiff"
> +	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
> +		# $EDITOR is vim so add vimdiff as a candidate
> +		tools="$tools vimdiff emerge"
> +	else
> +		tools="$tools emerge vimdiff"
> +	fi

...and delete them here with something like
    sed -e 's/ \+/ /g'
to have it look nice here?:

> +	echo >&2 "merge tool candidates: $tools"
> +
> +	# Loop over each candidate and stop when a valid merge tool is found.
> +	for i in $tools
> +	do
> +		merge_tool_path="$(translate_merge_tool_path "$i")"
> +		if type "$merge_tool_path" > /dev/null 2>&1; then
> +			merge_tool="$i"
> +			break
> +		fi
> +	done
> +
> +	if test -z "$merge_tool" ; then
> +		echo >&2 "No known merge resolution program available."
> +		return 1
> +	fi
> +	echo "$merge_tool"
> +}

Re: [PATCH v2 10/14] difftool: add various git-difftool tests

[Markus Heidelberg]

David Aguilar, 07.04.2009:
> t7800-difftool.sh tests the various command-line flags,
> git-config variables, and environment settings supported by
> git-difftool.
> 
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> 
> The original broke the test suite on Mac OS.
> Testing difftool.<path>.path is hard without internal support
> for a fake tool.

Mac OS doesn't have the "echo" command?

Re: [PATCH v2 10/14] difftool: add various git-difftool tests

[David Aguilar]

On  0, Markus Heidelberg <markus.heidelberg@web.de> wrote:
> David Aguilar, 07.04.2009:
> > t7800-difftool.sh tests the various command-line flags,
> > git-config variables, and environment settings supported by
> > git-difftool.
> > 
> > Signed-off-by: David Aguilar <davvid@gmail.com>
> > ---
> > 
> > The original broke the test suite on Mac OS.
> > Testing difftool.<path>.path is hard without internal support
> > for a fake tool.
> 
> Mac OS doesn't have the "echo" command?
> 

Hmmm maybe the thing that validates the merge tool isn't
using the $merge_tool_path when validating.  All I know is that
it was failing so figured I could add it again later.

-- 
		David

Re: [PATCH v2 10/14] difftool: add various git-difftool tests

[David Aguilar]

On  0, Markus Heidelberg <markus.heidelberg@web.de> wrote:
> David Aguilar, 07.04.2009:
> > t7800-difftool.sh tests the various command-line flags,
> > git-config variables, and environment settings supported by
> > git-difftool.
> > 
> > Signed-off-by: David Aguilar <davvid@gmail.com>
> > ---
> > 
> > The original broke the test suite on Mac OS.
> > Testing difftool.<path>.path is hard without internal support
> > for a fake tool.
> 
> Mac OS doesn't have the "echo" command?
> 

I figured it out.
The test was using wc -l.
On Mac OS the results are formatted differently (leading spaces)
so I modified the test to be more robust.

See [PATCH v3 10/14].

-- 
		David