* [PATCH 1/6] gitweb: Add parsing of raw combined diff format to parse_difftree_raw_line
2007-05-06 23:10 [PATCH 0/6] gitweb: Add combined diff support Jakub Narebski
@ 2007-05-06 23:10 ` Jakub Narebski
2007-05-06 23:10 ` [PATCH 2/6] gitweb: Add combined diff support to git_difftree_body Jakub Narebski
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
Add parsing line of raw combined diff ("git diff-tree -c/-cc" output)
as described in section "diff format for merges" in diff-format.txt
to parse_difftree_raw_line subroutine.
Returned hash (or hashref) has for combined diff 'nparents' key which
holds number of parents in a merge. At keys 'from_mode' and 'from_id'
there are arrayrefs holding modes and ids, respectively. There is no
'similarity' value, and there is only 'to_file' value and no
'from_file' value.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ba5cc43..dfba399 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1495,6 +1495,17 @@ sub parse_difftree_raw_line {
$res{'file'} = unquote($7);
}
}
+ # '::100755 100755 100755 60e79ca1b01bc8b057abe17ddab484699a7f5fdb 94067cc5f73388f33722d52ae02f44692bc07490 94067cc5f73388f33722d52ae02f44692bc07490 MR git-gui/git-gui.sh'
+ # combined diff (for merge commit)
+ elsif ($line =~ s/^(::+)((?:[0-7]{6} )+)((?:[0-9a-fA-F]{40} )+)([a-zA-Z]+)\t(.*)$//) {
+ $res{'nparents'} = length($1);
+ $res{'from_mode'} = [ split(' ', $2) ];
+ $res{'to_mode'} = pop @{$res{'from_mode'}};
+ $res{'from_id'} = [ split(' ', $3) ];
+ $res{'to_id'} = pop @{$res{'from_id'}};
+ $res{'status'} = [ split('', $4) ];
+ $res{'to_file'} = unquote($5);
+ }
# 'c512b523472485aef4fff9e57b229d9d243c967f'
elsif ($line =~ m/^([0-9a-fA-F]{40})$/) {
$res{'commit'} = $1;
--
1.5.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/6] gitweb: Add combined diff support to git_difftree_body
2007-05-06 23:10 [PATCH 0/6] gitweb: Add combined diff support Jakub Narebski
2007-05-06 23:10 ` [PATCH 1/6] gitweb: Add parsing of raw combined diff format to parse_difftree_raw_line Jakub Narebski
@ 2007-05-06 23:10 ` Jakub Narebski
2007-05-06 23:10 ` [PATCH 3/6] gitweb: Add combined diff support to git_patchset_body Jakub Narebski
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
You have to pass all parents as final parameters of git_difftree_body
subroutine; the number of parents of a diff must be equal to the
number derived from parsing git-diff-tree output, raw combined diff
for git_difftree_body to display combined diff correctly (but it is
not checked).
Currently the possibility of displaying diffree of combined diff is
not used in gitweb code; git_difftree_body is always caled for
ordinary diff, and with only one parent.
Description of output for combined diff:
----------------------------------------
The difftree table for combined diff starts with a cell with pathname
of changed blob (changed file), which if possible is hidden link
(class="list") to the 'blob' view of final version (if it exists),
like for difftree for ordinary diff. If file was deleted in the final
commit then filename is not hyperlinked.
There is no cell with single file status (new, deleted, mode change,
rename), as for combined diff as there is no single status: different
parents might have different status.
If git_difftree_body was called from git_commitdiff (for 'commitdiff'
action) there is inner link to anchor to appropriate fragment (patch)
in patchset body; the "patch" link does not replace "diff" link like
for ordinary diff.
Each of "diff" links is in separate cell, contrary to output for
ordinary diff in which all links are (at least for now) in a single
cell.
For each parent, if file was not present we leave cell empty. If file
was deleted in the result, we provide link to 'blob' view. Otherwise
we provide link to 'commitdiff' view, even if patch (diff) consist
only of extended diff header, and contents is not changed (pure
rename, pure mode change). The only difference is that link to
"blobdiff" view with no contents change is with 'nochange' class.
At last, there is provided link to current version of file as "blob"
link, if the file was not deleted in the result, and lik to history of
a file, if there exists one. (The link to file history might be
confused, at least for now, by renames.)
Note that git-diff-tree raw output dor combined diff does not provide
filename before change for renames and copies; we use
git_get_path_by_hash to get "src" filename for renames (this means
additional call to git-ls-tree for a _whole_ tree).
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Currently the work to find filename _before_ rename for renames (and
copies) in combined diff is done during difftree generation (link
generation). It is possible to change it to have this calculation done
on link target, in 'blobdiff' and 'blob' views which lack 'fp' or 'f'
parameter, and have 'hpb' and 'hp' or 'hb' and 'h' parameters needed
to find path by hash. Nevertheless renames in combined diff format are
rare I think, and non-empty combined diff for merge commit
(non-trivial merge result) is also usually rare.
[The above paragraph should perhaps be added to commit message, but is
quite long even without it.]
This is preliminary version: difftree output for combined diff is a
bit crude, and does not display all the information (mode changes,
renames, etc.). Nevertheless I think it is a good start.
CSS styling is a bit crude.
Patches (or corrections) and ideas how should HTML output of difftree
(whatchanged-like output) for combined diff look like are very
welcome.
gitweb/gitweb.css | 17 +++++++
gitweb/gitweb.perl | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 136 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 2b023bd..e795b70 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -181,6 +181,23 @@ table.diff_tree {
font-family: monospace;
}
+table.combined.diff_tree td {
+ padding-right: 24px;
+}
+
+table.combined.diff_tree td.link {
+ padding: 0px 2px;
+}
+
+table.combined.diff_tree td.nochange a {
+ color: #6666ff;
+}
+
+table.combined.diff_tree td.nochange a:hover,
+table.combined.diff_tree td.nochange a:visited {
+ color: #d06666;
+}
+
table.blame {
border-collapse: collapse;
}
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index dfba399..c6a2fef 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1015,6 +1015,30 @@ sub git_get_hash_by_path {
return $3;
}
+# get path of entry with given hash at given tree-ish (ref)
+# used to get 'from' filename for combined diff (merge commit) for renames
+sub git_get_path_by_hash {
+ my $base = shift || return;
+ my $hash = shift || return;
+
+ local $/ = "\0";
+
+ open my $fd, "-|", git_cmd(), "ls-tree", '-r', '-t', '-z', $base
+ or return undef;
+ while (my $line = <$fd>) {
+ chomp $line;
+
+ #'040000 tree 595596a6a9117ddba9fe379b6b012b558bac8423 gitweb'
+ #'100644 blob e02e90f0429be0d2a69b76571101f20b8f75530f gitweb/README'
+ if ($line =~ m/(?:[0-9]+) (?:.+) $hash\t(.+)$/) {
+ close $fd;
+ return $1;
+ }
+ }
+ close $fd;
+ return undef;
+}
+
## ......................................................................
## git utility functions, directly accessing git repository
@@ -2210,7 +2234,8 @@ sub git_print_tree_entry {
## functions printing large fragments of HTML
sub git_difftree_body {
- my ($difftree, $hash, $parent) = @_;
+ my ($difftree, $hash, @parents) = @_;
+ my ($parent) = $parents[0];
my ($have_blame) = gitweb_check_feature('blame');
print "<div class=\"list_head\">\n";
if ($#{$difftree} > 10) {
@@ -2218,7 +2243,9 @@ sub git_difftree_body {
}
print "</div>\n";
- print "<table class=\"diff_tree\">\n";
+ print "<table class=\"" .
+ (@parents > 1 ? "combined " : "") .
+ "diff_tree\">\n";
my $alternate = 1;
my $patchno = 0;
foreach my $line (@{$difftree}) {
@@ -2231,6 +2258,96 @@ sub git_difftree_body {
}
$alternate ^= 1;
+ if (exists $diff{'nparents'}) { # combined diff
+
+ if ($diff{'to_id'} ne ('0' x 40)) {
+ # file exists in the result (child) commit
+ print "<td>" .
+ $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
+ file_name=>$diff{'to_file'},
+ hash_base=>$hash),
+ -class => "list"}, esc_path($diff{'to_file'})) .
+ "</td>\n";
+ } else {
+ print "<td>" .
+ esc_path($diff{'to_file'}) .
+ "</td>\n";
+ }
+
+ if ($action eq 'commitdiff') {
+ # link to patch
+ $patchno++;
+ print "<td class=\"link\">" .
+ $cgi->a({-href => "#patch$patchno"}, "patch") .
+ " | " .
+ "</td>\n";
+ }
+
+ my $has_history = 0;
+ my $not_deleted = 0;
+ for (my $i = 0; $i < $diff{'nparents'}; $i++) {
+ my $hash_parent = $parents[$i];
+ my $from_hash = $diff{'from_id'}[$i];
+ my $from_path = undef;
+ my $status = $diff{'status'}[$i];
+
+ $has_history ||= ($status ne 'A');
+ $not_deleted ||= ($status ne 'D');
+
+ if ($status eq 'R' || $status eq 'C') {
+ $from_path = git_get_path_by_hash($hash_parent, $from_hash);
+ }
+
+ if ($status eq 'A') {
+ print "<td class=\"link\" align=\"right\"> | </td>\n";
+ } elsif ($status eq 'D') {
+ print "<td class=\"link\">" .
+ $cgi->a({-href => href(action=>"blob",
+ hash_base=>$hash,
+ hash=>$from_hash,
+ file_name=>$from_path)},
+ "blob" . ($i+1)) .
+ " | </td>\n";
+ } else {
+ if ($diff{'to_id'} eq $from_hash) {
+ print "<td class=\"link nochange\">";
+ } else {
+ print "<td class=\"link\">";
+ }
+ print $cgi->a({-href => href(action=>"blobdiff",
+ hash=>$diff{'to_id'},
+ hash_parent=>$from_hash,
+ hash_base=>$hash,
+ hash_parent_base=>$hash_parent,
+ file_name=>$diff{'to_file'},
+ file_parent=>$from_path)},
+ "diff" . ($i+1)) .
+ " | </td>\n";
+ }
+ }
+
+ print "<td class=\"link\">";
+ if ($not_deleted) {
+ print $cgi->a({-href => href(action=>"blob",
+ hash=>$diff{'to_id'},
+ file_name=>$diff{'to_file'},
+ hash_base=>$hash)},
+ "blob");
+ print " | " if ($has_history);
+ }
+ if ($has_history) {
+ print $cgi->a({-href => href(action=>"history",
+ file_name=>$diff{'to_file'},
+ hash_base=>$hash)},
+ "history");
+ }
+ print "</td>\n";
+
+ print "</tr>\n";
+ next; # instead of 'else' clause, to avoid extra indent
+ }
+ # else ordinary diff
+
my ($to_mode_oct, $to_mode_str, $to_file_type);
my ($from_mode_oct, $from_mode_str, $from_file_type);
if ($diff{'to_mode'} ne ('0' x 6)) {
--
1.5.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/6] gitweb: Add combined diff support to git_patchset_body
2007-05-06 23:10 [PATCH 0/6] gitweb: Add combined diff support Jakub Narebski
2007-05-06 23:10 ` [PATCH 1/6] gitweb: Add parsing of raw combined diff format to parse_difftree_raw_line Jakub Narebski
2007-05-06 23:10 ` [PATCH 2/6] gitweb: Add combined diff support to git_difftree_body Jakub Narebski
@ 2007-05-06 23:10 ` Jakub Narebski
2007-05-06 23:10 ` [PATCH 4/6] gitweb: Make it possible to use pre-parsed info in git_difftree_body Jakub Narebski
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
Calling convention for combined diff similar to the one for
git_difftree_body subroutine: difftree info (first parameter) must be
result of calling git-diff-tree with -c/--cc option, and all parents
of a commit must be passed as last parameters. See also description in
"gitweb: Add combined diff support to git_difftree_body"
This ability is not used yet.
Generating "src" file name for renames in combined diff was separated
into fill_from_file_info subroutine; git_difftree_body was modified to
use it. Currently git_difftree_body and git_patchset_body fills this
info separately.
The from-file line in two-line from-file/to-file header is not
hyperlinked: there can be more than one "from"/"src" file. This
differs from HTML output of ordinary (not combined) diff.
format_diff_line subroutine needs extra $from/$to parameters to format
combined diff patch line correctly.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Currently the following extended git diff header line in combined diff
new file mode <mode>
gets its <mode> (filetype) explained like for appropriate extended
header lines for non-combined (ordinary) diff
old mode <mode>
new mode <mode>
deleted file mode <mode>
new file mode <mode>
index <hash>..<hash> <mode>
while similar line
deleted file mode <mode>,<mode>
does not get <mode> explained. Perhaps we should remove this addition,
or make explanation using tooltip (and perhaps <abbr> HTML element).
gitweb/gitweb.perl | 221 ++++++++++++++++++++++++++++++++++++++++++----------
1 files changed, 181 insertions(+), 40 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c6a2fef..53ae0b8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -897,19 +897,34 @@ sub format_subject_html {
sub format_diff_line {
my $line = shift;
my ($from, $to) = @_;
- my $char = substr($line, 0, 1);
my $diff_class = "";
chomp $line;
- if ($char eq '+') {
- $diff_class = " add";
- } elsif ($char eq "-") {
- $diff_class = " rem";
- } elsif ($char eq "@") {
- $diff_class = " chunk_header";
- } elsif ($char eq "\\") {
- $diff_class = " incomplete";
+ if ($from && $to && ref($from->{'href'}) eq "ARRAY") {
+ # combined diff
+ my $prefix = substr($line, 0, scalar @{$from->{'href'}});
+ if ($line =~ m/^\@{3}/) {
+ $diff_class = " chunk_header";
+ } elsif ($line =~ m/^\\/) {
+ $diff_class = " incomplete";
+ } elsif ($prefix =~ tr/+/+/) {
+ $diff_class = " add";
+ } elsif ($prefix =~ tr/-/-/) {
+ $diff_class = " rem";
+ }
+ } else {
+ # assume ordinary diff
+ my $char = substr($line, 0, 1);
+ if ($char eq '+') {
+ $diff_class = " add";
+ } elsif ($char eq '-') {
+ $diff_class = " rem";
+ } elsif ($char eq '@') {
+ $diff_class = " chunk_header";
+ } elsif ($char eq "\\") {
+ $diff_class = " incomplete";
+ }
}
$line = untabify($line);
if ($from && $to && $line =~ m/^\@{2} /) {
@@ -930,6 +945,39 @@ sub format_diff_line {
$line = "<span class=\"chunk_info\">@@ $from_text $to_text @@</span>" .
"<span class=\"section\">" . esc_html($section, -nbsp=>1) . "</span>";
return "<div class=\"diff$diff_class\">$line</div>\n";
+ } elsif ($from && $to && $line =~ m/^\@{3}/) {
+ my ($prefix, $ranges, $section) = $line =~ m/^(\@+) (.*?) \@+(.*)$/;
+ my (@from_text, @from_start, @from_nlines, $to_text, $to_start, $to_nlines);
+
+ @from_text = split(' ', $ranges);
+ for (my $i = 0; $i < @from_text; ++$i) {
+ ($from_start[$i], $from_nlines[$i]) =
+ (split(',', substr($from_text[$i], 1)), 0);
+ }
+
+ $to_text = pop @from_text;
+ $to_start = pop @from_start;
+ $to_nlines = pop @from_nlines;
+
+ $line = "<span class=\"chunk_info\">$prefix ";
+ for (my $i = 0; $i < @from_text; ++$i) {
+ if ($from->{'href'}[$i]) {
+ $line .= $cgi->a({-href=>"$from->{'href'}[$i]#l$from_start[$i]",
+ -class=>"list"}, $from_text[$i]);
+ } else {
+ $line .= $from_text[$i];
+ }
+ $line .= " ";
+ }
+ if ($to->{'href'}) {
+ $line .= $cgi->a({-href=>"$to->{'href'}#l$to_start",
+ -class=>"list"}, $to_text);
+ } else {
+ $line .= $to_text;
+ }
+ $line .= " $prefix</span>" .
+ "<span class=\"section\">" . esc_html($section, -nbsp=>1) . "</span>";
+ return "<div class=\"diff$diff_class\">$line</div>\n";
}
return "<div class=\"diff$diff_class\">" . esc_html($line, -nbsp=>1) . "</div>\n";
}
@@ -2233,6 +2281,39 @@ sub git_print_tree_entry {
## ......................................................................
## functions printing large fragments of HTML
+sub fill_from_file_info {
+ my ($diff, @parents) = @_;
+
+ $diff->{'from_file'} = [ ];
+ $diff->{'from_file'}[$diff->{'nparents'} - 1] = undef;
+ for (my $i = 0; $i < $diff->{'nparents'}; $i++) {
+ if ($diff->{'status'}[$i] eq 'R' ||
+ $diff->{'status'}[$i] eq 'C') {
+ $diff->{'from_file'}[$i] =
+ git_get_path_by_hash($parents[$i], $diff->{'from_id'}[$i]);
+ }
+ }
+
+ return $diff;
+}
+
+# parameters can be strings, or references to arrays of strings
+sub from_ids_eq {
+ my ($a, $b) = @_;
+
+ if (ref($a) eq "ARRAY" && ref($b) eq "ARRAY" && @$a == @$b) {
+ for (my $i = 0; $i < @$a; ++$i) {
+ return 0 unless ($a->[$i] eq $b->[$i]);
+ }
+ return 1;
+ } elsif (!ref($a) && !ref($b)) {
+ return $a eq $b;
+ } else {
+ return 0;
+ }
+}
+
+
sub git_difftree_body {
my ($difftree, $hash, @parents) = @_;
my ($parent) = $parents[0];
@@ -2260,6 +2341,8 @@ sub git_difftree_body {
if (exists $diff{'nparents'}) { # combined diff
+ fill_from_file_info(\%diff, @parents);
+
if ($diff{'to_id'} ne ('0' x 40)) {
# file exists in the result (child) commit
print "<td>" .
@@ -2288,16 +2371,12 @@ sub git_difftree_body {
for (my $i = 0; $i < $diff{'nparents'}; $i++) {
my $hash_parent = $parents[$i];
my $from_hash = $diff{'from_id'}[$i];
- my $from_path = undef;
+ my $from_path = $diff{'from_file'}[$i];
my $status = $diff{'status'}[$i];
$has_history ||= ($status ne 'A');
$not_deleted ||= ($status ne 'D');
- if ($status eq 'R' || $status eq 'C') {
- $from_path = git_get_path_by_hash($hash_parent, $from_hash);
- }
-
if ($status eq 'A') {
print "<td class=\"link\" align=\"right\"> | </td>\n";
} elsif ($status eq 'D') {
@@ -2517,7 +2596,8 @@ sub git_difftree_body {
}
sub git_patchset_body {
- my ($fd, $difftree, $hash, $hash_parent) = @_;
+ my ($fd, $difftree, $hash, @hash_parents) = @_;
+ my ($hash_parent) = $hash_parents[0];
my $patch_idx = 0;
my $patch_number = 0;
@@ -2555,6 +2635,9 @@ sub git_patchset_body {
if ($patch_line =~ m/^index ([0-9a-fA-F]{40})..([0-9a-fA-F]{40})/) {
$from_id = $1;
$to_id = $2;
+ } elsif ($patch_line =~ m/^index ((?:[0-9a-fA-F]{40},)+[0-9a-fA-F]{40})..([0-9a-fA-F]{40})/) {
+ $from_id = [ split(',', $1) ];
+ $to_id = $2;
}
push @diff_header, $patch_line;
@@ -2564,8 +2647,8 @@ sub git_patchset_body {
# check if current patch belong to current raw line
# and parse raw git-diff line if needed
if (defined $diffinfo &&
- $diffinfo->{'from_id'} eq $from_id &&
- $diffinfo->{'to_id'} eq $to_id) {
+ from_ids_eq($diffinfo->{'from_id'}, $from_id) &&
+ $diffinfo->{'to_id'} eq $to_id) {
# this is split patch
print "<div class=\"patch cont\">\n";
} else {
@@ -2579,15 +2662,34 @@ sub git_patchset_body {
} else {
$diffinfo = parse_difftree_raw_line($difftree->[$patch_idx]);
}
- $from{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
- $to{'file'} = $diffinfo->{'to_file'} || $diffinfo->{'file'};
- if ($diffinfo->{'status'} ne "A") { # not new (added) file
- $from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
- hash=>$diffinfo->{'from_id'},
- file_name=>$from{'file'});
+ if ($diffinfo->{'nparents'}) {
+ # combined diff
+ $from{'file'} = [];
+ $from{'href'} = [];
+ fill_from_file_info($diffinfo, @hash_parents)
+ unless exists $diffinfo->{'from_file'};
+ for (my $i = 0; $i < $diffinfo->{'nparents'}; $i++) {
+ $from{'file'}[$i] = $diffinfo->{'from_file'}[$i] || $diffinfo->{'to_file'};
+ if ($diffinfo->{'status'}[$i] ne "A") { # not new (added) file
+ $from{'href'}[$i] = href(action=>"blob",
+ hash_base=>$hash_parents[$i],
+ hash=>$diffinfo->{'from_id'}[$i],
+ file_name=>$from{'file'}[$i]);
+ } else {
+ $from{'href'}[$i] = undef;
+ }
+ }
} else {
- delete $from{'href'};
+ $from{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
+ if ($diffinfo->{'status'} ne "A") { # not new (added) file
+ $from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
+ hash=>$diffinfo->{'from_id'},
+ file_name=>$from{'file'});
+ } else {
+ delete $from{'href'};
+ }
}
+ $to{'file'} = $diffinfo->{'to_file'} || $diffinfo->{'file'};
if ($diffinfo->{'status'} ne "D") { # not deleted file
$to{'href'} = href(action=>"blob", hash_base=>$hash,
hash=>$diffinfo->{'to_id'},
@@ -2602,19 +2704,34 @@ sub git_patchset_body {
# print "git diff" header
$patch_line = shift @diff_header;
- $patch_line =~ s!^(diff (.*?) )"?a/.*$!$1!;
- if ($from{'href'}) {
- $patch_line .= $cgi->a({-href => $from{'href'}, -class => "path"},
- 'a/' . esc_path($from{'file'}));
- } else { # file was added
- $patch_line .= 'a/' . esc_path($from{'file'});
- }
- $patch_line .= ' ';
- if ($to{'href'}) {
- $patch_line .= $cgi->a({-href => $to{'href'}, -class => "path"},
- 'b/' . esc_path($to{'file'}));
- } else { # file was deleted
- $patch_line .= 'b/' . esc_path($to{'file'});
+ if ($diffinfo->{'nparents'}) {
+
+ # combined diff
+ $patch_line =~ s!^(diff (.*?) )"?.*$!$1!;
+ if ($to{'href'}) {
+ $patch_line .= $cgi->a({-href => $to{'href'}, -class => "path"},
+ esc_path($to{'file'}));
+ } else { # file was deleted
+ $patch_line .= esc_path($to{'file'});
+ }
+
+ } else {
+
+ $patch_line =~ s!^(diff (.*?) )"?a/.*$!$1!;
+ if ($from{'href'}) {
+ $patch_line .= $cgi->a({-href => $from{'href'}, -class => "path"},
+ 'a/' . esc_path($from{'file'}));
+ } else { # file was added
+ $patch_line .= 'a/' . esc_path($from{'file'});
+ }
+ $patch_line .= ' ';
+ if ($to{'href'}) {
+ $patch_line .= $cgi->a({-href => $to{'href'}, -class => "path"},
+ 'b/' . esc_path($to{'file'}));
+ } else { # file was deleted
+ $patch_line .= 'b/' . esc_path($to{'file'});
+ }
+
}
print "<div class=\"diff header\">$patch_line</div>\n";
@@ -2631,14 +2748,37 @@ sub git_patchset_body {
$patch_line .= $cgi->a({-href=>$to{'href'}, -class=>"path"},
esc_path($to{'file'}));
}
- # match <mode>
+ # match single <mode>
if ($patch_line =~ m/\s(\d{6})$/) {
$patch_line .= '<span class="info"> (' .
file_type_long($1) .
')</span>';
}
# match <hash>
- if ($patch_line =~ m/^index/) {
+ if ($patch_line =~ m/^index [0-9a-fA-F]{40},[0-9a-fA-F]{40}/) {
+ # can match only for combined diff
+ $patch_line = 'index ';
+ for (my $i = 0; $i < $diffinfo->{'nparents'}; $i++) {
+ if ($from{'href'}[$i]) {
+ $patch_line .= $cgi->a({-href=>$from{'href'}[$i],
+ -class=>"hash"},
+ substr($diffinfo->{'from_id'}[$i],0,7));
+ } else {
+ $patch_line .= '0' x 7;
+ }
+ # separator
+ $patch_line .= ',' if ($i < $diffinfo->{'nparents'} - 1);
+ }
+ $patch_line .= '..';
+ if ($to{'href'}) {
+ $patch_line .= $cgi->a({-href=>$to{'href'}, -class=>"hash"},
+ substr($diffinfo->{'to_id'},0,7));
+ } else {
+ $patch_line .= '0' x 7;
+ }
+
+ } elsif ($patch_line =~ m/^index [0-9a-fA-F]{40}..[0-9a-fA-F]{40}/) {
+ # can match only for ordinary diff
my ($from_link, $to_link);
if ($from{'href'}) {
$from_link = $cgi->a({-href=>$from{'href'}, -class=>"hash"},
@@ -2674,7 +2814,8 @@ sub git_patchset_body {
}
next PATCH if ($patch_line =~ m/^diff /);
#assert($patch_line =~ m/^---/) if DEBUG;
- if ($from{'href'} && $patch_line =~ m!^--- "?a/!) {
+ if (!$diffinfo->{'nparents'} && # not from-file line for combined diff
+ $from{'href'} && $patch_line =~ m!^--- "?a/!) {
$patch_line = '--- a/' .
$cgi->a({-href=>$from{'href'}, -class=>"path"},
esc_path($from{'file'}));
--
1.5.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/6] gitweb: Make it possible to use pre-parsed info in git_difftree_body
2007-05-06 23:10 [PATCH 0/6] gitweb: Add combined diff support Jakub Narebski
` (2 preceding siblings ...)
2007-05-06 23:10 ` [PATCH 3/6] gitweb: Add combined diff support to git_patchset_body Jakub Narebski
@ 2007-05-06 23:10 ` Jakub Narebski
2007-05-06 23:10 ` [PATCH 5/6] gitweb: Show combined diff for merge commits in 'commitdiff' view Jakub Narebski
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
Make it possible to use pre-parsed, or generated by hand, difftree
info in git_difftree_body, similarly to how was and is it done in
git_patchset_body.
Use just introduced feature in git_commitdiff to parse difftree info
(raw diff output) only once: difftree info is now parsed in
git_commitdiff directly, and parsed information is passed to both
git_difftree_body and git_patchset_body. (Till now only git_blobdiff
made use of git_patchset_body ability to use pre-parsed or hand
generated info.) Additionally this makes rename info for combined diff
with renames (or copies) calculated only once in git_difftree_body;
the $difftree is modified and git_patchset_body makes use of added
info.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This corrects what was mentioned in the preceding commit (patch):
Generating "src" file name for renames in combined diff was separated
into fill_from_file_info subroutine; git_difftree_body was modified to
use it. Currently git_difftree_body and git_patchset_body fills this
info separately.
Now git_difftree_body fills this info, and git_patchset_body uses it
in the 'commitdiff' view (git_commitdiff subroutine).
gitweb/gitweb.perl | 139 +++++++++++++++++++++++++++-------------------------
1 files changed, 73 insertions(+), 66 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 53ae0b8..b3e2e07 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2330,7 +2330,13 @@ sub git_difftree_body {
my $alternate = 1;
my $patchno = 0;
foreach my $line (@{$difftree}) {
- my %diff = parse_difftree_raw_line($line);
+ my $diff;
+ if (ref($line) eq "HASH") {
+ # pre-parsed (or generated by hand)
+ $diff = $line;
+ } else {
+ $diff = parse_difftree_raw_line($line);
+ }
if ($alternate) {
print "<tr class=\"dark\">\n";
@@ -2339,21 +2345,22 @@ sub git_difftree_body {
}
$alternate ^= 1;
- if (exists $diff{'nparents'}) { # combined diff
+ if (exists $diff->{'nparents'}) { # combined diff
- fill_from_file_info(\%diff, @parents);
+ fill_from_file_info($diff, @parents)
+ unless exists $diff->{'from_file'};
- if ($diff{'to_id'} ne ('0' x 40)) {
+ if ($diff->{'to_id'} ne ('0' x 40)) {
# file exists in the result (child) commit
print "<td>" .
- $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
- file_name=>$diff{'to_file'},
+ $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+ file_name=>$diff->{'to_file'},
hash_base=>$hash),
- -class => "list"}, esc_path($diff{'to_file'})) .
+ -class => "list"}, esc_path($diff->{'to_file'})) .
"</td>\n";
} else {
print "<td>" .
- esc_path($diff{'to_file'}) .
+ esc_path($diff->{'to_file'}) .
"</td>\n";
}
@@ -2368,11 +2375,11 @@ sub git_difftree_body {
my $has_history = 0;
my $not_deleted = 0;
- for (my $i = 0; $i < $diff{'nparents'}; $i++) {
+ for (my $i = 0; $i < $diff->{'nparents'}; $i++) {
my $hash_parent = $parents[$i];
- my $from_hash = $diff{'from_id'}[$i];
- my $from_path = $diff{'from_file'}[$i];
- my $status = $diff{'status'}[$i];
+ my $from_hash = $diff->{'from_id'}[$i];
+ my $from_path = $diff->{'from_file'}[$i];
+ my $status = $diff->{'status'}[$i];
$has_history ||= ($status ne 'A');
$not_deleted ||= ($status ne 'D');
@@ -2388,17 +2395,17 @@ sub git_difftree_body {
"blob" . ($i+1)) .
" | </td>\n";
} else {
- if ($diff{'to_id'} eq $from_hash) {
+ if ($diff->{'to_id'} eq $from_hash) {
print "<td class=\"link nochange\">";
} else {
print "<td class=\"link\">";
}
print $cgi->a({-href => href(action=>"blobdiff",
- hash=>$diff{'to_id'},
+ hash=>$diff->{'to_id'},
hash_parent=>$from_hash,
hash_base=>$hash,
hash_parent_base=>$hash_parent,
- file_name=>$diff{'to_file'},
+ file_name=>$diff->{'to_file'},
file_parent=>$from_path)},
"diff" . ($i+1)) .
" | </td>\n";
@@ -2408,15 +2415,15 @@ sub git_difftree_body {
print "<td class=\"link\">";
if ($not_deleted) {
print $cgi->a({-href => href(action=>"blob",
- hash=>$diff{'to_id'},
- file_name=>$diff{'to_file'},
+ hash=>$diff->{'to_id'},
+ file_name=>$diff->{'to_file'},
hash_base=>$hash)},
"blob");
print " | " if ($has_history);
}
if ($has_history) {
print $cgi->a({-href => href(action=>"history",
- file_name=>$diff{'to_file'},
+ file_name=>$diff->{'to_file'},
hash_base=>$hash)},
"history");
}
@@ -2429,29 +2436,29 @@ sub git_difftree_body {
my ($to_mode_oct, $to_mode_str, $to_file_type);
my ($from_mode_oct, $from_mode_str, $from_file_type);
- if ($diff{'to_mode'} ne ('0' x 6)) {
- $to_mode_oct = oct $diff{'to_mode'};
+ if ($diff->{'to_mode'} ne ('0' x 6)) {
+ $to_mode_oct = oct $diff->{'to_mode'};
if (S_ISREG($to_mode_oct)) { # only for regular file
$to_mode_str = sprintf("%04o", $to_mode_oct & 0777); # permission bits
}
- $to_file_type = file_type($diff{'to_mode'});
+ $to_file_type = file_type($diff->{'to_mode'});
}
- if ($diff{'from_mode'} ne ('0' x 6)) {
- $from_mode_oct = oct $diff{'from_mode'};
+ if ($diff->{'from_mode'} ne ('0' x 6)) {
+ $from_mode_oct = oct $diff->{'from_mode'};
if (S_ISREG($to_mode_oct)) { # only for regular file
$from_mode_str = sprintf("%04o", $from_mode_oct & 0777); # permission bits
}
- $from_file_type = file_type($diff{'from_mode'});
+ $from_file_type = file_type($diff->{'from_mode'});
}
- if ($diff{'status'} eq "A") { # created
+ if ($diff->{'status'} eq "A") { # created
my $mode_chng = "<span class=\"file_status new\">[new $to_file_type";
$mode_chng .= " with mode: $to_mode_str" if $to_mode_str;
$mode_chng .= "]</span>";
print "<td>";
- print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
- hash_base=>$hash, file_name=>$diff{'file'}),
- -class => "list"}, esc_path($diff{'file'}));
+ print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+ hash_base=>$hash, file_name=>$diff->{'file'}),
+ -class => "list"}, esc_path($diff->{'file'}));
print "</td>\n";
print "<td>$mode_chng</td>\n";
print "<td class=\"link\">";
@@ -2461,17 +2468,17 @@ sub git_difftree_body {
print $cgi->a({-href => "#patch$patchno"}, "patch");
print " | ";
}
- print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
- hash_base=>$hash, file_name=>$diff{'file'})},
+ print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+ hash_base=>$hash, file_name=>$diff->{'file'})},
"blob");
print "</td>\n";
- } elsif ($diff{'status'} eq "D") { # deleted
+ } elsif ($diff->{'status'} eq "D") { # deleted
my $mode_chng = "<span class=\"file_status deleted\">[deleted $from_file_type]</span>";
print "<td>";
- print $cgi->a({-href => href(action=>"blob", hash=>$diff{'from_id'},
- hash_base=>$parent, file_name=>$diff{'file'}),
- -class => "list"}, esc_path($diff{'file'}));
+ print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'from_id'},
+ hash_base=>$parent, file_name=>$diff->{'file'}),
+ -class => "list"}, esc_path($diff->{'file'}));
print "</td>\n";
print "<td>$mode_chng</td>\n";
print "<td class=\"link\">";
@@ -2481,22 +2488,22 @@ sub git_difftree_body {
print $cgi->a({-href => "#patch$patchno"}, "patch");
print " | ";
}
- print $cgi->a({-href => href(action=>"blob", hash=>$diff{'from_id'},
- hash_base=>$parent, file_name=>$diff{'file'})},
+ print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'from_id'},
+ hash_base=>$parent, file_name=>$diff->{'file'})},
"blob") . " | ";
if ($have_blame) {
print $cgi->a({-href => href(action=>"blame", hash_base=>$parent,
- file_name=>$diff{'file'})},
+ file_name=>$diff->{'file'})},
"blame") . " | ";
}
print $cgi->a({-href => href(action=>"history", hash_base=>$parent,
- file_name=>$diff{'file'})},
+ file_name=>$diff->{'file'})},
"history");
print "</td>\n";
- } elsif ($diff{'status'} eq "M" || $diff{'status'} eq "T") { # modified, or type changed
+ } elsif ($diff->{'status'} eq "M" || $diff->{'status'} eq "T") { # modified, or type changed
my $mode_chnge = "";
- if ($diff{'from_mode'} != $diff{'to_mode'}) {
+ if ($diff->{'from_mode'} != $diff->{'to_mode'}) {
$mode_chnge = "<span class=\"file_status mode_chnge\">[changed";
if ($from_file_type ne $to_file_type) {
$mode_chnge .= " from $from_file_type to $to_file_type";
@@ -2511,9 +2518,9 @@ sub git_difftree_body {
$mode_chnge .= "]</span>\n";
}
print "<td>";
- print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
- hash_base=>$hash, file_name=>$diff{'file'}),
- -class => "list"}, esc_path($diff{'file'}));
+ print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+ hash_base=>$hash, file_name=>$diff->{'file'}),
+ -class => "list"}, esc_path($diff->{'file'}));
print "</td>\n";
print "<td>$mode_chnge</td>\n";
print "<td class=\"link\">";
@@ -2522,70 +2529,70 @@ sub git_difftree_body {
$patchno++;
print $cgi->a({-href => "#patch$patchno"}, "patch") .
" | ";
- } elsif ($diff{'to_id'} ne $diff{'from_id'}) {
+ } elsif ($diff->{'to_id'} ne $diff->{'from_id'}) {
# "commit" view and modified file (not onlu mode changed)
print $cgi->a({-href => href(action=>"blobdiff",
- hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},
+ hash=>$diff->{'to_id'}, hash_parent=>$diff->{'from_id'},
hash_base=>$hash, hash_parent_base=>$parent,
- file_name=>$diff{'file'})},
+ file_name=>$diff->{'file'})},
"diff") .
" | ";
}
- print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
- hash_base=>$hash, file_name=>$diff{'file'})},
+ print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+ hash_base=>$hash, file_name=>$diff->{'file'})},
"blob") . " | ";
if ($have_blame) {
print $cgi->a({-href => href(action=>"blame", hash_base=>$hash,
- file_name=>$diff{'file'})},
+ file_name=>$diff->{'file'})},
"blame") . " | ";
}
print $cgi->a({-href => href(action=>"history", hash_base=>$hash,
- file_name=>$diff{'file'})},
+ file_name=>$diff->{'file'})},
"history");
print "</td>\n";
- } elsif ($diff{'status'} eq "R" || $diff{'status'} eq "C") { # renamed or copied
+ } elsif ($diff->{'status'} eq "R" || $diff->{'status'} eq "C") { # renamed or copied
my %status_name = ('R' => 'moved', 'C' => 'copied');
- my $nstatus = $status_name{$diff{'status'}};
+ my $nstatus = $status_name{$diff->{'status'}};
my $mode_chng = "";
- if ($diff{'from_mode'} != $diff{'to_mode'}) {
+ if ($diff->{'from_mode'} != $diff->{'to_mode'}) {
# mode also for directories, so we cannot use $to_mode_str
$mode_chng = sprintf(", mode: %04o", $to_mode_oct & 0777);
}
print "<td>" .
$cgi->a({-href => href(action=>"blob", hash_base=>$hash,
- hash=>$diff{'to_id'}, file_name=>$diff{'to_file'}),
- -class => "list"}, esc_path($diff{'to_file'})) . "</td>\n" .
+ hash=>$diff->{'to_id'}, file_name=>$diff->{'to_file'}),
+ -class => "list"}, esc_path($diff->{'to_file'})) . "</td>\n" .
"<td><span class=\"file_status $nstatus\">[$nstatus from " .
$cgi->a({-href => href(action=>"blob", hash_base=>$parent,
- hash=>$diff{'from_id'}, file_name=>$diff{'from_file'}),
- -class => "list"}, esc_path($diff{'from_file'})) .
- " with " . (int $diff{'similarity'}) . "% similarity$mode_chng]</span></td>\n" .
+ hash=>$diff->{'from_id'}, file_name=>$diff->{'from_file'}),
+ -class => "list"}, esc_path($diff->{'from_file'})) .
+ " with " . (int $diff->{'similarity'}) . "% similarity$mode_chng]</span></td>\n" .
"<td class=\"link\">";
if ($action eq 'commitdiff') {
# link to patch
$patchno++;
print $cgi->a({-href => "#patch$patchno"}, "patch") .
" | ";
- } elsif ($diff{'to_id'} ne $diff{'from_id'}) {
+ } elsif ($diff->{'to_id'} ne $diff->{'from_id'}) {
# "commit" view and modified file (not only pure rename or copy)
print $cgi->a({-href => href(action=>"blobdiff",
- hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},
+ hash=>$diff->{'to_id'}, hash_parent=>$diff->{'from_id'},
hash_base=>$hash, hash_parent_base=>$parent,
- file_name=>$diff{'to_file'}, file_parent=>$diff{'from_file'})},
+ file_name=>$diff->{'to_file'}, file_parent=>$diff->{'from_file'})},
"diff") .
" | ";
}
- print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
- hash_base=>$parent, file_name=>$diff{'to_file'})},
+ print $cgi->a({-href => href(action=>"blob", hash=>$diff->{'to_id'},
+ hash_base=>$parent, file_name=>$diff->{'to_file'})},
"blob") . " | ";
if ($have_blame) {
print $cgi->a({-href => href(action=>"blame", hash_base=>$hash,
- file_name=>$diff{'to_file'})},
+ file_name=>$diff->{'to_file'})},
"blame") . " | ";
}
print $cgi->a({-href => href(action=>"history", hash_base=>$hash,
- file_name=>$diff{'to_file'})},
+ file_name=>$diff->{'to_file'})},
"history");
print "</td>\n";
@@ -4401,7 +4408,7 @@ sub git_commitdiff {
chomp $line;
# empty line ends raw part of diff-tree output
last unless $line;
- push @difftree, $line;
+ push @difftree, scalar parse_difftree_raw_line($line);
}
} elsif ($format eq 'plain') {
--
1.5.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/6] gitweb: Show combined diff for merge commits in 'commitdiff' view
2007-05-06 23:10 [PATCH 0/6] gitweb: Add combined diff support Jakub Narebski
` (3 preceding siblings ...)
2007-05-06 23:10 ` [PATCH 4/6] gitweb: Make it possible to use pre-parsed info in git_difftree_body Jakub Narebski
@ 2007-05-06 23:10 ` Jakub Narebski
2007-05-06 23:10 ` [PATCH 6/6] gitweb: Show combined diff for merge commits in 'commit' view Jakub Narebski
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
When 'commitdiff' action is requested without 'hp' (hash parent)
parameter, and commit given by 'h' (hash) parameter is merge commit,
show merge as combined diff.
Earlier for merge commits without 'hp' parameter diff to first parent
was shown.
Note that in compact combined (--cc) format 'uninteresting' hunks
omission mechanism can make that there is no patch corresponding to
line in raw format (difftree) output. That is why (at least for now)
we use --combined and not --cc format for showing commitdiff for merge
commits.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Note the following fragment of combined diff format description from
git-diff-tree(1) man page; note the final sentence
-c::
This flag changes the way a merge commit is displayed
(which means it is useful only when the command is given
one <tree-ish>, or '--stdin'). It shows the differences
from each of the parents to the merge result simultaneously
instead of showing pairwise diff between a parent and the
result one at a time (which is what the '-m' option does).
Furthermore, _it lists only files which were modified
from all parents_.
This means that now 'commitdiff' would show empty diff for all trival
(tree-level) merges, which I think is a majority of merges. Is showing
empty diff better than diff to first parent for merges, then? Or do we
need some way to show from which parent was final version of a file
taken?
Currently there is no way to generate 'commitdiff' view to one of the
parents by clicking some link directly from 'commitdiff' view: you
have to go via 'commit' view (click on commit subject, which functions
as switch between 'commitdiff' and 'commit' views, then on "diff" link
next to one of the parents in commit header). Perhaps this can be
improved with improving difftree/whatchanged output for combined
diff.
gitweb/gitweb.perl | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b3e2e07..c0e2473 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4391,8 +4391,10 @@ sub git_commitdiff {
}
}
+ my $hash_parent_param = $hash_parent;
if (!defined $hash_parent) {
- $hash_parent = $co{'parent'} || '--root';
+ $hash_parent_param =
+ @{$co{'parents'}} > 1 ? '-c' : $co{'parent'} || '--root';
}
# read commitdiff
@@ -4401,7 +4403,7 @@ sub git_commitdiff {
if ($format eq 'html') {
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
"--no-commit-id", "--patch-with-raw", "--full-index",
- $hash_parent, $hash, "--"
+ $hash_parent_param, $hash, "--"
or die_error(undef, "Open git-diff-tree failed");
while (my $line = <$fd>) {
@@ -4413,7 +4415,7 @@ sub git_commitdiff {
} elsif ($format eq 'plain') {
open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
- '-p', $hash_parent, $hash, "--"
+ '-p', $hash_parent_param, $hash, "--"
or die_error(undef, "Open git-diff-tree failed");
} else {
@@ -4469,10 +4471,10 @@ TEXT
# write patch
if ($format eq 'html') {
- git_difftree_body(\@difftree, $hash, $hash_parent);
+ git_difftree_body(\@difftree, $hash, $hash_parent || @{$co{'parents'}});
print "<br/>\n";
- git_patchset_body($fd, \@difftree, $hash, $hash_parent);
+ git_patchset_body($fd, \@difftree, $hash, $hash_parent || @{$co{'parents'}});
close $fd;
print "</div>\n"; # class="page_body"
git_footer_html();
--
1.5.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/6] gitweb: Show combined diff for merge commits in 'commit' view
2007-05-06 23:10 [PATCH 0/6] gitweb: Add combined diff support Jakub Narebski
` (4 preceding siblings ...)
2007-05-06 23:10 ` [PATCH 5/6] gitweb: Show combined diff for merge commits in 'commitdiff' view Jakub Narebski
@ 2007-05-06 23:10 ` Jakub Narebski
2007-05-06 23:10 ` [PATCH 7/6] todo: Remove "Gitweb diff on merge commits" entry Jakub Narebski
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
When commit shown is a merge commit (has more than one parent),
display combined difftree output (result of git-diff-tree -c).
Earlier (since commit 549ab4a30703012ff3a12b5455d319216805a8db)
difftree output (against first parent) was not printed for merges.
Examples of non-trivial merges:
5bac4a671907604b5fb4e24ff682d5b0e8431931 (includes rename)
addafaf92eeb86033da91323d0d3ad7a496dae83 (five parents)
95f97567c1887d77f3a46b42d8622c76414d964d (evil merge)
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The same note as for the patch before apply: for trivial merges there
is no difftree output. But for 'commit' view it doesn't matter much
because there was no difftree output at all for merges.
I plan to add HTML diffstat (using divs with set background color and
set width for bar graph for diff stats) for merges for 'commit' view
(diffstat to first parent, aka. "damages"), and text diffstat for
'commitdiff_plain' view. It would be controlled by new %feature.
But if somebody else want to implement this, feel free.
Examples are included to make it possible to check combined diff
output in gitweb: the sha1 of commits should be turned into
hyperlinks (committags support), and you can click on them to go to
'commit' view.
gitweb/gitweb.perl | 20 ++++++++------------
1 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c0e2473..53e6dce 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4026,14 +4026,13 @@ sub git_commit {
$parent = "--root";
}
my @difftree;
- if (@$parents <= 1) {
- # difftree output is not printed for merges
- open my $fd, "-|", git_cmd(), "diff-tree", '-r', "--no-commit-id",
- @diff_opts, $parent, $hash, "--"
- or die_error(undef, "Open git-diff-tree failed");
- @difftree = map { chomp; $_ } <$fd>;
- close $fd or die_error(undef, "Reading git-diff-tree failed");
- }
+ open my $fd, "-|", git_cmd(), "diff-tree", '-r', "--no-commit-id",
+ @diff_opts,
+ (@$parents <= 1 ? $parent : '-c'),
+ $hash, "--"
+ or die_error(undef, "Open git-diff-tree failed");
+ @difftree = map { chomp; $_ } <$fd>;
+ close $fd or die_error(undef, "Reading git-diff-tree failed");
# non-textual hash id's can be cached
my $expires;
@@ -4111,10 +4110,7 @@ sub git_commit {
git_print_log($co{'comment'});
print "</div>\n";
- if (@$parents <= 1) {
- # do not output difftree/whatchanged for merges
- git_difftree_body(\@difftree, $hash, $parent);
- }
+ git_difftree_body(\@difftree, $hash, @$parents);
git_footer_html();
}
--
1.5.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/6] todo: Remove "Gitweb diff on merge commits" entry
2007-05-06 23:10 [PATCH 0/6] gitweb: Add combined diff support Jakub Narebski
` (5 preceding siblings ...)
2007-05-06 23:10 ` [PATCH 6/6] gitweb: Show combined diff for merge commits in 'commit' view Jakub Narebski
@ 2007-05-06 23:10 ` Jakub Narebski
2007-05-08 1:31 ` [PATCH 0/6] gitweb: Add combined diff support Junio C Hamano
2007-05-16 22:05 ` [PATCH 7/6] gitweb: Empty patch for merge means trivial merge, not no differences Jakub Narebski
8 siblings, 0 replies; 12+ messages in thread
From: Jakub Narebski @ 2007-05-06 23:10 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch applies to 'todo' branch!
Perhaps TODO entry should be replaced (updated) to say about format
of gitweb diff for merge commits (diffstat in HTML, improving difftree
for merges), instead of just deleting it.
TODO | 11 -----------
1 files changed, 0 insertions(+), 11 deletions(-)
diff --git a/TODO b/TODO
index 6c603db..305363a 100644
--- a/TODO
+++ b/TODO
@@ -85,17 +85,6 @@ the box.
[jc: limbo?]
-* Gitweb diff on merge commits
-
-From: Linus Torvalds <torvalds@osdl.org>
-Subject: Re: git show and gitweb gives different result for kernel
-Message-ID: <Pine.LNX.4.64.0610061202060.3952@g5.osdl.org>
-
-Maybe allow gitweb to show diff with any parent and diff --cc,
-not just diff with the first parent for a merge.
-
-[jc: Jakub is interested in it]
-
* Delegate gitweb part to somebody else.
* Use gitattributes for more things.
--
1.5.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/6] gitweb: Add combined diff support
2007-05-06 23:10 [PATCH 0/6] gitweb: Add combined diff support Jakub Narebski
` (6 preceding siblings ...)
2007-05-06 23:10 ` [PATCH 7/6] todo: Remove "Gitweb diff on merge commits" entry Jakub Narebski
@ 2007-05-08 1:31 ` Junio C Hamano
2007-05-08 1:50 ` Jakub Narebski
2007-05-16 22:05 ` [PATCH 7/6] gitweb: Empty patch for merge means trivial merge, not no differences Jakub Narebski
8 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2007-05-08 1:31 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
I've minimally tried this on my private machine. Looks pretty
nice for simple merges, but I think we would want --cc not -c
most of the time.
Pushed out in 'next'. Thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/6] gitweb: Add combined diff support
2007-05-08 1:31 ` [PATCH 0/6] gitweb: Add combined diff support Junio C Hamano
@ 2007-05-08 1:50 ` Jakub Narebski
0 siblings, 0 replies; 12+ messages in thread
From: Jakub Narebski @ 2007-05-08 1:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Junio C Hamano wrote:
> I've minimally tried this on my private machine. Looks pretty
> nice for simple merges, but I think we would want --cc not -c
> most of the time.
Please note that --cc would need some changes to both git_difftree_body
and git_patchset_body; it is not just matter of replacing
'-c' by '--cc'. Hunk simplification might mean that whole patch
vanishes. So sometimes we have difftree (raw diff, whatchanged) line
which does not have corresponding patch, and there should be no "patch"
link (this is harder part). This mean also that in git_patchset_body we
need sometimes to skip some difftree line / difftree info line (this is
easier part).
--
Jakub Narebski
Poland
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 7/6] gitweb: Empty patch for merge means trivial merge, not no differences
2007-05-06 23:10 [PATCH 0/6] gitweb: Add combined diff support Jakub Narebski
` (7 preceding siblings ...)
2007-05-08 1:31 ` [PATCH 0/6] gitweb: Add combined diff support Junio C Hamano
@ 2007-05-16 22:05 ` Jakub Narebski
2007-05-16 22:08 ` Junio C Hamano
8 siblings, 1 reply; 12+ messages in thread
From: Jakub Narebski @ 2007-05-16 22:05 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
Earlier commit 4280cde95fa4e3fb012eb6d0c239a7777baaf60c made gitweb
show "No differences found" message for empty diff, for the HTML
output. But for merge commits, either -c format we use or --cc format,
empty diff doesn't mean no differences, but trivial merge.
Show "Trivial merge" message instead of "No differences found" for
merges.
While at it reword conditional in the code for easier reading.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
gitweb/gitweb.perl | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 549e027..8c688be 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2877,7 +2877,14 @@ sub git_patchset_body {
} continue {
print "</div>\n"; # class="patch"
}
- print "<div class=\"diff nodifferences\">No differences found</div>\n" if (!$patch_number);
+
+ if ($patch_number == 0) {
+ if (@hash_parents > 1) {
+ print "<div class=\"diff nodifferences\">Trivial merge</div>\n";
+ } else {
+ print "<div class=\"diff nodifferences\">No differences found</div>\n";
+ }
+ }
print "</div>\n"; # class="patchset"
}
--
1.5.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 7/6] gitweb: Empty patch for merge means trivial merge, not no differences
2007-05-16 22:05 ` [PATCH 7/6] gitweb: Empty patch for merge means trivial merge, not no differences Jakub Narebski
@ 2007-05-16 22:08 ` Junio C Hamano
0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2007-05-16 22:08 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
Jakub Narebski <jnareb@gmail.com> writes:
> Earlier commit 4280cde95fa4e3fb012eb6d0c239a7777baaf60c made gitweb
> show "No differences found" message for empty diff, for the HTML
> output. But for merge commits, either -c format we use or --cc format,
> empty diff doesn't mean no differences, but trivial merge.
>
> Show "Trivial merge" message instead of "No differences found" for
> merges.
Sounds good. Some people might want to treat "-s ours" merge
specially (in real life, when two branches fix the same bug
differently the result of the hand-resolution may end up being
the same as "-s ours" merge), although I do not think it is such
a big deal.
^ permalink raw reply [flat|nested] 12+ messages in thread