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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 13, 2022, 22:33 UTC
Message-ID: <xmqqa69cabhq.fsf@gitster.g>
In-Reply-To: <Ys9Ftu3z6zedCsoX@zacax395.localdomain>

Fernando Ramos <greenfoo@u92.eu> writes:

Show 12 quoted lines

> On 22/07/13 02:54PM, Junio C Hamano wrote:
>> 
>> 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?
>> 
> I think testing both is not really needed as the unit test is just making sure
> that filenames with spaces are processed correctly.  Whatever comes before
> (which changes depending on the value of "base_present") doesn't really matter
> as long as there is something.

As the quoting glitch are distributed on both sides

	if $base_present
	then
		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
		# before), close all BASE windows using vim's "quit" command

		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"
	fi

I suspect you can fix only one, and carefully choose which side to test, and leave the other side broken ;-)

In any case, to minimize disruption to Dscho's patch, I replaced the "1" with "false" (which will make his patch pass the new test without other stuff) and tweak its merge into 'seen' to adjust for the "leftabove" topic.

Thanks.