Subject: Re: [PATCH v9 5/5] mergetool: add automerge_enabled tool-specific override function

From: Junio C Hamano <[email-masked]>
Date: Tue, 05 Jan 2021 21:55:26 -0800

Seth House <[email-masked]> writes:

> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 929192d0f8..a44afd3822 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -336,7 +336,7 @@ merge_file () {
>  
>  	initialize_merge_tool "$merge_tool"
>  
> -	if test "$(
> +	if automerge_enabled && test "$(
>  		git config --get --bool "mergetool.$merge_tool.automerge" ||
>  		git config --get --bool "mergetool.automerge" ||
>  		echo true)" = true

This allows the tool author to say "nobody ever is allowed to use my
tool with the automerge feature".  I know I may have suggested
something like that, but I am not sure if we want to be all that
draconian.

If the user explicitly says "I want the new behaviour enabled for
this particular merge tool", we are better off letting the user use
it and take responsibility for the possible breakage.

My preference would probably be

 - if "mergetool.$merge_tool.automerge" is set to 'true' or 'false',
   that's final.

 - Your automerge_enabled helper that is by default 'true' (but
   allows individual merge_tool to return 'false') is asked, and if
   it says 'false', that's final.  But 'true' from automerge_enabled
   is not final at this step.

 - if "mergetool.automerge" is set to 'true' or 'false', that's
   final.

 - otherwise, your automerge_enabled helper's answer (either 'true'
   or 'false') gives the final answer.

That way, those who use a broad "mergetool.automerge = true/false"
would still honor what automerge_enabled yields (which is "enabled
by default but individual merge_tool can set its default to be
disabled"), while individual mergetool.$merge_tool.automerge
configuration would always win.

Hmm?