[RFC/PATCH] gitweb: Add committags support

[RFC/PATCH] gitweb: Add committags support

[Jakub Narebski]

Below there is preliminary (hence RFC) committag support for gitweb,
based on the idea introduced by Sham Chukoury to gitweb-xmms2.

The code has all the possible committags I could think of enabled;
not all are tested, though. This includes existing commitsha tag
support (full sha links to commit view, is sha is sha of commit),
mantis bug and feature tags from gitweb-xmms2 (there is no release
committag of gitweb-xmms2, but it should be fairly easy to add it),
bugzilla committag for the Linux kernel, plain text URL committag
(probably doesn't work that well, marking as links examples, and
sometimes protocol specification), and Message-Id committag via
GMane git mailing list (and not only) archive -- not tested.

Comments? Discussion?

This patch is rather not for inclusion; it is not in format-patch form.

P.S. I've corrected git_get_type for comparison
  git_get_type($hash) eq "commit"
to work without Perl syntax errors.

-- >8 --

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7ed963c..5eb0dd0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -173,6 +173,118 @@ sub feature_pickaxe {
 	return ($_[0]);
 }
 
+# You define site-wide comittags defaults here; override them with
+# $GITWEB_CONFIG as necessary.
+our %committags = (
+	# 'committag' => {
+	# 	'pattern' => regexp (use 'qr' quote-like operator)
+	# 	'sub' => committag-sub (subroutine),
+	# 	'enabled' => is given committag enabled,
+	# fields below can be defined, but don't need to
+	# 	'options' => [ default options...] (array reference),
+	# 	'insubject' => should given committag be enabled in commit/tag subject,
+	# 	'islink' => if the result is hyperlink,
+	# }
+	#
+	# You should ensure that enabled committags cannot overlap
+	#
+	# The committag subroutine is called with match for pattern,
+	# and options if they are defined. Match is replaced by return
+	# value of committag-sub.
+
+	'commitsha' => {
+		'pattern' => qr/[0-9a-fA-F]{40}/,
+		'enabled' => 1,
+		'islink'  => 1,
+		'sub' => \&tag_commit_id},
+
+	'mantis' => {
+		'pattern' => qr/(BUG|FEATURE)\(\d+\)/,
+		'enabled' => 1,
+		'insubject' => 1,
+		'islink' => 1,
+		'options' => [ 'http://bugs.xmms2.xmms.se/view.php?id=' ],
+		'sub' => \&tag_bugtracker},
+
+	'bugzilla' => {
+		'pattern' => qr/bug( )+\(\d+\)/,
+		'enabled' => 1,
+		'insubject' => 1,
+		'islink' => 1,
+		'options' => [ 'http://bugzilla.kernel.org/show_bug.cgi?id=' ],
+		'sub' => \&tag_bugtracker},
+
+	'URL' => {
+		'pattern' => qr!(http|ftp)s?://[a-zA-Z0-9_%./]+!,
+		'enabled' => 1,
+		'islink' => 1,
+		'sub' => \&tag_url},
+
+	'message_id' => {
+		'pattern' => qr/(message|msg)[- ]?id <([^&]*)&rt;/i,
+		'enabled' => 1,
+		'options' => [
+			'http://news.gmane.org/find-root.php?message_id=',
+			\&quote_msgid_gmane ],
+		'sub' => \&tag_msgid},
+);
+
+sub tag_commit_id {
+	my $hash_text = shift;
+
+	if (git_get_type($hash_text) eq "commit") {
+		return $cgi->a({-href => href(action=>"commit", hash=>$hash_text),
+		               -class => "text"}, $hash_text);
+	}
+
+	return;
+}
+
+sub tag_bugtracker {
+	my $match = shift;
+	my $URL = shift || return $match;
+	my ($issue) = $match =~ m/(\d+)/;
+
+	return $match if (!defined $issue);
+
+	return $cgi->a({-href => "$URL$issue"}, $match);
+}
+
+sub tag_url {
+	my $url_text = shift;
+
+	return $cgi->a({-href => $url_text}, $url_text);
+}
+
+sub quote_msgid_gmane {
+	my $msgid = shift || return;
+
+	return '<'.(quotemeta $msgid).'>';
+}
+
+sub quote_msgid_marc {
+	my $msgid = shift || return;
+	my ($user, $host) = split(/\@/, $msgid, 2);
+	$host =~ s/\./ ! /g;
+
+	return $user.' () '.$host;
+}
+
+
+sub tag_msgid {
+	my $text = shift;
+	my $URL = shift || return $text;
+	my $repl = shift;
+
+	my ($msgid) =~ m/&lt;([^&]*)&rt;/;
+	my $msgid_url = (ref($repl) eq "CODE") ? $repl->($msgid) : $msgid;
+	my $link = $cgi->a({-href => "$URL$msgid_url"}, $msgid);
+
+	$text =~ s/$msgid/$link/;
+
+	return $text;
+}
+
 # rename detection options for git-diff and git-diff-tree
 # - default is '-M', with the cost proportional to
 #   (number of removed files) * (number of new files).
@@ -191,6 +303,10 @@ our $git_version = qx($GIT --version) =~
 
 $projects_list ||= $projectroot;
 
+# enabled committags, and committags enabled for subject
+our @committags  = grep { $committags{$_}{'enabled'} } keys %committags;
+our @subjecttags = grep { $committags{$_}{'insubject'} } @committags;
+
 # ======================================================================
 # input validation and dispatch
 our $action = $cgi->param('a');
@@ -577,18 +693,43 @@ ## which don't beling to other sections
 # format line of commit message or tag comment
 sub format_log_line_html {
 	my $line = shift;
+	my @tags = @_;
+	my $a_attr;
+	my %subst;
+
+	if (!@tags) {
+		@tags = @committags;
+	} else {
+		$a_attr = ref($tags[0]) eq "HASH" ? shift @tags : undef;
+	}
 
 	$line = esc_html($line);
 	$line =~ s/ /&nbsp;/g;
-	if ($line =~ m/([0-9a-fA-F]{40})/) {
-		my $hash_text = $1;
-		if (git_get_type($hash_text) eq "commit") {
-			my $link =
-				$cgi->a({-href => href(action=>"commit", hash=>$hash_text),
-				        -class => "text"}, $hash_text);
-			$line =~ s/$hash_text/$link/;
+
+	foreach my $ct (@tags) {
+		next unless exists $committags{$ct};
+		my $wrap = $a_attr && %$a_attr && $committags{$ct}{'islink'};
+		my @opts =
+			exists $committags{$ct}{'options'} ?
+			@{$committags{$ct}{'options'}} :
+			();
+
+		while ($line =~ m/($committags{$ct}{'pattern'})/gc) {
+			my $match = $1;
+			my $repl = $committags{$ct}{'sub'}->($match, @opts);
+			next unless $repl;
+
+			if ($wrap) {
+				$repl = $cgi->end_a() . $repl . $cgi->start_a($a_attr);
+			}
+
+			$subst{quotemeta $match} = $repl;
 		}
 	}
+
+	while (my ($from, $to) = each %subst) {
+		$line =~ s/$from/$to/g;
+	}
 	return $line;
 }
 
@@ -626,12 +767,13 @@ sub format_subject_html {
 	$extra = '' unless defined($extra);
 
 	if (length($short) < length($long)) {
-		return $cgi->a({-href => $href, -class => "list subject",
-		                -title => $long},
-		       esc_html($short) . $extra);
+		my $a_attr = {-href => $href, -class => "list subject", -title => $long};
+		return $cgi->a($a_attr,
+		       format_log_line_html($short, $a_attr, @subjecttags) . $extra);
 	} else {
-		return $cgi->a({-href => $href, -class => "list subject"},
-		       esc_html($long)  . $extra);
+		my $a_attr = {-href => $href, -class => "list subject"};
+		return $cgi->a($a_attr,
+		       format_log_line_html($long,  $a_attr, @subjecttags) . $extra);
 	}
 }
 
@@ -693,9 +835,9 @@ sub git_get_type {
 
 	open my $fd, "-|", git_cmd(), "cat-file", '-t', $hash or return;
 	my $type = <$fd>;
-	close $fd or return;
+	close $fd or return "unknown";
 	chomp $type;
-	return $type;
+	return ($type || "unknown");
 }
 
 sub git_get_project_config {
@@ -1585,7 +1727,7 @@ sub git_print_log ($;%) {
 			$empty = 0;
 		}
 
-		print format_log_line_html($line) . "<br/>\n";
+		print format_log_line_html($line, @committags) . "<br/>\n";
 	}
 
 	if ($opts{'-final_empty_line'}) {

Re: [RFC/PATCH] gitweb: Add committags support

[Jakub Narebski]

Jakub Narebski wrote:

> Below there is preliminary (hence RFC) committag support for gitweb,
> based on the idea introduced by Sham Chukoury to gitweb-xmms2.
> 
> The code has all the possible committags I could think of enabled;
> not all are tested, though. This includes existing commitsha tag
> support (full sha links to commit view, is sha is sha of commit),
> mantis bug and feature tags from gitweb-xmms2 (there is no release
> committag of gitweb-xmms2, but it should be fairly easy to add it),
> bugzilla committag for the Linux kernel, plain text URL committag
> (probably doesn't work that well, marking as links examples, and
> sometimes protocol specification), and Message-Id committag via
> GMane git mailing list (and not only) archive -- not tested.
> 
> Comments? Discussion?

For example: is the committags code overengineered, too complex?
Or is it not generic enough? Should all committags code be put into
format_log_line_html, or should we add another subroutine, 
format_committags (or similar)?

What other committags would you like to have?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [RFC/PATCH] gitweb: Add committags support

[Petr Baudis]

Dear diary, on Thu, Sep 21, 2006 at 11:56:31PM CEST, I got a letter
where Jakub Narebski <jnareb@gmail.com> said that...
> Below there is preliminary (hence RFC) committag support for gitweb,
> based on the idea introduced by Sham Chukoury to gitweb-xmms2.
> 
> The code has all the possible committags I could think of enabled;
> not all are tested, though. This includes existing commitsha tag
> support (full sha links to commit view, is sha is sha of commit),
> mantis bug and feature tags from gitweb-xmms2 (there is no release
> committag of gitweb-xmms2, but it should be fairly easy to add it),
> bugzilla committag for the Linux kernel, plain text URL committag
> (probably doesn't work that well, marking as links examples, and
> sometimes protocol specification), and Message-Id committag via
> GMane git mailing list (and not only) archive -- not tested.

I think that's a good test. People will certainly want more but that can
be added over time.

> -- >8 --
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 7ed963c..5eb0dd0 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -173,6 +173,118 @@ sub feature_pickaxe {
>  	return ($_[0]);
>  }
>  
> +# You define site-wide comittags defaults here; override them with
> +# $GITWEB_CONFIG as necessary.
> +our %committags = (
> +	# 'committag' => {
> +	# 	'pattern' => regexp (use 'qr' quote-like operator)
> +	# 	'sub' => committag-sub (subroutine),
> +	# 	'enabled' => is given committag enabled,
> +	# fields below can be defined, but don't need to
> +	# 	'options' => [ default options...] (array reference),
> +	# 	'insubject' => should given committag be enabled in commit/tag subject,
> +	# 	'islink' => if the result is hyperlink,
> +	# }
> +	#
> +	# You should ensure that enabled committags cannot overlap
> +	#
> +	# The committag subroutine is called with match for pattern,
> +	# and options if they are defined. Match is replaced by return
> +	# value of committag-sub.

You should note that the pattern matches in an already HTML-escaped
text. Which is actually quite ugly especially wrt. the &nbsp;, perhaps
it would be worth considering converting the spaces after this? (The
Marc links would need fixing tho'.)

..snip..
> +	'message_id' => {
> +		'pattern' => qr/(message|msg)[- ]?id&nbsp;&lt;([^&]*)&rt;/i,
> +		'enabled' => 1,
> +		'options' => [
> +			'http://news.gmane.org/find-root.php?message_id=',
> +			\&quote_msgid_gmane ],
> +		'sub' => \&tag_msgid},

Mention quote_msgid_marc() (and the URL) in a comment so that it's not
completely unreferenced?

> +);
..snip..
> +sub quote_msgid_gmane {
> +	my $msgid = shift || return;
> +
> +	return '<'.(quotemeta $msgid).'>';
> +}

<> should be HTML-escaped (unless CGI::a() does that).

> @@ -577,18 +693,43 @@ ## which don't beling to other sections
..snip..
> -	if ($line =~ m/([0-9a-fA-F]{40})/) {
> -		my $hash_text = $1;
> -		if (git_get_type($hash_text) eq "commit") {
> -			my $link =
> -				$cgi->a({-href => href(action=>"commit", hash=>$hash_text),
> -				        -class => "text"}, $hash_text);
> -			$line =~ s/$hash_text/$link/;
> +
> +	foreach my $ct (@tags) {
> +		next unless exists $committags{$ct};

At this point, I'd complain (even die) rather than silently pass, that's
definitely a bug.

> +		my $wrap = $a_attr && %$a_attr && $committags{$ct}{'islink'};

$a_attr can't be but a hashref, that test is redundant.

> @@ -626,12 +767,13 @@ sub format_subject_html {
>  	$extra = '' unless defined($extra);
>  
>  	if (length($short) < length($long)) {
> -		return $cgi->a({-href => $href, -class => "list subject",
> -		                -title => $long},
> -		       esc_html($short) . $extra);
> +		my $a_attr = {-href => $href, -class => "list subject", -title => $long};
> +		return $cgi->a($a_attr,
> +		       format_log_line_html($short, $a_attr, @subjecttags) . $extra);
>  	} else {
> -		return $cgi->a({-href => $href, -class => "list subject"},
> -		       esc_html($long)  . $extra);
> +		my $a_attr = {-href => $href, -class => "list subject"};
> +		return $cgi->a($a_attr,
> +		       format_log_line_html($long,  $a_attr, @subjecttags) . $extra);
>  	}
>  }
>  

Subjects are often clickable and we don't want links in those.

> @@ -693,9 +835,9 @@ sub git_get_type {
>  
>  	open my $fd, "-|", git_cmd(), "cat-file", '-t', $hash or return;
>  	my $type = <$fd>;
> -	close $fd or return;
> +	close $fd or return "unknown";
>  	chomp $type;
> -	return $type;
> +	return ($type || "unknown");
>  }
>  
>  sub git_get_project_config {

> P.S. I've corrected git_get_type for comparison
>   git_get_type($hash) eq "commit"
> to work without Perl syntax errors.

Hey it's just a warning. ;-) And if you are calling git_get_type() on a
non-existing object, don't you have a problem you should better know
about? Couldn't this break git_get_refs_list()?

> @@ -1585,7 +1727,7 @@ sub git_print_log ($;%) {
>  			$empty = 0;
>  		}
>  
> -		print format_log_line_html($line) . "<br/>\n";
> +		print format_log_line_html($line, @committags) . "<br/>\n";
>  	}
>  
>  	if ($opts{'-final_empty_line'}) {

What about the tags? Or perhaps even blobs, for that matter?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

Re: [RFC/PATCH] gitweb: Add committags support

[Jakub Narebski]

Petr "Pasky" Baudis wrote:
> Dear diary, on Thu, Sep 21, 2006 at 11:56:31PM CEST, I got a letter
> where Jakub Narebski <jnareb@gmail.com> said that...

> > +	# The committag subroutine is called with match for pattern,
> > +	# and options if they are defined. Match is replaced by return
> > +	# value of committag-sub.
> 
> You should note that the pattern matches in an already HTML-escaped
> text. Which is actually quite ugly especially wrt. the &nbsp;, perhaps
> it would be worth considering converting the spaces after this? (The
> Marc links would need fixing tho'.)

Perhaps we better use "white-space: pre;" for commit (and tag) messages
instead of 'manually' doing this via converting ' ' to '&nbsp;'.
 
> ..snip..
> > +	'message_id' => {
> > +		'pattern' => qr/(message|msg)[- ]?id&nbsp;&lt;([^&]*)&rt;/i,
> > +		'enabled' => 1,
> > +		'options' => [
> > +			'http://news.gmane.org/find-root.php?message_id=',
> > +			\&quote_msgid_gmane ],
> > +		'sub' => \&tag_msgid},
> 
> Mention quote_msgid_marc() (and the URL) in a comment so that it's not
> completely unreferenced?

Well, that is RFC. I'm not sure if the final patch would have all those
committags, all those committags but disabled with exception of commitsha/sha,
or just commitsha tag in source and the rest in commit message as examples.

For MARC:

	'options' => [
		'http://marc.theaimsgroup.com/?i=',
		\&quote_msgid_marc ],

> > +);
> ..snip..
> > +sub quote_msgid_gmane {
> > +	my $msgid = shift || return;
> > +
> > +	return '<'.(quotemeta $msgid).'>';
> > +}
> 
> <> should be HTML-escaped (unless CGI::a() does that).

CGI::a() probably does that. But true, "<" and ">" should be "params quoted":

	return '%3c' . (quotemeta $msgid) . '%3e';

(Quotemeta is because Message-Id contains '@').

> > @@ -577,18 +693,43 @@ ## which don't beling to other sections
> ..snip..
> > -	if ($line =~ m/([0-9a-fA-F]{40})/) {
> > -		my $hash_text = $1;
> > -		if (git_get_type($hash_text) eq "commit") {
> > -			my $link =
> > -				$cgi->a({-href => href(action=>"commit", hash=>$hash_text),
> > -				        -class => "text"}, $hash_text);
> > -			$line =~ s/$hash_text/$link/;
> > +
> > +	foreach my $ct (@tags) {
> > +		next unless exists $committags{$ct};
> 
> At this point, I'd complain (even die) rather than silently pass,
> that's definitely a bug.

I've programmed defensively. Perhaps too defensively.

> > +		my $wrap = $a_attr && %$a_attr && $committags{$ct}{'islink'};
> 
> $a_attr can't be but a hashref, that test is redundant.

$a_attr can be undefined or be a hashref. It could theoretically
be empty hashref, but that is a mistake (<a>...</a>).

> > @@ -626,12 +767,13 @@ sub format_subject_html {
> >  	$extra = '' unless defined($extra);
> >  
> >  	if (length($short) < length($long)) {
> > -		return $cgi->a({-href => $href, -class => "list subject",
> > -		                -title => $long},
> > -		       esc_html($short) . $extra);
> > +		my $a_attr = {-href => $href, -class => "list subject", -title => $long};
> > +		return $cgi->a($a_attr,
> > +		       format_log_line_html($short, $a_attr, @subjecttags) . $extra);
> >  	} else {
> > -		return $cgi->a({-href => $href, -class => "list subject"},
> > -		       esc_html($long)  . $extra);
> > +		my $a_attr = {-href => $href, -class => "list subject"};
> > +		return $cgi->a($a_attr,
> > +		       format_log_line_html($long,  $a_attr, @subjecttags) . $extra);
> >  	}
> >  }
> >  
> 
> Subjects are often clickable and we don't want links in those.

The extra code with $a_attr is to have links within links. It works
quite well, I'd say. The subject link is broken, and the committag
link is inserted in the break (gitweb-xmms2 committag code did the same,
but did not preserved all the subject link attributes, like title or class,
only the target of the link).

The result is somethink like:

  <a href="..." class="subject" ...>Fix </a><a href="...=137">BUG(137)</a><a href="..." class="subject" ...>: ...</a> 

> > @@ -693,9 +835,9 @@ sub git_get_type {
> >  
> >  	open my $fd, "-|", git_cmd(), "cat-file", '-t', $hash or return;
> >  	my $type = <$fd>;
> > -	close $fd or return;
> > +	close $fd or return "unknown";
> >  	chomp $type;
> > -	return $type;
> > +	return ($type || "unknown");
> >  }
> >  
> >  sub git_get_project_config {
> 
> > P.S. I've corrected git_get_type for comparison
> >   git_get_type($hash) eq "commit"
> > to work without Perl syntax errors.
> 
> Hey it's just a warning. ;-) And if you are calling git_get_type() on a
> non-existing object, don't you have a problem you should better know
> about? Couldn't this break git_get_refs_list()?

There are many cherry-picked comments with commitsha of object which
existed on some topic branch, was pruned, and does not exist anymore.
Perhaps we should add "2> /dev/null", too...

I've not tested how this change works with git_get_refs_list(), and other
subroutines that use git_get_type().

> > @@ -1585,7 +1727,7 @@ sub git_print_log ($;%) {
> >  			$empty = 0;
> >  		}
> >  
> > -		print format_log_line_html($line) . "<br/>\n";
> > +		print format_log_line_html($line, @committags) . "<br/>\n";
> >  	}
> >  
> >  	if ($opts{'-final_empty_line'}) {
> 
> What about the tags? Or perhaps even blobs, for that matter?

What about? In commit messages you usually reference other commits
(as: this correct some commit, this finishes what was started in commit,
this reverts commit (!), cherry-picked commit).

Thanks for all the comments.
-- 
Jakub Narebski
Poland

Re: [RFC/PATCH] gitweb: Add committags support

[Petr Baudis]

Dear diary, on Sat, Sep 23, 2006 at 10:34:49AM CEST, I got a letter
where Jakub Narebski <jnareb@gmail.com> said that...
> Perhaps we better use "white-space: pre;" for commit (and tag) messages
> instead of 'manually' doing this via converting ' ' to '&nbsp;'.

+1

> > ..snip..
> > > +	'message_id' => {
> > > +		'pattern' => qr/(message|msg)[- ]?id&nbsp;&lt;([^&]*)&rt;/i,
> > > +		'enabled' => 1,
> > > +		'options' => [
> > > +			'http://news.gmane.org/find-root.php?message_id=',
> > > +			\&quote_msgid_gmane ],
> > > +		'sub' => \&tag_msgid},
> > 
> > Mention quote_msgid_marc() (and the URL) in a comment so that it's not
> > completely unreferenced?
> 
> Well, that is RFC. I'm not sure if the final patch would have all those
> committags, all those committags but disabled with exception of commitsha/sha,
> or just commitsha tag in source and the rest in commit message as examples.

I'd disable mantis and bugzilla by default (but leave them in the code
commented out) since those are totally project-specific, but a lot of
OSS projects use gmane and URL and commitsha are universally beneficial.

Also, there is a fundamental limitation for the multi-word patterns that
they won't work if the line wraps at that point in the log message. This
will likely be a problem especially for the msgids, because those are
very long and are very likely to cause a linewrap immediately before.

By the way, I don't think taking just 40-digits sha1s is very useful,
since that's insanely long and besides the linewrap issue, a lot of
people just shorten that to some 8 to 12 digits now - I'd use {8-40}
instead (the enforced minimum is 4 in the Git autocompletion code but we
shouldn't encourage people to write so ambiguous sha1s to persistent
records).

> > > +);
> > ..snip..
> > > +sub quote_msgid_gmane {
> > > +	my $msgid = shift || return;
> > > +
> > > +	return '<'.(quotemeta $msgid).'>';
> > > +}
> > 
> > <> should be HTML-escaped (unless CGI::a() does that).
> 
> CGI::a() probably does that. But true, "<" and ">" should be "params quoted":
> 
> 	return '%3c' . (quotemeta $msgid) . '%3e';

Ah, sillly me, I didn't notice that it goes into the URL.

> (Quotemeta is because Message-Id contains '@').

Hmm, and at which point would that be eaten?

> > > @@ -577,18 +693,43 @@ ## which don't beling to other sections
> > > +		my $wrap = $a_attr && %$a_attr && $committags{$ct}{'islink'};
> > 
> > $a_attr can't be but a hashref, that test is redundant.
> 
> $a_attr can be undefined or be a hashref. It could theoretically
> be empty hashref, but that is a mistake (<a>...</a>).

Yes so no point in testing the emptiness. But this is just nitpicking.

> > > @@ -626,12 +767,13 @@ sub format_subject_html {
> > >  	$extra = '' unless defined($extra);
> > >  
> > >  	if (length($short) < length($long)) {
> > > -		return $cgi->a({-href => $href, -class => "list subject",
> > > -		                -title => $long},
> > > -		       esc_html($short) . $extra);
> > > +		my $a_attr = {-href => $href, -class => "list subject", -title => $long};
> > > +		return $cgi->a($a_attr,
> > > +		       format_log_line_html($short, $a_attr, @subjecttags) . $extra);
> > >  	} else {
> > > -		return $cgi->a({-href => $href, -class => "list subject"},
> > > -		       esc_html($long)  . $extra);
> > > +		my $a_attr = {-href => $href, -class => "list subject"};
> > > +		return $cgi->a($a_attr,
> > > +		       format_log_line_html($long,  $a_attr, @subjecttags) . $extra);
> > >  	}
> > >  }
> > >  
> > 
> > Subjects are often clickable and we don't want links in those.
> 
> The extra code with $a_attr is to have links within links. It works
> quite well, I'd say. The subject link is broken, and the committag
> link is inserted in the break (gitweb-xmms2 committag code did the same,
> but did not preserved all the subject link attributes, like title or class,
> only the target of the link).
> 
> The result is somethink like:
> 
>   <a href="..." class="subject" ...>Fix </a><a href="...=137">BUG(137)</a><a href="..." class="subject" ...>: ...</a> 

I don't think this is good idea though - if I'm clicking at links, I
don't want to have to carefully watch where that bit of the link leads.
IMHO this would be just annoying.

> > > @@ -693,9 +835,9 @@ sub git_get_type {
> > >  
> > >  	open my $fd, "-|", git_cmd(), "cat-file", '-t', $hash or return;
> > >  	my $type = <$fd>;
> > > -	close $fd or return;
> > > +	close $fd or return "unknown";
> > >  	chomp $type;
> > > -	return $type;
> > > +	return ($type || "unknown");
> > >  }
> > >  
> > >  sub git_get_project_config {
> > 
> > > P.S. I've corrected git_get_type for comparison
> > >   git_get_type($hash) eq "commit"
> > > to work without Perl syntax errors.
> > 
> > Hey it's just a warning. ;-) And if you are calling git_get_type() on a
> > non-existing object, don't you have a problem you should better know
> > about? Couldn't this break git_get_refs_list()?
> 
> There are many cherry-picked comments with commitsha of object which
> existed on some topic branch, was pruned, and does not exist anymore.

Then I think it's better to explicitly state that "in this case, this
object may not exist" by checking the return value for undef first.

> Perhaps we should add "2> /dev/null", too...

Yes that would be sensible.

> > > @@ -1585,7 +1727,7 @@ sub git_print_log ($;%) {
> > >  			$empty = 0;
> > >  		}
> > >  
> > > -		print format_log_line_html($line) . "<br/>\n";
> > > +		print format_log_line_html($line, @committags) . "<br/>\n";
> > >  	}
> > >  
> > >  	if ($opts{'-final_empty_line'}) {
> > 
> > What about the tags? Or perhaps even blobs, for that matter?
> 
> What about? In commit messages you usually reference other commits
> (as: this correct some commit, this finishes what was started in commit,
> this reverts commit (!), cherry-picked commit).

I meant that we should consider substituting the committags in those as
well.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

Re: [RFC/PATCH] gitweb: Add committags support

[Jakub Narebski]

Petr Baudis wrote:

> Dear diary, on Sat, Sep 23, 2006 at 10:34:49AM CEST, I got a letter
> where Jakub Narebski <jnareb@gmail.com> said that...

> Also, there is a fundamental limitation for the multi-word patterns that
> they won't work if the line wraps at that point in the log message. This
> will likely be a problem especially for the msgids, because those are
> very long and are very likely to cause a linewrap immediately before.

We do not wrap log messages in gitweb. So the problem is only when
commit message is wrongly wrapped itself (pre imples nowrap).

>> > > +);
>> > ..snip..
>> > > +sub quote_msgid_gmane {
>> > > +	my $msgid = shift || return;
>> > > +
>> > > +	return '<'.(quotemeta $msgid).'>';
>> > > +}
>> > 
>> > <> should be HTML-escaped (unless CGI::a() does that).
>> 
>> CGI::a() probably does that. But true, "<" and ">" should be "params quoted":
>> 
>> 	return '%3c' . (quotemeta $msgid) . '%3e';
> 
> Ah, silly me, I didn't notice that it goes into the URL.

Should it be "params quoted" or not? What are valid characters
in message-id? (And in which RFC it is defined?)

>> (Quotemeta is because Message-Id contains '@').
> 
> Hmm, and at which point would that be eaten?

In the substitution phase... but perhaps I was to defensive.
s/$from/$to/g where $to can have '@'. 

>> > > @@ -626,12 +767,13 @@ sub format_subject_html {
>> > >  	$extra = '' unless defined($extra);
>> > >  
>> > >  	if (length($short) < length($long)) {
>> > > -		return $cgi->a({-href => $href, -class => "list subject",
>> > > -		                -title => $long},
>> > > -		       esc_html($short) . $extra);
>> > > +		my $a_attr = {-href => $href, -class => "list subject", -title => $long};
>> > > +		return $cgi->a($a_attr,
>> > > +		       format_log_line_html($short, $a_attr, @subjecttags) . $extra);
>> > >  	} else {
>> > > -		return $cgi->a({-href => $href, -class => "list subject"},
>> > > -		       esc_html($long)  . $extra);
>> > > +		my $a_attr = {-href => $href, -class => "list subject"};
>> > > +		return $cgi->a($a_attr,
>> > > +		       format_log_line_html($long,  $a_attr, @subjecttags) . $extra);
>> > >  	}
>> > >  }
>> > >  
>> > 
>> > Subjects are often clickable and we don't want links in those.
>> 
>> The extra code with $a_attr is to have links within links. It works
>> quite well, I'd say. The subject link is broken, and the committag
>> link is inserted in the break (gitweb-xmms2 committag code did the same,
>> but did not preserved all the subject link attributes, like title or class,
>> only the target of the link).
>> 
>> The result is somethink like:
>> 
>>   <a href="..." class="subject" ...>Fix </a><a href="...=137">BUG(137)</a><a href="..." class="subject" ...>: ...</a> 
> 
> I don't think this is good idea though - if I'm clicking at links, I
> don't want to have to carefully watch where that bit of the link leads.
> IMHO this would be just annoying.

The committag links within subject link are clearly visually distinguished:
first they have default link color (blue for not visited, dark red for
visited links), second they are not bold width (as opposed to gitweb-xmms2,
where bold font was due to <b>...</b> element and not CSS styling 
of a.subject).

I have just noticed that somehow git_log ("log" view) doesn't use
format_subject_line (perhaps because it is not shortened) and that
committags are not used there. And that is not only place where
subjecttags (committags in clickable subject line) are not used.

By the way, you can easily disable some committags in subject, either
removing 'insubject' field, or setting it to false.

>> > > @@ -1585,7 +1727,7 @@ sub git_print_log ($;%) {
>> > >  			$empty = 0;
>> > >  		}
>> > >  
>> > > -		print format_log_line_html($line) . "<br/>\n";
>> > > +		print format_log_line_html($line, @committags) . "<br/>\n";
>> > >  	}
>> > >  
>> > >  	if ($opts{'-final_empty_line'}) {
>> > 
>> > What about the tags? Or perhaps even blobs, for that matter?
>> 
>> What about? In commit messages you usually reference other commits
>> (as: this correct some commit, this finishes what was started in commit,
>> this reverts commit (!), cherry-picked commit).
> 
> I meant that we should consider substituting the committags in those as
> well.

Ahh... For tags I guess it is a good idea (especially that I think that
fixes for bugtracker tracked bugs and feature request should be marked by tags,
e.g. b/<bugid>, to be used to link to commit/change/patch from bugtracker. 

By the way, should we use some color for PGP signature block in signed tags?

-- 
Jakub Narebski
ShadeHawk on #git
Torun, Poland

Re: [RFC/PATCH] gitweb: Add committags support

[Petr Baudis]

Dear diary, on Sat, Sep 23, 2006 at 03:33:01PM CEST, I got a letter
where Jakub Narebski <jnareb@gmail.com> said that...
> Petr Baudis wrote:
> 
> > Dear diary, on Sat, Sep 23, 2006 at 10:34:49AM CEST, I got a letter
> > where Jakub Narebski <jnareb@gmail.com> said that...
> 
> > Also, there is a fundamental limitation for the multi-word patterns that
> > they won't work if the line wraps at that point in the log message. This
> > will likely be a problem especially for the msgids, because those are
> > very long and are very likely to cause a linewrap immediately before.
> 
> We do not wrap log messages in gitweb. So the problem is only when
> commit message is wrongly wrapped itself (pre imples nowrap).

The commit message is not "wrongly" wrapped but just wrapped to fit into
72 or whatever columns. It would be silly to mandate users to use msg-id: <200609231533.02455.jnareb@gmail.com>
with the message id stretching far away just for the sake of some
gitweb limitations when having the message wrapped such as msg-id:
<200609231533.02455.jnareb@gmail.com> looks much more reasonable.

> >> > > +);
> >> > ..snip..
> >> > > +sub quote_msgid_gmane {
> >> > > +	my $msgid = shift || return;
> >> > > +
> >> > > +	return '<'.(quotemeta $msgid).'>';
> >> > > +}
> >> > 
> >> > <> should be HTML-escaped (unless CGI::a() does that).
> >> 
> >> CGI::a() probably does that. But true, "<" and ">" should be "params quoted":
> >> 
> >> 	return '%3c' . (quotemeta $msgid) . '%3e';
> > 
> > Ah, silly me, I didn't notice that it goes into the URL.
> 
> Should it be "params quoted" or not? What are valid characters
> in message-id? (And in which RFC it is defined?)

RFC2822. Unfortunately there can be all kind of crap inside if you put
it inside quotes, so yes, it should.

What a pity that (apparently) noone supports RFC2392.

> >> (Quotemeta is because Message-Id contains '@').
> > 
> > Hmm, and at which point would that be eaten?
> 
> In the substitution phase... but perhaps I was to defensive.
> s/$from/$to/g where $to can have '@'. 

$ perl -le 'my $x="a\@bc"; my $y="@"; $x =~ s/$y/$y.$y/g; print $x;'
a@.@bc

You probably should regexp-quote $from if you don't yet, though,
although a misbehaviour from that side is not very probable.

> >> > Subjects are often clickable and we don't want links in those.
> >> 
> >> The extra code with $a_attr is to have links within links. It works
> >> quite well, I'd say. The subject link is broken, and the committag
> >> link is inserted in the break (gitweb-xmms2 committag code did the same,
> >> but did not preserved all the subject link attributes, like title or class,
> >> only the target of the link).
> >> 
> >> The result is somethink like:
> >> 
> >>   <a href="..." class="subject" ...>Fix </a><a href="...=137">BUG(137)</a><a href="..." class="subject" ...>: ...</a> 
> > 
> > I don't think this is good idea though - if I'm clicking at links, I
> > don't want to have to carefully watch where that bit of the link leads.
> > IMHO this would be just annoying.
> 
> The committag links within subject link are clearly visually distinguished:
> first they have default link color (blue for not visited, dark red for
> visited links), second they are not bold width (as opposed to gitweb-xmms2,
> where bold font was due to <b>...</b> element and not CSS styling 
> of a.subject).

Ok, in that case it's better though I'm still feeling somewhat
uncomfortable about it. What if the _whole_ subject is just "Bug 1324"?
I can click the 'commit' link instead but it throws exception in my
brain I need to handle and I need to move my mouse around.

> > I meant that we should consider substituting the committags in those as
> > well.
> 
> Ahh... For tags I guess it is a good idea (especially that I think that
> fixes for bugtracker tracked bugs and feature request should be marked by tags,
> e.g. b/<bugid>, to be used to link to commit/change/patch from bugtracker. 

For blobs, the point is mainly comments, those can contain all kinds of
stuff.

> By the way, should we use some color for PGP signature block in signed tags?

Sounds like a good idea.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

Re: [RFC/PATCH] gitweb: Add committags support

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> Also, there is a fundamental limitation for the multi-word patterns that
> they won't work if the line wraps at that point in the log message. This
> will likely be a problem especially for the msgids, because those are
> very long and are very likely to cause a linewrap immediately before.
>
> By the way, I don't think taking just 40-digits sha1s is very useful,
> since that's insanely long and besides the linewrap issue, a lot of
> people just shorten that to some 8 to 12 digits now - I'd use {8-40}
> instead (the enforced minimum is 4 in the Git autocompletion code but we
> shouldn't encourage people to write so ambiguous sha1s to persistent
> records).

True.  Without looking at Jakub's code under discussion, off the
top of my head, I wonder if we can do something like this:

 (1) take the whole commit log message, not line by line, into a
     Perl variable, say $log.  Note that this is _before_ HTML
     escaping.

 (2) each enabled tag-marking sub are expected to take a list of
     string or ref as its parameter.  The first one in the chain
     takes a single parameter which is ($log).  You call all the
     enabled ones, in some order, and feed the return value from
     the previous one as the input to the next one.

 (3) a tag-marking sub are expected to inspect the list it
     received as its input parameter, and build its return value
     by looking at each element in the list:

	(3-1) if the element is not a string, pass it down as-is
	in the output list.

	(3-2) if the element is a string, do the pattern match,
	and output one or more elements in the output list.
	Literal string you did not care about are placed as
	string for later taggers to be processed, the links you
	generate are placed as a ref to string.

    For example, commit_id tag-marker might look like this:

        sub tag_marker_commit_id {
                my @input = (@_);
                my @output;
                for (@input) {
                        if (!ref $_) {
                                push @output, $_;
                                next;
                        }
                        while ($_ ne '' && m/(.*?)\b([0-9a-fA-F]{6,})\b(.*)/s) {
                                my ($pre, $sha1, $post) = ($1, $2, $3);
                                if (!is_a_commit_id($sha1)) {
                                        push @output, "$pre$sha1";
                                }
                                else {
					my $subst =
                                          href(action => 'commit', hash => $2);
                                        push @output, $1;
                                        push @output, \$subst;
                                }
                                $_ = $post;
                        }
                }
                return @output;
        }

 (4) when you finished calling the chain, you would have a list
     of string and ref.  You html quote the strings and pass
     the ref's (which are supposed to be already HTML) as is,
     and concatenate the result:

	sub markup_all {
        	my ($log) = @_;
                my @current = $log;
                for my $tag_marker (@tag_markers) {
			@current = $tag_marker->(@current);
		}
                return join('',
                	map {
                        	(ref $_) ? $$_ : esc_html($_)
			} @current);
	}

Hmm?

     

Re: [RFC/PATCH] gitweb: Add committags support

[Jakub Narebski]

Junio C Hamano wrote:

> True.  Without looking at Jakub's code under discussion, off the
> top of my head, I wonder if we can do something like this:

The code was to analyse log message line after line, doing loop
over all committags, building list (well, hash to be more exact)
of _exact_ replacements (e.g. in the case of commit_id/commitsha
committag we use exact hash as the replaced part, not generic
regexp to find a sha1).

The idea of doing committag parsing, and preserving replacements
from matching committag and being subject to HTML escaping by using
reference to string has it's merits.

I'd have to think about it.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [RFC/PATCH] gitweb: Add committags support

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> The idea of doing committag parsing, and preserving replacements
> from matching committag and being subject to HTML escaping by using
> reference to string has it's merits.
>
> I'd have to think about it.

Good.  My obviously buggy illustration would be easier to read
with this patch ;-).

--- gomi	2006-09-23 13:47:52.000000000 -0700
+++ gomi	2006-09-23 13:48:23.000000000 -0700
@@ -2,7 +2,7 @@
                 my @input = (@_);
                 my @output;
                 for (@input) {
-                        if (!ref $_) {
+                        if (ref $_) {
                                 push @output, $_;
                                 next;
                         }
@@ -19,6 +19,9 @@
                                 }
                                 $_ = $post;
                         }
+			if ($_ ne '') {
+				push @output, $_;
+			}
                 }
                 return @output;
         }

Re: [RFC/PATCH] gitweb: Add committags support

[Jakub Narebski]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> The idea of doing committag parsing, and preserving replacements
>> from matching committag and being subject to HTML escaping by using
>> reference to string has it's merits.
>>
>> I'd have to think about it.
> 
> Good.  My obviously buggy illustration would be easier to read
> with this patch ;-).

Don't forget about final s,\n,<br/>\n,gm if we parse it as a whole,
and not line by line. 

By the way, I think that only the driver (i.e. format_log_line_html) has to
be changed...
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [RFC/PATCH] gitweb: Add committags support

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Don't forget about final s,\n,<br/>\n,gm if we parse it as a whole,
> and not line by line. 

Yes, the driver and individual tag markers need to have a
convention on who is responsible for doing <br />.
One reasonable convention would be to make the driver side
responsible for taking \n and turns it into <br /> in plain
strings, and make the tag markers responsible for doing so in
their marked-up output.

For example, when bugzilla_tagger is annotating this commit
message:

    This rewrites frotz() routine to work around Bug
    #22574.  The fix was confirmed by Xyzzy.

    Thanks for everybody who helped with this rewrite.

it would match with a regexp /\b(?i:bug)\s*#?(\d+)/s
to pick up "Bug\n#22574", extract 22574, and produce:

    ('This rewrites frotz() routine to work around ',
     \'<a href="/cgi-bin/bugzilla.cgi?id=22574">Bug<br/>#22574</a>',
     '.  The fix was confirmed by Xyzzy.\n\nThanks for everybody...')

Re: [RFC/PATCH] gitweb: Add committags support

[Jakub Narebski]

Jakub Narebski wrote:

> Junio C Hamano wrote:
> 
>> Jakub Narebski <jnareb@gmail.com> writes:
>> 
>>> The idea of doing committag parsing, and preserving replacements
>>> from matching committag and being subject to HTML escaping by using
>>> reference to string has it's merits.
>>>
>>> I'd have to think about it.
>> 
>> Good.  My obviously buggy illustration would be easier to read
>> with this patch ;-).
> 
> Don't forget about final s,\n,<br/>\n,gm if we parse it as a whole,
> and not line by line. 
> 
> By the way, I think that only the driver (i.e. format_log_line_html) has to
> be changed...

Yet another question is how to deal with commit message specific
"syntax highlighting". Currently, parsing commit message line by line,
we treat specially signoff lines (syntax highlighting, and removing
trailing empty lines after signoff), empty lines (we collapse consecutive
empty lines); the rest goes through format_log_line_html... and committags.

Tag messages have another specific syntax highlighting, namely PGP/GPG
signature part, which should be syntax highlighted I think (and not 
subject to committags replacements).


Some of the problems of marking replacements as not to be committags 
searched and not to be HTML escaped can be dealt with by changing the
order of committags. For example having 'URL' tag as first tag to check
we avoid invoking it on committag replacements links.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [RFC/PATCH] gitweb: Add committags support

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Yet another question is how to deal with commit message specific
> "syntax highlighting". Currently, parsing commit message line by line,
> we treat specially signoff lines (syntax highlighting, and removing
> trailing empty lines after signoff), empty lines (we collapse consecutive
> empty lines); the rest goes through format_log_line_html... and committags.

Actually I was hoping that my bright idea(tm) of passing the
whole message to a chain of tag markers would solve that
automatically.  You define a std marker whose purpose is _not_ 
to add anchor elements leading to other URLs but mark up
sign-off lines and stuff.  Put that at the beginning or at the
end of the chain as you see fit and it would do its work just
like other real tag markers would do.

For example, one of the things it would do is to unhighlight
the Signed-off-by: lines.  It would match and split with

	/(.*?)^(Signed-off-by: .*?)$(.*)/ms

then add:

	( $1, # pass as literal we did not touch
          \'<span class="signoff">', # unhighlight
          $2, # give chance to muck with e-mail address to downstream
	      # markers
          \'</span>'
        )

to its output buffer and process the rest ($3).  A downstream
tag maker may match e-mail address the above left and might mark
it up with mailto: URL.


          

	

Re: [RFC/PATCH] gitweb: Add committags support

[Jakub Narebski]

Petr Baudis wrote:

> Dear diary, on Sat, Sep 23, 2006 at 03:33:01PM CEST, I got a letter
> where Jakub Narebski <jnareb@gmail.com> said that...
>> Petr Baudis wrote:
>> 
>> > Dear diary, on Sat, Sep 23, 2006 at 10:34:49AM CEST, I got a letter
>> > where Jakub Narebski <jnareb@gmail.com> said that...
>> 
>> > Also, there is a fundamental limitation for the multi-word patterns that
>> > they won't work if the line wraps at that point in the log message. This
>> > will likely be a problem especially for the msgids, because those are
>> > very long and are very likely to cause a linewrap immediately before.
>> 
>> We do not wrap log messages in gitweb. So the problem is only when
>> commit message is wrongly wrapped itself (pre imples nowrap).
> 
> The commit message is not "wrongly" wrapped but just wrapped to fit into
> 72 or whatever columns. It would be silly to mandate users to use msg-id: <200609231533.02455.jnareb@gmail.com>
> with the message id stretching far away just for the sake of some
> gitweb limitations when having the message wrapped such as msg-id:
> <200609231533.02455.jnareb@gmail.com> looks much more reasonable.

So when putting message if into commit message, just put the commit
message in separate line, perhaps even with some indentation, like
below:
  Msg-Id: <200609231533.02455.jnareb@gmail.com>

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [RFC/PATCH] gitweb: Add committags support

[Petr Baudis]

Dear diary, on Mon, Sep 25, 2006 at 08:06:44PM CEST, I got a letter
where Jakub Narebski <jnareb@gmail.com> said that...
> So when putting message if into commit message, just put the commit
> message in separate line, perhaps even with some indentation, like
> below:
>   Msg-Id: <200609231533.02455.jnareb@gmail.com>

Oh please. :-) The tool should bend, not the user.

Are you going to mandate that for bug references as well? Or should I
proofread all of my commits (if they aren't actually just autoformatted
by fmt without my further review) to verify that they don't actually end
up wrapped, and manually reformat the whole paragraph?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj
$/=unpack('H*',$_);$_=`echo 16dio\U$k"SK$/SM$n\EsN0p[lN*1
lK[d2%Sa2/d0$^Ixp"|dc`;s/\W//g;$_=pack('H*',/((..)*)$/)

Re: [RFC/PATCH] gitweb: Add committags support

[Jakub Narebski]

Petr Baudis wrote:

> Dear diary, on Mon, Sep 25, 2006 at 08:06:44PM CEST, I got a letter
> where Jakub Narebski <jnareb@gmail.com> said that...
>> So when putting message if into commit message, just put the commit
>> message in separate line, perhaps even with some indentation, like
>> below:
>>   Msg-Id: <200609231533.02455.jnareb@gmail.com>
> 
> Oh please. :-) The tool should bend, not the user.

Still it is more (much more) readable to have Msg-Id "header" (or
equivalent), and message id itself in one line.

> Are you going to mandate that for bug references as well? Or should I
> proofread all of my commits (if they aren't actually just autoformatted
> by fmt without my further review) to verify that they don't actually end
> up wrapped, and manually reformat the whole paragraph?

O.K. that convinces me (although wrapped hyperlink is not a pretty thing).
Besides for tags we have natural multiline "tag", namely PGP signature,
which we wpuld (most probably) want to syntax highlight wrapping it in
<div class="GPG_signature">...</div> element.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git