[PATCH] gitweb: Extra columns in blame

[PATCH] gitweb: Extra columns in blame

[Petr Baudis]

This patch adds extra columns to blame output, containing
line author and creation date. These columns are by default hidden by
display: none but by clicking on the expansion "button" you can display
them (and hide again). I think seeing this information without tooltips
fishing can give much better overview of the content evolution.

This patch depends on the interactive blame patch (but can be factored out;
the common required parts are just the blame.js infrastructure).

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 gitweb/blame.js    |   45 ++++++++++++++++++++++++++++++++++++++++++++-
 gitweb/gitweb.css  |    5 +++++
 gitweb/gitweb.perl |   23 +++++++++++++++++++----
 3 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/gitweb/blame.js b/gitweb/blame.js
index 88b6499..438bc9c 100644
--- a/gitweb/blame.js
+++ b/gitweb/blame.js
@@ -1,4 +1,41 @@
 // Copyright (C) 2007, Fredrik Kuivinen <frekui@gmail.com>
+// Copyright (C) 2007, Petr Baudis <pasky@suse.cz>
+
+
+// blame extra columns
+
+// I would like to note here that JavaScript is utterly stupid.
+function findStyleRule(styleName) {
+	for (i = 0; i < document.styleSheets.length; i++) { 
+		for (j = 0; j < document.styleSheets[i].cssRules.length; j++) {
+			var rule = document.styleSheets[i].cssRules[j];
+			if (rule.selectorText == styleName) {
+				return rule;
+			}
+		}     
+	}
+}
+var extra_column_rule;
+
+var extra_columns = 0;
+function extra_blame_columns() {
+	if (!extra_column_rule)
+		extra_column_rule = findStyleRule("th.extra_column, td.extra_column");
+
+	if (!extra_columns) {
+		document.getElementById("columns_expander").innerHTML = "[-]";
+		extra_column_rule.style.cssText = extra_column_rule.style.cssText.replace("none", "table-cell");
+		extra_columns = 1;
+	} else {
+		document.getElementById("columns_expander").innerHTML = "[+]";
+		extra_column_rule.style.cssText = extra_column_rule.style.cssText.replace("table-cell", "none");
+		extra_columns = 0;
+	}
+}
+
+
+// blame_interactive
+
 
 var DEBUG = 0;
 function debug(str)
@@ -72,14 +109,20 @@ function handleLine(commit)
             zeroPad(date.getUTCSeconds());
         tr.firstChild.title = commit.author + ', ' + dateStr + ' ' + timeStr;
         var shaAnchor = tr.firstChild.firstChild;
+	var authorField = tr.firstChild.nextSibling;
+	var dateField = tr.firstChild.nextSibling.nextSibling;
         if (i == 0) {
             shaAnchor.href = baseUrl + ';a=commit;h=' + commit.sha1;
             shaAnchor.innerHTML = commit.sha1.substr(0, 8);
+	    authorField.innerHTML = commit.author;
+	    dateField.innerHTML = dateStr + ' ' + timeStr;
         } else {
             shaAnchor.innerHTML = '';
+            authorField.innerHTML = '';
+            dateField.innerHTML = '';
         }
 
-        var lineAnchor = tr.firstChild.nextSibling.firstChild;
+        var lineAnchor = tr.firstChild.nextSibling.nextSibling.nextSibling.firstChild;
         lineAnchor.href = baseUrl + ';a=blame;hb=' + commit.sha1 +
             ';f=' + commit.filename + '#l' + commit.srcline;
         resline++;
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 9f0822f..b76e839 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -488,3 +488,8 @@ span.match {
 div.binary {
 	font-style: italic;
 }
+
+/* This selector is hardcoded in gitweb.perl */
+th.extra_column, td.extra_column {
+	display: none;
+}
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cc671b1..2dc7be8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3671,9 +3671,14 @@ sub git_blame_incremental {
 	my $last_rev;
 	print "<script type=\"text/javascript\" src=\"$blamejs\"></script>\n";
 	print <<HTML;
+
 <div class="page_body">
 <table class="blame">
-<tr><th>Commit</th><th>Line</th><th>Data</th></tr>
+<tr><th>Commit&nbsp;<a href="javascript:extra_blame_columns()" id="columns_expander">[+]</a></th>
+<th class="extra_column">Author</th>
+<th class="extra_column">Date</th>
+<th>Line</th>
+<th>Data</th></tr>
 HTML
 	my %metainfo = ();
 	my $linenr = 0;
@@ -3682,6 +3687,8 @@ HTML
 		$linenr += 1;
 		print "<tr id=\"l$linenr\" class=\"light2\">";
 		print '<td class="sha1"><a href=""></a></td>';
+		print "<td class=\"extra_column\"></td>";
+		print "<td class=\"extra_column\"></td>";
 		print "<td class=\"linenr\"><a class=\"linenr\" href=\"\">$linenr</a></td><td class=\"pre\">" . esc_html($_) . "</td>\n";
 		print "</tr>\n"
 	}
@@ -3738,10 +3745,16 @@ sub git_blame2 {
 	my $num_colors = scalar(@rev_color);
 	my $current_color = 0;
 	my $last_rev;
+	print "<script type=\"text/javascript\" src=\"$blamejs\"></script>\n";
 	print <<HTML;
+
 <div class="page_body">
 <table class="blame">
-<tr><th>Commit</th><th>Line</th><th>Data</th></tr>
+<tr><th>Commit&nbsp;<a href="javascript:extra_blame_columns()" id="columns_expander">[+]</a></th>
+<th class="extra_column">Author</th>
+<th class="extra_column">Date</th>
+<th>Line</th>
+<th>Data</th></tr>
 HTML
 	my %metainfo = ();
 	while (1) {
@@ -3771,15 +3784,17 @@ HTML
 		}
 		print "<tr class=\"$rev_color[$current_color]\">\n";
 		if ($group_size) {
+			my $rowspan = $group_size > 1 ? " rowspan=\"$group_size\"" : "";
 			print "<td class=\"sha1\"";
 			print " title=\"". esc_html($author) . ", $date\"";
-			print " rowspan=\"$group_size\"" if ($group_size > 1);
-			print ">";
+			print "$rowspan>";
 			print $cgi->a({-href => href(action=>"commit",
 			                             hash=>$full_rev,
 			                             file_name=>$file_name)},
 			              esc_html($rev));
 			print "</td>\n";
+			print "<td class=\"extra_column\" $rowspan>". esc_html($author) . "</td>";
+			print "<td class=\"extra_column\" $rowspan>". $date . "</td>";
 		}
 		open (my $dd, "-|", git_cmd(), "rev-parse", "$full_rev^")
 			or die_error(undef, "Open git-rev-parse failed");

Re: [PATCH] gitweb: Extra columns in blame

[Junio C Hamano]

Good job, except that I think you should also show the filename
especially as you seem to run with -C (I haven't looked at the
code yet, though).

E.g.

http://repo.or.cz/w/linux-2.6.git?a=blame_incremental;f=block/ll_rw_blk.c;h=6b5173ac81313d8adb5c1d7b521559f565bb209b;hb=347b4599dd6ffef27e18c227532d1ec66556000b

the first few hunks that came from 1da177e4 are from a different
file, drivers/block/ll_rw_blk.c.

Also the incremental thing using JavaScript does not seem to
work for me incrementally for some reason, although if I wait
long enough I get the fully blamed picture that seems to match
nonincremental one.  While I am waiting, the browser goes silent
and does not even let me switch to other tabs, so it is not all
that useful to me in its current shape.

Re: [PATCH] gitweb: Extra columns in blame

[Petr Baudis]

On Fri, May 18, 2007 at 11:01:01PM CEST, Junio C Hamano wrote:
> Good job, except that I think you should also show the filename
> especially as you seem to run with -C (I haven't looked at the
> code yet, though).
> 
> E.g.
> 
> http://repo.or.cz/w/linux-2.6.git?a=blame_incremental;f=block/ll_rw_blk.c;h=6b5173ac81313d8adb5c1d7b521559f565bb209b;hb=347b4599dd6ffef27e18c227532d1ec66556000b
> 
> the first few hunks that came from 1da177e4 are from a different
> file, drivers/block/ll_rw_blk.c.

I don't use git-blame -C - I pass virtually no extra parameters to
git-blame (except some output controlling). Passing -C to git-blame
might be an interesting idea but the possible performance hit is a bit
scary; when the dust settles and this gets merged or something, I can
experiment with it a bit further...

(Also, I fear a bit about making it _too_ wide even with the extra
columns; there should be reasonable portion of line still visible on
usual resolutions with usual font sizes. Might be nice UI challenge.

> Also the incremental thing using JavaScript does not seem to
> work for me incrementally for some reason, although if I wait
> long enough I get the fully blamed picture that seems to match
> nonincremental one.  While I am waiting, the browser goes silent
> and does not even let me switch to other tabs, so it is not all
> that useful to me in its current shape.

Strange, what browser are you using?

The trouble is, I'm not really very good at this kind of web development
because I have access only to a rather narrow portion of the browser
market - Firefox at Linux, at work also Galeon, Epiphany and Konqueror,
and of course ELinks. I have theoretical access to MSIE at work but it's
quite a hassle. With all browser I've tested it with, it worked without
a problem, so I'm not sure how much will I be able to debug it (and I'm
really bad at debugging javascript anyway).

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett

Re: [PATCH] gitweb: Extra columns in blame

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>> Also the incremental thing using JavaScript does not seem to
>> work for me incrementally for some reason, although if I wait
>> long enough I get the fully blamed picture that seems to match
>> nonincremental one.  While I am waiting, the browser goes silent
>> and does not even let me switch to other tabs, so it is not all
>> that useful to me in its current shape.
>
> Strange, what browser are you using?

Firefox (I think it is 2.0) running on a W2k box behind two http
proxies.

Re: [PATCH] gitweb: Extra columns in blame

[A Large Angry SCM]

Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
>>> Also the incremental thing using JavaScript does not seem to
>>> work for me incrementally for some reason, although if I wait
>>> long enough I get the fully blamed picture that seems to match
>>> nonincremental one.  While I am waiting, the browser goes silent
>>> and does not even let me switch to other tabs, so it is not all
>>> that useful to me in its current shape.
>> Strange, what browser are you using?
> 
> Firefox (I think it is 2.0) running on a W2k box behind two http
> proxies.

Same symptoms with FireFox 1.5.0.10 running on Suse 9.3.