git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mergetools: vimdiff: use correct tool's name when reading mergetool config
@ 2024-02-15  8:39 Kipras Melnikovas
  2024-02-15 14:20 ` [PATCH v2] " Kipras Melnikovas
  0 siblings, 1 reply; 8+ messages in thread
From: Kipras Melnikovas @ 2024-02-15  8:39 UTC (permalink / raw)
  To: git; +Cc: greenfoo, Kipras Melnikovas

I was curious why layout customizations, such as the multi-tab layout,
worked fine with vimdiff, but when changing to nvimdiff, it wouldn't anymore,
and instead showed the default view.

I was testing with the following config, everything was fine:

```conf
[merge]
	tool = vimdiff

[mergetool "vimdiff"]
	layout = local,base,remote / merged + base,local + base,remote + (local/base/remote),merged
```

```sh
git mergetool # opens vim w/ the custom 4-tab layout
```

But then I'd swap the tool to nvimdiff:

```diff
[merge]
-	tool = vimdiff
+	tool = nvimdiff

-[mergetool "vimdiff"]
+[mergetool "nvimdiff"]
	layout = local,base,remote / merged + base,local + base,remote + (local/base/remote),merged
```

and I'd get only the default 1-tab layout in neovim.

At first I thought that unlike vim,
neovim was somehow unable to launch multiple tabs.. Not the case.

Turns out, the /mergetools/vimdiff script, which handles both vimdiff, nvimdiff
and gvimdiff mergetools (the latter 2 simply source the vimdiff script), had a
function merge_cmd() which read the layout variable from git config, and it
would always read the value of mergetool.**vimdiff**.layout, instead of the
mergetool being currently used (vimdiff or nvimdiff or gvimdiff).

It looks like in 7b5cf8be18 (vimdiff: add tool documentation, 2022-03-30),
we explained the current behavior in Documentation/config/mergetool.txt:

---
mergetool.vimdiff.layout::
	The vimdiff backend uses this variable to control how its split
	windows look like. Applies even if you are using Neovim (`nvim`) or
	gVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section
---

which makes sense why it's explained this way - the vimdiff backend is used by
gvim and nvim. But the mergetool's configuration should be separate for each tool,
and indeed that's confirmed in same commit at Documentation/mergetools/vimdiff.txt:

---
Variants

Instead of `--tool=vimdiff`, you can also use one of these other variants:
  * `--tool=gvimdiff`, to open gVim instead of Vim.
  * `--tool=nvimdiff`, to open Neovim instead of Vim.

When using these variants, in order to specify a custom layout you will have to
set configuration variables `mergetool.gvimdiff.layout` and
`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout`
---

So it looks like we just forgot to update the 1 part of the vimdiff script
that read the config variable. Cheers.

Signed-off-by: Kipras Melnikovas <kipras@kipras.org>
---
 Documentation/config/mergetool.txt | 9 +++++----
 mergetools/vimdiff                 | 6 ++++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt
index 294f61efd1..8e3d321a57 100644
--- a/Documentation/config/mergetool.txt
+++ b/Documentation/config/mergetool.txt
@@ -45,10 +45,11 @@ mergetool.meld.useAutoMerge::
 	value of `false` avoids using `--auto-merge` altogether, and is the
 	default value.
 
-mergetool.vimdiff.layout::
-	The vimdiff backend uses this variable to control how its split
-	windows appear. Applies even if you are using Neovim (`nvim`) or
-	gVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section
+mergetool.{g,n,}vimdiff.layout::
+	The vimdiff backend uses this variable to control how its split windows
+	appear. Use `mergetool.vimdiff` for regular Vim, `mergetool.nvimdiff` for
+	Neovim and `mergetool.gvimdiff` for gVim to configure the merge tool. See
+	BACKEND SPECIFIC HINTS section
 ifndef::git-mergetool[]
 	in linkgit:git-mergetool[1].
 endif::[]
diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 06937acbf5..dd6bc411d9 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -371,9 +371,11 @@ diff_cmd_help () {
 
 
 merge_cmd () {
-	layout=$(git config mergetool.vimdiff.layout)
+	TOOL=$1
 
-	case "$1" in
+	layout=$(git config mergetool.$TOOL.layout)
+
+	case "$TOOL" in
 	*vimdiff)
 		if test -z "$layout"
 		then
-- 
2.43.1


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

end of thread, other threads:[~2024-02-21  5:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-15  8:39 [PATCH] mergetools: vimdiff: use correct tool's name when reading mergetool config Kipras Melnikovas
2024-02-15 14:20 ` [PATCH v2] " Kipras Melnikovas
2024-02-15 18:42   ` Junio C Hamano
2024-02-15 20:43   ` Fernando Ramos
2024-02-17  7:43   ` [PATCH v3] " Kipras Melnikovas
2024-02-17 16:27     ` [PATCH v4] " Kipras Melnikovas
2024-02-20  2:52       ` Junio C Hamano
2024-02-21  5:20         ` Kipras Melnikovas

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