git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Seth House <seth@eseth.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v8 4/4] mergetool: Add automerge_enabled tool-specific override function
Date: Mon, 28 Dec 2020 05:19:53 -0800	[thread overview]
Message-ID: <xmqqtus6hxty.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20201228045427.1166911-5-seth@eseth.com>

Seth House <seth@eseth.com> writes:

> Hat-tip to Junio C Hamano for the implementation.

That is the least interesting thing the log message for this commit
can talk about.  Instead readers should be able to learn things like
these from the log message (I am not saying the log message should
have these in an enumerated list; I am just enumerating these as
samples):

 - Why does this exist?  

 - What does a tool author want to use the mechanism for, and how
   does the tool author use it?  

 - This mechanism allows tool authors to say "never allow autoMerge
   for this tool", but there is no provision to let them say "always
   use autoMerge without allowing users to turn it off".

It also needs a bit of documentation update to mention that
individual mergetool backend can choose not to trigger the feature
at all, even if the user configures it with mergetool.autoMerge and
mergetool.<tool>.autoMerge options.

Thanks.

> Signed-off-by: Seth House <seth@eseth.com>
> ---
>  git-mergetool--lib.sh | 4 ++++
>  git-mergetool.sh      | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index e059b3559e..5084ceffeb 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -164,6 +164,10 @@ setup_tool () {
>  		return 1
>  	}
>  
> +	automerge_enabled () {
> +		true
> +	}
> +
>  	translate_merge_tool_path () {
>  		echo "$1"
>  	}
> diff --git a/git-mergetool.sh b/git-mergetool.sh
> index 929192d0f8..a44afd3822 100755
> --- a/git-mergetool.sh
> +++ b/git-mergetool.sh
> @@ -336,7 +336,7 @@ merge_file () {
>  
>  	initialize_merge_tool "$merge_tool"
>  
> -	if test "$(
> +	if automerge_enabled && test "$(
>  		git config --get --bool "mergetool.$merge_tool.automerge" ||
>  		git config --get --bool "mergetool.automerge" ||
>  		echo true)" = true

  parent reply	other threads:[~2020-12-28 16:11 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-23  4:53 [PATCH v5 0/1] mergetool: remove unconflicted lines Felipe Contreras
2020-12-23  4:53 ` [PATCH v5 1/1] mergetool: add automerge configuration Felipe Contreras
2020-12-23 13:34   ` Junio C Hamano
2020-12-23 14:23     ` Felipe Contreras
2020-12-23 20:21       ` Junio C Hamano
2020-12-24  0:14         ` Felipe Contreras
2020-12-24  0:32           ` Junio C Hamano
2020-12-24  1:36             ` Felipe Contreras
2020-12-24  6:17               ` Junio C Hamano
2020-12-24 15:59                 ` Felipe Contreras
2020-12-24 22:32                   ` Junio C Hamano
2020-12-27 18:01                     ` Felipe Contreras
2020-12-24  9:24 ` [PATCH v5 0/1] mergetool: remove unconflicted lines Junio C Hamano
2020-12-24 16:16   ` Felipe Contreras
2020-12-30  5:47   ` Johannes Schindelin
2020-12-30 22:33     ` Felipe Contreras
2020-12-27 20:58 ` [PATCH v6 0/1] mergetool: add automerge configuration Seth House
2020-12-27 20:58   ` [PATCH v6 1/2] " Seth House
2020-12-27 22:06     ` Junio C Hamano
2020-12-27 22:29       ` Seth House
2020-12-27 22:59         ` Junio C Hamano
2020-12-27 20:58   ` [PATCH v6 2/2] mergetool: Add per-tool support for the autoMerge flag Seth House
2020-12-27 22:31     ` Junio C Hamano
2020-12-28  0:41   ` [PATCH v7 0/2] mergetool: add automerge configuration Seth House
2020-12-28  0:41     ` [PATCH v7 1/2] " Seth House
2020-12-28  0:41     ` [PATCH v7 2/2] mergetool: Add per-tool support for the autoMerge flag Seth House
2020-12-28  1:18       ` Felipe Contreras
2020-12-28  4:54     ` [PATCH v8 0/4] mergetool: add automerge configuration Seth House
2020-12-28  4:54       ` [PATCH v8 1/4] " Seth House
2020-12-28 11:30         ` Johannes Sixt
2020-12-28  4:54       ` [PATCH v8 2/4] mergetool: Add per-tool support for the autoMerge flag Seth House
2020-12-28 13:09         ` Junio C Hamano
2020-12-28  4:54       ` [PATCH v8 3/4] mergetool: Break setup_tool out into separate initialization function Seth House
2020-12-28 11:48         ` Johannes Sixt
2020-12-28  4:54       ` [PATCH v8 4/4] mergetool: Add automerge_enabled tool-specific override function Seth House
2020-12-28 11:57         ` Johannes Sixt
2020-12-28 13:19         ` Junio C Hamano [this message]
2020-12-28 19:29       ` [PATCH v9 0/5] mergetool: add automerge configuration Seth House
2020-12-28 19:29         ` [PATCH v9 1/5] " Seth House
2020-12-28 19:29         ` [PATCH v9 2/5] mergetool: alphabetize the mergetool config docs Seth House
2020-12-28 19:29         ` [PATCH v9 3/5] mergetool: add per-tool support for the autoMerge flag Seth House
2020-12-28 19:29         ` [PATCH v9 4/5] mergetool: break setup_tool out into separate initialization function Seth House
2020-12-29  8:50           ` Johannes Sixt
2020-12-29 17:23             ` Seth House
2020-12-28 19:29         ` [PATCH v9 5/5] mergetool: add automerge_enabled tool-specific override function Seth House
2020-12-29  2:01           ` Felipe Contreras
2021-01-06  5:55           ` Junio C Hamano
2021-01-07  3:58             ` Seth House
2021-01-07  6:38               ` Junio C Hamano
2021-01-07  9:27                 ` Seth House
2021-01-07 21:26                   ` Junio C Hamano
2021-01-08 15:04                     ` Johannes Schindelin
2021-01-30  5:46         ` [PATCH v10 0/3] mergetool: add hideResolved configuration (was automerge) Seth House
2021-01-30  5:46           ` [PATCH v10 1/3] mergetool: add hideResolved configuration Seth House
2021-01-30  8:08             ` Junio C Hamano
2021-01-30  5:46           ` [PATCH v10 2/3] mergetool: break setup_tool out into separate initialization function Seth House
2021-01-30  5:46           ` [PATCH v10 3/3] mergetool: add per-tool support and overrides for the hideResolved flag Seth House
2021-01-30  8:08             ` Junio C Hamano
2021-02-09 20:07           ` [PATCH v11 0/3] mergetool: add hideResolved configuration (was automerge) Seth House
2021-02-09 20:07             ` [PATCH v11 1/3] mergetool: add hideResolved configuration Seth House
2021-03-09  2:29               ` [PATCH] mergetool: do not enable hideResolved by default Jonathan Nieder
2021-03-09  5:42                 ` Seth House
2021-03-10  1:23                   ` Jonathan Nieder
2021-03-10  8:06                 ` Junio C Hamano
2021-03-11  1:57                   ` Junio C Hamano
2021-03-12 23:12                     ` Junio C Hamano
2021-03-12 23:29                       ` Jonathan Nieder
2021-03-12 23:36                         ` Junio C Hamano
2021-03-13  8:36                           ` [PATCH v2 0/2] " Jonathan Nieder
2021-03-13  8:38                             ` [PATCH 1/2] " Jonathan Nieder
2021-03-13  8:41                             ` [PATCH 2/2] doc: describe mergetool configuration in git-mergetool(1) Jonathan Nieder
2021-03-13 23:34                               ` Junio C Hamano
2021-03-13 23:37                               ` Junio C Hamano
2021-02-09 20:07             ` [PATCH v11 2/3] mergetool: break setup_tool out into separate initialization function Seth House
2021-02-09 20:07             ` [PATCH v11 3/3] mergetool: add per-tool support and overrides for the hideResolved flag Seth House
2021-02-09 22:11             ` [PATCH v11 0/3] mergetool: add hideResolved configuration (was automerge) Junio C Hamano
2021-02-09 23:27               ` Seth House
2020-12-28 10:29     ` [PATCH v7 0/2] mergetool: add automerge configuration Junio C Hamano
2020-12-28 14:40       ` Felipe Contreras
2020-12-28  1:02   ` [PATCH v6 0/1] " Felipe Contreras

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=xmqqtus6hxty.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=seth@eseth.com \
    --subject='Re: [PATCH v8 4/4] mergetool: Add automerge_enabled tool-specific override function' \
    /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

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 NNTP newsgroup(s).