threads / discuss / 16703

Updates to the previous mergetool patches

Subject: Updates to the previous mergetool patches

## tl;dr

4 messages between Dec 12, 2008 and Dec 12, 2008.

replies: 3people: 2as markdown or json

Charles Bailey· Dec 12, 2008, 21:48 UTC · lore

These two patches go on top of 682b451f and replace the 'DONTMERGE' commit: 09b511016. If it would be easier if I cooked these in a different way then let me know.

I've implemented a replacement for the "keep going" option which is now to prompt after a failed merge to see if the user wants to continue merging the remaining paths. This gets rid of a need for an option of configuration option.

I haven't implemented any change to the logic as to whether to pause before invoking the merge tool based on which merge tool is being used.

The added tests for the return status of the merge are slightly less neat than I would have liked, but I think are necessary. I test after invoking the merge tool to make sure that the "roll-up" status is correct, and before invoking the next tool to see if the user wants to continue despite the failure of the previous merge.

Only testing after the merge tool invocation can end up with a "do you want to continue" before mergetool has determined that there are, in fact, no more files to merge. Only testing before the merge means that the status of the last merge doesn't contribute to the script's return value. I don't know how many people rely on the return value of git mergetool, but why not keep it consistent?

The second patch makes optional the retention of temporary files on a failed merge optional. It seperates out the behaviour change that I'd forgotten I'd left in the previous version of the "-k" patch.

Charles Bailey· Dec 12, 2008, 21:48 UTC · re: Charles Bailey · lore

[PATCH 1/2] mergetool: Add prompt to continue after failing to merge a file

This option stops git mergetool from aborting at the first failed merge. After a failed merge the user will be prompted to indicated whether he wishes to continue with attempting to merge subsequent paths or to abort.

This allows some additional use patterns. Merge conflicts can now be previewed one at time and merges can also be skipped so that they can be performed in a later pass.

Signed-off-by: Charles Bailey <charles@hashpling.org>
---
 git-mergetool.sh |   52 +++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 507028f..5144971 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -70,16 +70,16 @@ resolve_symlink_merge () {
 		git checkout-index -f --stage=2 -- "$MERGED"
 		git add -- "$MERGED"
 		cleanup_temp_files --save-backup
-		return
+		return 0
 		;;
 	    [rR]*)
 		git checkout-index -f --stage=3 -- "$MERGED"
 		git add -- "$MERGED"
 		cleanup_temp_files --save-backup
-		return
+		return 0
 		;;
 	    [aA]*)
-		exit 1
+		return 1
 		;;
 	    esac
 	done
@@ -97,15 +97,15 @@ resolve_deleted_merge () {
 	    [mMcC]*)
 		git add -- "$MERGED"
 		cleanup_temp_files --save-backup
-		return
+		return 0
 		;;
 	    [dD]*)
 		git rm -- "$MERGED" > /dev/null
 		cleanup_temp_files
-		return
+		return 0
 		;;
 	    [aA]*)
-		exit 1
+		return 1
 		;;
 	    esac
 	done
@@ -137,7 +137,7 @@ merge_file () {
 	else
 	    echo "$MERGED: file does not need merging"
 	fi
-	exit 1
+	return 1
     fi
 
     ext="$$$(expr "$MERGED" : '.*\(\.[^/]*\)$')"
@@ -269,7 +269,7 @@ merge_file () {
     if test "$status" -ne 0; then
 	echo "merge of $MERGED failed" 1>&2
 	mv -- "$BACKUP" "$MERGED"
-	exit 1
+	return 1
     fi
 
     if test "$merge_keep_backup" = "true"; then
@@ -280,6 +280,7 @@ merge_file () {
 
     git add -- "$MERGED"
     cleanup_temp_files
+    return 0
 }
 
 prompt=$(git config --bool mergetool.prompt || echo true)
@@ -350,6 +351,22 @@ init_merge_tool_path() {
 	fi
 }
 
+prompt_after_failed_merge() {
+    while true; do
+	printf "Continue merging other unresolved paths (y/n) ? "
+	read ans
+	case "$ans" in
+
+	    [yY]*)
+		return 0
+		;;
+
+	    [nN]*)
+		return 1
+		;;
+	esac
+    done
+}
 
 if test -z "$merge_tool"; then
     merge_tool=`git config merge.tool`
@@ -409,6 +426,8 @@ else
     fi
 fi
 
+last_status=0
+rollup_status=0
 
 if test $# -eq 0 ; then
     files=`git ls-files -u | sed -e 's/^[^	]*	//' | sort -u`
@@ -422,14 +441,29 @@ if test $# -eq 0 ; then
     sort -u |
     while IFS= read i
     do
+	if test $last_status -ne 0; then
+	    prompt_after_failed_merge < /dev/tty || exit 1
+	fi
 	printf "\n"
 	merge_file "$i" < /dev/tty > /dev/tty
+	last_status=$?
+	if test $last_status -ne 0; then
+	    rollup_status=1
+	fi
     done
 else
     while test $# -gt 0; do
+	if test $last_status -ne 0; then
+	    prompt_after_failed_merge || exit 1
+	fi
 	printf "\n"
 	merge_file "$1"
+	last_status=$?
+	if test $last_status -ne 0; then
+	    rollup_status=1
+	fi
 	shift
     done
 fi
-exit 0
+
+exit $rollup_status

-- 
1.6.1.rc1.342.g83b24d
Charles Bailey· Dec 12, 2008, 21:48 UTC · re: Charles Bailey · lore

[PATCH 2/2] mergetool: Don't keep temporary merge files unless told to

This changes git mergetool to remove the temporary files used to invoke the merge tool even if it returns non-zero.

This also adds a configuration option (mergetool.keepTemporaries) to retain the previous behaviour if desired.

Signed-off-by: Charles Bailey <charles@hashpling.org>
---
 Documentation/config.txt |    7 +++++++
 git-mergetool.sh         |    6 ++++++
 2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bc5642d..3d5a8df 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -976,6 +976,13 @@ mergetool.keepBackup::
 	is set to `false` then this file is not preserved.  Defaults to
 	`true` (i.e. keep the backup files).
 
+mergetool.keepTemporaries::
+	When invoking a custom merge tool, git uses a set of temporary
+	files to pass to the tool. If the tool returns an error and this
+	variable is set to `true`, then these temporary files will be
+	preserved, otherwise they will be removed after the tool has
+	exited. Defaults to `false`.
+
 mergetool.prompt::
 	Prompt before each invocation of the merge resolution program.
 
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 5144971..f04240d 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -269,6 +269,11 @@ merge_file () {
     if test "$status" -ne 0; then
 	echo "merge of $MERGED failed" 1>&2
 	mv -- "$BACKUP" "$MERGED"
+
+	if test "$merge_keep_temporaries" = "false"; then
+	    cleanup_temp_files
+	fi
+
 	return 1
     fi
 
@@ -415,6 +420,7 @@ else
     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'"

-- 
1.6.1.rc1.342.g83b24d
Jakub Narebski· Dec 12, 2008, 22:11 UTC · re: Charles Bailey · lore

Re: [PATCH 2/2] mergetool: Don't keep temporary merge files unless told to

Charles Bailey wrote:
> +mergetool.keepTemporaries::
mergetool.keepTemporaryFiles ?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

← back to recent threads