Subject: Re: [PATCH v10 1/3] mergetool: add hideResolved configuration

From: Junio C Hamano <[email-masked]>
Date: Sat, 30 Jan 2021 00:08:10 -0800

Seth House <[email-masked]> 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