All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,  Seth House <seth@eseth.com>,
	 David Aguilar <davvid@gmail.com>,  Johannes Sixt <j6t@kdbg.org>,
	 Philippe Blain <levraiphilippeblain@gmail.com>
Subject: Re: [PATCH 4/5] git-mergetool--lib.sh: add error message for unknown tool variant
Date: Wed, 13 Nov 2024 11:01:35 +0900	[thread overview]
Message-ID: <xmqqr07f28ow.fsf@gitster.g> (raw)
In-Reply-To: <74b83caa1e5c1a63248dd4dcbaf2cf450f9cf32d.1731459128.git.gitgitgadget@gmail.com> (Philippe Blain via GitGitGadget's message of "Wed, 13 Nov 2024 00:52:07 +0000")

"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

  reply	other threads:[~2024-11-13  2:01 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-13  0:52 [PATCH 0/5] git-mergetool: improve error code paths and messages Philippe Blain via GitGitGadget
2024-11-13  0:52 ` [PATCH 1/5] completion: complete '--tool-help' in 'git mergetool' Philippe Blain via GitGitGadget
2024-11-13  0:52 ` [PATCH 2/5] git-mergetool--lib.sh: use TOOL_MODE when erroring about unknown tool Philippe Blain via GitGitGadget
2024-11-13  1:27   ` Junio C Hamano
2024-11-22 18:57     ` Philippe Blain
2024-11-22 19:32       ` Philippe Blain
2024-11-13  0:52 ` [PATCH 3/5] git-mergetool--lib.sh: add error message in 'setup_user_tool' Philippe Blain via GitGitGadget
2024-11-13  1:48   ` Junio C Hamano
2024-11-22 19:02     ` Philippe Blain
2024-11-13  0:52 ` [PATCH 4/5] git-mergetool--lib.sh: add error message for unknown tool variant Philippe Blain via GitGitGadget
2024-11-13  2:01   ` Junio C Hamano [this message]
2024-11-22 19:08     ` Philippe Blain
2024-11-13  0:52 ` [PATCH 5/5] git-difftool--helper.sh: exit upon initialize_merge_tool errors Philippe Blain via GitGitGadget
2024-11-22 19:50 ` [PATCH v2 0/5] git-mergetool: improve error code paths and messages Philippe Blain via GitGitGadget
2024-11-22 19:50   ` [PATCH v2 1/5] completion: complete '--tool-help' in 'git mergetool' Philippe Blain via GitGitGadget
2024-11-22 19:50   ` [PATCH v2 2/5] git-mergetool--lib.sh: use TOOL_MODE when erroring about unknown tool Philippe Blain via GitGitGadget
2024-11-22 19:50   ` [PATCH v2 3/5] git-mergetool--lib.sh: add error message if 'setup_user_tool' fails Philippe Blain via GitGitGadget
2024-11-22 19:50   ` [PATCH v2 4/5] git-mergetool--lib.sh: add error message for unknown tool variant Philippe Blain via GitGitGadget
2024-11-22 19:50   ` [PATCH v2 5/5] git-difftool--helper.sh: exit upon initialize_merge_tool errors Philippe Blain via GitGitGadget
2024-11-26  4:33   ` [PATCH v2 0/5] git-mergetool: improve error code paths and messages Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqr07f28ow.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=j6t@kdbg.org \
    --cc=levraiphilippeblain@gmail.com \
    --cc=seth@eseth.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.