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

On Tue, Sep 28, 2010 at 14:25, Jakub Narebski [off-list ref] wrote:

Magnus Hagander [off-list ref] writes:

quoted

Allow the gitweb.tabwidth option to control how many spaces a tab
gets expanded to.

Signed-off-by: Magnus Hagander <redacted>

This might be a good idea, but the solution looks like it includes
some unnecessary performance hit (see coment below).

quoted

---

In the PostgreSQL project, we're using 4-space tabs, but we have other projects
as well on our gitweb server, so we need to be able to control this on a
per-project basis.

Some of this comment should IMHO make it into commit message, e.g.

Check.

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

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?

In order to put it on all the callsites to untabify() it needs to for
example go as a parameter to format_diff_line to make any difference
(or it'd still be called once per line), so that would be quite a bit
of pollution I think.

Putting it as a global variable is certainly an option - but I didn't
find a pattern of something else to follow for that, so I figured it
was frowned upon?

We might want to turn 'tabwidth' into a feature, though that is
probably overengineering.

What advantage(s) would it give? (I admit I'm not particularly
familiar with the code - this was just the easiest way to get it done
quickly)

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.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/