[PATCH] Fix gitk this->selected diffs

[PATCH] Fix gitk this->selected diffs

[Yann Dirson]

The change made in 8b7e5d76e836396a097bb6f61cf930ea872a7bd3 to
accomodate dense revlists in single-commit diffs has broken computing
of diffs between arbitrary trees, which does need to consider two
commit ids.

This patch changes the two git-diff-tree calls to get the necessary
two ids in this case.  It does so by propagating a "singlecommit" flag
through all functions involved via an additional argument.

Signed-off-by: Yann Dirson <ydirson@altern.org>

---

this->selected diffs have been broken for a month now - I'd like it to
get fixed before 1.0 :)

A previous version of this patch was sent under the 'Make "gitk" work
better with dense revlists' thread.  I have done some minor cleanups
to it.  I'm still not comfortable with the number of funcs I had to
touch to propagate the information - any better idea ?

 gitk |   53 ++++++++++++++++++++++++++++++++---------------------
 1 files changed, 32 insertions(+), 21 deletions(-)

applies-to: 1dc0816b6c98ef90985d2761f1ab80739e4fbc7d
4664daceca4faac92f0e15d3e80824f172192d84
diff --git a/gitk b/gitk
index 3dd97e2..4841a80 100755
--- a/gitk
+++ b/gitk
@@ -2164,9 +2164,9 @@ proc selectline {l isnew} {
     $cflist delete 0 end
     $cflist insert end "Comments"
     if {$nparents($id) == 1} {
-	startdiff [concat $id $parents($id)]
+	startdiff [concat $id $parents($id)] 1
     } elseif {$nparents($id) > 1} {
-	mergediff $id
+	mergediff $id 1
     }
 }
 
@@ -2235,7 +2235,7 @@ proc goforw {} {
     }
 }
 
-proc mergediff {id} {
+proc mergediff {id singlecommit} {
     global parents diffmergeid diffmergegca mergefilelist diffpindex
 
     set diffmergeid $id
@@ -2246,7 +2246,7 @@ proc mergediff {id} {
 	    showmergediff
 	}
     } else {
-	contmergediff {}
+	contmergediff {} $singlecommit
     }
 }
 
@@ -2266,7 +2266,7 @@ proc findgca {ids} {
     return $gca
 }
 
-proc contmergediff {ids} {
+proc contmergediff {ids singlecommit} {
     global diffmergeid diffpindex parents nparents diffmergegca
     global treediffs mergefilelist diffids treepending
 
@@ -2283,7 +2283,7 @@ proc contmergediff {ids} {
 	if {![info exists treediffs($ids)]} {
 	    set diffids $ids
 	    if {![info exists treepending]} {
-		gettreediffs $ids
+		gettreediffs $ids $singlecommit
 	    }
 	    return
 	}
@@ -2761,39 +2761,45 @@ proc similarity {pnum l nlc f events} {
     return [expr {200 * $same / (2 * $same + $diff)}]
 }
 
-proc startdiff {ids} {
+proc startdiff {ids singlecommit} {
     global treediffs diffids treepending diffmergeid
 
     set diffids $ids
     catch {unset diffmergeid}
     if {![info exists treediffs($ids)]} {
 	if {![info exists treepending]} {
-	    gettreediffs $ids
+	    gettreediffs $ids $singlecommit
 	}
     } else {
-	addtocflist $ids
+	addtocflist $ids $singlecommit
     }
 }
 
-proc addtocflist {ids} {
+proc addtocflist {ids singlecommit} {
     global treediffs cflist
     foreach f $treediffs($ids) {
 	$cflist insert end $f
     }
-    getblobdiffs $ids
+    getblobdiffs $ids $singlecommit
 }
 
-proc gettreediffs {ids} {
+proc gettreediffs {ids singlecommit} {
     global treediff parents treepending
     set treepending $ids
     set treediff {}
     set id [lindex $ids 0]
-    if [catch {set gdtf [open "|git-diff-tree --no-commit-id -r $id" r]}] return
+    if {$singlecommit == 1} {
+	set range "$id"
+    } else {
+	set p [lindex $ids 1]
+	set range "$p $id"
+    }
+    if [catch {set gdtf [open "|git-diff-tree --no-commit-id -r $range" r]}] return
     fconfigure $gdtf -blocking 0
-    fileevent $gdtf readable [list gettreediffline $gdtf $ids]
+    fileevent $gdtf readable [list gettreediffline $gdtf $ids $singlecommit]
 }
 
-proc gettreediffline {gdtf ids} {
+proc gettreediffline {gdtf ids singlecommit} {
     global treediff treediffs treepending diffids diffmergeid
 
     set n [gets $gdtf line]
@@ -2803,12 +2809,12 @@ proc gettreediffline {gdtf ids} {
 	set treediffs($ids) $treediff
 	unset treepending
 	if {$ids != $diffids} {
-	    gettreediffs $diffids
+	    gettreediffs $diffids $singlecommit
 	} else {
 	    if {[info exists diffmergeid]} {
-		contmergediff $ids
+		contmergediff $ids $singlecommit
 	    } else {
-		addtocflist $ids
+		addtocflist $ids $singlecommit
 	    }
 	}
 	return
@@ -2817,13 +2823,18 @@ proc gettreediffline {gdtf ids} {
     lappend treediff $file
 }
 
-proc getblobdiffs {ids} {
+proc getblobdiffs {ids singlecommit} {
     global diffopts blobdifffd diffids env curdifftag curtagstart
     global difffilestart nextupdate diffinhdr treediffs
 
     set id [lindex $ids 0]
     set env(GIT_DIFF_OPTS) $diffopts
-    set cmd [list | git-diff-tree --no-commit-id -r -p -C $id]
+    if {$singlecommit == 1} {
+	set cmd [list | git-diff-tree --no-commit-id -r -p -C $id]
+    } else {
+	set p [lindex $ids 1]
+	set cmd [list | git-diff-tree --no-commit-id -r -p -C $p $id]
+    }
     if {[catch {set bdf [open $cmd r]} err]} {
 	puts "error getting diffs: $err"
 	return
@@ -3340,7 +3351,7 @@ proc doseldiff {oldid newid} {
     $ctext conf -state disabled
     $ctext tag delete Comments
     $ctext tag remove found 1.0 end
-    startdiff [list $newid $oldid]
+    startdiff [list $newid $oldid] 0
 }
 
 proc mkpatch {} {
---
0.99.9.GIT

-- 
Yann Dirson    <ydirson@altern.org> |
Debian-related: <dirson@debian.org> |   Support Debian GNU/Linux:
                                    |  Freedom, Power, Stability, Gratis
     http://ydirson.free.fr/        | Check <http://www.debian.org/>

Re: [PATCH] Fix gitk this->selected diffs

[Paul Mackerras]

Yann Dirson writes:

> This patch changes the two git-diff-tree calls to get the necessary
> two ids in this case.  It does so by propagating a "singlecommit" flag
> through all functions involved via an additional argument.

Seems a bit ugly...  Why can't we just make the ids list that we pass
around have either 1 or 2 elements instead?  I'll have a closer look,
but assuming that works I think it'll be a lot cleaner.

Thanks,
Paul.