git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Michael Schindler via GitGitGadget" <gitgitgadget@gmail.com>,
	David Aguilar <davvid@gmail.com>
Cc: git@vger.kernel.org, Michael Schindler <michael@compressconsult.com>
Subject: Re: [PATCH] use get_merge_tool_path() also in is_available() to honor settings
Date: Tue, 08 Jun 2021 10:30:17 +0900	[thread overview]
Message-ID: <xmqq7dj5rvme.fsf@gitster.g> (raw)
In-Reply-To: <pull.1032.git.git.1623098845733.gitgitgadget@gmail.com> (Michael Schindler via GitGitGadget's message of "Mon, 07 Jun 2021 20:47:25 +0000")

Adding David Aguilar as an area expert for help reviewing.

Thanks.

"Michael Schindler via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Michael Schindler <michael@compressconsult.com>
>
> fix the is_available test used in git mergetool --tool-help or
> git difftool --tool-help or to check the list of tools available when no
> tool is configured/given with --tool
>
> symtoms: the actual tool running run_merge_tool () considers the difftool.
> "$merge_tool".path and mergetool."$merge_tool".path and if configured
> honors these. See get_merge_tool_path () in git-mergetool--lib.sh
> If not set use fallback: translate_merge_tool_path "$merge_tool".
>
> The is_available () just uses translate_merge_tool_path "$merge_tool".
>
> repo 1: Configure an invalid path in mergetool."$merge_tool".path for a
> tool of your choice.
> You will be informed that the tool is available, but when trying to use
> it will not be found because the invalid configured path is used.
>
> repo2: Install a tool of your choice on a nonstandard place (e.g. rename
> the program) and configure mergetool."$merge_tool".path for this tool.
> You will be informed that the tool is not available (because not found on
> standard place), but when trying to run it will work.
>
> This fix will make the information consistent by using get_merge_tool_path()
> also in is_available()
>
> Signed-off-by: Michael Schindler <michael@compressconsult.com>
> ---
>     use get_merge_tool_path() also in is_available() to honor settings
>     
>     fix the is_available() used in git mergetool --tool-help or git difftool
>     --tool-help or used to check the list of tools available when no tool is
>     configured/given with --tool
>     
>     symtoms: the actual tool running run_merge_tool () considers the
>     difftool."$merge_tool".path and mergetool."$merge_tool".path and if
>     configured honors these. See get_merge_tool_path () in
>     git-mergetool--lib.sh If not defined use fallback:
>     translate_merge_tool_path "$merge_tool".
>     
>     The is_available () just uses translate_merge_tool_path "$merge_tool".
>     
>     repo 1: Configure an invalid path in mergetool."$merge_tool".path for a
>     tool of your choice. You will be informed that the tool is available,
>     but when trying to use it it will not be found because the invalid
>     configured path is used.
>     
>     repo2: Install a tool of your choice on a nonstandard place (e.g. rename
>     the program) and configure mergetool."$merge_tool".path for this tool.
>     You will be informed that the tool is not available (because not found
>     on standard place), but when trying to run it will work.
>     
>     This fix will make the information consistent by using
>     get_merge_tool_path() also in is_available()
>     
>     Signed-off-by: Michael Schindler michael@compressconsult.com
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1032%2Fmichaelcompressconsult%2Fmergetoollib_is_available-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1032/michaelcompressconsult/mergetoollib_is_available-v1
> Pull-Request: https://github.com/git/git/pull/1032
>
>  git-mergetool--lib.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 542a6a75eb3c..8b946e585d7f 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -18,7 +18,7 @@ mode_ok () {
>  }
>  
>  is_available () {
> -	merge_tool_path=$(translate_merge_tool_path "$1") &&
> +	merge_tool_path=$(get_merge_tool_path "$1") &&
>  	type "$merge_tool_path" >/dev/null 2>&1
>  }
>  
>
> base-commit: c09b6306c6ca275ed9d0348a8c8014b2ff723cfb

  reply	other threads:[~2021-06-08  1:30 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07 20:47 [PATCH] use get_merge_tool_path() also in is_available() to honor settings Michael Schindler via GitGitGadget
2021-06-08  1:30 ` Junio C Hamano [this message]
2021-08-30  0:08 ` David Aguilar
2021-08-31 11:01   ` Adam Dinwoodie

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqq7dj5rvme.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=michael@compressconsult.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).