mergetool.vimdiff.layout behavior differs from documentation

mergetool.vimdiff.layout behavior differs from documentation

[kawarimidoll]

Hello,

This is my first time reporting an issue with Git. I am not a native
English speaker, so I apologize if there are any translation mistakes.

Below is my response to `git bugreport`:

> What did you do before the bug happened? (Steps to reproduce your issue)
1. I checked the `git mergetool --help` documentation and configured
`$ git config --global mergetool.vimdiff.layout "@REMOTE"`.
2. In a repository with merge conflicts, I opened vimdiff using `git
mergetool`, updated the REMOTE buffer, and saved the changes.

> What did you expect to happen? (Expected behavior)
I expected the changes made to the REMOTE buffer to be reflected in the file.

> What happened instead? (Actual behavior)
The changes made to the REMOTE buffer were **not** reflected in the file.

> What’s different between what you expected and what actually happened?
The documentation states:
”@ is used to indicate the file containing the final version after
solving the conflicts. If not present, MERGED will be used by
default.”
So I thought that the changes made to the REMOTE buffer will be
reflected in the file if I use @REMOTE.
However, in my tests, @LOCAL and @MERGED worked as expected, but @BASE
and @REMOTE did not behave correctly.

> Anything else you want to add:
I’ve uploaded a video reproducing this issue to a GitHub Gist.
Please check here:
https://gist.github.com/kawarimidoll/3e603664432702e434c27f343fb35f85

[System Info]
git version:
git version 2.48.1
cpu: aarch64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /nix/store/3z1jypscq2ld48kl089ywgwd8ri2rjxq-bash-5.2p37/bin/bash
feature: fsmonitor--daemon
libcurl: 8.12.1
OpenSSL: OpenSSL 3.4.1 11 Feb 2025
zlib: 1.3.1
uname: Darwin 24.3.0 Darwin Kernel Version 24.3.0: Thu Jan  2 20:24:23
PST 2025; root:xnu-11215.81.4~3/RELEASE_ARM64_T8122 arm64
compiler info: clang: 19.1.7
libc info: no libc information available
$SHELL (typically, interactive shell): /bin/zsh


I appreciate your time in reviewing this. Thank you!

kawarimidoll
GitHub: https://github.com/kawarimidoll

Re: mergetool.vimdiff.layout behavior differs from documentation

[D. Ben Knoble]

On Mon, Mar 24, 2025 at 10:54 AM kawarimidoll
<kawarimidoll+git@gmail.com> wrote:
>
> Hello,
>
> This is my first time reporting an issue with Git. I am not a native
> English speaker, so I apologize if there are any translation mistakes.
>
> Below is my response to `git bugreport`:
>
> > What did you do before the bug happened? (Steps to reproduce your issue)
> 1. I checked the `git mergetool --help` documentation and configured
> `$ git config --global mergetool.vimdiff.layout "@REMOTE"`.
> 2. In a repository with merge conflicts, I opened vimdiff using `git
> mergetool`, updated the REMOTE buffer, and saved the changes.
>
> > What did you expect to happen? (Expected behavior)
> I expected the changes made to the REMOTE buffer to be reflected in the file.
>
> > What happened instead? (Actual behavior)
> The changes made to the REMOTE buffer were **not** reflected in the file.
>
> > What’s different between what you expected and what actually happened?
> The documentation states:
> ”@ is used to indicate the file containing the final version after
> solving the conflicts. If not present, MERGED will be used by
> default.”
> So I thought that the changes made to the REMOTE buffer will be
> reflected in the file if I use @REMOTE.
> However, in my tests, @LOCAL and @MERGED worked as expected, but @BASE
> and @REMOTE did not behave correctly.

Interesting; I haven't tried to reproduce this, but the docs appear to
(vaguely) indicate that this is the case.
I've CC'd the author of most of that mergetool for some help, but the
issue is likely here (lines 298–310 of the script mergetools/vimdiff
on 683c54c999 (Git 2.49, 2025-03-14)):

# Search for a "@" in one of the files identifiers ("LOCAL", "BASE",
# "REMOTE", "MERGED"). If not found, use "MERGE" as the default file
# where changes will be saved.

if echo "$LAYOUT" | grep @LOCAL >/dev/null
then
FINAL_TARGET="LOCAL"
elif echo "$LAYOUT" | grep @BASE >/dev/null
then
FINAL_TARGET="BASE"
else
FINAL_TARGET="MERGED"
fi

(Apologies that my mail client appears to strip leading indentation on
paste, ugh)

A GitHub link, for those who prefer it:
https://github.com/git/git/blob/683c54c999c301c2cd6f715c411407c413b1d84e/mergetools/vimdiff#L298-L310

That code goes back to 0041797449 (vimdiff: new implementation with
layout support, 2022-03-30), whose author is also CC'd.

-- 
D. Ben Knoble

Re: mergetool.vimdiff.layout behavior differs from documentation

[Fernando Ramos]

On 25/03/24 04:35PM, D. Ben Knoble wrote:
> I've CC'd the author of most of that mergetool for some help, but the
> issue is likely here (lines 298–310 of the script mergetools/vimdiff
> on 683c54c999 (Git 2.49, 2025-03-14)):
> 
> # Search for a "@" in one of the files identifiers ("LOCAL", "BASE",
> # "REMOTE", "MERGED"). If not found, use "MERGE" as the default file
> # where changes will be saved.
> 
> if echo "$LAYOUT" | grep @LOCAL >/dev/null
> then
> FINAL_TARGET="LOCAL"
> elif echo "$LAYOUT" | grep @BASE >/dev/null
> then
> FINAL_TARGET="BASE"
> else
> FINAL_TARGET="MERGED"
> fi

You are completely right. This is a bug which can probably be fixed by simply
adding one extra "elif":

   if echo "$LAYOUT" | grep @LOCAL >/dev/null
   then
   FINAL_TARGET="LOCAL"
   elif echo "$LAYOUT" | grep @BASE >/dev/null
   then
   FINAL_TARGET="BASE"
  +elif echo "$LAYOUT" | grep @REMOTE >/dev/null
  +then
  +FINAL_TARGET="REMOTE"
   else
   FINAL_TARGET="MERGED"
   fi
    
If you can test it and prepare a patch, that would be great. Otherwise I will
try to do it myself in a few days.

Thanks!