Re: [PATCH 4/5] git-mergetool--lib.sh: add error message for unknown tool variant

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 13, 2024, 02:01 UTC
Message-ID: <xmqqr07f28ow.fsf@gitster.g>
In-Reply-To: <74b83caa1e5c1a63248dd4dcbaf2cf450f9cf32d.1731459128.git.gitgitgadget@gmail.com>

"Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 9 quoted lines

> From: Philippe Blain <levraiphilippeblain@gmail.com>
>
> In setup_tool, we check if the given tool is a known variant of a tool,
> and quietly return with an error if not. This leads to the following
> invocation quietly failing:
>
> 	git mergetool --tool=vimdiff4
>
> Add an error message before returning in this case.

Makes sense, but ...

Show 14 quoted lines

> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> ---
>  git-mergetool--lib.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index f4786afc63f..9a00fabba27 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -263,6 +263,7 @@ setup_tool () {
>  
>  	if ! list_tool_variants | grep -q "^$tool$"
>  	then
> +		echo "error: unknown ${tool%[0-9]} variant '$tool'" >&2

... I do not understand why you strip a single digit from the end.

    git mergetool --tool=nvimdiff4

says 'nvimdiff4' is not known as a variant of 'nvimdiff', but wouldn't it still be a variant of 'vimdiff'? Of course,

    git mergetool --tool=nvimdiff48

gets a vastly different error message ;-)

Saying

	echo >&2 "error: unknown variant '$tool'"

may be sufficient, perhaps?  I dunno.

>  		return 1
>  	fi