{"thread":{"id":"16231","subject":"[RFC] Configuring (future) committags support in gitweb","startedAt":"2008-11-08T19:07:53Z","lastAt":"2009-02-24T16:33:37Z","messageCount":20,"participants":["Jakub Narebski","Francis Galiegue","Marcel M. Cary","Giuseppe Bilotta","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"95228","messageId":"200811082007.55045.jnareb@gmail.com","threadId":"16231","inReplyTo":null,"subject":"[RFC] Configuring (future) committags support in gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-11-08T19:07:53Z","receivedAt":"2008-11-08T19:07:53Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Francis Galiegue writes\nin \"Need help for migration from CVS to git in one go...\" \n\n> * third: also Bonsai-related; Bonsai can link to Bugzilla by\n> matching (wild guess) /\\b(?:#?)(\\d+)\\b/ and transforming this into\n> http://your.bugzilla.fqdn.here/show_bug.cgi?id=$1. Does gitweb have\n> this built-in? (haven't looked yet) Is this planned, or has it been\n> discussed and been considered not worth the hassle?\n\nHere below there is proposal how the committags support could look like\nfor gitweb _user_, which means how to configure gitweb to use (or do not\nuse) committags, how to configure committags, and how to define new\ncommittags.\n\n\nCommittags are \"tags\" in commit messages, expanded when rendering commit\nmessage, like gitweb now does for (shortened) SHA-1, converting them to\n'object' view link. It should be done in a way to make it easy\nconfigurable, preferably having to configure only variable part, and not\nhaving to write whole replacement rule.\n\nPossible committags include: _BUG(n)_, bug _#n_, _FEATURE(n),\nMessage-Id, plain text URL e.g. _http://repo.or.cz_, spam protecting\nof email addresses, \"rich text formatting\" like *bold* and _underline_,\nsyntax highlighting of signoff lines.\n\n\nI think it would be good idea to use repository config file for\nsetting-up repository-specific committags, and use whatever Perl\nstructure for global configuration. The config language can be\nborrowed from \"drivers\" in gitattributes (`diff' and `merge' drivers).\n\nSo the example configuration could look like this:\n\n [gitweb]\n \tcommittags = sha1 signoff bugzilla\n\n [committag \"bugzilla\"]\n \tmatch = \"\\\\b(?:#?)(\\\\d+)\\\\b\"\n \tlink = \"http://your.bugzilla.fqdn.here/show_bug.cgi?id=$1\"\n\nwhere 'sha1' and 'signoff' are built-in committags, committags are\napplied in the order they are put in gitweb.committags; possible actions\nfor committag driver include:\n * link: replace $match by '__$match__'\n * html: replace $match by '_$html_'\n * text: replace $match by '$text'\nwhere '_a_' means that 'a' is treated as HTML, and is not expanded\nfurther, and 'b' means that it can be further expanded by later\ncommittags, and finally is HTML-escaped (esc_html).\n\n\nWhat do you think about this?\n-- \nJakub Narebski\nPoland\n"},{"id":"95231","messageId":"200811082102.44919.fg@one2team.net","threadId":"16231","inReplyTo":"200811082007.55045.jnareb@gmail.com","subject":"Re: [RFC] Configuring (future) committags support in gitweb","fromName":"Francis Galiegue","fromEmail":"fg@one2team.net","sentAt":"2008-11-08T20:02:44Z","receivedAt":"2008-11-08T20:02:44Z","isPatch":false,"sender":{"key":"fg@one2team.net","avatar":null},"body":"Le Saturday 08 November 2008 20:07:53 Jakub Narebski, vous avez écrit :\n> Francis Galiegue writes\n> in \"Need help for migration from CVS to git in one go...\"\n>\n> > * third: also Bonsai-related; Bonsai can link to Bugzilla by\n> > matching (wild guess) /\\b(?:#?)(\\d+)\\b/ and transforming this into\n> > http://your.bugzilla.fqdn.here/show_bug.cgi?id=$1. Does gitweb have\n> > this built-in? (haven't looked yet) Is this planned, or has it been\n> > discussed and been considered not worth the hassle?\n>\n> Here below there is proposal how the committags support could look like\n> for gitweb _user_, which means how to configure gitweb to use (or do not\n> use) committags, how to configure committags, and how to define new\n> committags.\n>\n\nYour proposal goes much further than my initial question, but I thought I'd \njump in anyway :p\n\n> Committags are \"tags\" in commit messages, expanded when rendering commit\n> message, like gitweb now does for (shortened) SHA-1, converting them to\n> 'object' view link. It should be done in a way to make it easy\n> configurable, preferably having to configure only variable part, and not\n> having to write whole replacement rule.\n>\n> Possible committags include: _BUG(n)_, bug _#n_, _FEATURE(n),\n> Message-Id, plain text URL e.g. _http://repo.or.cz_, spam protecting\n> of email addresses, \"rich text formatting\" like *bold* and _underline_,\n> syntax highlighting of signoff lines.\n>\n\nWhat do you mean with \"not having to write whole replacement rule\"?\n\n> I think it would be good idea to use repository config file for\n> setting-up repository-specific committags, and use whatever Perl\n> structure for global configuration. The config language can be\n> borrowed from \"drivers\" in gitattributes (`diff' and `merge' drivers).\n>\n> So the example configuration could look like this:\n>\n> [gitweb]\n> \tcommittags = sha1 signoff bugzilla\n>\n> [committag \"bugzilla\"]\n> \tmatch = \"\\\\b(?:#?)(\\\\d+)\\\\b\"\n> \tlink = \"http://your.bugzilla.fqdn.here/show_bug.cgi?id=$1\"\n>\n> where 'sha1' and 'signoff' are built-in committags, committags are\n> applied in the order they are put in gitweb.committags;\n\nI don't understand what the \"signoff\" builtin is : is that a link to see only \ncommits \"Signed-off-by:\" a particular person?\n\nIf so, might I suggest that an \"alt\" tells \"Only show commits signed off by \nthis person\"?\n\nAnd also, what about the sha1 builtin? AFAIK, a SHA1 can point to a commit, a \ntree, and others... In fact, it points to any of these right now, but how \nwould you tell apart these different SHA1s in a commit message? The only \nobvious use I see for it is the builtin \"Revert ...\" commit message, that the \ncommiter _can_ override...\n\nOr would that be:\n\nmy $sha1_re = qr/[a-z[0-9]{40}/;\n\n/(?:(?i:commit\\s+))?\\b($sha1_re)\\b/ => [link to commit $1]\n/(?:(?i:tree\\s+))\\b($sha1_re)\\b)/ => [link to tree $1]\n/(?:(?:tag\\s+))\\b($sha1_re)\\b)/ => [link to tag $1]\n\nFinally, is there any reason to think that a sha1 or signoff committag will \never need to be overriden in some way?\n\n> possible actions \n> for committag driver include:\n> * link: replace $match by '__$match__'\n> * html: replace $match by '_$html_'\n> * text: replace $match by '$text'\n> where '_a_' means that 'a' is treated as HTML, and is not expanded\n> further, and 'b' means that it can be further expanded by later\n> committags, and finally is HTML-escaped (esc_html).\n>\n\nWhat use do you see for the html match? Just asking...\n\nAnd I don't see what you '_a_' and '_b_' are about...\n\n-- \nfge\n"},{"id":"95241","messageId":"200811082335.49505.jnareb@gmail.com","threadId":"16231","inReplyTo":"200811082102.44919.fg@one2team.net","subject":"Re: [RFC] Configuring (future) committags support in gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-11-08T22:35:48Z","receivedAt":"2008-11-08T22:35:48Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia sobota 8. listopada 2008 21:02, Francis Galiegue napisał:\n> Le Saturday 08 November 2008 20:07:53 Jakub Narebski, vous avez écrit :\n>> Francis Galiegue writes\n>> in \"Need help for migration from CVS to git in one go...\"\n>>\n>>> * third: also Bonsai-related; Bonsai can link to Bugzilla by\n>>> matching (wild guess) /\\b(?:#?)(\\d+)\\b/ and transforming this into\n>>> http://your.bugzilla.fqdn.here/show_bug.cgi?id=$1. Does gitweb have\n>>> this built-in? (haven't looked yet) Is this planned, or has it been\n>>> discussed and been considered not worth the hassle?\n[...]\n\n>> Committags are \"tags\" in commit messages, expanded when rendering commit\n>> message, like gitweb now does for (shortened) SHA-1, converting them to\n>> 'object' view link. It should be done in a way to make it easy\n>> configurable, preferably having to configure only variable part, and not\n>> having to write whole replacement rule.\n>>\n>> Possible committags include: _BUG(n)_, bug _#n_, _FEATURE(n),\n>> Message-Id, plain text URL e.g. _http://repo.or.cz_, spam protecting\n>> of email addresses, \"rich text formatting\" like *bold* and _underline_,\n>> syntax highlighting of signoff lines.\n>>\n> \n> What do you mean with \"not having to write whole replacement rule\"?\n\nLike in example with 'link' rule, not having to write whole\n$&\n(or something like that).\n\n>> I think it would be good idea to use repository config file for\n>> setting-up repository-specific committags, and use whatever Perl\n>> structure for global configuration. The config language can be\n>> borrowed from \"drivers\" in gitattributes (`diff' and `merge' drivers).\n>>\n>> So the example configuration could look like this:\n>>\n>> [gitweb]\n>> \tcommittags = sha1 signoff bugzilla\n>>\n>> [committag \"bugzilla\"]\n>> \tmatch = \"\\\\b(?:#?)(\\\\d+)\\\\b\"\n>> \tlink = \"http://your.bugzilla.fqdn.here/show_bug.cgi?id=$1\"\n>>\n>> where 'sha1' and 'signoff' are built-in committags, committags are\n>> applied in the order they are put in gitweb.committags;\n> \n> I don't understand what the \"signoff\" builtin is : is that a link to see only \n> commits \"Signed-off-by:\" a particular person?\n\nCommittags doesn't need to be replaced by links. In this case I meant\nhere using 'signoff' class for Signed-off-by: (and the like) lines, by\nwrapping it in '' ... ''.\n\n> And also, what about the sha1 builtin? AFAIK, a SHA1 can point to a commit, a \n> tree, and others... In fact, it points to any of these right now, but how \n> would you tell apart these different SHA1s in a commit message? The only \n> obvious use I see for it is the builtin \"Revert ...\" commit message, that the \n> commiter _can_ override...\n\nSHA1 (or shortened SHA1 from 8 charasters to 40 characters, or to be\neven more exact something that looks like SHA1) is replaced by link\nto 'object' view, which in turn finds type of object and _redirect_\nto proper view, be it 'commit' (most frequent), 'tag', 'blob' or 'tree'.\n\nWe could have used instead gitweb link with 'h' (hash) parameter, but\nwithout 'a' (action) parameter, which currently finds type of object\nand _uses_ correct view...\n \n> Finally, is there any reason to think that a sha1 or signoff committag will \n> ever need to be overriden in some way?\n\nOne might not want to link SHA1, for example if there are lots of false\npositives because of commit message conventions or something, or refine\n'signoff' committag to use different styles for different types of\nsignoff: Signed-off-by, Acked-by, Tested-by, other. Having explicit\n'signoff' committag allows us also to put some committags _after_ it,\nfor example SPAM-protection of emails, or add some committag before\n'sha1' to filter out some SHA1 match false positives.\n \n>> possible actions \n>> for committag driver include:\n>> * link: replace $match by '__$match__'\n>> * html: replace $match by '_$html_'\n>> * text: replace $match by '$text'\n>> where '_a_' means that 'a' is treated as HTML, and is not expanded\n>> further, and 'b' means that it can be further expanded by later\n>> committags, and finally is HTML-escaped (esc_html).\n>>\n> \n> What use do you see for the html match? Just asking...\n\nFor example 'signoff' committag... well, it is not exactly pure \"html\"\nbut rather something like template.\n\n [committag \"signoff\"]\n \tmatch = \"(?i)^ *(signed[ \\\\-]off[ \\\\-]by[ :]|acked[ \\\\-]by[ :]|cc[ :])\"\n \ttempl = \"{}$1{}\"\n\nOr simpler\n\n [committag \"signoff\"]\n \tmatch = \"(?i)^ *(signed[ \\\\-]off[ \\\\-]by[ :]|acked[ \\\\-]by[ :]|cc[ :])\"\n \tclass = signoff\n\n> And I don't see what you '_a_' and '_b_' are about...\n\nFor example in link match, the text of the link can be further refined\nby committags later in sequence.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"95242","messageId":"200811090027.14343.fg@one2team.net","threadId":"16231","inReplyTo":"200811082335.49505.jnareb@gmail.com","subject":"Re: [RFC] Configuring (future) committags support in gitweb","fromName":"Francis Galiegue","fromEmail":"fg@one2team.net","sentAt":"2008-11-08T23:27:14Z","receivedAt":"2008-11-08T23:27:14Z","isPatch":false,"sender":{"key":"fg@one2team.net","avatar":null},"body":"Le Saturday 08 November 2008 23:35:48 Jakub Narebski, vous avez écrit :\n\n> >\n> > What do you mean with \"not having to write whole replacement rule\"?\n>\n> Like in example with 'link' rule, not having to write whole\n> $&\n> (or something like that).\n>\n\nOK, good one.\n\n> >> I think it would be good idea to use repository config file for\n> >> setting-up repository-specific committags, and use whatever Perl\n> >> structure for global configuration. The config language can be\n> >> borrowed from \"drivers\" in gitattributes (`diff' and `merge' drivers).\n> >>\n> >> So the example configuration could look like this:\n> >>\n> >> [gitweb]\n> >> \tcommittags = sha1 signoff bugzilla\n> >>\n> >> [committag \"bugzilla\"]\n> >> \tmatch = \"\\\\b(?:#?)(\\\\d+)\\\\b\"\n> >> \tlink = \"http://your.bugzilla.fqdn.here/show_bug.cgi?id=$1\"\n> >>\n> >> where 'sha1' and 'signoff' are built-in committags, committags are\n> >> applied in the order they are put in gitweb.committags;\n> >\n> > I don't understand what the \"signoff\" builtin is : is that a link to see\n> > only commits \"Signed-off-by:\" a particular person?\n>\n> Committags doesn't need to be replaced by links. In this case I meant\n> here using 'signoff' class for Signed-off-by: (and the like) lines, by\n> wrapping it in '' ... ''.\n>\n\nWell, this would also mean to update gitweb.css, wouldn't it?\n\n> > And also, what about the sha1 builtin? AFAIK, a SHA1 can point to a\n> > commit, a tree, and others... In fact, it points to any of these right\n> > now, but how would you tell apart these different SHA1s in a commit\n> > message? The only obvious use I see for it is the builtin \"Revert ...\"\n> > commit message, that the commiter _can_ override...\n>\n> SHA1 (or shortened SHA1 from 8 charasters to 40 characters, or to be\n> even more exact something that looks like SHA1) is replaced by link\n> to 'object' view, which in turn finds type of object and _redirect_\n> to proper view, be it 'commit' (most frequent), 'tag', 'blob' or 'tree'.\n>\n> We could have used instead gitweb link with 'h' (hash) parameter, but\n> without 'a' (action) parameter, which currently finds type of object\n> and _uses_ correct view...\n>\n\nOK, you lost me somewhat.\n\nWhat I understand is that right now, the SHA1 links are \"pre-processed\" by \ngitweb so that the 'a' parameter is correct, right?\n\nOut of curiosity, I just went to the kernel git repository (I don't know the \ngit version that git.kernel.org uses offhand) and altered the 'a' parameter \nto something which is not even an 'a' command at all: 500...\n\nHowever, if I try a VALID 'a' command with an \"irrelevant\" 'h' parameter, it \nacts quite funny: it just looks like it wants to try the closest match, but \ntakes some time figuring it out... Sometimes to something relevant, sometimes \nto nothing really relevant. See for instance [1], in which 'a' was \noriginally \"commit\".\n\nOw.\n\n [1]\nhttp://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.27.y.git;a=tag;h=788a5f3f70e2a9c46020bdd3a195f2a866441c5d\n\n> > Finally, is there any reason to think that a sha1 or signoff committag\n> > will ever need to be overriden in some way?\n>\n> One might not want to link SHA1, for example if there are lots of false\n> positives because of commit message conventions or something, or refine\n> 'signoff' committag to use different styles for different types of\n> signoff: Signed-off-by, Acked-by, Tested-by, other. Having explicit\n> 'signoff' committag allows us also to put some committags _after_ it,\n> for example SPAM-protection of emails, or add some committag before\n> 'sha1' to filter out some SHA1 match false positives.\n>\n\nHmmm, so this means you'd want to make styles customizable somewhat (signoff). \nIn fact, what you really want is span for CSS! Then why not, just, making a \ndocument to say \"This is what you can do with CSS for gitweb\", and say \"these \nare the available CSS tags\", and then be done with it?\n\nI mean, when comes the day that someone will WANT other spans to be defined, \nbadly, it's not like it will be unheard of, won't it? \n\n>\n> > And I don't see what you '_a_' and '_b_' are about...\n>\n> For example in link match, the text of the link can be further refined\n> by committags later in sequence.\n\nI still don't get it. Can you give an example?\n\n[personal thoughts: it would be really, really nice if, somewhat, gitweb.perl \nwere splitted somewhat into different modules, and ideally use more \nof \"what's out there on CPAN\". I'm convinced that some CPAN modules would be \nof GREAT help to gitweb, as well as I'm convinced that not many people out \nthere use Windows to run gitweb anyway :p]\n\n-- \nfge\n"},{"id":"95244","messageId":"200811090125.12625.jnareb@gmail.com","threadId":"16231","inReplyTo":"200811090027.14343.fg@one2team.net","subject":"Re: [RFC] Configuring (future) committags support in gitweb","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-11-09T00:25:11Z","receivedAt":"2008-11-09T00:25:11Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia niedziela 9. listopada 2008 00:27, Francis Galiegue napisał:\n> Le Saturday 08 November 2008 23:35:48 Jakub Narebski, vous avez écrit :\n\n>>> I don't understand what the \"signoff\" builtin is : is that a link to see\n>>> only commits \"Signed-off-by:\" a particular person?\n>>\n>> Committags doesn't need to be replaced by links. In this case I meant\n>> here using 'signoff' class for Signed-off-by: (and the like) lines, by\n>> wrapping it in '' ... ''.\n> \n> Well, this would also mean to update gitweb.css, wouldn't it?\n\nNot necessary. Please remember that you can configure gitweb to either\nuse _alternate_ stylesheet (instead of provided gitweb.css), or use\n_additional_ stylesheet (for example gitweb-commit.css in addition to\ngitweb.css)\n \n>>> And also, what about the sha1 builtin? AFAIK, a SHA1 can point to a\n>>> commit, a tree, and others... In fact, it points to any of these right\n>>> now, but how would you tell apart these different SHA1s in a commit\n>>> message? The only obvious use I see for it is the builtin \"Revert ...\"\n>>> commit message, that the commiter _can_ override...\n>>\n>> SHA1 (or shortened SHA1 from 8 charasters to 40 characters, or to be\n>> even more exact something that looks like SHA1) is replaced by link\n>> to 'object' view, which in turn finds type of object and _redirect_\n>> to proper view, be it 'commit' (most frequent), 'tag', 'blob' or 'tree'.\n>>\n>> We could have used instead gitweb link with 'h' (hash) parameter, but\n>> without 'a' (action) parameter, which currently finds type of object\n>> and _uses_ correct view...\n> \n> OK, you lost me somewhat.\n\nI'm sorry about that. Perhaps I should use only one mechanism.\n \n> What I understand is that right now, the SHA1 links are \"pre-processed\" by \n> gitweb so that the 'a' parameter is correct, right?\n\nNo, they are 'post-processed': finding correct action is left to\n_after_ you click on the link (it is more natural, and helps\nperformance).\n\nIf you don't know action for given SHA-1 you can use either\n\n http://example.com/gitweb.cgi?a=object;h=deadbeef\n\nwhich finds correct type (for example 'commit') and redirects using\nHTTP 302 Found redirection to\n\n http://example.com/gitweb.cgi?a=commit;h=deadbeef\n\nThis way is used bu SHA-1 committag in commit messages.\n\n\nAlternate solution (meant for bugtrackers), done by another author,\nis to simply skip action ('a') parameter:\n\n http://example.com/gitweb.cgi?h=deadbeef\n\nand then gitweb finds type of object and act accordingly (without\nredirect to correct view).\n\n[...]\n>>> Finally, is there any reason to think that a sha1 or signoff committag\n>>> will ever need to be overriden in some way?\n>>\n>> One might not want to link SHA1, for example if there are lots of false\n>> positives because of commit message conventions or something, or refine\n>> 'signoff' committag to use different styles for different types of\n>> signoff: Signed-off-by, Acked-by, Tested-by, other. Having explicit\n>> 'signoff' committag allows us also to put some committags _after_ it,\n>> for example SPAM-protection of emails, or add some committag before\n>> 'sha1' to filter out some SHA1 match false positives.\n> \n> Hmmm, so this means you'd want to make styles customizable somewhat (signoff). \n> In fact, what you really want is span for CSS! Then why not, just, making a \n> document to say \"This is what you can do with CSS for gitweb\", and say \"these \n> are the available CSS tags\", and then be done with it?\n> \n> I mean, when comes the day that someone will WANT other spans to be defined, \n> badly, it's not like it will be unheard of, won't it? \n\nErrr... I don't understand.\n\nThe examples perhaps are not the best. One would be for example to use\ndifferent class (different style) for Signed-off-by signoff (which\ndenotes signing Certificate of Origin), and the rest of (informative)\nsignoff:\n\n [gitweb]\n \tcommittags = sha1 signoff_signed signoff\n\nAnother example would be to add SPAM-protection of emails, for example\nreplacing 'user@example.com' by 'user AT example DOT com', or something\nmore advanced. This have to be used _after_ signoff, because otherwise\nregexp could have difficulties matching mangled email\n\n [gitweb]\n \tcommittags = sha1 signoff mailto\n\n>>\n>>> And I don't see what you '_a_' and '_b_' are about...\n>>\n>> For example in link match, the text of the link can be further refined\n>> by committags later in sequence.\n> \n> I still don't get it. Can you give an example?\n\nFor example signoff line\n\n Signed-off-by: A U Thor \n\nwould be replaced by\n\n {}Signed-off-by: A U Thor {}\n \nwhere parts inside {...} is HTML code, and should be not expanded\nfurther, and parts outside it could be expanded further by following\nlower priority committags (like anti-SPAM for emails), and have to be\nfinally HTML escaped (like '<' and '>' in email in signoff).\n\n\n.....................................................................\n\n> [personal thoughts: it would be really, really nice if, somewhat, gitweb.perl \n> were splitted somewhat into different modules, and ideally use more \n> of \"what's out there on CPAN\". I'm convinced that some CPAN modules would be \n> of GREAT help to gitweb, as well as I'm convinced that not many people out \n> there use Windows to run gitweb anyway :p]\n\nFirst, having gitweb in (almost) one piece makes for easier installation.\nBut there are plans to have gitweb use Git.pm or future Got::Repo and\nfriends. I'm not sure about further splitting...\n\nSecond, we cannot in good faith use CPAN modules which cannot be found\nin standard Perl distributions, or at least in some trusted extras\npackage (application) repositories, as gitweb is sometimes run on\nmachines with tight security (think kernel.org for example) where you\ncannot simply ask admin to install some third-party alpha-version CPAN\nmodule.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"105166","messageId":"499AD871.8000808@oak.homeunix.org","threadId":"16231","inReplyTo":"200811082335.49505.jnareb@gmail.com","subject":"Re: [RFC] Configuring (future) committags support in gitweb, especially bug linking","fromName":"Marcel M. Cary","fromEmail":"marcel@oak.homeunix.org","sentAt":"2009-02-17T15:32:01Z","receivedAt":"2009-02-17T15:32:01Z","isPatch":false,"sender":{"key":"marcel@oak.homeunix.org","avatar":"https://gravatar.com/avatar/2bb524e4f383167b7e256bb93256c88353748d9873c34cde0fd461f1165baa0f?d=mp&s=160"},"body":"I'm interested in cross-linking bug references in commit messages to a\nbug tracking system. I started tinkering a couple weeks ago and am\nfinally understanding that committags encompass this functionality.\n(From the subject line I first understood \"tags\" to mean git tags rather\nthan commit message munging.)\n\nIs the committags idea still under active development?\n\nI'd be happy to share what I have, which is for bug linking only, but\nvery little code would apply to the more general concept of committags.\n Here are some ideas that might apply...\n\n\nTwo regexes would make it easier to configure a driver without needing\nlook-ahead and look-behind assertions. For example, if you want to\nmatch non-negative integers but only in the context of a Resolves-bug\nheader:\n\n Resolves-bug: 1234, 1235\n\nWith two regexes you can write:\n\n /^Resolves-bug: \\d+(, \\d+)*/\n /\\d+/\n\nBut with only one, you'd have to write:\n\n /(?<=^Resolves-bug: (\\d+, )*)(\\d+)/\n\nThe need for a lookbehind assertion means I need to stop at perlreref to\nlookup syntax. Hrm... and with testing I see that it's worse than that:\n\n $ perl -wpe 's/(?<=^Resolves-bug: (\\d+, )*)(\\d+)/[$2]/g'\n Variable length lookbehind not implemented in regex; marked by <--\n HERE in m/(?<=^Resolves-bug: (\\d+, )*)(\\d+) <-- HERE / at -e line 1.\n\nI guess it can't be done even with the look-behind assertion.\n\n\nI got the two-regex idea from a spec I ran across while evaluating\nSubversion:\n\nhttp://guest:@tortoisesvn.tigris.org/svn/tortoisesvn/trunk/doc/issuetrackers.txt\n\nIf there is interest in BTS integration beyond gitweb, for example in\ngit-gui, gitk, or the Windows UIs, then perhaps this spec would be worth\nconsidering. It covers more than just hyperlinking. It also considers\nissues like how to draw the form field for a bug ID as part of a commit\nmessage form, how to validate that form field, and then how to munge the\nlog message to include the entered ID. Some details, like using a\nnewline to separate the two regexes, might be more awkward for Git than\nSubversion.\n\n\nI like the idea of allowing a regex writer -- a gitweb admin or a\nrepository owner -- to ignore issues regarding HTML escaping. For\nexample, I'd rather not have   in the regex. And I don't want the\nreplacement to have to escape \"&\" in a query string. That's a strength\nof not having to write the whole link replacement rule. And I think\nhyperlinking will be one of the most common uses of this committag\nfeature, so it's worth special support.\n\nIn the case of false positives, it might also be helpful to have a title\nattribute that explains the committag's interpretation of the text.\n\nI also like the idea of giving the admin full control to specify a Perl\nfunction of some sort, which might go as far as looking up bug summaries\nfor the \"title\" attribute or adding JS to fetch it via AJAX on\nmouse-over. But I doubt I would bother with that myself.\n\n\nAppealing as it is, the use of '$1' in my replacements didn't work for me:\n\n $ perl -wpe '$reg = \"(\\\\d+)\"; $rep = \".\\$1.\"; s/$reg/$rep/g'\n 123\n .$1.\n\nI think usage of capturing parenthesis is important, even with two\nregexes, because it makes it easier to specify link text that's broader\nthan the data that goes in the URL. Specifically, I wanted to be able\nto produce HTML like this, with the hash mark hyperlinked but not used\nin the URL:\n\n #123\n\nI guess that's just my aesthetic. To support that, my code calls\nsprintf with $&, $1, $2, ... $9, and that particualr replacement URL\nuses %2$s.\n\n\nI'm concerned about the composition of these committag drivers. In\nother words, will it be hard for the configurer to manage interactions\nbetween committag drivers? To choose a sane order, will I have to\nunderstand the implementation details of each committag driver?\n\nPerhaps a simpler alternative would be to let at most one driver process\na given snippet of text, forbidding nesting of replacements. (If I\nunderstand Junio's suggestion to use a list of strings and refs,\nnon-nesting overlaps are already not supported.) If all replacements\nwere hyperlinks -- and I expect that to be the common case -- they\nwouldn't be nestable anyway. I wouldn't see it as a huge loss for the\nnesting examples I can think of: Separate rules for span around S-o-b\nand linking or obfuscation of email could be combined into one... A\nrule to shade text quoted email-style with leading angle brackets could\njust clobber any further processing of that text. And it might simplify\nthe code and testing of it quite a bit.\n\nIf committags do turn out to support nesting, perhaps it would make\nsense to stratify the ordering so that it's clear whether a particular\ndriver takes as input HTML vs. text and outputs HTML vs. text. (For\nexample, weak email obfuscation might be text -> text.) I guess to\nstrictly honor the input and output types of a driver, the text -> html\ndrivers still have to be expanded in a single pass.\n\n\nA few ideas for drivers that I don't think have been mentioned yet:\n\n* Wiki page names, like to [[Feature Documentation]]. These are notable\nbecause they tend to contain punctuation that get HTML-escaped, like\nquotes and ampersands.\n\n* Links to gitweb itself, such as 123abc:file.txt and HEAD:file.txt. I\nguess the current hash linking sort of does the first case except that\nyou have to get the hash of the blob instead of using the commit hash,\nand the current hash linking wouldn't reveal the filename until after\nyou click, nor when viewing textual log messages. I'm not sure whether\nspecial support for linking to multi-commit diffs or other object types\nwould be as helpful.\n\nMarcel\n\n\nJakub Narebski wrote:\n> Dnia sobota 8. listopada 2008 21:02, Francis Galiegue napisał:\n>> Le Saturday 08 November 2008 20:07:53 Jakub Narebski, vous avez écrit :\n>>> Francis Galiegue writes\n>>> in \"Need help for migration from CVS to git in one go...\"\n>>>\n>>>> * third: also Bonsai-related; Bonsai can link to Bugzilla by\n>>>> matching (wild guess) /\\b(?:#?)(\\d+)\\b/ and transforming this into\n>>>> http://your.bugzilla.fqdn.here/show_bug.cgi?id=$1. Does gitweb have\n>>>> this built-in? (haven't looked yet) Is this planned, or has it been\n>>>> discussed and been considered not worth the hassle?\n> [...]\n>\n>>> Committags are \"tags\" in commit messages, expanded when rendering commit\n>>> message, like gitweb now does for (shortened) SHA-1, converting them to\n>>> 'object' view link. It should be done in a way to make it easy\n>>> configurable, preferably having to configure only variable part, and not\n>>> having to write whole replacement rule.\n>>>\n>>> Possible committags include: _BUG(n)_, bug _#n_, _FEATURE(n),\n>>> Message-Id, plain text URL e.g. _http://repo.or.cz_, spam protecting\n>>> of email addresses, \"rich text formatting\" like *bold* and _underline_,\n>>> syntax highlighting of signoff lines.\n>>>\n>> What do you mean with \"not having to write whole replacement rule\"?\n>\n> Like in example with 'link' rule, not having to write whole\n> $&\n> (or something like that).\n>\n>>> I think it would be good idea to use repository config file for\n>>> setting-up repository-specific committags, and use whatever Perl\n>>> structure for global configuration. The config language can be\n>>> borrowed from \"drivers\" in gitattributes (`diff' and `merge' drivers).\n>>>\n>>> So the example configuration could look like this:\n>>>\n>>> [gitweb]\n>>> committags = sha1 signoff bugzilla\n>>>\n>>> [committag \"bugzilla\"]\n>>> match = \"\\\\b(?:#?)(\\\\d+)\\\\b\"\n>>> link = \"http://your.bugzilla.fqdn.here/show_bug.cgi?id=$1\"\n>>>\n>>> where 'sha1' and 'signoff' are built-in committags, committags are\n>>> applied in the order they are put in gitweb.committags;\n>> I don't understand what the \"signoff\" builtin is : is that a link to see only\n>> commits \"Signed-off-by:\" a particular person?\n>\n> Committags doesn't need to be replaced by links. In this case I meant\n> here using 'signoff' class for Signed-off-by: (and the like) lines, by\n> wrapping it in '' ... ''.\n>\n>> And also, what about the sha1 builtin? AFAIK, a SHA1 can point to a commit, a\n>> tree, and others... In fact, it points to any of these right now, but how\n>> would you tell apart these different SHA1s in a commit message? The only\n>> obvious use I see for it is the builtin \"Revert ...\" commit message, that the\n>> commiter _can_ override...\n>\n> SHA1 (or shortened SHA1 from 8 charasters to 40 characters, or to be\n> even more exact something that looks like SHA1) is replaced by link\n> to 'object' view, which in turn finds type of object and _redirect_\n> to proper view, be it 'commit' (most frequent), 'tag', 'blob' or 'tree'.\n>\n> We could have used instead gitweb link with 'h' (hash) parameter, but\n> without 'a' (action) parameter, which currently finds type of object\n> and _uses_ correct view...\n>\n>> Finally, is there any reason to think that a sha1 or signoff committag will\n>> ever need to be overriden in some way?\n>\n> One might not want to link SHA1, for example if there are lots of false\n> positives because of commit message conventions or something, or refine\n> 'signoff' committag to use different styles for different types of\n> signoff: Signed-off-by, Acked-by, Tested-by, other. Having explicit\n> 'signoff' committag allows us also to put some committags _after_ it,\n> for example SPAM-protection of emails, or add some committag before\n> 'sha1' to filter out some SHA1 match false positives.\n>\n>>> possible actions\n>>> for committag driver include:\n>>> * link: replace $match by '__$match__'\n>>> * html: replace $match by '_$html_'\n>>> * text: replace $match by '$text'\n>>> where '_a_' means that 'a' is treated as HTML, and is not expanded\n>>> further, and 'b' means that it can be further expanded by later\n>>> committags, and finally is HTML-escaped (esc_html).\n>>>\n>> What use do you see for the html match? Just asking...\n>\n> For example 'signoff' committag... well, it is not exactly pure \"html\"\n> but rather something like template.\n>\n> [committag \"signoff\"]\n> match = \"(?i)^ *(signed[ \\\\-]off[ \\\\-]by[ :]|acked[ \\\\-]by[ :]|cc[ :])\"\n> templ = \"{}$1{}\"\n>\n> Or simpler\n>\n> [committag \"signoff\"]\n> match = \"(?i)^ *(signed[ \\\\-]off[ \\\\-]by[ :]|acked[ \\\\-]by[ :]|cc[ :])\"\n> class = signoff\n>\n>> And I don't see what you '_a_' and '_b_' are about...\n>\n> For example in link match, the text of the link can be further refined\n> by committags later in sequence.\n>\n> --\n> Jakub Narebski\n> Poland\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"105264","messageId":"1234926043-7471-1-git-send-email-marcel@oak.homeunix.org","threadId":"16231","inReplyTo":"499AD871.8000808@oak.homeunix.org","subject":"[PATCH RFC 1/2] gitweb: Fix warnings with override permitted but no repo override","fromName":"Marcel M. Cary","fromEmail":"marcel@oak.homeunix.org","sentAt":"2009-02-18T03:00:42Z","receivedAt":"2009-02-18T03:00:42Z","isPatch":true,"sender":{"key":"marcel@oak.homeunix.org","avatar":"https://gravatar.com/avatar/2bb524e4f383167b7e256bb93256c88353748d9873c34cde0fd461f1165baa0f?d=mp&s=160"},"body":"When a feature like \"blame\" is permitted to be overridden in the\nrepository configuration but it is not actually set in the\nrepository, a warning is emitted due to the undefined value\nof the repository configuration, even though it's a perfectly\nnormal condition.\n\nThe warning is grounds for test failure in the gitweb test script,\nso it causes some new feature tests of mine to fail.\n\nThis patch prevents warning and adds a test case to exercise it.\n\nSigned-off-by: Marcel M. Cary \n---\n\nHere's a small patch I put together while tinkering with bug hyperlinking.\nDoes this look reasonable?\n\nMarcel\n\n\n gitweb/gitweb.perl | 8 +++++---\n t/t9500-gitweb-standalone-no-errors.sh | 5 +++++\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 7c48181..653f0be 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -402,13 +402,13 @@ sub feature_bool {\n \tmy $key = shift;\n \tmy ($val) = git_get_project_config($key, '--bool');\n \n-\tif ($val eq 'true') {\n+\tif (!defined $val) {\n+\t\treturn ($_[0]);\n+\t} elsif ($val eq 'true') {\n \t\treturn (1);\n \t} elsif ($val eq 'false') {\n \t\treturn (0);\n \t}\n-\n-\treturn ($_[0]);\n }\n \n sub feature_snapshot {\n@@ -1978,6 +1978,8 @@ sub git_get_project_config {\n \t\t$config_file = \"$git_dir/config\";\n \t}\n \n+\treturn undef if (!defined $config{\"gitweb.$key\"});\n+\n \t# ensure given type\n \tif (!defined $type) {\n \t\treturn $config{\"gitweb.$key\"};\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 7c6f70b..559045e 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -662,6 +662,11 @@ cat >>gitweb_config.perl <\n---\n\nAnd here's another.\n\nMarcel\n\n\n gitweb/gitweb.perl | 12 +++++-------\n 1 files changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 653f0be..51b7f56 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1384,13 +1384,11 @@ sub format_log_line_html {\n \tmy $line = shift;\n \n \t$line = esc_html($line, -nbsp=>1);\n-\tif ($line =~ m/\\b([0-9a-fA-F]{8,40})\\b/) {\n-\t\tmy $hash_text = $1;\n-\t\tmy $link =\n-\t\t\t$cgi->a({-href => href(action=>\"object\", hash=>$hash_text),\n-\t\t\t -class => \"text\"}, $hash_text);\n-\t\t$line =~ s/$hash_text/$link/;\n-\t}\n+\t$line =~ s{\\b([0-9a-fA-F]{8,40})\\b}{\n+\t\treturn $cgi->a({-href => href(action=>\"object\", hash=>$1),\n+\t\t\t\t\t -class => \"text\"}, $1);\n+\t}eg;\n+\n \treturn $line;\n }\n \n-- \n1.6.1\n"},{"id":"105267","messageId":"200902180438.55081.jnareb@gmail.com","threadId":"16231","inReplyTo":"499AD871.8000808@oak.homeunix.org","subject":"Re: [RFC] Configuring (future) committags support in gitweb, especially bug linking","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-18T03:38:53Z","receivedAt":"2009-02-18T03:38:53Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 17 Feb 2009, Marcel M. Cary wrote:\n\n> I'm interested in cross-linking bug references in commit messages to a\n> bug tracking system. I started tinkering a couple weeks ago and am\n> finally understanding that committags encompass this functionality.\n> (From the subject line I first understood \"tags\" to mean git tags rather\n> than commit message munging.)\n\nWhat would you name this feature, then?\n\n> \n> Is the committags idea still under active development?\n\nWell, it is in my todo list, rather further on...\n\n[...]\n> Two regexes would make it easier to configure a driver without needing\n> look-ahead and look-behind assertions. For example, if you want to\n> match non-negative integers but only in the context of a Resolves-bug\n> header:\n> \n> Resolves-bug: 1234, 1235\n\n[...]\n> I got the two-regex idea from a spec I ran across while evaluating\n> Subversion:\n> \n> http://guest:@tortoisesvn.tigris.org/svn/tortoisesvn/trunk/doc/issuetrackers.txt\n\nYou don't need multiple regexps for that, and in above example it is\nused _single_ regexp; only with more than one catching group.\n\n> I like the idea of allowing a regex writer -- a gitweb admin or a\n> repository owner -- to ignore issues regarding HTML escaping. For\n> example, I'd rather not have   in the regex. And I don't want the\n> replacement to have to escape \"&\" in a query string. That's a strength\n> of not having to write the whole link replacement rule. And I think\n> hyperlinking will be one of the most common uses of this committag\n> feature, so it's worth special support.\n\n[...]\n> I'm concerned about the composition of these committag drivers. In\n> other words, will it be hard for the configurer to manage interactions\n> between committag drivers? To choose a sane order, will I have to\n> understand the implementation details of each committag driver?\n\nIn current proposal the order of running committags drivers is\nspecified in configuration...\n\n> \n> Perhaps a simpler alternative would be to let at most one driver process\n> a given snippet of text, forbidding nesting of replacements. (If I\n> understand Junio's suggestion to use a list of strings and refs,\n> non-nesting overlaps are already not supported.) If all replacements\n> were hyperlinks -- and I expect that to be the common case -- they\n> wouldn't be nestable anyway. I wouldn't see it as a huge loss for the\n> nesting examples I can think of: Separate rules for span around S-o-b\n> and linking or obfuscation of email could be combined into one... A\n> rule to shade text quoted email-style with leading angle brackets could\n> just clobber any further processing of that text. And it might simplify\n> the code and testing of it quite a bit.\n\n... but I guess that at first attempt we could support non-overlapping\ncommittags only, i.e. replacement is always as whole not passed to\nlater committags.\n\nStill there is a problem how to specify which parts of replacement for\ncommittags have to be HTML escaped, and which are HTML and should not\nbe (and which are attributes, and have to be escaped too).\n\n[...]\n> A few ideas for drivers that I don't think have been mentioned yet:\n> \n> * Wiki page names, like to [[Feature Documentation]]. These are notable\n> because they tend to contain punctuation that get HTML-escaped, like\n> quotes and ampersands.\n\nWell, I think if it would be supported, it would be a very special\ncase, so I don't think generic support for this is needed nor required.\n\n> \n> * Links to gitweb itself, such as 123abc:file.txt and HEAD:file.txt. I\n> guess the current hash linking sort of does the first case except that\n> you have to get the hash of the blob instead of using the commit hash,\n> and the current hash linking wouldn't reveal the filename until after\n> you click, nor when viewing textual log messages. I'm not sure whether\n> special support for linking to multi-commit diffs or other object types\n> would be as helpful.\n\nAlso 'v1.5.4' etc linking to tag; both would be a good idea. At this\npoint I think we have already list of all references (for ref markers)\nso it wouldn't require additional call to git command.\n\n\nP.S. I understand that this post is an exception (send after long, long\ntime), but please do not toppost in replies. It goes against natural\nreading order.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"105276","messageId":"cb7bb73a0902172341x2265e9d4r24a16ef2913bcda6@mail.gmail.com","threadId":"16231","inReplyTo":"1234926043-7471-1-git-send-email-marcel@oak.homeunix.org","subject":"Re: [PATCH RFC 1/2] gitweb: Fix warnings with override permitted but no repo override","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-18T07:41:54Z","receivedAt":"2009-02-18T07:41:54Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Wed, Feb 18, 2009 at 4:00 AM, Marcel M. Cary wrote:\n> When a feature like \"blame\" is permitted to be overridden in the\n> repository configuration but it is not actually set in the\n> repository, a warning is emitted due to the undefined value\n> of the repository configuration, even though it's a perfectly\n> normal condition.\n>\n> The warning is grounds for test failure in the gitweb test script,\n> so it causes some new feature tests of mine to fail.\n>\n> This patch prevents warning and adds a test case to exercise it.\n>\n> Signed-off-by: Marcel M. Cary \n> ---\n>\n> Here's a small patch I put together while tinkering with bug hyperlinking.\n> Does this look reasonable?\n\nMy only perplexity is about this:\n\n> @@ -1978,6 +1978,8 @@ sub git_get_project_config {\n> $config_file = \"$git_dir/config\";\n> }\n>\n> + return undef if (!defined $config{\"gitweb.$key\"});\n> +\n\nI'm no Perl expert, so I have no idea: how do non-bool config checks\n(which expect arrays) cope with an undef? Also, you may want to add a\nnon-bool override test in the test suite.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"105280","messageId":"7vvdr8yw78.fsf@gitster.siamese.dyndns.org","threadId":"16231","inReplyTo":"1234926043-7471-1-git-send-email-marcel@oak.homeunix.org","subject":"Re: [PATCH RFC 1/2] gitweb: Fix warnings with override permitted but no repo override","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-18T08:40:27Z","receivedAt":"2009-02-18T08:40:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Marcel M. Cary\" writes:\n\n> When a feature like \"blame\" is permitted to be overridden in the\n> repository configuration but it is not actually set in the\n> repository, a warning is emitted due to the undefined value\n> of the repository configuration, even though it's a perfectly\n> normal condition.\n>\n> The warning is grounds for test failure in the gitweb test script,\n> so it causes some new feature tests of mine to fail.\n>\n> This patch prevents warning and adds a test case to exercise it.\n>\n> Signed-off-by: Marcel M. Cary \n> ---\n>\n> Here's a small patch I put together while tinkering with bug hyperlinking.\n> Does this look reasonable?\n\nTo my cursory look, it doesn't, and it is not entirely your fault, but if\nwe applied this patch, it would not improve things very much. It just\nwould shift the same problem around.\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 7c48181..653f0be 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -402,13 +402,13 @@ sub feature_bool {\n> \tmy $key = shift;\n> \tmy ($val) = git_get_project_config($key, '--bool');\n> \n> -\tif ($val eq 'true') {\n> +\tif (!defined $val) {\n> +\t\treturn ($_[0]);\n> +\t} elsif ($val eq 'true') {\n> \t\treturn (1);\n> \t} elsif ($val eq 'false') {\n> \t\treturn (0);\n> \t}\n\nI think the warning you are talking about is to compare $val with 'true'\nwith 'eq' operator when $val could be undef. The check to see if $val is\nundefined ato avoid that 'eq' comparison is fine, and the intent to return\nfalse is also good, but I think feature_bool is meant to say \"yes\" or\n\"no\", and existing code for 'false' is returning (0). I'd rather see your\nnew codepath normalize incoming undef the same way string 'false' is\nnormalized to return (0). Granted, the caller should be prepared to take\nthe answer as boolean and treat the usual Perl false values (numeric zero,\na string with single \"0\", or an undef) without barfing, so returning (undef)\nfrom here ought to be safe (otherwise the callers are broken), but I'd\nrather see this function play safe.\n\nBut it certainly is not my main complaint.\n\n> sub feature_snapshot {\n> @@ -1978,6 +1978,8 @@ sub git_get_project_config {\n> \t\t$config_file = \"$git_dir/config\";\n> \t}\n> \n> +\treturn undef if (!defined $config{\"gitweb.$key\"});\n> +\n\nI think this change is missing a lot of necessary fixes associated with\nit. Have you actually audited all the callers of this function you are\nmodifying? For example, feature_bool does this:\n\n sub feature_bool {\n my $key = shift;\n my ($val) = git_get_project_config($key, '--bool');\n\n if ($val eq 'true') {\n return (1);\n } elsif ($val eq 'false') {\n\t...\n\nWith your above change, I think a missing configuration variable will\nstuff undef in $val, and trigger the same \"$val eq 'true'\" comparison\nwarning here.\n\nGranted, without your change the existing code triggers the same error in\nanother way, by calling config_to_bool sub with undef here:\n\n\t# ensure given type\n\tif (!defined $type) {\n\t\treturn $config{\"gitweb.$key\"};\n\t} elsif ($type eq 'bool') {\n\t\t# backward compatibility: 'git config --bool' returns true/false\n\t\treturn config_to_bool($config{\"gitweb.$key\"}) ? 'true' : 'false';\n\nand config_to_bool sub is written in the same carelessness like so:\n\n sub config_to_bool {\n my $val = shift;\n\n # strip leading and trailing whitespace\n $val =~ s/^\\s+//;\n\nand triggers the same error here in the s/// operation. I think the right\nfix for this part would look like this:\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 7c48181..2b140cc 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1920,6 +1920,8 @@ sub git_parse_project_config {\n sub config_to_bool {\n \tmy $val = shift;\n \n+\treturn 1 if (!defined $val);\n+\n \t# strip leading and trailing whitespace\n \t$val =~ s/^\\s+//;\n \t$val =~ s/\\s+$//;\n\nBecause\n\n\t[gitweb]\n \tvariable\n\nparsed by git_parse_project_config('gitweb') should return a hash that\nmaps \"gitweb.variable\" to undef it must be fed as undef to\nconfig_to_bool. This variable should be reported as \"true\".\n\nThere are tons of undef unsafeness in this file from a very cursory look.\n\nUnrelated to any of these, I think the following is wrong:\n\n sub feature_patches {\n my @val = (git_get_project_config('patches', '--int'));\n\n if (@val) {\n return @val;\n }\n\n return ($_[0]);\n }\n\nAs git_get_project_config() always returns something, hence \"if (@val)\"\ncan never be false.\n"},{"id":"105296","messageId":"200902181409.42407.jnareb@gmail.com","threadId":"16231","inReplyTo":"7vvdr8yw78.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH RFC 1/2] gitweb: Fix warnings with override permitted but no repo override","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-18T13:09:41Z","receivedAt":"2009-02-18T13:09:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Fixed up patch at the bottom.\n\nJunio C Hamano wrote:\n> \"Marcel M. Cary\" writes:\n> \n> > When a feature like \"blame\" is permitted to be overridden in the\n> > repository configuration but it is not actually set in the\n> > repository, a warning is emitted due to the undefined value\n> > of the repository configuration, even though it's a perfectly\n> > normal condition.\n> >\n> > The warning is grounds for test failure in the gitweb test script,\n> > so it causes some new feature tests of mine to fail.\n> >\n> > This patch prevents warning and adds a test case to exercise it.\n> >\n> > Signed-off-by: Marcel M. Cary \n> > ---\n> >\n> > Here's a small patch I put together while tinkering with bug hyperlinking.\n> > Does this look reasonable?\n\nSomewhat.\n\nCorrected patch at the bottom.\n\n> \n> To my cursory look, it doesn't, and it is not entirely your fault, but if\n> we applied this patch, it would not improve things very much. It just\n> would shift the same problem around.\n> \n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index 7c48181..653f0be 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -402,13 +402,13 @@ sub feature_bool {\n> > \tmy $key = shift;\n> > \tmy ($val) = git_get_project_config($key, '--bool');\n> > \n> > -\tif ($val eq 'true') {\n> > +\tif (!defined $val) {\n> > +\t\treturn ($_[0]);\n> > +\t} elsif ($val eq 'true') {\n> > \t\treturn (1);\n> > \t} elsif ($val eq 'false') {\n> > \t\treturn (0);\n> > \t}\n> \n> I think the warning you are talking about is to compare $val with 'true'\n> with 'eq' operator when $val could be undef. The check to see if $val is\n> undefined to avoid that 'eq' comparison is fine, and the intent to return\n> false is also good, but I think feature_bool is meant to say \"yes\" or\n> \"no\", and existing code for 'false' is returning (0). I'd rather see your\n> new codepath normalize incoming undef the same way string 'false' is\n> normalized to return (0).\n\nActually git_get_project_config($key, '--bool') can return only three\nvalues: \n * 'true' if gitweb.$key config variable is 'true', 'yes', 1, or (after\n fixes in the fixed up patch at the bottom) is undefined, i.e.\n [gitweb]\n \tblame\n case\n * 'false' if gitweb.$key exists and has other value (that includes\n empty string value: \"[section] val\" is git-config --bool true, while\n \"[section] val = \" is --bool false).\n * undef if gitweb.$key does not exist in the config;\n earlier version which used \"git config --bool \" returned\n empty string ('') here.\n\nWe want to fall back to %feature value (i.e. do not override) if\nvariable is not set in config. Variable not set was '', and now is undef,\ntherefore need for this (correct) change.\n\n> Granted, the caller should be prepared to take \n> the answer as boolean and treat the usual Perl false values (numeric zero,\n> a string with single \"0\", or an undef) without barfing, so returning (undef)\n> from here ought to be safe (otherwise the callers are broken), but I'd\n> rather see this function play safe.\n> \n> But it certainly is not my main complaint.\n\nWell, I think we can now get rid of backwards compatibility (which is\nnot complete anyway: '' for not existent variable for old version, undef\nfor new version) with the old version which ran git-config once for each\nvariable, and do not go through 'true'/'false' to imitate calling\n\"git config --bool\", which has to be converted back to Perl boolean\nanyway.\n\n> \n> > sub feature_snapshot {\n> > @@ -1978,6 +1978,8 @@ sub git_get_project_config {\n> > \t\t$config_file = \"$git_dir/config\";\n> > \t}\n> > \n> > +\treturn undef if (!defined $config{\"gitweb.$key\"});\n> > +\n\nIt should be !exists, and not !defined here, see my fixed up patch\nbelow.\n\n> \n> I think this change is missing a lot of necessary fixes associated with\n> it. Have you actually audited all the callers of this function you are\n> modifying? For example, feature_bool does this:\n> \n> sub feature_bool {\n> my $key = shift;\n> my ($val) = git_get_project_config($key, '--bool');\n> \n> if ($val eq 'true') {\n> return (1);\n> } elsif ($val eq 'false') {\n> \t...\n> \n> With your above change, I think a missing configuration variable will\n> stuff undef in $val, and trigger the same \"$val eq 'true'\" comparison\n> warning here.\n\nErrr... Junio, Marcel DID fix feature_bool, see above:\n\n> > @@ -402,13 +402,13 @@ sub feature_bool {\n> > \tmy $key = shift;\n> > \tmy ($val) = git_get_project_config($key, '--bool');\n> > \n> > -\tif ($val eq 'true') {\n> > +\tif (!defined $val) {\n> > +\t\treturn ($_[0]);\n> > +\t} elsif ($val eq 'true') {\n> > \t\treturn (1);\n> > \t} elsif ($val eq 'false') {\n> > \t\treturn (0);\n> > \t}\n\n\n> \n> Granted, without your change the existing code triggers the same error in\n> another way, by calling config_to_bool sub with undef here:\n> \n> \t# ensure given type\n> \tif (!defined $type) {\n> \t\treturn $config{\"gitweb.$key\"};\n> \t} elsif ($type eq 'bool') {\n> \t\t# backward compatibility: 'git config --bool' returns true/false\n> \t\treturn config_to_bool($config{\"gitweb.$key\"}) ? 'true' : 'false';\n> \n> and config_to_bool sub is written in the same carelessness like so:\n> \n> sub config_to_bool {\n> my $val = shift;\n> \n> # strip leading and trailing whitespace\n> $val =~ s/^\\s+//;\n> \n> and triggers the same error here in the s/// operation. I think the right\n> fix for this part would look like this:\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 7c48181..2b140cc 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -1920,6 +1920,8 @@ sub git_parse_project_config {\n> sub config_to_bool {\n> \tmy $val = shift;\n> \n> +\treturn 1 if (!defined $val);\n> +\n> \t# strip leading and trailing whitespace\n> \t$val =~ s/^\\s+//;\n> \t$val =~ s/\\s+$//;\n> \n> Because\n> \n> \t[gitweb]\n> \tvariable\n> \n> parsed by git_parse_project_config('gitweb') should return a hash that\n> maps \"gitweb.variable\" to undef it must be fed as undef to\n> config_to_bool. This variable should be reported as \"true\".\n\nRight (but not complete).\n\nWe do check !defined $val, but too late: _after_ trying to strip leading\nand trailing whitespace. When going through various versions of\nconfig_to_bool I have somehow forgot about this issue...\n\n> \n> There are tons of undef unsafeness in this file from a very cursory look.\n> \n> Unrelated to any of these, I think the following is wrong:\n> \n> sub feature_patches {\n> my @val = (git_get_project_config('patches', '--int'));\n> \n> if (@val) {\n> return @val;\n> }\n> \n> return ($_[0]);\n> }\n> \n> As git_get_project_config() always returns something, hence \"if (@val)\"\n> can never be false.\n\nActually after the patch below I think that git_get_project_config\nreturns empty list () in the list (array) context now if variable\ndoes not exist in the config thanks to \"return ;\" magic. And empty\nlist evaluates to false in scalar context: \"if (@val)\" would be false\nif variable does not exist in the config... in which case we would not\noverride 'default' value in %feature.\n\nAlternate solution would be to use\n\n sub feature_patches {\n my $val = git_get_project_config('patches', '--int');\n \n if (defined $val) {\n return ($val);\n }\n \n return ($_[0]);\n }\n\n\n\n-- >8 --\nFrom: Marcel M. Cary \nSubject: [PATCH] gitweb: Fix warnings with override permitted but no repo override\n\nWhen a feature like \"blame\" is permitted to be overridden in the\nrepository configuration but it is not actually set in the repository,\na warning is emitted due to the undefined value of the repository\nconfiguration, even though it's a perfectly normal condition.\nEmitting warning is grounds for test failure in the gitweb test\nscript.\n\nThis error was caused by rewrite of git_get_project_config from using\n\"git config [] \" for each individual configuration\nvariable checked to parsing \"git config --list --null\" output in\ncommit b201927 (gitweb: Read repo config using 'git config -z -l').\nEarlier version of git_get_project_config was returning empty string\nif variable do not exist in config; newer version is meant to return\nundef in this case, therefore change in feature_bool was needed.\n\nAdditionally config_to_* subroutines were meant to be invoked only if\nconfiguration variable exists; therefore we added early return to\ngit_get_project_config: it now returns no value if variable does not\nexists in config. Otherwise config_to_* subroutines (config_to_bool\nin paryicular) wouldn't be able to distinguish between the case where\nvariable does not exist and the case where variable doesn't have value\n(the \"[section] noval\" case, which evaluates to true for boolean).\n\nWhile at it fix bug in config_to_bool, where checking if $val is\ndefined (if config variable has value) was done _after_ stripping\nleading and trailing whitespace, which lead to 'Use of uninitialized\nvalue' warning.\n\nAdd test case for features overridable but not overriden in repo\nconfig, and case for no value boolean configuration variable.\n\nSigned-off-by: Marcel M. Cary \nSigned-off-by: Jakub Narebski \n---\n gitweb/gitweb.perl | 16 ++++++++++------\n t/t9500-gitweb-standalone-no-errors.sh | 18 +++++++++++++++++-\n 2 files changed, 27 insertions(+), 7 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 7c48181..83858fb 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -402,13 +402,13 @@ sub feature_bool {\n \tmy $key = shift;\n \tmy ($val) = git_get_project_config($key, '--bool');\n \n-\tif ($val eq 'true') {\n+\tif (!defined $val) {\n+\t\treturn ($_[0]);\n+\t} elsif ($val eq 'true') {\n \t\treturn (1);\n \t} elsif ($val eq 'false') {\n \t\treturn (0);\n \t}\n-\n-\treturn ($_[0]);\n }\n \n sub feature_snapshot {\n@@ -1914,18 +1914,19 @@ sub git_parse_project_config {\n \treturn %config;\n }\n \n-# convert config value to boolean, 'true' or 'false'\n+# convert config value to boolean: 'true' or 'false'\n # no value, number > 0, 'true' and 'yes' values are true\n # rest of values are treated as false (never as error)\n sub config_to_bool {\n \tmy $val = shift;\n \n+\treturn 1 if !defined $val; # section.key\n+\n \t# strip leading and trailing whitespace\n \t$val =~ s/^\\s+//;\n \t$val =~ s/\\s+$//;\n \n-\treturn (!defined $val || # section.key\n-\t ($val =~ /^\\d+$/ && $val) || # section.key = 1\n+\treturn (($val =~ /^\\d+$/ && $val) || # section.key = 1\n \t ($val =~ /^(?:true|yes)$/i)); # section.key = true\n }\n \n@@ -1978,6 +1979,9 @@ sub git_get_project_config {\n \t\t$config_file = \"$git_dir/config\";\n \t}\n \n+\t# check if config variable (key) exists\n+\treturn unless exists $config{\"gitweb.$key\"};\n+\n \t# ensure given type\n \tif (!defined $type) {\n \t\treturn $config{\"gitweb.$key\"};\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex 7c6f70b..6ed10d0 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -662,6 +662,11 @@ cat >>gitweb_config.perl <.git/config <<\\EOF\n+# testing noval and alternate separator\n+[gitweb]\n+\tblame\n+\tsnapshot = zip tgz\n+EOF\n+test_expect_success \\\n+\t'config override: tree view, features enabled in repo config (2)' \\\n+\t'gitweb_run \"p=.git;a=tree\"'\n+test_debug 'cat gitweb.log'\n+\n # ----------------------------------------------------------------------\n # non-ASCII in README.html\n \n-- \n1.6.1\n"},{"id":"105334","messageId":"7vr61vwotp.fsf@gitster.siamese.dyndns.org","threadId":"16231","inReplyTo":"200902181409.42407.jnareb@gmail.com","subject":"Re: [PATCH RFC 1/2] gitweb: Fix warnings with override permitted but no repo override","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-18T19:02:42Z","receivedAt":"2009-02-18T19:02:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Fixed up patch at the bottom.\n\nThanks.\n"},{"id":"105349","messageId":"200902182255.13983.jnareb@gmail.com","threadId":"16231","inReplyTo":"1234926043-7471-2-git-send-email-marcel@oak.homeunix.org","subject":"Re: [PATCH RFC 2/2] gitweb: Hyperlink multiple git hashes on the same commit message line","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-18T21:55:11Z","receivedAt":"2009-02-18T21:55:11Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 18 Feb 2009, Marcel M. Cary wrote:\n\n> The current implementation only hyperlinks the first hash on\n> a given line of the commit message. It seems sensible to\n> highlight all of them if there are multiple, and it seems\n> plausible that there would be multiple even with a tidy line\n> length limit, because they can be abbreviated as short as 8\n> characters.\n\nThat is a good catch. Code simply was not modified since we required\nfill-length 40-characters SHA-1 id.\n\n> \n> Benchmark:\n> \n> I wanted to make sure that using the 'e' switch to the Perl regex\n> wasn't going to kill performance, since this is called once per commit\n> message line displayed.\n> \n> In all three A/B scenarios I tried, the A and B yielded the same\n> results within 2%, where A is the version of code before this patch\n> and B is the version after.\n> \n> 1: View a commit message containing the last 1000 commit hashes\n> 2: View a commit message containing 1000 lines of 40 dots to avoid\n> hyperlinking at the same message length\n> 3: View a short merge commit message with a few lines of text and\n> no hashes\n\nI don't think we should worry about that; after all esc_path and\nunescape subroutines also use 'e' switch to Perl regexp.\n\nSo the benchmark is nice addition, but I don't think it is really\nnecessary, especially that the change results in shorter and easier\n(I think) to maintain code.\n\n[...]\n> So I think the patch has no noticeable effect on performance.\n> \n> Signed-off-by: Marcel M. Cary \n> ---\n> \n> And here's another.\n> \n> Marcel\n\nDo I understand correctly that those patches are not related at all\nsemantically or textually, only in that you have them one after other\n(and blob sha-1 in the index line reflects state after former), isn't\nit?\n\n> \n> \n> gitweb/gitweb.perl | 12 +++++-------\n> 1 files changed, 5 insertions(+), 7 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 653f0be..51b7f56 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -1384,13 +1384,11 @@ sub format_log_line_html {\n> \tmy $line = shift;\n> \n> \t$line = esc_html($line, -nbsp=>1);\n> -\tif ($line =~ m/\\b([0-9a-fA-F]{8,40})\\b/) {\n> -\t\tmy $hash_text = $1;\n> -\t\tmy $link =\n> -\t\t\t$cgi->a({-href => href(action=>\"object\", hash=>$hash_text),\n> -\t\t\t -class => \"text\"}, $hash_text);\n> -\t\t$line =~ s/$hash_text/$link/;\n> -\t}\n> +\t$line =~ s{\\b([0-9a-fA-F]{8,40})\\b}{\n> +\t\treturn $cgi->a({-href => href(action=>\"object\", hash=>$1),\n> +\t\t\t\t\t -class => \"text\"}, $1);\n> +\t}eg;\n> +\n\nAlmost correct... but for this unnecessary 'return' statement.\nWithout it: ACK.\n\n> \treturn $line;\n> }\n> \n> -- \n> 1.6.1\n> \n> \n\nP.S. Why bare emails (without user names), e.g. \"pasky@suse.cz\"\nand not \"Petr Baudis \"? Just curious...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"105482","messageId":"499D91F5.6010605@oak.homeunix.org","threadId":"16231","inReplyTo":"200902180438.55081.jnareb@gmail.com","subject":"Re: [RFC] Configuring (future) committags support in gitweb, especially bug linking","fromName":"Marcel M. Cary","fromEmail":"marcel@oak.homeunix.org","sentAt":"2009-02-19T17:08:05Z","receivedAt":"2009-02-19T17:08:05Z","isPatch":false,"sender":{"key":"marcel@oak.homeunix.org","avatar":"https://gravatar.com/avatar/2bb524e4f383167b7e256bb93256c88353748d9873c34cde0fd461f1165baa0f?d=mp&s=160"},"body":"Jakub Narebski wrote:\n> On Tue, 17 Feb 2009, Marcel M. Cary wrote:\n>\n>> I'm interested in cross-linking bug references in commit messages to\n>> a bug tracking system. I started tinkering a couple weeks ago and am\n>> finally understanding that committags encompass this functionality.\n>> (From the subject line I first understood \"tags\" to mean git tags\n>> rather than commit message munging.)\n>\n> What would you name this feature, then?\n\nHeh, I'm not sure. It's like a filter in the unix pipeline sense, but\n\"commit message filter\" sounds to me like some messages might be\nrejected. Most of the drivers markup static text with HTML tags, but\nnot all of them. Maybe \"commit message embellishment\".\n\nPerhaps a more important question is: will people find the feature once\nit's implemented? I think that won't be a problem provided that it's\nlisted in the gitweb docs like the other configs.\n\n>> Is the committags idea still under active development?\n>\n> Well, it is in my todo list, rather further on...\n\nIs any code for it published in a repository anywhere? I see a branch\njn/gitweb-committag merged into master that looks relevant, but it only\nhas the sha1 regex improvement.\n\n>> Two regexes would make it easier to configure a driver without\n>> needing look-ahead and look-behind assertions. For example, if you\n>> want to match non-negative integers but only in the context of a\n>> Resolves-bug header:\n>>\n>> Resolves-bug: 1234, 1235\n>\n> [...]\n>> I got the two-regex idea from a spec I ran across while evaluating\n>> Subversion:\n>>\n>>\nhttp://guest:@tortoisesvn.tigris.org/svn/tortoisesvn/trunk/doc/issuetrackers.txt\n>\n> You don't need multiple regexps for that, and in above example it is\n> used _single_ regexp; only with more than one catching group.\n\nI'm not sure what exactly you propose. In the second example in the\nbugtraq spec, there are two regexes. Maybe you mean something like\nthis, but it breaks with three bugs:\n\n $ perl -MData::Dumper -wne '\n m/^Resolves-bug: (\\d+)(?:, (\\d+))*/;\n print Dumper([$1, $2, $3, $4]);\n '\n Resolves-bug: 123\n $VAR1 = [\n '123',\n undef,\n undef,\n undef\n ];\n Resolves-bug: 123, 124\n $VAR1 = [\n '123',\n '124',\n undef,\n undef\n ];\n Resolves-bug: 123, 124, 125\n $VAR1 = [\n '123',\n '125',\n undef,\n undef\n ];\n\nMaybe something like this? But it's limited to an arbitrary number of\nbug matches. Maybe it's good enough for pratical purposes, but it's\nprone to unexpected breakage when the user exceeds the threshold of, in\nthis case, four bugs.\n\n /^Resolves-bug: (\\d+)(?:, (\\d+))?(?:, (\\d+))?(?:, (\\d+))?/\n\n\nMarcel\n"},{"id":"105583","messageId":"7v4oypfqua.fsf@gitster.siamese.dyndns.org","threadId":"16231","inReplyTo":"200902182255.13983.jnareb@gmail.com","subject":"Re: [PATCH RFC 2/2] gitweb: Hyperlink multiple git hashes on the same commit message line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-20T08:35:41Z","receivedAt":"2009-02-20T08:35:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n>> +\t$line =~ s{\\b([0-9a-fA-F]{8,40})\\b}{\n>> +\t\treturn $cgi->a({-href => href(action=>\"object\", hash=>$1),\n>> +\t\t\t\t\t -class => \"text\"}, $1);\n>> +\t}eg;\n>> +\n>\n> Almost correct... but for this unnecessary 'return' statement.\n> Without it: ACK.\n\nI've applied this directly on 'master' without the return from inside s///e\nwith your Ack. Please check the result.\n\nThanks.\n"},{"id":"105603","messageId":"200902201247.00670.jnareb@gmail.com","threadId":"16231","inReplyTo":"7v4oypfqua.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH RFC 2/2] gitweb: Hyperlink multiple git hashes on the same commit message line","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-20T11:46:57Z","receivedAt":"2009-02-20T11:46:57Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n> Jakub Narebski writes:\n> \n> >> +\t$line =~ s{\\b([0-9a-fA-F]{8,40})\\b}{\n> >> +\t\treturn $cgi->a({-href => href(action=>\"object\", hash=>$1),\n> >> +\t\t\t\t\t -class => \"text\"}, $1);\n> >> +\t}eg;\n> >> +\n> >\n> > Almost correct... but for this unnecessary 'return' statement.\n> > Without it: ACK.\n> \n> I've applied this directly on 'master' without the return from inside s///e\n> with your Ack. Please check the result.\n\nI did quick test by installing newest gitweb (with above commit applied),\ndoing in gitweb searching commit message for '[a-f0-9]{8,40}' with regexp\nsearch; everything looks all right. But I didn't do extensive tests.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"106061","messageId":"49A41484.1010501@oak.homeunix.org","threadId":"16231","inReplyTo":"200902182255.13983.jnareb@gmail.com","subject":"Addresses with full names in patch emails","fromName":"Marcel M. Cary","fromEmail":"marcel@oak.homeunix.org","sentAt":"2009-02-24T15:38:44Z","receivedAt":"2009-02-24T15:38:44Z","isPatch":false,"sender":{"key":"marcel@oak.homeunix.org","avatar":"https://gravatar.com/avatar/2bb524e4f383167b7e256bb93256c88353748d9873c34cde0fd461f1165baa0f?d=mp&s=160"},"body":"Thanks for the two patch tweaks.\n\nJakub Narebski wrote:\n> P.S. Why bare emails (without user names), e.g. \"pasky@suse.cz\"\n> and not \"Petr Baudis \"? Just curious...\n\nI've been using \"git send-email\" for patches, and have Thunderbird as my\nMUA otherwise. (I'd use (al)pine if I could make it work with\nExchange/NTLM at work, but that's another story...) I've been\ntransfering recipients (--to and --cc) from Thunderbird to the\ncommandline with copy/paste.\n\nIn Thurderbird, copying an email address from a message only gets you\nthe user@domain part, not the \"Full Name\" . To get the\n\"Full Name\" I would have to View Message Source and\npickout the CC line, which is marginally harder.\n\nAnd on the commandline, instead of just pasting an email as a shell\nword, I'd have to add single quotes (I think) to keep the whole \"Full\nName\" as one word and quote the shell meta characters.\n\nNeither piece is all that onerous, I guess. Sounds like you would see\nsome value in the full names. Maybe I'll try including them on my next\npatch. Looks like --cc-cmd or sendemail.aliasesfile might make it\neasier, but I'd have to set them up.\n\nMarcel\n"},{"id":"106070","messageId":"200902241658.52498.jnareb@gmail.com","threadId":"16231","inReplyTo":"49A41484.1010501@oak.homeunix.org","subject":"Re: Addresses with full names in patch emails","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-02-24T15:58:51Z","receivedAt":"2009-02-24T15:58:51Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 24 Feb 2009, Marcel M. Cary wrote:\n> Thanks for the two patch tweaks.\n> \n> Jakub Narebski wrote:\n> > P.S. Why bare emails (without user names), e.g. \"pasky@suse.cz\"\n> > and not \"Petr Baudis \"? Just curious...\n> \n> I've been using \"git send-email\" for patches, and have Thunderbird as my\n> MUA otherwise. (I'd use (al)pine if I could make it work with\n> Exchange/NTLM at work, but that's another story...) I've been\n> transfering recipients (--to and --cc) from Thunderbird to the\n> commandline with copy/paste.\n[...]\n\nWell, 'technical reasons with copy'n'paste' would be enough for me.\n\nP.S. But '\"pasky@suse.cz\" ' looks very silly...\n-- \nJakub Narebski\nPoland\n"},{"id":"106074","messageId":"49A42161.4040101@oak.homeunix.org","threadId":"16231","inReplyTo":"200902182255.13983.jnareb@gmail.com","subject":"Re: [PATCH RFC 2/2] gitweb: Hyperlink multiple git hashes on the same commit message line","fromName":"Marcel M. Cary","fromEmail":"marcel@oak.homeunix.org","sentAt":"2009-02-24T16:33:37Z","receivedAt":"2009-02-24T16:33:37Z","isPatch":true,"sender":{"key":"marcel@oak.homeunix.org","avatar":"https://gravatar.com/avatar/2bb524e4f383167b7e256bb93256c88353748d9873c34cde0fd461f1165baa0f?d=mp&s=160"},"body":"Jakub Narebski wrote:\n> Do I understand correctly that those patches are not related at all\n> semantically or textually, only in that you have them one after other\n> (and blob sha-1 in the index line reflects state after former), isn't\n> it?\n\nYes, I think they were independent. They are related only in that they\nare small tweaks to gitweb and products of my bug hyperlinking patch\n(not submitted). I submitted them as a series mainly to group them.\nAre you suggesting that I reserve threading patches together for when\none builds upon the previous one?\n\nMarcel\n"}]}