{"thread":{"id":"31968","subject":"gitweb","startedAt":"2012-10-28T23:56:47Z","lastAt":"2012-10-29T07:14:11Z","messageCount":4,"participants":["rh","Jeff King","Jakub Narębski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"202077","messageId":"20121028165647.b79fe3fcb6784c4ae547439e@lavabit.com","threadId":"31968","inReplyTo":null,"subject":"gitweb","fromName":"rh","fromEmail":"richard_hubbe11@lavabit.com","sentAt":"2012-10-28T23:56:47Z","receivedAt":"2012-10-28T23:56:47Z","isPatch":false,"sender":{"key":"richard_hubbe11@lavabit.com","avatar":null},"body":"I'm not using gitweb I was thinking about using it and was looking at the \ncgi and saw this in this file:\nhttps://github.com/git/git/blob/master/gitweb/gitweb.perl\n\nI think I understand the intention but the outcome is wrong.\n\nour %highlight_ext = (\n\t# main extensions, defining name of syntax;\n\t# see files in /usr/share/highlight/langDefs/ directory\n\tmap { $_ => $_ }\n\tqw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make),\n\t# alternate extensions, see /etc/highlight/filetypes.conf\n\t'h' => 'c',\n\tmap { $_ => 'sh' } qw(bash zsh ksh),\n\tmap { $_ => 'cpp' } qw(cxx c++ cc),\n\tmap { $_ => 'php' } qw(php3 php4 php5 phps),\n\tmap { $_ => 'pl' } qw(perl pm), # perhaps also 'cgi'\n\tmap { $_ => 'make'} qw(mak mk),\n\tmap { $_ => 'xml' } qw(xhtml html htm),\n);\n\nI think the intent is better met with this, (the print is for show)\n\nour %he = ();\n$he{'h'} = 'c';\n$he{$_} = $_     for (qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make));\n$he{$_} = 'cpp'  for (qw(cxx c++ cc));\n$he{$_} = 'php'  for (qw(php3 php4 php5 phps));\n$he{$_} = 'pl'   for (qw(cgi perl pm));\n$he{$_} = 'make' for (qw(mak mk));\n$he{$_} = 'xml'  for (qw(xhtml html htm));\n$he{$_} = 'sh'   for (qw(bash zsh ksh));\n\nprint \"$he{$_} $_\\n\" for(sort {$he{$a} cmp $he{$b}} keys %he);\n\nBut then again maybe I misunderstood the intent.  And maybe everyone's happy\nwith it as-is.\n"},{"id":"202082","messageId":"20121029052815.GA30186@sigill.intra.peff.net","threadId":"31968","inReplyTo":"20121028165647.b79fe3fcb6784c4ae547439e@lavabit.com","subject":"Re: gitweb","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-29T05:28:15Z","receivedAt":"2012-10-29T05:28:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 28, 2012 at 04:56:47PM -0700, rh wrote:\n\n> I'm not using gitweb I was thinking about using it and was looking at the \n> cgi and saw this in this file:\n> https://github.com/git/git/blob/master/gitweb/gitweb.perl\n> \n> I think I understand the intention but the outcome is wrong.\n> \n> our %highlight_ext = (\n> \t# main extensions, defining name of syntax;\n> \t# see files in /usr/share/highlight/langDefs/ directory\n> \tmap { $_ => $_ }\n> \tqw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make),\n> \t# alternate extensions, see /etc/highlight/filetypes.conf\n> \t'h' => 'c',\n> \tmap { $_ => 'sh' } qw(bash zsh ksh),\n> \tmap { $_ => 'cpp' } qw(cxx c++ cc),\n> \tmap { $_ => 'php' } qw(php3 php4 php5 phps),\n> \tmap { $_ => 'pl' } qw(perl pm), # perhaps also 'cgi'\n> \tmap { $_ => 'make'} qw(mak mk),\n> \tmap { $_ => 'xml' } qw(xhtml html htm),\n> );\n\nYeah, this is wrong. The first map will eat the rest of the list, and\nyou will get \"h => h\", \"cxx => cxx\", and so forth. I do not know this\nchunk of code, but that does not seem like it is the likely intent.\n\nYou could fix it with extra parentheses:\n\n  our %he = (\n    (map { $_ => $_ } qw(py c cpp ...)),\n    'h' => 'c',\n    (map { $_ => 'sh' } qw(bash zsh ksh)),\n    ... etc ...\n  );\n\n> I think the intent is better met with this, (the print is for show)\n> \n> our %he = ();\n> $he{'h'} = 'c';\n> $he{$_} = $_     for (qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make));\n> $he{$_} = 'cpp'  for (qw(cxx c++ cc));\n> $he{$_} = 'php'  for (qw(php3 php4 php5 phps));\n> $he{$_} = 'pl'   for (qw(cgi perl pm));\n> $he{$_} = 'make' for (qw(mak mk));\n> $he{$_} = 'xml'  for (qw(xhtml html htm));\n> $he{$_} = 'sh'   for (qw(bash zsh ksh));\n\nThat is more readable to me (though it does lose the obviousness that it\nis a variable initialization).\n\nLooks like this was broken since 592ea41 (gitweb: Refactor syntax\nhighlighting support, 2010-04-27). I do not use gitweb (nor highlight)\nat all, but I'd guess the user-visible impact is that \"*.h\" files are\nnot correctly highlighted (unless highlight does this extension mapping\nitself, but then why are we doing it here?).\n\nJakub, can you confirm the intent and a fix like the one above makes\nthings better?\n\n-Peff\n"},{"id":"202096","messageId":"CANQwDwd6EP94PEFkEcx8gBX1B5+-95qtjGMD6iU3ao8G+rCbLw@mail.gmail.com","threadId":"31968","inReplyTo":"20121029052815.GA30186@sigill.intra.peff.net","subject":"Re: gitweb","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2012-10-29T07:12:46Z","receivedAt":"2012-10-29T07:12:46Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, Oct 29, 2012 at 6:28 AM, Jeff King <peff@peff.net> wrote:\n> On Sun, Oct 28, 2012 at 04:56:47PM -0700, rh wrote:\n>\n>> I'm not using gitweb I was thinking about using it and was looking at the\n>> cgi and saw this in this file:\n>> https://github.com/git/git/blob/master/gitweb/gitweb.perl\n>>\n>> I think I understand the intention but the outcome is wrong.\n>>\n>> our %highlight_ext = (\n>>       # main extensions, defining name of syntax;\n>>       # see files in /usr/share/highlight/langDefs/ directory\n>>       map { $_ => $_ }\n>>       qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make),\n>>       # alternate extensions, see /etc/highlight/filetypes.conf\n>>       'h' => 'c',\n[...]\n> Yeah, this is wrong. The first map will eat the rest of the list, and\n> you will get \"h => h\", \"cxx => cxx\", and so forth. I do not know this\n> chunk of code, but that does not seem like it is the likely intent.\n>\n> You could fix it with extra parentheses:\n>\n>   our %he = (\n>     (map { $_ => $_ } qw(py c cpp ...)),\n>     'h' => 'c',\n>     (map { $_ => 'sh' } qw(bash zsh ksh)),\n>     ... etc ...\n>   );\n>\n>> I think the intent is better met with this, (the print is for show)\n>>\n>> our %he = ();\n>> $he{'h'} = 'c';\n>> $he{$_} = $_     for (qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl sql make));\n>> $he{$_} = 'cpp'  for (qw(cxx c++ cc));\n[...]\n\n> That is more readable to me (though it does lose the obviousness that it\n> is a variable initialization).\n>\n> Looks like this was broken since 592ea41 (gitweb: Refactor syntax\n> highlighting support, 2010-04-27). I do not use gitweb (nor highlight)\n> at all, but I'd guess the user-visible impact is that \"*.h\" files are\n> not correctly highlighted\n>\n> Jakub, can you confirm the intent and a fix like the one above makes\n> things better?\n\nYes, either of those makes things better.\n\n>                                   (unless highlight does this extension mapping\n> itself, but then why are we doing it here?).\n\nHighlight does extension mapping itself... but for that it needs file name,\nand not to be feed file contents from pipe.\n\n-- \nJakub Narebski\n"},{"id":"202098","messageId":"20121029071411.GA4229@sigill.intra.peff.net","threadId":"31968","inReplyTo":"CANQwDwd6EP94PEFkEcx8gBX1B5+-95qtjGMD6iU3ao8G+rCbLw@mail.gmail.com","subject":"Re: gitweb","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-29T07:14:11Z","receivedAt":"2012-10-29T07:14:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 29, 2012 at 08:12:46AM +0100, Jakub Narębski wrote:\n\n> > Jakub, can you confirm the intent and a fix like the one above makes\n> > things better?\n> \n> Yes, either of those makes things better.\n\nThanks.\n\n> >                                   (unless highlight does this extension mapping\n> > itself, but then why are we doing it here?).\n> \n> Highlight does extension mapping itself... but for that it needs file name,\n> and not to be feed file contents from pipe.\n\nAh, that makes sense.\n\nRichard, do you want to roll a patch that fixes it?\n\n-Peff\n"}]}