diff mbox series

[v10,1/3] mergetool: add hideResolved configuration

Message ID 20210130054655.48237-2-seth@eseth.com (mailing list archive)
State Superseded
Headers show
Series mergetool: add hideResolved configuration (was automerge) | expand

Commit Message

Seth House Jan. 30, 2021, 5:46 a.m. UTC
The purpose of a mergetool is to help the user resolve any conflicts
that Git cannot automatically resolve. If there is a conflict that must
be resolved manually Git will write a file named MERGED which contains
everything Git was able to resolve by itself and also everything that it
was not able to resolve wrapped in conflict markers.

One way to think of MERGED is as a two- or three-way diff. If each
"side" of the conflict markers is separately extracted an external tool
can represent those conflicts as a side-by-side diff.

However many mergetools instead diff LOCAL and REMOTE both of which
contain versions of the file from before the merge. Since the conflicts
Git resolved automatically are not present it forces the user to
manually re-resolve those conflicts. Some mergetools also show MERGED
but often only for reference and not as the focal point to resolve the
conflicts.

This adds a `mergetool.hideResolved` flag that will overwrite LOCAL and
REMOTE with each corresponding "side" of a conflicted file and thus hide
all conflicts that Git was able to resolve itself. Overwriting these
files will immediately benefit any mergetool that uses them without
requiring any changes.

No adverse effects were noted in a small survey of popular mergetools[1]
so this behavior defaults to `true`. However it can be globally disabled
by setting `mergetool.hideResolved` to `false`.

[1] https://www.eseth.org/2020/mergetools.html
    https://github.com/whiteinge/eseth/blob/c884424769fffb05d87afb33b2cf80cecb4044c3/2020/mergetools.md

Original-implementation-by: Felipe Contreras <felipe.contreras@gmail.com>
Helped-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Seth House <seth@eseth.com>
---
 Documentation/config/mergetool.txt |  9 +++++++++
 git-mergetool.sh                   | 14 ++++++++++++++
 t/t7610-mergetool.sh               | 18 ++++++++++++++++++
 3 files changed, 41 insertions(+)

Comments

Junio C Hamano Jan. 30, 2021, 8:08 a.m. UTC | #1
Seth House <seth@eseth.com> writes:

> +mergetool.hideResolved::
> +	During a merge Git will automatically resolve as many conflicts as
> +	possible and then wrap conflict markers around any conflicts that it
> +	cannot resolve. This flag writes the non-conflicting parts into the
> +	corresponding 'LOCAL' and 'REMOTE' files so that only the unresolved
> +	conflicts are presented to the merge tool. Can be overriden per-tool
> +	via the `mergetool.<tool>.hideResolved` configuration variable.
> +	Defaults to `true`.

This description makes the readers expect that the configuration
variable is a boolean, and setting it to 'no' would disable the
feature, but ...

> @@ -322,6 +331,11 @@ merge_file () {
>  	checkout_staged_file 2 "$MERGED" "$LOCAL"
>  	checkout_staged_file 3 "$MERGED" "$REMOTE"
>  
> +	if test "$(git config --get mergetool.hideResolved)" != "false"

... without --type=bool, any boolean 'false' value that is not
exactly spelled 'false' won't be normalized and fail this test.

I haven't read the remaining 2 patches, so I cannot yet tell if I
can just insert "--type=bool" here and everything would be fine,
or if there are other fallouts for doing so.

> +	then
> +		hide_resolved
> +	fi
> +
>  	if test -z "$local_mode" || test -z "$remote_mode"
>  	then
>  		echo "Deleted merge conflict for '$MERGED':"
> diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
> index 70afdd06fa..0e34b87e37 100755
> --- a/t/t7610-mergetool.sh
> +++ b/t/t7610-mergetool.sh
> @@ -828,4 +828,22 @@ test_expect_success 'mergetool -Oorder-file is honored' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'mergetool hideResolved' '
> +	test_config mergetool.hideResolved true &&
> +	test_when_finished "git reset --hard" &&
> +	git checkout -b test${test_count}_b master &&

As a new feature, this should work with the tip of 'master', but I
think the t7610 test forces the initial branch name to be 'main'.

I'll tweak locally while queuing.

> +	test_write_lines >file1 base "" a &&
> +	git commit -a -m "base" &&
> +	test_write_lines >file1 base "" c &&
> +	git commit -a -m "remote update" &&
> +	git checkout -b test${test_count}_a HEAD~ &&
> +	test_write_lines >file1 local "" b &&
> +	git commit -a -m "local update" &&
> +	test_must_fail git merge test${test_count}_b &&
> +	yes "" | git mergetool file1 &&
> +	test_write_lines >expect local "" c &&
> +	test_cmp expect file1 &&
> +	git commit -m "test resolved with mergetool"
> +'
> +
>  test_done
diff mbox series

Patch

diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt
index 16a27443a3..3171bacf91 100644
--- a/Documentation/config/mergetool.txt
+++ b/Documentation/config/mergetool.txt
@@ -40,6 +40,15 @@  mergetool.meld.useAutoMerge::
 	value of `false` avoids using `--auto-merge` altogether, and is the
 	default value.
 
+mergetool.hideResolved::
+	During a merge Git will automatically resolve as many conflicts as
+	possible and then wrap conflict markers around any conflicts that it
+	cannot resolve. This flag writes the non-conflicting parts into the
+	corresponding 'LOCAL' and 'REMOTE' files so that only the unresolved
+	conflicts are presented to the merge tool. Can be overriden per-tool
+	via the `mergetool.<tool>.hideResolved` configuration variable.
+	Defaults to `true`.
+
 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
diff --git a/git-mergetool.sh b/git-mergetool.sh
index e3f6d543fb..5b0d15ed89 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -239,6 +239,13 @@  checkout_staged_file () {
 	fi
 }
 
+hide_resolved () {
+	git merge-file --ours -q -p "$LOCAL" "$BASE" "$REMOTE" >"$LCONFL"
+	git merge-file --theirs -q -p "$LOCAL" "$BASE" "$REMOTE" >"$RCONFL"
+	mv -- "$LCONFL" "$LOCAL"
+	mv -- "$RCONFL" "$REMOTE"
+}
+
 merge_file () {
 	MERGED="$1"
 
@@ -276,7 +283,9 @@  merge_file () {
 
 	BACKUP="$MERGETOOL_TMPDIR/${BASE}_BACKUP_$$$ext"
 	LOCAL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_$$$ext"
+	LCONFL="$MERGETOOL_TMPDIR/${BASE}_LOCAL_LCONFL_$$$ext"
 	REMOTE="$MERGETOOL_TMPDIR/${BASE}_REMOTE_$$$ext"
+	RCONFL="$MERGETOOL_TMPDIR/${BASE}_REMOTE_RCONFL_$$$ext"
 	BASE="$MERGETOOL_TMPDIR/${BASE}_BASE_$$$ext"
 
 	base_mode= local_mode= remote_mode=
@@ -322,6 +331,11 @@  merge_file () {
 	checkout_staged_file 2 "$MERGED" "$LOCAL"
 	checkout_staged_file 3 "$MERGED" "$REMOTE"
 
+	if test "$(git config --get mergetool.hideResolved)" != "false"
+	then
+		hide_resolved
+	fi
+
 	if test -z "$local_mode" || test -z "$remote_mode"
 	then
 		echo "Deleted merge conflict for '$MERGED':"
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 70afdd06fa..0e34b87e37 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -828,4 +828,22 @@  test_expect_success 'mergetool -Oorder-file is honored' '
 	test_cmp expect actual
 '
 
+test_expect_success 'mergetool hideResolved' '
+	test_config mergetool.hideResolved true &&
+	test_when_finished "git reset --hard" &&
+	git checkout -b test${test_count}_b master &&
+	test_write_lines >file1 base "" a &&
+	git commit -a -m "base" &&
+	test_write_lines >file1 base "" c &&
+	git commit -a -m "remote update" &&
+	git checkout -b test${test_count}_a HEAD~ &&
+	test_write_lines >file1 local "" b &&
+	git commit -a -m "local update" &&
+	test_must_fail git merge test${test_count}_b &&
+	yes "" | git mergetool file1 &&
+	test_write_lines >expect local "" c &&
+	test_cmp expect file1 &&
+	git commit -m "test resolved with mergetool"
+'
+
 test_done