Re: [PATCH v2] RFC: mergetool: new config guiDefault supports auto-toggling gui by DISPLAY

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 14, 2022, 15:45 UTC
Message-ID: <xmqqczaube8o.fsf@gitster.g>
In-Reply-To: <pull.1381.v2.git.1665734440009.gitgitgadget@gmail.com>

"Tao Klerks via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 5 quoted lines

>      * In git-mergetool--lib.sh the way I implemented the "auto" special
>        value means that if you put an arbitrary value in the config, eg the
>        typo "uato", you get an error about it being an invalid boolean
>        config value; is that OK? Is there a better way to handle "boolean or
>        special value" config validation? Are there any examples?

I think the ideal behaviour would be:

 * Unless running difftool and difftool.guiChoice has a wrong value,
   or running mergetool and mergetool.guiChoice has a wrong value,
   we should not even complain.

 * If the command line says --gui or --no-gui that makes the setting
   irrelevant, it is OK for us to give a warning to remind the user
   that they may want to fix the spelling of the variable, but
   otherwise go ahead and perform the action as they asked us to.

 * If the command line lacks --gui or --no-gui, we do need to have a
   usable value in the configuration, and we should error out
   without spawning either gui or no-gui tool backend.

It may be usable without the second one and always fail difftool and mergetool until the setting gets fixed, but that is less than ideal. We do allow less than ideal code in, as long as it is an improvement over the status quo, and its presence does not make it harder to later get closer to the ideal.

Thanks.