On Mon, Oct 29, 2012 at 6:28 AM, Jeff King <p...@peff.net> wrote:
> On Sun, Oct 28, 2012 at 04:56:47PM -0700, rh wrote:
>
>> I'm not using gitweb I was thinking about using it and was looking at the
>> cgi and saw this in this file:
>> https://github.com/git/git/blob/master/gitweb/gitweb.perl
>>
>> I think I understand the intention but the outcome is wrong.
>>
>> our %highlight_ext = (
>>       # main extensions, defining name of syntax;
>>       # see files in /usr/share/highlight/langDefs/ directory
>>       map { $_ => $_ }
>>       qw(py c cpp rb java css php sh pl js tex bib xml awk bat ini spec tcl 
>> sql make),
>>       # alternate extensions, see /etc/highlight/filetypes.conf
>>       'h' => 'c',
[...]
> Yeah, this is wrong. The first map will eat the rest of the list, and
> you will get "h => h", "cxx => cxx", and so forth. I do not know this
> chunk of code, but that does not seem like it is the likely intent.
>
> You could fix it with extra parentheses:
>
>   our %he = (
>     (map { $_ => $_ } qw(py c cpp ...)),
>     'h' => 'c',
>     (map { $_ => 'sh' } qw(bash zsh ksh)),
>     ... etc ...
>   );
>
>> I think the intent is better met with this, (the print is for show)
>>
>> our %he = ();
>> $he{'h'} = 'c';
>> $he{$_} = $_     for (qw(py c cpp rb java css php sh pl js tex bib xml awk 
>> bat ini spec tcl sql make));
>> $he{$_} = 'cpp'  for (qw(cxx c++ cc));
[...]

> That is more readable to me (though it does lose the obviousness that it
> is a variable initialization).
>
> Looks like this was broken since 592ea41 (gitweb: Refactor syntax
> highlighting support, 2010-04-27). I do not use gitweb (nor highlight)
> at all, but I'd guess the user-visible impact is that "*.h" files are
> not correctly highlighted
>
> Jakub, can you confirm the intent and a fix like the one above makes
> things better?

Yes, either of those makes things better.

>                                   (unless highlight does this extension 
> mapping
> itself, but then why are we doing it here?).

Highlight does extension mapping itself... but for that it needs file name,
and not to be feed file contents from pipe.

-- 
Jakub Narebski
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to