Re: gitweb

[Jeff King <peff@peff.net>]

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',
> 	map { $_ => 'sh' } qw(bash zsh ksh),
> 	map { $_ => 'cpp' } qw(cxx c++ cc),
> 	map { $_ => 'php' } qw(php3 php4 php5 phps),
> 	map { $_ => 'pl' } qw(perl pm), # perhaps also 'cgi'
> 	map { $_ => 'make'} qw(mak mk),
> 	map { $_ => 'xml' } qw(xhtml html htm),
> );

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));
> $he{$_} = 'php'  for (qw(php3 php4 php5 phps));
> $he{$_} = 'pl'   for (qw(cgi perl pm));
> $he{$_} = 'make' for (qw(mak mk));
> $he{$_} = 'xml'  for (qw(xhtml html htm));
> $he{$_} = 'sh'   for (qw(bash zsh ksh));

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 (unless highlight does this extension mapping
itself, but then why are we doing it here?).

Jakub, can you confirm the intent and a fix like the one above makes
things better?

-Peff