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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Magnus Hagander <magnus@xxxxxxxxxxxx> writes:

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

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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]