[PATCH 2/6] gitweb: Add combined diff support to git_difftree_body

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
---
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

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]