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

[Junio C Hamano <gitster@pobox.com>]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

>  	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

If there were another syntactic fix we need in the future, we may by
mistake fix one but not the other, and the test we add in this patch
checks only one side but not the other.  In a follow-up we may want
to unify the two eval invocations to make the testing of this part
more robust.

But as a minimum and obvious fix, stopping at the above is
appropriate for this patch.

> + ...
> +	diff -u expect actual || at_least_one_ko=true

I wonder if we still should care about platforms that need to set
GIT_TEST_CMP_USE_COPIED_CONTEXT while running our tests.  If we use
"diff -u" hardcoded like this here, we are declaring that they are
now unwelcome to run our tests.

It might be just the matter of using "cmp -s" here (or run "diff"
without any option).  Do we care about emitting the difference in
the output?  I doubt it.

>  	if test "$at_least_one_ko" = "true"
>  	then
>  		return 255

Thanks.