{"thread":{"id":"43212","subject":"Re: [RFC] gitweb: Add committags support (take 2)","startedAt":"2006-12-03T23:01:12Z","lastAt":"2006-12-06T20:35:44Z","messageCount":11,"participants":["Junio C Hamano","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"296271","messageId":"200612040001.13640.jnareb@gmail.com","threadId":"43212","inReplyTo":null,"subject":"[RFC] gitweb: Add committags support (take 2)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-03T23:01:12Z","receivedAt":"2006-12-03T23:01:12Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This is my second attemt at adding properly committags support, similar\nto what gitweb-xmms2 added in e89abf232a83e579b2bbc76398aefa048661dddf.\n\nIn this attempt I follow Junio idea of sequential committags support,\nmarking some fragments as not-for-parsing for later committags nor\nesc_html, instead of building the list of literal replacements to be\napplied simultaneously.\n\nThe plan of patch series is the following:\n * [PATCH 1/3] gitweb: Add committags support\n   (which would add generic committags support, reusing existing\n    infrastructure of commit message line by line parsing and\n    simplification: leave git_print_log as is, replacing only\n    format_log_line_html by committags capable version)\n * [PATCH 2/3] gitweb: Abuse committags support\n   (which would use committags for marking signoff lines, and for\n    commit message simplification: compacting empty lines, removing\n    initial and trailing empty lines)\n * [PATCH 3/3] gitweb: Use committags in subject line and tag comment\n   (which would add committags support to subject line/title of commit,\n    which is first line of commit message, and to tag message/comment)\n\nBut there are still some problems to be solved before posting proper\neven the first patch; I'll discuss them part by part.\n\n1. Should we provide examples of committags, even if they _have_ to have\nbe per project, or at least per gitweb instalation configuration, like\nbugtracker support (depending on the bugtracker used by\nproject/projects) and message id support (depending on mailing list\narchive project/projects use)? So far I have come with the following\ncommittag examples (they are for [PATCH 1/3] in the series, so they\ndon't include signoff committag, not empty_lines_simplification\ncommittag).\n\nShould we use anonymous subroutines, or should we define one subroutine\nper committag (how to name them: format_<name>_tag, or <name>_committag,\nor committag_<name>,... ?) and reference it? The code below uses\nanonymous subroutines.\n\nThe subroutine should return either reference to scalar which means\n\"do not process\", scalar which means changed available for further\nprocessing, or void (undef) which means no change. In [PATCH 2/3] we\nwill enable returning also list of elements, each of which could be\nreference to scalar or scalar (for example signoff would return three\nelement list: opening span element as ref, signoff text as scalar,\nclosing span element as ref).\n\n  our %committags = (\n  \t'sha1' => {\n  \t\t'pattern' => qr/[0-9a-fA-F]{40}/,\n  \t\t'sub' => sub {\n  \t\t\tmy $hash_text = shift;\n  \t\t\tmy $type = git_get_type($hash_text);\n  \t\t\tif ($type) {\n  \t\t\t\treturn \\$cgi->a({-href => href(action=>$type, hash=>$hash_text),\n  \t\t\t\t                -class => \"text\"}, $hash_text);\n  \t\t\t}\n  \t\t\treturn undef;\n  \t\t},\n  \t},\n  \t'mantis' => {\n  \t\t'pattern' => qr/(BUG|FEATURE)\\(\\d+\\)/,\n  \t\t'options' => [ 'http://bugs.xmms2.xmms.se/view.php?id=' ],\n  \t\t'sub' => sub {\n  \t\t\tmy $match = shift;\n  \t\t\tmy $URL = shift;\n  \t\t\tmy ($issue) = $match =~ /(\\d+)/;\n  \t\t\treturn\n  \t\t\t\t\\$cgi->a({-href => \"$URL$issue\"},\n  \t\t\t\t         $match);\n  \t\t},\n  \t},\n  \t'bugzilla' => {\n  \t\t'pattern' => qr/bug\\s+\\d+/,\n  \t\t'options' => [ 'http://bugzilla.kernel.org/show_bug.cgi?id=' ],\n  \t\t'sub' => sub {\n  \t\t\tmy $match = shift;\n  \t\t\tmy $URL = shift;\n  \t\t\tmy ($bugno) = $match =~ /(\\d+)/;\n  \t\t\treturn\n  \t\t\t\t\\$cgi->a({-href => \"$URL=$bugno\"},\n  \t\t\t\t         $match);\n  \t\t},\n  \t},\n  \t'URL' => {\n  \t\t'pattern' => qr!(http|ftp)s?://[a-zA-Z0-9%./]+[a-zA-Z0-9/]!,\n  \t\t'sub' => sub {\n  \t\t\tmy $url = shift;\n  \t\t\treturn\n  \t\t\t\t\\$cgi->a({-href => $url},\n  \t\t\t\t         $url); # should be perhaps shortened\n  \t\t},\n  \t},\n  \t# this is committags v1 subroutine\n  \t'message_id' => { # use mailing list archive\n  \t\t'pattern' => qr!(message|msg)-id:?\\s+<[^>]*>;!i,\n  \t\t'options' => [ 'http://news.gmane.org/find-root.php?message_id=' ],\n  \t\t'sub' => sub {\n  \t\t\tmy $text = shift;\n  \t\t\tmy $URL = shift;\n  \n  \t\t\t$text =~ m/<([^>]*)>/;\n  \t\t\tmy $msgid = $1;\n  \t\t\tmy $link = $cgi->a({-href => \"$URL<$msgid>\"},\n  \t\t\t                   $msgid);\n  \t\t\t$text =~ s/$msgid/$link/;\n  \n  \t\t\treturn \\$text;\n  \t\t},\n  \t},\n  );\n\nFrom those committags, only 'sha1' (used now in the limited commitsha\nform currently in gitweb) and 'URL' are installation independent.\nFor xmms2 the 'mantis' tag is needed; for Linux kernel it would be nice\nto have both 'bugzilla' and 'message_id' (with some LKML archive) used.\n\nBy the way, if you have ideas for other committags, please do write.\nFor example should esc_html be the last committag?\n\n\n2. We might not want to use all defined committags in %committags hash.\nWe might want only some of them for commit message [body], some of them\nfor subject line, and some of them for tag message. Additionally the\n_sequence_ of committags is important. As we cannot rely on \n\"keys %committags\" to be in any specific order, we have to provide\nthe ordering ourself. My idea is to use array with the hash keys\n(the names of committags) to both define which committags are enabled,\nand the sequence of committags itself.\n\nIn [PATCH 1/3] there is need for only one such array.\n\n  our @committags = ('sha1');\n\n\n3. To not split message into many fragments we concatenate strings\nif possible. This happens when committag subroutine returns string\n(for example we might want to hyperlink only bug number in 'bugzilla'\ncommittag), or return undef (for example 'sha1' committag if the\nthere does not exist object with given hash, for example in the commit\nmessage of commit cherrypicked from no longer existing branch).\n\nThis is example where Perl subroutine prototypes makes sense.\n\n  # push_or_append ARRAY,LIST\n  # if ARRAY ends with scalar, and LIST begins with scalar, concatenate\n  sub push_or_append (\\@@) {\n  \tmy $list = shift;\n  \n  \tif (ref $_[0] || ! @$list || ref $list->[-1]) {\n  \t\tpush @$list, @_;\n  \t} else {\n  \t\tmy $a = pop @$list;\n  \t\tmy $b = shift @_;\n  \n  \t\tpush @$list, $a . $b, @_;\n  \t}\n  \t# imitate push\n  \treturn scalar @$list;\n  }\n\n\n4. Here is the main subroutine for committags support, currently without\nthe extra parts which would allow committags in the subject line: the\nsubject is link (<a> element) itself, and in (X)HTML link elements cannot\nbe nested, so the encompassing <a> element has to be closed and reopened).\n\nI have thought about three solutions, neither without disadvantages.\n * Mark committag as returning link in %committag hash, and wrap it in\n   the format_log_line_html subroutine. Disadvantages: prone to config\n   errors, doesn't work if link is only part of replacement.\n * Use special a_link subroutine in the committag subs instead of\n   $cgi->a(). Disadvantages: need to pass attributes of encompassing\n   <a> element to committag subroutine, prone to committag subroutine\n   coding errors.\n * Wrap on dereferencing in the final output stage, checking if referenced\n   string is wholly <a> element. Disadvantages: committags subroutine\n   must return <a> elements separately.\n\nWell, yet another solution would be to forbid committags support in the\nsubject/title line. gitweb-xmms2 tries to implement this, but not fully,\nand not without errors.\n\n  sub format_log_line_html {\n\tmy $line = shift;\n\tmy @committags = @_;\n\n\tmy @list = ( $line );\n\n   COMMITTAG:\n  \tforeach my $ctname (@committags) {\n  \t\tnext COMMITTAG unless exists $committags{$ctname};\n  \n  \t\tmy @opts =\n  \t\t\texists $committags{$ctname}{'options'} ?\n  \t\t\t@{$committags{$ctname}{'options'}} :\n  \t\t\t();\n  \t\tmy @newlist = ();\n\n  \tPART:\n  \t\tforeach my $part (@list) {\n  \t\t\tnext PART unless $part;\n  \t\t\tif (ref($part)) {\n  \t\t\t\tpush @newlist, $part;\n  \t\t\t\tnext PART;\n  \t\t\t}\n  \n  \t\t\tmy $oldpos = 0;\n  \t\tMATCH:\n  \t\t\twhile ($part =~ m/($committags{$ctname}{'pattern'})/gc) {\n  \t\t\t\tmy $match = $1;\n  \t\t\t\tmy ($prepos, $postpos) = ($-[0], $+[0]);\n  \t\t\t\tmy $repl = $committags{$ctname}{'sub'}->($match, @opts);\n  \n  \t\t\t\tif (defined $repl) {\n  \t\t\t\t\tmy $pre = substr($part, $oldpos, $prepos - $oldpos);\n  \n  \t\t\t\t\tpush_or_append @newlist, $pre if $pre;\n  \t\t\t\t\tpush_or_append @newlist, $repl;\n  \t\t\t\t} else {\n  \t\t\t\t\tmy $all = substr($part, $oldpos, $postpos - $oldpos);\n  \n  \t\t\t\t\tpush_or_append @newlist, $all if $all;\n  \t\t\t\t}\n  \n  \t\t\t\t$oldpos = $postpos;\n  \t\t\t} # end while [regexp matches]\n  \n  \t\t\tmy $rest = substr($part, $oldpos);\n  \t\t\tpush_or_append @newlist, $rest if $rest;\n  \n  \t\t} # end foreach (@list)\n  \n  \t\t@list = @newlist;\n  \t} # end foreach (@committags)\n  \n  \t# print it\n  \tmy $result = '';\n  \tfor my $part (@list) {\n  \t\tif (ref($part)) {\n  \t\t\t$result .= $$part;\n  \t\t} else {\n  \t\t\t$result .= esc_html($part, -nbsp=>1);\n \t\t}\n  \t}\n  \n  \treturn $result;\n  }\n\n-- \nJakub Narebski\n"},{"id":"294045","messageId":"7virgstmg6.fsf@assigned-by-dhcp.cox.net","threadId":"43212","inReplyTo":"200612040001.13640.jnareb@gmail.com","subject":"Re: [RFC] gitweb: Add committags support (take 2)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-04T02:38:17Z","receivedAt":"2006-12-04T02:38:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> The subroutine should return either reference to scalar which means\n> \"do not process\", scalar which means changed available for further\n> processing, or void (undef) which means no change. In [PATCH 2/3] we\n> will enable returning also list of elements, each of which could be\n> reference to scalar or scalar (for example signoff would return three\n> element list: opening span element as ref, signoff text as scalar,\n> closing span element as ref).\n\nPersonally I think that \"list of elements\" should be in the\nfirst patch to build the foundation.\n\n>   our %committags = (\n>   \t'sha1' => {\n>   \t\t'pattern' => qr/[0-9a-fA-F]{40}/,\n>   \t\t'sub' => sub {\n>   \t\t\tmy $hash_text = shift;\n>   \t\t\tmy $type = git_get_type($hash_text);\n>   \t\t\tif ($type) {\n>   \t\t\t\treturn \\$cgi->a({-href => href(action=>$type, hash=>$hash_text),\n>   \t\t\t\t                -class => \"text\"}, $hash_text);\n>   \t\t\t}\n>   \t\t\treturn undef;\n>   \t\t},\n>   \t},\n\nIt might make sense to do a /[0-9a-f]{8,40}/ pattern, and make\nsure that the named object exists in the sub (which you already\ndo).  People often write abbreviated commit object name that has\na high chance of staying unique during the life of the project.\n\n>   \t'mantis' => {\n>   \t\t'pattern' => qr/(BUG|FEATURE)\\(\\d+\\)/,\n>   \t'bugzilla' => {\n>   \t\t'pattern' => qr/bug\\s+\\d+/,\n\nThis is not wrong per-se but somehow feels funny.  It feels as\nif there is a convention that bugzilla bugs are usually spelled\nin lowercase while mantis bugs are in uppercase, but I do not\nthink you are suggesting that.\n\nI do not know how this %committags{} is used per project.  With\na setting like repo.or.cz, it is likely that one instance of\ngitweb is serving unrelated projects that have their issue\ntracker at different locations using different \"committags\"\nconvention.  Is the idea to eventually allow enabling/disabling\nelements from the global %committags per repository somehow\n(perhaps not just enable/disable but even overriding patterns or\nparameters)?\n\n> 3. To not split message into many fragments we concatenate strings\n> if possible.\n\nI do not know why \"avoiding splits\" is needed, if it raises \nissues that you need to ask the list about in a message like\nthis...\n"},{"id":"294191","messageId":"200612041133.44816.jnareb@gmail.com","threadId":"43212","inReplyTo":"7virgstmg6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC] gitweb: Add committags support (take 2)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-04T10:33:44Z","receivedAt":"2006-12-04T10:33:44Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>> The subroutine should return either reference to scalar which means\n>> \"do not process\", scalar which means changed available for further\n>> processing, or void (undef) which means no change. In [PATCH 2/3] we\n>> will enable returning also list of elements, each of which could be\n>> reference to scalar or scalar (for example signoff would return three\n>> element list: opening span element as ref, signoff text as scalar,\n>> closing span element as ref).\n> \n> Personally I think that \"list of elements\" should be in the\n> first patch to build the foundation.\n\nO.K. (By the way, that is why I posted this RFC without sending patch.)\nEspecially that in this version this is easy and there are no reasons\nagainst it (in previous version %committag entries had 'islink' field,\nand returned value was \"wrapped\" in format_log_line_html in\n</a>...<a ...>; I think I'll chose pre-output wrapping <a> elements).\n\n>>   our %committags = (\n>>   \t'sha1' => {\n>>   \t\t'pattern' => qr/[0-9a-fA-F]{40}/,\n>>   \t\t'sub' => sub {\n>>   \t\t\tmy $hash_text = shift;\n>>   \t\t\tmy $type = git_get_type($hash_text);\n>>   \t\t\tif ($type) {\n>>   \t\t\t\treturn \\$cgi->a({-href => href(action=>$type, hash=>$hash_text),\n>>   \t\t\t\t                -class => \"text\"}, $hash_text);\n>>   \t\t\t}\n>>   \t\t\treturn undef;\n>>   \t\t},\n>>   \t},\n> \n> It might make sense to do a /[0-9a-f]{8,40}/ pattern, and make\n> sure that the named object exists in the sub (which you already\n> do).  People often write abbreviated commit object name that has\n> a high chance of staying unique during the life of the project.\n\nThat's right. I'll do it. By the way, it might make sense to add \nqr/\\bv[0-9]+(\\.[0-9]+){1,3}(-[a-z]+[0-9]+)\\b/ to match \"version\" tags.\n\nI have one gripe about \"git-cat-file -t\". I'd like it to have \n-q/--quiet, -s/--silent, --hush (or --dont-spew-errors-on-stdout)\nwhich would prohibit writing \"object not found\" errors on stderr\n(and in gitweb case to webserver logs). I know I can use \"git-cat-file -e\"\nto check if object exists, or modify git_get_type subroutine\n\n  # get type of given object\n  sub git_get_type {\n\tmy $hash = shift;\n  \tmy $git_command = git_cmd_str();\n  \n  \topen my $fd, \"-|\",\n  \t\t\"$git_command cat-file -t $hash 2>/dev/null\"\n  \t\tor return '';\n  \tmy $type = <$fd>;\n  \tclose $fd\n  \t\tor return '';\n  \tchomp $type;\n  \treturn $type;\n  }\n\nbut both those solutions means one fork more.\n\n>>   \t'mantis' => {\n>>   \t\t'pattern' => qr/(BUG|FEATURE)\\(\\d+\\)/,\n>>   \t'bugzilla' => {\n>>   \t\t'pattern' => qr/bug\\s+\\d+/,\n> \n> This is not wrong per-se but somehow feels funny.  It feels as\n> if there is a convention that bugzilla bugs are usually spelled\n> in lowercase while mantis bugs are in uppercase, but I do not\n> think you are suggesting that.\n\nThe Mantis pattern is taken from committags support in gitweb-xmms2,\nhttp://git.xmms.se/?p=gitweb-xmms2.git while Bugzilla pattern is\ntaken from Mozilla Bugzilla entries.\n \n> I do not know how this %committags{} is used per project.  With\n> a setting like repo.or.cz, it is likely that one instance of\n> gitweb is serving unrelated projects that have their issue\n> tracker at different locations using different \"committags\"\n> convention.  Is the idea to eventually allow enabling/disabling\n> elements from the global %committags per repository somehow\n> (perhaps not just enable/disable but even overriding patterns or\n> parameters)?\n\nI have thought about putting %committags and @committags before\nloading config file\n  do $GITWEB_CONFIG if -e $GITWEB_CONFIG;\nwhich can load config file depending on the project, but perhaps\nit is too complicated solution.\n\nAnother solution would be to add 'committags' feature, which would\ncontrol at least some of the configurable elements of %committags\nand @committags (of committags support). At minimum control which\ncommittags are turned on, and in what sequence.\n\n>> 3. To not split message into many fragments we concatenate strings\n>> if possible.\n> \n> I do not know why \"avoiding splits\" is needed, if it raises \n> issues that you need to ask the list about in a message like\n> this...\n\n\"Avoiding splits\" is needed first for performance, and second to\navoid situation where pattern would match on the boundary between\ntwo strings in a list of tokens to process.\n\nThe improved version of push_or_append follows:\n\n  sub push_or_append (\\@@) {\n  \tmy $list = shift;\n  \n  \tif (ref $_[0] || ref $list->[-1] || ! @$list) {\n  \t\treturn push @$list, @_;\n  \t} else {\n  \t\tmy $a = pop @$list;\n  \t\tmy $b = shift @_;\n  \n  \t\treturn push @$list, $a . $b, @_;\n  \t}\n  }\n\n\nOr the variant which I wouldn't use ;-P (well, at least not in the\nform below).\n\n  sub push_or_append (\\@@) {\n  \tmy $aref = shift;\n  \tmy @list = @_;\n  \n  \ttry {\n  \t\treturn push @$aref[0..$#$aref-1],\n  \t\t            $aref->[-1] . $list[0],\n  \t\t            @list[1..$#list]\n  \t} catch {\n  \t\treturn push @$aref, @list;\n  \t}\n  }\n\n-- \nJakub Narebski\n"},{"id":"296538","messageId":"7vmz64ortu.fsf@assigned-by-dhcp.cox.net","threadId":"43212","inReplyTo":"200612041133.44816.jnareb@gmail.com","subject":"Re: [RFC] gitweb: Add committags support (take 2)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-04T10:53:17Z","receivedAt":"2006-12-04T10:53:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> I have one gripe about \"git-cat-file -t\". I'd like it to have \n> -q/--quiet, -s/--silent, --hush (or --dont-spew-errors-on-stdout)\n> which would prohibit writing \"object not found\" errors on stderr\n> (and in gitweb case to webserver logs). I know I can use \"git-cat-file -e\"\n> to check if object exists, or modify git_get_type subroutine\n>\n>   # get type of given object\n>   sub git_get_type {\n> \tmy $hash = shift;\n>   \tmy $git_command = git_cmd_str();\n>   \n>   \topen my $fd, \"-|\",\n>   \t\t\"$git_command cat-file -t $hash 2>/dev/null\"\n>   \t\tor return '';\n\nIt is one thing if you tend to randomly throw garbage at this\nfunction and use it to check for object's existence, but I hope\nyou are already checking the user input (which is what $hash is,\nI think, here), and the object is supposed to exist in the\nrepository you are looking at.  In such a case, I think you and\nyour server administrator have right to know about that\nsituation; I do not see why you would want to squelch it.\n\n>> I do not know how this %committags{} is used per project.  With\n>> a setting like repo.or.cz, it is likely that one instance of\n>> gitweb is serving unrelated projects that have their issue\n>> tracker at different locations using different \"committags\"\n>> convention.  Is the idea to eventually allow enabling/disabling\n>> elements from the global %committags per repository somehow\n>> (perhaps not just enable/disable but even overriding patterns or\n>> parameters)?\n>\n> I have thought about putting %committags and @committags before\n> loading config file\n>   do $GITWEB_CONFIG if -e $GITWEB_CONFIG;\n> which can load config file depending on the project, but perhaps\n> it is too complicated solution.\n\nI think you are talking about a gitweb-instance wide\ncustomization, but that's not what I meant.  I meant per-project\nconfiguration where w/git-gui.git and w/git.git are served by\nthe same instance of gitweb but have pointers to different issue\ntrackers.\n\n>>> 3. To not split message into many fragments we concatenate strings\n>>> if possible.\n>> \n>> I do not know why \"avoiding splits\" is needed, if it raises \n>> issues that you need to ask the list about in a message like\n>> this...\n>\n> \"Avoiding splits\" is needed first for performance, and second to\n> avoid situation where pattern would match on the boundary between\n> two strings in a list of tokens to process.\n\nI wouldn't know if constantly splitting and then concatenating\nis faster than just concatenatting once before output without\nbenchmarking, so I'd refrain from talking about performance.\nTwo string case may be a valid concern, though.\n"},{"id":"295901","messageId":"200612041233.19377.jnareb@gmail.com","threadId":"43212","inReplyTo":"7vmz64ortu.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC] gitweb: Add committags support (take 2)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-04T11:33:19Z","receivedAt":"2006-12-04T11:33:19Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia poniedziałek 4. grudnia 2006 11:53, Junio C Hamano napisał:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>> I have one gripe about \"git-cat-file -t\". I'd like it to have \n>> -q/--quiet, -s/--silent, --hush (or --dont-spew-errors-on-stdout)\n>> which would prohibit writing \"object not found\" errors on stderr\n>> (and in gitweb case to webserver logs). I know I can use\n>> \"git-cat-file -e\" to check if object exists, or modify git_get_type\n>> subroutine \n>>\n>>   # get type of given object\n>>   sub git_get_type {\n>> \tmy $hash = shift;\n>>   \tmy $git_command = git_cmd_str();\n>>   \n>>   \topen my $fd, \"-|\",\n>>   \t\t\"$git_command cat-file -t $hash 2>/dev/null\"\n>>   \t\tor return '';\n> \n> It is one thing if you tend to randomly throw garbage at this\n> function and use it to check for object's existence, but I hope\n> you are already checking the user input (which is what $hash is,\n> I think, here), and the object is supposed to exist in the\n> repository you are looking at.  In such a case, I think you and\n> your server administrator have right to know about that\n> situation; I do not see why you would want to squelch it.\n\nI'm sorry, I should mention that this \"quiet\" mode of operation is \nneeded _only_ for committags support, for example by using\ngit_get_type($hash_text, -quiet=>1) in 'sha1' committag subroutine.\n\nYou might have sha1 ids in commit message which no longer point to valid \n(existing) object, for example commit which is result of \n\"git cherry-pick -x\" from no longer existing temporary branch, or commit \nwhich is result of \"git revert\" on a branch which got rebased (but not \nreorganized), or shortened sha1 which is no longer unique. This should \nnot cause errors to be written to webserver log.\n \n\nBy the way, is it better to use anonymous subroutines for committags \nsubs, or use explicit subroutines?\n\n>>> I do not know how this %committags{} is used per project.  With\n>>> a setting like repo.or.cz, it is likely that one instance of\n>>> gitweb is serving unrelated projects that have their issue\n>>> tracker at different locations using different \"committags\"\n>>> convention.  Is the idea to eventually allow enabling/disabling\n>>> elements from the global %committags per repository somehow\n>>> (perhaps not just enable/disable but even overriding patterns or\n>>> parameters)?\n>>\n>> I have thought about putting %committags and @committags before\n>> loading config file\n>>   do $GITWEB_CONFIG if -e $GITWEB_CONFIG;\n>> which can load config file depending on the project, but perhaps\n>> it is too complicated solution.\n> \n> I think you are talking about a gitweb-instance wide\n> customization, but that's not what I meant.  I meant per-project\n> configuration where w/git-gui.git and w/git.git are served by\n> the same instance of gitweb but have pointers to different issue\n> trackers.\n\nYou can always use $ENV{PATH_INFO} in $GITWEB_CONFIG value,\nor check out path_info and/or $cgi->params('p') in the config file.\nOr perhaps other way to set-up per repository config file.\nBut this is a bit complicated to set up.\n\nI don't have definite answer about how configure committags\n(both the committags enabled and sequence of committags, and\ncommittags parameters) per repository. We can use gitweb.committags \nconfig variable for committags enables/sequence, but how configure \ncommittags? gitweb.committag.<name>?\n\nBTW in some cases (e.g. xmms2 projects) issue tracker is common for\nall projects hosted.\n\n>>>> 3. To not split message into many fragments we concatenate strings\n>>>> if possible.\n>>> \n>>> I do not know why \"avoiding splits\" is needed, if it raises \n>>> issues that you need to ask the list about in a message like\n>>> this...\n>>\n>> \"Avoiding splits\" is needed first for performance, and second to\n>> avoid situation where pattern would match on the boundary between\n>> two strings in a list of tokens to process.\n> \n> I wouldn't know if constantly splitting and then concatenating\n> is faster than just concatenatting once before output without\n> benchmarking, so I'd refrain from talking about performance.\n> Two string case may be a valid concern, though.\n\nWith current implementation it is very easy to switch this one and off.\nYou simply either use push, or push_or_append (or make push_or_append\ndo just push).\n\nPrevious (not published) version used $acc variable to concatenate \nstrings, but I think this solution is better (and simpler).\n\n-- \nJakub Narebski\n"},{"id":"297797","messageId":"7v3b7vm9o4.fsf@assigned-by-dhcp.cox.net","threadId":"43212","inReplyTo":"200612041233.19377.jnareb@gmail.com","subject":"Re: [RFC] gitweb: Add committags support (take 2)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-05T01:08:27Z","receivedAt":"2006-12-05T01:08:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> You might have sha1 ids in commit message which no longer point to valid \n> (existing) object, for example commit which is result of \n> \"git cherry-pick -x\" from no longer existing temporary branch, or commit \n> which is result of \"git revert\" on a branch which got rebased (but not \n> reorganized), or shortened sha1 which is no longer unique. This should \n> not cause errors to be written to webserver log.\n\nTrue.\n\n> By the way, is it better to use anonymous subroutines for committags \n> subs, or use explicit subroutines?\n\nI vaguely recall a thread that discussed pros and cons of using\nanonymous subroutines in certain parts of gitweb some time ago\nin which even Merlyn had some comments in, but I do not recall\nthe technical details, sorry.  My gut feeling is that the way\nyou illustrated your example \"our %committags\" definition is\nfine, but it _might_ turn out that it is easier for sites or for\nprojects to customize their own set of rewrite rules if you had\nexplicitly named subroutines available.  I dunno.\n"},{"id":"296025","messageId":"200612061351.02712.jnareb@gmail.com","threadId":"43212","inReplyTo":"7vmz64ortu.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC] gitweb: Add committags support (take 2)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-06T12:51:01Z","receivedAt":"2006-12-06T12:51:01Z","isPatch":false,"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>> I have thought about putting %committags and @committags before\n>> loading config file\n>>   do $GITWEB_CONFIG if -e $GITWEB_CONFIG;\n>> which can load config file depending on the project, but perhaps\n>> it is too complicated solution.\n> \n> I think you are talking about a gitweb-instance wide\n> customization, but that's not what I meant.  I meant per-project\n> configuration where w/git-gui.git and w/git.git are served by\n> the same instance of gitweb but have pointers to different issue\n> trackers.\n\nIt looks like the hardest part with committags support wouldn't be the \nactual implementation of it, but coming with easy and fast way to set \nup those committags.\n\ngitweb-xmms2 project from which the idea of committags support in gitweb \ncame (I think, correct me if I'm wrong) avoids this issue by having \nissue tracker / bug tracker the same for all projects served by single \ngitweb installation; the configuration is site-wide, and there is no \nper project committags configuration.\n\n\nI have imagined the following twofold solution.\n\n1. Make it easier to have per repository gitweb configuration, for\n   example by having gitweb configuration file in GIT_DIR for a project,\n   \"gitweb_conf.perl\" by default:\n\n  our $GITWEB_REPO_CONFIG = $ENV{'GITWEB_REPO_CONFIG'} ||\n  \t\"++GITWEB_CONFIG++\";\n  do \"$projectroot/$project/$GITWEB_REPO_CONFIG\"\n  \tif -e \"$projectroot/$project/$GITWEB_CONFIG\";\n\n2. Put the configuration in config file, using/like %features support.\n   For example gitweb.committags.<committag name> would hold parameters\n   for <committag>. Committags sequence would be given by sequence of\n   entries in config file. Comittags without options would have sole\n   variable entry (which I think is equivalent to being bool variable\n   and having 1 or 'yes' as value).\n\n   The trouble with this approach is not overriding defaults provided\n   while still turning on/off specific committag. And of course the fact\n   that for that we need rather config reader in Perl (Git.pm or\n   gitweb).\n\nWhat do you think about it? Junio? Pasky?\n-- \nJakub Narebski\n"},{"id":"297120","messageId":"7vveko3km4.fsf@assigned-by-dhcp.cox.net","threadId":"43212","inReplyTo":"200612061351.02712.jnareb@gmail.com","subject":"Re: [RFC] gitweb: Add committags support (take 2)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-06T19:11:31Z","receivedAt":"2006-12-06T19:11:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> 1. Make it easier to have per repository gitweb configuration, for\n>    example by having gitweb configuration file in GIT_DIR for a project,\n>    \"gitweb_conf.perl\" by default:\n>\n>   our $GITWEB_REPO_CONFIG = $ENV{'GITWEB_REPO_CONFIG'} ||\n>   \t\"++GITWEB_CONFIG++\";\n>   do \"$projectroot/$project/$GITWEB_REPO_CONFIG\"\n>   \tif -e \"$projectroot/$project/$GITWEB_CONFIG\";\n\nI suspect site administrators would hesitate to have perl\nscripts in repositories that could run arbitrary things as their\n\"side effects\".\n\n> 2. Put the configuration in config file, using/like %features support.\n>    For example gitweb.committags.<committag name> would hold parameters\n>    for <committag>. Committags sequence would be given by sequence of\n>    entries in config file. Comittags without options would have sole\n>    variable entry (which I think is equivalent to being bool variable\n>    and having 1 or 'yes' as value).\n\nI think gitweb.* in $GIT_DIR/config and a config reader in Perl\nare very sensible things to do, and you would need the config\nreader eventually anyway.  The longer we postpone it, the more\nwe risk the temptation to pollute $GIT_DIR/ with the likes of\n\"description\", \"owner\", and \"homepage\", and I do not think we\nwant to make this worse.\n\n> What do you think about it? Junio? Pasky?\n\nIf you are asking Pasky, you should CC him, I think.  I added\nhim to the CC list.\n"},{"id":"294275","messageId":"200612062032.58706.jnareb@gmail.com","threadId":"43212","inReplyTo":"7vveko3km4.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC] gitweb: Add committags support (take 2)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-06T19:32:58Z","receivedAt":"2006-12-06T19:32:58Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>> 2. Put the configuration in config file, using/like %features support.\n>>    For example gitweb.committags.<committag name> would hold parameters\n>>    for <committag>. Committags sequence would be given by sequence of\n>>    entries in config file. Comittags without options would have sole\n>>    variable entry (which I think is equivalent to being bool variable\n>>    and having 1 or 'yes' as value).\n> \n> I think gitweb.* in $GIT_DIR/config and a config reader in Perl\n> are very sensible things to do, and you would need the config\n> reader eventually anyway.  The longer we postpone it, the more\n> we risk the temptation to pollute $GIT_DIR/ with the likes of\n> \"description\", \"owner\", and \"homepage\", and I do not think we\n> want to make this worse.\n\nBy the way, what is the formal structure of the config file? Perhaps\nsomething like the notation used in RFC?\n\nIs it possible (and doesn't crash current git config parser) having\n\n  [gitweb]\n  \tblame = yes\n  \tpickaxe = no\n  \tsnapshot = bzip2\n\n  [gitweb \"committags\"]\n  \tmessage-id = \"http://news.gmane.org/find-root.php?message_id=\"\n  \tmantis = \"http://bugs.or.cz/view.php?id=\"\n  \turl\n  \tsha1\n\nin the config file?\n-- \nJakub Narebski\n"},{"id":"296075","messageId":"7vpsaw3iyp.fsf@assigned-by-dhcp.cox.net","threadId":"43212","inReplyTo":"200612062032.58706.jnareb@gmail.com","subject":"Re: [RFC] gitweb: Add committags support (take 2)","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-06T19:47:10Z","receivedAt":"2006-12-06T19:47:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> By the way, what is the formal structure of the config file? Perhaps\n> something like the notation used in RFC?\n\nIs there an RFC for .INI format?\n\n> Is it possible (and doesn't crash current git config parser) having\n>\n>   [gitweb]\n>   \tblame = yes\n>   \tpickaxe = no\n>   \tsnapshot = bzip2\n>\n>   [gitweb \"committags\"]\n>   \tmessage-id = \"http://news.gmane.org/find-root.php?message_id=\"\n>   \tmantis = \"http://bugs.or.cz/view.php?id=\"\n>   \turl\n>   \tsha1\n>\n> in the config file?\n\nIf you are asking about [gitweb] stanza and (seemingly\noverlapping) [gitweb \"foo\"] stanza, we already have prior\nexamples:\n\n\t[diff]\n        \tcolor = auto\n        [diff \"color\"]\n        \twhitespace = blue reverse\n\nso your example in the above would be legal (the dash in\n'message-id' part might be questionable, though).\n\n\n\n\n"},{"id":"297978","messageId":"200612062135.44658.jnareb@gmail.com","threadId":"43212","inReplyTo":"7vpsaw3iyp.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC] gitweb: Add committags support (take 2)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-06T20:35:44Z","receivedAt":"2006-12-06T20:35:44Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n>> By the way, what is the formal structure of the config file? Perhaps\n>> something like the notation used in RFC?\n> \n> Is there an RFC for .INI format?\n\nI don't think INI is standarized at all. The format of INI files is not \nwell defined. That's why I'd like to know exact format of git INI-like \nconfig file format.\n\nBut what I meant was description of git config file in (used by RFCs)\naugmented Backus-Naur Form (BNF) - RFC 4234.\n \n>> Is it possible (and doesn't crash current git config parser) having\n>>\n>>   [gitweb]\n>>   \tblame = yes\n>>   \tpickaxe = no\n>>   \tsnapshot = bzip2\n>>\n>>   [gitweb \"committags\"]\n>>   \tmessage-id = \"http://news.gmane.org/find-root.php?message_id=\"\n>>   \tmantis = \"http://bugs.or.cz/view.php?id=\"\n>>   \turl\n>>   \tsha1\n>>\n>> in the config file?\n> \n> If you are asking about [gitweb] stanza and (seemingly\n> overlapping) [gitweb \"foo\"] stanza, we already have prior\n> examples:\n> \n> \t[diff]\n>         \tcolor = auto\n>         [diff \"color\"]\n>         \twhitespace = blue reverse\n> \n> so your example in the above would be legal (the dash in\n> 'message-id' part might be questionable, though).\n\nThat's what I was asking for. Thanks for an explanation.\n\n-- \nJakub Narebski\n"}]}