git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] mergetools: vimdiff: use correct tool's name when reading mergetool config

From
Kipras Melnikovas <kipras@kipras.org>
Date
Feb 15, 2024, 14:20 UTC
Message-ID
<20240215142002.36870-1-kipras@kipras.org>
In-Reply-To
<20240215083917.98218-2-kipras@kipras.org>

The /mergetools/vimdiff script, which handles both vimdiff, nvimdiff and gvimdiff mergetools (the latter 2 simply source the vimdiff script), has 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.

Though, for backwards-compatibility, I've kept the mergetool.vimdiff fallback, so that people who unknowingly relied on it, won't have their setup broken now.

Signed-off-by: Kipras Melnikovas <kipras@kipras.org>
---
Range-diff against v1:
1:  197e42deef ! 1:  070280d95d mergetools: vimdiff: use correct tool's name when reading mergetool config
    @@ Metadata
      ## Commit message ##
         So it looks like we just forgot to update the 1 part of the vimdiff script
         that read the config variable. Cheers.
     
    +    Though, for backwards-compatibility, I've kept the mergetool.vimdiff
    +    fallback, so that people who unknowingly relied on it, won't have their
    +    setup broken now.
    +
         Signed-off-by: Kipras Melnikovas <kipras@kipras.org>
     
    @@ mergetools/vimdiff: diff_cmd_help () {
     -	case "$1" in
     +	layout=$(git config mergetool.$TOOL.layout)
     +
    ++	# backwards-compatibility:
    ++	if test -z "$layout"
    ++	then
    ++		layout=$(git config mergetool.vimdiff.layout)
    ++	fi
    ++
     +	case "$TOOL" in
      	*vimdiff)
      		if test -z "$layout"
 Documentation/config/mergetool.txt |  9 +++++----
 mergetools/vimdiff                 | 12 ++++++++++--
 2 files changed, 15 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..0e3058868a 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -371,9 +371,17 @@ diff_cmd_help () {
 
 
 merge_cmd () {
-	layout=$(git config mergetool.vimdiff.layout)
+	TOOL=$1
 
-	case "$1" in
+	layout=$(git config mergetool.$TOOL.layout)
+
+	# backwards-compatibility:
+	if test -z "$layout"
+	then
+		layout=$(git config mergetool.vimdiff.layout)
+	fi
+
+	case "$TOOL" in
 	*vimdiff)
 		if test -z "$layout"
 		then

base-commit: 4fc51f00ef18d2c0174ab2fd39d0ee473fd144bd
-- 
2.43.1
Previous: Kipras MelnikovasNext: Junio C Hamano
Message 2 of 8 in “mergetools: vimdiff: use correct tool's name when reading mergetool config”
  1. mergetools: vimdiff: use correct tool's name when reading mergetool configKipras Melnikovas, Feb 15, 2024
  2. mergetools: vimdiff: use correct tool's name when reading mergetool configKipras Melnikovas, Feb 15, 2024
  3. Junio C HamanoFeb 15, 2024
  4. Fernando RamosFeb 15, 2024
  5. mergetools: vimdiff: use correct tool's name when reading mergetool configKipras Melnikovas, Feb 17, 2024
  6. mergetools: vimdiff: use correct tool's name when reading mergetool configKipras Melnikovas, Feb 17, 2024
  7. Junio C HamanoFeb 20, 2024
  8. Kipras MelnikovasFeb 21, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.