# [PATCH 1/2] mergetool-lib: combine vimdiff and gvimdiff run blocks 10 messages from 2010-09-15 to 2010-09-27. Participants: Dan McGee, David Aguilar, Felipe Contreras, Jacob Helwig, Jeff King. Thread: https://gitlist.dev/t/25106 ## Dan McGee, 2010-09-15 02:21 Subject: [PATCH 1/2] mergetool-lib: combine vimdiff and gvimdiff run blocks Message-ID: <1284517303-17244-1-git-send-email-dpmcgee@gmail.com> URL: https://gitlist.dev/e/1284517303-17244-1-git-send-email-dpmcgee%40gmail.com ``` 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 --- 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 ``` ## Dan McGee, 2010-09-15 02:21 Subject: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Message-ID: <1284517303-17244-2-git-send-email-dpmcgee@gmail.com> URL: https://gitlist.dev/e/1284517303-17244-2-git-send-email-dpmcgee%40gmail.com In-Reply-To: <1284517303-17244-1-git-send-email-dpmcgee@gmail.com> ``` 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 --- 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 ``` ## David Aguilar, 2010-09-18 07:34 Subject: Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Message-ID: <20100918073428.GA9850@gmail.com> URL: https://gitlist.dev/e/20100918073428.GA9850%40gmail.com In-Reply-To: <1284517303-17244-2-git-send-email-dpmcgee@gmail.com> ``` 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 > --- > > 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 ``` ## Felipe Contreras, 2010-09-19 09:48 Subject: Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Message-ID: URL: https://gitlist.dev/e/AANLkTim_hwduHk-ENM73dwHUj9XbwfihZPRfsHX%2BM3DE%40mail.gmail.com In-Reply-To: <20100918073428.GA9850@gmail.com> ``` On Sat, Sep 18, 2010 at 10:34 AM, David Aguilar 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 >> --- >> >> 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 ``` ## Dan McGee, 2010-09-24 19:01 Subject: Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Message-ID: URL: https://gitlist.dev/e/AANLkTin-BSAFwvuTyJ96BW6MqrKVEni%2BAf2M0u7WE_yZ%40mail.gmail.com In-Reply-To: ``` On Sun, Sep 19, 2010 at 4:48 AM, Felipe Contreras wrote: > On Sat, Sep 18, 2010 at 10:34 AM, David Aguilar 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 >>> --- >>> >>> 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 ``` ## Jacob Helwig, 2010-09-24 19:09 Subject: Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Message-ID: <20100924190928.GC3768@vfa-6z.perlninja.com> URL: https://gitlist.dev/e/20100924190928.GC3768%40vfa-6z.perlninja.com In-Reply-To: ``` On Fri, 24 Sep 2010 14:01:01 -0500, Dan McGee wrote: > > On Sun, Sep 19, 2010 at 4:48 AM, Felipe Contreras > wrote: > > On Sat, Sep 18, 2010 at 10:34 AM, David Aguilar 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 > >>> --- > >>> > >>> 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 ``` ## Jeff King, 2010-09-24 21:31 Subject: Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Message-ID: <20100924213116.GA19676@sigill.intra.peff.net> URL: https://gitlist.dev/e/20100924213116.GA19676%40sigill.intra.peff.net In-Reply-To: ``` On Fri, Sep 24, 2010 at 02:01:01PM -0500, Dan McGee wrote: > On Sun, Sep 19, 2010 at 4:48 AM, Felipe Contreras > wrote: > > On Sat, Sep 18, 2010 at 10:34 AM, David Aguilar 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 > >>> --- > >>> > >>> 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 ``` ## Jeff King, 2010-09-24 21:38 Subject: Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Message-ID: <20100924213852.GB19676@sigill.intra.peff.net> URL: https://gitlist.dev/e/20100924213852.GB19676%40sigill.intra.peff.net In-Reply-To: <20100924190928.GC3768@vfa-6z.perlninja.com> ``` 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 ``` ## David Aguilar, 2010-09-25 03:17 Subject: Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Message-ID: <20100925031745.GB30550@gmail.com> URL: https://gitlist.dev/e/20100925031745.GB30550%40gmail.com In-Reply-To: <20100924213852.GB19676@sigill.intra.peff.net> ``` 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 ``` ## Dan McGee, 2010-09-27 15:19 Subject: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Message-ID: <1285600749-1088-1-git-send-email-dpmcgee@gmail.com> URL: https://gitlist.dev/e/1285600749-1088-1-git-send-email-dpmcgee%40gmail.com In-Reply-To: <20100925031745.GB30550@gmail.com> ``` 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 --- 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 ```