[PATCH 0/5] gitweb: Additions to commitdiff view

[PATCH 0/5] gitweb: Additions to commitdiff view

[Jakub Narebski]

This series of patches makes signoff lines to be not removed in commitdiff and log 
views, adds author information to comittdiff view, and adds diff tree with links
to patches in commitdiff view.

You will be able to see it at work at
  http://front.fuw.edu.pl/cgi-bin/jnareb/gitweb.cgi

Shortlog:
---------
1/5  gitweb: Make git_print_log generic; git_print_simplified_log uses it
2/5  gitweb: Do not remove signoff lines in git_print_simplified_log
3/5  gitweb: Add author information to commitdiff view
4/5  gitweb: git_print_log: signoff line is non-empty line
5/5  gitweb: Add diff tree, with links to patches, to commitdiff view

-- 
Jakub Narebski
ShadeHawk on #git
Poland

[PATCH 1/5] gitweb: Make git_print_log generic; git_print_simplified_log uses it

[Jakub Narebski]

Collapse git_print_log and git_print_simplified_log into one
subroutine git_print_log.  git_print_simplified_log now simply calls
git_print_log with proper options.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   63 ++++++++++++++++++++++++++--------------------------
 1 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 59f1a87..e318f50 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1364,9 +1364,15 @@ sub git_print_page_path {
 	}
 }
 
-sub git_print_log {
+# sub git_print_log (\@;%) {
+sub git_print_log ($;%) {
 	my $log = shift;
+	my %opts = @_;
 
+	if ($opts{'-remove_title'}) {
+		# remove title, i.e. first line of log
+		shift @$log;
+	}
 	# remove leading empty lines
 	while (defined $log->[0] && $log->[0] eq "") {
 		shift @$log;
@@ -1376,6 +1382,19 @@ sub git_print_log {
 	my $signoff = 0;
 	my $empty = 0;
 	foreach my $line (@$log) {
+		if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
+			$signoff = 1;
+			if (! $opts{'-remove_signoff'}) {
+				print "<span class=\"signoff\">" . esc_html($line) . "</span><br/>\n";
+				next;
+			} else {
+				# remove signoff lines
+				next;
+			}
+		} else {
+			$signoff = 0;
+		}
+
 		# print only one empty line
 		# do not print empty line after signoff
 		if ($line eq "") {
@@ -1384,13 +1403,13 @@ sub git_print_log {
 		} else {
 			$empty = 0;
 		}
-		if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
-			$signoff = 1;
-			print "<span class=\"signoff\">" . esc_html($line) . "</span><br/>\n";
-		} else {
-			$signoff = 0;
-			print format_log_line_html($line) . "<br/>\n";
-		}
+
+		print format_log_line_html($line) . "<br/>\n";
+	}
+
+	if ($opts{'-final_empty_line'}) {
+		# end with single empty line
+		print "<br/>\n" unless $empty;
 	}
 }
 
@@ -1398,30 +1417,10 @@ sub git_print_simplified_log {
 	my $log = shift;
 	my $remove_title = shift;
 
-	shift @$log if $remove_title;
-	# remove leading empty lines
-	while (defined $log->[0] && $log->[0] eq "") {
-		shift @$log;
-	}
-
-	# simplify and print log
-	my $empty = 0;
-	foreach my $line (@$log) {
-		# remove signoff lines
-		if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
-			next;
-		}
-		# print only one empty line
-		if ($line eq "") {
-			next if $empty;
-			$empty = 1;
-		} else {
-			$empty = 0;
-		}
-		print format_log_line_html($line) . "<br/>\n";
-	}
-	# end with single empty line
-	print "<br/>\n" unless $empty;
+	git_print_log($log,
+		-final_empty_line=> 1,
+		-remove_signoff => 1,
+		-remove_title => $remove_title);
 }
 
 ## ......................................................................
-- 
1.4.1.1

[PATCH 2/5] gitweb: Do not remove signoff lines in git_print_simplified_log

[Jakub Narebski]

Remove '-remove_signoff => 1' option to git_print_log call in the
git_print_simplified_log subroutine.  This means that in "log" and
"commitdiff" views (git_log and git_commitdiff subroutines) signoff
lines will be shown.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e318f50..c2dcc6c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1419,7 +1419,6 @@ sub git_print_simplified_log {
 
 	git_print_log($log,
 		-final_empty_line=> 1,
-		-remove_signoff => 1,
 		-remove_title => $remove_title);
 }
 
-- 
1.4.1.1

[PATCH 3/5] gitweb: Add author information to commitdiff view

[Jakub Narebski]

Add subroutine git_print_authorship to print author and date of
commit, div.author_date style to CSS, and use them in git_commitdiff.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.css  |    7 +++++++
 gitweb/gitweb.perl |   11 +++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index afd9e8a..eb9fc38 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -116,6 +116,13 @@ div.list_head {
 	font-style: italic;
 }
 
+div.author_date {
+	padding: 8px;
+	border: solid #d9d8d1;
+	border-width: 0px 0px 1px 0px;
+	font-style: italic;
+}
+
 a.list {
 	text-decoration: none;
 	color: #000000;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c2dcc6c..81d3e76 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1341,6 +1341,16 @@ sub git_print_header_div {
 	      "\n</div>\n";
 }
 
+#sub git_print_authorship (\%) {
+sub git_print_authorship {
+	my $co = shift;
+
+	my %ad = parse_date($co->{'author_epoch'});
+	print "<div class=\"author_date\">" .
+	      esc_html($co->{'author_name'}) .
+	      " [$ad{'rfc2822'}]</div>\n";
+}
+
 sub git_print_page_path {
 	my $name = shift;
 	my $type = shift;
@@ -2914,6 +2924,7 @@ sub git_commitdiff {
 		git_header_html(undef, $expires);
 		git_print_page_nav('commitdiff','', $hash,$co{'tree'},$hash, $formats_nav);
 		git_print_header_div('commit', esc_html($co{'title'}) . $ref, $hash);
+		git_print_authorship(\%co);
 		print "<div class=\"page_body\">\n";
 		print "<div class=\"log\">\n";
 		git_print_simplified_log($co{'comment'}, 1); # skip title
-- 
1.4.1.1

[PATCH 4/5] gitweb: git_print_log: signoff line is non-empty line

[Jakub Narebski]

This correct minor error in git_print_log that didn't add final empty
line when requested, if commit log ended with signoff.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 81d3e76..8987967 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1394,6 +1394,7 @@ sub git_print_log ($;%) {
 	foreach my $line (@$log) {
 		if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
 			$signoff = 1;
+			$empty = 0;
 			if (! $opts{'-remove_signoff'}) {
 				print "<span class=\"signoff\">" . esc_html($line) . "</span><br/>\n";
 				next;
-- 
1.4.1.1

[PATCH 5/5] gitweb: Add diff tree, with links to patches, to commitdiff view

[Jakub Narebski]

Added/uncommented git_difftree_body invocation in git_commitdiff.
Added anchors (via 'id' attribute) to patches in patchset.
git_difftree_body is modified to link to patch anchor when called from
git_commitdiff, instead of link to blobdiff.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   71 ++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8987967..ef09cf5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1447,6 +1447,7 @@ sub git_difftree_body {
 
 	print "<table class=\"diff_tree\">\n";
 	my $alternate = 0;
+	my $patchno = 0;
 	foreach my $line (@{$difftree}) {
 		my %diff = parse_difftree_raw_line($line);
 
@@ -1487,8 +1488,14 @@ sub git_difftree_body {
 			      "<td class=\"link\">" .
 			      $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
 			                             hash_base=>$hash, file_name=>$diff{'file'})},
-			              "blob") .
-			      "</td>\n";
+			              "blob");
+			if ($action == "commitdiff") {
+				# link to patch
+				$patchno++;
+				print " | " .
+				      $cgi->a({-href => "#patch$patchno"}, "patch");
+			}
+			print "</td>\n";
 
 		} elsif ($diff{'status'} eq "D") { # deleted
 			my $mode_chng = "<span class=\"file_status deleted\">[deleted $from_file_type]</span>";
@@ -1502,8 +1509,14 @@ sub git_difftree_body {
 			      $cgi->a({-href => href(action=>"blob", hash=>$diff{'from_id'},
 			                             hash_base=>$parent, file_name=>$diff{'file'})},
 			              "blob") .
-			      " | " .
-			      $cgi->a({-href => href(action=>"history", hash_base=>$parent,
+			      " | ";
+			if ($action == "commitdiff") {
+				# link to patch
+				$patchno++;
+				print " | " .
+				      $cgi->a({-href => "#patch$patchno"}, "patch");
+			}
+			print $cgi->a({-href => href(action=>"history", hash_base=>$parent,
 			                             file_name=>$diff{'file'})},
 			              "history") .
 			      "</td>\n";
@@ -1539,16 +1552,23 @@ sub git_difftree_body {
 			print "</td>\n" .
 			      "<td>$mode_chnge</td>\n" .
 			      "<td class=\"link\">" .
-				$cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
-				                       hash_base=>$hash, file_name=>$diff{'file'})},
-				        "blob");
+			      $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
+			                             hash_base=>$hash, file_name=>$diff{'file'})},
+			              "blob");
 			if ($diff{'to_id'} ne $diff{'from_id'}) { # modified
-				print " | " .
-					$cgi->a({-href => href(action=>"blobdiff",
-					                       hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},
-					                       hash_base=>$hash, hash_parent_base=>$parent,
-					                       file_name=>$diff{'file'})},
-					        "diff");
+				if ($action == "commitdiff") {
+					# link to patch
+					$patchno++;
+					print " | " .
+						$cgi->a({-href => "#patch$patchno"}, "patch");
+				} else {
+					print " | " .
+						$cgi->a({-href => href(action=>"blobdiff",
+						                       hash=>$diff{'to_id'}, hash_parent=>$diff{'from_id'},
+						                       hash_base=>$hash, hash_parent_base=>$parent,
+						                       file_name=>$diff{'file'})},
+						        "diff");
+				}
 			}
 			print " | " .
 				$cgi->a({-href => href(action=>"history",
@@ -1578,12 +1598,19 @@ sub git_difftree_body {
 			                             hash=>$diff{'to_id'}, file_name=>$diff{'to_file'})},
 			              "blob");
 			if ($diff{'to_id'} ne $diff{'from_id'}) {
-				print " | " .
-					$cgi->a({-href => href(action=>"blobdiff",
-					                       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'})},
-					        "diff");
+				if ($action == "commitdiff") {
+					# link to patch
+					$patchno++;
+					print " | " .
+						$cgi->a({-href => "#patch$patchno"}, "patch");
+				} else {
+					print " | " .
+						$cgi->a({-href => href(action=>"blobdiff",
+						                       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'})},
+						        "diff");
+				}
 			}
 			print "</td>\n";
 
@@ -1616,7 +1643,7 @@ sub git_patchset_body {
 				# first patch in patchset
 				$patch_found = 1;
 			}
-			print "<div class=\"patch\">\n";
+			print "<div class=\"patch\" id=\"patch". ($patch_idx+1) ."\">\n";
 
 			if (ref($difftree->[$patch_idx]) eq "HASH") {
 				$diffinfo = $difftree->[$patch_idx];
@@ -2958,8 +2985,8 @@ TEXT
 
 	# write patch
 	if ($format eq 'html') {
-		#git_difftree_body(\@difftree, $hash, $hash_parent);
-		#print "<br/>\n";
+		git_difftree_body(\@difftree, $hash, $hash_parent);
+		print "<br/>\n";
 
 		git_patchset_body($fd, \@difftree, $hash, $hash_parent);
 		close $fd;
-- 
1.4.1.1

Re: [PATCH 0/5] gitweb: Additions to commitdiff view

[Linus Torvalds]

On Mon, 28 Aug 2006, Jakub Narebski wrote:
>
> This series of patches makes signoff lines to be not removed in commitdiff and log 
> views, adds author information to comittdiff view, and adds diff tree with links
> to patches in commitdiff view.
> 
> You will be able to see it at work at
>   http://front.fuw.edu.pl/cgi-bin/jnareb/gitweb.cgi

Goodie, that looks exactly like I envisioned. And the shortcuts to find 
the patch to a specific file (when the patch is larger) works beautifully.

Now I think "commit-diff" is prettier than the "commit" view (the latter 
ends up showing the "--pretty=full" information, which can be useful, but 
usually is just distracting). That's as it should be - I consider 
"commit-diff" to be the _normal_ thing, and then the "commit" view is the 
"give me all the ugly details in just the commit".

I've got _one_ small beef with gitweb still, which is that it seems to 
like always showing things in UTC rather than the "native" timezone, but I 
can see why people would sometimes want that. So I'm not actually sure 
it's wrong.

I think it _may_ be worth showing the native timezone in the "commit-diff" 
view (when you see only one commit), and then show the UTC time in the 
"log" view (when you see a lot of commits, and might want to compare times 
in different timezones more easily).

But I think that timezone thing is probably a matter of taste rather than 
much anything else.

Thanks,

		Linus

[PATCH] gitweb: Add local time and timezone to git_print_authorship

[Jakub Narebski]

Linus Torvalds wrote:
> I've got _one_ small beef with gitweb still, which is that it seems to 
> like always showing things in UTC rather than the "native" timezone, but I 
> can see why people would sometimes want that. So I'm not actually sure 
> it's wrong.
> 
> I think it _may_ be worth showing the native timezone in the "commit-diff" 
> view (when you see only one commit), and then show the UTC time in the 
> "log" view (when you see a lot of commits, and might want to compare times 
> in different timezones more easily).
> 
> But I think that timezone thing is probably a matter of taste rather than 
> much anything else.

-- >8 --
Add local time (hours and minutes) and local timezone to the output of
git_print_authorship command, used by git_commitdiff.  The code was
taken from git_commit subroutine.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ef09cf5..fa7f62a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1345,10 +1345,18 @@ #sub git_print_authorship (\%) {
 sub git_print_authorship {
 	my $co = shift;
 
-	my %ad = parse_date($co->{'author_epoch'});
+	my %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});
 	print "<div class=\"author_date\">" .
 	      esc_html($co->{'author_name'}) .
-	      " [$ad{'rfc2822'}]</div>\n";
+	      " [$ad{'rfc2822'}";
+	if ($ad{'hour_local'} < 6) {
+		printf(" (<span class=\"atnight\">%02d:%02d</span> %s)",
+		       $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});
+	} else {
+		printf(" (%02d:%02d %s)",
+		       $ad{'hour_local'}, $ad{'minute_local'}, $ad{'tz_local'});
+	}
+	print "]</div>\n";
 }
 
 sub git_print_page_path {
-- 
1.4.1.1

Re: [PATCH] gitweb: Add local time and timezone to git_print_authorship

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Add local time (hours and minutes) and local timezone to the output of
> git_print_authorship command, used by git_commitdiff.

Looks nice, thanks.

Now I got envious seeing people are having SO MUCH FUN with
gitweb, so here is mine...

Likes, dislikes, "your color selection sucks ;-)",... ?

-- >8 --
gitweb: show age and author in blame output

This does two things.

 - The commit object name link on each link uses "title", which
   shows the author and age of the particular line when hovered
   over.

 - The background of these links are painted on darker color as
   they become older and the links on younger lines are shown on
   lighter background.

---
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index eb9fc38..008ee70 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -215,6 +215,12 @@ td.sha1 {
 	font-family: monospace;
 }
 
+td.age-week   { color: #00f; background-color: #fff; }
+td.age-month  { color: #00f; background-color: #eef; }
+td.age-season { color: #00f; background-color: #ddf; }
+td.age-year   { color: #00f; background-color: #ccf; }
+td.age-old    { color: #00f; background-color: #bbf; }
+
 td.error {
 	color: red;
 	background-color: yellow;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 9324d71..7dbd40f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2190,7 +2190,7 @@ sub git_blame2 {
 	if ($ftype !~ "blob") {
 		die_error("400 Bad Request", "Object is not a blob");
 	}
-	open ($fd, "-|", git_cmd(), "blame", '-l', $file_name, $hash_base)
+	open ($fd, "-|", git_cmd(), "blame", '-l', '-t', $file_name, $hash_base)
 		or die_error(undef, "Open git-blame failed");
 	git_header_html();
 	my $formats_nav =
@@ -2211,12 +2211,21 @@ sub git_blame2 {
 <table class="blame">
 <tr><th>Commit</th><th>Line</th><th>Data</th></tr>
 HTML
+	my $now = time();
 	while (<$fd>) {
-		/^([0-9a-fA-F]{40}).*?(\d+)\)\s{1}(\s*.*)/;
-		my $full_rev = $1;
+		my ($full_rev, $author, $timestamp, $zone, $lineno, $data) =
+		    /^([0-9a-fA-F]{40})\s\((.*?)\s+(\d+)\s
+		    ([-+\d]{5})\s+(\d+)\)\s{1}(\s*.*)/x;
 		my $rev = substr($full_rev, 0, 8);
-		my $lineno = $2;
-		my $data = $3;
+
+		my $age = $now - $timestamp;
+		my $ago = age_string($age);
+		my $pop = "$author, $ago";
+		my $agegroup =
+		    (($age < 60*60*24*7) ? "age-week" :
+		     ($age < 60*60*24*30) ? "age-month" :
+		     ($age < 60*60*24*120) ? "age-season" :
+		     ($age < 60*60*24*360) ? "age-year" : "age-old");
 
 		if (!defined $last_rev) {
 			$last_rev = $full_rev;
@@ -2225,7 +2234,8 @@ HTML
 			$current_color = ++$current_color % $num_colors;
 		}
 		print "<tr class=\"$rev_color[$current_color]\">\n";
-		print "<td class=\"sha1\">" .
+		print "<td class=\"sha1 $agegroup\" title=\"" .
+			esc_html($pop) ."\">" .
 			$cgi->a({-href => href(action=>"commit", hash=>$full_rev, file_name=>$file_name)},
 			        esc_html($rev)) . "</td>\n";
 		print "<td class=\"linenr\"><a id=\"l$lineno\" href=\"#l$lineno\" class=\"linenr\">" .

Re: [PATCH] gitweb: Add local time and timezone to git_print_authorship

[Jakub Narebski]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Add local time (hours and minutes) and local timezone to the output of
>> git_print_authorship command, used by git_commitdiff.
> 
> Looks nice, thanks.
> 
> Now I got envious seeing people are having SO MUCH FUN with
> gitweb, so here is mine...
> 
> Likes, dislikes, "your color selection sucks ;-)",... ?
> 
> -- >8 --

> +td.age-week   { color: #00f; background-color: #fff; }
> +td.age-month  { color: #00f; background-color: #eef; }
> +td.age-season { color: #00f; background-color: #ddf; }
> +td.age-year   { color: #00f; background-color: #ccf; }
> +td.age-old    { color: #00f; background-color: #bbf; }

Could you use full hex color length? Everywhere else in CSS we use 
6-char wide hex colours.

> +     my $now = time();
>       while (<$fd>) {
> -             /^([0-9a-fA-F]{40}).*?(\d+)\)\s{1}(\s*.*)/;
> -             my $full_rev = $1;
> +             my ($full_rev, $author, $timestamp, $zone, $lineno, $data) =
> +                 /^([0-9a-fA-F]{40})\s\((.*?)\s+(\d+)\s
> +                 ([-+\d]{5})\s+(\d+)\)\s{1}(\s*.*)/x;

Nice compact style. But different from other parsing using regexp in gitweb.
And some of those other, e.g. parse_difftree_raw_line cannot use this style.
Doesn't matter much.

>               my $rev = substr($full_rev, 0, 8);
> -             my $lineno = $2;
> -             my $data = $3;
> +
> +             my $age = $now - $timestamp;
> +             my $ago = age_string($age);
> +             my $pop = "$author, $ago";
> +             my $agegroup =
> +                 (($age < 60*60*24*7) ? "age-week" :
> +                  ($age < 60*60*24*30) ? "age-month" :
> +                  ($age < 60*60*24*120) ? "age-season" :
> +                  ($age < 60*60*24*360) ? "age-year" : "age-old");

We have age_class subroutine which does something similar.
I'm not sure if one subroutine can be used for those two situations.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] gitweb: Add local time and timezone to git_print_authorship

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Could you use full hex color length? Everywhere else in CSS we use 
> 6-char wide hex colours.

Will do for consistency's sake, although I do not think we
really care about the extra precision in this application.

> We have age_class subroutine which does something similar.

Yeah, but...

 (1) When somebody says "there are three age classes: age0,
     age1, and age2", can you tell which one represents older
     ones?  I think they are misnamed.

 (2) The classification of age_class seems to be useful for the
     project age, which is to check if anything happened in the
     last two hours and last two days.  Unless the project is
     really dead/dormant it should not be more than a month.  On
     the other hand, a mature and stable project should have
     more than 80% lines that are more than a year old, even if
     it is very active and have many lines that are less than
     two hours old.

     The yardstick age_class uses is really geared toward
     checking project's last activity and not appropriate for
     use in blame.  Luckily, blame2 does not use it.

 (3) I'd like to eventually get rid of the abbreviated commit
     object name from blame output, so the setting in gitweb.css
     for table.blame td.age[012] (different colors and font
     styles) is not appropriate for what I am shooting at.

I have a bit updated 3-series for review.

[PATCH] gitweb: split output routine of blame2

[Junio C Hamano]

Make the loop over blame output to collect lines belonging to the
same revision, and send it over to the output routine "flush_blame_lines".
This will let me do interesting things later.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 gitweb/gitweb.perl |   73 +++++++++++++++++++++++++++++++++++++---------------
 1 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 555441d..28e1cfd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2170,6 +2170,33 @@ sub git_tag {
 	git_footer_html();
 }
 
+sub flush_blame_lines {
+	my ($color, $file_name, @line) = @_;
+	my $full_rev = shift @line;
+	my $rev = substr($full_rev, 0, 8);
+
+	for (@line) {
+		unless (/^[0-9a-fA-F]{40}.*?(\d+)\)\s(.*)/) {
+			print "<tr><td>(malformed blame output)</td></tr>";
+			next;
+		}
+		my $lineno = $1;
+		my $data = $2;
+		print "<tr class=\"$color\">\n";
+		print "<td class=\"sha1\">" .
+		$cgi->a({-href => href(action=>"commit",
+				       hash=>$full_rev,
+				       file_name=>$file_name)},
+			esc_html($rev)) . "</td>\n";
+		print +("<td class=\"linenr\">".
+			"<a id=\"l$lineno\" href=\"#l$lineno\" ".
+			"class=\"linenr\">" .
+			esc_html($lineno) . "</a></td>\n");
+		print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
+		print "</tr>\n";
+	}
+}
+
 sub git_blame2 {
 	my $fd;
 	my $ftype;
@@ -2205,33 +2232,37 @@ sub git_blame2 {
 	my @rev_color = (qw(light2 dark2));
 	my $num_colors = scalar(@rev_color);
 	my $current_color = 0;
-	my $last_rev;
+	my @current_group = ();
 	print <<HTML;
 <div class="page_body">
 <table class="blame">
 <tr><th>Commit</th><th>Line</th><th>Data</th></tr>
 HTML
 	while (<$fd>) {
-		/^([0-9a-fA-F]{40}).*?(\d+)\)\s{1}(\s*.*)/;
-		my $full_rev = $1;
-		my $rev = substr($full_rev, 0, 8);
-		my $lineno = $2;
-		my $data = $3;
-
-		if (!defined $last_rev) {
-			$last_rev = $full_rev;
-		} elsif ($last_rev ne $full_rev) {
-			$last_rev = $full_rev;
-			$current_color = ++$current_color % $num_colors;
-		}
-		print "<tr class=\"$rev_color[$current_color]\">\n";
-		print "<td class=\"sha1\">" .
-			$cgi->a({-href => href(action=>"commit", hash=>$full_rev, file_name=>$file_name)},
-			        esc_html($rev)) . "</td>\n";
-		print "<td class=\"linenr\"><a id=\"l$lineno\" href=\"#l$lineno\" class=\"linenr\">" .
-		      esc_html($lineno) . "</a></td>\n";
-		print "<td class=\"pre\">" . esc_html($data) . "</td>\n";
-		print "</tr>\n";
+		my $full_rev;
+		unless (($full_rev) = /^([0-9a-fA-F]{40})/) {
+			print "<tr><td>(malformed blame output)</td></tr>\n";
+			next;
+		}
+		if (@current_group) {
+			if (($current_group[0] ne $full_rev)) {
+				$current_color = ++$current_color % $num_colors;
+				flush_blame_lines($rev_color[$current_color],
+						  $file_name,
+						  @current_group);
+			}
+			else {
+				push @current_group, $_;
+				next;
+			}
+		}
+		@current_group = ($full_rev, $_);
+	}
+	if (@current_group) {
+		$current_color = ++$current_color % $num_colors;
+		flush_blame_lines($rev_color[$current_color],
+				  $file_name,
+				  @current_group);
 	}
 	print "</table>\n";
 	print "</div>";
-- 
1.4.2.g39ee

[PATCH] gitweb: show rev only on the first line of each group in blame

[Junio C Hamano]

Thanks to the last round, this is now easy to do.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 gitweb/gitweb.perl |   21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 28e1cfd..5c82d3f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2174,8 +2174,11 @@ sub flush_blame_lines {
 	my ($color, $file_name, @line) = @_;
 	my $full_rev = shift @line;
 	my $rev = substr($full_rev, 0, 8);
+	my $cnt = scalar @line;
+	my $this = 0;
 
 	for (@line) {
+		$this++;
 		unless (/^[0-9a-fA-F]{40}.*?(\d+)\)\s(.*)/) {
 			print "<tr><td>(malformed blame output)</td></tr>";
 			next;
@@ -2183,11 +2186,19 @@ sub flush_blame_lines {
 		my $lineno = $1;
 		my $data = $2;
 		print "<tr class=\"$color\">\n";
-		print "<td class=\"sha1\">" .
-		$cgi->a({-href => href(action=>"commit",
-				       hash=>$full_rev,
-				       file_name=>$file_name)},
-			esc_html($rev)) . "</td>\n";
+
+		if ($this == 1) {
+			if (1 < $cnt) {
+				print "<td class=\"sha1\" rowspan=\"$cnt\">";
+			}
+			else {
+				print "<td class=\"sha1\">";
+			}
+			print $cgi->a({-href => href(action=>"commit",
+						     hash=>$full_rev,
+						     file_name=>$file_name)},
+				      esc_html($rev)) . "</td>\n";
+		}
 		print +("<td class=\"linenr\">".
 			"<a id=\"l$lineno\" href=\"#l$lineno\" ".
 			"class=\"linenr\">" .
-- 
1.4.2.g39ee

Re: [PATCH] gitweb: Add local time and timezone to git_print_authorship

[Jakub Narebski]

Junio C Hamano wrote:

>  (3) I'd like to eventually get rid of the abbreviated commit
>      object name from blame output, so the setting in gitweb.css
>      for table.blame td.age[012] (different colors and font
>      styles) is not appropriate for what I am shooting at.

What do you want to replace it with? Link can be to "commit" or "commitdiff"
view, but some marker for commit (perhaps 'git-name-rev --tags'?) is
needed.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] gitweb: Add local time and timezone to git_print_authorship

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano wrote:
>
>>  (3) I'd like to eventually get rid of the abbreviated commit
>>      object name from blame output, so the setting in gitweb.css
>>      for table.blame td.age[012] (different colors and font
>>      styles) is not appropriate for what I am shooting at.
>
> What do you want to replace it with? Link can be to "commit" or "commitdiff"
> view, but some marker for commit (perhaps 'git-name-rev --tags'?) is
> needed.

I was hoping I can get away with a narrow single column of solid
color that says nothing.  8 hexadecimal digits do not mean much
to humans and it is taking 8-column or so of screen real estate
that could otherwise be used to show the source lines instead.

I've tried doing that (just set $rev to a single space, and make
the "Commit" column narrower in sub blame2 {}).  One drawback is
that while 8 hexadecimal digits do not mean anything they do
help to match lines that came from the same rev (i.e. "I do not
know what this 8fad7343 mean but this group of lines and that
group are tagged together with that same 8fad7343 so they must
come from the same revision").

Re: [PATCH] gitweb: Add local time and timezone to git_print_authorship

[Jakub Narebski]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Junio C Hamano wrote:
>>
>>>  (3) I'd like to eventually get rid of the abbreviated commit
>>>      object name from blame output, so the setting in gitweb.css
>>>      for table.blame td.age[012] (different colors and font
>>>      styles) is not appropriate for what I am shooting at.
>>
>> What do you want to replace it with? Link can be to "commit" 
>> or "commitdiff" view, but some marker for commit 
>> (perhaps 'git-name-rev --tags'?) is needed.
> 
> I was hoping I can get away with a narrow single column of solid
> color that says nothing.  8 hexadecimal digits do not mean much
> to humans and it is taking 8-column or so of screen real estate
> that could otherwise be used to show the source lines instead.

I don't think that is much an issue. Source code is usually (read:
should be) 80-columns wide, and with default font size there is
certainly place for 8-column revision number. Anything more, and
much more (like e.g. default git_blame a.k.a. git_annotate output)
and it is less readable.

> I've tried doing that (just set $rev to a single space, and make
> the "Commit" column narrower in sub blame2 {}).  One drawback is
> that while 8 hexadecimal digits do not mean anything they do
> help to match lines that came from the same rev (i.e. "I do not
> know what this 8fad7343 mean but this group of lines and that
> group are tagged together with that same 8fad7343 so they must
> come from the same revision").

So what you need is to solve graph coloring problem (which is not
map coloring, as the same revisions needs the same color) for 
revisions ;-) 

I have an idea to use first character of commit hash together with
dark/light (odd/even) class to color blocks of lines in the same 
revisions, 16 colors (we could reduce it to e.g. 6 or 8 colors)
with darker/lighter version.

The problem is to reduce hash to 16 or 8 bits with as small number of
collisions as possible (in average).


Yet another blame improvement idea would be to "highlight" whole _block_
on hover (on mouseover), but it needs changing blame output format from
table to divs (like "blob" view). BTW. table view should be I think
reserved for tabular data, i.e. when sorting by at least some of columns
have sense.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git