{"thread":{"id":"5642","subject":"[RFC/PATCH] gitweb: Add committags support","startedAt":"2006-09-21T21:56:31Z","lastAt":"2006-09-26T11:52:59Z","messageCount":17,"participants":["Jakub Narebski","Petr Baudis","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"27388","messageId":"200609212356.31806.jnareb@gmail.com","threadId":"5642","inReplyTo":null,"subject":"[RFC/PATCH] gitweb: Add committags support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-21T21:56:31Z","receivedAt":"2006-09-21T21:56:31Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Below there is preliminary (hence RFC) committag support for gitweb,\nbased on the idea introduced by Sham Chukoury to gitweb-xmms2.\n\nThe code has all the possible committags I could think of enabled;\nnot all are tested, though. This includes existing commitsha tag\nsupport (full sha links to commit view, is sha is sha of commit),\nmantis bug and feature tags from gitweb-xmms2 (there is no release\ncommittag of gitweb-xmms2, but it should be fairly easy to add it),\nbugzilla committag for the Linux kernel, plain text URL committag\n(probably doesn't work that well, marking as links examples, and\nsometimes protocol specification), and Message-Id committag via\nGMane git mailing list (and not only) archive -- not tested.\n\nComments? Discussion?\n\nThis patch is rather not for inclusion; it is not in format-patch form.\n\nP.S. I've corrected git_get_type for comparison\n  git_get_type($hash) eq \"commit\"\nto work without Perl syntax errors.\n\n-- >8 --\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 7ed963c..5eb0dd0 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -173,6 +173,118 @@ sub feature_pickaxe {\n \treturn ($_[0]);\n }\n \n+# You define site-wide comittags defaults here; override them with\n+# $GITWEB_CONFIG as necessary.\n+our %committags = (\n+\t# 'committag' => {\n+\t# \t'pattern' => regexp (use 'qr' quote-like operator)\n+\t# \t'sub' => committag-sub (subroutine),\n+\t# \t'enabled' => is given committag enabled,\n+\t# fields below can be defined, but don't need to\n+\t# \t'options' => [ default options...] (array reference),\n+\t# \t'insubject' => should given committag be enabled in commit/tag subject,\n+\t# \t'islink' => if the result is hyperlink,\n+\t# }\n+\t#\n+\t# You should ensure that enabled committags cannot overlap\n+\t#\n+\t# The committag subroutine is called with match for pattern,\n+\t# and options if they are defined. Match is replaced by return\n+\t# value of committag-sub.\n+\n+\t'commitsha' => {\n+\t\t'pattern' => qr/[0-9a-fA-F]{40}/,\n+\t\t'enabled' => 1,\n+\t\t'islink'  => 1,\n+\t\t'sub' => \\&tag_commit_id},\n+\n+\t'mantis' => {\n+\t\t'pattern' => qr/(BUG|FEATURE)\\(\\d+\\)/,\n+\t\t'enabled' => 1,\n+\t\t'insubject' => 1,\n+\t\t'islink' => 1,\n+\t\t'options' => [ 'http://bugs.xmms2.xmms.se/view.php?id=' ],\n+\t\t'sub' => \\&tag_bugtracker},\n+\n+\t'bugzilla' => {\n+\t\t'pattern' => qr/bug(&nbsp;)+\\(\\d+\\)/,\n+\t\t'enabled' => 1,\n+\t\t'insubject' => 1,\n+\t\t'islink' => 1,\n+\t\t'options' => [ 'http://bugzilla.kernel.org/show_bug.cgi?id=' ],\n+\t\t'sub' => \\&tag_bugtracker},\n+\n+\t'URL' => {\n+\t\t'pattern' => qr!(http|ftp)s?://[a-zA-Z0-9_%./]+!,\n+\t\t'enabled' => 1,\n+\t\t'islink' => 1,\n+\t\t'sub' => \\&tag_url},\n+\n+\t'message_id' => {\n+\t\t'pattern' => qr/(message|msg)[- ]?id&nbsp;&lt;([^&]*)&rt;/i,\n+\t\t'enabled' => 1,\n+\t\t'options' => [\n+\t\t\t'http://news.gmane.org/find-root.php?message_id=',\n+\t\t\t\\&quote_msgid_gmane ],\n+\t\t'sub' => \\&tag_msgid},\n+);\n+\n+sub tag_commit_id {\n+\tmy $hash_text = shift;\n+\n+\tif (git_get_type($hash_text) eq \"commit\") {\n+\t\treturn $cgi->a({-href => href(action=>\"commit\", hash=>$hash_text),\n+\t\t               -class => \"text\"}, $hash_text);\n+\t}\n+\n+\treturn;\n+}\n+\n+sub tag_bugtracker {\n+\tmy $match = shift;\n+\tmy $URL = shift || return $match;\n+\tmy ($issue) = $match =~ m/(\\d+)/;\n+\n+\treturn $match if (!defined $issue);\n+\n+\treturn $cgi->a({-href => \"$URL$issue\"}, $match);\n+}\n+\n+sub tag_url {\n+\tmy $url_text = shift;\n+\n+\treturn $cgi->a({-href => $url_text}, $url_text);\n+}\n+\n+sub quote_msgid_gmane {\n+\tmy $msgid = shift || return;\n+\n+\treturn '<'.(quotemeta $msgid).'>';\n+}\n+\n+sub quote_msgid_marc {\n+\tmy $msgid = shift || return;\n+\tmy ($user, $host) = split(/\\@/, $msgid, 2);\n+\t$host =~ s/\\./ ! /g;\n+\n+\treturn $user.' () '.$host;\n+}\n+\n+\n+sub tag_msgid {\n+\tmy $text = shift;\n+\tmy $URL = shift || return $text;\n+\tmy $repl = shift;\n+\n+\tmy ($msgid) =~ m/&lt;([^&]*)&rt;/;\n+\tmy $msgid_url = (ref($repl) eq \"CODE\") ? $repl->($msgid) : $msgid;\n+\tmy $link = $cgi->a({-href => \"$URL$msgid_url\"}, $msgid);\n+\n+\t$text =~ s/$msgid/$link/;\n+\n+\treturn $text;\n+}\n+\n # rename detection options for git-diff and git-diff-tree\n # - default is '-M', with the cost proportional to\n #   (number of removed files) * (number of new files).\n@@ -191,6 +303,10 @@ our $git_version = qx($GIT --version) =~\n \n $projects_list ||= $projectroot;\n \n+# enabled committags, and committags enabled for subject\n+our @committags  = grep { $committags{$_}{'enabled'} } keys %committags;\n+our @subjecttags = grep { $committags{$_}{'insubject'} } @committags;\n+\n # ======================================================================\n # input validation and dispatch\n our $action = $cgi->param('a');\n@@ -577,18 +693,43 @@ ## which don't beling to other sections\n # format line of commit message or tag comment\n sub format_log_line_html {\n \tmy $line = shift;\n+\tmy @tags = @_;\n+\tmy $a_attr;\n+\tmy %subst;\n+\n+\tif (!@tags) {\n+\t\t@tags = @committags;\n+\t} else {\n+\t\t$a_attr = ref($tags[0]) eq \"HASH\" ? shift @tags : undef;\n+\t}\n \n \t$line = esc_html($line);\n \t$line =~ s/ /&nbsp;/g;\n-\tif ($line =~ m/([0-9a-fA-F]{40})/) {\n-\t\tmy $hash_text = $1;\n-\t\tif (git_get_type($hash_text) eq \"commit\") {\n-\t\t\tmy $link =\n-\t\t\t\t$cgi->a({-href => href(action=>\"commit\", hash=>$hash_text),\n-\t\t\t\t        -class => \"text\"}, $hash_text);\n-\t\t\t$line =~ s/$hash_text/$link/;\n+\n+\tforeach my $ct (@tags) {\n+\t\tnext unless exists $committags{$ct};\n+\t\tmy $wrap = $a_attr && %$a_attr && $committags{$ct}{'islink'};\n+\t\tmy @opts =\n+\t\t\texists $committags{$ct}{'options'} ?\n+\t\t\t@{$committags{$ct}{'options'}} :\n+\t\t\t();\n+\n+\t\twhile ($line =~ m/($committags{$ct}{'pattern'})/gc) {\n+\t\t\tmy $match = $1;\n+\t\t\tmy $repl = $committags{$ct}{'sub'}->($match, @opts);\n+\t\t\tnext unless $repl;\n+\n+\t\t\tif ($wrap) {\n+\t\t\t\t$repl = $cgi->end_a() . $repl . $cgi->start_a($a_attr);\n+\t\t\t}\n+\n+\t\t\t$subst{quotemeta $match} = $repl;\n \t\t}\n \t}\n+\n+\twhile (my ($from, $to) = each %subst) {\n+\t\t$line =~ s/$from/$to/g;\n+\t}\n \treturn $line;\n }\n \n@@ -626,12 +767,13 @@ sub format_subject_html {\n \t$extra = '' unless defined($extra);\n \n \tif (length($short) < length($long)) {\n-\t\treturn $cgi->a({-href => $href, -class => \"list subject\",\n-\t\t                -title => $long},\n-\t\t       esc_html($short) . $extra);\n+\t\tmy $a_attr = {-href => $href, -class => \"list subject\", -title => $long};\n+\t\treturn $cgi->a($a_attr,\n+\t\t       format_log_line_html($short, $a_attr, @subjecttags) . $extra);\n \t} else {\n-\t\treturn $cgi->a({-href => $href, -class => \"list subject\"},\n-\t\t       esc_html($long)  . $extra);\n+\t\tmy $a_attr = {-href => $href, -class => \"list subject\"};\n+\t\treturn $cgi->a($a_attr,\n+\t\t       format_log_line_html($long,  $a_attr, @subjecttags) . $extra);\n \t}\n }\n \n@@ -693,9 +835,9 @@ sub git_get_type {\n \n \topen my $fd, \"-|\", git_cmd(), \"cat-file\", '-t', $hash or return;\n \tmy $type = <$fd>;\n-\tclose $fd or return;\n+\tclose $fd or return \"unknown\";\n \tchomp $type;\n-\treturn $type;\n+\treturn ($type || \"unknown\");\n }\n \n sub git_get_project_config {\n@@ -1585,7 +1727,7 @@ sub git_print_log ($;%) {\n \t\t\t$empty = 0;\n \t\t}\n \n-\t\tprint format_log_line_html($line) . \"<br/>\\n\";\n+\t\tprint format_log_line_html($line, @committags) . \"<br/>\\n\";\n \t}\n \n \tif ($opts{'-final_empty_line'}) {\n"},{"id":"27416","messageId":"ef182j$88e$1@sea.gmane.org","threadId":"5642","inReplyTo":"200609212356.31806.jnareb@gmail.com","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-22T17:55:27Z","receivedAt":"2006-09-22T17:55:27Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jakub Narebski wrote:\n\n> Below there is preliminary (hence RFC) committag support for gitweb,\n> based on the idea introduced by Sham Chukoury to gitweb-xmms2.\n> \n> The code has all the possible committags I could think of enabled;\n> not all are tested, though. This includes existing commitsha tag\n> support (full sha links to commit view, is sha is sha of commit),\n> mantis bug and feature tags from gitweb-xmms2 (there is no release\n> committag of gitweb-xmms2, but it should be fairly easy to add it),\n> bugzilla committag for the Linux kernel, plain text URL committag\n> (probably doesn't work that well, marking as links examples, and\n> sometimes protocol specification), and Message-Id committag via\n> GMane git mailing list (and not only) archive -- not tested.\n> \n> Comments? Discussion?\n\nFor example: is the committags code overengineered, too complex?\nOr is it not generic enough? Should all committags code be put into\nformat_log_line_html, or should we add another subroutine, \nformat_committags (or similar)?\n\nWhat other committags would you like to have?\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"27440","messageId":"20060923032948.GE8259@pasky.or.cz","threadId":"5642","inReplyTo":"200609212356.31806.jnareb@gmail.com","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-09-23T03:29:48Z","receivedAt":"2006-09-23T03:29:48Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Thu, Sep 21, 2006 at 11:56:31PM CEST, I got a letter\nwhere Jakub Narebski <jnareb@gmail.com> said that...\n> Below there is preliminary (hence RFC) committag support for gitweb,\n> based on the idea introduced by Sham Chukoury to gitweb-xmms2.\n> \n> The code has all the possible committags I could think of enabled;\n> not all are tested, though. This includes existing commitsha tag\n> support (full sha links to commit view, is sha is sha of commit),\n> mantis bug and feature tags from gitweb-xmms2 (there is no release\n> committag of gitweb-xmms2, but it should be fairly easy to add it),\n> bugzilla committag for the Linux kernel, plain text URL committag\n> (probably doesn't work that well, marking as links examples, and\n> sometimes protocol specification), and Message-Id committag via\n> GMane git mailing list (and not only) archive -- not tested.\n\nI think that's a good test. People will certainly want more but that can\nbe added over time.\n\n> -- >8 --\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 7ed963c..5eb0dd0 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -173,6 +173,118 @@ sub feature_pickaxe {\n>  \treturn ($_[0]);\n>  }\n>  \n> +# You define site-wide comittags defaults here; override them with\n> +# $GITWEB_CONFIG as necessary.\n> +our %committags = (\n> +\t# 'committag' => {\n> +\t# \t'pattern' => regexp (use 'qr' quote-like operator)\n> +\t# \t'sub' => committag-sub (subroutine),\n> +\t# \t'enabled' => is given committag enabled,\n> +\t# fields below can be defined, but don't need to\n> +\t# \t'options' => [ default options...] (array reference),\n> +\t# \t'insubject' => should given committag be enabled in commit/tag subject,\n> +\t# \t'islink' => if the result is hyperlink,\n> +\t# }\n> +\t#\n> +\t# You should ensure that enabled committags cannot overlap\n> +\t#\n> +\t# The committag subroutine is called with match for pattern,\n> +\t# and options if they are defined. Match is replaced by return\n> +\t# value of committag-sub.\n\nYou should note that the pattern matches in an already HTML-escaped\ntext. Which is actually quite ugly especially wrt. the &nbsp;, perhaps\nit would be worth considering converting the spaces after this? (The\nMarc links would need fixing tho'.)\n\n..snip..\n> +\t'message_id' => {\n> +\t\t'pattern' => qr/(message|msg)[- ]?id&nbsp;&lt;([^&]*)&rt;/i,\n> +\t\t'enabled' => 1,\n> +\t\t'options' => [\n> +\t\t\t'http://news.gmane.org/find-root.php?message_id=',\n> +\t\t\t\\&quote_msgid_gmane ],\n> +\t\t'sub' => \\&tag_msgid},\n\nMention quote_msgid_marc() (and the URL) in a comment so that it's not\ncompletely unreferenced?\n\n> +);\n..snip..\n> +sub quote_msgid_gmane {\n> +\tmy $msgid = shift || return;\n> +\n> +\treturn '<'.(quotemeta $msgid).'>';\n> +}\n\n<> should be HTML-escaped (unless CGI::a() does that).\n\n> @@ -577,18 +693,43 @@ ## which don't beling to other sections\n..snip..\n> -\tif ($line =~ m/([0-9a-fA-F]{40})/) {\n> -\t\tmy $hash_text = $1;\n> -\t\tif (git_get_type($hash_text) eq \"commit\") {\n> -\t\t\tmy $link =\n> -\t\t\t\t$cgi->a({-href => href(action=>\"commit\", hash=>$hash_text),\n> -\t\t\t\t        -class => \"text\"}, $hash_text);\n> -\t\t\t$line =~ s/$hash_text/$link/;\n> +\n> +\tforeach my $ct (@tags) {\n> +\t\tnext unless exists $committags{$ct};\n\nAt this point, I'd complain (even die) rather than silently pass, that's\ndefinitely a bug.\n\n> +\t\tmy $wrap = $a_attr && %$a_attr && $committags{$ct}{'islink'};\n\n$a_attr can't be but a hashref, that test is redundant.\n\n> @@ -626,12 +767,13 @@ sub format_subject_html {\n>  \t$extra = '' unless defined($extra);\n>  \n>  \tif (length($short) < length($long)) {\n> -\t\treturn $cgi->a({-href => $href, -class => \"list subject\",\n> -\t\t                -title => $long},\n> -\t\t       esc_html($short) . $extra);\n> +\t\tmy $a_attr = {-href => $href, -class => \"list subject\", -title => $long};\n> +\t\treturn $cgi->a($a_attr,\n> +\t\t       format_log_line_html($short, $a_attr, @subjecttags) . $extra);\n>  \t} else {\n> -\t\treturn $cgi->a({-href => $href, -class => \"list subject\"},\n> -\t\t       esc_html($long)  . $extra);\n> +\t\tmy $a_attr = {-href => $href, -class => \"list subject\"};\n> +\t\treturn $cgi->a($a_attr,\n> +\t\t       format_log_line_html($long,  $a_attr, @subjecttags) . $extra);\n>  \t}\n>  }\n>  \n\nSubjects are often clickable and we don't want links in those.\n\n> @@ -693,9 +835,9 @@ sub git_get_type {\n>  \n>  \topen my $fd, \"-|\", git_cmd(), \"cat-file\", '-t', $hash or return;\n>  \tmy $type = <$fd>;\n> -\tclose $fd or return;\n> +\tclose $fd or return \"unknown\";\n>  \tchomp $type;\n> -\treturn $type;\n> +\treturn ($type || \"unknown\");\n>  }\n>  \n>  sub git_get_project_config {\n\n> P.S. I've corrected git_get_type for comparison\n>   git_get_type($hash) eq \"commit\"\n> to work without Perl syntax errors.\n\nHey it's just a warning. ;-) And if you are calling git_get_type() on a\nnon-existing object, don't you have a problem you should better know\nabout? Couldn't this break git_get_refs_list()?\n\n> @@ -1585,7 +1727,7 @@ sub git_print_log ($;%) {\n>  \t\t\t$empty = 0;\n>  \t\t}\n>  \n> -\t\tprint format_log_line_html($line) . \"<br/>\\n\";\n> +\t\tprint format_log_line_html($line, @committags) . \"<br/>\\n\";\n>  \t}\n>  \n>  \tif ($opts{'-final_empty_line'}) {\n\nWhat about the tags? Or perhaps even blobs, for that matter?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj\n$/=unpack('H*',$_);$_=`echo 16dio\\U$k\"SK$/SM$n\\EsN0p[lN*1\nlK[d2%Sa2/d0$^Ixp\"|dc`;s/\\W//g;$_=pack('H*',/((..)*)$/)\n"},{"id":"27453","messageId":"200609231034.49545.jnareb@gmail.com","threadId":"5642","inReplyTo":"20060923032948.GE8259@pasky.or.cz","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-23T08:34:49Z","receivedAt":"2006-09-23T08:34:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Petr \"Pasky\" Baudis wrote:\n> Dear diary, on Thu, Sep 21, 2006 at 11:56:31PM CEST, I got a letter\n> where Jakub Narebski <jnareb@gmail.com> said that...\n\n> > +\t# The committag subroutine is called with match for pattern,\n> > +\t# and options if they are defined. Match is replaced by return\n> > +\t# value of committag-sub.\n> \n> You should note that the pattern matches in an already HTML-escaped\n> text. Which is actually quite ugly especially wrt. the &nbsp;, perhaps\n> it would be worth considering converting the spaces after this? (The\n> Marc links would need fixing tho'.)\n\nPerhaps we better use \"white-space: pre;\" for commit (and tag) messages\ninstead of 'manually' doing this via converting ' ' to '&nbsp;'.\n \n> ..snip..\n> > +\t'message_id' => {\n> > +\t\t'pattern' => qr/(message|msg)[- ]?id&nbsp;&lt;([^&]*)&rt;/i,\n> > +\t\t'enabled' => 1,\n> > +\t\t'options' => [\n> > +\t\t\t'http://news.gmane.org/find-root.php?message_id=',\n> > +\t\t\t\\&quote_msgid_gmane ],\n> > +\t\t'sub' => \\&tag_msgid},\n> \n> Mention quote_msgid_marc() (and the URL) in a comment so that it's not\n> completely unreferenced?\n\nWell, that is RFC. I'm not sure if the final patch would have all those\ncommittags, all those committags but disabled with exception of commitsha/sha,\nor just commitsha tag in source and the rest in commit message as examples.\n\nFor MARC:\n\n\t'options' => [\n\t\t'http://marc.theaimsgroup.com/?i=',\n\t\t\\&quote_msgid_marc ],\n\n> > +);\n> ..snip..\n> > +sub quote_msgid_gmane {\n> > +\tmy $msgid = shift || return;\n> > +\n> > +\treturn '<'.(quotemeta $msgid).'>';\n> > +}\n> \n> <> should be HTML-escaped (unless CGI::a() does that).\n\nCGI::a() probably does that. But true, \"<\" and \">\" should be \"params quoted\":\n\n\treturn '%3c' . (quotemeta $msgid) . '%3e';\n\n(Quotemeta is because Message-Id contains '@').\n\n> > @@ -577,18 +693,43 @@ ## which don't beling to other sections\n> ..snip..\n> > -\tif ($line =~ m/([0-9a-fA-F]{40})/) {\n> > -\t\tmy $hash_text = $1;\n> > -\t\tif (git_get_type($hash_text) eq \"commit\") {\n> > -\t\t\tmy $link =\n> > -\t\t\t\t$cgi->a({-href => href(action=>\"commit\", hash=>$hash_text),\n> > -\t\t\t\t        -class => \"text\"}, $hash_text);\n> > -\t\t\t$line =~ s/$hash_text/$link/;\n> > +\n> > +\tforeach my $ct (@tags) {\n> > +\t\tnext unless exists $committags{$ct};\n> \n> At this point, I'd complain (even die) rather than silently pass,\n> that's definitely a bug.\n\nI've programmed defensively. Perhaps too defensively.\n\n> > +\t\tmy $wrap = $a_attr && %$a_attr && $committags{$ct}{'islink'};\n> \n> $a_attr can't be but a hashref, that test is redundant.\n\n$a_attr can be undefined or be a hashref. It could theoretically\nbe empty hashref, but that is a mistake (<a>...</a>).\n\n> > @@ -626,12 +767,13 @@ sub format_subject_html {\n> >  \t$extra = '' unless defined($extra);\n> >  \n> >  \tif (length($short) < length($long)) {\n> > -\t\treturn $cgi->a({-href => $href, -class => \"list subject\",\n> > -\t\t                -title => $long},\n> > -\t\t       esc_html($short) . $extra);\n> > +\t\tmy $a_attr = {-href => $href, -class => \"list subject\", -title => $long};\n> > +\t\treturn $cgi->a($a_attr,\n> > +\t\t       format_log_line_html($short, $a_attr, @subjecttags) . $extra);\n> >  \t} else {\n> > -\t\treturn $cgi->a({-href => $href, -class => \"list subject\"},\n> > -\t\t       esc_html($long)  . $extra);\n> > +\t\tmy $a_attr = {-href => $href, -class => \"list subject\"};\n> > +\t\treturn $cgi->a($a_attr,\n> > +\t\t       format_log_line_html($long,  $a_attr, @subjecttags) . $extra);\n> >  \t}\n> >  }\n> >  \n> \n> Subjects are often clickable and we don't want links in those.\n\nThe extra code with $a_attr is to have links within links. It works\nquite well, I'd say. The subject link is broken, and the committag\nlink is inserted in the break (gitweb-xmms2 committag code did the same,\nbut did not preserved all the subject link attributes, like title or class,\nonly the target of the link).\n\nThe result is somethink like:\n\n  <a href=\"...\" class=\"subject\" ...>Fix </a><a href=\"...=137\">BUG(137)</a><a href=\"...\" class=\"subject\" ...>: ...</a> \n\n> > @@ -693,9 +835,9 @@ sub git_get_type {\n> >  \n> >  \topen my $fd, \"-|\", git_cmd(), \"cat-file\", '-t', $hash or return;\n> >  \tmy $type = <$fd>;\n> > -\tclose $fd or return;\n> > +\tclose $fd or return \"unknown\";\n> >  \tchomp $type;\n> > -\treturn $type;\n> > +\treturn ($type || \"unknown\");\n> >  }\n> >  \n> >  sub git_get_project_config {\n> \n> > P.S. I've corrected git_get_type for comparison\n> >   git_get_type($hash) eq \"commit\"\n> > to work without Perl syntax errors.\n> \n> Hey it's just a warning. ;-) And if you are calling git_get_type() on a\n> non-existing object, don't you have a problem you should better know\n> about? Couldn't this break git_get_refs_list()?\n\nThere are many cherry-picked comments with commitsha of object which\nexisted on some topic branch, was pruned, and does not exist anymore.\nPerhaps we should add \"2> /dev/null\", too...\n\nI've not tested how this change works with git_get_refs_list(), and other\nsubroutines that use git_get_type().\n\n> > @@ -1585,7 +1727,7 @@ sub git_print_log ($;%) {\n> >  \t\t\t$empty = 0;\n> >  \t\t}\n> >  \n> > -\t\tprint format_log_line_html($line) . \"<br/>\\n\";\n> > +\t\tprint format_log_line_html($line, @committags) . \"<br/>\\n\";\n> >  \t}\n> >  \n> >  \tif ($opts{'-final_empty_line'}) {\n> \n> What about the tags? Or perhaps even blobs, for that matter?\n\nWhat about? In commit messages you usually reference other commits\n(as: this correct some commit, this finishes what was started in commit,\nthis reverts commit (!), cherry-picked commit).\n\nThanks for all the comments.\n-- \nJakub Narebski\nPoland\n"},{"id":"27467","messageId":"20060923121134.GM13132@pasky.or.cz","threadId":"5642","inReplyTo":"200609231034.49545.jnareb@gmail.com","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-09-23T12:11:34Z","receivedAt":"2006-09-23T12:11:34Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sat, Sep 23, 2006 at 10:34:49AM CEST, I got a letter\nwhere Jakub Narebski <jnareb@gmail.com> said that...\n> Perhaps we better use \"white-space: pre;\" for commit (and tag) messages\n> instead of 'manually' doing this via converting ' ' to '&nbsp;'.\n\n+1\n\n> > ..snip..\n> > > +\t'message_id' => {\n> > > +\t\t'pattern' => qr/(message|msg)[- ]?id&nbsp;&lt;([^&]*)&rt;/i,\n> > > +\t\t'enabled' => 1,\n> > > +\t\t'options' => [\n> > > +\t\t\t'http://news.gmane.org/find-root.php?message_id=',\n> > > +\t\t\t\\&quote_msgid_gmane ],\n> > > +\t\t'sub' => \\&tag_msgid},\n> > \n> > Mention quote_msgid_marc() (and the URL) in a comment so that it's not\n> > completely unreferenced?\n> \n> Well, that is RFC. I'm not sure if the final patch would have all those\n> committags, all those committags but disabled with exception of commitsha/sha,\n> or just commitsha tag in source and the rest in commit message as examples.\n\nI'd disable mantis and bugzilla by default (but leave them in the code\ncommented out) since those are totally project-specific, but a lot of\nOSS projects use gmane and URL and commitsha are universally beneficial.\n\nAlso, there is a fundamental limitation for the multi-word patterns that\nthey won't work if the line wraps at that point in the log message. This\nwill likely be a problem especially for the msgids, because those are\nvery long and are very likely to cause a linewrap immediately before.\n\nBy the way, I don't think taking just 40-digits sha1s is very useful,\nsince that's insanely long and besides the linewrap issue, a lot of\npeople just shorten that to some 8 to 12 digits now - I'd use {8-40}\ninstead (the enforced minimum is 4 in the Git autocompletion code but we\nshouldn't encourage people to write so ambiguous sha1s to persistent\nrecords).\n\n> > > +);\n> > ..snip..\n> > > +sub quote_msgid_gmane {\n> > > +\tmy $msgid = shift || return;\n> > > +\n> > > +\treturn '<'.(quotemeta $msgid).'>';\n> > > +}\n> > \n> > <> should be HTML-escaped (unless CGI::a() does that).\n> \n> CGI::a() probably does that. But true, \"<\" and \">\" should be \"params quoted\":\n> \n> \treturn '%3c' . (quotemeta $msgid) . '%3e';\n\nAh, sillly me, I didn't notice that it goes into the URL.\n\n> (Quotemeta is because Message-Id contains '@').\n\nHmm, and at which point would that be eaten?\n\n> > > @@ -577,18 +693,43 @@ ## which don't beling to other sections\n> > > +\t\tmy $wrap = $a_attr && %$a_attr && $committags{$ct}{'islink'};\n> > \n> > $a_attr can't be but a hashref, that test is redundant.\n> \n> $a_attr can be undefined or be a hashref. It could theoretically\n> be empty hashref, but that is a mistake (<a>...</a>).\n\nYes so no point in testing the emptiness. But this is just nitpicking.\n\n> > > @@ -626,12 +767,13 @@ sub format_subject_html {\n> > >  \t$extra = '' unless defined($extra);\n> > >  \n> > >  \tif (length($short) < length($long)) {\n> > > -\t\treturn $cgi->a({-href => $href, -class => \"list subject\",\n> > > -\t\t                -title => $long},\n> > > -\t\t       esc_html($short) . $extra);\n> > > +\t\tmy $a_attr = {-href => $href, -class => \"list subject\", -title => $long};\n> > > +\t\treturn $cgi->a($a_attr,\n> > > +\t\t       format_log_line_html($short, $a_attr, @subjecttags) . $extra);\n> > >  \t} else {\n> > > -\t\treturn $cgi->a({-href => $href, -class => \"list subject\"},\n> > > -\t\t       esc_html($long)  . $extra);\n> > > +\t\tmy $a_attr = {-href => $href, -class => \"list subject\"};\n> > > +\t\treturn $cgi->a($a_attr,\n> > > +\t\t       format_log_line_html($long,  $a_attr, @subjecttags) . $extra);\n> > >  \t}\n> > >  }\n> > >  \n> > \n> > Subjects are often clickable and we don't want links in those.\n> \n> The extra code with $a_attr is to have links within links. It works\n> quite well, I'd say. The subject link is broken, and the committag\n> link is inserted in the break (gitweb-xmms2 committag code did the same,\n> but did not preserved all the subject link attributes, like title or class,\n> only the target of the link).\n> \n> The result is somethink like:\n> \n>   <a href=\"...\" class=\"subject\" ...>Fix </a><a href=\"...=137\">BUG(137)</a><a href=\"...\" class=\"subject\" ...>: ...</a> \n\nI don't think this is good idea though - if I'm clicking at links, I\ndon't want to have to carefully watch where that bit of the link leads.\nIMHO this would be just annoying.\n\n> > > @@ -693,9 +835,9 @@ sub git_get_type {\n> > >  \n> > >  \topen my $fd, \"-|\", git_cmd(), \"cat-file\", '-t', $hash or return;\n> > >  \tmy $type = <$fd>;\n> > > -\tclose $fd or return;\n> > > +\tclose $fd or return \"unknown\";\n> > >  \tchomp $type;\n> > > -\treturn $type;\n> > > +\treturn ($type || \"unknown\");\n> > >  }\n> > >  \n> > >  sub git_get_project_config {\n> > \n> > > P.S. I've corrected git_get_type for comparison\n> > >   git_get_type($hash) eq \"commit\"\n> > > to work without Perl syntax errors.\n> > \n> > Hey it's just a warning. ;-) And if you are calling git_get_type() on a\n> > non-existing object, don't you have a problem you should better know\n> > about? Couldn't this break git_get_refs_list()?\n> \n> There are many cherry-picked comments with commitsha of object which\n> existed on some topic branch, was pruned, and does not exist anymore.\n\nThen I think it's better to explicitly state that \"in this case, this\nobject may not exist\" by checking the return value for undef first.\n\n> Perhaps we should add \"2> /dev/null\", too...\n\nYes that would be sensible.\n\n> > > @@ -1585,7 +1727,7 @@ sub git_print_log ($;%) {\n> > >  \t\t\t$empty = 0;\n> > >  \t\t}\n> > >  \n> > > -\t\tprint format_log_line_html($line) . \"<br/>\\n\";\n> > > +\t\tprint format_log_line_html($line, @committags) . \"<br/>\\n\";\n> > >  \t}\n> > >  \n> > >  \tif ($opts{'-final_empty_line'}) {\n> > \n> > What about the tags? Or perhaps even blobs, for that matter?\n> \n> What about? In commit messages you usually reference other commits\n> (as: this correct some commit, this finishes what was started in commit,\n> this reverts commit (!), cherry-picked commit).\n\nI meant that we should consider substituting the committags in those as\nwell.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj\n$/=unpack('H*',$_);$_=`echo 16dio\\U$k\"SK$/SM$n\\EsN0p[lN*1\nlK[d2%Sa2/d0$^Ixp\"|dc`;s/\\W//g;$_=pack('H*',/((..)*)$/)\n"},{"id":"27473","messageId":"200609231533.02455.jnareb@gmail.com","threadId":"5642","inReplyTo":"20060923121134.GM13132@pasky.or.cz","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-23T13:33:01Z","receivedAt":"2006-09-23T13:33:01Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Petr Baudis wrote:\n\n> Dear diary, on Sat, Sep 23, 2006 at 10:34:49AM CEST, I got a letter\n> where Jakub Narebski <jnareb@gmail.com> said that...\n\n> Also, there is a fundamental limitation for the multi-word patterns that\n> they won't work if the line wraps at that point in the log message. This\n> will likely be a problem especially for the msgids, because those are\n> very long and are very likely to cause a linewrap immediately before.\n\nWe do not wrap log messages in gitweb. So the problem is only when\ncommit message is wrongly wrapped itself (pre imples nowrap).\n\n>> > > +);\n>> > ..snip..\n>> > > +sub quote_msgid_gmane {\n>> > > +\tmy $msgid = shift || return;\n>> > > +\n>> > > +\treturn '<'.(quotemeta $msgid).'>';\n>> > > +}\n>> > \n>> > <> should be HTML-escaped (unless CGI::a() does that).\n>> \n>> CGI::a() probably does that. But true, \"<\" and \">\" should be \"params quoted\":\n>> \n>> \treturn '%3c' . (quotemeta $msgid) . '%3e';\n> \n> Ah, silly me, I didn't notice that it goes into the URL.\n\nShould it be \"params quoted\" or not? What are valid characters\nin message-id? (And in which RFC it is defined?)\n\n>> (Quotemeta is because Message-Id contains '@').\n> \n> Hmm, and at which point would that be eaten?\n\nIn the substitution phase... but perhaps I was to defensive.\ns/$from/$to/g where $to can have '@'. \n\n>> > > @@ -626,12 +767,13 @@ sub format_subject_html {\n>> > >  \t$extra = '' unless defined($extra);\n>> > >  \n>> > >  \tif (length($short) < length($long)) {\n>> > > -\t\treturn $cgi->a({-href => $href, -class => \"list subject\",\n>> > > -\t\t                -title => $long},\n>> > > -\t\t       esc_html($short) . $extra);\n>> > > +\t\tmy $a_attr = {-href => $href, -class => \"list subject\", -title => $long};\n>> > > +\t\treturn $cgi->a($a_attr,\n>> > > +\t\t       format_log_line_html($short, $a_attr, @subjecttags) . $extra);\n>> > >  \t} else {\n>> > > -\t\treturn $cgi->a({-href => $href, -class => \"list subject\"},\n>> > > -\t\t       esc_html($long)  . $extra);\n>> > > +\t\tmy $a_attr = {-href => $href, -class => \"list subject\"};\n>> > > +\t\treturn $cgi->a($a_attr,\n>> > > +\t\t       format_log_line_html($long,  $a_attr, @subjecttags) . $extra);\n>> > >  \t}\n>> > >  }\n>> > >  \n>> > \n>> > Subjects are often clickable and we don't want links in those.\n>> \n>> The extra code with $a_attr is to have links within links. It works\n>> quite well, I'd say. The subject link is broken, and the committag\n>> link is inserted in the break (gitweb-xmms2 committag code did the same,\n>> but did not preserved all the subject link attributes, like title or class,\n>> only the target of the link).\n>> \n>> The result is somethink like:\n>> \n>>   <a href=\"...\" class=\"subject\" ...>Fix </a><a href=\"...=137\">BUG(137)</a><a href=\"...\" class=\"subject\" ...>: ...</a> \n> \n> I don't think this is good idea though - if I'm clicking at links, I\n> don't want to have to carefully watch where that bit of the link leads.\n> IMHO this would be just annoying.\n\nThe committag links within subject link are clearly visually distinguished:\nfirst they have default link color (blue for not visited, dark red for\nvisited links), second they are not bold width (as opposed to gitweb-xmms2,\nwhere bold font was due to <b>...</b> element and not CSS styling \nof a.subject).\n\nI have just noticed that somehow git_log (\"log\" view) doesn't use\nformat_subject_line (perhaps because it is not shortened) and that\ncommittags are not used there. And that is not only place where\nsubjecttags (committags in clickable subject line) are not used.\n\nBy the way, you can easily disable some committags in subject, either\nremoving 'insubject' field, or setting it to false.\n\n>> > > @@ -1585,7 +1727,7 @@ sub git_print_log ($;%) {\n>> > >  \t\t\t$empty = 0;\n>> > >  \t\t}\n>> > >  \n>> > > -\t\tprint format_log_line_html($line) . \"<br/>\\n\";\n>> > > +\t\tprint format_log_line_html($line, @committags) . \"<br/>\\n\";\n>> > >  \t}\n>> > >  \n>> > >  \tif ($opts{'-final_empty_line'}) {\n>> > \n>> > What about the tags? Or perhaps even blobs, for that matter?\n>> \n>> What about? In commit messages you usually reference other commits\n>> (as: this correct some commit, this finishes what was started in commit,\n>> this reverts commit (!), cherry-picked commit).\n> \n> I meant that we should consider substituting the committags in those as\n> well.\n\nAhh... For tags I guess it is a good idea (especially that I think that\nfixes for bugtracker tracked bugs and feature request should be marked by tags,\ne.g. b/<bugid>, to be used to link to commit/change/patch from bugtracker. \n\nBy the way, should we use some color for PGP signature block in signed tags?\n\n-- \nJakub Narebski\nShadeHawk on #git\nTorun, Poland\n"},{"id":"27474","messageId":"20060923140535.GK8259@pasky.or.cz","threadId":"5642","inReplyTo":"200609231533.02455.jnareb@gmail.com","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-09-23T14:05:35Z","receivedAt":"2006-09-23T14:05:35Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sat, Sep 23, 2006 at 03:33:01PM CEST, I got a letter\nwhere Jakub Narebski <jnareb@gmail.com> said that...\n> Petr Baudis wrote:\n> \n> > Dear diary, on Sat, Sep 23, 2006 at 10:34:49AM CEST, I got a letter\n> > where Jakub Narebski <jnareb@gmail.com> said that...\n> \n> > Also, there is a fundamental limitation for the multi-word patterns that\n> > they won't work if the line wraps at that point in the log message. This\n> > will likely be a problem especially for the msgids, because those are\n> > very long and are very likely to cause a linewrap immediately before.\n> \n> We do not wrap log messages in gitweb. So the problem is only when\n> commit message is wrongly wrapped itself (pre imples nowrap).\n\nThe commit message is not \"wrongly\" wrapped but just wrapped to fit into\n72 or whatever columns. It would be silly to mandate users to use msg-id: <200609231533.02455.jnareb@gmail.com>\nwith the message id stretching far away just for the sake of some\ngitweb limitations when having the message wrapped such as msg-id:\n<200609231533.02455.jnareb@gmail.com> looks much more reasonable.\n\n> >> > > +);\n> >> > ..snip..\n> >> > > +sub quote_msgid_gmane {\n> >> > > +\tmy $msgid = shift || return;\n> >> > > +\n> >> > > +\treturn '<'.(quotemeta $msgid).'>';\n> >> > > +}\n> >> > \n> >> > <> should be HTML-escaped (unless CGI::a() does that).\n> >> \n> >> CGI::a() probably does that. But true, \"<\" and \">\" should be \"params quoted\":\n> >> \n> >> \treturn '%3c' . (quotemeta $msgid) . '%3e';\n> > \n> > Ah, silly me, I didn't notice that it goes into the URL.\n> \n> Should it be \"params quoted\" or not? What are valid characters\n> in message-id? (And in which RFC it is defined?)\n\nRFC2822. Unfortunately there can be all kind of crap inside if you put\nit inside quotes, so yes, it should.\n\nWhat a pity that (apparently) noone supports RFC2392.\n\n> >> (Quotemeta is because Message-Id contains '@').\n> > \n> > Hmm, and at which point would that be eaten?\n> \n> In the substitution phase... but perhaps I was to defensive.\n> s/$from/$to/g where $to can have '@'. \n\n$ perl -le 'my $x=\"a\\@bc\"; my $y=\"@\"; $x =~ s/$y/$y.$y/g; print $x;'\na@.@bc\n\nYou probably should regexp-quote $from if you don't yet, though,\nalthough a misbehaviour from that side is not very probable.\n\n> >> > Subjects are often clickable and we don't want links in those.\n> >> \n> >> The extra code with $a_attr is to have links within links. It works\n> >> quite well, I'd say. The subject link is broken, and the committag\n> >> link is inserted in the break (gitweb-xmms2 committag code did the same,\n> >> but did not preserved all the subject link attributes, like title or class,\n> >> only the target of the link).\n> >> \n> >> The result is somethink like:\n> >> \n> >>   <a href=\"...\" class=\"subject\" ...>Fix </a><a href=\"...=137\">BUG(137)</a><a href=\"...\" class=\"subject\" ...>: ...</a> \n> > \n> > I don't think this is good idea though - if I'm clicking at links, I\n> > don't want to have to carefully watch where that bit of the link leads.\n> > IMHO this would be just annoying.\n> \n> The committag links within subject link are clearly visually distinguished:\n> first they have default link color (blue for not visited, dark red for\n> visited links), second they are not bold width (as opposed to gitweb-xmms2,\n> where bold font was due to <b>...</b> element and not CSS styling \n> of a.subject).\n\nOk, in that case it's better though I'm still feeling somewhat\nuncomfortable about it. What if the _whole_ subject is just \"Bug 1324\"?\nI can click the 'commit' link instead but it throws exception in my\nbrain I need to handle and I need to move my mouse around.\n\n> > I meant that we should consider substituting the committags in those as\n> > well.\n> \n> Ahh... For tags I guess it is a good idea (especially that I think that\n> fixes for bugtracker tracked bugs and feature request should be marked by tags,\n> e.g. b/<bugid>, to be used to link to commit/change/patch from bugtracker. \n\nFor blobs, the point is mainly comments, those can contain all kinds of\nstuff.\n\n> By the way, should we use some color for PGP signature block in signed tags?\n\nSounds like a good idea.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj\n$/=unpack('H*',$_);$_=`echo 16dio\\U$k\"SK$/SM$n\\EsN0p[lN*1\nlK[d2%Sa2/d0$^Ixp\"|dc`;s/\\W//g;$_=pack('H*',/((..)*)$/)\n"},{"id":"27507","messageId":"7veju2s6bi.fsf@assigned-by-dhcp.cox.net","threadId":"5642","inReplyTo":"20060923121134.GM13132@pasky.or.cz","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-23T19:58:57Z","receivedAt":"2006-09-23T19:58:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis <pasky@suse.cz> writes:\n\n> Also, there is a fundamental limitation for the multi-word patterns that\n> they won't work if the line wraps at that point in the log message. This\n> will likely be a problem especially for the msgids, because those are\n> very long and are very likely to cause a linewrap immediately before.\n>\n> By the way, I don't think taking just 40-digits sha1s is very useful,\n> since that's insanely long and besides the linewrap issue, a lot of\n> people just shorten that to some 8 to 12 digits now - I'd use {8-40}\n> instead (the enforced minimum is 4 in the Git autocompletion code but we\n> shouldn't encourage people to write so ambiguous sha1s to persistent\n> records).\n\nTrue.  Without looking at Jakub's code under discussion, off the\ntop of my head, I wonder if we can do something like this:\n\n (1) take the whole commit log message, not line by line, into a\n     Perl variable, say $log.  Note that this is _before_ HTML\n     escaping.\n\n (2) each enabled tag-marking sub are expected to take a list of\n     string or ref as its parameter.  The first one in the chain\n     takes a single parameter which is ($log).  You call all the\n     enabled ones, in some order, and feed the return value from\n     the previous one as the input to the next one.\n\n (3) a tag-marking sub are expected to inspect the list it\n     received as its input parameter, and build its return value\n     by looking at each element in the list:\n\n\t(3-1) if the element is not a string, pass it down as-is\n\tin the output list.\n\n\t(3-2) if the element is a string, do the pattern match,\n\tand output one or more elements in the output list.\n\tLiteral string you did not care about are placed as\n\tstring for later taggers to be processed, the links you\n\tgenerate are placed as a ref to string.\n\n    For example, commit_id tag-marker might look like this:\n\n        sub tag_marker_commit_id {\n                my @input = (@_);\n                my @output;\n                for (@input) {\n                        if (!ref $_) {\n                                push @output, $_;\n                                next;\n                        }\n                        while ($_ ne '' && m/(.*?)\\b([0-9a-fA-F]{6,})\\b(.*)/s) {\n                                my ($pre, $sha1, $post) = ($1, $2, $3);\n                                if (!is_a_commit_id($sha1)) {\n                                        push @output, \"$pre$sha1\";\n                                }\n                                else {\n\t\t\t\t\tmy $subst =\n                                          href(action => 'commit', hash => $2);\n                                        push @output, $1;\n                                        push @output, \\$subst;\n                                }\n                                $_ = $post;\n                        }\n                }\n                return @output;\n        }\n\n (4) when you finished calling the chain, you would have a list\n     of string and ref.  You html quote the strings and pass\n     the ref's (which are supposed to be already HTML) as is,\n     and concatenate the result:\n\n\tsub markup_all {\n        \tmy ($log) = @_;\n                my @current = $log;\n                for my $tag_marker (@tag_markers) {\n\t\t\t@current = $tag_marker->(@current);\n\t\t}\n                return join('',\n                \tmap {\n                        \t(ref $_) ? $$_ : esc_html($_)\n\t\t\t} @current);\n\t}\n\nHmm?\n\n     \n"},{"id":"27508","messageId":"ef44r4$km0$1@sea.gmane.org","threadId":"5642","inReplyTo":"7veju2s6bi.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-23T20:18:50Z","receivedAt":"2006-09-23T20:18:50Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> True.  Without looking at Jakub's code under discussion, off the\n> top of my head, I wonder if we can do something like this:\n\nThe code was to analyse log message line after line, doing loop\nover all committags, building list (well, hash to be more exact)\nof _exact_ replacements (e.g. in the case of commit_id/commitsha\ncommittag we use exact hash as the replaced part, not generic\nregexp to find a sha1).\n\nThe idea of doing committag parsing, and preserving replacements\nfrom matching committag and being subject to HTML escaping by using\nreference to string has it's merits.\n\nI'd have to think about it.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"27511","messageId":"7v4puys3ye.fsf@assigned-by-dhcp.cox.net","threadId":"5642","inReplyTo":"ef44r4$km0$1@sea.gmane.org","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-23T20:50:01Z","receivedAt":"2006-09-23T20:50:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> The idea of doing committag parsing, and preserving replacements\n> from matching committag and being subject to HTML escaping by using\n> reference to string has it's merits.\n>\n> I'd have to think about it.\n\nGood.  My obviously buggy illustration would be easier to read\nwith this patch ;-).\n\n--- gomi\t2006-09-23 13:47:52.000000000 -0700\n+++ gomi\t2006-09-23 13:48:23.000000000 -0700\n@@ -2,7 +2,7 @@\n                 my @input = (@_);\n                 my @output;\n                 for (@input) {\n-                        if (!ref $_) {\n+                        if (ref $_) {\n                                 push @output, $_;\n                                 next;\n                         }\n@@ -19,6 +19,9 @@\n                                 }\n                                 $_ = $post;\n                         }\n+\t\t\tif ($_ ne '') {\n+\t\t\t\tpush @output, $_;\n+\t\t\t}\n                 }\n                 return @output;\n         }\n"},{"id":"27533","messageId":"ef5jkv$gep$1@sea.gmane.org","threadId":"5642","inReplyTo":"7v4puys3ye.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-24T09:37:45Z","receivedAt":"2006-09-24T09:37:45Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>> The idea of doing committag parsing, and preserving replacements\n>> from matching committag and being subject to HTML escaping by using\n>> reference to string has it's merits.\n>>\n>> I'd have to think about it.\n> \n> Good.  My obviously buggy illustration would be easier to read\n> with this patch ;-).\n\nDon't forget about final s,\\n,<br/>\\n,gm if we parse it as a whole,\nand not line by line. \n\nBy the way, I think that only the driver (i.e. format_log_line_html) has to\nbe changed...\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"27534","messageId":"7vy7s9bljc.fsf@assigned-by-dhcp.cox.net","threadId":"5642","inReplyTo":"ef5jkv$gep$1@sea.gmane.org","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-24T10:34:31Z","receivedAt":"2006-09-24T10:34:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Don't forget about final s,\\n,<br/>\\n,gm if we parse it as a whole,\n> and not line by line. \n\nYes, the driver and individual tag markers need to have a\nconvention on who is responsible for doing <br />.\nOne reasonable convention would be to make the driver side\nresponsible for taking \\n and turns it into <br /> in plain\nstrings, and make the tag markers responsible for doing so in\ntheir marked-up output.\n\nFor example, when bugzilla_tagger is annotating this commit\nmessage:\n\n    This rewrites frotz() routine to work around Bug\n    #22574.  The fix was confirmed by Xyzzy.\n\n    Thanks for everybody who helped with this rewrite.\n\nit would match with a regexp /\\b(?i:bug)\\s*#?(\\d+)/s\nto pick up \"Bug\\n#22574\", extract 22574, and produce:\n\n    ('This rewrites frotz() routine to work around ',\n     \\'<a href=\"/cgi-bin/bugzilla.cgi?id=22574\">Bug<br/>#22574</a>',\n     '.  The fix was confirmed by Xyzzy.\\n\\nThanks for everybody...')\n"},{"id":"27557","messageId":"ef6c42$h1b$1@sea.gmane.org","threadId":"5642","inReplyTo":"ef5jkv$gep$1@sea.gmane.org","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-24T16:35:26Z","receivedAt":"2006-09-24T16:35:26Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jakub Narebski wrote:\n\n> Junio C Hamano wrote:\n> \n>> Jakub Narebski <jnareb@gmail.com> writes:\n>> \n>>> The idea of doing committag parsing, and preserving replacements\n>>> from matching committag and being subject to HTML escaping by using\n>>> reference to string has it's merits.\n>>>\n>>> I'd have to think about it.\n>> \n>> Good.  My obviously buggy illustration would be easier to read\n>> with this patch ;-).\n> \n> Don't forget about final s,\\n,<br/>\\n,gm if we parse it as a whole,\n> and not line by line. \n> \n> By the way, I think that only the driver (i.e. format_log_line_html) has to\n> be changed...\n\nYet another question is how to deal with commit message specific\n\"syntax highlighting\". Currently, parsing commit message line by line,\nwe treat specially signoff lines (syntax highlighting, and removing\ntrailing empty lines after signoff), empty lines (we collapse consecutive\nempty lines); the rest goes through format_log_line_html... and committags.\n\nTag messages have another specific syntax highlighting, namely PGP/GPG\nsignature part, which should be syntax highlighted I think (and not \nsubject to committags replacements).\n\n\nSome of the problems of marking replacements as not to be committags \nsearched and not to be HTML escaped can be dealt with by changing the\norder of committags. For example having 'URL' tag as first tag to check\nwe avoid invoking it on committag replacements links.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"27574","messageId":"7vac4p846a.fsf@assigned-by-dhcp.cox.net","threadId":"5642","inReplyTo":"ef6c42$h1b$1@sea.gmane.org","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-24T19:17:49Z","receivedAt":"2006-09-24T19:17:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Yet another question is how to deal with commit message specific\n> \"syntax highlighting\". Currently, parsing commit message line by line,\n> we treat specially signoff lines (syntax highlighting, and removing\n> trailing empty lines after signoff), empty lines (we collapse consecutive\n> empty lines); the rest goes through format_log_line_html... and committags.\n\nActually I was hoping that my bright idea(tm) of passing the\nwhole message to a chain of tag markers would solve that\nautomatically.  You define a std marker whose purpose is _not_ \nto add anchor elements leading to other URLs but mark up\nsign-off lines and stuff.  Put that at the beginning or at the\nend of the chain as you see fit and it would do its work just\nlike other real tag markers would do.\n\nFor example, one of the things it would do is to unhighlight\nthe Signed-off-by: lines.  It would match and split with\n\n\t/(.*?)^(Signed-off-by: .*?)$(.*)/ms\n\nthen add:\n\n\t( $1, # pass as literal we did not touch\n          \\'<span class=\"signoff\">', # unhighlight\n          $2, # give chance to muck with e-mail address to downstream\n\t      # markers\n          \\'</span>'\n        )\n\nto its output buffer and process the rest ($3).  A downstream\ntag maker may match e-mail address the above left and might mark\nit up with mailto: URL.\n\n\n          \n\n\t\n"},{"id":"27632","messageId":"ef95rk$o4q$1@sea.gmane.org","threadId":"5642","inReplyTo":"20060923140535.GK8259@pasky.or.cz","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-25T18:06:44Z","receivedAt":"2006-09-25T18:06:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Petr Baudis wrote:\n\n> Dear diary, on Sat, Sep 23, 2006 at 03:33:01PM CEST, I got a letter\n> where Jakub Narebski <jnareb@gmail.com> said that...\n>> Petr Baudis wrote:\n>> \n>> > Dear diary, on Sat, Sep 23, 2006 at 10:34:49AM CEST, I got a letter\n>> > where Jakub Narebski <jnareb@gmail.com> said that...\n>> \n>> > Also, there is a fundamental limitation for the multi-word patterns that\n>> > they won't work if the line wraps at that point in the log message. This\n>> > will likely be a problem especially for the msgids, because those are\n>> > very long and are very likely to cause a linewrap immediately before.\n>> \n>> We do not wrap log messages in gitweb. So the problem is only when\n>> commit message is wrongly wrapped itself (pre imples nowrap).\n> \n> The commit message is not \"wrongly\" wrapped but just wrapped to fit into\n> 72 or whatever columns. It would be silly to mandate users to use msg-id: <200609231533.02455.jnareb@gmail.com>\n> with the message id stretching far away just for the sake of some\n> gitweb limitations when having the message wrapped such as msg-id:\n> <200609231533.02455.jnareb@gmail.com> looks much more reasonable.\n\nSo when putting message if into commit message, just put the commit\nmessage in separate line, perhaps even with some indentation, like\nbelow:\n  Msg-Id: <200609231533.02455.jnareb@gmail.com>\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"27636","messageId":"20060925201500.GG20017@pasky.or.cz","threadId":"5642","inReplyTo":"ef95rk$o4q$1@sea.gmane.org","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-09-25T20:15:00Z","receivedAt":"2006-09-25T20:15:00Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Mon, Sep 25, 2006 at 08:06:44PM CEST, I got a letter\nwhere Jakub Narebski <jnareb@gmail.com> said that...\n> So when putting message if into commit message, just put the commit\n> message in separate line, perhaps even with some indentation, like\n> below:\n>   Msg-Id: <200609231533.02455.jnareb@gmail.com>\n\nOh please. :-) The tool should bend, not the user.\n\nAre you going to mandate that for bug references as well? Or should I\nproofread all of my commits (if they aren't actually just autoformatted\nby fmt without my further review) to verify that they don't actually end\nup wrapped, and manually reformat the whole paragraph?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i<X+d*lMLa^*lN%0]dsXx++lMlN/dsM0<j]dsj\n$/=unpack('H*',$_);$_=`echo 16dio\\U$k\"SK$/SM$n\\EsN0p[lN*1\nlK[d2%Sa2/d0$^Ixp\"|dc`;s/\\W//g;$_=pack('H*',/((..)*)$/)\n"},{"id":"27674","messageId":"efb4an$lg9$1@sea.gmane.org","threadId":"5642","inReplyTo":"20060925201500.GG20017@pasky.or.cz","subject":"Re: [RFC/PATCH] gitweb: Add committags support","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-09-26T11:52:59Z","receivedAt":"2006-09-26T11:52:59Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Petr Baudis wrote:\n\n> Dear diary, on Mon, Sep 25, 2006 at 08:06:44PM CEST, I got a letter\n> where Jakub Narebski <jnareb@gmail.com> said that...\n>> So when putting message if into commit message, just put the commit\n>> message in separate line, perhaps even with some indentation, like\n>> below:\n>>   Msg-Id: <200609231533.02455.jnareb@gmail.com>\n> \n> Oh please. :-) The tool should bend, not the user.\n\nStill it is more (much more) readable to have Msg-Id \"header\" (or\nequivalent), and message id itself in one line.\n\n> Are you going to mandate that for bug references as well? Or should I\n> proofread all of my commits (if they aren't actually just autoformatted\n> by fmt without my further review) to verify that they don't actually end\n> up wrapped, and manually reformat the whole paragraph?\n\nO.K. that convinces me (although wrapped hyperlink is not a pretty thing).\nBesides for tags we have natural multiline \"tag\", namely PGP signature,\nwhich we wpuld (most probably) want to syntax highlight wrapping it in\n<div class=\"GPG_signature\">...</div> element.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"}]}