diff mbox series

[v8,4/4] mergetool: Add automerge_enabled tool-specific override function

Message ID 20201228045427.1166911-5-seth@eseth.com (mailing list archive)
State New, archived
Headers show
Series mergetool: add automerge configuration | expand

Commit Message

Seth House Dec. 28, 2020, 4:54 a.m. UTC
Hat-tip to Junio C Hamano for the implementation.

Signed-off-by: Seth House <seth@eseth.com>
---
 git-mergetool--lib.sh | 4 ++++
 git-mergetool.sh      | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Johannes Sixt Dec. 28, 2020, 11:57 a.m. UTC | #1
Am 28.12.20 um 05:54 schrieb Seth House:
> Hat-tip to Junio C Hamano for the implementation.

We usually write this as

Helped-by: Junio C Hamano <gitster@pobox.com>

> 
> 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

I would have written this as `return 0` instead of `true` like some of
the functions above this hunk.

> +	}
> +
>  	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
> 

-- Hannes
Junio C Hamano Dec. 28, 2020, 1:19 p.m. UTC | #2
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
diff mbox series

Patch

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