[RFC 1/4] Add diff-diff, which compares the diffs of two commits

[RFC 1/4] Add diff-diff, which compares the diffs of two commits

[Steffen Prohaska]

The following patch series adds experimental diff-diff support.
It adds a very basic command line version and experimental gitk
support for displaying the differences between the patches
associated with two commits.

"git diff-diff commit1 commit2" displays the differences between
the changes introduced by commit1 and commit2.  This is displayed
as a unified diff between the two patches.

In gitk you can select one commit, right click on a second
commit, and select "Diff Diff selected -> this" to display the
changes between the patches for the two commits.  Highlighting
of the result could certainly be improved.  But it is already
kind of helpful.

This can, for example, be used to compare a first version of a
patch with an improved version of the same patch.  In this case
it is helpful to display the difference between the patches (not
the files changed).  If you commented on the first version, you
may just want to check if the original author improved the patch
according to your comments.

At this point, I'm only seeking comments about the general direction.
The patches should not be applied to git.git.

 - Do you think something like this would be helpful?
 - Are similar approaches already available?
 - How do you use git to support code review; besides discussing
   patches on mailing lists?

    Steffen

---
 Makefile         |    3 ++-
 git-diff-diff.sh |   22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletions(-)
 create mode 100755 git-diff-diff.sh

diff --git a/Makefile b/Makefile
index b9fe40b..2e015ad 100644
--- a/Makefile
+++ b/Makefile
@@ -227,7 +227,8 @@ SCRIPT_SH = \
 	git-lost-found.sh git-quiltimport.sh git-submodule.sh \
 	git-filter-branch.sh \
 	git-stash.sh \
-	git-browse--help.sh
+	git-browse--help.sh \
+	git-diff-diff.sh
 
 SCRIPT_PERL = \
 	git-add--interactive.perl \
diff --git a/git-diff-diff.sh b/git-diff-diff.sh
new file mode 100755
index 0000000..aa402b2
--- /dev/null
+++ b/git-diff-diff.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+OPTIONS_KEEPDASHDASH=
+OPTIONS_SPEC="\
+git-diff-diff <commit> <commit>
+--
+"
+
+. git-sh-setup
+require_work_tree
+
+[ $# = 3 ] || usage
+
+from=$(git rev-parse --verify $2) || exit
+to=$(git rev-parse --verify $3) || exit
+
+git show $from >.git-commit-$from
+git show $to >.git-commit-$to
+
+diff -u .git-commit-$from .git-commit-$to
+
+rm .git-commit-$from .git-commit-$to
-- 
1.5.4.rc0.37.geff3a-dirty

[RFC 2/4] gitk: Add diff-diff support

[Steffen Prohaska]

---
 gitk-git/gitk |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 1da0b0a..ca121e4 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -1019,6 +1019,8 @@ proc makewindow {} {
 	-command {diffvssel 0}
     $rowctxmenu add command -label "Diff selected -> this" \
 	-command {diffvssel 1}
+    $rowctxmenu add command -label "Diff Diff selected -> this" \
+	-command {diffdiffvssel 1}
     $rowctxmenu add command -label "Make patch" -command mkpatch
     $rowctxmenu add command -label "Create tag" -command mktag
     $rowctxmenu add command -label "Write commit to file" -command writecommit
@@ -6010,6 +6012,50 @@ proc doseldiff {oldid newid} {
     startdiff [list $oldid $newid]
 }
 
+proc diffdiffvssel {dirn} {
+    global rowmenuid selectedline displayorder
+
+    if {![info exists selectedline]} return
+    if {$dirn} {
+	set oldid [lindex $displayorder $selectedline]
+	set newid $rowmenuid
+    } else {
+	set oldid $rowmenuid
+	set newid [lindex $displayorder $selectedline]
+    }
+    addtohistory [list doseldiffdiff $oldid $newid]
+    doseldiffdiff $oldid $newid
+}
+
+proc doseldiffdiff {oldid newid} {
+    global ctext
+    global commitinfo
+
+    $ctext conf -state normal
+    clear_ctext
+    init_flist "Top"
+    $ctext insert end "From Commit "
+    $ctext insert end $oldid link0
+    setlink $oldid link0
+    $ctext insert end "\n     "
+    $ctext insert end [lindex $commitinfo($oldid) 0]
+    $ctext insert end "\n\nTo Commit   "
+    $ctext insert end $newid link1
+    setlink $newid link1
+    $ctext insert end "\n     "
+    $ctext insert end [lindex $commitinfo($newid) 0]
+    $ctext insert end "\n"
+    $ctext insert end "\n"
+    $ctext conf -state disabled
+    $ctext tag remove found 1.0 end
+
+    set diff [exec git diff-diff $oldid $newid]
+    $ctext conf -state normal
+    $ctext insert end "\n"
+    $ctext insert end $diff
+    $ctext conf -state disabled
+}
+
 proc mkpatch {} {
     global rowmenuid currentid commitinfo patchtop patchnum
 
-- 
1.5.4.rc0.37.geff3a-dirty

[RFC 3/4] gitk: Refactor getblobdiffline to split off adddiffline

[Steffen Prohaska]

getblobdiffline used to read from a file descriptor, parsed
the line read, and displayed the line nicely formatted.

Parsing and displaying the line is split off to a separate
function adddiffline.

---
 gitk-git/gitk |   30 +++++++++++++++++++-----------
 1 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index ca121e4..3c4ec03 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -5324,18 +5324,11 @@ proc makediffhdr {fname ids} {
     $ctext insert $curdiffstart "$pad $fname $pad" filesep
 }
 
-proc getblobdiffline {bdf ids} {
+proc adddiffline {line ids} {
     global diffids blobdifffd ctext curdiffstart
     global diffnexthead diffnextnote difffilestart
     global diffinhdr treediffs
 
-    set nr 0
-    $ctext conf -state normal
-    while {[incr nr] <= 1000 && [gets $bdf line] >= 0} {
-	if {$ids != $diffids || $bdf != $blobdifffd($ids)} {
-	    close $bdf
-	    return 0
-	}
 	if {![string compare -length 11 "diff --git " $line]} {
 	    # trim off "diff --git "
 	    set line [string range $line 11 end]
@@ -5357,7 +5350,7 @@ proc getblobdiffline {bdf ids} {
 	    if {!(($l & 1) && [string index $line $i] eq " " &&
 		  [string range $line 2 [expr {$i - 1}]] eq \
 		      [string range $line [expr {$i + 3}] end])} {
-		continue
+		return
 	    }
 	    # unescape if quoted and chop off the a/ from the front
 	    if {[string index $line 0] eq "\""} {
@@ -5391,10 +5384,10 @@ proc getblobdiffline {bdf ids} {
 		makediffhdr $fname $ids
 	    } elseif {[string compare -length 3 $line "---"] == 0} {
 		# do nothing
-		continue
+		return
 	    } elseif {[string compare -length 3 $line "+++"] == 0} {
 		set diffinhdr 0
-		continue
+		return
 	    }
 	    $ctext insert end "$line\n" filesep
 
@@ -5411,6 +5404,21 @@ proc getblobdiffline {bdf ids} {
 		$ctext insert end "$line\n" hunksep
 	    }
 	}
+}
+
+proc getblobdiffline {bdf ids} {
+    global diffids blobdifffd ctext curdiffstart
+    global diffnexthead diffnextnote difffilestart
+    global diffinhdr treediffs
+
+    set nr 0
+    $ctext conf -state normal
+    while {[incr nr] <= 1000 && [gets $bdf line] >= 0} {
+	if {$ids != $diffids || $bdf != $blobdifffd($ids)} {
+	    close $bdf
+	    return 0
+	}
+	adddiffline $line $ids
     }
     $ctext conf -state disabled
     if {[eof $bdf]} {
-- 
1.5.4.rc0.37.geff3a-dirty

[RFC 4/4] gitk: Use adddiffline to nicely format diff-diff

[Steffen Prohaska]

---
 gitk-git/gitk |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 3c4ec03..31b2b5f 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -6059,8 +6059,9 @@ proc doseldiffdiff {oldid newid} {
 
     set diff [exec git diff-diff $oldid $newid]
     $ctext conf -state normal
-    $ctext insert end "\n"
-    $ctext insert end $diff
+    foreach line [split $diff "\n"] {
+	adddiffline $line [list $oldid $newid]
+    }
     $ctext conf -state disabled
 }
 
-- 
1.5.4.rc0.37.geff3a-dirty

Re: [RFC 1/4] Add diff-diff, which compares the diffs of two commits

[Johannes Schindelin]

Hi,

On Sat, 15 Dec 2007, Steffen Prohaska wrote:

> The following patch series adds experimental diff-diff support. It adds 
> a very basic command line version and experimental gitk support for 
> displaying the differences between the patches associated with two 
> commits.

I briefly considered adding something like interdiff a year ago, but 
decided that installing patchutils and having an alias was enough for my 
needs.

IOW I do not need this to be a git command.

Ciao,
Dscho

Re: [RFC 1/4] Add diff-diff, which compares the diffs of two commits

[Björn Steinbrink]

On 2007.12.15 17:51:42 +0100, Steffen Prohaska wrote:
> The following patch series adds experimental diff-diff support.
[...]
> At this point, I'm only seeking comments about the general direction.
> The patches should not be applied to git.git.
> 
>  - Do you think something like this would be helpful?

I think I might use it to e.g. compare stuff when upstream modified the
patch I submitted.

>  - Are similar approaches already available?

interdiff from patchutils. Probably interdiff would also be a better
name than diff-diff, just to be consistent with what is already there.
And using interdiff instead of plain diff in your script might also
yield better results (I didn't use interdiff that often yet).

Björn

Re: [RFC 1/4] Add diff-diff, which compares the diffs of two commits

[Steffen Prohaska]

On Dec 15, 2007, at 6:04 PM, Björn Steinbrink wrote:

> On 2007.12.15 17:51:42 +0100, Steffen Prohaska wrote:
>> The following patch series adds experimental diff-diff support.
> [...]
>> At this point, I'm only seeking comments about the general direction.
>> The patches should not be applied to git.git.
>>
>>  - Are similar approaches already available?
>
> interdiff from patchutils. Probably interdiff would also be a better
> name than diff-diff, just to be consistent with what is already there.
> And using interdiff instead of plain diff in your script might also
> yield better results (I didn't use interdiff that often yet).


At a first glance, interdiff yields more intuitive results if it
works, but it may fail if the two patches differ to much.  I need
to play more to see how useful it is in real world examples.  I
did not know interdiff before.

At this point I'm not convinced that interdiff is exactly what I
have in mind.  For example it doesn't show differences between
commit messages of two git patches.

	Steffen

Re: [RFC 1/4] Add diff-diff, which compares the diffs of two commits

[Junio C Hamano]

Steffen Prohaska <prohaska@zib.de> writes:

> At this point, I'm only seeking comments about the general direction.
> The patches should not be applied to git.git.
>
>  - Do you think something like this would be helpful?
>  - Are similar approaches already available?
>  - How do you use git to support code review; besides discussing
>    patches on mailing lists?

I think there are other good building blocks you can use outside git to
help reviewing evolution of patches (interdiff comes to mind).

When I receive a replacement series to a topic, I use RP script
(available in my 'todo' branch and checked out in Meta/ directory in my
work area, together with other scripts from 'todo'), which:

 * finds where the older series were applied;
 * detaches HEAD at that commit, and apply the new series;
 * runs diff between the old and new;
 * updates the tip of that topic with the new series.

After it finishes, "diff topic@{1} topic" becomes the incremental diff
between the old and the new series, and if I do not like the end result,
I can reset --hard back to topic@{1}.