Re: [PATCH] Allow gitweb tab width to be set per project.

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

>>>> +     $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).