Re: [PATCH] mergetool: reorder vim/gvim buffers in three-way diffs

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 10, 2016, 17:45 UTC
Message-ID: <xmqqlh6szcfk.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <56BB4BDF.3010407@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

Show 7 quoted lines

>> Second call for help.  Any comments on this from anybody other than
>> the author that I missed to support this change?
>
> OK, applied it (on top of next), looks sane and improves the situation
> for the majority of people who read left to right, then top down and
> assign buffers 1 2 3 4 "mentally" to local base remote merge windows
> based on that. Their expectation is met now. Thanks!

Thanks.

Does this mean that I should warn in the release notes that some existing users might get their expectation broken but we are going ahead anyway because we think most people read left to right and then top down? I am OK with saying that--I just wanted to make sure we know that it is what we are doing.

> (Also, the other vim variants don't need a corresponding change.)

A stupid question while I am here. What are these different variants? When reviewing this patch for the first time I tried to find where they are documented, but didn't spot anything.

I can see from the code that vimdiff2 variant does not do anything special when it is doing a 3-way merge, but vimdiff3 variant does behave differently when it has $BASE. It does not need a change like this because it already arranges and numbers the windows sensibly (in other words, we can label this patch as aligning the behaviour of vimdiff to that of vimdiff3)?

Thanks.

Show 35 quoted lines

> Michael
>
>>>
>>>>  mergetools/vimdiff | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/mergetools/vimdiff b/mergetools/vimdiff
>>>> index 1ddfbfc..74ea6d5 100644
>>>> --- a/mergetools/vimdiff
>>>> +++ b/mergetools/vimdiff
>>>> @@ -2,22 +2,22 @@ diff_cmd () {
>>>>  	"$merge_tool_path" -R -f -d \
>>>>  		-c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE"
>>>>  }
>>>>  
>>>>  merge_cmd () {
>>>>  	touch "$BACKUP"
>>>>  	case "$1" in
>>>>  	gvimdiff|vimdiff)
>>>>  		if $base_present
>>>>  		then
>>>> -			"$merge_tool_path" -f -d -c 'wincmd J' \
>>>> -				"$MERGED" "$LOCAL" "$BASE" "$REMOTE"
>>>> +			"$merge_tool_path" -f -d -c '4wincmd w | wincmd J' \
>>>> +				"$LOCAL" "$BASE" "$REMOTE" "$MERGED"
>>>>  		else
>>>>  			"$merge_tool_path" -f -d -c 'wincmd l' \
>>>>  				"$LOCAL" "$MERGED" "$REMOTE"
>>>>  		fi
>>>>  		;;
>>>>  	gvimdiff2|vimdiff2)
>>>>  		"$merge_tool_path" -f -d -c 'wincmd l' \
>>>>  			"$LOCAL" "$MERGED" "$REMOTE"
>>>>  		;;
>>>>  	gvimdiff3|vimdiff3)