Subject: Re: [PATCH] gitweb: The "Owner" column can be hidden

From: Jan Stępień <[email-masked]>
Date: Wed, 31 Mar 2010 15:16:48 +0200

On Wed, 31 Mar 2010 03:28:24 -0700 (PDT)
Jakub Narebski <[email-masked]> wrote:
> Jan Stępień  <[email-masked]> writes:
> 
> > This commit adds $show_owner_column configuration variable which allows
> > to hide the project list "Owner" column if desired.
> >
> 
> This commit message is a bit lacking, in that it does not describe
> *why* one would want to hide "Owner" column in projects list page.
> It is not described either why "Owner" column is unique among all
> other columns, in that it is the only column that can be hidden.
> 
> Besides for completeness you might want to hide "Owner" row in project
> summary page.
> 

The reason why I think gitweb should allow hiding the "Owner" row are
situations when its used for displaying a single person's repositories.
In such situation repeating the owner's name in each row may be
unnecessary in one's point of view and an option for hiding the whole
column could be a solution.

> > Signed-off-by: Jan Stępień <[email-masked]>
> > ---
> >  gitweb/gitweb.perl |    9 ++++++---
> >  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> While "Gitweb config file variables" section in gitweb/README does not
> need to include _all_ gitweb configuration variables, you should thing
> about it whether to include description of '$show_owner_column' (or
> just '$show_owner') in gitweb/README, and whether to include note
> about this variable in "Projects list file format" in this file.
> 

I agree with you on adding it to the "Gitweb config file variables"
section. On the other hand, I think that mentioning it in "Projects
list file format" isn't necessary unless '$show_owner' would affect the
way the projects list file would be parsed.

> > 
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index a2d2283..95f7f06 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -160,6 +160,9 @@ our @diff_opts = ('-M'); # taken from git_commit
> >  # the gitweb domain.
> >  our $prevent_xss = 0;
> >  
> > +# presense of the projects list "Owner" column
> 
> I'd rather have here
> 
> +# If false, hide "Owner" column on projects list page, and "Owner" row
> +# in the project summary page.
> 

Yes, that sounds better.

> > +our $show_owner_column = 1;
> 
> Perhaps it should be '$show_owner', or '$hide_owner'.
> 

Agreed. Given that displaying owner information is the default behaviour
I guess that "$hide_owner" or "$omit_owner" will be a better name for
it.

> BTW. if it would hide owner everywhere, you might want to skip
> generating / reading projects owners.
> 

Broadening the scope of this variable and hiding owner information also
on the project info page is more than I initially thought of, but it
seems like a good idea. In this case I'll add skipping owner generation
to the next version of the patch.

Thanks for comments!

Cheers,
-- 
Jan Stępień <[email-masked]>