[PATCH 1/2] mergetool-lib: combine vimdiff and gvimdiff run blocks

[PATCH 1/2] mergetool-lib: combine vimdiff and gvimdiff run blocks

[Dan McGee]

They are nearly identical outside of the foreground flag, which can safely
be passed to both vim and gvim. The merge tool itself is named in
$merge_tool_path.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---
 git-mergetool--lib.sh |   17 +++--------------
 1 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index b5e1943..f9a51ba 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -169,25 +169,14 @@ run_merge_tool () {
 			"$merge_tool_path" "$LOCAL" "$REMOTE" | cat
 		fi
 		;;
-	vimdiff)
-		if merge_mode; then
-			touch "$BACKUP"
-			"$merge_tool_path" -d -c "wincmd l" \
-				"$LOCAL" "$MERGED" "$REMOTE"
-			check_unchanged
-		else
-			"$merge_tool_path" -d -c "wincmd l" \
-				"$LOCAL" "$REMOTE"
-		fi
-		;;
-	gvimdiff)
+	vimdiff|gvimdiff)
 		if merge_mode; then
 			touch "$BACKUP"
-			"$merge_tool_path" -d -c "wincmd l" -f \
+			"$merge_tool_path" -f -d -c "wincmd l" \
 				"$LOCAL" "$MERGED" "$REMOTE"
 			check_unchanged
 		else
-			"$merge_tool_path" -d -c "wincmd l" -f \
+			"$merge_tool_path" -f -d -c "wincmd l" \
 				"$LOCAL" "$REMOTE"
 		fi
 		;;
-- 
1.7.2.3

[PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim

[Dan McGee]

When the base version is available, use a three-way, four panel view by
default. This shows the (local, base, remote) revisions up top and the
merged result by itself in the lower pane. All revisions will still scroll
together by default, and the cursor still defaults to the merged result edit
pane.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---

Vim was one of the few diff commands to not support a three-way merge showing
the base revision, so this is a stab at resolving that shortfall. The biggest
objection I can see to this is making the interface a bit more cumbersome and
bloated.

An example screenshot of what this produces:
http://www.toofishes.net/media/extra/vim_three_way.png

-Dan

 git-mergetool--lib.sh |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index f9a51ba..b84ac58 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -172,8 +172,13 @@ run_merge_tool () {
 	vimdiff|gvimdiff)
 		if merge_mode; then
 			touch "$BACKUP"
-			"$merge_tool_path" -f -d -c "wincmd l" \
-				"$LOCAL" "$MERGED" "$REMOTE"
+			if $base_present; then
+				"$merge_tool_path" -f -d -c "wincmd J" \
+					"$MERGED" "$LOCAL" "$BASE" "$REMOTE"
+			else
+				"$merge_tool_path" -f -d -c "wincmd l" \
+					"$LOCAL" "$MERGED" "$REMOTE"
+			fi
 			check_unchanged
 		else
 			"$merge_tool_path" -f -d -c "wincmd l" \
-- 
1.7.2.3

Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim

[David Aguilar]

On Tue, Sep 14, 2010 at 09:21:43PM -0500, Dan McGee wrote:
> When the base version is available, use a three-way, four panel view by
> default. This shows the (local, base, remote) revisions up top and the
> merged result by itself in the lower pane. All revisions will still scroll
> together by default, and the cursor still defaults to the merged result edit
> pane.
> 
> Signed-off-by: Dan McGee <dpmcgee@gmail.com>
> ---
> 
> Vim was one of the few diff commands to not support a three-way merge showing
> the base revision, so this is a stab at resolving that shortfall. The biggest
> objection I can see to this is making the interface a bit more cumbersome and
> bloated.
> 
> An example screenshot of what this produces:
> http://www.toofishes.net/media/extra/vim_three_way.png
> 
> -Dan


Patch 1/2 of this series looks good to me.

Is it worth keeping the old behavior and calling this new
mode "vimdiff3" or something along those lines?

I'm not a vimdiff user so I'm not be the best person to
judge the merits of this change.  I like what it's trying
to accomplish, though.  Are there any vimdiff users
with strong feelings either way?

-- 

		David

Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim

[Felipe Contreras]

On Sat, Sep 18, 2010 at 10:34 AM, David Aguilar <davvid@gmail.com> wrote:
> On Tue, Sep 14, 2010 at 09:21:43PM -0500, Dan McGee wrote:
>> When the base version is available, use a three-way, four panel view by
>> default. This shows the (local, base, remote) revisions up top and the
>> merged result by itself in the lower pane. All revisions will still scroll
>> together by default, and the cursor still defaults to the merged result edit
>> pane.
>>
>> Signed-off-by: Dan McGee <dpmcgee@gmail.com>
>> ---
>>
>> Vim was one of the few diff commands to not support a three-way merge showing
>> the base revision, so this is a stab at resolving that shortfall. The biggest
>> objection I can see to this is making the interface a bit more cumbersome and
>> bloated.
>>
>> An example screenshot of what this produces:
>> http://www.toofishes.net/media/extra/vim_three_way.png
>>
>> -Dan
>
>
> Patch 1/2 of this series looks good to me.
>
> Is it worth keeping the old behavior and calling this new
> mode "vimdiff3" or something along those lines?
>
> I'm not a vimdiff user so I'm not be the best person to
> judge the merits of this change.  I like what it's trying
> to accomplish, though.  Are there any vimdiff users
> with strong feelings either way?

I think this is a definite improvement; the old mode wasn't really
useful for me.

-- 
Felipe Contreras

Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim

[Dan McGee]

On Sun, Sep 19, 2010 at 4:48 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Sat, Sep 18, 2010 at 10:34 AM, David Aguilar <davvid@gmail.com> wrote:
>> On Tue, Sep 14, 2010 at 09:21:43PM -0500, Dan McGee wrote:
>>> When the base version is available, use a three-way, four panel view by
>>> default. This shows the (local, base, remote) revisions up top and the
>>> merged result by itself in the lower pane. All revisions will still scroll
>>> together by default, and the cursor still defaults to the merged result edit
>>> pane.
>>>
>>> Signed-off-by: Dan McGee <dpmcgee@gmail.com>
>>> ---
>>>
>>> Vim was one of the few diff commands to not support a three-way merge showing
>>> the base revision, so this is a stab at resolving that shortfall. The biggest
>>> objection I can see to this is making the interface a bit more cumbersome and
>>> bloated.
>>>
>>> An example screenshot of what this produces:
>>> http://www.toofishes.net/media/extra/vim_three_way.png
>>>
>>> -Dan
>>
>>
>> Patch 1/2 of this series looks good to me.
>>
>> Is it worth keeping the old behavior and calling this new
>> mode "vimdiff3" or something along those lines?
>>
>> I'm not a vimdiff user so I'm not be the best person to
>> judge the merits of this change.  I like what it's trying
>> to accomplish, though.  Are there any vimdiff users
>> with strong feelings either way?
>
> I think this is a definite improvement; the old mode wasn't really
> useful for me.

Not as much feedback as I had hoped, but thanks to those that did
speak up. I was thinking of adding a separate mode, but I think it
would then get under-used and as I said, every other merge tool was
already doing this anyway.

So are these patches good to go forward with? No major objections in a
over a week's time.

-Dan

Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim

[Jacob Helwig]

[-- Attachment #1: Type: text/plain, Size: 2236 bytes --]

On Fri, 24 Sep 2010 14:01:01 -0500, Dan McGee wrote:
> 
> On Sun, Sep 19, 2010 at 4:48 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> > On Sat, Sep 18, 2010 at 10:34 AM, David Aguilar <davvid@gmail.com> wrote:
> >> On Tue, Sep 14, 2010 at 09:21:43PM -0500, Dan McGee wrote:
> >>> When the base version is available, use a three-way, four panel view by
> >>> default. This shows the (local, base, remote) revisions up top and the
> >>> merged result by itself in the lower pane. All revisions will still scroll
> >>> together by default, and the cursor still defaults to the merged result edit
> >>> pane.
> >>>
> >>> Signed-off-by: Dan McGee <dpmcgee@gmail.com>
> >>> ---
> >>>
> >>> Vim was one of the few diff commands to not support a three-way merge showing
> >>> the base revision, so this is a stab at resolving that shortfall. The biggest
> >>> objection I can see to this is making the interface a bit more cumbersome and
> >>> bloated.
> >>>
> >>> An example screenshot of what this produces:
> >>> http://www.toofishes.net/media/extra/vim_three_way.png
> >>>
> >>> -Dan
> >>
> >>
> >> Patch 1/2 of this series looks good to me.
> >>
> >> Is it worth keeping the old behavior and calling this new
> >> mode "vimdiff3" or something along those lines?
> >>
> >> I'm not a vimdiff user so I'm not be the best person to
> >> judge the merits of this change.  I like what it's trying
> >> to accomplish, though.  Are there any vimdiff users
> >> with strong feelings either way?
> >
> > I think this is a definite improvement; the old mode wasn't really
> > useful for me.
> 
> Not as much feedback as I had hoped, but thanks to those that did
> speak up. I was thinking of adding a separate mode, but I think it
> would then get under-used and as I said, every other merge tool was
> already doing this anyway.
> 
> So are these patches good to go forward with? No major objections in a
> over a week's time.
> 
> -Dan

I'd +1 David's suggestion of calling this "vimdiff3", I'd like to still
be able to access the current behavior, since I have merge.conflictstyle
= diff3, and already see the merge base when I use (g)vimdiff with
mergetool.

-- 
Jacob Helwig

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim

[Jeff King]

On Fri, Sep 24, 2010 at 02:01:01PM -0500, Dan McGee wrote:

> On Sun, Sep 19, 2010 at 4:48 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> > On Sat, Sep 18, 2010 at 10:34 AM, David Aguilar <davvid@gmail.com> wrote:
> >> On Tue, Sep 14, 2010 at 09:21:43PM -0500, Dan McGee wrote:
> >>> When the base version is available, use a three-way, four panel view by
> >>> default. This shows the (local, base, remote) revisions up top and the
> >>> merged result by itself in the lower pane. All revisions will still scroll
> >>> together by default, and the cursor still defaults to the merged result edit
> >>> pane.
> >>>
> >>> Signed-off-by: Dan McGee <dpmcgee@gmail.com>
> >>> ---
> >>>
> >>> Vim was one of the few diff commands to not support a three-way merge showing
> >>> the base revision, so this is a stab at resolving that shortfall. The biggest
> >>> objection I can see to this is making the interface a bit more cumbersome and
> >>> bloated.
> >>>
> >>> An example screenshot of what this produces:
> >>> http://www.toofishes.net/media/extra/vim_three_way.png
> >>>
> >>> -Dan
> >>
> >>
> >> Patch 1/2 of this series looks good to me.
> >>
> >> Is it worth keeping the old behavior and calling this new
> >> mode "vimdiff3" or something along those lines?
> >>
> >> I'm not a vimdiff user so I'm not be the best person to
> >> judge the merits of this change.  I like what it's trying
> >> to accomplish, though.  Are there any vimdiff users
> >> with strong feelings either way?
> >
> > I think this is a definite improvement; the old mode wasn't really
> > useful for me.
> 
> Not as much feedback as I had hoped, but thanks to those that did
> speak up. I was thinking of adding a separate mode, but I think it
> would then get under-used and as I said, every other merge tool was
> already doing this anyway.

A little more feedback:

I use vim but don't use vimdiff, because the original mode seemed
useless to me. Your change makes it much better. I haven't actually had
to do any merging lately, though, so I can't comment in practice.

Given that nobody has objected, you have a few comments in support, and
the fact that it makes it similar to every other mergetool driver, I
think it should probably be the default. If somebody really finds it
objectionable, it is not hard for them to configure the old behavior.

-Peff

Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim

[Jeff King]

On Fri, Sep 24, 2010 at 12:09:28PM -0700, Jacob Helwig wrote:

> > So are these patches good to go forward with? No major objections in a
> > over a week's time.
> > 
> > -Dan
> 
> I'd +1 David's suggestion of calling this "vimdiff3", I'd like to still
> be able to access the current behavior, since I have merge.conflictstyle
> = diff3, and already see the merge base when I use (g)vimdiff with
> mergetool.

Of course as soon as I say "nobody objected" in my other email, this
arrives. :)

Can we provide both, but make the vimdiff3 behavior the preferred
default? It better matches the default merge.conflictstyle, and people
who are using diff3 obviously understand how to tweak config.

-Peff

Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim

[David Aguilar]

On Fri, Sep 24, 2010 at 05:38:52PM -0400, Jeff King wrote:
> On Fri, Sep 24, 2010 at 12:09:28PM -0700, Jacob Helwig wrote:
> 
> > > So are these patches good to go forward with? No major objections in a
> > > over a week's time.
> > > 
> > > -Dan
> > 
> > I'd +1 David's suggestion of calling this "vimdiff3", I'd like to still
> > be able to access the current behavior, since I have merge.conflictstyle
> > = diff3, and already see the merge base when I use (g)vimdiff with
> > mergetool.
> 
> Of course as soon as I say "nobody objected" in my other email, this
> arrives. :)
> 
> Can we provide both, but make the vimdiff3 behavior the preferred
> default? It better matches the default merge.conflictstyle, and people
> who are using diff3 obviously understand how to tweak config.
> 
> -Peff

+1 to Peff's suggestion.

Dan, can you reroll the patch so that the new behavior is
"(g)vimdiff" and the old behavior is available as
"(g)vimdiff2"?

I do slightly dislike having both from the maintenance POV.
But, it's better to keep it around than to rip it away from
happy users' hands.  Thanks for speaking up Jacob.


Cheers,
-- 

		David

[PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim

[Dan McGee]

When the base version is available, use a three-way, four panel view by
default. This shows the (local, base, remote) revisions up top and the
merged result by itself in the lower pane. All revisions will still scroll
together by default, and the cursor still defaults to the merged result edit
pane.

The original vimdiff/gvimdiff configuration is now available by using
'vimdiff2' or 'gvimdiff2' as the preferred merge tool.

Signed-off-by: Dan McGee <dpmcgee@gmail.com>
---

This should address the comments I got once I pestered people. The new behavior
is the default, but it at least becomes possible to use the previous behavior
without much hassle (just set your mergetool appropriately).

-Dan

 git-mergetool--lib.sh |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index f9a51ba..77d4aee 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -10,10 +10,10 @@ merge_mode() {
 
 translate_merge_tool_path () {
 	case "$1" in
-	vimdiff)
+	vimdiff|vimdiff2)
 		echo vim
 		;;
-	gvimdiff)
+	gvimdiff|gvimdiff2)
 		echo gvim
 		;;
 	emerge)
@@ -47,7 +47,8 @@ check_unchanged () {
 valid_tool () {
 	case "$1" in
 	kdiff3 | tkdiff | xxdiff | meld | opendiff | \
-	emerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis | p4merge)
+	vimdiff | gvimdiff | vimdiff2 | gvimdiff2 | \
+	emerge | ecmerge | diffuse | araxis | p4merge)
 		;; # happy
 	tortoisemerge)
 		if ! merge_mode; then
@@ -172,6 +173,22 @@ run_merge_tool () {
 	vimdiff|gvimdiff)
 		if merge_mode; then
 			touch "$BACKUP"
+			if $base_present; then
+				"$merge_tool_path" -f -d -c "wincmd J" \
+					"$MERGED" "$LOCAL" "$BASE" "$REMOTE"
+			else
+				"$merge_tool_path" -f -d -c "wincmd l" \
+					"$LOCAL" "$MERGED" "$REMOTE"
+			fi
+			check_unchanged
+		else
+			"$merge_tool_path" -f -d -c "wincmd l" \
+				"$LOCAL" "$REMOTE"
+		fi
+		;;
+	vimdiff2|gvimdiff2)
+		if merge_mode; then
+			touch "$BACKUP"
 			"$merge_tool_path" -f -d -c "wincmd l" \
 				"$LOCAL" "$MERGED" "$REMOTE"
 			check_unchanged
-- 
1.7.3