* [PATCH 1/2] mergetool-lib: combine vimdiff and gvimdiff run blocks @ 2010-09-15 2:21 Dan McGee 2010-09-15 2:21 ` [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Dan McGee 0 siblings, 1 reply; 10+ messages in thread From: Dan McGee @ 2010-09-15 2:21 UTC (permalink / raw) To: git; +Cc: Markus Heidelberg, David Aguilar, Charles Bailey, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim 2010-09-15 2:21 [PATCH 1/2] mergetool-lib: combine vimdiff and gvimdiff run blocks Dan McGee @ 2010-09-15 2:21 ` Dan McGee 2010-09-18 7:34 ` David Aguilar 0 siblings, 1 reply; 10+ messages in thread From: Dan McGee @ 2010-09-15 2:21 UTC (permalink / raw) To: git; +Cc: Markus Heidelberg, David Aguilar, Charles Bailey, 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim 2010-09-15 2:21 ` [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Dan McGee @ 2010-09-18 7:34 ` David Aguilar 2010-09-19 9:48 ` Felipe Contreras 0 siblings, 1 reply; 10+ messages in thread From: David Aguilar @ 2010-09-18 7:34 UTC (permalink / raw) To: Dan McGee; +Cc: git, Markus Heidelberg, Charles Bailey 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim 2010-09-18 7:34 ` David Aguilar @ 2010-09-19 9:48 ` Felipe Contreras 2010-09-24 19:01 ` Dan McGee 0 siblings, 1 reply; 10+ messages in thread From: Felipe Contreras @ 2010-09-19 9:48 UTC (permalink / raw) To: David Aguilar; +Cc: Dan McGee, git, Markus Heidelberg, Charles Bailey 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim 2010-09-19 9:48 ` Felipe Contreras @ 2010-09-24 19:01 ` Dan McGee 2010-09-24 19:09 ` Jacob Helwig 2010-09-24 21:31 ` Jeff King 0 siblings, 2 replies; 10+ messages in thread From: Dan McGee @ 2010-09-24 19:01 UTC (permalink / raw) To: Felipe Contreras; +Cc: David Aguilar, git, Markus Heidelberg, Charles Bailey 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim 2010-09-24 19:01 ` Dan McGee @ 2010-09-24 19:09 ` Jacob Helwig 2010-09-24 21:38 ` Jeff King 2010-09-24 21:31 ` Jeff King 1 sibling, 1 reply; 10+ messages in thread From: Jacob Helwig @ 2010-09-24 19:09 UTC (permalink / raw) To: Dan McGee Cc: Felipe Contreras, David Aguilar, git, Markus Heidelberg, Charles Bailey [-- 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 --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim 2010-09-24 19:09 ` Jacob Helwig @ 2010-09-24 21:38 ` Jeff King 2010-09-25 3:17 ` David Aguilar 0 siblings, 1 reply; 10+ messages in thread From: Jeff King @ 2010-09-24 21:38 UTC (permalink / raw) To: Jacob Helwig Cc: Dan McGee, Felipe Contreras, David Aguilar, git, Markus Heidelberg, Charles Bailey 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim 2010-09-24 21:38 ` Jeff King @ 2010-09-25 3:17 ` David Aguilar 2010-09-27 15:19 ` Dan McGee 0 siblings, 1 reply; 10+ messages in thread From: David Aguilar @ 2010-09-25 3:17 UTC (permalink / raw) To: Jeff King Cc: Jacob Helwig, Dan McGee, Felipe Contreras, git, Markus Heidelberg, Charles Bailey 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim 2010-09-25 3:17 ` David Aguilar @ 2010-09-27 15:19 ` Dan McGee 0 siblings, 0 replies; 10+ messages in thread From: Dan McGee @ 2010-09-27 15:19 UTC (permalink / raw) To: git Cc: Dan McGee, David Aguilar, Jeff King, Jacob Helwig, Felipe Contreras, Markus Heidelberg, Charles Bailey 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim 2010-09-24 19:01 ` Dan McGee 2010-09-24 19:09 ` Jacob Helwig @ 2010-09-24 21:31 ` Jeff King 1 sibling, 0 replies; 10+ messages in thread From: Jeff King @ 2010-09-24 21:31 UTC (permalink / raw) To: Dan McGee Cc: Felipe Contreras, David Aguilar, git, Markus Heidelberg, Charles Bailey 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-09-27 15:19 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-15 2:21 [PATCH 1/2] mergetool-lib: combine vimdiff and gvimdiff run blocks Dan McGee 2010-09-15 2:21 ` [PATCH 2/2] mergetool-lib: add a three-way diff view for vim/gvim Dan McGee 2010-09-18 7:34 ` David Aguilar 2010-09-19 9:48 ` Felipe Contreras 2010-09-24 19:01 ` Dan McGee 2010-09-24 19:09 ` Jacob Helwig 2010-09-24 21:38 ` Jeff King 2010-09-25 3:17 ` David Aguilar 2010-09-27 15:19 ` Dan McGee 2010-09-24 21:31 ` Jeff King
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).
