[PATCH 1/4] Teach git mergetool to use custom commands defined at config time

[PATCH 1/4] Teach git mergetool to use custom commands defined at config time

[Charles Bailey]

Currently git mergetool is restricted to a set of commands defined
in the script. You can subvert the mergetool.<tool>.path to force
git mergetool to use a different command, but if you have a command
whose invocation syntax does not match one of the current tools then
you would have to write a wrapper script for it.

This patch adds three git config variable patterns which allow a more
flexible choice of merge tool.

If you run git mergetool with -t/--tool or the merge.tool config
variable set to an unrecognized tool then git mergetool will query the
mergetool.<tool>.cmd config variable. If this variable exists, then
git mergetool will treat the specified tool as a custom command and
will use a shell eval to run the command so that the appropriate shell
variables can be used to find the merge temporary files.

mergetool.<tool>.trustExitCode can be used to indicate that the exit
code of the custom command can be used to determine the success of the
merge. mergetool.<tool>.keepBackup can be used to specify whether the
original pre-merge file with conflict markers should be kept as a
'.orig' file after the merge tool completes.

Signed-off-by: Charles Bailey <charles@hashpling.org>
---

This series of patches are a cleaned and improved version of the
earlier RFC patch that I sent.

They are in decreasing order of how much I care about / like them, and
I'm happy to have them squashed or to submit a squashed set of 2,3 or 4
of the patches.

 Documentation/config.txt |   25 +++++++++++++++++++++++--
 git-mergetool.sh         |   29 +++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f9bdb16..12d87d4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -689,8 +689,10 @@ merge.summary::
 
 merge.tool::
 	Controls which merge resolution program is used by
-	linkgit:git-mergetool[1].  Valid values are: "kdiff3", "tkdiff",
-	"meld", "xxdiff", "emerge", "vimdiff", "gvimdiff", and "opendiff".
+	linkgit:git-mergetool[1].  Valid built-in values are: "kdiff3",
+	"tkdiff", "meld", "xxdiff", "emerge", "vimdiff", "gvimdiff", and
+	"opendiff".  Any other value is treated is custom merge tool
+	and there must be a corresponing mergetool.<tool>.cmd option.
 
 merge.verbosity::
 	Controls the amount of output shown by the recursive merge
@@ -717,6 +719,25 @@ mergetool.<tool>.path::
 	Override the path for the given tool.  This is useful in case
 	your tool is not in the PATH.
 
+mergetool.<tool>.cmd::
+	Specify the command to invoke the specified merge tool.  The
+	specified command is evaluated in shell with the following
+	variables available: 'BASE' is the name of a temporary file
+	containing the common base of the files to be merged, if available;
+	'LOCAL' is the name of a temporary file containing the contents of
+	the file on the current branch; 'REMOTE' is the name of a temporary
+	file containing the contents of the file from the branch being
+	merged; 'path' contains the name of the file to which the merge
+	tool should write the results of a successful merge.
+
+mergetool.<tool>.trustExitCode::
+	For a custom merge command, specify whether the exit code of
+	the merge command can be used to determine whether the merge was
+	successful.  If this is not set to true then the merge target file
+	timestamp is checked and the merge assumed to have been successful
+	if the file has been updated, otherwise the user is prompted to
+	indicate the success of the merge.
+
 pack.window::
 	The size of the window used by linkgit:git-pack-objects[1] when no
 	window size is given on the command line. Defaults to 10.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index cbbb707..cf30e21 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -271,6 +271,19 @@ merge_file () {
 	    status=$?
 	    save_backup
 	    ;;
+	*)
+	    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
+		save_backup
+	    fi
+	    ;;
     esac
     if test "$status" -ne 0; then
 	echo "merge of $path failed" 1>&2
@@ -309,12 +322,20 @@ 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)
 			;; # happy
 		*)
-			return 1
+			if ! valid_custom_tool "$1"; then
+				return 1
+			fi
 			;;
 	esac
 }
@@ -380,10 +401,14 @@ else
 
     init_merge_tool_path "$merge_tool"
 
-    if ! type "$merge_tool_path" > /dev/null 2>&1; then
+    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
 
 
-- 
1.5.4.1.34.g94bf

[PATCH 2/4] Changed the documented variable for mergetool's target file

[Charles Bailey]

Previously it was $path, now it is $MERGED which is more consistent
with the other documented variables for merge tools and a little
more meaningful.

Signed-off-by: Charles Bailey <charles@hashpling.org>
---
 Documentation/config.txt |    2 +-
 git-mergetool.sh         |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 12d87d4..53f790d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -727,7 +727,7 @@ mergetool.<tool>.cmd::
 	'LOCAL' is the name of a temporary file containing the contents of
 	the file on the current branch; 'REMOTE' is the name of a temporary
 	file containing the contents of the file from the branch being
-	merged; 'path' contains the name of the file to which the merge
+	merged; 'MERGED' contains the name of the file to which the merge
 	tool should write the results of a successful merge.
 
 mergetool.<tool>.trustExitCode::
diff --git a/git-mergetool.sh b/git-mergetool.sh
index cf30e21..4681e9a 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -275,10 +275,10 @@ merge_file () {
 	    if test -n "$merge_tool_cmd"; then
 		if test "$merge_tool_trust_exit_code" = "false"; then
 		    touch "$BACKUP"
-		    ( eval $merge_tool_cmd )
+		    ( MERGED="$path" eval $merge_tool_cmd )
 		    check_unchanged
 		else
-		    ( eval $merge_tool_cmd )
+		    ( MERGED="$path" eval $merge_tool_cmd )
 		    status=$?
 		fi
 		save_backup
-- 
1.5.4.1.34.g94bf

[PATCH 3/4] Allow a different mergetool command to be used for 'no base' merges

[Charles Bailey]

This adds a mergetool.<tool>.cmdNoBase config variable pattern to
allow a different command syntax to be specified for custom merge
tools for add/add conflicts where there is no base.

The same effects could have been achieved before this patch by adding
some shell test syntax in the mergetool.<tool>.cmd config variable but
this allows for easier to setup and more readable configurations.

Signed-off-by: Charles Bailey <charles@hashpling.org>
---
 Documentation/config.txt |    6 ++++++
 git-mergetool.sh         |   10 ++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 53f790d..06a93e0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -730,6 +730,12 @@ mergetool.<tool>.cmd::
 	merged; 'MERGED' contains the name of the file to which the merge
 	tool should write the results of a successful merge.
 
+mergetool.<tool>.cmdNoBase::
+	If present, the value of this variable is used instead of
+	mergetool.<tool>.cmd for conflicts where there is no common base
+	(i.e. add/add conflicts). The interpretation of this variable is
+	otherwise identical to mergetool.<tool>.cmd.
+
 mergetool.<tool>.trustExitCode::
 	For a custom merge command, specify whether the exit code of
 	the merge command can be used to determine whether the merge was
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 4681e9a..2437c37 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -273,12 +273,17 @@ merge_file () {
 	    ;;
 	*)
 	    if test -n "$merge_tool_cmd"; then
+		if base_present || test -z "$merge_tool_no_base_cmd"; then
+		    tool="$merge_tool_cmd"
+		else
+		    tool="$merge_tool_no_base_cmd"
+		fi
 		if test "$merge_tool_trust_exit_code" = "false"; then
 		    touch "$BACKUP"
-		    ( MERGED="$path" eval $merge_tool_cmd )
+		    ( MERGED="$path" eval $tool )
 		    check_unchanged
 		else
-		    ( MERGED="$path" eval $merge_tool_cmd )
+		    ( MERGED="$path" eval $tool )
 		    status=$?
 		fi
 		save_backup
@@ -408,6 +413,7 @@ else
 
     if ! test -z "$merge_tool_cmd"; then
         merge_tool_trust_exit_code="$(git config --bool mergetool.$merge_tool.trustExitCode || echo false)"
+        merge_tool_no_base_cmd="$(git config mergetool.$merge_tool.cmdNoBase)"
     fi
 fi
 
-- 
1.5.4.1.34.g94bf

[PATCH 4/4] Tidy up git mergetool's handling of backup files

[Charles Bailey]

Currently a backup pre-merge file with conflict markers is sometimes
kept with a .orig extenstion and sometimes removed depending on the
particular merge tool used.

This patch makes the handling consistent across all merge tools and
configurable via a new mergetool.keepBackup config variable

Signed-off-by: Charles Bailey <charles@hashpling.org>
---
 Documentation/config.txt |    5 +++++
 git-mergetool.sh         |   31 ++++++++++---------------------
 2 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 06a93e0..d9f62f2 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -744,6 +744,11 @@ mergetool.<tool>.trustExitCode::
 	if the file has been updated, otherwise the user is prompted to
 	indicate the success of the merge.
 
+mergetool.keepBackup::
+	After performing a merge, the original file with conflict markers
+	can be saved as a file with a `.orig` extension.  If this variable
+	is set to `false` then this file is not preserved.
+
 pack.window::
 	The size of the window used by linkgit:git-pack-objects[1] when no
 	window size is given on the command line. Defaults to 10.
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 2437c37..4397f47 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -127,18 +127,6 @@ check_unchanged () {
     fi
 }
 
-save_backup () {
-    if test "$status" -eq 0; then
-	mv -- "$BACKUP" "$path.orig"
-    fi
-}
-
-remove_backup () {
-    if test "$status" -eq 0; then
-	rm "$BACKUP"
-    fi
-}
-
 merge_file () {
     path="$1"
 
@@ -201,7 +189,6 @@ merge_file () {
 		    -o "$path" -- "$LOCAL" "$REMOTE" > /dev/null 2>&1)
 	    fi
 	    status=$?
-	    remove_backup
 	    ;;
 	tkdiff)
 	    if base_present ; then
@@ -210,19 +197,16 @@ merge_file () {
 		"$merge_tool_path" -o "$path" -- "$LOCAL" "$REMOTE"
 	    fi
 	    status=$?
-	    save_backup
 	    ;;
 	meld|vimdiff)
 	    touch "$BACKUP"
 	    "$merge_tool_path" -- "$LOCAL" "$path" "$REMOTE"
 	    check_unchanged
-	    save_backup
 	    ;;
 	gvimdiff)
 		touch "$BACKUP"
 		"$merge_tool_path" -f -- "$LOCAL" "$path" "$REMOTE"
 		check_unchanged
-		save_backup
 		;;
 	xxdiff)
 	    touch "$BACKUP"
@@ -240,7 +224,6 @@ merge_file () {
 		    --merged-file "$path" -- "$LOCAL" "$REMOTE"
 	    fi
 	    check_unchanged
-	    save_backup
 	    ;;
 	opendiff)
 	    touch "$BACKUP"
@@ -250,7 +233,6 @@ merge_file () {
 		"$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$path" | cat
 	    fi
 	    check_unchanged
-	    save_backup
 	    ;;
 	ecmerge)
 	    touch "$BACKUP"
@@ -260,7 +242,6 @@ merge_file () {
 		"$merge_tool_path" "$LOCAL" "$REMOTE" --mode=merge2 --to="$path"
 	    fi
 	    check_unchanged
-	    save_backup
 	    ;;
 	emerge)
 	    if base_present ; then
@@ -269,7 +250,6 @@ merge_file () {
 		"$merge_tool_path" -f emerge-files-command "$LOCAL" "$REMOTE" "$(basename "$path")"
 	    fi
 	    status=$?
-	    save_backup
 	    ;;
 	*)
 	    if test -n "$merge_tool_cmd"; then
@@ -286,15 +266,22 @@ merge_file () {
 		    ( MERGED="$path" eval $tool )
 		    status=$?
 		fi
-		save_backup
 	    fi
 	    ;;
     esac
+
     if test "$status" -ne 0; then
 	echo "merge of $path failed" 1>&2
 	mv -- "$BACKUP" "$path"
 	exit 1
     fi
+
+    if test "$merge_keep_backup" = "true"; then
+	mv -- "$BACKUP" "$path.orig"
+    else
+	rm -- "$BACKUP"
+    fi
+
     git add -- "$path"
     cleanup_temp_files
 }
@@ -406,6 +393,8 @@ else
 
     init_merge_tool_path "$merge_tool"
 
+    merge_keep_backup="$(git config --bool merge.keepBackup || echo true)"
+
     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
-- 
1.5.4.1.34.g94bf

Re: [PATCH 1/4] Teach git mergetool to use custom commands defined at config time

[Junio C Hamano]

Charles Bailey <charles@hashpling.org> writes:

> This series of patches are a cleaned and improved version of the
> earlier RFC patch that I sent.

About the organization of the series, I think (note that what I
think does not count as much as what Ted thinks around this
area) it would make much more sense to do 4 first (unless there
is a reason why it behaves differently for existing backends),
then 2 (remove $path everywhere and use $MERGED consistently,
not just where you call out to the custom tool), and then
finally 1.  The general idea is to clean-up first and then add
features on solidified base.

I do not personally see much need for 3, as the custom script
should be able to check the situation and adjust its behaviour
accordingly, and that way you do not have to maintain two
scripts.  I.e. if $BASE does not exist, there is no base, no?