{"thread":{"id":"60924","subject":"[PATCH] mergetools: vimdiff: use correct tool's name when reading mergetool config","startedAt":"2024-02-15T09:01:00Z","lastAt":"2024-02-21T05:20:57Z","messageCount":8,"participants":["Kipras Melnikovas","Junio C Hamano","Fernando Ramos"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"488704","messageId":"20240215083917.98218-2-kipras@kipras.org","threadId":"60924","inReplyTo":null,"subject":"[PATCH] mergetools: vimdiff: use correct tool's name when reading mergetool config","fromName":"Kipras Melnikovas","fromEmail":"kipras@kipras.org","sentAt":"2024-02-15T08:39:18Z","receivedAt":"2024-02-15T09:01:00Z","isPatch":true,"sender":{"key":"kipras@kipras.org","avatar":"https://avatars.githubusercontent.com/u/29430509?v=4"},"body":"I was curious why layout customizations, such as the multi-tab layout,\nworked fine with vimdiff, but when changing to nvimdiff, it wouldn't anymore,\nand instead showed the default view.\n\nI was testing with the following config, everything was fine:\n\n```conf\n[merge]\n\ttool = vimdiff\n\n[mergetool \"vimdiff\"]\n\tlayout = local,base,remote / merged + base,local + base,remote + (local/base/remote),merged\n```\n\n```sh\ngit mergetool # opens vim w/ the custom 4-tab layout\n```\n\nBut then I'd swap the tool to nvimdiff:\n\n```diff\n[merge]\n-\ttool = vimdiff\n+\ttool = nvimdiff\n\n-[mergetool \"vimdiff\"]\n+[mergetool \"nvimdiff\"]\n\tlayout = local,base,remote / merged + base,local + base,remote + (local/base/remote),merged\n```\n\nand I'd get only the default 1-tab layout in neovim.\n\nAt first I thought that unlike vim,\nneovim was somehow unable to launch multiple tabs.. Not the case.\n\nTurns out, the /mergetools/vimdiff script, which handles both vimdiff, nvimdiff\nand gvimdiff mergetools (the latter 2 simply source the vimdiff script), had a\nfunction merge_cmd() which read the layout variable from git config, and it\nwould always read the value of mergetool.**vimdiff**.layout, instead of the\nmergetool being currently used (vimdiff or nvimdiff or gvimdiff).\n\nIt looks like in 7b5cf8be18 (vimdiff: add tool documentation, 2022-03-30),\nwe explained the current behavior in Documentation/config/mergetool.txt:\n\n---\nmergetool.vimdiff.layout::\n\tThe vimdiff backend uses this variable to control how its split\n\twindows look like. Applies even if you are using Neovim (`nvim`) or\n\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n---\n\nwhich makes sense why it's explained this way - the vimdiff backend is used by\ngvim and nvim. But the mergetool's configuration should be separate for each tool,\nand indeed that's confirmed in same commit at Documentation/mergetools/vimdiff.txt:\n\n---\nVariants\n\nInstead of `--tool=vimdiff`, you can also use one of these other variants:\n  * `--tool=gvimdiff`, to open gVim instead of Vim.\n  * `--tool=nvimdiff`, to open Neovim instead of Vim.\n\nWhen using these variants, in order to specify a custom layout you will have to\nset configuration variables `mergetool.gvimdiff.layout` and\n`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout`\n---\n\nSo it looks like we just forgot to update the 1 part of the vimdiff script\nthat read the config variable. Cheers.\n\nSigned-off-by: Kipras Melnikovas <kipras@kipras.org>\n---\n Documentation/config/mergetool.txt | 9 +++++----\n mergetools/vimdiff                 | 6 ++++--\n 2 files changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 294f61efd1..8e3d321a57 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -45,10 +45,11 @@ mergetool.meld.useAutoMerge::\n \tvalue of `false` avoids using `--auto-merge` altogether, and is the\n \tdefault value.\n \n-mergetool.vimdiff.layout::\n-\tThe vimdiff backend uses this variable to control how its split\n-\twindows appear. Applies even if you are using Neovim (`nvim`) or\n-\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n+mergetool.{g,n,}vimdiff.layout::\n+\tThe vimdiff backend uses this variable to control how its split windows\n+\tappear. Use `mergetool.vimdiff` for regular Vim, `mergetool.nvimdiff` for\n+\tNeovim and `mergetool.gvimdiff` for gVim to configure the merge tool. See\n+\tBACKEND SPECIFIC HINTS section\n ifndef::git-mergetool[]\n \tin linkgit:git-mergetool[1].\n endif::[]\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex 06937acbf5..dd6bc411d9 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -371,9 +371,11 @@ diff_cmd_help () {\n \n \n merge_cmd () {\n-\tlayout=$(git config mergetool.vimdiff.layout)\n+\tTOOL=$1\n \n-\tcase \"$1\" in\n+\tlayout=$(git config mergetool.$TOOL.layout)\n+\n+\tcase \"$TOOL\" in\n \t*vimdiff)\n \t\tif test -z \"$layout\"\n \t\tthen\n-- \n2.43.1\n\n"},{"id":"488738","messageId":"20240215142002.36870-1-kipras@kipras.org","threadId":"60924","inReplyTo":"20240215083917.98218-2-kipras@kipras.org","subject":"[PATCH v2] mergetools: vimdiff: use correct tool's name when reading mergetool config","fromName":"Kipras Melnikovas","fromEmail":"kipras@kipras.org","sentAt":"2024-02-15T14:20:02Z","receivedAt":"2024-02-15T14:21:10Z","isPatch":true,"sender":{"key":"kipras@kipras.org","avatar":"https://avatars.githubusercontent.com/u/29430509?v=4"},"body":"The /mergetools/vimdiff script, which handles both vimdiff, nvimdiff\nand gvimdiff mergetools (the latter 2 simply source the vimdiff script), has a\nfunction merge_cmd() which read the layout variable from git config, and it\nwould always read the value of mergetool.**vimdiff**.layout, instead of the\nmergetool being currently used (vimdiff or nvimdiff or gvimdiff).\n\nIt looks like in 7b5cf8be18 (vimdiff: add tool documentation, 2022-03-30),\nwe explained the current behavior in Documentation/config/mergetool.txt:\n\n---\nmergetool.vimdiff.layout::\n\tThe vimdiff backend uses this variable to control how its split\n\twindows look like. Applies even if you are using Neovim (`nvim`) or\n\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n---\n\nwhich makes sense why it's explained this way - the vimdiff backend is used by\ngvim and nvim. But the mergetool's configuration should be separate for each tool,\nand indeed that's confirmed in same commit at Documentation/mergetools/vimdiff.txt:\n\n---\nVariants\n\nInstead of `--tool=vimdiff`, you can also use one of these other variants:\n  * `--tool=gvimdiff`, to open gVim instead of Vim.\n  * `--tool=nvimdiff`, to open Neovim instead of Vim.\n\nWhen using these variants, in order to specify a custom layout you will have to\nset configuration variables `mergetool.gvimdiff.layout` and\n`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout`\n---\n\nSo it looks like we just forgot to update the 1 part of the vimdiff script\nthat read the config variable. Cheers.\n\nThough, for backwards-compatibility, I've kept the mergetool.vimdiff\nfallback, so that people who unknowingly relied on it, won't have their\nsetup broken now.\n\nSigned-off-by: Kipras Melnikovas <kipras@kipras.org>\n---\nRange-diff against v1:\n1:  197e42deef ! 1:  070280d95d mergetools: vimdiff: use correct tool's name when reading mergetool config\n    @@ Metadata\n      ## Commit message ##\n         So it looks like we just forgot to update the 1 part of the vimdiff script\n         that read the config variable. Cheers.\n     \n    +    Though, for backwards-compatibility, I've kept the mergetool.vimdiff\n    +    fallback, so that people who unknowingly relied on it, won't have their\n    +    setup broken now.\n    +\n         Signed-off-by: Kipras Melnikovas <kipras@kipras.org>\n     \n    @@ mergetools/vimdiff: diff_cmd_help () {\n     -\tcase \"$1\" in\n     +\tlayout=$(git config mergetool.$TOOL.layout)\n     +\n    ++\t# backwards-compatibility:\n    ++\tif test -z \"$layout\"\n    ++\tthen\n    ++\t\tlayout=$(git config mergetool.vimdiff.layout)\n    ++\tfi\n    ++\n     +\tcase \"$TOOL\" in\n      \t*vimdiff)\n      \t\tif test -z \"$layout\"\n\n Documentation/config/mergetool.txt |  9 +++++----\n mergetools/vimdiff                 | 12 ++++++++++--\n 2 files changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 294f61efd1..8e3d321a57 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -45,10 +45,11 @@ mergetool.meld.useAutoMerge::\n \tvalue of `false` avoids using `--auto-merge` altogether, and is the\n \tdefault value.\n \n-mergetool.vimdiff.layout::\n-\tThe vimdiff backend uses this variable to control how its split\n-\twindows appear. Applies even if you are using Neovim (`nvim`) or\n-\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n+mergetool.{g,n,}vimdiff.layout::\n+\tThe vimdiff backend uses this variable to control how its split windows\n+\tappear. Use `mergetool.vimdiff` for regular Vim, `mergetool.nvimdiff` for\n+\tNeovim and `mergetool.gvimdiff` for gVim to configure the merge tool. See\n+\tBACKEND SPECIFIC HINTS section\n ifndef::git-mergetool[]\n \tin linkgit:git-mergetool[1].\n endif::[]\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex 06937acbf5..0e3058868a 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -371,9 +371,17 @@ diff_cmd_help () {\n \n \n merge_cmd () {\n-\tlayout=$(git config mergetool.vimdiff.layout)\n+\tTOOL=$1\n \n-\tcase \"$1\" in\n+\tlayout=$(git config mergetool.$TOOL.layout)\n+\n+\t# backwards-compatibility:\n+\tif test -z \"$layout\"\n+\tthen\n+\t\tlayout=$(git config mergetool.vimdiff.layout)\n+\tfi\n+\n+\tcase \"$TOOL\" in\n \t*vimdiff)\n \t\tif test -z \"$layout\"\n \t\tthen\n\nbase-commit: 4fc51f00ef18d2c0174ab2fd39d0ee473fd144bd\n-- \n2.43.1\n\n"},{"id":"488749","messageId":"xmqq8r3lr2l2.fsf@gitster.g","threadId":"60924","inReplyTo":"20240215142002.36870-1-kipras@kipras.org","subject":"Re: [PATCH v2] mergetools: vimdiff: use correct tool's name when reading mergetool config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-15T18:42:49Z","receivedAt":"2024-02-15T18:42:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kipras Melnikovas <kipras@kipras.org> writes:\n\n> Though, for backwards-compatibility, I've kept the mergetool.vimdiff\n> fallback, so that people who unknowingly relied on it, won't have their\n> setup broken now.\n\nIt is a good consideration, and should be documented ...\n\n> diff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\n> index 294f61efd1..8e3d321a57 100644\n> --- a/Documentation/config/mergetool.txt\n> +++ b/Documentation/config/mergetool.txt\n> @@ -45,10 +45,11 @@ mergetool.meld.useAutoMerge::\n>  \tvalue of `false` avoids using `--auto-merge` altogether, and is the\n>  \tdefault value.\n>  \n> -mergetool.vimdiff.layout::\n> -\tThe vimdiff backend uses this variable to control how its split\n> -\twindows appear. Applies even if you are using Neovim (`nvim`) or\n> -\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n> +mergetool.{g,n,}vimdiff.layout::\n> +\tThe vimdiff backend uses this variable to control how its split windows\n> +\tappear. Use `mergetool.vimdiff` for regular Vim, `mergetool.nvimdiff` for\n> +\tNeovim and `mergetool.gvimdiff` for gVim to configure the merge tool. See\n> +\tBACKEND SPECIFIC HINTS section\n\n... perhaps before \"See BACKEND SPECIFIC HINTS section.\"  E.g.\n\n\tWhen a variant of vimdiff (vim, Neovim, or gVim) is used as\n\ta mergetool backend, they use this variable to control how\n\tthe split windows appear.\n\tThe variable `mergetool.<variant>.layout` (where <variant>\n\tis one of `vimdiff`, `nvimdiff`, or `gvimdiff`, depending on\n\twhat you are using) is consulted first, and if it is missing,\n\t`mergetool.vimdiff.layout` is used as a fallback.  See\n\tBACKEND SPECIFIC HINTS section.\n\nor something?\t\n\n\n> diff --git a/mergetools/vimdiff b/mergetools/vimdiff\n> index 06937acbf5..0e3058868a 100644\n> --- a/mergetools/vimdiff\n> +++ b/mergetools/vimdiff\n> @@ -371,9 +371,17 @@ diff_cmd_help () {\n>  \n>  \n>  merge_cmd () {\n> -\tlayout=$(git config mergetool.vimdiff.layout)\n> +\tTOOL=$1\n>  \n> -\tcase \"$1\" in\n> +\tlayout=$(git config mergetool.$TOOL.layout)\n\nThe callers of merge_cmd are careful to do\n\n\tmerge_cmd \"$1\"\n\nso it would be a good hygiene to also quote $TOOL here, i.e.\n\n\tlayout=$(git config \"mergetool.$TOOL.layout\")\n\nIt might not matter if the caller of run_merge_cmd (which calls\nmerge_cmd) eventually chooses from a known set of strings hardcoded\nin mergetools--lib.sh, but it is much easier to show that you are\ndoing the right thing without relying on such a detail of what\nhappens far in the code to quote what you get from the caller\nappropriately.\n\n> +\n> +\t# backwards-compatibility:\n> +\tif test -z \"$layout\"\n> +\tthen\n> +\t\tlayout=$(git config mergetool.vimdiff.layout)\n> +\tfi\n> +\n> +\tcase \"$TOOL\" in\n\nThis one is quoted properly (and TOOL=$1 at the beginning does not\nrequire quoting).  The \"git config\" call above is the only one that\nneeds to be fixed.\n\nThanks.\n\n>  \t*vimdiff)\n>  \t\tif test -z \"$layout\"\n>  \t\tthen\n>\n> base-commit: 4fc51f00ef18d2c0174ab2fd39d0ee473fd144bd\n"},{"id":"488756","messageId":"Zc53XnK1TJVQNjOK@zacax395.localdomain","threadId":"60924","inReplyTo":"20240215142002.36870-1-kipras@kipras.org","subject":"Re: [PATCH v2] mergetools: vimdiff: use correct tool's name when reading mergetool config","fromName":"Fernando Ramos","fromEmail":"greenfoo@u92.eu","sentAt":"2024-02-15T20:43:10Z","receivedAt":"2024-02-15T20:43:16Z","isPatch":true,"sender":{"key":"greenfoo@u92.eu","avatar":"https://avatars.githubusercontent.com/u/42691086?v=4"},"body":"On 24/02/15 04:20PM, Kipras Melnikovas wrote:\n> The /mergetools/vimdiff script, which handles both vimdiff, nvimdiff\n> and gvimdiff mergetools (the latter 2 simply source the vimdiff script), has a\n> function merge_cmd() which read the layout variable from git config, and it\n> would always read the value of mergetool.**vimdiff**.layout, instead of the\n> mergetool being currently used (vimdiff or nvimdiff or gvimdiff).\n\nYou are 100% right. I completely overlooked this detail in the original\nimplementation.\n\nThanks for the fix!\n\n"},{"id":"488845","messageId":"20240217074343.12608-1-kipras@kipras.org","threadId":"60924","inReplyTo":"20240215142002.36870-1-kipras@kipras.org","subject":"[PATCH v3] mergetools: vimdiff: use correct tool's name when reading mergetool config","fromName":"Kipras Melnikovas","fromEmail":"kipras@kipras.org","sentAt":"2024-02-17T07:43:43Z","receivedAt":"2024-02-17T07:44:23Z","isPatch":true,"sender":{"key":"kipras@kipras.org","avatar":"https://avatars.githubusercontent.com/u/29430509?v=4"},"body":"The /mergetools/vimdiff script, which handles both vimdiff, nvimdiff\nand gvimdiff mergetools (the latter 2 simply source the vimdiff script), has a\nfunction merge_cmd() which read the layout variable from git config, and it\nwould always read the value of mergetool.**vimdiff**.layout, instead of the\nmergetool being currently used (vimdiff or nvimdiff or gvimdiff).\n\nIt looks like in 7b5cf8be18 (vimdiff: add tool documentation, 2022-03-30),\nwe explained the current behavior in Documentation/config/mergetool.txt:\n\n```\nmergetool.vimdiff.layout::\n\tThe vimdiff backend uses this variable to control how its split\n\twindows look like. Applies even if you are using Neovim (`nvim`) or\n\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n```\n\nwhich makes sense why it's explained this way - the vimdiff backend is used by\ngvim and nvim. But the mergetool's configuration should be separate for each tool,\nand indeed that's confirmed in same commit at Documentation/mergetools/vimdiff.txt:\n\n```\nVariants\n\nInstead of `--tool=vimdiff`, you can also use one of these other variants:\n  * `--tool=gvimdiff`, to open gVim instead of Vim.\n  * `--tool=nvimdiff`, to open Neovim instead of Vim.\n\nWhen using these variants, in order to specify a custom layout you will have to\nset configuration variables `mergetool.gvimdiff.layout` and\n`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout`\n```\n\nSo it looks like we just forgot to update the 1 part of the vimdiff script\nthat read the config variable. Cheers.\n\nThough, for backward compatibility, I've kept the mergetool.vimdiff\nfallback, so that people who unknowingly relied on it, won't have their\nsetup broken now.\n\nSigned-off-by: Kipras Melnikovas <kipras@kipras.org>\n---\n\nHere's some variants I considered for mergetool.<vimdiff variant>.layout\nin Documentation/config/mergetool.txt, but discarded for a shorter\nversion. Feel free to pick & edit the final.\n\na)\n\tThe `<vimdiff variant>` is any of `vimdiff`, `nvimdiff`, `gvimdiff`. When\n\tyou run `git mergetool` with `--tool=<vimdiff variant>`, Git will consult\n\t`mergetool.<vimdiff variant>.layout` to determine the tool's layout. If it's\n\tnot specified, `vimdiff`'s is used as fallback. If that too is not available,\n\ta default layout with 4 windows is used. See BACKEND SPECIFIC HINTS section\nifndef::git-mergetool[]\n\tin linkgit:git-mergetool[1]\nendif::[]\n\tfor details.\n\nb)\n\tConfigure a custom layout for your mergetool. The `<variant>` is any\n\tof `vimdiff`, `nvimdiff`, `gvimdiff`.\n\n\tUpon launching `git mergetool` with `--tool=<variant>` (or without\n\t`--tool` if `merge.tool` is configured as `<variant>`), Git\n\twill consult `mergetool.<vimdiff variant>.layout` to determine the tool's\n\tlayout.  If the variant-specific config is not available, `vimdiff`'s is\n\tused as fallback. If that too is not available, a default layout with 4\n\twindows will be used. See BACKEND SPECIFIC HINTS section\nifndef::git-mergetool[]\n\tin linkgit:git-mergetool[1]\nendif::[]\n\tfor details.\n\n\nThe ifdef + ifndef is used to avoid an extra space before the final \".\"\n\n\nRange-diff against v2:\n1:  070280d95d ! 1:  60be87c3d5 mergetools: vimdiff: use correct tool's name when reading mergetool config\n    @@ Documentation/config/mergetool.txt: mergetool.meld.useAutoMerge::\n     -\tThe vimdiff backend uses this variable to control how its split\n     -\twindows appear. Applies even if you are using Neovim (`nvim`) or\n     -\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n    -+mergetool.{g,n,}vimdiff.layout::\n    -+\tThe vimdiff backend uses this variable to control how its split windows\n    -+\tappear. Use `mergetool.vimdiff` for regular Vim, `mergetool.nvimdiff` for\n    -+\tNeovim and `mergetool.gvimdiff` for gVim to configure the merge tool. See\n    -+\tBACKEND SPECIFIC HINTS section\n    - ifndef::git-mergetool[]\n    - \tin linkgit:git-mergetool[1].\n    +-ifndef::git-mergetool[]\n    +-\tin linkgit:git-mergetool[1].\n    ++mergetool.<vimdiff variant>.layout::\n    ++\tGit's vimdiff backend uses this variable to control how the split windows of\n    ++\t`<vimdiff variant>` appear. Here `<vimdiff variant>` is any of `vimdiff`,\n    ++\t`nvimdiff`, `gvimdiff`. To configure the layout and use the tool, see the\n    ++\t`BACKEND SPECIFIC HINTS`\n    ++ifdef::git-mergetool[]\n    ++\tsection.\n    ++endif::[]\n    ++ifndef::git-mergetool[]\n    ++\tsection in linkgit:git-mergetool[1].\n      endif::[]\n    +-\tfor details.\n    + \n    + mergetool.hideResolved::\n    + \tDuring a merge, Git will automatically resolve as many conflicts as\n     \n      ## mergetools/vimdiff ##\n     @@ mergetools/vimdiff: diff_cmd_help () {\n    @@ mergetools/vimdiff: diff_cmd_help () {\n     +\tTOOL=$1\n      \n     -\tcase \"$1\" in\n    -+\tlayout=$(git config mergetool.$TOOL.layout)\n    ++\tlayout=$(git config \"mergetool.$TOOL.layout\")\n     +\n    -+\t# backwards-compatibility:\n    ++\t# backward compatibility:\n     +\tif test -z \"$layout\"\n     +\tthen\n     +\t\tlayout=$(git config mergetool.vimdiff.layout)\n\n Documentation/config/mergetool.txt | 17 ++++++++++-------\n mergetools/vimdiff                 | 12 ++++++++++--\n 2 files changed, 20 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 294f61efd1..f79c798b74 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -45,14 +45,17 @@ mergetool.meld.useAutoMerge::\n \tvalue of `false` avoids using `--auto-merge` altogether, and is the\n \tdefault value.\n \n-mergetool.vimdiff.layout::\n-\tThe vimdiff backend uses this variable to control how its split\n-\twindows appear. Applies even if you are using Neovim (`nvim`) or\n-\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n-ifndef::git-mergetool[]\n-\tin linkgit:git-mergetool[1].\n+mergetool.<vimdiff variant>.layout::\n+\tGit's vimdiff backend uses this variable to control how the split windows of\n+\t`<vimdiff variant>` appear. Here `<vimdiff variant>` is any of `vimdiff`,\n+\t`nvimdiff`, `gvimdiff`. To configure the layout and use the tool, see the\n+\t`BACKEND SPECIFIC HINTS`\n+ifdef::git-mergetool[]\n+\tsection.\n+endif::[]\n+ifndef::git-mergetool[]\n+\tsection in linkgit:git-mergetool[1].\n endif::[]\n-\tfor details.\n \n mergetool.hideResolved::\n \tDuring a merge, Git will automatically resolve as many conflicts as\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex 06937acbf5..97e376329b 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -371,9 +371,17 @@ diff_cmd_help () {\n \n \n merge_cmd () {\n-\tlayout=$(git config mergetool.vimdiff.layout)\n+\tTOOL=$1\n \n-\tcase \"$1\" in\n+\tlayout=$(git config \"mergetool.$TOOL.layout\")\n+\n+\t# backward compatibility:\n+\tif test -z \"$layout\"\n+\tthen\n+\t\tlayout=$(git config mergetool.vimdiff.layout)\n+\tfi\n+\n+\tcase \"$TOOL\" in\n \t*vimdiff)\n \t\tif test -z \"$layout\"\n \t\tthen\n\nbase-commit: 3e0d3cd5c7def4808247caf168e17f2bbf47892b\n-- \n2.43.1\n\n"},{"id":"488848","messageId":"20240217162718.21272-1-kipras@kipras.org","threadId":"60924","inReplyTo":"20240217074343.12608-1-kipras@kipras.org","subject":"[PATCH v4] mergetools: vimdiff: use correct tool's name when reading mergetool config","fromName":"Kipras Melnikovas","fromEmail":"kipras@kipras.org","sentAt":"2024-02-17T16:27:18Z","receivedAt":"2024-02-17T16:28:08Z","isPatch":true,"sender":{"key":"kipras@kipras.org","avatar":"https://avatars.githubusercontent.com/u/29430509?v=4"},"body":"The /mergetools/vimdiff script, which handles both vimdiff, nvimdiff\nand gvimdiff mergetools (the latter 2 simply source the vimdiff script), has a\nfunction merge_cmd() which read the layout variable from git config, and it\nwould always read the value of mergetool.**vimdiff**.layout, instead of the\nmergetool being currently used (vimdiff or nvimdiff or gvimdiff).\n\nIt looks like in 7b5cf8be18 (vimdiff: add tool documentation, 2022-03-30),\nwe explained the current behavior in Documentation/config/mergetool.txt:\n\n```\nmergetool.vimdiff.layout::\n\tThe vimdiff backend uses this variable to control how its split\n\twindows look like. Applies even if you are using Neovim (`nvim`) or\n\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n```\n\nwhich makes sense why it's explained this way - the vimdiff backend is used by\ngvim and nvim. But the mergetool's configuration should be separate for each tool,\nand indeed that's confirmed in same commit at Documentation/mergetools/vimdiff.txt:\n\n```\nVariants\n\nInstead of `--tool=vimdiff`, you can also use one of these other variants:\n  * `--tool=gvimdiff`, to open gVim instead of Vim.\n  * `--tool=nvimdiff`, to open Neovim instead of Vim.\n\nWhen using these variants, in order to specify a custom layout you will have to\nset configuration variables `mergetool.gvimdiff.layout` and\n`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout`\n```\n\nSo it looks like we just forgot to update the 1 part of the vimdiff script\nthat read the config variable. Cheers.\n\nThough, for backward compatibility, I've kept the mergetool.vimdiff\nfallback, so that people who unknowingly relied on it, won't have their\nsetup broken now.\n\nSigned-off-by: Kipras Melnikovas <kipras@kipras.org>\n---\n\nOkay I've finalised the documentation, should be the last patch.\nAlso I realise I've forgotten to cc the mailing list on my replies to\nJunio and Fernando - sorry! First time..\n\nRange-diff against v3:\n1:  0018c7e18c = 1:  0018c7e18c mergetools: vimdiff: use correct tool's name when reading mergetool config\n\n Documentation/config/mergetool.txt   | 21 ++++++++++++++-------\n Documentation/mergetools/vimdiff.txt |  3 ++-\n mergetools/vimdiff                   | 12 ++++++++++--\n 3 files changed, 26 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config/mergetool.txt b/Documentation/config/mergetool.txt\nindex 294f61efd1..00bf665aa0 100644\n--- a/Documentation/config/mergetool.txt\n+++ b/Documentation/config/mergetool.txt\n@@ -45,14 +45,21 @@ mergetool.meld.useAutoMerge::\n \tvalue of `false` avoids using `--auto-merge` altogether, and is the\n \tdefault value.\n \n-mergetool.vimdiff.layout::\n-\tThe vimdiff backend uses this variable to control how its split\n-\twindows appear. Applies even if you are using Neovim (`nvim`) or\n-\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n-ifndef::git-mergetool[]\n-\tin linkgit:git-mergetool[1].\n+mergetool.<vimdiff variant>.layout::\n+\tConfigure the split window layout for vimdiff's `<variant>`, which is any of `vimdiff`,\n+\t`nvimdiff`, `gvimdiff`.\n+\tUpon launching `git mergetool` with `--tool=<variant>` (or without `--tool`\n+\tif `merge.tool` is configured as `<variant>`), Git will consult\n+\t`mergetool.<variant>.layout` to determine the tool's layout. If the\n+\tvariant-specific configuration is not available, `vimdiff`'s is used as\n+\tfallback.  If that too is not available, a default layout with 4 windows\n+\twill be used.  To configure the layout, see the `BACKEND SPECIFIC HINTS`\n+ifdef::git-mergetool[]\n+\tsection.\n+endif::[]\n+ifndef::git-mergetool[]\n+\tsection in linkgit:git-mergetool[1].\n endif::[]\n-\tfor details.\n \n mergetool.hideResolved::\n \tDuring a merge, Git will automatically resolve as many conflicts as\ndiff --git a/Documentation/mergetools/vimdiff.txt b/Documentation/mergetools/vimdiff.txt\nindex d1a4c468e6..befa86d692 100644\n--- a/Documentation/mergetools/vimdiff.txt\n+++ b/Documentation/mergetools/vimdiff.txt\n@@ -177,7 +177,8 @@ Instead of `--tool=vimdiff`, you can also use one of these other variants:\n \n When using these variants, in order to specify a custom layout you will have to\n set configuration variables `mergetool.gvimdiff.layout` and\n-`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout`\n+`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout` (though the\n+latter will be used as fallback if the variant-specific one is not set).\n \n In addition, for backwards compatibility with previous Git versions, you can\n also append `1`, `2` or `3` to either `vimdiff` or any of the variants (ex:\ndiff --git a/mergetools/vimdiff b/mergetools/vimdiff\nindex 06937acbf5..97e376329b 100644\n--- a/mergetools/vimdiff\n+++ b/mergetools/vimdiff\n@@ -371,9 +371,17 @@ diff_cmd_help () {\n \n \n merge_cmd () {\n-\tlayout=$(git config mergetool.vimdiff.layout)\n+\tTOOL=$1\n \n-\tcase \"$1\" in\n+\tlayout=$(git config \"mergetool.$TOOL.layout\")\n+\n+\t# backward compatibility:\n+\tif test -z \"$layout\"\n+\tthen\n+\t\tlayout=$(git config mergetool.vimdiff.layout)\n+\tfi\n+\n+\tcase \"$TOOL\" in\n \t*vimdiff)\n \t\tif test -z \"$layout\"\n \t\tthen\n\nbase-commit: 3e0d3cd5c7def4808247caf168e17f2bbf47892b\n-- \n2.43.1\n\n"},{"id":"488953","messageId":"xmqq8r3fyhhg.fsf@gitster.g","threadId":"60924","inReplyTo":"20240217162718.21272-1-kipras@kipras.org","subject":"Re: [PATCH v4] mergetools: vimdiff: use correct tool's name when reading mergetool config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-02-20T02:52:43Z","receivedAt":"2024-02-20T02:52:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kipras Melnikovas <kipras@kipras.org> writes:\n\n> Okay I've finalised the documentation, should be the last patch.\n> Also I realise I've forgotten to cc the mailing list on my replies to\n> Junio and Fernando - sorry! First time..\n>\n> Range-diff against v3:\n> 1:  0018c7e18c = 1:  0018c7e18c mergetools: vimdiff: use correct tool's name when reading mergetool config\n>\n>  Documentation/config/mergetool.txt   | 21 ++++++++++++++-------\n>  Documentation/mergetools/vimdiff.txt |  3 ++-\n>  mergetools/vimdiff                   | 12 ++++++++++--\n>  3 files changed, 26 insertions(+), 10 deletions(-)\n\nThat's curious.  This is v4 and no changes from v3?\n\n"},{"id":"489050","messageId":"20240221052041.53239-1-kipras@kipras.org","threadId":"60924","inReplyTo":"xmqq8r3fyhhg.fsf@gitster.g","subject":"Re: [PATCH v4] mergetools: vimdiff: use correct tool's name when reading mergetool config","fromName":"Kipras Melnikovas","fromEmail":"kipras@kipras.org","sentAt":"2024-02-21T05:20:41Z","receivedAt":"2024-02-21T05:20:57Z","isPatch":true,"sender":{"key":"kipras@kipras.org","avatar":"https://avatars.githubusercontent.com/u/29430509?v=4"},"body":"On 24/02/19 06:52PM, Junio C Hamano wrote:\n\n> That's curious.  This is v4 and no changes from v3?\n\nMy bad. The patches are slightly different [1], I just managed to screw up the\nrange-diff args [2]. Won't happen again [3].\n\nSorry, I know this is wasting your time; this whole submission should've\nbeen a single reroll not 3. I'll be more thorough next time.\n\nThank you for your time, help and quick responses.\n\nKipras\n\n---\n\n[3] https://github.com/kiprasmel/git-reroll/commit/3aa14a2860b19f86bb51f70ce0d5cec99a5aca59#diff-59ce06aeaf7a77de81c8f24c0301d95d017a83fdaec8bc1dce9877af708d1ad3R213-R227\n\n[2]\nWhat essentially happened was: I marked a new checkpoint \"v4\" *before*\nsending the email, and ran format-patch with --range-diff \"<checkpoint>..@\",\nwhich of course was \"v4 against v4\" instead of \"v3 against v4\"..\n\n[1] fwiw,\nRange-diff against v3:\n1:  60be87c3d5 ! 1:  0018c7e18c mergetools: vimdiff: use correct tool's name when reading mergetool config\n    @@ Documentation/config/mergetool.txt: mergetool.meld.useAutoMerge::\n     -\twindows appear. Applies even if you are using Neovim (`nvim`) or\n     -\tgVim (`gvim`) as the merge tool. See BACKEND SPECIFIC HINTS section\n     +mergetool.<vimdiff variant>.layout::\n    -+\tGit's vimdiff backend uses this variable to control how the split windows of\n    -+\t`<vimdiff variant>` appear. Here `<vimdiff variant>` is any of `vimdiff`,\n    -+\t`nvimdiff`, `gvimdiff`. To configure the layout and use the tool, see the\n    -+\t`BACKEND SPECIFIC HINTS`\n    ++\tConfigure the split window layout for vimdiff's `<variant>`, which is any of `vimdiff`,\n    ++\t`nvimdiff`, `gvimdiff`.\n    ++\tUpon launching `git mergetool` with `--tool=<variant>` (or without `--tool`\n    ++\tif `merge.tool` is configured as `<variant>`), Git will consult\n    ++\t`mergetool.<variant>.layout` to determine the tool's layout. If the\n    ++\tvariant-specific configuration is not available, `vimdiff`'s is used as\n    ++\tfallback.  If that too is not available, a default layout with 4 windows\n    ++\twill be used.  To configure the layout, see the `BACKEND SPECIFIC HINTS`\n     +ifdef::git-mergetool[]\n     +\tsection.\n     +endif::[]\n    @@ Documentation/config/mergetool.txt: mergetool.meld.useAutoMerge::\n      mergetool.hideResolved::\n      \tDuring a merge, Git will automatically resolve as many conflicts as\n     \n    + ## Documentation/mergetools/vimdiff.txt ##\n    +@@ Documentation/mergetools/vimdiff.txt: Instead of `--tool=vimdiff`, you can also use one of these other variants:\n    + \n    + When using these variants, in order to specify a custom layout you will have to\n    + set configuration variables `mergetool.gvimdiff.layout` and\n    +-`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout`\n    ++`mergetool.nvimdiff.layout` instead of `mergetool.vimdiff.layout` (though the\n    ++latter will be used as fallback if the variant-specific one is not set).\n    + \n    + In addition, for backwards compatibility with previous Git versions, you can\n    + also append `1`, `2` or `3` to either `vimdiff` or any of the variants (ex:\n    +\n      ## mergetools/vimdiff ##\n     @@ mergetools/vimdiff: diff_cmd_help () {\n\n\n Documentation/config/mergetool.txt   | 19 +++++++++++++------\n Documentation/mergetools/vimdiff.txt |  3 ++-\n mergetools/vimdiff                   | 12 ++++++++++--\n 3 files changed, 25 insertions(+), 9 deletions(-)\n\n"}]}