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

[Philippe Blain <levraiphilippeblain@gmail.com>]

Le 2024-11-12 à 21:01, Junio C Hamano a écrit :
> "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> 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 ...
> 
>> 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.

the stripping of the last digit is just because I copied from 
the 'if' a few lines above, where we source "$MERGE_TOOLS_DIR/${tool%[0-9]}".
In MERGE_TOOLS_DIR we have 'nvimdiff' and 'gvimdiff' that simply source vimdiff,
so this works. But I agree that we can simplify the error message, I'll do that.