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 [off-list ref] wrote:

quoted

Magnus Hagander [off-list ref] writes:

quoted

quoted

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

quoted

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.
 

quoted

quoted

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