# gitweb

4 messages from 2012-10-28 to 2012-10-29. Participants: rh, Jeff King, Jakub Narębski.
Thread: https://gitlist.dev/t/31968

## rh, 2012-10-28 23:56

Subject: gitweb
Message-ID: <20121028165647.b79fe3fcb6784c4ae547439e@lavabit.com>
URL: https://gitlist.dev/e/20121028165647.b79fe3fcb6784c4ae547439e%40lavabit.com

```
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),
);

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));

print "$he{$_} $_\n" for(sort {$he{$a} cmp $he{$b}} keys %he);

But then again maybe I misunderstood the intent.  And maybe everyone's happy
with it as-is.

```

## Jeff King, 2012-10-29 05:28

Subject: Re: gitweb
Message-ID: <20121029052815.GA30186@sigill.intra.peff.net>
URL: https://gitlist.dev/e/20121029052815.GA30186%40sigill.intra.peff.net
In-Reply-To: <20121028165647.b79fe3fcb6784c4ae547439e@lavabit.com>

```
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

```

## Jakub Narębski, 2012-10-29 07:12

Subject: Re: gitweb
Message-ID: <CANQwDwd6EP94PEFkEcx8gBX1B5+-95qtjGMD6iU3ao8G+rCbLw@mail.gmail.com>
URL: https://gitlist.dev/e/CANQwDwd6EP94PEFkEcx8gBX1B5%2B-95qtjGMD6iU3ao8G%2BrCbLw%40mail.gmail.com
In-Reply-To: <20121029052815.GA30186@sigill.intra.peff.net>

```
On Mon, Oct 29, 2012 at 6:28 AM, Jeff King <peff@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

```

## Jeff King, 2012-10-29 07:14

Subject: Re: gitweb
Message-ID: <20121029071411.GA4229@sigill.intra.peff.net>
URL: https://gitlist.dev/e/20121029071411.GA4229%40sigill.intra.peff.net
In-Reply-To: <CANQwDwd6EP94PEFkEcx8gBX1B5+-95qtjGMD6iU3ao8G+rCbLw@mail.gmail.com>

```
On Mon, Oct 29, 2012 at 08:12:46AM +0100, Jakub Narębski wrote:

> > Jakub, can you confirm the intent and a fix like the one above makes
> > things better?
> 
> Yes, either of those makes things better.

Thanks.

> >                                   (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.

Ah, that makes sense.

Richard, do you want to roll a patch that fixes it?

-Peff

```