{"thread":{"id":"2222","subject":"Make \"gitk\" work better with dense revlists","startedAt":"2005-10-25T20:01:42Z","lastAt":"2005-10-27T15:04:47Z","messageCount":5,"participants":["Linus Torvalds","Paul Mackerras","Junio C Hamano","Sven Verdoolaege"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"10604","messageId":"Pine.LNX.4.64.0510251253110.10477@g5.osdl.org","threadId":"2222","inReplyTo":null,"subject":"Make \"gitk\" work better with dense revlists","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-10-25T20:01:42Z","receivedAt":"2005-10-25T20:01:42Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nTo generate the diff for a commit, gitk used to do\n\n\tgit-diff-tree -p -C $p $id\n\n(and same thing to generate filenames, except using just \"-r\" there) which \ndoes actually generate the diff from the parent to the $id, exactly like \nit meant to do.\n\nHowever, that really sucks with --dense, where the \"parent\" information \nhas all been rewritten to point to the previous commit. The diff actually \nworks exactly right, but now it's the diff of the _whole_ sequence of \ncommits all the way to the previous commit that last changed the file(s) \nthat we are looking at.\n\nAnd that's really not what we want 99.9% of the time, even if it may be \nperfectly sensible. Not only will the diff not actually match the commit \nmessage, but it will usually be _huge_, and all of it will be totally \nuninteresting to us, since we were only interested in a particular set of \nfiles.\n\nIt also doesn't match what we do when we write the patch to a file.\n\nSo this makes gitk just show the diff of _that_ commit.\n\nWe might even want to have some way to limit the diff to only the \nfilenames we're interested in, but it's often nice to see what else \nchanged at the same time, so that's secondary.\n\nThe merge diff handling is left alone, although I think that should also \nbe changed to only look at what that _particular_ merge did, not what it \ndid when compared to the faked-out parents.\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n---\n\nHmm?\n\nAlso, having now tested the previous \"handle root commit in the \nTREECHANGED\" logic a bit more, I think it's (a) stable and (b) the right \nthing to do. Sign me off on that one too.\n\n\t\tLinus\n\ndiff --git a/gitk b/gitk\nindex f1ea4e1..a9d37d9 100755\n--- a/gitk\n+++ b/gitk\n@@ -2806,7 +2806,7 @@ proc gettreediffs {ids} {\n     set treediff {}\n     set id [lindex $ids 0]\n     set p [lindex $ids 1]\n-    if [catch {set gdtf [open \"|git-diff-tree -r $p $id\" r]}] return\n+    if [catch {set gdtf [open \"|git-diff-tree -r $id\" r]}] return\n     fconfigure $gdtf -blocking 0\n     fileevent $gdtf readable [list gettreediffline $gdtf $ids]\n }\n@@ -2842,7 +2842,7 @@ proc getblobdiffs {ids} {\n     set id [lindex $ids 0]\n     set p [lindex $ids 1]\n     set env(GIT_DIFF_OPTS) $diffopts\n-    set cmd [list | git-diff-tree -r -p -C $p $id]\n+    set cmd [list | git-diff-tree -r -p -C $id]\n     if {[catch {set bdf [open $cmd r]} err]} {\n \tputs \"error getting diffs: $err\"\n \treturn\n"},{"id":"10690","messageId":"17248.28857.90315.543669@cargo.ozlabs.ibm.com","threadId":"2222","inReplyTo":"Pine.LNX.4.64.0510251253110.10477@g5.osdl.org","subject":"Re: Make \"gitk\" work better with dense revlists","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2005-10-27T06:16:25Z","receivedAt":"2005-10-27T06:16:25Z","isPatch":false,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Linus Torvalds writes:\n\n> So this makes gitk just show the diff of _that_ commit.\n\nCommitted and pushed out, but ...\n\n> Also, having now tested the previous \"handle root commit in the \n> TREECHANGED\" logic a bit more, I think it's (a) stable and (b) the right \n> thing to do. Sign me off on that one too.\n\nWhat is that about?\n\nI'm hoping to get back to gitk hacking RSN - I've been going flat out\non the ppc32/ppc64 merge.  Thanks for doing the --dense thing; I was\nthinking about doing something like that inside gitk but doing it in\ngit-rev-list is better.  It does mean that I now want to be able to\nget gitk to contract the view to just a given set of files or\ndirectories and then expand back to the whole tree view, which means\nrunning git-rev-list multiple times, which gitk can't do at the\nmoment...\n\nPaul.\n"},{"id":"10692","messageId":"7vhdb34dm3.fsf@assigned-by-dhcp.cox.net","threadId":"2222","inReplyTo":"17248.28857.90315.543669@cargo.ozlabs.ibm.com","subject":"Re: Make \"gitk\" work better with dense revlists","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-27T07:34:44Z","receivedAt":"2005-10-27T07:34:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras <paulus@samba.org> writes:\n\n> Linus Torvalds writes:\n>\n>> So this makes gitk just show the diff of _that_ commit.\n>\n> Committed and pushed out, but ...\n\nThanks.  Pulled and pushed out.\n"},{"id":"10706","messageId":"Pine.LNX.4.64.0510270743490.4664@g5.osdl.org","threadId":"2222","inReplyTo":"17248.28857.90315.543669@cargo.ozlabs.ibm.com","subject":"Re: Make \"gitk\" work better with dense revlists","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-10-27T14:51:32Z","receivedAt":"2005-10-27T14:51:32Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 27 Oct 2005, Paul Mackerras wrote:\n>\n> Linus Torvalds writes:\n> >\n> > So this makes gitk just show the diff of _that_ commit.\n> \n> Committed and pushed out, but ...\n\nBtw, the \"show diff for commit\" fails for the root case - you never see \nthe initial commit. \n\nThat is definitely correct for big archives that started out of nothing \n(due to being imported), but it's sad/wrong for something like git itself, \nand it'a usually wrong when using --dense and a filename, since then the \ndiff is often smaller (ie it might be a simple rename).\n\n> > Also, having now tested the previous \"handle root commit in the \n> > TREECHANGED\" logic a bit more, I think it's (a) stable and (b) the right \n> > thing to do. Sign me off on that one too.\n> \n> What is that about?\n\nThat's just me improving how --dense works wrt root commits. Junio already \nmerged it (\"git-rev-list: fix --dense flag\")\n\n> I'm hoping to get back to gitk hacking RSN - I've been going flat out\n> on the ppc32/ppc64 merge.  Thanks for doing the --dense thing; I was\n> thinking about doing something like that inside gitk but doing it in\n> git-rev-list is better.\n\nA _lot_ better. You'd have been totally screwed performance-wise trying to \ndo it in tcl/tk and executing git-diff-tree for everything.\n\n>\t\t  It does mean that I now want to be able to\n> get gitk to contract the view to just a given set of files or\n> directories and then expand back to the whole tree view, which means\n> running git-rev-list multiple times, which gitk can't do at the\n> moment...\n\nYes. And please also give the option to contract the diffs to a set of \nfiles (I think that should be independently controlled, although perhaps \nwith some way to set them both at the same time).\n\n\t\tLinus\n"},{"id":"10707","messageId":"20051027150447.GZ8383MdfPADPa@greensroom.kotnet.org","threadId":"2222","inReplyTo":"17248.28857.90315.543669@cargo.ozlabs.ibm.com","subject":"Re: Make \"gitk\" work better with dense revlists","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2005-10-27T15:04:47Z","receivedAt":"2005-10-27T15:04:47Z","isPatch":false,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Thu, Oct 27, 2005 at 04:16:25PM +1000, Paul Mackerras wrote:\n> git-rev-list is better.  It does mean that I now want to be able to\n> get gitk to contract the view to just a given set of files or\n> directories and then expand back to the whole tree view, which means\n> running git-rev-list multiple times, which gitk can't do at the\n> moment...\n\nMy \"gitk Update\" patch could be useful here.\nInstead of being careful about what is new and what is old,\nyou can just throw everything out.\n\nskimo\n------------------\n\ngitk: add Update menu item.\n\nUpdate will redraw the commits if any commits have been added to any\nof the selected heads.  The new commits appear on the top.\n\nSigned-off-by: Sven Verdoolaege <skimo@kotnet.org>\n\n---\ncommit d867cc8df4b7756dbc738358540e845ac5cd8a86\ntree f762e01ce91365d95cfed147f3b605d492aeb7f4\nparent f3123c4ab3d3698262e59561ac084de45b10365a\nauthor Sven Verdoolaege <skimo@kotnet.org> Thu, 22 Sep 2005 15:27:19 +0200\ncommitter Sven Verdoolaege <skimo@kotnet.org> Wed, 26 Oct 2005 23:05:23 +0200\n\n gitk |  175 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------------\n 1 files changed, 141 insertions(+), 34 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex f1ea4e1..8c37b60 100755\n--- a/gitk\n+++ b/gitk\n@@ -16,8 +16,24 @@ proc gitdir {} {\n     }\n }\n \n+proc parse_args {rargs} {\n+    global parsed_args\n+\n+    if [catch {\n+\tset parse_args [concat --default HEAD $rargs]\n+\tset parsed_args [split [eval exec git-rev-parse $parse_args] \"\\n\"]\n+    }] {\n+\t# if git-rev-parse failed for some reason...\n+\tif {$rargs == {}} {\n+\t    set rargs HEAD\n+\t}\n+\tset parsed_args $rargs\n+    }\n+    return $parsed_args\n+}\n+\n proc getcommits {rargs} {\n-    global commits commfd phase canv mainfont env\n+    global oldcommits commits commfd phase canv mainfont env\n     global startmsecs nextupdate ncmupdate\n     global ctext maincursor textcursor leftover\n \n@@ -27,21 +43,13 @@ proc getcommits {rargs} {\n \terror_popup \"Cannot find the git directory \\\"$gitdir\\\".\"\n \texit 1\n     }\n+    set oldcommits {}\n     set commits {}\n     set phase getcommits\n     set startmsecs [clock clicks -milliseconds]\n     set nextupdate [expr $startmsecs + 100]\n     set ncmupdate 1\n-    if [catch {\n-\tset parse_args [concat --default HEAD $rargs]\n-\tset parsed_args [split [eval exec git-rev-parse $parse_args] \"\\n\"]\n-    }] {\n-\t# if git-rev-parse failed for some reason...\n-\tif {$rargs == {}} {\n-\t    set rargs HEAD\n-\t}\n-\tset parsed_args $rargs\n-    }\n+    set parsed_args [parse_args $rargs]\n     if [catch {\n \tset commfd [open \"|git-rev-list --header --topo-order --parents $parsed_args\" r]\n     } err] {\n@@ -59,9 +67,10 @@ proc getcommits {rargs} {\n }\n \n proc getcommitlines {commfd}  {\n-    global commits parents cdate children\n+    global oldcommits commits parents cdate children nchildren\n     global commitlisted phase commitinfo nextupdate\n     global stopped redisplaying leftover\n+    global canv\n \n     set stuff [read $commfd]\n     if {$stuff == {}} {\n@@ -119,10 +128,18 @@ to allow selection of commits to be disp\n \tset id [lindex $ids 0]\n \tset olds [lrange $ids 1 end]\n \tset cmit [string range $cmit [expr {$j + 1}] end]\n+\tif {$phase == \"updatecommits\"} {\n+\t    $canv delete all\n+\t    set oldcommits $commits\n+\t    set commits {}\n+\t    unset children\n+\t    unset nchildren\n+\t    set phase getcommits\n+\t}\n \tlappend commits $id\n \tset commitlisted($id) 1\n \tparsecommit $id $cmit 1 [lrange $ids 1 end]\n-\tdrawcommit $id\n+\tdrawcommit $id 1\n \tif {[clock clicks -milliseconds] >= $nextupdate} {\n \t    doupdate 1\n \t}\n@@ -132,7 +149,7 @@ to allow selection of commits to be disp\n \t\tset stopped 0\n \t\tset phase \"getcommits\"\n \t\tforeach id $commits {\n-\t\t    drawcommit $id\n+\t\t    drawcommit $id 1\n \t\t    if {$stopped} break\n \t\t    if {[clock clicks -milliseconds] >= $nextupdate} {\n \t\t\tdoupdate 1\n@@ -168,16 +185,9 @@ proc readcommit {id} {\n     parsecommit $id $contents 0 {}\n }\n \n-proc parsecommit {id contents listed olds} {\n-    global commitinfo children nchildren parents nparents cdate ncleft\n+proc updatechildren {id olds} {\n+    global children nchildren parents nparents ncleft\n \n-    set inhdr 1\n-    set comment {}\n-    set headline {}\n-    set auname {}\n-    set audate {}\n-    set comname {}\n-    set comdate {}\n     if {![info exists nchildren($id)]} {\n \tset children($id) {}\n \tset nchildren($id) 0\n@@ -196,6 +206,19 @@ proc parsecommit {id contents listed old\n \t    incr ncleft($p)\n \t}\n     }\n+}\n+\n+proc parsecommit {id contents listed olds} {\n+    global commitinfo cdate\n+\n+    set inhdr 1\n+    set comment {}\n+    set headline {}\n+    set auname {}\n+    set audate {}\n+    set comname {}\n+    set comdate {}\n+    updatechildren $id $olds\n     foreach line [split $contents \"\\n\"] {\n \tif {$inhdr} {\n \t    if {$line == {}} {\n@@ -240,6 +263,9 @@ proc parsecommit {id contents listed old\n proc readrefs {} {\n     global tagids idtags headids idheads tagcontents\n \n+    foreach v {tagids idtags headids idheads otherrefids idotherrefs} {\n+\tcatch {unset $v}\n+    }\n     set tags [glob -nocomplain -types f [gitdir]/refs/tags/*]\n     foreach f $tags {\n \tcatch {\n@@ -324,7 +350,7 @@ proc error_popup msg {\n     tkwait window $w\n }\n \n-proc makewindow {} {\n+proc makewindow {rargs} {\n     global canv canv2 canv3 linespc charspc ctext cflist textfont\n     global findtype findtypemenu findloc findstring fstring geometry\n     global entries sha1entry sha1string sha1but\n@@ -334,6 +360,7 @@ proc makewindow {} {\n     menu .bar\n     .bar add cascade -label \"File\" -menu .bar.file\n     menu .bar.file\n+    .bar.file add command -label \"Update\" -command [list updatecommits $rargs]\n     .bar.file add command -label \"Reread references\" -command rereadrefs\n     .bar.file add command -label \"Quit\" -command doquit\n     menu .bar.help\n@@ -1445,7 +1472,7 @@ proc decidenext {{noread 0}} {\n     return $level\n }\n \n-proc drawcommit {id} {\n+proc drawcommit {id reading} {\n     global phase todo nchildren datemode nextupdate\n     global numcommits ncmupdate displayorder todo onscreen\n \n@@ -1474,20 +1501,29 @@ proc drawcommit {id} {\n \t    break\n \t}\n     }\n-    drawmore 1\n+    drawmore $reading\n }\n \n proc finishcommits {} {\n-    global phase\n+    global phase oldcommits commits\n     global canv mainfont ctext maincursor textcursor\n+    global parents\n \n-    if {$phase != \"incrdraw\"} {\n+    if {$phase == \"incrdraw\" || $phase == \"removecommits\"} {\n+\tforeach id $oldcommits {\n+\t    lappend commits $id\n+\t    updatechildren $id $parents($id)\n+\t    drawcommit $id 0\n+\t}\n+\tset oldcommits {}\n+\tdrawrest\n+    } elseif {$phase == \"updatecommits\"} {\n+\tset phase {}\n+    } else {\n \t$canv delete all\n \t$canv create text 3 3 -anchor nw -text \"No commits selected\" \\\n \t    -font $mainfont -tags textitems\n \tset phase {}\n-    } else {\n-\tdrawrest\n     }\n     . config -cursor $maincursor\n     settextcursor $textcursor\n@@ -3611,9 +3647,6 @@ proc rereadrefs {} {\n \t    set ref($id) [listrefs $id]\n \t}\n     }\n-    foreach v {tagids idtags headids idheads otherrefids idotherrefs} {\n-\tcatch {unset $v}\n-    }\n     readrefs\n     set refids [lsort -unique [concat $refids [array names idtags] \\\n \t\t\t[array names idheads] [array names idotherrefs]]]\n@@ -3625,6 +3658,80 @@ proc rereadrefs {} {\n     }\n }\n \n+proc updatecommits {rargs} {\n+    global commitlisted commfd phase\n+    global startmsecs nextupdate ncmupdate\n+    global idtags idheads idotherrefs\n+    global leftover\n+    global parsed_args\n+    global canv\n+    global oldcommits commits\n+    global parents nchildren children ncleft\n+\n+    set old_args $parsed_args\n+    parse_args $rargs\n+\n+    foreach id $old_args {\n+\tif {![regexp {^[0-9a-f]{40}$} $id]} continue\n+\tif {[info exists oldref($id)]} continue\n+\tset oldref($id) $id\n+\tlappend ignoreold \"^$id\"\n+    }\n+    foreach id $parsed_args {\n+\tif {![regexp {^[0-9a-f]{40}$} $id]} continue\n+\tif {[info exists ref($id)]} continue\n+\tset ref($id) $id\n+\tlappend ignorenew \"^$id\"\n+    }\n+\n+    foreach a $old_args {\n+\tif {![info exists ref($a)]} {\n+\t    lappend ignorenew $a\n+\t}\n+    }\n+\n+    set phase updatecommits\n+    set removed_commits [split [eval exec git-rev-list $ignorenew] \"\\n\" ]\n+    if {[llength $removed_commits] > 0} {\n+\t$canv delete all\n+\tset oldcommits {}\n+\tforeach c $commits {\n+\t    if {[lsearch $c $removed_commits] < 0} {\n+\t\tlappend oldcommits $c\n+\t    } else {\n+\t\tunset commitlisted($c)\n+\t    }\n+\t}\n+\tset commits {}\n+\tunset children\n+\tunset nchildren\n+\tset phase removecommits\n+    }\n+\n+    set args {}\n+    foreach a $parsed_args {\n+\tif {![info exists oldref($a)]} {\n+\t    lappend args $a\n+\t}\n+    }\n+\n+    readrefs\n+    if [catch {\n+\tset commfd [open \"|git-rev-list --header --topo-order --parents $args $ignoreold\" r]\n+    } err] {\n+\tputs stderr \"Error executing git-rev-list: $err\"\n+\texit 1\n+    }\n+    set startmsecs [clock clicks -milliseconds]\n+    set nextupdate [expr $startmsecs + 100]\n+    set ncmupdate 1\n+    set leftover {}\n+    fconfigure $commfd -blocking 0 -translation lf\n+    fileevent $commfd readable [list getcommitlines $commfd]\n+    . config -cursor watch\n+    settextcursor watch\n+}\n+\n proc showtag {tag isnew} {\n     global ctext cflist tagcontents tagids linknum\n \n@@ -3692,6 +3799,6 @@ set redisplaying 0\n set stuffsaved 0\n set patchnum 0\n setcoords\n-makewindow\n+makewindow $revtreeargs\n readrefs\n getcommits $revtreeargs\n"}]}