Re: [PATCH] mergetool(vimdiff): allow paths to contain spaces again

From

Junio C Hamano <gitster@pobox.com>

Date

Jul 13, 2022, 22:06 UTC

Message-ID

<xmqqmtdcacr7.fsf@gitster.g>

In-Reply-To

<xmqqr12oadbk.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

>> For one, as you said, "1" should be "true". That also changes the expected
>> output.
>
> OK, because "1" fails to execute, the expected output Dscho had
> (which is for the case without base) would become invalid when we
> use "true".
>
> Perhaps we should use "false" instead?  Or do we need to test both?

This is how it would look like on top of the "leftabove" topic.

 mergetools/vimdiff | 39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git c/mergetools/vimdiff w/mergetools/vimdiff
index b045b10fd7..f770b8fe24 100644
--- c/mergetools/vimdiff
+++ w/mergetools/vimdiff
@@ -414,8 +414,8 @@ merge_cmd () {
 
 	if $base_present
 	then
-		eval "$merge_tool_path" \
-			-f "$FINAL_CMD" "$LOCAL" "$BASE" "$REMOTE" "$MERGED"
+		eval '"$merge_tool_path"' \
+			-f "$FINAL_CMD" '"$LOCAL"' '"$BASE"' '"$REMOTE"' '"$MERGED"'
 	else
 		# If there is no BASE (example: a merge conflict in a new file
 		# with the same name created in both braches which didn't exist
@@ -424,8 +424,8 @@ merge_cmd () {
 		FINAL_CMD=$(echo "$FINAL_CMD" | \
 			sed -e 's:2b:quit:g' -e 's:3b:2b:g' -e 's:4b:3b:g')
 
-		eval "$merge_tool_path" \
-			-f "$FINAL_CMD" "$LOCAL" "$REMOTE" "$MERGED"
+		eval '"$merge_tool_path"' \
+			-f "$FINAL_CMD" '"$LOCAL"' '"$REMOTE"' '"$MERGED"'
 	fi
 
 	ret="$?"
@@ -614,6 +614,37 @@ run_unit_tests () {
 		fi
 	done
 
+	# verify that `merge_cmd` handles paths with spaces
+	record_parameters () {
+		>actual
+		for arg
+		do
+			echo "$arg" >>actual
+		done
+	}
+
+	base_present=false
+	LOCAL='lo cal'
+	BASE='ba se'
+	REMOTE="' '"
+	MERGED='mer ged'
+	merge_tool_path=record_parameters
+
+	merge_cmd vimdiff || at_least_one_ko=true
+
+	cat >expect <<-\EOF
+	-f
+	-c
+	echo | leftabove split | leftabove vertical split | 1b | wincmd l | leftabove vertical split | quit | wincmd l | 2b | wincmd j | 3b | tabdo windo diffthis
+	-c
+	tabfirst
+	lo cal
+	' '
+	mer ged
+	EOF
+
+	diff -u expect actual || at_least_one_ko=true
+
 	if test "$at_least_one_ko" = "true"
 	then
 		return 255