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

[Kacper Kornet <draenog@pld-linux.org>]

On Wed, Apr 04, 2012 at 01:12:01AM +0200, Jakub Narebski wrote:
> On Tue, 3 Apr 2012, Kacper Kornet wrote:
> > 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).

What about:

$no_list_age::
	Whether to show the column with date of the most current commit on the
	projects list page. It can save a bit of I/O.

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

Any of $no_age_column, $omit_age_column, $no_last_commit would be better?


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

Yes, that's better. Especially that I would like also to introduce
option to prevent printing repository owner everywhere.

-- 
  Kacper Kornet