threads / patch / 5421

patch, 5 partsgitweb: Add diff tree, with links to patches, to commitdiff view

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

## tl;dr

16 messages between Aug 28, 2006 and Aug 30, 2006. Diffs are folded; open one to read it.

replies: 15people: 3as markdown or json

Jakub Narebski· Aug 28, 2006, 12:48 UTC · lore

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

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
Jakub Narebski· Aug 28, 2006, 12:48 UTC · re: Jakub Narebski · lore

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

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(-)
Show changes to gitweb/gitweb.perl +31 −32
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
Jakub Narebski· Aug 28, 2006, 12:48 UTC · re: Jakub Narebski · lore

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

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(-)
Show changes to gitweb/gitweb.perl +0 −1
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
Jakub Narebski· Aug 28, 2006, 12:48 UTC · re: Jakub Narebski · lore

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

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(-)
Show changes to 2 files +18 −0

gitweb/gitweb.css, gitweb/gitweb.perl

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
Jakub Narebski· Aug 28, 2006, 12:48 UTC · re: Jakub Narebski · lore

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

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(-)
Show changes to gitweb/gitweb.perl +1 −0
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
Jakub Narebski· Aug 28, 2006, 12:48 UTC · re: Jakub Narebski · lore

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(-)
Show changes to gitweb/gitweb.perl +49 −22
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
Linus Torvalds· Aug 28, 2006, 17:26 UTC · re: Jakub Narebski · lore

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

On Mon, 28 Aug 2006, Jakub Narebski wrote:
Show 7 quoted lines
>
> 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
Jakub Narebski· Aug 28, 2006, 21:17 UTC · re: Linus Torvalds · lore

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

Linus Torvalds wrote:
Show 12 quoted lines
> 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(-)
Show changes to gitweb/gitweb.perl +10 −2
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
Junio C Hamano· Aug 29, 2006, 00:16 UTC · re: Jakub Narebski · lore

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

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.
---
Show changes to 2 files +22 −6

gitweb/gitweb.css, gitweb/gitweb.perl

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\">" .
Jakub Narebski· Aug 29, 2006, 08:23 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 13 quoted lines
> 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 --
Show 5 quoted lines
> +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.

Show 7 quoted lines
> +     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.

Show 12 quoted lines
>               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
Junio C Hamano· Aug 29, 2006, 09:05 UTC · re: Jakub Narebski · lore

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

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.
Jakub Narebski· Aug 29, 2006, 10:15 UTC · re: Junio C Hamano · lore

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

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
Junio C Hamano· Aug 30, 2006, 04:26 UTC · re: Jakub Narebski · lore

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

Jakub Narebski <jnareb@gmail.com> writes:
Show 10 quoted lines
> 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").

Jakub Narebski· Aug 30, 2006, 09:47 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 17 quoted lines
> 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.

Show 7 quoted lines
> 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
Junio C Hamano· Aug 29, 2006, 09:06 UTC · re: Junio C Hamano · lore

[PATCH] gitweb: split output routine of blame2

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(-)
Show changes to gitweb/gitweb.perl +52 −21
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
Junio C Hamano· Aug 29, 2006, 09:06 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to gitweb/gitweb.perl +16 −5
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

← back to recent threads