Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category

[Jakub Narebski <jnareb@gmail.com>]

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

> This adds the $projects_list_group_categories option which, if enabled,
> will result in grouping projects by category on the project list page.
> The category is specified for each project by the $GIT_DIR/category file
> or the 'category' variable in its configuration file. By default, projects
> are put in the $project_list_default_category category.
> 
> Note:
> - Categories are always sorted alphabetically, with projects in
>   each category sorted according to the globally selected $order.
> - When displaying a subset of all the projects (page limiting), the
>   category headers are only displayed for projects present on the page.
> 
> The feature is inspired from Sham Chukoury's patch for the XMMS2
> gitweb, but has been rewritten for the current gitweb development
> HEAD. The CSS for categories is inspired from Gustavo Sverzut Barbieri's
> patch to group projects by path.
> 
> Thanks to Florian Ragwitz for Perl tips.

Very nice, and nicely done and thought out, idea.

> 
> Signed-off-by: Sebastien Cevey <seb@cine7.net>
> ---
> 
> Cleaner patch this time indeed.  Still no fancy sorting of categories,
> only alphabetical.
> 
>  gitweb/README      |   16 ++++++++++++
>  gitweb/gitweb.css  |    7 +++++
>  gitweb/gitweb.perl |   67 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 87 insertions(+), 3 deletions(-)
> 
> diff --git a/gitweb/README b/gitweb/README
> index 825162a..f8f8872 100644
> --- a/gitweb/README
> +++ b/gitweb/README
> @@ -188,6 +188,15 @@ not include variables usually directly set during build):
>     full description is available as 'title' attribute (usually shown on
>     mouseover).  By default set to 25, which might be too small if you
>     use long project descriptions.
> + * $projects_list_group_categories
> +   Enables the grouping of projects by category on the project list page.
> +   The category of a project is determined by the $GIT_DIR/category
> +   file or the 'gitweb.category' variable in its repository configuration.
> +   Disabled by default.
> + * $project_list_default_category
> +   Default category for projects for which none is specified.  If set
> +   to the empty string, such projects will remain uncategorized and
> +   listed at the top, above categorized projects.
>   * @git_base_url_list
>     List of git base URLs used for URL to where fetch project from, shown
>     in project summary page.  Full URL is "$git_base_url/$project".

Good.

> @@ -269,6 +278,13 @@ You can use the following files in repository:
>     from the template during repository creation. You can use the
>     gitweb.description repo configuration variable, but the file takes
>     precedence.
> + * category (or gitweb.category)
> +   Singe line category of a project, used to group projects if
> +   $projects_list_group_categories is enabled. By default (file and
> +   configuration variable absent), uncategorized projects are put in
> +   the $project_list_default_category category. You can use the
> +   gitweb.category repo configuration variable, but the file takes
> +   precedence.
>   * cloneurl (or multiple-valued gitweb.url)
>     File with repository URL (used for clone and fetch), one per line.
>     Displayed in the project summary page. You can use multiple-valued

Good.

> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index a01eac8..64f2a41 100644
> --- a/gitweb/gitweb.css
> +++ b/gitweb/gitweb.css
> @@ -264,6 +264,13 @@ td.current_head {
>  	text-decoration: underline;
>  }
>  
> +td.category {
> +	background-color: #d9d8d1;
> +	border-top: 1px solid #000000;
> +	border-left: 1px solid #000000;
> +	font-weight: bold;
> +}
> +
>  table.diff_tree span.file_status.new {
>  	color: #008000;
>  }
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a6bb702..97a9b73 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -87,6 +87,14 @@ our $projects_list = "++GITWEB_LIST++";
>  # the width (in characters) of the projects list "Description" column
>  our $projects_list_description_width = 25;
>  
> +# group projects by category on the projects list
> +# (enabled if this variable evaluates to true)
> +our $projects_list_group_categories = 0;
> +
> +# default category if none specified
> +# (leave the empty string for no category)
> +our $project_list_default_category = "";
> +
>  # default order of projects list
>  # valid values are none, project, descr, owner, and age
>  our $default_projects_order = "project";

Nice.

> @@ -2023,6 +2031,11 @@ sub git_get_project_description {
>  	return git_get_file_or_project_config('description', $path);
>  }
>  
> +sub git_get_project_category {
> +	my $path = shift;
> +	return git_get_file_or_project_config('category', $path);
> +}
> +

Good. Nicely uses earlier patch, which adds this infrastructure.

>  sub git_get_project_ctags {
>  	my $path = shift;
>  	my $ctags = {};
> @@ -3915,8 +3928,9 @@ sub git_patchset_body {
>  
>  # . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
>  
> -# fills project list info (age, description, owner, forks) for each
> -# project in the list, removing invalid projects from returned list
> +# fills project list info (age, description, owner, category, forks)
> +# for each project in the list, removing invalid projects from
> +# returned list
>  # NOTE: modifies $projlist, but does not remove entries from it
>  sub fill_project_list_info {
>  	my ($projlist, $check_forks) = @_;
> @@ -3939,6 +3953,11 @@ sub fill_project_list_info {
>  		if (!defined $pr->{'owner'}) {
>  			$pr->{'owner'} = git_get_project_owner("$pr->{'path'}") || "";
>  		}
> +		if ($projects_list_group_categories && !defined $pr->{'category'}) {
> +			my $cat = git_get_project_category($pr->{'path'}) ||
> +			                                   $project_list_default_category;
> +			$pr->{'category'} = to_utf8($cat);
> +		}
>  		if ($check_forks) {
>  			my $pname = $pr->{'path'};
>  			if (($pname =~ s/\.git$//) &&

Nice. I see that you choose to go with $pr->{'category'} like existing
$pr->{'owner'}, rather than $pr->{'cat'} like existing $pr->{'descr'}.

> @@ -3956,6 +3975,19 @@ sub fill_project_list_info {
>  	return @projects;
>  }
>  
> +# returns a hash of categories, containing the list of project
> +# belonging to each category
> +sub build_projlist_by_category {
> +	my $projlist = shift;
> +	my %categories;
> +
> +	for my $pr (@$projlist) {
> +		push @{$categories{ $pr->{'category'} }}, $pr;
> +	}
> +
> +	return %categories;
> +}

This is very nice and simple way to group by categories, and it works
quite well with sorting (assuming that @$projlist is already sorted),
but I wonder how it works with $from / $to, i.e. with selecting
projects.

> +
>  # print 'sort by' <th> element, generating 'sort by $name' replay link
>  # if that order is not selected
>  sub print_sort_th {
> @@ -4077,7 +4109,36 @@ sub git_project_list_body {
>  		      "</tr>\n";
>  	}
>  
> -	print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
> +	if ($projects_list_group_categories) {
> +		# only display categories with projects in the $from-$to window
> +		my %categories = build_projlist_by_category(\@projects);

Nice... but perhaps it would be better to simply pass $from / $to to
build_projlist_by_category function, and have in %categories only
projects which are shown... well, unless filtered out in 
print_project_rows() by $show_ctags; so I think that there is nonzero
probability of empty (no project shown) categories.

> +		foreach my $cat (sort keys %categories) {
> +			my $num_cats = @{$categories{$cat}};
> +
> +			# out of the window to display, done
> +			last if defined $to and $to < 0;
> +
> +			# in the window to display
> +			if (!defined $from or $from < $num_cats) {
> +				unless ($cat eq "") {
> +					print "<tr>\n";
> +					if ($check_forks) {
> +						print "<td></td>\n";
> +					}
> +					print "<td class=\"category\" colspan=\"5\">$cat</td>\n";
> +					print "</tr>\n";
> +				}
> +
> +				print_project_rows($categories{$cat}, $from, $to, $check_forks, $show_ctags);
> +			}
> +
> +			# adjust $from/$to offset, keep $from positive
> +			$from = ($from > $num_cats) ? $from - $num_cats : 0 if defined $from;
> +			$to -= $num_cats if defined $to;

I don't think that the games we play with $from / $to would be enough.
Check what happens (I think that it wouldn't work correctly) if we have
something like that:

 project | category | shown
 --------------------------
  1      | A        |
  2      | A        |
  3      | B        | Y
  4      | C        | Y
  5      | B        | Y
  6      | C        |
  7      | C        |

It means that we display for example second page in projects list.

> +		}
> +	} else {
> +		print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
> +	}

Nice.

>  
>  	if (defined $extra) {
>  		print "<tr>\n";
> -- 
> 1.5.6.5
> 
> 

I'll try to examine the code in more detail later... currently I don't
know why but I can't git-am the second patch (this patch) correctly...

-- 
Jakub Narębski
Poland