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
next prev parent 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).