git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] Support nvim as mergetool
@ 2020-07-18 19:20 pudinha
  2020-07-18 20:30 ` Junio C Hamano
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: pudinha @ 2020-07-18 19:20 UTC (permalink / raw)
  To: git; +Cc: pudinha

---
Hello giters! Hope you are well!

I made this little patch to support nvim as a merge tool. What do you
think?

Best regards,
Pudinha <3

 mergetools/nvimdiff  | 1 +
 mergetools/nvimdiff2 | 1 +
 mergetools/nvimdiff3 | 1 +
 mergetools/vimdiff   | 9 ++++++---
 4 files changed, 9 insertions(+), 3 deletions(-)
 create mode 100644 mergetools/nvimdiff
 create mode 100644 mergetools/nvimdiff2
 create mode 100644 mergetools/nvimdiff3

diff --git a/mergetools/nvimdiff b/mergetools/nvimdiff
new file mode 100644
index 0000000000..04a5bb0ea8
--- /dev/null
+++ b/mergetools/nvimdiff
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/nvimdiff2 b/mergetools/nvimdiff2
new file mode 100644
index 0000000000..04a5bb0ea8
--- /dev/null
+++ b/mergetools/nvimdiff2
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/nvimdiff3 b/mergetools/nvimdiff3
new file mode 100644
index 0000000000..04a5bb0ea8
--- /dev/null
+++ b/mergetools/nvimdiff3
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 10d86f3e19..be559062ee 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -5,7 +5,7 @@ diff_cmd () {
 
 merge_cmd () {
 	case "$1" in
-	gvimdiff|vimdiff)
+	nvimdiff|gvimdiff|vimdiff)
 		if $base_present
 		then
 			"$merge_tool_path" -f -d -c '4wincmd w | wincmd J' \
@@ -15,11 +15,11 @@ merge_cmd () {
 				"$LOCAL" "$MERGED" "$REMOTE"
 		fi
 		;;
-	gvimdiff2|vimdiff2)
+	nvimdiff2|gvimdiff2|vimdiff2)
 		"$merge_tool_path" -f -d -c 'wincmd l' \
 			"$LOCAL" "$MERGED" "$REMOTE"
 		;;
-	gvimdiff3|vimdiff3)
+	nvimdiff3|gvimdiff3|vimdiff3)
 		if $base_present
 		then
 			"$merge_tool_path" -f -d -c 'hid | hid | hid' \
@@ -34,6 +34,9 @@ merge_cmd () {
 
 translate_merge_tool_path() {
 	case "$1" in
+	nvimdiff|nvimdiff2|nvimdiff3)
+		echo nvim
+		;;
 	gvimdiff|gvimdiff2|gvimdiff3)
 		echo gvim
 		;;
-- 
2.27.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Support nvim as mergetool
  2020-07-18 19:20 [PATCH] Support nvim as mergetool pudinha
@ 2020-07-18 20:30 ` Junio C Hamano
  2020-07-19  4:23 ` [PATCH v2 0/2] Support nvim as merge tool pudinha
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2020-07-18 20:30 UTC (permalink / raw)
  To: pudinha; +Cc: git

pudinha <rogi@skylittlesystem.org> writes:

> ---
> Hello giters! Hope you are well!
>
> I made this little patch to support nvim as a merge tool. What do you
> think?

Uninspiring.

But that is not your fault.  

The way vimdiff, vimdiff2 and vimdiff3 waste one file for each just
to have a different name is simply bad, and the way it was extended
to cover gvimdiff family is even more horrible.  This patch makes
the existing mess even worse.  That part is your fault, I'd have to
say.

I'd rather see us explore ways to improve the current arrangement
used to support these 6 variants before adding yet another 3 new
ones.

For example, we could add another method the backends could define,
call it list_tool_variants, and whenever the control flow goes from
run_merge_tool through setup_tool for a tool whose name ends with
[1-9], e.g. "foomerge3", we first see if there is "foomerge" tool
and if there is ask it if it knows about "foomerge3" variant by
calling its list_tool_variants.

That way, we probably can remove the files for vimdiff2, vimdiff3,
gvimdiff2 and gvimdiff3 (gvimdiff needs to stay there, as we do not
want to make the name derivation rule too complex) only to hold a
single line ". vimdiff".  Then the next person who adds yet another
set of backends based on a yet another reimplementation or skin of
vim would only have to add a single file in mergetools/ directory,
not three.

Hmm?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 0/2] Support nvim as merge tool
  2020-07-18 19:20 [PATCH] Support nvim as mergetool pudinha
  2020-07-18 20:30 ` Junio C Hamano
@ 2020-07-19  4:23 ` pudinha
  2020-07-29 21:31   ` [PATCH v3 0/2] mergetools: add support for nvimdiff (neovim) family pudinha
  2020-07-19  4:23 ` [PATCH v2 1/2] Refactor vimdiff and bc merge tool variants pudinha
  2020-07-19  4:23 ` [PATCH v2 2/2] Support nvim as merge tool pudinha
  3 siblings, 1 reply; 10+ messages in thread
From: pudinha @ 2020-07-19  4:23 UTC (permalink / raw)
  To: git; +Cc: pudinha, Junio C Hamano

> For example, we could add another method the backends could define,
> call it list_tool_variants, and whenever the control flow goes from
> run_merge_tool through setup_tool for a tool whose name ends with
> [1-9], e.g. "foomerge3", we first see if there is "foomerge" tool
> and if there is ask it if it knows about "foomerge3" variant by
> calling its list_tool_variants.
>
> That way, we probably can remove the files for vimdiff2, vimdiff3,
> gvimdiff2 and gvimdiff3 (gvimdiff needs to stay there, as we do not
> want to make the name derivation rule too complex) only to hold a
> single line ". vimdiff".  Then the next person who adds yet another
> set of backends based on a yet another reimplementation or skin of
> vim would only have to add a single file in mergetools/ directory,
> not three.

This is what I managed to do.

pudinha (2):
  Refactor vimdiff and bc merge tool variants
  Support nvim as merge tool

 contrib/completion/git-completion.bash |  4 +--
 git-mergetool--lib.sh                  | 35 ++++++++++++++++++++------
 mergetools/bc                          |  5 ++++
 mergetools/bc3                         |  1 -
 mergetools/gvimdiff3                   |  1 -
 mergetools/{gvimdiff2 => nvimdiff}     |  0
 mergetools/vimdiff                     | 21 ++++++++++++----
 mergetools/vimdiff2                    |  1 -
 mergetools/vimdiff3                    |  1 -
 9 files changed, 51 insertions(+), 18 deletions(-)
 delete mode 100644 mergetools/bc3
 delete mode 100644 mergetools/gvimdiff3
 rename mergetools/{gvimdiff2 => nvimdiff} (100%)
 delete mode 100644 mergetools/vimdiff2
 delete mode 100644 mergetools/vimdiff3

-- 
2.27.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] Refactor vimdiff and bc merge tool variants
  2020-07-18 19:20 [PATCH] Support nvim as mergetool pudinha
  2020-07-18 20:30 ` Junio C Hamano
  2020-07-19  4:23 ` [PATCH v2 0/2] Support nvim as merge tool pudinha
@ 2020-07-19  4:23 ` pudinha
  2020-07-20 18:51   ` Junio C Hamano
  2020-07-19  4:23 ` [PATCH v2 2/2] Support nvim as merge tool pudinha
  3 siblings, 1 reply; 10+ messages in thread
From: pudinha @ 2020-07-19  4:23 UTC (permalink / raw)
  To: git; +Cc: pudinha

The merge tools vimdiff2, vimdiff3, gvimdiff2, gvimdiff3 and bc3 are all
variants of the main tools vimdiff and bc. They are implemented in the
main and a one-liner script that just sources it exist for each.

This patch allows variants ending in [0-9] to be correctly wired without
the need for such one-liners, so instead of 5 scripts, only 1 (gvimdiff)
is needed.
---
 git-mergetool--lib.sh | 28 +++++++++++++++++++++++-----
 mergetools/bc         |  5 +++++
 mergetools/bc3        |  1 -
 mergetools/gvimdiff2  |  1 -
 mergetools/gvimdiff3  |  1 -
 mergetools/vimdiff    |  8 ++++++++
 mergetools/vimdiff2   |  1 -
 mergetools/vimdiff3   |  1 -
 8 files changed, 36 insertions(+), 10 deletions(-)
 delete mode 100644 mergetools/bc3
 delete mode 100644 mergetools/gvimdiff2
 delete mode 100644 mergetools/gvimdiff3
 delete mode 100644 mergetools/vimdiff2
 delete mode 100644 mergetools/vimdiff3

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 204a5acd66..29fecc340f 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -43,7 +43,14 @@ show_tool_names () {
 
 	shown_any=
 	( cd "$MERGE_TOOLS_DIR" && ls ) | {
-		while read toolname
+		while read scriptname
+		do
+			setup_tool "$scriptname" 2>/dev/null
+			variants="$variants$(list_tool_variants)\n"
+		done
+		variants="$(echo "$variants" | sort | uniq)"
+
+		for toolname in $variants
 		do
 			if setup_tool "$toolname" 2>/dev/null &&
 				(eval "$condition" "$toolname")
@@ -157,6 +164,10 @@ setup_tool () {
 		echo "$1"
 	}
 
+	list_tool_variants () {
+		echo "$tool"
+	}
+
 	# Most tools' exit codes cannot be trusted, so By default we ignore
 	# their exit code and check the merged file's modification time in
 	# check_unchanged() to determine whether or not the merge was
@@ -178,19 +189,26 @@ setup_tool () {
 		false
 	}
 
-
-	if ! test -f "$MERGE_TOOLS_DIR/$tool"
+	if test -f "$MERGE_TOOLS_DIR/$tool"
 	then
+		. "$MERGE_TOOLS_DIR/$tool"
+	elif test -f "$MERGE_TOOLS_DIR/${tool%[0-9]}"
+	then
+		. "$MERGE_TOOLS_DIR/${tool%[0-9]}"
+	else
 		setup_user_tool
 		return $?
 	fi
 
-	# Load the redefined functions
-	. "$MERGE_TOOLS_DIR/$tool"
 	# Now let the user override the default command for the tool.  If
 	# they have not done so then this will return 1 which we ignore.
 	setup_user_tool
 
+	if ! list_tool_variants | grep -q "^$tool$"
+	then
+		return 1
+	fi
+
 	if merge_mode && ! can_merge
 	then
 		echo "error: '$tool' can not be used to resolve merges" >&2
diff --git a/mergetools/bc b/mergetools/bc
index 3a69e60faa..a89086ee72 100644
--- a/mergetools/bc
+++ b/mergetools/bc
@@ -21,3 +21,8 @@ translate_merge_tool_path() {
 		echo bcompare
 	fi
 }
+
+list_tool_variants () {
+	echo bc
+	echo bc3
+}
diff --git a/mergetools/bc3 b/mergetools/bc3
deleted file mode 100644
index 5d8dd48184..0000000000
--- a/mergetools/bc3
+++ /dev/null
@@ -1 +0,0 @@
-. "$MERGE_TOOLS_DIR/bc"
diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
deleted file mode 100644
index 04a5bb0ea8..0000000000
--- a/mergetools/gvimdiff2
+++ /dev/null
@@ -1 +0,0 @@
-. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/gvimdiff3 b/mergetools/gvimdiff3
deleted file mode 100644
index 04a5bb0ea8..0000000000
--- a/mergetools/gvimdiff3
+++ /dev/null
@@ -1 +0,0 @@
-. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 10d86f3e19..3925e1fc3e 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -46,3 +46,11 @@ translate_merge_tool_path() {
 exit_code_trustable () {
 	true
 }
+
+list_tool_variants () {
+	for prefix in '' g; do
+		for suffix in '' 2 3; do
+			echo "${prefix}vimdiff${suffix}"
+		done
+	done
+}
diff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2
deleted file mode 100644
index 04a5bb0ea8..0000000000
--- a/mergetools/vimdiff2
+++ /dev/null
@@ -1 +0,0 @@
-. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/vimdiff3 b/mergetools/vimdiff3
deleted file mode 100644
index 04a5bb0ea8..0000000000
--- a/mergetools/vimdiff3
+++ /dev/null
@@ -1 +0,0 @@
-. "$MERGE_TOOLS_DIR/vimdiff"
-- 
2.27.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] Support nvim as merge tool
  2020-07-18 19:20 [PATCH] Support nvim as mergetool pudinha
                   ` (2 preceding siblings ...)
  2020-07-19  4:23 ` [PATCH v2 1/2] Refactor vimdiff and bc merge tool variants pudinha
@ 2020-07-19  4:23 ` pudinha
  2020-07-20 18:52   ` Junio C Hamano
  3 siblings, 1 reply; 10+ messages in thread
From: pudinha @ 2020-07-19  4:23 UTC (permalink / raw)
  To: git; +Cc: pudinha

---
 contrib/completion/git-completion.bash |  4 ++--
 git-mergetool--lib.sh                  |  7 +++++--
 mergetools/nvimdiff                    |  1 +
 mergetools/vimdiff                     | 15 +++++++++------
 4 files changed, 17 insertions(+), 10 deletions(-)
 create mode 100644 mergetools/nvimdiff

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ee468ea3b0..aed08f8df5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1712,8 +1712,8 @@ _git_diff ()
 }
 
 __git_mergetools_common="diffuse diffmerge ecmerge emerge kdiff3 meld opendiff
-			tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc
-			codecompare smerge
+			tkdiff vimdiff nvimdiff gvimdiff xxdiff araxis p4merge
+			bc codecompare smerge
 "
 
 _git_difftool ()
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 29fecc340f..2defef28cd 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -304,11 +304,14 @@ list_merge_tool_candidates () {
 		tools="$tools smerge"
 	fi
 	case "${VISUAL:-$EDITOR}" in
+	*nvim*)
+		tools="$tools nvimdiff vimdiff emerge"
+		;;
 	*vim*)
-		tools="$tools vimdiff emerge"
+		tools="$tools vimdiff nvimdiff emerge"
 		;;
 	*)
-		tools="$tools emerge vimdiff"
+		tools="$tools emerge vimdiff nvimdiff"
 		;;
 	esac
 }
diff --git a/mergetools/nvimdiff b/mergetools/nvimdiff
new file mode 100644
index 0000000000..04a5bb0ea8
--- /dev/null
+++ b/mergetools/nvimdiff
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 3925e1fc3e..abc8ce4ec4 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -5,7 +5,7 @@ diff_cmd () {
 
 merge_cmd () {
 	case "$1" in
-	gvimdiff|vimdiff)
+	*vimdiff)
 		if $base_present
 		then
 			"$merge_tool_path" -f -d -c '4wincmd w | wincmd J' \
@@ -15,11 +15,11 @@ merge_cmd () {
 				"$LOCAL" "$MERGED" "$REMOTE"
 		fi
 		;;
-	gvimdiff2|vimdiff2)
+	*vimdiff2)
 		"$merge_tool_path" -f -d -c 'wincmd l' \
 			"$LOCAL" "$MERGED" "$REMOTE"
 		;;
-	gvimdiff3|vimdiff3)
+	*vimdiff3)
 		if $base_present
 		then
 			"$merge_tool_path" -f -d -c 'hid | hid | hid' \
@@ -34,10 +34,13 @@ merge_cmd () {
 
 translate_merge_tool_path() {
 	case "$1" in
-	gvimdiff|gvimdiff2|gvimdiff3)
+	nvimdiff*)
+		echo nvim
+		;;
+	gvimdiff*)
 		echo gvim
 		;;
-	vimdiff|vimdiff2|vimdiff3)
+	vimdiff*)
 		echo vim
 		;;
 	esac
@@ -48,7 +51,7 @@ exit_code_trustable () {
 }
 
 list_tool_variants () {
-	for prefix in '' g; do
+	for prefix in '' g n; do
 		for suffix in '' 2 3; do
 			echo "${prefix}vimdiff${suffix}"
 		done
-- 
2.27.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] Refactor vimdiff and bc merge tool variants
  2020-07-19  4:23 ` [PATCH v2 1/2] Refactor vimdiff and bc merge tool variants pudinha
@ 2020-07-20 18:51   ` Junio C Hamano
  0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2020-07-20 18:51 UTC (permalink / raw)
  To: pudinha; +Cc: git

pudinha <rogi@skylittlesystem.org> writes:

> Subject: Re: [PATCH v2 1/2] Refactor vimdiff and bc merge tool variants

cf. https://git-scm.com/docs/SubmittingPatches#describe-changes

Two tricks to pick a good title are:

 - Read a pageful or two of "git shortlog --no-merges" output to get
   accustomed to the general pattern in the entire project.  It
   would become clear why titles with "area:" prefix help the
   patches with them easier to locate.

 - Read a pageful of "git shortlog --no-merges -- mergetools"
   (i.e. the same but limited to the files you are touching) to see
   how the changes that contributed over time to build the subsystem
   are called, so that the new patches can fit in the pattern.

> The merge tools vimdiff2, vimdiff3, gvimdiff2, gvimdiff3 and bc3 are all
> variants of the main tools vimdiff and bc. They are implemented in the
> main and a one-liner script that just sources it exist for each.
>
> This patch allows variants ending in [0-9] to be correctly wired without
> the need for such one-liners, so instead of 5 scripts, only 1 (gvimdiff)
> is needed.

Well explained.  The final paragraph could be made more in line with
the existing commit log messages by writing in the imperative mood
(e.g. "Introduce a new list_tool_variants method, and when a tool
whose name ends with a digit is asked for, and if there is no such
script in mergetools/ directoroy, ask the tool with the name without
the final digit, if exists, if it knows how to handle the variant.
That way, instead of 5 scriptss, ..."), but what is written above is
already good enough.

> ---

cf. https://git-scm.com/docs/SubmittingPatches#sign-off

Thanks.

>  git-mergetool--lib.sh | 28 +++++++++++++++++++++++-----
>  mergetools/bc         |  5 +++++
>  mergetools/bc3        |  1 -
>  mergetools/gvimdiff2  |  1 -
>  mergetools/gvimdiff3  |  1 -
>  mergetools/vimdiff    |  8 ++++++++
>  mergetools/vimdiff2   |  1 -
>  mergetools/vimdiff3   |  1 -
>  8 files changed, 36 insertions(+), 10 deletions(-)
>  delete mode 100644 mergetools/bc3
>  delete mode 100644 mergetools/gvimdiff2
>  delete mode 100644 mergetools/gvimdiff3
>  delete mode 100644 mergetools/vimdiff2
>  delete mode 100644 mergetools/vimdiff3
>
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 204a5acd66..29fecc340f 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -43,7 +43,14 @@ show_tool_names () {
>  
>  	shown_any=
>  	( cd "$MERGE_TOOLS_DIR" && ls ) | {
> -		while read toolname
> +		while read scriptname
> +		do
> +			setup_tool "$scriptname" 2>/dev/null
> +			variants="$variants$(list_tool_variants)\n"
> +		done
> +		variants="$(echo "$variants" | sort | uniq)"
> +
> +		for toolname in $variants
>  		do
>  			if setup_tool "$toolname" 2>/dev/null &&
>  				(eval "$condition" "$toolname")
> @@ -157,6 +164,10 @@ setup_tool () {
>  		echo "$1"
>  	}
>  
> +	list_tool_variants () {
> +		echo "$tool"
> +	}
> +
>  	# Most tools' exit codes cannot be trusted, so By default we ignore
>  	# their exit code and check the merged file's modification time in
>  	# check_unchanged() to determine whether or not the merge was
> @@ -178,19 +189,26 @@ setup_tool () {
>  		false
>  	}
>  
> -
> -	if ! test -f "$MERGE_TOOLS_DIR/$tool"
> +	if test -f "$MERGE_TOOLS_DIR/$tool"
>  	then
> +		. "$MERGE_TOOLS_DIR/$tool"
> +	elif test -f "$MERGE_TOOLS_DIR/${tool%[0-9]}"
> +	then
> +		. "$MERGE_TOOLS_DIR/${tool%[0-9]}"
> +	else

Nice---if the thing has a custom script in mergetools/ then we do
not do anything special, but if a script is missing for a tool whose
name ends with a digit, and a script exists for a tool with the same
name without that digit, we try to use that instead, and we make
sure it does support the named variant or bail out later...

>  		setup_user_tool
>  		return $?
>  	fi
>  
> -	# Load the redefined functions
> -	. "$MERGE_TOOLS_DIR/$tool"
>  	# Now let the user override the default command for the tool.  If
>  	# they have not done so then this will return 1 which we ignore.
>  	setup_user_tool
>  
> +	if ! list_tool_variants | grep -q "^$tool$"
> +	then
> +		return 1
> +	fi

... like this.  Excellent.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] Support nvim as merge tool
  2020-07-19  4:23 ` [PATCH v2 2/2] Support nvim as merge tool pudinha
@ 2020-07-20 18:52   ` Junio C Hamano
  0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2020-07-20 18:52 UTC (permalink / raw)
  To: pudinha; +Cc: git

pudinha <rogi@skylittlesystem.org> writes:

> Subject: Re: [PATCH v2 2/2] Support nvim as merge tool

Perhaps "mergetools: add support for nvimdiff family" or something
along that line.

> ---

Missing sign-off.

Thanks.


>  contrib/completion/git-completion.bash |  4 ++--
>  git-mergetool--lib.sh                  |  7 +++++--
>  mergetools/nvimdiff                    |  1 +
>  mergetools/vimdiff                     | 15 +++++++++------
>  4 files changed, 17 insertions(+), 10 deletions(-)
>  create mode 100644 mergetools/nvimdiff
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index ee468ea3b0..aed08f8df5 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1712,8 +1712,8 @@ _git_diff ()
>  }
>  
>  __git_mergetools_common="diffuse diffmerge ecmerge emerge kdiff3 meld opendiff
> -			tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc
> -			codecompare smerge
> +			tkdiff vimdiff nvimdiff gvimdiff xxdiff araxis p4merge
> +			bc codecompare smerge
>  "
>  
>  _git_difftool ()
> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> index 29fecc340f..2defef28cd 100644
> --- a/git-mergetool--lib.sh
> +++ b/git-mergetool--lib.sh
> @@ -304,11 +304,14 @@ list_merge_tool_candidates () {
>  		tools="$tools smerge"
>  	fi
>  	case "${VISUAL:-$EDITOR}" in
> +	*nvim*)
> +		tools="$tools nvimdiff vimdiff emerge"
> +		;;
>  	*vim*)
> -		tools="$tools vimdiff emerge"
> +		tools="$tools vimdiff nvimdiff emerge"
>  		;;
>  	*)
> -		tools="$tools emerge vimdiff"
> +		tools="$tools emerge vimdiff nvimdiff"
>  		;;
>  	esac
>  }
> diff --git a/mergetools/nvimdiff b/mergetools/nvimdiff
> new file mode 100644
> index 0000000000..04a5bb0ea8
> --- /dev/null
> +++ b/mergetools/nvimdiff
> @@ -0,0 +1 @@
> +. "$MERGE_TOOLS_DIR/vimdiff"
> diff --git a/mergetools/vimdiff b/mergetools/vimdiff
> index 3925e1fc3e..abc8ce4ec4 100644
> --- a/mergetools/vimdiff
> +++ b/mergetools/vimdiff
> @@ -5,7 +5,7 @@ diff_cmd () {
>  
>  merge_cmd () {
>  	case "$1" in
> -	gvimdiff|vimdiff)
> +	*vimdiff)
>  		if $base_present
>  		then
>  			"$merge_tool_path" -f -d -c '4wincmd w | wincmd J' \
> @@ -15,11 +15,11 @@ merge_cmd () {
>  				"$LOCAL" "$MERGED" "$REMOTE"
>  		fi
>  		;;
> -	gvimdiff2|vimdiff2)
> +	*vimdiff2)
>  		"$merge_tool_path" -f -d -c 'wincmd l' \
>  			"$LOCAL" "$MERGED" "$REMOTE"
>  		;;
> -	gvimdiff3|vimdiff3)
> +	*vimdiff3)
>  		if $base_present
>  		then
>  			"$merge_tool_path" -f -d -c 'hid | hid | hid' \
> @@ -34,10 +34,13 @@ merge_cmd () {
>  
>  translate_merge_tool_path() {
>  	case "$1" in
> -	gvimdiff|gvimdiff2|gvimdiff3)
> +	nvimdiff*)
> +		echo nvim
> +		;;
> +	gvimdiff*)
>  		echo gvim
>  		;;
> -	vimdiff|vimdiff2|vimdiff3)
> +	vimdiff*)
>  		echo vim
>  		;;
>  	esac
> @@ -48,7 +51,7 @@ exit_code_trustable () {
>  }
>  
>  list_tool_variants () {
> -	for prefix in '' g; do
> +	for prefix in '' g n; do
>  		for suffix in '' 2 3; do
>  			echo "${prefix}vimdiff${suffix}"
>  		done

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 0/2] mergetools: add support for nvimdiff (neovim) family
  2020-07-19  4:23 ` [PATCH v2 0/2] Support nvim as merge tool pudinha
@ 2020-07-29 21:31   ` pudinha
  2020-07-29 21:31     ` [PATCH v3 1/2] mergetool--lib: improve support for vimdiff-style tool variants pudinha
  2020-07-29 21:31     ` [PATCH v3 2/2] mergetools: add support for nvimdiff (neovim) family pudinha
  0 siblings, 2 replies; 10+ messages in thread
From: pudinha @ 2020-07-29 21:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, pudinha

Junio,

thanks for your time. I think I addressed all your comments regarding
the commit messages. One of them is a bit long, but I saw some others
are too and I'd rather have it long than not descriptive enough.

Please, let me know if anything else needs changing.

Best regards

pudinha (2):
  mergetool--lib: improve support for vimdiff-style tool variants
  mergetools: add support for nvimdiff (neovim) family

 contrib/completion/git-completion.bash |  4 +--
 git-mergetool--lib.sh                  | 35 ++++++++++++++++++++------
 mergetools/bc                          |  5 ++++
 mergetools/bc3                         |  1 -
 mergetools/gvimdiff3                   |  1 -
 mergetools/{gvimdiff2 => nvimdiff}     |  0
 mergetools/vimdiff                     | 21 ++++++++++++----
 mergetools/vimdiff2                    |  1 -
 mergetools/vimdiff3                    |  1 -
 9 files changed, 51 insertions(+), 18 deletions(-)
 delete mode 100644 mergetools/bc3
 delete mode 100644 mergetools/gvimdiff3
 rename mergetools/{gvimdiff2 => nvimdiff} (100%)
 delete mode 100644 mergetools/vimdiff2
 delete mode 100644 mergetools/vimdiff3

-- 
2.28.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/2] mergetool--lib: improve support for vimdiff-style tool variants
  2020-07-29 21:31   ` [PATCH v3 0/2] mergetools: add support for nvimdiff (neovim) family pudinha
@ 2020-07-29 21:31     ` pudinha
  2020-07-29 21:31     ` [PATCH v3 2/2] mergetools: add support for nvimdiff (neovim) family pudinha
  1 sibling, 0 replies; 10+ messages in thread
From: pudinha @ 2020-07-29 21:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, pudinha

The merge tools vimdiff2, vimdiff3, gvimdiff2, gvimdiff3 and bc3 are all
variants of the main tools vimdiff and bc. They are implemented in the
main and a one-liner script that just sources it exist for each.

Allow variants ending in [0-9] to be correctly wired without the need
for such one-liners, so instead of 5 scripts, only 1 (gvimdiff) is
needed.

Signed-off-by: pudinha <rogi@skylittlesystem.org>
---
 git-mergetool--lib.sh | 28 +++++++++++++++++++++++-----
 mergetools/bc         |  5 +++++
 mergetools/bc3        |  1 -
 mergetools/gvimdiff2  |  1 -
 mergetools/gvimdiff3  |  1 -
 mergetools/vimdiff    |  8 ++++++++
 mergetools/vimdiff2   |  1 -
 mergetools/vimdiff3   |  1 -
 8 files changed, 36 insertions(+), 10 deletions(-)
 delete mode 100644 mergetools/bc3
 delete mode 100644 mergetools/gvimdiff2
 delete mode 100644 mergetools/gvimdiff3
 delete mode 100644 mergetools/vimdiff2
 delete mode 100644 mergetools/vimdiff3

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 204a5acd66..29fecc340f 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -43,7 +43,14 @@ show_tool_names () {
 
 	shown_any=
 	( cd "$MERGE_TOOLS_DIR" && ls ) | {
-		while read toolname
+		while read scriptname
+		do
+			setup_tool "$scriptname" 2>/dev/null
+			variants="$variants$(list_tool_variants)\n"
+		done
+		variants="$(echo "$variants" | sort | uniq)"
+
+		for toolname in $variants
 		do
 			if setup_tool "$toolname" 2>/dev/null &&
 				(eval "$condition" "$toolname")
@@ -157,6 +164,10 @@ setup_tool () {
 		echo "$1"
 	}
 
+	list_tool_variants () {
+		echo "$tool"
+	}
+
 	# Most tools' exit codes cannot be trusted, so By default we ignore
 	# their exit code and check the merged file's modification time in
 	# check_unchanged() to determine whether or not the merge was
@@ -178,19 +189,26 @@ setup_tool () {
 		false
 	}
 
-
-	if ! test -f "$MERGE_TOOLS_DIR/$tool"
+	if test -f "$MERGE_TOOLS_DIR/$tool"
 	then
+		. "$MERGE_TOOLS_DIR/$tool"
+	elif test -f "$MERGE_TOOLS_DIR/${tool%[0-9]}"
+	then
+		. "$MERGE_TOOLS_DIR/${tool%[0-9]}"
+	else
 		setup_user_tool
 		return $?
 	fi
 
-	# Load the redefined functions
-	. "$MERGE_TOOLS_DIR/$tool"
 	# Now let the user override the default command for the tool.  If
 	# they have not done so then this will return 1 which we ignore.
 	setup_user_tool
 
+	if ! list_tool_variants | grep -q "^$tool$"
+	then
+		return 1
+	fi
+
 	if merge_mode && ! can_merge
 	then
 		echo "error: '$tool' can not be used to resolve merges" >&2
diff --git a/mergetools/bc b/mergetools/bc
index 3a69e60faa..a89086ee72 100644
--- a/mergetools/bc
+++ b/mergetools/bc
@@ -21,3 +21,8 @@ translate_merge_tool_path() {
 		echo bcompare
 	fi
 }
+
+list_tool_variants () {
+	echo bc
+	echo bc3
+}
diff --git a/mergetools/bc3 b/mergetools/bc3
deleted file mode 100644
index 5d8dd48184..0000000000
--- a/mergetools/bc3
+++ /dev/null
@@ -1 +0,0 @@
-. "$MERGE_TOOLS_DIR/bc"
diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
deleted file mode 100644
index 04a5bb0ea8..0000000000
--- a/mergetools/gvimdiff2
+++ /dev/null
@@ -1 +0,0 @@
-. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/gvimdiff3 b/mergetools/gvimdiff3
deleted file mode 100644
index 04a5bb0ea8..0000000000
--- a/mergetools/gvimdiff3
+++ /dev/null
@@ -1 +0,0 @@
-. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 10d86f3e19..3925e1fc3e 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -46,3 +46,11 @@ translate_merge_tool_path() {
 exit_code_trustable () {
 	true
 }
+
+list_tool_variants () {
+	for prefix in '' g; do
+		for suffix in '' 2 3; do
+			echo "${prefix}vimdiff${suffix}"
+		done
+	done
+}
diff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2
deleted file mode 100644
index 04a5bb0ea8..0000000000
--- a/mergetools/vimdiff2
+++ /dev/null
@@ -1 +0,0 @@
-. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/vimdiff3 b/mergetools/vimdiff3
deleted file mode 100644
index 04a5bb0ea8..0000000000
--- a/mergetools/vimdiff3
+++ /dev/null
@@ -1 +0,0 @@
-. "$MERGE_TOOLS_DIR/vimdiff"
-- 
2.28.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 2/2] mergetools: add support for nvimdiff (neovim) family
  2020-07-29 21:31   ` [PATCH v3 0/2] mergetools: add support for nvimdiff (neovim) family pudinha
  2020-07-29 21:31     ` [PATCH v3 1/2] mergetool--lib: improve support for vimdiff-style tool variants pudinha
@ 2020-07-29 21:31     ` pudinha
  1 sibling, 0 replies; 10+ messages in thread
From: pudinha @ 2020-07-29 21:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, pudinha

Signed-off-by: pudinha <rogi@skylittlesystem.org>
---
 contrib/completion/git-completion.bash |  4 ++--
 git-mergetool--lib.sh                  |  7 +++++--
 mergetools/nvimdiff                    |  1 +
 mergetools/vimdiff                     | 15 +++++++++------
 4 files changed, 17 insertions(+), 10 deletions(-)
 create mode 100644 mergetools/nvimdiff

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ee468ea3b0..aed08f8df5 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1712,8 +1712,8 @@ _git_diff ()
 }
 
 __git_mergetools_common="diffuse diffmerge ecmerge emerge kdiff3 meld opendiff
-			tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc
-			codecompare smerge
+			tkdiff vimdiff nvimdiff gvimdiff xxdiff araxis p4merge
+			bc codecompare smerge
 "
 
 _git_difftool ()
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 29fecc340f..2defef28cd 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -304,11 +304,14 @@ list_merge_tool_candidates () {
 		tools="$tools smerge"
 	fi
 	case "${VISUAL:-$EDITOR}" in
+	*nvim*)
+		tools="$tools nvimdiff vimdiff emerge"
+		;;
 	*vim*)
-		tools="$tools vimdiff emerge"
+		tools="$tools vimdiff nvimdiff emerge"
 		;;
 	*)
-		tools="$tools emerge vimdiff"
+		tools="$tools emerge vimdiff nvimdiff"
 		;;
 	esac
 }
diff --git a/mergetools/nvimdiff b/mergetools/nvimdiff
new file mode 100644
index 0000000000..04a5bb0ea8
--- /dev/null
+++ b/mergetools/nvimdiff
@@ -0,0 +1 @@
+. "$MERGE_TOOLS_DIR/vimdiff"
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 3925e1fc3e..abc8ce4ec4 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -5,7 +5,7 @@ diff_cmd () {
 
 merge_cmd () {
 	case "$1" in
-	gvimdiff|vimdiff)
+	*vimdiff)
 		if $base_present
 		then
 			"$merge_tool_path" -f -d -c '4wincmd w | wincmd J' \
@@ -15,11 +15,11 @@ merge_cmd () {
 				"$LOCAL" "$MERGED" "$REMOTE"
 		fi
 		;;
-	gvimdiff2|vimdiff2)
+	*vimdiff2)
 		"$merge_tool_path" -f -d -c 'wincmd l' \
 			"$LOCAL" "$MERGED" "$REMOTE"
 		;;
-	gvimdiff3|vimdiff3)
+	*vimdiff3)
 		if $base_present
 		then
 			"$merge_tool_path" -f -d -c 'hid | hid | hid' \
@@ -34,10 +34,13 @@ merge_cmd () {
 
 translate_merge_tool_path() {
 	case "$1" in
-	gvimdiff|gvimdiff2|gvimdiff3)
+	nvimdiff*)
+		echo nvim
+		;;
+	gvimdiff*)
 		echo gvim
 		;;
-	vimdiff|vimdiff2|vimdiff3)
+	vimdiff*)
 		echo vim
 		;;
 	esac
@@ -48,7 +51,7 @@ exit_code_trustable () {
 }
 
 list_tool_variants () {
-	for prefix in '' g; do
+	for prefix in '' g n; do
 		for suffix in '' 2 3; do
 			echo "${prefix}vimdiff${suffix}"
 		done
-- 
2.28.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-07-29 21:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-18 19:20 [PATCH] Support nvim as mergetool pudinha
2020-07-18 20:30 ` Junio C Hamano
2020-07-19  4:23 ` [PATCH v2 0/2] Support nvim as merge tool pudinha
2020-07-29 21:31   ` [PATCH v3 0/2] mergetools: add support for nvimdiff (neovim) family pudinha
2020-07-29 21:31     ` [PATCH v3 1/2] mergetool--lib: improve support for vimdiff-style tool variants pudinha
2020-07-29 21:31     ` [PATCH v3 2/2] mergetools: add support for nvimdiff (neovim) family pudinha
2020-07-19  4:23 ` [PATCH v2 1/2] Refactor vimdiff and bc merge tool variants pudinha
2020-07-20 18:51   ` Junio C Hamano
2020-07-19  4:23 ` [PATCH v2 2/2] Support nvim as merge tool pudinha
2020-07-20 18:52   ` Junio C Hamano

Code repositories for project(s) associated with this 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).