Re: [PATCH] gitweb: Option to omit column with time of the last change

[Jakub Narebski <jnareb@gmail.com>]

On Tue, 3 Apr 2012, Kacper Kornet wrote:

> Generating information about last change for a large number of git
> repositories can be time consuming. This commit adds an option to
> omit 'Last Change' column when presenting the list of repositories.
>
This is quite a good idea, I think.

Even a better solution would be to add caching support to gitweb, but
first it is a long way from being ready, and second not in all cases
you are able to / wants to have a cache.
 
> Signed-off-by: Kacper Kornet <draenog@pld-linux.org>
> ---
>  Documentation/gitweb.conf.txt |    3 +++
>  gitweb/gitweb.perl            |   16 +++++++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt
> index 7aba497..bfeef21 100644
> --- a/Documentation/gitweb.conf.txt
> +++ b/Documentation/gitweb.conf.txt
> @@ -403,6 +403,9 @@ $default_projects_order::
>  	i.e. path to repository relative to `$projectroot`), "descr"
>  	(project description), "owner", and "age" (by date of most current
>  	commit).
> +
> +$no_list_age::
> +	Omit column with date of the most curren commit

s/curren/current/

Thanks for adding documentation, though I would prefer if you expanded
this description (for example including the information that it touches
projects list page).

>  +

This '+' here means "continuation".  You by accident inserted
description of new $no_list_age in the middle of description for
$default_projects_order variable...

>  Default value is "project".  Unknown value means unsorted.

...as you can see here.

>  
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a8b5fad..f42468c 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -133,6 +133,9 @@ our $default_projects_order = "project";
>  # (only effective if this variable evaluates to true)
>  our $export_ok = "++GITWEB_EXPORT_OK++";
>  
> +# don't generate age column
> +our $no_list_age = 0;

"age" column where?

Hmmm... can't we come with a better name than $no_list_age?

> +
>  # show repository only if this subroutine returns true
>  # when given the path to the project, for example:
>  #    sub { return -e "$_[0]/git-daemon-export-ok"; }
> @@ -5462,9 +5465,11 @@ sub git_project_list_rows {
>  		                        : esc_html($pr->{'descr'})) .
>  		      "</td>\n" .
>  		      "<td><i>" . chop_and_escape_str($pr->{'owner'}, 15) . "</i></td>\n";
> -		print "<td class=\"". age_class($pr->{'age'}) . "\">" .
> -		      (defined $pr->{'age_string'} ? $pr->{'age_string'} : "No commits") . "</td>\n" .
> -		      "<td class=\"link\">" .
> +		unless ($no_list_age) {
> +		        print "<td class=\"". age_class($pr->{'age'}) . "\">" .
> +		            (defined $pr->{'age_string'} ? $pr->{'age_string'} : "No commits") . "</td>\n";
> +		}
> +		print"<td class=\"link\">" .

O.K.  I guess that it ismore readable than

  +		print "<td class=\"". age_class($pr->{'age'}) . "\">" .
  +		      (defined $pr->{'age_string'} ? $pr->{'age_string'} : "No commits") . "</td>\n" .
  +		      "<td class=\"link\">"
  +			unless ($no_list_age);

>  		      $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary")}, "summary")   . " | " .
>  		      $cgi->a({-href => href(project=>$pr->{'path'}, action=>"shortlog")}, "shortlog") . " | " .
>  		      $cgi->a({-href => href(project=>$pr->{'path'}, action=>"log")}, "log") . " | " .
> @@ -5495,7 +5500,8 @@ sub git_project_list_body {
>  	                                 'tagfilter'  => $tagfilter)
>  		if ($tagfilter || $search_regexp);
>  	# fill the rest
> -	@projects = fill_project_list_info(\@projects);
> +	my @all_fields = $no_list_age ? ('descr', 'descr_long', 'owner', 'ctags', 'category') : ();
> +	@projects = fill_project_list_info(\@projects, @all_fields);

That looks quite strange on first glance.  I know that empty list means
filling all fields, but the casual reader migh wonder about this
conditional expression.

Perhaps it would be better to write it this way:

  -	@projects = fill_project_list_info(\@projects);
  +	my @fields = qw(descr descr_long owner ctags category);
  +	push @fields, 'age' unless ($no_list_age);
  +	@projects = fill_project_list_info(\@projects, @fields);

or something like that.

Well, at least until we come up with a better way to specify "all fields
except those specified".

>  
>  	$order ||= $default_projects_order;
>  	$from = 0 unless defined $from;
> @@ -5527,7 +5533,7 @@ sub git_project_list_body {
>  		print_sort_th('project', $order, 'Project');
>  		print_sort_th('descr', $order, 'Description');
>  		print_sort_th('owner', $order, 'Owner');
> -		print_sort_th('age', $order, 'Last Change');
> +		print_sort_th('age', $order, 'Last Change') unless $no_list_age;

  +		print_sort_th('age', $order, 'Last Change')
  +			unless $no_list_age;

might be more readable.

>  		print "<th></th>\n" . # for links
>  		      "</tr>\n";
>  	}
> -- 
> 1.7.10.rc3
> 

-- 
Jakub Narebski
Poland