[PATCH] gitk: Add "First parent" checkbox

Sometimes it's desirable to see what changes were introduced by a
merge commit, rather than how conflicts were resolved. This adds
a checkbox which, when turned on, makes gitk show the equivalent
of "git show --first-parent <commit>" for merge commits.

Signed-off-by: Stefan Haller <redacted>
---
I realize this conflicts with Thomas Rast's recent patch to
add a word-diff dropdown box; things are fighting for space
in the diff pane header...

 gitk |   24 +++++++++++++++++++++---
 1 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/gitk b/gitk
index 45e3380..db0f022 100755
--- a/gitk
+++ b/gitk

@@ -2245,6 +2245,9 @@ proc makewindow {} {
     ${NS}::checkbutton .bleft.mid.ignspace -text [mc "Ignore space change"] \
 	-command changeignorespace -variable ignorespace
     pack .bleft.mid.ignspace -side left -padx 5
+    ${NS}::checkbutton .bleft.mid.firstparent -text [mc "First parent"] \
+	-command changefirstparent -variable firstparent
+    pack .bleft.mid.firstparent -side left -padx 5
     set ctext .bleft.bottom.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-state disabled -font textfont \

@@ -6872,6 +6875,7 @@ proc selectline {l isnew {desired_loc {}}} {
     global cmitmode showneartags allcommits
     global targetrow targetid lastscrollrows
     global autoselect jump_to_here
+    global firstparent
 
     catch {unset pending_select}
     $canv delete hover

@@ -7013,7 +7017,7 @@ proc selectline {l isnew {desired_loc {}}} {
     init_flist [mc "Comments"]
     if {$cmitmode eq "tree"} {
 	gettree $id
-    } elseif {[llength $olds] <= 1} {
+    } elseif {[llength $olds] <= 1 || $firstparent} {
 	startdiff $id
     } else {
 	mergediff $id

@@ -7416,7 +7420,7 @@ proc diffcmd {ids flags} {
 proc gettreediffs {ids} {
     global treediff treepending
 
-    if {[catch {set gdtf [open [diffcmd $ids {--no-commit-id}] r]}]} return
+    if {[catch {set gdtf [open [diffcmd $ids {--no-commit-id -m --first-parent}] r]}]} return
 
     set treepending $ids
     set treediff {}

@@ -7504,11 +7508,19 @@ proc changeignorespace {} {
     reselectline
 }
 
+proc changefirstparent {} {
+    global treediffs
+    catch {unset treediffs}
+
+    reselectline
+}
+
 proc getblobdiffs {ids} {
     global blobdifffd diffids env
     global diffinhdr treediffs
     global diffcontext
     global ignorespace
+    global firstparent
     global limitdiffs vfilelimit curview
     global diffencoding targetline diffnparents
     global git_version currdiffsubmod

@@ -7521,10 +7533,15 @@ proc getblobdiffs {ids} {
     if {[package vcompare $git_version "1.6.6"] >= 0} {
 	set submodule "--submodule"
     }
-    set cmd [diffcmd $ids "-p $textconv $submodule  -C --cc --no-commit-id -U$diffcontext"]
+    set cmd [diffcmd $ids "-p $textconv $submodule  -C --no-commit-id -U$diffcontext"]
     if {$ignorespace} {
 	append cmd " -w"
     }
+    if {$firstparent} {
+	append cmd " -m --first-parent"
+    } else {
+	append cmd " --cc"
+    }
     if {$limitdiffs && $vfilelimit($curview) ne {}} {
 	set cmd [concat $cmd -- $vfilelimit($curview)]
     }

@@ -11393,6 +11410,7 @@ if {[tk windowingsystem] eq "win32"} {
 set diffcolors {red "#00a000" blue}
 set diffcontext 3
 set ignorespace 0
+set firstparent 0
 set markbgcolor "#e0e0ff"
 
 set circlecolors {white blue gray blue blue}

-- 
1.7.3.2.153.g8250e

Stefan Haller wrote:

Sometimes it's desirable to see what changes were introduced by a
merge commit, rather than how conflicts were resolved. This adds
a checkbox which, when turned on, makes gitk show the equivalent
of "git show --first-parent <commit>" for merge commits.

To be clear: this is a diff option (like -m but limited to one
parent), not a history traversal option, right?

I realize this conflicts with Thomas Rast's recent patch to
add a word-diff dropdown box; things are fighting for space
in the diff pane header...

Any ideas for addressing the space shortage?

Jonathan Nieder [off-list ref] wrote:

Stefan Haller wrote:

quoted

Sometimes it's desirable to see what changes were introduced by a
merge commit, rather than how conflicts were resolved. This adds
a checkbox which, when turned on, makes gitk show the equivalent
of "git show --first-parent <commit>" for merge commits.

To be clear: this is a diff option (like -m but limited to one
parent), not a history traversal option, right?

Yes, that's right.  There is an existing history traversal option called
"Limit to first parent" in the "Edit View" dialog, which can be
controlled with the --first-parent command line option.  Do you think
this will cause confusion?

quoted

I realize this conflicts with Thomas Rast's recent patch to
add a word-diff dropdown box; things are fighting for space
in the diff pane header...

Any ideas for addressing the space shortage?

Matthieu made the suggestion of "line-wrapping" the widgets in the diff
pane header if it becomes too narrow.  I guess this could work; I don't
know enough Tcl/Tk to be able to try it out though.


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

Stefan Haller wrote:

quoted

Stefan Haller wrote:

quoted

quoted

I realize this conflicts with Thomas Rast's recent patch to
add a word-diff dropdown box; things are fighting for space
in the diff pane header...

[...]

Matthieu made the suggestion of "line-wrapping" the widgets in the diff
pane header if it becomes too narrow.  I guess this could work; I don't
know enough Tcl/Tk to be able to try it out though.

Hmm.  I don't like where this method tends in the limit.

Maybe we need a notion of a "diff view", for setting up various
diff-tree options (word diff, whitespace options, context lines,
patience, diffstat, first-parent)?  The diff pane could then provide a
drop-down box for diff views already set up.

Jonathan Nieder [off-list ref] wrote:

Stefan Haller wrote:

quoted

Matthieu made the suggestion of "line-wrapping" the widgets in the diff
pane header if it becomes too narrow.

Hmm.  I don't like where this method tends in the limit.

Maybe we need a notion of a "diff view", for setting up various
diff-tree options (word diff, whitespace options, context lines,
patience, diffstat, first-parent)?  The diff pane could then provide a
drop-down box for diff views already set up.

I don't think I would like this.  Most of the diff related options are
ones that I want to toggle with one click.  That's certainly true for
"Ignore space change", "First parent", and the "Diff/Old Version/New
Version" radio buttons.  I would hate to see any of them be removed from
the diff pane.


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

Stefan Haller wrote:

                                  Most of the diff related options are
ones that I want to toggle with one click.  That's certainly true for
"Ignore space change", "First parent", and the "Diff/Old Version/New
Version" radio buttons.  I would hate to see any of them be removed from
the diff pane.

Thanks, that's useful information.  Could you give example workflows
to illustrate this?  Here's an example:

   Toggling between "Diff/Old version/New version" with a single click
   allows one to compare the preimage and postimage of a patch visually.
   It makes what changed much more obvious.

   Historically these have been radio buttons on the top-left part of
   the diff pane.  If they move, it might be hard for people already
   familiar with gitk to find them.

   The preimage/postimage toggle is a useful and novel feature, and it
   is greedy with respect to screen real estate (a few extra diff
   lines can go a long way), so it would not be good to hide it in a
   separate "more diff options" toolbar.

The case for one-click "diff against parent" is less clear.  Is it
that it gets toggled in practice often (i.e., avoiding pointless
repetitive actions)?

Hope that helps.

Jonathan Nieder [off-list ref] wrote:

Stefan Haller wrote:

quoted

                                  Most of the diff related options are
ones that I want to toggle with one click.  That's certainly true for
"Ignore space change", "First parent", and the "Diff/Old Version/New
Version" radio buttons.  I would hate to see any of them be removed from
the diff pane.

Thanks, that's useful information.  Could you give example workflows
to illustrate this?

When browsing history, I usually have "First parent" off, so that I see
empty diffs for most merges, except the ones that had conflicts.
Occasionally though, when looking at a merge of a larger branch, I want
to see what changes were introduced by the merge; turning on "First
parent" allows me to quickly see that, or in general gives me an idea of
just how "big" the merge was.  I usually turn it back off right
afterwards.

Similar for "Ignore space change": I usually have it off most of the
time, so that I see faithful diffs.  Occasionally, when just the
indentation of a block of C code has changed, I turn it on just for that
one diff to make it easier to read; then I turn it back off right
afterwards.

Also similar for "Lines of context": this is usually set to a medium
value, like 3 or 4; sometimes I'll increase it to 12 or more for a
single diff to see more of the surrounding code, and sometimes I'll set
it to 1 for a merge diff with hunks so close to each other that they
otherwise show as conflicts.

Your proposal of having presets for diff options has the drawback that I
would need a preset for each combination of these options.

-Stefan


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

On Mon, Nov 08, 2010 at 11:42:59AM +0100, Stefan Haller wrote:

Sometimes it's desirable to see what changes were introduced by a
merge commit, rather than how conflicts were resolved. This adds
a checkbox which, when turned on, makes gitk show the equivalent
of "git show --first-parent <commit>" for merge commits.

Signed-off-by: Stefan Haller <redacted>
---
I realize this conflicts with Thomas Rast's recent patch to
add a word-diff dropdown box; things are fighting for space
in the diff pane header...

I just applied Thomas Rast's patch, so you'll need to rebase.  Also
you're right that we're running out of space; perhaps we need to make
the pane header two rows high.  Finally, "First parent" doesn't really
convey to me immediately what it does -- I have to think about it, so
it will probably confuse new users.  I don't know what would be
better, though.

Paul.

From: Stefan Haller <redacted>

Sometimes it's desirable to see what changes were introduced by a
merge commit, rather than how conflicts were resolved. This adds
a checkbox which, when turned on, makes gitk show the equivalent
of "git show --first-parent <commit>" for merge commits.

Signed-off-by: Stefan Haller <redacted>
---
Paul Mackerras [off-list ref] wrote:

I just applied Thomas Rast's patch, so you'll need to rebase. 

OK, here's a new patch, rebased onto current master (but otherwise
unchanged for now).

Also you're right that we're running out of space; perhaps we need to make
the pane header two rows high.

The suggestion was to make it two rows high only if it doesn't fit on
one row (i.e. dynamically "line-wrap"), and I like the idea.  Unfortunately
that's beyond my Tk skills; anybody willing to help?

Finally, "First parent" doesn't really convey to me immediately what it
does -- I have to think about it, so it will probably confuse new users.
I don't know what would be better, though.

What I like about it is that it's consistent with the command-line
client, "git show --first-parent".  But I don't insist on it if anybody
has a better suggestion.

 gitk |   25 ++++++++++++++++++++++---
 1 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/gitk b/gitk
index e82c6bf..7201ba0 100755
--- a/gitk
+++ b/gitk

@@ -2269,6 +2269,10 @@ proc makewindow {} {
 	pack .bleft.mid.worddiff -side left -padx 5
     }
 
+    ${NS}::checkbutton .bleft.mid.firstparent -text [mc "First parent"] \
+	-command changefirstparent -variable firstparent
+    pack .bleft.mid.firstparent -side left -padx 5
+
     set ctext .bleft.bottom.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-state disabled -font textfont \

@@ -6897,6 +6901,7 @@ proc selectline {l isnew {desired_loc {}}} {
     global cmitmode showneartags allcommits
     global targetrow targetid lastscrollrows
     global autoselect jump_to_here
+    global firstparent
 
     catch {unset pending_select}
     $canv delete hover

@@ -7038,7 +7043,7 @@ proc selectline {l isnew {desired_loc {}}} {
     init_flist [mc "Comments"]
     if {$cmitmode eq "tree"} {
 	gettree $id
-    } elseif {[llength $olds] <= 1} {
+    } elseif {[llength $olds] <= 1 || $firstparent} {
 	startdiff $id
     } else {
 	mergediff $id

@@ -7442,7 +7447,7 @@ proc diffcmd {ids flags} {
 proc gettreediffs {ids} {
     global treediff treepending
 
-    if {[catch {set gdtf [open [diffcmd $ids {--no-commit-id}] r]}]} return
+    if {[catch {set gdtf [open [diffcmd $ids {--no-commit-id -m --first-parent}] r]}]} return
 
     set treepending $ids
     set treediff {}

@@ -7534,12 +7539,20 @@ proc changeworddiff {name ix op} {
     reselectline
 }
 
+proc changefirstparent {} {
+    global treediffs
+    catch {unset treediffs}
+
+    reselectline
+}
+
 proc getblobdiffs {ids} {
     global blobdifffd diffids env
     global diffinhdr treediffs
     global diffcontext
     global ignorespace
     global worddiff
+    global firstparent
     global limitdiffs vfilelimit curview
     global diffencoding targetline diffnparents
     global git_version currdiffsubmod

@@ -7552,13 +7565,18 @@ proc getblobdiffs {ids} {
     if {[package vcompare $git_version "1.6.6"] >= 0} {
 	set submodule "--submodule"
     }
-    set cmd [diffcmd $ids "-p $textconv $submodule  -C --cc --no-commit-id -U$diffcontext"]
+    set cmd [diffcmd $ids "-p $textconv $submodule  -C --no-commit-id -U$diffcontext"]
     if {$ignorespace} {
 	append cmd " -w"
     }
     if {$worddiff ne [mc "Line diff"]} {
 	append cmd " --word-diff=porcelain"
     }
+    if {$firstparent} {
+	append cmd " -m --first-parent"
+    } else {
+	append cmd " --cc"
+    }
     if {$limitdiffs && $vfilelimit($curview) ne {}} {
 	set cmd [concat $cmd -- $vfilelimit($curview)]
     }

@@ -11453,6 +11471,7 @@ set diffcolors {red "#00a000" blue}
 set diffcontext 3
 set ignorespace 0
 set worddiff ""
+set firstparent 0
 set markbgcolor "#e0e0ff"
 
 set circlecolors {white blue gray blue blue}

-- 
1.7.3.2.442.g97e50