Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mergetool(vimdiff): allow paths to contain spaces again #3960

Merged
merged 1 commit into from
Jul 25, 2022

Conversation

dscho
Copy link
Member

@dscho dscho commented Jul 25, 2022

This fixes #3945

In 0041797 (vimdiff: new implementation with layout support,
2022-03-30), we introduced a completely new implementation of the
`vimdiff` backend for `git mergetool`.

In this implementation, we no longer call `vim` directly but we
accumulate in the variable `FINAL_CMD` an arbitrary number of commands
for `vim` to execute, which necessitates the use of `eval` to split the
commands properly into multiple command-line arguments.

That same `eval` command also needs to pass the paths to `vim`, and
while it looks as if they are quoted correctly, that quoting only
reaches the `eval` instruction and is lost after that, therefore paths
that contain whitespace characters (or other characters that are
interpreted by the POSIX shell) are handled incorrectly.

This is a simple reproducer:

	git init -b main bam-merge-fail
	cd bam-merge-fail
	echo a>"a file.txt"
	git add "a file.txt"
	git commit -m "added 'a file.txt'"
	echo b>"a file.txt"
	git add "a file.txt"
	git commit -m "diverged b 'a file.txt'"
	git checkout -b c HEAD~
	echo c>"a file.txt"
	git add "a file.txt"
	git commit -m "diverged c 'a file.txt'"
	git checkout main
	git merge c
	git mergetool --tool=vimdiff

With Git v2.37.0/v2.37.1, this will open 7 buffers, not four, and not
display the correct contents at all.

To fix this, let's not expand the variables containing the path
parameters before passing them to the `eval` command, but let that
command expand the variables instead.

This fixes git-for-windows#3945

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho self-assigned this Jul 25, 2022
@dscho dscho added this to the Next release milestone Jul 25, 2022
@dscho
Copy link
Member Author

dscho commented Jul 25, 2022

This has been reviewed on the Git mailing list already and made it into git/git's main branch as git@ccc7b51.

@dscho dscho merged commit eb13b4d into git-for-windows:main Jul 25, 2022
@dscho dscho deleted the fix-vimdiff-with-spaces-in-paths branch July 25, 2022 14:02
git-for-windows-ci pushed a commit that referenced this pull request Jul 25, 2022
mergetool(vimdiff): allow paths to contain spaces again
dscho added a commit to git-for-windows/build-extra that referenced this pull request Jul 25, 2022
A [regression in the `vimdiff` mode of `git
mergetool`](git-for-windows/git#3945)
has been [fixed](git-for-windows/git#3960).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
git-for-windows-ci pushed a commit that referenced this pull request Aug 11, 2022
mergetool(vimdiff): allow paths to contain spaces again
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vimdiff Mergetool Regressed Between 2.36.1.windows.1 and 2.37.0.windows.1
1 participant