Skip to content

Commit dde9c7c

Browse files
committed
mergetool(vimdiff): allow paths to contain spaces again
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 <[email protected]>
1 parent 980145f commit dde9c7c

File tree

1 file changed

+35
-4
lines changed

1 file changed

+35
-4
lines changed

mergetools/vimdiff

+35-4
Original file line numberDiff line numberDiff line change
@@ -414,8 +414,8 @@ merge_cmd () {
414414

415415
if $base_present
416416
then
417-
eval "$merge_tool_path" \
418-
-f "$FINAL_CMD" "$LOCAL" "$BASE" "$REMOTE" "$MERGED"
417+
eval '"$merge_tool_path"' \
418+
-f "$FINAL_CMD" '"$LOCAL"' '"$BASE"' '"$REMOTE"' '"$MERGED"'
419419
else
420420
# If there is no BASE (example: a merge conflict in a new file
421421
# with the same name created in both braches which didn't exist
@@ -424,8 +424,8 @@ merge_cmd () {
424424
FINAL_CMD=$(echo "$FINAL_CMD" | \
425425
sed -e 's:2b:quit:g' -e 's:3b:2b:g' -e 's:4b:3b:g')
426426

427-
eval "$merge_tool_path" \
428-
-f "$FINAL_CMD" "$LOCAL" "$REMOTE" "$MERGED"
427+
eval '"$merge_tool_path"' \
428+
-f "$FINAL_CMD" '"$LOCAL"' '"$REMOTE"' '"$MERGED"'
429429
fi
430430

431431
ret="$?"
@@ -614,6 +614,37 @@ run_unit_tests () {
614614
fi
615615
done
616616

617+
# verify that `merge_cmd` handles paths with spaces
618+
record_parameters () {
619+
>actual
620+
for arg
621+
do
622+
echo "$arg" >>actual
623+
done
624+
}
625+
626+
base_present=1
627+
LOCAL='lo cal'
628+
BASE='ba se'
629+
REMOTE="' '"
630+
MERGED='mer ged'
631+
merge_tool_path=record_parameters
632+
633+
merge_cmd vimdiff || at_least_one_ko=true
634+
635+
cat >expect <<-\EOF
636+
-f
637+
-c
638+
echo | split | vertical split | 1b | wincmd l | vertical split | quit | wincmd l | 2b | wincmd j | 3b | tabdo windo diffthis
639+
-c
640+
tabfirst
641+
lo cal
642+
' '
643+
mer ged
644+
EOF
645+
646+
diff -u expect actual || at_least_one_ko=true
647+
617648
if test "$at_least_one_ko" = "true"
618649
then
619650
return 255

0 commit comments

Comments
 (0)