diff mbox series

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

Message ID 74b83caa1e5c1a63248dd4dcbaf2cf450f9cf32d.1731459128.git.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series git-mergetool: improve error code paths and messages | expand

Commit Message

Philippe Blain Nov. 13, 2024, 12:52 a.m. UTC
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.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 git-mergetool--lib.sh | 1 +
 1 file changed, 1 insertion(+)

Comments

Junio C Hamano Nov. 13, 2024, 2:01 a.m. UTC | #1
"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.


>  		return 1
>  	fi
diff mbox series

Patch

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
 		return 1
 	fi