git-gui: display summary when showing diff of a submodule
As it is hard to say what changed in a submodule by looking at the hashes, let's show the colored submodule summary instead.
---
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(-)
git-gui/lib/diff.tcl +25 -2
@@ -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 {
Signature · 1 line
1.6.3.3.386.g1d0c9.dirty
Jens Lehmann venit, vidit, dixit 06.07.2009 23:31:
Quoted 3 lines
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
Quoted 2 lines · Michael J Gruber
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