From: Junio C Hamano <gitster@pobox.com>
To: Nicholas Guriev <nicholas@guriev.su>
Cc: git@vger.kernel.org
Subject: Re: [RFC PATCH v2 1/3] mergetools: support difftool.tabbed setting
Date: Mon, 18 Jan 2021 15:25:12 -0800 [thread overview]
Message-ID: <xmqqv9btj09z.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20210118210003.3071205-2-nicholas@guriev.su> (Nicholas Guriev's message of "Tue, 19 Jan 2021 00:00:01 +0300")
Nicholas Guriev <nicholas@guriev.su> writes:
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 78f3647ed9..2a1edbb9b9 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -195,6 +195,11 @@ setup_tool () {
> false
> }
>
> + # Clear combo function declared by a previous tool (if any) to
> + # unambiguously indicate that the current one supports the feature or
> + # not.
> + unset -f diff_combo_cmd
> +
I know this is so that you can use "type diff_combo_cmd" in a
different part of the patch, but I do not think you want to be
fooled by an unrelated ~/bin/diff_combo_cmd that the user may have
under the home directory.
Instead, the fallback definitions can have
diff_combo_supported () {
return 1
}
and have backends that does support diff_combo_supported to override
with their own
diff_combo_supported () {
return 0
}
And then, the part of is_difftool_tabbed that wants to see if the
current backend supports the operation can become:
test "$GIT_DIFFTOOL_TABBED" = true &&
test "${GIT_DIFF_PATH_TOTAL=0}" -gt 1 &&
- type diff_combo_cmd >/dev/null 2>&1
+ diff_combo_supported
}
That way, you do not have to do "unset -f" up above.
> @@ -250,6 +255,28 @@ trust_exit_code () {
> fi
> }
>
> +# Check whether tabbed mode is requested and current loaded tool supports it.
> +is_difftool_tabbed () {
> + : "${GIT_DIFFTOOL_TABBED=$(git config --type=bool \
> + --default=false difftool.tabbed || echo bad value)}"
> + case $(printf "%s" "$GIT_DIFFTOOL_TABBED" | tr '[:upper:]' '[:lower:]') in
> + yes|on|true|1)
> + GIT_DIFFTOOL_TABBED=true
> + ;;
> + no|off|false|0|'')
> + GIT_DIFFTOOL_TABBED=false
> + ;;
> + *)
> + echo "error: bad boolean value of GIT_DIFFTOOL_TABBED" >&2
> + exit 1
> + ;;
> + esac
> +
> + test "$GIT_DIFFTOOL_TABBED" = true &&
> + test "${GIT_DIFF_PATH_TOTAL=0}" -gt 1 &&
> + type diff_combo_cmd >/dev/null 2>&1
> +}
next prev parent reply other threads:[~2021-01-18 23:27 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-13 5:59 [RFC PATCH] mergetools: support difftool.tabbed setting Nicholas Guriev
2021-01-18 21:00 ` [RFC PATCH v2 0/3] implement tabbed mode in difftools Nicholas Guriev
2021-01-18 21:00 ` [RFC PATCH v2 1/3] mergetools: support difftool.tabbed setting Nicholas Guriev
2021-01-18 23:25 ` Junio C Hamano [this message]
2021-01-18 21:00 ` [RFC PATCH v2 2/3] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings Nicholas Guriev
2021-01-18 21:00 ` [RFC PATCH v2 3/3] doc: describe new difftool.tabbed feature Nicholas Guriev
2021-01-25 21:21 ` [PATCH v3 0/4] difftools in tabbed mode Nicholas Guriev
2021-01-25 21:21 ` [PATCH v3 1/4] mergetools: support difftool.tabbed setting Nicholas Guriev
2021-01-25 21:21 ` [PATCH v3 2/4] difftool-helper: conciliate difftool.tabbed and difftool.prompt settings Nicholas Guriev
2021-01-25 23:04 ` Junio C Hamano
2021-01-25 21:21 ` [PATCH v3 3/4] doc: describe new difftool.tabbed feature Nicholas Guriev
2021-01-25 21:21 ` [PATCH v3 4/4] t7800: new tests of " Nicholas Guriev
2021-02-12 5:51 ` [RFC PATCH] mergetools: support difftool.tabbed setting David Aguilar
2021-02-12 22:21 ` 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
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=xmqqv9btj09z.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=nicholas@guriev.su \
/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).