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