Re: [PATCH] Allow gitweb tab width to be set per project.
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Sep 29, 2010, 09:22 UTC
- Message-ID
- <201009291122.01272.jnareb@gmail.com>
- In-Reply-To
- <AANLkTikMjVQgEzLQ5Z95cmb5fkQ5iSzqfA4T=D1zzy=j@mail.gmail.com>
On Wed, 29 Sep 2010, Magnus Hagander wrote:
> On Tue, Sep 28, 2010 at 14:25, Jakub Narebski <jnareb@gmail.com> wrote: >> Magnus Hagander <magnus@hagander.net> writes:
Show 26 quoted lines
>>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>>> index a85e2f6..ef92a4f 100755
>>> --- a/gitweb/gitweb.perl
>>> +++ b/gitweb/gitweb.perl
>>> @@ -1465,9 +1465,11 @@ sub unquote {
>>> # escape tabs (convert tabs to spaces)
>>> sub untabify {
>>> my $line = shift;
>>> + my $tabwidth = git_get_project_config('tabwidth', '--int');
>>
>> Note that untabify() is called once for each _line_ in a file or a
>> diff...
>
> Ha, that's what I get for thinking it was too easy. It actually was :-)
>
>
>> This has acceptable performance only because gitweb config is cached
>> in %config hash by git_get_project_config() subroutine.
>>
>>
>> I'm not sure if it wouldn't be better to have $tabwidth be passed as
>> an (optional) argument to untabify(), and calculated either in calling
>> sites for untabify(), or be calculated per-request and save in a
>> global variable.
>
> Given that it's cached, will it actually make a big difference?Well, I agree that with config cached it could be left like this... but I would like very much to perhaps have a comment about this, so other people don't have to wonder.
Show 12 quoted lines
>>> + $tabwidth = 8 if ($tabwidth <= 0);
>>
>> git_get_project_config('tabwidth', '--int') can return 'undef' if a
>> configuration key does not exist, resulting in
>>
>> Use of uninitialized value in numeric le (<=) at
>>
>> warning in web server logs.
>
> Ah, I knew that would go somewhere. Interestingly enough, it doesn't
> show up in the logs of the server I run it on now. But still should be
> fixed.Whether such warning shows in web server logs might depend on whether you are running gitweb under mod_perl, or as plain CGI script. Nevertheless it is a good practice to check if a change passess appropriate tests from git testsuite; t9500-gitweb-standalone-no-errors should detect this.
Simply use
+ $tabwidth = 8 if (!defined $tabwidth || $tabwidth <= 0);
or
+ $tabwidth = 8 if (!$tabwidth || $tabwidth <= 0);
(though second version is more cryptic).
P.S. If it is not a %feature, we might want to add description of gitweb.tabwidth to the "Per-repository gitweb configuration" section in gitweb/README (as next to last item)
-- Jakub Narebski Poland