From: Kipras Melnikovas <kipras@kipras.org>
To: git@vger.kernel.org
Cc: greenfoo@u92.eu, Kipras Melnikovas <kipras@kipras.org>
Subject: [PATCH v2] mergetools: vimdiff: use correct tool's name when reading mergetool config
Date: Thu, 15 Feb 2024 16:20:02 +0200 [thread overview]
Message-ID: <20240215142002.36870-1-kipras@kipras.org> (raw)
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
next prev parent reply other threads:[~2024-02-15 14:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-15 8:39 [PATCH] mergetools: vimdiff: use correct tool's name when reading mergetool config Kipras Melnikovas
2024-02-15 14:20 ` Kipras Melnikovas [this message]
2024-02-15 18:42 ` [PATCH v2] " 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
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=20240215142002.36870-1-kipras@kipras.org \
--to=kipras@kipras.org \
--cc=git@vger.kernel.org \
--cc=greenfoo@u92.eu \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).