Re: [PATCH v10 1/3] mergetool: add hideResolved configuration
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 30, 2021, 08:08 UTC
- Message-ID
- <xmqqmtwqq239.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20210130054655.48237-2-seth@eseth.com>
Seth House <seth@eseth.com> writes:
Show 8 quoted lines
> +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 ...
Show 5 quoted lines
> @@ -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.
Show 19 quoted lines
> + 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.
Show 15 quoted lines
> + 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