Make "gitk" work better with dense revlists

Make "gitk" work better with dense revlists

[Linus Torvalds]

To generate the diff for a commit, gitk used to do

	git-diff-tree -p -C $p $id

(and same thing to generate filenames, except using just "-r" there) which 
does actually generate the diff from the parent to the $id, exactly like 
it meant to do.

However, that really sucks with --dense, where the "parent" information 
has all been rewritten to point to the previous commit. The diff actually 
works exactly right, but now it's the diff of the _whole_ sequence of 
commits all the way to the previous commit that last changed the file(s) 
that we are looking at.

And that's really not what we want 99.9% of the time, even if it may be 
perfectly sensible. Not only will the diff not actually match the commit 
message, but it will usually be _huge_, and all of it will be totally 
uninteresting to us, since we were only interested in a particular set of 
files.

It also doesn't match what we do when we write the patch to a file.

So this makes gitk just show the diff of _that_ commit.

We might even want to have some way to limit the diff to only the 
filenames we're interested in, but it's often nice to see what else 
changed at the same time, so that's secondary.

The merge diff handling is left alone, although I think that should also 
be changed to only look at what that _particular_ merge did, not what it 
did when compared to the faked-out parents.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

Hmm?

Also, having now tested the previous "handle root commit in the 
TREECHANGED" logic a bit more, I think it's (a) stable and (b) the right 
thing to do. Sign me off on that one too.

		Linus

diff --git a/gitk b/gitk
index f1ea4e1..a9d37d9 100755
--- a/gitk
+++ b/gitk
@@ -2806,7 +2806,7 @@ proc gettreediffs {ids} {
     set treediff {}
     set id [lindex $ids 0]
     set p [lindex $ids 1]
-    if [catch {set gdtf [open "|git-diff-tree -r $p $id" r]}] return
+    if [catch {set gdtf [open "|git-diff-tree -r $id" r]}] return
     fconfigure $gdtf -blocking 0
     fileevent $gdtf readable [list gettreediffline $gdtf $ids]
 }
@@ -2842,7 +2842,7 @@ proc getblobdiffs {ids} {
     set id [lindex $ids 0]
     set p [lindex $ids 1]
     set env(GIT_DIFF_OPTS) $diffopts
-    set cmd [list | git-diff-tree -r -p -C $p $id]
+    set cmd [list | git-diff-tree -r -p -C $id]
     if {[catch {set bdf [open $cmd r]} err]} {
 	puts "error getting diffs: $err"
 	return

Re: Make "gitk" work better with dense revlists

[Paul Mackerras]

Linus Torvalds writes:

> So this makes gitk just show the diff of _that_ commit.

Committed and pushed out, but ...

> Also, having now tested the previous "handle root commit in the 
> TREECHANGED" logic a bit more, I think it's (a) stable and (b) the right 
> thing to do. Sign me off on that one too.

What is that about?

I'm hoping to get back to gitk hacking RSN - I've been going flat out
on the ppc32/ppc64 merge.  Thanks for doing the --dense thing; I was
thinking about doing something like that inside gitk but doing it in
git-rev-list is better.  It does mean that I now want to be able to
get gitk to contract the view to just a given set of files or
directories and then expand back to the whole tree view, which means
running git-rev-list multiple times, which gitk can't do at the
moment...

Paul.

Re: Make "gitk" work better with dense revlists

[Junio C Hamano]

Paul Mackerras <paulus@samba.org> writes:

> Linus Torvalds writes:
>
>> So this makes gitk just show the diff of _that_ commit.
>
> Committed and pushed out, but ...

Thanks.  Pulled and pushed out.

Re: Make "gitk" work better with dense revlists

[Linus Torvalds]

On Thu, 27 Oct 2005, Paul Mackerras wrote:
>
> Linus Torvalds writes:
> >
> > So this makes gitk just show the diff of _that_ commit.
> 
> Committed and pushed out, but ...

Btw, the "show diff for commit" fails for the root case - you never see 
the initial commit. 

That is definitely correct for big archives that started out of nothing 
(due to being imported), but it's sad/wrong for something like git itself, 
and it'a usually wrong when using --dense and a filename, since then the 
diff is often smaller (ie it might be a simple rename).

> > Also, having now tested the previous "handle root commit in the 
> > TREECHANGED" logic a bit more, I think it's (a) stable and (b) the right 
> > thing to do. Sign me off on that one too.
> 
> What is that about?

That's just me improving how --dense works wrt root commits. Junio already 
merged it ("git-rev-list: fix --dense flag")

> I'm hoping to get back to gitk hacking RSN - I've been going flat out
> on the ppc32/ppc64 merge.  Thanks for doing the --dense thing; I was
> thinking about doing something like that inside gitk but doing it in
> git-rev-list is better.

A _lot_ better. You'd have been totally screwed performance-wise trying to 
do it in tcl/tk and executing git-diff-tree for everything.

>		  It does mean that I now want to be able to
> get gitk to contract the view to just a given set of files or
> directories and then expand back to the whole tree view, which means
> running git-rev-list multiple times, which gitk can't do at the
> moment...

Yes. And please also give the option to contract the diffs to a set of 
files (I think that should be independently controlled, although perhaps 
with some way to set them both at the same time).

		Linus

Re: Make "gitk" work better with dense revlists

[Sven Verdoolaege]

On Thu, Oct 27, 2005 at 04:16:25PM +1000, Paul Mackerras wrote:
> git-rev-list is better.  It does mean that I now want to be able to
> get gitk to contract the view to just a given set of files or
> directories and then expand back to the whole tree view, which means
> running git-rev-list multiple times, which gitk can't do at the
> moment...

My "gitk Update" patch could be useful here.
Instead of being careful about what is new and what is old,
you can just throw everything out.

skimo
------------------

gitk: add Update menu item.

Update will redraw the commits if any commits have been added to any
of the selected heads.  The new commits appear on the top.

Signed-off-by: Sven Verdoolaege <skimo@kotnet.org>

---
commit d867cc8df4b7756dbc738358540e845ac5cd8a86
tree f762e01ce91365d95cfed147f3b605d492aeb7f4
parent f3123c4ab3d3698262e59561ac084de45b10365a
author Sven Verdoolaege <skimo@kotnet.org> Thu, 22 Sep 2005 15:27:19 +0200
committer Sven Verdoolaege <skimo@kotnet.org> Wed, 26 Oct 2005 23:05:23 +0200

 gitk |  175 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 141 insertions(+), 34 deletions(-)

diff --git a/gitk b/gitk
index f1ea4e1..8c37b60 100755
--- a/gitk
+++ b/gitk
@@ -16,8 +16,24 @@ proc gitdir {} {
     }
 }
 
+proc parse_args {rargs} {
+    global parsed_args
+
+    if [catch {
+	set parse_args [concat --default HEAD $rargs]
+	set parsed_args [split [eval exec git-rev-parse $parse_args] "\n"]
+    }] {
+	# if git-rev-parse failed for some reason...
+	if {$rargs == {}} {
+	    set rargs HEAD
+	}
+	set parsed_args $rargs
+    }
+    return $parsed_args
+}
+
 proc getcommits {rargs} {
-    global commits commfd phase canv mainfont env
+    global oldcommits commits commfd phase canv mainfont env
     global startmsecs nextupdate ncmupdate
     global ctext maincursor textcursor leftover
 
@@ -27,21 +43,13 @@ proc getcommits {rargs} {
 	error_popup "Cannot find the git directory \"$gitdir\"."
 	exit 1
     }
+    set oldcommits {}
     set commits {}
     set phase getcommits
     set startmsecs [clock clicks -milliseconds]
     set nextupdate [expr $startmsecs + 100]
     set ncmupdate 1
-    if [catch {
-	set parse_args [concat --default HEAD $rargs]
-	set parsed_args [split [eval exec git-rev-parse $parse_args] "\n"]
-    }] {
-	# if git-rev-parse failed for some reason...
-	if {$rargs == {}} {
-	    set rargs HEAD
-	}
-	set parsed_args $rargs
-    }
+    set parsed_args [parse_args $rargs]
     if [catch {
 	set commfd [open "|git-rev-list --header --topo-order --parents $parsed_args" r]
     } err] {
@@ -59,9 +67,10 @@ proc getcommits {rargs} {
 }
 
 proc getcommitlines {commfd}  {
-    global commits parents cdate children
+    global oldcommits commits parents cdate children nchildren
     global commitlisted phase commitinfo nextupdate
     global stopped redisplaying leftover
+    global canv
 
     set stuff [read $commfd]
     if {$stuff == {}} {
@@ -119,10 +128,18 @@ to allow selection of commits to be disp
 	set id [lindex $ids 0]
 	set olds [lrange $ids 1 end]
 	set cmit [string range $cmit [expr {$j + 1}] end]
+	if {$phase == "updatecommits"} {
+	    $canv delete all
+	    set oldcommits $commits
+	    set commits {}
+	    unset children
+	    unset nchildren
+	    set phase getcommits
+	}
 	lappend commits $id
 	set commitlisted($id) 1
 	parsecommit $id $cmit 1 [lrange $ids 1 end]
-	drawcommit $id
+	drawcommit $id 1
 	if {[clock clicks -milliseconds] >= $nextupdate} {
 	    doupdate 1
 	}
@@ -132,7 +149,7 @@ to allow selection of commits to be disp
 		set stopped 0
 		set phase "getcommits"
 		foreach id $commits {
-		    drawcommit $id
+		    drawcommit $id 1
 		    if {$stopped} break
 		    if {[clock clicks -milliseconds] >= $nextupdate} {
 			doupdate 1
@@ -168,16 +185,9 @@ proc readcommit {id} {
     parsecommit $id $contents 0 {}
 }
 
-proc parsecommit {id contents listed olds} {
-    global commitinfo children nchildren parents nparents cdate ncleft
+proc updatechildren {id olds} {
+    global children nchildren parents nparents ncleft
 
-    set inhdr 1
-    set comment {}
-    set headline {}
-    set auname {}
-    set audate {}
-    set comname {}
-    set comdate {}
     if {![info exists nchildren($id)]} {
 	set children($id) {}
 	set nchildren($id) 0
@@ -196,6 +206,19 @@ proc parsecommit {id contents listed old
 	    incr ncleft($p)
 	}
     }
+}
+
+proc parsecommit {id contents listed olds} {
+    global commitinfo cdate
+
+    set inhdr 1
+    set comment {}
+    set headline {}
+    set auname {}
+    set audate {}
+    set comname {}
+    set comdate {}
+    updatechildren $id $olds
     foreach line [split $contents "\n"] {
 	if {$inhdr} {
 	    if {$line == {}} {
@@ -240,6 +263,9 @@ proc parsecommit {id contents listed old
 proc readrefs {} {
     global tagids idtags headids idheads tagcontents
 
+    foreach v {tagids idtags headids idheads otherrefids idotherrefs} {
+	catch {unset $v}
+    }
     set tags [glob -nocomplain -types f [gitdir]/refs/tags/*]
     foreach f $tags {
 	catch {
@@ -324,7 +350,7 @@ proc error_popup msg {
     tkwait window $w
 }
 
-proc makewindow {} {
+proc makewindow {rargs} {
     global canv canv2 canv3 linespc charspc ctext cflist textfont
     global findtype findtypemenu findloc findstring fstring geometry
     global entries sha1entry sha1string sha1but
@@ -334,6 +360,7 @@ proc makewindow {} {
     menu .bar
     .bar add cascade -label "File" -menu .bar.file
     menu .bar.file
+    .bar.file add command -label "Update" -command [list updatecommits $rargs]
     .bar.file add command -label "Reread references" -command rereadrefs
     .bar.file add command -label "Quit" -command doquit
     menu .bar.help
@@ -1445,7 +1472,7 @@ proc decidenext {{noread 0}} {
     return $level
 }
 
-proc drawcommit {id} {
+proc drawcommit {id reading} {
     global phase todo nchildren datemode nextupdate
     global numcommits ncmupdate displayorder todo onscreen
 
@@ -1474,20 +1501,29 @@ proc drawcommit {id} {
 	    break
 	}
     }
-    drawmore 1
+    drawmore $reading
 }
 
 proc finishcommits {} {
-    global phase
+    global phase oldcommits commits
     global canv mainfont ctext maincursor textcursor
+    global parents
 
-    if {$phase != "incrdraw"} {
+    if {$phase == "incrdraw" || $phase == "removecommits"} {
+	foreach id $oldcommits {
+	    lappend commits $id
+	    updatechildren $id $parents($id)
+	    drawcommit $id 0
+	}
+	set oldcommits {}
+	drawrest
+    } elseif {$phase == "updatecommits"} {
+	set phase {}
+    } else {
 	$canv delete all
 	$canv create text 3 3 -anchor nw -text "No commits selected" \
 	    -font $mainfont -tags textitems
 	set phase {}
-    } else {
-	drawrest
     }
     . config -cursor $maincursor
     settextcursor $textcursor
@@ -3611,9 +3647,6 @@ proc rereadrefs {} {
 	    set ref($id) [listrefs $id]
 	}
     }
-    foreach v {tagids idtags headids idheads otherrefids idotherrefs} {
-	catch {unset $v}
-    }
     readrefs
     set refids [lsort -unique [concat $refids [array names idtags] \
 			[array names idheads] [array names idotherrefs]]]
@@ -3625,6 +3658,80 @@ proc rereadrefs {} {
     }
 }
 
+proc updatecommits {rargs} {
+    global commitlisted commfd phase
+    global startmsecs nextupdate ncmupdate
+    global idtags idheads idotherrefs
+    global leftover
+    global parsed_args
+    global canv
+    global oldcommits commits
+    global parents nchildren children ncleft
+
+    set old_args $parsed_args
+    parse_args $rargs
+
+    foreach id $old_args {
+	if {![regexp {^[0-9a-f]{40}$} $id]} continue
+	if {[info exists oldref($id)]} continue
+	set oldref($id) $id
+	lappend ignoreold "^$id"
+    }
+    foreach id $parsed_args {
+	if {![regexp {^[0-9a-f]{40}$} $id]} continue
+	if {[info exists ref($id)]} continue
+	set ref($id) $id
+	lappend ignorenew "^$id"
+    }
+
+    foreach a $old_args {
+	if {![info exists ref($a)]} {
+	    lappend ignorenew $a
+	}
+    }
+
+    set phase updatecommits
+    set removed_commits [split [eval exec git-rev-list $ignorenew] "\n" ]
+    if {[llength $removed_commits] > 0} {
+	$canv delete all
+	set oldcommits {}
+	foreach c $commits {
+	    if {[lsearch $c $removed_commits] < 0} {
+		lappend oldcommits $c
+	    } else {
+		unset commitlisted($c)
+	    }
+	}
+	set commits {}
+	unset children
+	unset nchildren
+	set phase removecommits
+    }
+
+    set args {}
+    foreach a $parsed_args {
+	if {![info exists oldref($a)]} {
+	    lappend args $a
+	}
+    }
+
+    readrefs
+    if [catch {
+	set commfd [open "|git-rev-list --header --topo-order --parents $args $ignoreold" r]
+    } err] {
+	puts stderr "Error executing git-rev-list: $err"
+	exit 1
+    }
+    set startmsecs [clock clicks -milliseconds]
+    set nextupdate [expr $startmsecs + 100]
+    set ncmupdate 1
+    set leftover {}
+    fconfigure $commfd -blocking 0 -translation lf
+    fileevent $commfd readable [list getcommitlines $commfd]
+    . config -cursor watch
+    settextcursor watch
+}
+
 proc showtag {tag isnew} {
     global ctext cflist tagcontents tagids linknum
 
@@ -3692,6 +3799,6 @@ set redisplaying 0
 set stuffsaved 0
 set patchnum 0
 setcoords
-makewindow
+makewindow $revtreeargs
 readrefs
 getcommits $revtreeargs