Re: [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic

[Jakub Narebski <jnareb@gmail.com>]

On Thu, 4 Dec 2008, at 01:42, Sébastien Cevey wrote:

> Introduce a git_get_file_or_project_config utility function to
> retrieve a repository variable either from a plain text file in the
> $GIT_DIR 

I would say that we try $GIT_DIR/$variable file.

> or else from 'gitweb.$variable' in the repository config 
> (e.g. 'description').

It _might_ also be added (just in case) that currently the only user
of this new subroutine is git_get_project_description, but this is
to change, and that is why this split was introduced.

> 
> Signed-off-by: Sebastien Cevey <seb@cine7.net>

But those are minor issues. So, FWIW

Acked-by: Jakub Narebski <jnareb@gmail.com>

> ---
>  gitweb/gitweb.perl |   24 ++++++++++++++++--------
>  1 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 933e137..b31274c 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2001,18 +2001,26 @@ sub git_get_path_by_hash {
>  ## ......................................................................
>  ## git utility functions, directly accessing git repository
>  
> -sub git_get_project_description {
> -	my $path = shift;
> +# get the value of a config variable either from a file with the same
> +# name in the repository, or the gitweb.$name value in the repository
> +# config file.

It would probably be better to explicitly say that we use $git_dir/$name
file, or if it doesn't exist, gitweb.$name configuration variable.

> +sub git_get_file_or_project_config {
> +	my ($name, $path) = @_;

I think that $project, or $projectpath _might_ be better name for the
second argument to this subroutine.

>  
>  	$git_dir = "$projectroot/$path";
> -	open my $fd, "$git_dir/description"
> -		or return git_get_project_config('description');
> -	my $descr = <$fd>;
> +	open my $fd, "$git_dir/$name"
> +		or return git_get_project_config($name);
> +	my $conf = <$fd>;
>  	close $fd;
> -	if (defined $descr) {
> -		chomp $descr;
> +	if (defined $conf) {
> +		chomp $conf;
>  	}
> -	return $descr;
> +	return $conf;
> +}
> +
> +sub git_get_project_description {
> +	my $path = shift;
> +	return git_get_file_or_project_config('description', $path);
>  }

Nicely done.

>  
>  sub git_get_project_ctags {
> -- 
> 1.5.6.5
> 
> 

-- 
Jakub Narębski
Poland