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

[Jakub Narebski <jnareb@gmail.com>]

Magnus Hagander <magnus@hagander.net> writes:

> Allow the gitweb.tabwidth option to control how many spaces a tab
> gets expanded to.
> 
> Signed-off-by: Magnus Hagander <magnus@hagander.net>

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

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

  Different project scan use different tab widths, even on the same
  gitweb server.

Or something like that.

> 
> 
>  gitweb/gitweb.perl |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> 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...

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.

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

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

>  
>  	while ((my $pos = index($line, "\t")) != -1) {
> -		if (my $count = (8 - ($pos % 8))) {
> +		if (my $count = ($tabwidth - ($pos % $tabwidth))) {
>  			my $spaces = ' ' x $count;
>  			$line =~ s/\t/$spaces/;
>  		}
> -- 
> 1.7.0.4
> 

-- 
Jakub Narebski
Poland
ShadeHawk on #git