[RFC/PATCH] git-gui: display summary when showing diff of a submodule

[RFC/PATCH] git-gui: display summary when showing diff of a submodule

[Jens Lehmann]

As it is hard to say what changed in a submodule by looking at the hashes,
let's show the colored submodule summary instead.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

While starting to work productively with submodules a few weeks ago i
noticed that it is very inconvenient not to know what really changed
in a submodule when looking at it through git gui in the supermodule.
So i wrote this patch as a starting point for a discussion of how a
solution might look like.

Basically i would like to see what happened in the submodule between the
two hashes, so it seemed like git submodule summary will provide exactly
that information and should be preferred over git diff in that case.
(I also thought about teaching git diff to provide the information, but
came to the conclusion that it would be better to do that in git gui)

As i don't have that much experience with submodules yet i'm not sure to
have taken all corner cases into account ... so a few more eyeballs really
are appreciated!

 git-gui/lib/diff.tcl |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
index 925b3f5..67bc49f 100644
--- a/git-gui/lib/diff.tcl
+++ b/git-gui/lib/diff.tcl
@@ -255,7 +255,7 @@ proc show_other_diff {path w m cont_info} {

 proc start_show_diff {cont_info {add_opts {}}} {
 	global file_states file_lists
-	global is_3way_diff diff_active repo_config
+	global is_3way_diff is_submodule_diff diff_active repo_config
 	global ui_diff ui_index ui_workdir
 	global current_diff_path current_diff_side current_diff_header

@@ -265,6 +265,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 	set s $file_states($path)
 	set m [lindex $s 0]
 	set is_3way_diff 0
+	set is_submodule_diff 0
 	set diff_active 1
 	set current_diff_header {}

@@ -295,6 +296,10 @@ proc start_show_diff {cont_info {add_opts {}}} {
 		lappend cmd $path
 	}

+	if {[string match {160000 *} [lindex $s 3]]} {
+		set cmd {submodule summary -- $current_diff_path}
+	}
+
 	if {[catch {set fd [eval git_read --nice $cmd]} err]} {
 		set diff_active 0
 		unlock_index
@@ -312,7 +317,7 @@ proc start_show_diff {cont_info {add_opts {}}} {
 }

 proc read_diff {fd cont_info} {
-	global ui_diff diff_active
+	global ui_diff diff_active is_submodule_diff
 	global is_3way_diff is_conflict_diff current_diff_header
 	global current_diff_queue
 	global diff_empty_count
@@ -337,6 +342,9 @@ proc read_diff {fd cont_info} {
 		}
 		set ::current_diff_inheader 0

+		if {[regexp {^\* } $line]} {
+			set is_submodule_diff 1
+		}
 		# -- Automatically detect if this is a 3 way diff.
 		#
 		if {[string match {@@@ *} $line]} {set is_3way_diff 1}
@@ -374,6 +382,21 @@ proc read_diff {fd cont_info} {
 				set tags {}
 			}
 			}
+		} elseif {$is_submodule_diff} {
+			if {$line == ""} continue
+			if {[regexp {^\* } $line]} {
+				set tags {}
+			} else {
+				set op [string range $line 0 2]
+				switch -- $op {
+				{  <} {set tags d_+}
+				{  >} {set tags d_-}
+				default {
+					puts "error: Unhandled submodule diff marker: {$op}"
+					set tags {}
+				}
+				}
+			}
 		} else {
 			set op [string index $line 0]
 			switch -- $op {
-- 
1.6.3.3.386.g1d0c9.dirty

Re: [RFC/PATCH] git-gui: display summary when showing diff of a submodule

[Michael J Gruber]

Jens Lehmann venit, vidit, dixit 06.07.2009 23:31:
> As it is hard to say what changed in a submodule by looking at the hashes,
> let's show the colored submodule summary instead.
> 

I can't comment on the patch but I find the goal very valid. I just
updated the submodule tutorial wiki to use "git submodule summary" in
addition to plain diff.

Me wonders whether the summary should be the default diff output, or at
least a builtin merge driver which can be selected using attributes or such.

Michael

Re: [RFC/PATCH] git-gui: display summary when showing diff of a submodule

[Jens Lehmann]

Michael J Gruber schrieb:
> Me wonders whether the summary should be the default diff output, or at
> least a builtin merge driver which can be selected using attributes or such.

Yes, i was thinking about that too. But i have no idea if it would be
a good or bad idea to extend git diff that way.

And for git gui it doesn't really matter. The only difference to my patch
would be the line where the diff command is replaced with submodule summary
(assuming the output would stay compatible). The rest of the patch is about
handling that output, no matter how it is generated.


Jens