Kipras Melnikovas <kipras@xxxxxxxxxx> writes:
> 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.
It is a good consideration, and should be documented ...
> 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
... perhaps before "See BACKEND SPECIFIC HINTS section." E.g.
When a variant of vimdiff (vim, Neovim, or gVim) is used as
a mergetool backend, they use this variable to control how
the split windows appear.
The variable `mergetool.<variant>.layout` (where <variant>
is one of `vimdiff`, `nvimdiff`, or `gvimdiff`, depending on
what you are using) is consulted first, and if it is missing,
`mergetool.vimdiff.layout` is used as a fallback. See
BACKEND SPECIFIC HINTS section.
or something?
> 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)
The callers of merge_cmd are careful to do
merge_cmd "$1"
so it would be a good hygiene to also quote $TOOL here, i.e.
layout=$(git config "mergetool.$TOOL.layout")
It might not matter if the caller of run_merge_cmd (which calls
merge_cmd) eventually chooses from a known set of strings hardcoded
in mergetools--lib.sh, but it is much easier to show that you are
doing the right thing without relying on such a detail of what
happens far in the code to quote what you get from the caller
appropriately.
> +
> + # backwards-compatibility:
> + if test -z "$layout"
> + then
> + layout=$(git config mergetool.vimdiff.layout)
> + fi
> +
> + case "$TOOL" in
This one is quoted properly (and TOOL=$1 at the beginning does not
require quoting). The "git config" call above is the only one that
needs to be fixed.
Thanks.
> *vimdiff)
> if test -z "$layout"
> then
>
> base-commit: 4fc51f00ef18d2c0174ab2fd39d0ee473fd144bd
