git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gitweb: Optional grouping of projects by category
@ 2008-12-02 15:04 Sebastien Cevey
  2008-12-02 23:36 ` Jakub Narebski
  0 siblings, 1 reply; 24+ messages in thread
From: Sebastien Cevey @ 2008-12-02 15:04 UTC (permalink / raw)
  To: git; +Cc: jnareb, gitster, pasky

This adds the GITWEB_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.

The feature is inspired from Sham Chukoury's patch for the XMMS2
gitweb, but has been rewritten for the current gitweb development
HEAD.

Thanks to Florian Ragwitz for Perl tips.

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

I submitted a previous version of this patch on July 27, but was told
to wait for the end of the feature freeze.  I submitted it again on
September 5, but didn't get any reply.  Hope to be luckier this time!

This is a new version of the patch, which has been rebased onto the
current HEAD of the master branch.

 Makefile           |    2 +
 gitweb/README      |   11 +++
 gitweb/gitweb.css  |    5 ++
 gitweb/gitweb.perl |  173 +++++++++++++++++++++++++++++++++++-----------------
 4 files changed, 135 insertions(+), 56 deletions(-)

diff --git a/Makefile b/Makefile
index 649cfb8..a8e8bbf 100644
--- a/Makefile
+++ b/Makefile
@@ -211,6 +211,7 @@ GITWEB_EXPORT_OK =
 GITWEB_STRICT_EXPORT =
 GITWEB_BASE_URL =
 GITWEB_LIST =
+GITWEB_GROUP_CATEGORIES =
 GITWEB_HOMETEXT = indextext.html
 GITWEB_CSS = gitweb.css
 GITWEB_LOGO = git-logo.png
@@ -1189,6 +1190,7 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl
 	    -e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \
 	    -e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \
 	    -e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \
+	    -e 's|++GITWEB_GROUP_CATEGORIES++|$(GITWEB_GROUP_CATEGORIES)|g' \
 	    -e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \
 	    -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
 	    -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \
diff --git a/gitweb/README b/gitweb/README
index 825162a..a6f82c5 100644
--- a/gitweb/README
+++ b/gitweb/README
@@ -38,6 +38,11 @@ You can specify the following configuration variables when building GIT:
    using gitweb" in INSTALL file for gitweb to find out how to generate
    such file from scan of a directory. [No default, which means use root
    directory for projects]
+ * GITWEB_GROUP_CATEGORIES
+   Groups projects by category on the main projects list page if set
+   to true.  The category of a project is determined by the
+   $GIT_DIR/category file or the 'category' variable in its
+   configuration file.  [No default / Not set]
  * GITWEB_EXPORT_OK
    Show repository only if this file exists (in repository).  Only
    effective if this variable evaluates to true.  [No default / Not set]
@@ -188,6 +193,12 @@ 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.
+ * $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".
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index a01eac8..2dd45d6 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -264,6 +264,11 @@ td.current_head {
 	text-decoration: underline;
 }
 
+td.category {
+	padding-top: 1em;
+	font-weight: bold;
+}
+
 table.diff_tree span.file_status.new {
 	color: #008000;
 }
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 933e137..c1bcd96 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -87,6 +87,13 @@ 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
+our $projects_list_group_categories = "++GITWEB_GROUP_CATEGORIES++";
+
+# 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";
@@ -2001,18 +2008,28 @@ sub git_get_path_by_hash {
 ## ......................................................................
 ## git utility functions, directly accessing git repository
 
-sub git_get_project_description {
-	my $path = shift;
+sub git_get_project_config_from_file {
+	my ($name, $path) = @_;
 
 	$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_project_config_from_file('description', $path);
+}
+
+sub git_get_project_category {
+	my $path = shift;
+	return git_get_project_config_from_file('category', $path);
 }
 
 sub git_get_project_ctags {
@@ -3907,8 +3924,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) = @_;
@@ -3931,6 +3949,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->{'cat'}) {
+			my $cat = git_get_project_category($pr->{'path'}) ||
+			                                   $project_list_default_category;
+			$pr->{'cat'} = to_utf8($cat);
+		}
 		if ($check_forks) {
 			my $pname = $pr->{'path'};
 			if (($pname =~ s/\.git$//) &&
@@ -3948,6 +3971,19 @@ sub fill_project_list_info {
 	return @projects;
 }
 
+# returns a hash of categories, containing the list of project
+# belonging to each category
+sub build_category_list {
+	my ($projlist) = @_;
+	my %categories;
+
+	for my $pr (@{ $projlist }) {
+		push @{$categories{ $pr->{'cat'} }}, $pr;
+	}
+
+	return %categories;
+}
+
 # print 'sort by' <th> element, generating 'sort by $name' replay link
 # if that order is not selected
 sub print_sort_th {
@@ -3964,59 +4000,17 @@ sub print_sort_th {
 	}
 }
 
-sub git_project_list_body {
+sub print_project_rows {
 	# actually uses global variable $project
-	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
-
-	my ($check_forks) = gitweb_check_feature('forks');
-	my @projects = fill_project_list_info($projlist, $check_forks);
+	my ($projects, $from, $to, $check_forks, $show_ctags) = @_;
 
-	$order ||= $default_projects_order;
 	$from = 0 unless defined $from;
-	$to = $#projects if (!defined $to || $#projects < $to);
+	$to = $#$projects if (!defined $to || $#$projects < $to);
 
-	my %order_info = (
-		project => { key => 'path', type => 'str' },
-		descr => { key => 'descr_long', type => 'str' },
-		owner => { key => 'owner', type => 'str' },
-		age => { key => 'age', type => 'num' }
-	);
-	my $oi = $order_info{$order};
-	if ($oi->{'type'} eq 'str') {
-		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
-	} else {
-		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
-	}
-
-	my $show_ctags = gitweb_check_feature('ctags');
-	if ($show_ctags) {
-		my %ctags;
-		foreach my $p (@projects) {
-			foreach my $ct (keys %{$p->{'ctags'}}) {
-				$ctags{$ct} += $p->{'ctags'}->{$ct};
-			}
-		}
-		my $cloud = git_populate_project_tagcloud(\%ctags);
-		print git_show_project_tagcloud($cloud, 64);
-	}
-
-	print "<table class=\"project_list\">\n";
-	unless ($no_header) {
-		print "<tr>\n";
-		if ($check_forks) {
-			print "<th></th>\n";
-		}
-		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 "<th></th>\n" . # for links
-		      "</tr>\n";
-	}
 	my $alternate = 1;
 	my $tagfilter = $cgi->param('by_tag');
 	for (my $i = $from; $i <= $to; $i++) {
-		my $pr = $projects[$i];
+		my $pr = $projects->[$i];
 
 		next if $tagfilter and $show_ctags and not grep { lc $_ eq lc $tagfilter } keys %{$pr->{'ctags'}};
 		next if $searchtext and not $pr->{'path'} =~ /$searchtext/
@@ -4060,6 +4054,73 @@ sub git_project_list_body {
 		      "</td>\n" .
 		      "</tr>\n";
 	}
+}
+
+sub git_project_list_body {
+	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
+
+	my ($check_forks) = gitweb_check_feature('forks');
+	my @projects = fill_project_list_info($projlist, $check_forks);
+
+	$order ||= $default_projects_order;
+
+	my %order_info = (
+		project => { key => 'path', type => 'str' },
+		descr => { key => 'descr_long', type => 'str' },
+		owner => { key => 'owner', type => 'str' },
+		age => { key => 'age', type => 'num' }
+	);
+	my $oi = $order_info{$order};
+	if ($oi->{'type'} eq 'str') {
+		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
+	} else {
+		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
+	}
+
+	my $show_ctags = gitweb_check_feature('ctags');
+	if ($show_ctags) {
+		my %ctags;
+		foreach my $p (@projects) {
+			foreach my $ct (keys %{$p->{'ctags'}}) {
+				$ctags{$ct} += $p->{'ctags'}->{$ct};
+			}
+		}
+		my $cloud = git_populate_project_tagcloud(\%ctags);
+		print git_show_project_tagcloud($cloud, 64);
+	}
+
+	print "<table class=\"project_list\">\n";
+	unless ($no_header) {
+		print "<tr>\n";
+		if ($check_forks) {
+			print "<th></th>\n";
+		}
+		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 "<th></th>\n" . # for links
+		      "</tr>\n";
+	}
+
+	if ($projects_list_group_categories) {
+		my %categories = build_category_list(\@projects);
+		foreach my $cat (sort keys %categories) {
+			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);
+		}
+	} else {
+		print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
+	}
+
 	if (defined $extra) {
 		print "<tr>\n";
 		if ($check_forks) {
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] gitweb: Optional grouping of projects by category
  2008-12-02 15:04 [PATCH] gitweb: Optional grouping of projects by category Sebastien Cevey
@ 2008-12-02 23:36 ` Jakub Narebski
  2008-12-03 14:22   ` [PATCH v2] " Sébastien Cevey
                     ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Jakub Narebski @ 2008-12-02 23:36 UTC (permalink / raw)
  To: Sebastien Cevey
  Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

On Tue, 2 Dec 2008, Sebastien Cevey wrote:

> This adds the GITWEB_GROUP_CATEGORIES option which, if enabled, will
> result in grouping projects by category on the project list page.

Should this really be build time configuration (to set default value)?

> The category is specified for each project by the $GIT_DIR/category file
> or the 'category' variable in its configuration file.
> 
> The feature is inspired from Sham Chukoury's patch for the XMMS2
> gitweb, but has been rewritten for the current gitweb development
> HEAD.
> 
> Thanks to Florian Ragwitz for Perl tips.
> 
> Signed-off-by: Sebastien Cevey <seb@cine7.net>
> ---
> 
> I submitted a previous version of this patch on July 27, but was told
> to wait for the end of the feature freeze.  I submitted it again on
> September 5, but didn't get any reply.  Hope to be luckier this time!

Unfortunately it looks like you hit the edge of feature freeze again.
It is not announced yet, as far as I remember, but we are now at 
1.6.1-pre1.  I'll add this patch to my queue to resubmit after 1.6.1
is released, if it wouldn't be accepted in time.

By the way, there was alternate patch series by Gustavo Sverzut Barbieri
on 29 July 2008 adding categories support to gitweb:
  http://thread.gmane.org/gmane.comp.version-control.git/90553
with live demo at http://staff.get-e.org/
 
> This is a new version of the patch, which has been rebased onto the
> current HEAD of the master branch.
> 
>  Makefile           |    2 +
>  gitweb/README      |   11 +++
>  gitweb/gitweb.css  |    5 ++
>  gitweb/gitweb.perl |  173 +++++++++++++++++++++++++++++++++++-----------------
>  4 files changed, 135 insertions(+), 56 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 649cfb8..a8e8bbf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -211,6 +211,7 @@ GITWEB_EXPORT_OK =
>  GITWEB_STRICT_EXPORT =
>  GITWEB_BASE_URL =
>  GITWEB_LIST =
> +GITWEB_GROUP_CATEGORIES =
>  GITWEB_HOMETEXT = indextext.html
>  GITWEB_CSS = gitweb.css
>  GITWEB_LOGO = git-logo.png
> @@ -1189,6 +1190,7 @@ gitweb/gitweb.cgi: gitweb/gitweb.perl
>  	    -e 's|++GITWEB_STRICT_EXPORT++|$(GITWEB_STRICT_EXPORT)|g' \
>  	    -e 's|++GITWEB_BASE_URL++|$(GITWEB_BASE_URL)|g' \
>  	    -e 's|++GITWEB_LIST++|$(GITWEB_LIST)|g' \
> +	    -e 's|++GITWEB_GROUP_CATEGORIES++|$(GITWEB_GROUP_CATEGORIES)|g' \
>  	    -e 's|++GITWEB_HOMETEXT++|$(GITWEB_HOMETEXT)|g' \
>  	    -e 's|++GITWEB_CSS++|$(GITWEB_CSS)|g' \
>  	    -e 's|++GITWEB_LOGO++|$(GITWEB_LOGO)|g' \

O.K. That is just support for build time configuration of default value
(default configuration).

> diff --git a/gitweb/README b/gitweb/README
> index 825162a..a6f82c5 100644
> --- a/gitweb/README
> +++ b/gitweb/README
> @@ -38,6 +38,11 @@ You can specify the following configuration variables when building GIT:
>     using gitweb" in INSTALL file for gitweb to find out how to generate
>     such file from scan of a directory. [No default, which means use root
>     directory for projects]
> + * GITWEB_GROUP_CATEGORIES
> +   Groups projects by category on the main projects list page if set
> +   to true.  The category of a project is determined by the
> +   $GIT_DIR/category file or the 'category' variable in its
> +   configuration file.  [No default / Not set]
>   * GITWEB_EXPORT_OK
>     Show repository only if this file exists (in repository).  Only
>     effective if this variable evaluates to true.  [No default / Not set]
> @@ -188,6 +193,12 @@ 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.
> + * $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".

Nice documenting it, but I think you should also update "Per-repository
gitweb configuration" section in gitweb/README and mention category
(file) or gitweb.category repository configuration variable.

> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
> index a01eac8..2dd45d6 100644
> --- a/gitweb/gitweb.css
> +++ b/gitweb/gitweb.css
> @@ -264,6 +264,11 @@ td.current_head {
>  	text-decoration: underline;
>  }
>  
> +td.category {
> +	padding-top: 1em;
> +	font-weight: bold;
> +}
> +
>  table.diff_tree span.file_status.new {
>  	color: #008000;
>  }
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 933e137..c1bcd96 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -87,6 +87,13 @@ 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
> +our $projects_list_group_categories = "++GITWEB_GROUP_CATEGORIES++";
> +
> +# 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";
> @@ -2001,18 +2008,28 @@ sub git_get_path_by_hash {
>  ## ......................................................................
>  ## git utility functions, directly accessing git repository
>  
> -sub git_get_project_description {
> -	my $path = shift;
> +sub git_get_project_config_from_file {
> +	my ($name, $path) = @_;
>  
>  	$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_project_config_from_file('description', $path);
> +}
> +
> +sub git_get_project_category {
> +	my $path = shift;
> +	return git_get_project_config_from_file('category', $path);
>  }

I see you have resurrected (?) here git_get_project_config_from_file.
But I don't think it is correct name for this subroutine: it gets
config from a file in GIT_DIR _or_ from repository config.

Nevertheless nice that you avoided code duplication here.

>  
>  sub git_get_project_ctags {
> @@ -3907,8 +3924,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) = @_;
> @@ -3931,6 +3949,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->{'cat'}) {
> +			my $cat = git_get_project_category($pr->{'path'}) ||
> +			                                   $project_list_default_category;
> +			$pr->{'cat'} = to_utf8($cat);
> +		}
>  		if ($check_forks) {
>  			my $pname = $pr->{'path'};
>  			if (($pname =~ s/\.git$//) &&

O.K., although I wonder if we wouldn't spell key name in full, i.e as
$pr->{'category'}. But I think you follow conventions established
earlier here...

> @@ -3948,6 +3971,19 @@ sub fill_project_list_info {
>  	return @projects;
>  }
>  
> +# returns a hash of categories, containing the list of project
> +# belonging to each category
> +sub build_category_list {

I'm not sure about the name of this subroutine...

> +	my ($projlist) = @_;

I would use 

	my $projlist = shift;

here (or even $projects).

> +	my %categories;
> +
> +	for my $pr (@{ $projlist }) {

I would use simpler

	for my $pr (@$projlist) {

here

> +		push @{$categories{ $pr->{'cat'} }}, $pr;
> +	}
> +
> +	return %categories;
> +}
> +
>  # print 'sort by' <th> element, generating 'sort by $name' replay link
>  # if that order is not selected
>  sub print_sort_th {
> @@ -3964,59 +4000,17 @@ sub print_sort_th {
>  	}
>  }
>  

This chunk looks worse that it really is by accident...

> -sub git_project_list_body {

It would be nice to have description what this subroutine does.

> +sub print_project_rows {
>  	# actually uses global variable $project
> -	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
> -
> -	my ($check_forks) = gitweb_check_feature('forks');
> -	my @projects = fill_project_list_info($projlist, $check_forks);
> +	my ($projects, $from, $to, $check_forks, $show_ctags) = @_;
>  
> -	$order ||= $default_projects_order;
>  	$from = 0 unless defined $from;
> -	$to = $#projects if (!defined $to || $#projects < $to);
> +	$to = $#$projects if (!defined $to || $#$projects < $to);
>  
> -	my %order_info = (
> -		project => { key => 'path', type => 'str' },
> -		descr => { key => 'descr_long', type => 'str' },
> -		owner => { key => 'owner', type => 'str' },
> -		age => { key => 'age', type => 'num' }
> -	);
> -	my $oi = $order_info{$order};
> -	if ($oi->{'type'} eq 'str') {
> -		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
> -	} else {
> -		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
> -	}
> -
> -	my $show_ctags = gitweb_check_feature('ctags');
> -	if ($show_ctags) {
> -		my %ctags;
> -		foreach my $p (@projects) {
> -			foreach my $ct (keys %{$p->{'ctags'}}) {
> -				$ctags{$ct} += $p->{'ctags'}->{$ct};
> -			}
> -		}
> -		my $cloud = git_populate_project_tagcloud(\%ctags);
> -		print git_show_project_tagcloud($cloud, 64);
> -	}
> -
> -	print "<table class=\"project_list\">\n";
> -	unless ($no_header) {
> -		print "<tr>\n";
> -		if ($check_forks) {
> -			print "<th></th>\n";
> -		}
> -		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 "<th></th>\n" . # for links
> -		      "</tr>\n";
> -	}
>  	my $alternate = 1;
>  	my $tagfilter = $cgi->param('by_tag');
>  	for (my $i = $from; $i <= $to; $i++) {
> -		my $pr = $projects[$i];
> +		my $pr = $projects->[$i];
>  
>  		next if $tagfilter and $show_ctags and not grep { lc $_ eq lc $tagfilter } keys %{$pr->{'ctags'}};
>  		next if $searchtext and not $pr->{'path'} =~ /$searchtext/
> @@ -4060,6 +4054,73 @@ sub git_project_list_body {
>  		      "</td>\n" .
>  		      "</tr>\n";
>  	}
> +}
> +
> +sub git_project_list_body {
> +	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
> +
> +	my ($check_forks) = gitweb_check_feature('forks');
> +	my @projects = fill_project_list_info($projlist, $check_forks);
> +
> +	$order ||= $default_projects_order;
> +
> +	my %order_info = (
> +		project => { key => 'path', type => 'str' },
> +		descr => { key => 'descr_long', type => 'str' },
> +		owner => { key => 'owner', type => 'str' },
> +		age => { key => 'age', type => 'num' }
> +	);
> +	my $oi = $order_info{$order};
> +	if ($oi->{'type'} eq 'str') {
> +		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
> +	} else {
> +		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
> +	}
> +
> +	my $show_ctags = gitweb_check_feature('ctags');
> +	if ($show_ctags) {
> +		my %ctags;
> +		foreach my $p (@projects) {
> +			foreach my $ct (keys %{$p->{'ctags'}}) {
> +				$ctags{$ct} += $p->{'ctags'}->{$ct};
> +			}
> +		}
> +		my $cloud = git_populate_project_tagcloud(\%ctags);
> +		print git_show_project_tagcloud($cloud, 64);
> +	}
> +
> +	print "<table class=\"project_list\">\n";
> +	unless ($no_header) {
> +		print "<tr>\n";
> +		if ($check_forks) {
> +			print "<th></th>\n";
> +		}
> +		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 "<th></th>\n" . # for links
> +		      "</tr>\n";
> +	}
> +
> +	if ($projects_list_group_categories) {
> +		my %categories = build_category_list(\@projects);
> +		foreach my $cat (sort keys %categories) {
> +			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);
> +		}

Here is and important issue when adding categories support: how division
into categories interplays with sorting of projects.  I see that you
choose to always sort categories alphabetically, and to sort projects
by given column within categories, instead of (more complicated) sorting
categories by first project (first by given order) in a category.

This I think should be mentioned (at least briefly) in commit message.


Another issue is how categories interplay with limiting number of
projects displayed. Currently it is no issue, because projects list
page is not divided into pages, but I think you didn't address this
in your patch.

> +	} else {
> +		print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
> +	}
> +
>  	if (defined $extra) {
>  		print "<tr>\n";
>  		if ($check_forks) {
> -- 
> 1.5.6.5
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2] gitweb: Optional grouping of projects by category
  2008-12-02 23:36 ` Jakub Narebski
@ 2008-12-03 14:22   ` Sébastien Cevey
  2008-12-03 22:51     ` Junio C Hamano
  2008-12-03 14:29   ` [PATCH] gitweb: Optional grouping of projects by category Sébastien Cevey
  2008-12-03 21:14   ` Junio C Hamano
  2 siblings, 1 reply; 24+ messages in thread
From: Sébastien Cevey @ 2008-12-03 14:22 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

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, only the categories with
  projects in the chosen subset are displayed.

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.

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

Version 2 of the patch including corrections based on Jakub's comment,
notably removed the build time configuration variable, added more doc,
improved CSS look, cleaned some code and added support for displaying
a subset of all the projects (using $from/$to).

 gitweb/README      |   16 +++++
 gitweb/gitweb.css  |    7 ++
 gitweb/gitweb.perl |  190 +++++++++++++++++++++++++++++++++++++---------------
 3 files changed, 158 insertions(+), 55 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".
@@ -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
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 933e137..5b11b66 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";
@@ -2001,18 +2009,31 @@ 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.
+sub git_get_project_config_or_file {
+	my ($name, $path) = @_;
 
 	$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_project_config_or_file('description', $path);
+}
+
+sub git_get_project_category {
+	my $path = shift;
+	return git_get_project_config_or_file('category', $path);
 }
 
 sub git_get_project_ctags {
@@ -3907,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) = @_;
@@ -3931,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$//) &&
@@ -3948,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;
+}
+
 # print 'sort by' <th> element, generating 'sort by $name' replay link
 # if that order is not selected
 sub print_sort_th {
@@ -3964,16 +4004,71 @@ sub print_sort_th {
 	}
 }
 
-sub git_project_list_body {
+# print a row for each project in the given list, using the given
+# range and extra display options
+sub print_project_rows {
 	# actually uses global variable $project
+	my ($projects, $from, $to, $check_forks, $show_ctags) = @_;
+
+	$from = 0 unless defined $from;
+	$to = $#$projects if (!defined $to || $#$projects < $to);
+
+	my $alternate = 1;
+	my $tagfilter = $cgi->param('by_tag');
+	for (my $i = $from; $i <= $to; $i++) {
+		my $pr = $projects->[$i];
+
+		next if $tagfilter and $show_ctags and not grep { lc $_ eq lc $tagfilter } keys %{$pr->{'ctags'}};
+		next if $searchtext and not $pr->{'path'} =~ /$searchtext/
+			and not $pr->{'descr_long'} =~ /$searchtext/;
+		# Weed out forks or non-matching entries of search
+		if ($check_forks) {
+			my $forkbase = $project; $forkbase ||= ''; $forkbase =~ s#\.git$#/#;
+			$forkbase="^$forkbase" if $forkbase;
+			next if not $searchtext and not $tagfilter and $show_ctags
+				and $pr->{'path'} =~ m#$forkbase.*/.*#; # regexp-safe
+		}
+
+		if ($alternate) {
+			print "<tr class=\"dark\">\n";
+		} else {
+			print "<tr class=\"light\">\n";
+		}
+		$alternate ^= 1;
+		if ($check_forks) {
+			print "<td>";
+			if ($pr->{'forks'}) {
+				print "<!-- $pr->{'forks'} -->\n";
+				print $cgi->a({-href => href(project=>$pr->{'path'}, action=>"forks")}, "+");
+			}
+			print "</td>\n";
+		}
+		print "<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
+		                        -class => "list"}, esc_html($pr->{'path'})) . "</td>\n" .
+		      "<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
+		                        -class => "list", -title => $pr->{'descr_long'}},
+		                        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\">" .
+		      $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") . " | " .
+		      $cgi->a({-href => href(project=>$pr->{'path'}, action=>"tree")}, "tree") .
+		      ($pr->{'forks'} ? " | " . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"forks")}, "forks") : '') .
+		      "</td>\n" .
+		      "</tr>\n";
+	}
+}
+
+sub git_project_list_body {
 	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
 
 	my ($check_forks) = gitweb_check_feature('forks');
 	my @projects = fill_project_list_info($projlist, $check_forks);
 
 	$order ||= $default_projects_order;
-	$from = 0 unless defined $from;
-	$to = $#projects if (!defined $to || $#projects < $to);
 
 	my %order_info = (
 		project => { key => 'path', type => 'str' },
@@ -4013,53 +4108,38 @@ sub git_project_list_body {
 		print "<th></th>\n" . # for links
 		      "</tr>\n";
 	}
-	my $alternate = 1;
-	my $tagfilter = $cgi->param('by_tag');
-	for (my $i = $from; $i <= $to; $i++) {
-		my $pr = $projects[$i];
 
-		next if $tagfilter and $show_ctags and not grep { lc $_ eq lc $tagfilter } keys %{$pr->{'ctags'}};
-		next if $searchtext and not $pr->{'path'} =~ /$searchtext/
-			and not $pr->{'descr_long'} =~ /$searchtext/;
-		# Weed out forks or non-matching entries of search
-		if ($check_forks) {
-			my $forkbase = $project; $forkbase ||= ''; $forkbase =~ s#\.git$#/#;
-			$forkbase="^$forkbase" if $forkbase;
-			next if not $searchtext and not $tagfilter and $show_ctags
-				and $pr->{'path'} =~ m#$forkbase.*/.*#; # regexp-safe
-		}
+	if ($projects_list_group_categories) {
+		# only display categories with projects in the $from-$to window
+		my %categories = build_projlist_by_category(\@projects);
+		foreach my $cat (sort keys %categories) {
+			my $num_cats = @{$categories{$cat}};
 
-		if ($alternate) {
-			print "<tr class=\"dark\">\n";
-		} else {
-			print "<tr class=\"light\">\n";
-		}
-		$alternate ^= 1;
-		if ($check_forks) {
-			print "<td>";
-			if ($pr->{'forks'}) {
-				print "<!-- $pr->{'forks'} -->\n";
-				print $cgi->a({-href => href(project=>$pr->{'path'}, action=>"forks")}, "+");
+			# 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);
 			}
-			print "</td>\n";
+
+			# adjust $from/$to offset, keep $from positive
+			$from = ($from > $num_cats) ? $from - $num_cats : 0 if defined $from;
+			$to -= $num_cats if defined $to;
 		}
-		print "<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
-		                        -class => "list"}, esc_html($pr->{'path'})) . "</td>\n" .
-		      "<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
-		                        -class => "list", -title => $pr->{'descr_long'}},
-		                        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\">" .
-		      $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") . " | " .
-		      $cgi->a({-href => href(project=>$pr->{'path'}, action=>"tree")}, "tree") .
-		      ($pr->{'forks'} ? " | " . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"forks")}, "forks") : '') .
-		      "</td>\n" .
-		      "</tr>\n";
+	} else {
+		print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
 	}
+
 	if (defined $extra) {
 		print "<tr>\n";
 		if ($check_forks) {
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] gitweb: Optional grouping of projects by category
  2008-12-02 23:36 ` Jakub Narebski
  2008-12-03 14:22   ` [PATCH v2] " Sébastien Cevey
@ 2008-12-03 14:29   ` Sébastien Cevey
  2008-12-03 21:14   ` Junio C Hamano
  2 siblings, 0 replies; 24+ messages in thread
From: Sébastien Cevey @ 2008-12-03 14:29 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

At Wed, 3 Dec 2008 00:36:12 +0100, Jakub Narebski wrote:

Hello,

> > This adds the GITWEB_GROUP_CATEGORIES option which, if enabled, will
> > result in grouping projects by category on the project list page.
> 
> Should this really be build time configuration (to set default value)?

Probably not, I removed it.  I corrected my patch using all your other
comments and sent it again in another email.

> Unfortunately it looks like you hit the edge of feature freeze again.

Damn, time is always against me :-)
Hope it's gonna be included eventually!

> By the way, there was alternate patch series by Gustavo Sverzut Barbieri
> on 29 July 2008 adding categories support to gitweb:
>   http://thread.gmane.org/gmane.comp.version-control.git/90553
> with live demo at http://staff.get-e.org/

Right, I saw that in July but it's not exactly the same feature
(arbitrary category in config vs repository path).  I think both are
useful independently.. If both get supported, there should be a way to
do it modularly to reuse code though.

Btw you can see a demo of my patch here: http://inso.cc/git

> Here is and important issue when adding categories support: how division
> into categories interplays with sorting of projects.

I kept it how it was but added a comment in the commitlog.
 
> Another issue is how categories interplay with limiting number of
> projects displayed. Currently it is no issue, because projects list
> page is not divided into pages, but I think you didn't address this
> in your patch.

I think it's now better in v2, I added support for $from/$to also in
the categorized project list.

Thanks for your in depth comments, and I hope this patch will get
applied in the near future!

-- 
Sébastien Cevey / inso.cc

" The question of whether a computer can think is no more
  interesting than the question of whether a submarine can swim. "
Edsger W. Dijkstra  

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] gitweb: Optional grouping of projects by category
  2008-12-02 23:36 ` Jakub Narebski
  2008-12-03 14:22   ` [PATCH v2] " Sébastien Cevey
  2008-12-03 14:29   ` [PATCH] gitweb: Optional grouping of projects by category Sébastien Cevey
@ 2008-12-03 21:14   ` Junio C Hamano
  2 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2008-12-03 21:14 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Sebastien Cevey, git, Petr Baudis, Gustavo Sverzut Barbieri

Jakub Narebski <jnareb@gmail.com> writes:

> On Tue, 2 Dec 2008, Sebastien Cevey wrote:
> ...
>> I submitted a previous version of this patch on July 27, but was told
>> to wait for the end of the feature freeze.  I submitted it again on
>> September 5, but didn't get any reply.  Hope to be luckier this time!
>
> Unfortunately it looks like you hit the edge of feature freeze again.

Freeze for 1.6.1 by itself does not have to stop you or anybody to review
the patch and Ack for queuing for 'next' ;-)

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2] gitweb: Optional grouping of projects by category
  2008-12-03 14:22   ` [PATCH v2] " Sébastien Cevey
@ 2008-12-03 22:51     ` Junio C Hamano
  2008-12-03 23:12       ` Jakub Narebski
                         ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Junio C Hamano @ 2008-12-03 22:51 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: Jakub Narebski, git, Petr Baudis, Gustavo Sverzut Barbieri

Sébastien Cevey <seb@cine7.net> writes:

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

I am not sure if always sorting the categories alphabetically is a severe
enough restriction, but if it was, you can use @project_list_categories
array that disables the feature when empty and otherwise enumerates the
categories in order.  Just an idle thought.

> - When displaying a subset of all the projects, only the categories with
>   projects in the chosen subset are displayed.

Could you clarify this bit?

It is not very clear to me how this subset selection happens.  As far as I
can see, the user does not choose the category to view, but lets the usual
page limiting to decide at which project to start and stop placing on the
page, and only show the ones in the same category as the one that happened
to be the first on the page.

While I think both the introduction of "git_get_project_config_or_file"
which is a generalized interface usable between description and the new
feature and the refactoring of project_list_body into a seprate function
"print_project_rows" is a good idea, the patch would have been much easier
to read if these preparatory refactoring steps (without any new feature)
were done as a separate patch followed by the main patch to introduce the
new feature.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2] gitweb: Optional grouping of projects by category
  2008-12-03 22:51     ` Junio C Hamano
@ 2008-12-03 23:12       ` Jakub Narebski
  2008-12-04  0:39       ` [PATCH v3 0/3] " Sébastien Cevey
  2008-12-04  0:42       ` [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic Sébastien Cevey
  2 siblings, 0 replies; 24+ messages in thread
From: Jakub Narebski @ 2008-12-03 23:12 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Sébastien Cevey, git, Petr Baudis, Gustavo Sverzut Barbieri

On Wed, 3 Dec 2008, Junio C Hamano wrote:
> Sébastien Cevey <seb@cine7.net> writes:

> > 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.
> 
> I am not sure if always sorting the categories alphabetically is a severe
> enough restriction, but if it was, you can use @project_list_categories
> array that disables the feature when empty and otherwise enumerates the
> categories in order.  Just an idle thought.

Well, this way of sorting, i.e. keeping order of categories constant,
and changing order _within_ categories has the advantage that changing
order (the column we sort by) doesn't change page layout, i.e. in this
case doesn't change categories layout.


It means that if we had:

  project1
  project2
  [category A]
  A/pr1
  A/pr2
  [category B]
  B/pr1
  B/pr2
  B/pr3

when sorting by project name, we will have similarly looking

  project2
  project1
  [category A]
  A/pr1
  A/pr2
  [category B]
  B/pr3
  B/pr1
  B/pr2

for example if we order by age (not shown here)
 
> > - When displaying a subset of all the projects, only the categories with
> >   projects in the chosen subset are displayed.
> 
> Could you clarify this bit?
> 
> It is not very clear to me how this subset selection happens.  As far as I
> can see, the user does not choose the category to view, but lets the usual
> page limiting to decide at which project to start and stop placing on the
> page, and only show the ones in the same category as the one that happened
> to be the first on the page.

Currently the only selection is by tag, but lets assume that we have
some page length limit and only first N projects (first N in given sort
order) are shown.

If we have selection by tag, only those categories that have at least
one project tagged with selected tag, and only projects with given tag
would be shown. If we have page limit on projects list page, and we
sort for example by age, then only categories with freshest projects
will be shown.

At least it is how I understand it...

 
> While I think both the introduction of "git_get_project_config_or_file"

Errr... I think it is git_get_file_or_project_config ;-))))

> which is a generalized interface usable between description and the new
> feature and the refactoring of project_list_body into a seprate function
> "print_project_rows" is a good idea, the patch would have been much easier
> to read if these preparatory refactoring steps (without any new feature)
> were done as a separate patch followed by the main patch to introduce the
> new feature.
 
I agree, especially the second part, as adding print_project_rows patch
looks harder than it actually is.

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 0/3] gitweb: Optional grouping of projects by category
  2008-12-03 22:51     ` Junio C Hamano
  2008-12-03 23:12       ` Jakub Narebski
@ 2008-12-04  0:39       ` Sébastien Cevey
  2008-12-04  0:43         ` Junio C Hamano
  2008-12-04  0:42       ` [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic Sébastien Cevey
  2 siblings, 1 reply; 24+ messages in thread
From: Sébastien Cevey @ 2008-12-04  0:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, git, Petr Baudis, Gustavo Sverzut Barbieri

At Wed, 03 Dec 2008 14:51:07 -0800, Junio C Hamano wrote:

Good evening,

> I am not sure if always sorting the categories alphabetically is a
> severe enough restriction, but if it was, you can use
> @project_list_categories array that disables the feature when empty
> and otherwise enumerates the categories in order.  Just an idle
> thought.

I could add that if requested to, though Jakub seems to think
alphabetical order is also fine.  I'll be pushing v3 in a minute with
commits split by feature update and the rename to
git_get_file_or_project_config.

All this gymnastics helped me getting more familiar with add -i and
other fun workflow, thanks for the exercise[1] ;-)

> > - When displaying a subset of all the projects, only the categories with
> >   projects in the chosen subset are displayed.
> 
> Could you clarify this bit?
> 
> It is not very clear to me how this subset selection happens.  As far as I
> can see, the user does not choose the category to view, but lets the usual
> page limiting to decide at which project to start and stop placing on the
> page

Right,

> and only show the ones in the same category as the one that happened
> to be the first on the page.

I'm not sure I understand what you meant by this.

What I meant is that, as you said, the subset of projects to display
is defined by the usual $from/$to mechanism.  And that the category
header for all the displayed projects is present; in other words, the
header for some category C is shown iff one or more of the projects in
C is present on the page.  It's really that simple.

It also means that we might only see a subset of the projects in the
first and last category but hey, if you asked for range N-M of the
projects, that's what you get!

I hope I did not confuse you further.


[1] OT: I didn't find a simple command to do this:

    $ git diff ..the-end-state > finish.patch
    $ patch -p1 < finish.patch
    $ git commit -a -s

    (where the original HEAD and the-end-state have an older MRCA with
     rewritten history inbetween, and I don't want to apply that
     history and solve conflicts, just "get my tree to the end state".)

    Any tip?

-- 
Sébastien Cevey / inso.cc

" Rest is for the weak and the dead. "

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic
  2008-12-03 22:51     ` Junio C Hamano
  2008-12-03 23:12       ` Jakub Narebski
  2008-12-04  0:39       ` [PATCH v3 0/3] " Sébastien Cevey
@ 2008-12-04  0:42       ` Sébastien Cevey
  2008-12-04  0:44         ` [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions Sébastien Cevey
  2008-12-05  1:38         ` [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic Jakub Narebski
  2 siblings, 2 replies; 24+ messages in thread
From: Sébastien Cevey @ 2008-12-04  0:42 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski, Junio C Hamano, Petr Baudis,
	Gustavo Sverzut Barbieri

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 or else from 'gitweb.$variable' in the repository config
(e.g. 'description').

Signed-off-by: Sebastien Cevey <seb@cine7.net>
---
 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.
+sub git_get_file_or_project_config {
+	my ($name, $path) = @_;
 
 	$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);
 }
 
 sub git_get_project_ctags {
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 0/3] gitweb: Optional grouping of projects by category
  2008-12-04  0:39       ` [PATCH v3 0/3] " Sébastien Cevey
@ 2008-12-04  0:43         ` Junio C Hamano
  0 siblings, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2008-12-04  0:43 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: Jakub Narebski, git, Petr Baudis, Gustavo Sverzut Barbieri

Sébastien Cevey <seb@cine7.net> writes:

> [1] OT: I didn't find a simple command to do this:
>
>     $ git diff ..the-end-state > finish.patch
>     $ patch -p1 < finish.patch
>     $ git commit -a -s
>
>     (where the original HEAD and the-end-state have an older MRCA with
>      rewritten history inbetween, and I don't want to apply that
>      history and solve conflicts, just "get my tree to the end state".)
>
>     Any tip?

$ git read-tree -m -u the-end-state
$ git commit -a -s

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions
  2008-12-04  0:42       ` [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic Sébastien Cevey
@ 2008-12-04  0:44         ` Sébastien Cevey
  2008-12-04  0:46           ` [PATCH v3 3/3] gitweb: Optional grouping of projects by category Sébastien Cevey
  2008-12-05  1:52           ` [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions Jakub Narebski
  2008-12-05  1:38         ` [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic Jakub Narebski
  1 sibling, 2 replies; 24+ messages in thread
From: Sébastien Cevey @ 2008-12-04  0:44 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski, Junio C Hamano, Petr Baudis,
	Gustavo Sverzut Barbieri

Extract the printing of project rows on the project page into a
separate print_project_rows function. This makes it easier to reuse
the code to print different subsets of the whole project list.

The row printing code is merely moved into a separate function, but
note that $projects is passed as a reference now.

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

Boo, the diff is still quite scarier than it really should be..

 gitweb/gitweb.perl |  103 +++++++++++++++++++++++++++++-----------------------
 1 files changed, 57 insertions(+), 46 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b31274c..a6bb702 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3972,59 +3972,19 @@ sub print_sort_th {
 	}
 }
 
-sub git_project_list_body {
+# print a row for each project in the given list, using the given
+# range and extra display options
+sub print_project_rows {
 	# actually uses global variable $project
-	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
-
-	my ($check_forks) = gitweb_check_feature('forks');
-	my @projects = fill_project_list_info($projlist, $check_forks);
+	my ($projects, $from, $to, $check_forks, $show_ctags) = @_;
 
-	$order ||= $default_projects_order;
 	$from = 0 unless defined $from;
-	$to = $#projects if (!defined $to || $#projects < $to);
-
-	my %order_info = (
-		project => { key => 'path', type => 'str' },
-		descr => { key => 'descr_long', type => 'str' },
-		owner => { key => 'owner', type => 'str' },
-		age => { key => 'age', type => 'num' }
-	);
-	my $oi = $order_info{$order};
-	if ($oi->{'type'} eq 'str') {
-		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
-	} else {
-		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
-	}
+	$to = $#$projects if (!defined $to || $#$projects < $to);
 
-	my $show_ctags = gitweb_check_feature('ctags');
-	if ($show_ctags) {
-		my %ctags;
-		foreach my $p (@projects) {
-			foreach my $ct (keys %{$p->{'ctags'}}) {
-				$ctags{$ct} += $p->{'ctags'}->{$ct};
-			}
-		}
-		my $cloud = git_populate_project_tagcloud(\%ctags);
-		print git_show_project_tagcloud($cloud, 64);
-	}
-
-	print "<table class=\"project_list\">\n";
-	unless ($no_header) {
-		print "<tr>\n";
-		if ($check_forks) {
-			print "<th></th>\n";
-		}
-		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 "<th></th>\n" . # for links
-		      "</tr>\n";
-	}
 	my $alternate = 1;
 	my $tagfilter = $cgi->param('by_tag');
 	for (my $i = $from; $i <= $to; $i++) {
-		my $pr = $projects[$i];
+		my $pr = $projects->[$i];
 
 		next if $tagfilter and $show_ctags and not grep { lc $_ eq lc $tagfilter } keys %{$pr->{'ctags'}};
 		next if $searchtext and not $pr->{'path'} =~ /$searchtext/
@@ -4068,6 +4028,57 @@ sub git_project_list_body {
 		      "</td>\n" .
 		      "</tr>\n";
 	}
+}
+
+sub git_project_list_body {
+	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
+
+	my ($check_forks) = gitweb_check_feature('forks');
+	my @projects = fill_project_list_info($projlist, $check_forks);
+
+	$order ||= $default_projects_order;
+
+	my %order_info = (
+		project => { key => 'path', type => 'str' },
+		descr => { key => 'descr_long', type => 'str' },
+		owner => { key => 'owner', type => 'str' },
+		age => { key => 'age', type => 'num' }
+	);
+	my $oi = $order_info{$order};
+	if ($oi->{'type'} eq 'str') {
+		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
+	} else {
+		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
+	}
+
+	my $show_ctags = gitweb_check_feature('ctags');
+	if ($show_ctags) {
+		my %ctags;
+		foreach my $p (@projects) {
+			foreach my $ct (keys %{$p->{'ctags'}}) {
+				$ctags{$ct} += $p->{'ctags'}->{$ct};
+			}
+		}
+		my $cloud = git_populate_project_tagcloud(\%ctags);
+		print git_show_project_tagcloud($cloud, 64);
+	}
+
+	print "<table class=\"project_list\">\n";
+	unless ($no_header) {
+		print "<tr>\n";
+		if ($check_forks) {
+			print "<th></th>\n";
+		}
+		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 "<th></th>\n" . # for links
+		      "</tr>\n";
+	}
+
+	print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
+
 	if (defined $extra) {
 		print "<tr>\n";
 		if ($check_forks) {
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-04  0:44         ` [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions Sébastien Cevey
@ 2008-12-04  0:46           ` Sébastien Cevey
  2008-12-04 19:37             ` Junio C Hamano
  2008-12-05  2:08             ` Jakub Narebski
  2008-12-05  1:52           ` [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions Jakub Narebski
  1 sibling, 2 replies; 24+ messages in thread
From: Sébastien Cevey @ 2008-12-04  0:46 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski, Junio C Hamano, Petr Baudis,
	Gustavo Sverzut Barbieri

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.

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".
@@ -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
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";
@@ -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);
+}
+
 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$//) &&
@@ -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;
+}
+
 # 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);
+		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;
+		}
+	} else {
+		print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
+	}
 
 	if (defined $extra) {
 		print "<tr>\n";
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-04  0:46           ` [PATCH v3 3/3] gitweb: Optional grouping of projects by category Sébastien Cevey
@ 2008-12-04 19:37             ` Junio C Hamano
  2008-12-05  2:08             ` Jakub Narebski
  1 sibling, 0 replies; 24+ messages in thread
From: Junio C Hamano @ 2008-12-04 19:37 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: git, Jakub Narebski, Petr Baudis, Gustavo Sverzut Barbieri

Thanks, will queue so that they won't get lost in the noise.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic
  2008-12-04  0:42       ` [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic Sébastien Cevey
  2008-12-04  0:44         ` [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions Sébastien Cevey
@ 2008-12-05  1:38         ` Jakub Narebski
  1 sibling, 0 replies; 24+ messages in thread
From: Jakub Narebski @ 2008-12-05  1:38 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions
  2008-12-04  0:44         ` [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions Sébastien Cevey
  2008-12-04  0:46           ` [PATCH v3 3/3] gitweb: Optional grouping of projects by category Sébastien Cevey
@ 2008-12-05  1:52           ` Jakub Narebski
  1 sibling, 0 replies; 24+ messages in thread
From: Jakub Narebski @ 2008-12-05  1:52 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

On Tue, 4 Dec 2008 at 01:44, Sébastien Cevey wrote:

> Extract the printing of project rows on the project page into a
> separate print_project_rows function. This makes it easier to reuse
> the code to print different subsets of the whole project list.

Err... it was not obvious for me that 'project rows' are rows of
projects list table, i.e. currently the body of projects list table.

But I think it is a good description.

> 
> The row printing code is merely moved into a separate function, but
> note that $projects is passed as a reference now.
> 
> Signed-off-by: Sebastien Cevey <seb@cine7.net>

Nicely done.

For what it is worth:

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

> ---
> 
> Boo, the diff is still quite scarier than it really should be...

Well, nothing short of blame -C would make it work; the issue is that
we split subroutine into two, extracting from the middle, but with
some common/similar header

  2        1
  1        1
  2        1
  2
  2   ->   2
  1        2
  1        2
  2        2
  2        2
           2

And diff is forward preferring, i.e. it finds match then second part
of split as add, rather than first first part of split as add then
match.
> 
>  gitweb/gitweb.perl |  103 +++++++++++++++++++++++++++++-----------------------
>  1 files changed, 57 insertions(+), 46 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index b31274c..a6bb702 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3972,59 +3972,19 @@ sub print_sort_th {
>  	}
>  }
>  
> -sub git_project_list_body {
> +# print a row for each project in the given list, using the given
> +# range and extra display options
> +sub print_project_rows {
>  	# actually uses global variable $project
> -	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
> -
> -	my ($check_forks) = gitweb_check_feature('forks');
> -	my @projects = fill_project_list_info($projlist, $check_forks);
> +	my ($projects, $from, $to, $check_forks, $show_ctags) = @_;

I see that print_project_rows gets @$projects list already sorted, and
it passes explicitly $check_forks and $show_ctags from caller.

>  
> -	$order ||= $default_projects_order;
>  	$from = 0 unless defined $from;
> -	$to = $#projects if (!defined $to || $#projects < $to);
> -
> -	my %order_info = (
> -		project => { key => 'path', type => 'str' },
> -		descr => { key => 'descr_long', type => 'str' },
> -		owner => { key => 'owner', type => 'str' },
> -		age => { key => 'age', type => 'num' }
> -	);
> -	my $oi = $order_info{$order};
> -	if ($oi->{'type'} eq 'str') {
> -		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
> -	} else {
> -		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
> -	}
> +	$to = $#$projects if (!defined $to || $#$projects < $to);
>  
> -	my $show_ctags = gitweb_check_feature('ctags');
> -	if ($show_ctags) {
> -		my %ctags;
> -		foreach my $p (@projects) {
> -			foreach my $ct (keys %{$p->{'ctags'}}) {
> -				$ctags{$ct} += $p->{'ctags'}->{$ct};
> -			}
> -		}
> -		my $cloud = git_populate_project_tagcloud(\%ctags);
> -		print git_show_project_tagcloud($cloud, 64);
> -	}
> -
> -	print "<table class=\"project_list\">\n";
> -	unless ($no_header) {
> -		print "<tr>\n";
> -		if ($check_forks) {
> -			print "<th></th>\n";
> -		}
> -		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 "<th></th>\n" . # for links
> -		      "</tr>\n";
> -	}
>  	my $alternate = 1;
>  	my $tagfilter = $cgi->param('by_tag');
>  	for (my $i = $from; $i <= $to; $i++) {
> -		my $pr = $projects[$i];
> +		my $pr = $projects->[$i];
>  
>  		next if $tagfilter and $show_ctags and not grep { lc $_ eq lc $tagfilter } keys %{$pr->{'ctags'}};
>  		next if $searchtext and not $pr->{'path'} =~ /$searchtext/
> @@ -4068,6 +4028,57 @@ sub git_project_list_body {
>  		      "</td>\n" .
>  		      "</tr>\n";
>  	}
> +}
> +
> +sub git_project_list_body {
> +	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
> +
> +	my ($check_forks) = gitweb_check_feature('forks');
> +	my @projects = fill_project_list_info($projlist, $check_forks);
> +
> +	$order ||= $default_projects_order;
> +
> +	my %order_info = (
> +		project => { key => 'path', type => 'str' },
> +		descr => { key => 'descr_long', type => 'str' },
> +		owner => { key => 'owner', type => 'str' },
> +		age => { key => 'age', type => 'num' }
> +	);
> +	my $oi = $order_info{$order};
> +	if ($oi->{'type'} eq 'str') {
> +		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
> +	} else {
> +		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
> +	}
> +
> +	my $show_ctags = gitweb_check_feature('ctags');
> +	if ($show_ctags) {
> +		my %ctags;
> +		foreach my $p (@projects) {
> +			foreach my $ct (keys %{$p->{'ctags'}}) {
> +				$ctags{$ct} += $p->{'ctags'}->{$ct};
> +			}
> +		}
> +		my $cloud = git_populate_project_tagcloud(\%ctags);
> +		print git_show_project_tagcloud($cloud, 64);
> +	}
> +
> +	print "<table class=\"project_list\">\n";
> +	unless ($no_header) {
> +		print "<tr>\n";
> +		if ($check_forks) {
> +			print "<th></th>\n";
> +		}
> +		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 "<th></th>\n" . # for links
> +		      "</tr>\n";
> +	}
> +
> +	print_project_rows(\@projects, $from, $to, $check_forks, $show_ctags);
> +
>  	if (defined $extra) {
>  		print "<tr>\n";
>  		if ($check_forks) {
> -- 
> 1.5.6.5
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-04  0:46           ` [PATCH v3 3/3] gitweb: Optional grouping of projects by category Sébastien Cevey
  2008-12-04 19:37             ` Junio C Hamano
@ 2008-12-05  2:08             ` Jakub Narebski
  2008-12-05  2:32               ` Sébastien Cevey
  1 sibling, 1 reply; 24+ messages in thread
From: Jakub Narebski @ 2008-12-05  2:08 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-05  2:08             ` Jakub Narebski
@ 2008-12-05  2:32               ` Sébastien Cevey
  2008-12-05 10:45                 ` Jakub Narebski
  0 siblings, 1 reply; 24+ messages in thread
From: Sébastien Cevey @ 2008-12-05  2:32 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

At Fri, 5 Dec 2008 03:08:52 +0100, Jakub Narebski wrote:

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

Ah you're right, we could do that, I hadn't thought of it.  Sounds
cleaner than the $from/$to dance I did in this patch.

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

Mh indeed, in fact this could happen any time we reach one of the
'next' statements in the loop in print_project_rows(), when performing
search, tag filtering, etc...

Actually, assuming the project list is split into page, this can also
lead to empty pages (if no project on the page matches the filter).
To avoid empty categories, it's a bit tricky since the header is
printed before we determine whether there are matching projects..

I need to read the code more carefully, but it seems that one solution
would be to add a function that determines whether a project should be
displayed or not (according to tags and search and forks); then we can
map this function on the list of projects.

I could do it if it sounds sane to you.

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

AFAIK this cannot happen with the current code, since projects are
grouped by category (according to the foreach on categories).

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

This is the third patch, are you sure you applied 1 and 2 before?


Thanks for your careful and supportive comments!

-- 
Sébastien Cevey / inso.cc

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-05  2:32               ` Sébastien Cevey
@ 2008-12-05 10:45                 ` Jakub Narebski
  2008-12-11 23:34                   ` Sébastien Cevey
  0 siblings, 1 reply; 24+ messages in thread
From: Jakub Narebski @ 2008-12-05 10:45 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

On Fri, 5 Dec 2008 at 03:32, Sébastien Cevey wrote:
> At Fri, 5 Dec 2008 03:08:52 +0100, Jakub Narebski wrote:
> 
> > 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...
> 
> Ah you're right, we could do that, I hadn't thought of it.  Sounds
> cleaner than the $from/$to dance I did in this patch.

Usually simpler is better. The more complicated code, the more chances
for bugs, and less maintability. I haven't even tried to follow 
$from/$to updating logic, but noticed that we can simply pass $from
and $to to build_projlist_by_category(), and use loop $from..$to there.

> > 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.
> 
> Mh indeed, in fact this could happen any time we reach one of the
> 'next' statements in the loop in print_project_rows(), when performing
> search, tag filtering, etc...
> 
> Actually, assuming the project list is split into page, this can also
> lead to empty pages (if no project on the page matches the filter).
> To avoid empty categories, it's a bit tricky since the header is
> printed before we determine whether there are matching projects..
> 
> I need to read the code more carefully, but it seems that one solution
> would be to add a function that determines whether a project should be
> displayed or not (according to tags and search and forks); then we can
> map this function on the list of projects.
> 
> I could do it if it sounds sane to you.

Nah, I think it would be best to postpone dealing with this issue, and
keep the code simple at the cost of possibly empty categories. It is
not as empty categories are an actual error...

I guess that correct way to deal with this would be to filter projects
earlier, before grouping by category, and not "at the last minute", 
when displaying them. It might be even better solution wrt. dividing
projects list into pages. But that in my opinion is certainly matter
for a separate patch.

> > 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...
> 
> This is the third patch, are you sure you applied 1 and 2 before?
 
Somehow I couldn't apply second patch in series:

 $ git am -3 -u "[PATCH v3 2_3] gitweb: Split git_project_list_body in two functions.txt"
 Applying: gitweb: Split git_project_list_body in two functions
 error: patch failed: gitweb/gitweb.perl:3972
 error: gitweb/gitweb.perl: patch does not apply
 fatal: sha1 information is lacking or useless (gitweb/gitweb.perl).
 Repository lacks necessary blobs to fall back on 3-way merge.
 Cannot fall back to three-way merge.
 Patch failed at 0001.
 
> Thanks for your careful and supportive comments!

You are welcome. Nice to have another gitweb developer :-)
-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-05 10:45                 ` Jakub Narebski
@ 2008-12-11 23:34                   ` Sébastien Cevey
  2008-12-12  0:13                     ` Jakub Narebski
  0 siblings, 1 reply; 24+ messages in thread
From: Sébastien Cevey @ 2008-12-11 23:34 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

At Fri, 5 Dec 2008 11:45:08 +0100, Jakub Narebski wrote:

> > > 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...
> > 
> > Ah you're right, we could do that, I hadn't thought of it.  Sounds
> > cleaner than the $from/$to dance I did in this patch.
>
> [...] we can simply pass $from and $to to
> build_projlist_by_category(), and use loop $from..$to there.

I just tried, it works but we first need to sort @projects by
category.

> Somehow I couldn't apply second patch in series:
> 
>  $ git am -3 -u "[PATCH v3 2_3] gitweb: Split git_project_list_body in two functions.txt"
>  Applying: gitweb: Split git_project_list_body in two functions
>  error: patch failed: gitweb/gitweb.perl:3972
>  error: gitweb/gitweb.perl: patch does not apply
>  fatal: sha1 information is lacking or useless (gitweb/gitweb.perl).
>  Repository lacks necessary blobs to fall back on 3-way merge.
>  Cannot fall back to three-way merge.
>  Patch failed at 0001.

Where does it find the reference to the "necessary blobs"?  Could it
be because I didn't leave the From OBJECTID DATE line in my email
header?

Seems like there was a recent change on the line

  my $check_forks = gitweb_check_feature('forks');

so I had to update my patch to apply cleanly on the current master
HEAD.

I'm gonna re-send the three patches as a new version.

-- 
Sébastien Cevey / inso.cc

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-11 23:34                   ` Sébastien Cevey
@ 2008-12-12  0:13                     ` Jakub Narebski
  2008-12-12  0:40                       ` Sébastien Cevey
  0 siblings, 1 reply; 24+ messages in thread
From: Jakub Narebski @ 2008-12-12  0:13 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

On Fri, 12 Dec 2008, Sébastien Cevey wrote:
> At Fri, 5 Dec 2008 11:45:08 +0100, Jakub Narebski wrote:
> 
>>>> 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...
>>> 
>>> Ah you're right, we could do that, I hadn't thought of it.  Sounds
>>> cleaner than the $from/$to dance I did in this patch.
>>
>> [...] we can simply pass $from and $to to
>> build_projlist_by_category(), and use loop $from..$to there.
> 
> I just tried, it works but we first need to sort @projects by
> category.

I don't understand. You have the following code:

+# 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;
+}

I propose to change it to:

+# returns a hash of categories, containing the list of project
+# belonging to each category
+sub build_projlist_by_category {
+       my ($projlist, $from, $to) = shift;                       # <<<<<<<<<<<<<<<<<<<<<<<
+	$from = 0 unless defined $from;                           # +++++++++++++++++++++++
+	$to = $#$projlist if (!defined $to || $#$projlist < $to); # +++++++++++++++++++++++
+       my %categories;
+
+       for (my $i = $from; $i <= $to; $i++) {                    # <<<<<<<<<<<<<<<<<<<<<<<
+		$pr = $projlist->[$i];                            # +++++++++++++++++++++++
+               push @{$categories{ $pr->{'category'} }}, $pr;
+       }
+
+       return %categories;
+}

And of course update callers to pass $from and $to parameters.

This code doesn't change _anything_ beside the fact that it can operate
only on part of @$projlist.


But as you have noticed some kinds of limiting shown project list size
make sense only if done _after_ dividing into categories. Some but not
all. For example page length limiting after sorting by name, 
description, owner or age (sorting which takes place before dividing
into categories) makes sense if it is done _before_ categorization:
we want to show e.g. 100 freshest projects, and not up to 100 projects
from first categories, with last category showing maybe only freshest
projects in this category.


Besides making selection of projects to show operate on list of projects
instead of being done just before display is IMHO better, more flexible,
and more robust solution.

> I'm gonna re-send the three patches as a new version.

Thanks in advance.

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-12  0:13                     ` Jakub Narebski
@ 2008-12-12  0:40                       ` Sébastien Cevey
  2008-12-12  2:03                         ` Jakub Narebski
  0 siblings, 1 reply; 24+ messages in thread
From: Sébastien Cevey @ 2008-12-12  0:40 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

At Fri, 12 Dec 2008 01:13:45 +0100, Jakub Narebski wrote:

> I just tried, it works but we first need to sort @projects by
> category.
> 
> I don't understand.
> [...]
> I propose to change it to:

Well in my previous iteration of the patch (v3), the printing of
projects with categories is done using:

  foreach my $cat (sort keys %categories) {

So everything was already sorted by category (and then by whichever
property you picked inside each category).  You seemed okay with it,
but requested that I documented that behaviour in the commit log.

To maintain the same result with your proposed change (which is what I
submitted in my patch), we need to sort by categories first (AFAIK
Perl sort retains the original order inside equivalence classes of
comparison key?), otherwise splice(projlist, from, to) doesn't return
the expected subset.

-- 
Sébastien Cevey / inso.cc

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-12  0:40                       ` Sébastien Cevey
@ 2008-12-12  2:03                         ` Jakub Narebski
  2008-12-12  3:10                           ` Sébastien Cevey
  0 siblings, 1 reply; 24+ messages in thread
From: Jakub Narebski @ 2008-12-12  2:03 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

On Fri, 12 Dec 2008, Sébastien Cevey wrote:
> At Fri, 12 Dec 2008 01:13:45 +0100, Jakub Narebski wrote:
> 
> > I just tried, it works but we first need to sort @projects by
> > category.
> > 
> > I don't understand.
> > [...]
> > I propose to change it to:
> 
> Well in my previous iteration of the patch (v3), the printing of
> projects with categories is done using:
> 
>   foreach my $cat (sort keys %categories) {
> 
> So everything was already sorted by category (and then by whichever
> property you picked inside each category).  You seemed okay with it,
> but requested that I documented that behaviour in the commit log.

But this does not mean that sorting by categories is necessary, or
even wanted (see below). This foreach _sorts_ by categories as primary
key using kind of bucket sort algorithm.
 
> To maintain the same result with your proposed change (which is what I
> submitted in my patch), we need to sort by categories first (AFAIK
> Perl sort retains the original order inside equivalence classes of
> comparison key?), otherwise splice(projlist, from, to) doesn't return 
> the expected subset.

Perl requires "use sort 'stable';" pragma to ensure stable sort.

And no, we don't need to sort by categories first.  Let me explain
in more detail a bit.

Let us assume that $from and $to is actually used to divide projects
list into categories (which goal is incompatible with searching
projects, limiting to given tag/tagset and hiding forked as it is done
now, at the display time; it has to be done _before_ pagination).
They are used to display first page, i.e. repositories numbered 1..N
in current ordering, or N..2*N, or 2*N..3*N to show next pages. Let us
have the following project list:

  Project       Category
  ---------------------------
  1             a
  2             b
  3             b
  4             a

If categories are not shown, and page limit is 2, then project would
be displayed like this:


  page 1          page 2
  ------          ------
  1               3
  2               4

Now _without_ sorting by category upfront, those pages would look like
the following if grouping by category is enabled:

A.page 1          page 2
  ------          ------
  [a]             [a]
  1               4
  [b]             [b]
  2               3

What is not visible in this example is that projects inside category
would be sorted by given order.


Now if you would sort by categories _before_ pagination, like you
(from what I understand) proposed, you would have (assuming that
you used "use sort 'stable'" inside block):

  Project       Category
  ---------------------------
  1             a
  4             a
  2             b
  3             b

Pagination would then look like the following:

B.page 1          page 2
  ------          ------
  [a]             [b]
  1               2
  4               3


Now which result you consider correct depends on the point of view.
First is sort, paginate, sort; second is sort, sort, paginate.
First have first N repositories in given order on first page, perhaps
reordered a bit by categories, second doesn't have this feature.
I think that the case A is more correct, but you might disagree.


Let us change example a bit:

  Project       Category
  ---------------------------
  1             b
  2             a
  3             a
  4             b

A.page 1          page 2
  ------          ------
  [a]             [a]
  2               3
  [b]             [b]
  1               4

B.page 1          page 2
  ------          ------
  [a]             [b]
  2               1
  3               4

P.S. It is IMHO better to use

 	for (my $i = $from; $i <= $to; $i++) {

than the style which is not used elsewhere in gitweb, from what
I remember

 	foreach my $i ($from..$to) {

I might also be inefficient as it generates temporary array which
might be quite big; I don't know if Perl 5.8.x, the oldest version
one can sensibly use with gitweb I think, has this bug or not.
-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-12  2:03                         ` Jakub Narebski
@ 2008-12-12  3:10                           ` Sébastien Cevey
  2008-12-12  9:26                             ` Jakub Narebski
  0 siblings, 1 reply; 24+ messages in thread
From: Sébastien Cevey @ 2008-12-12  3:10 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

At Fri, 12 Dec 2008 03:03:55 +0100, Jakub Narebski wrote:

> And no, we don't need to sort by categories first.  Let me explain
> in more detail a bit.

Thanks for the detailed explanation, I understand your preference.
But as you said, it's a bit arbitrary, I think none is completely
obvious.


I don't really have a strong opinion about which is best, but just to
illustrate what made me go for the solution B, let me show another
example:

name / date / cat
1      2006    A
2      2003    B
3      2005    B
4      2003    A
5      2000    A
6      2008    C
7      2007    C
8      2001    B
9      2005    A

We then sort by name and split in pages of N=3 (sorted by cat on each
page):

A:sort(name) split(max=3) subsort(cat)
  1  2006  A     4  2003  A     9  2005  A
  2  2003  B     5  2000  A     8  2001  B
  3  2005  B     6  2008  C     7  2007  C

B:sort(cat) subsort(name) split(max=4)
  1  2006  A     9  2005  A     8  2001  B
  4  2003  A     2  2003  B     6  2008  C
  5  2000  A     3  2005  B     7  2007  C

With B, we have the second top-entry (2) relegated to the second page,
which might be surprising because we ordered by name.  But what I find
weird about A is that because of the per-page category sorting, the
"top-sorted entries at the top" isn't true either (page 3).  We have
"reshuffled" the result of 'sort(name) split(max=3)' on each page.

To be truly fair to the main sorting, we should not try to group
categories and display the header for each consecutive group of
entries from a distinct category:

A-:sort(name) split(max=3)
  1  2006  A     4  2003  A     7  2007  C
  2  2003  B     5  2000  A     8  2001  B
  3  2005  B     6  2008  C     9  2005  A

Which in this case is painless as it only affects page 3, but it could
lead to a mess of interleaved categories, and we kind of lose the
purpose of category groups in the first place..

The point is that with A, you cannot determine whether you're on the
right page to find project P (even if you know its category) by
checking whether it's in the interval between the top and bottom
entries.


It's perhaps even more apparent if we sort by date:

A:sort(year) split(max=3) subsort(cat)
  1  2006  A     9  2005  A     4  2003  A
  6  2008  C     3  2005  B     5  2000  A
  7  2007  C     2  2003  B     8  2001  B

B:sort(cat) subsort(year) split(max=4)
  1  2006  A     4  2003  A     3  2005  B
  9  2005  A     8  2001  B     7  2007  C
  5  2000  A     2  2003  B     6  2008  C

It feels kind of unnatural that not only projects are not sorted by
date on each page (they are inside categories), but moreover
categories are spread over all pages.


I guess it depends on your use case, and whether categories are
important or known by the user.  I personally don't really care (I
never split stuff into pages in the gitweb I use), so I can do a new
version of my patch that does A if you prefer, just let me know.  I
just wanted to clarify that both solutions sort of suck :-)

> P.S. It is IMHO better to use
> 
>  	for (my $i = $from; $i <= $to; $i++) {

Ah sorry, I took the words from your earlier email too literally ("we
can [...] use loop $from..$to there"). That's what happens when
pseudocode is actually valid syntax in a language!

Cheers,

-- 
Sébastien Cevey / inso.cc

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v3 3/3] gitweb: Optional grouping of projects by category
  2008-12-12  3:10                           ` Sébastien Cevey
@ 2008-12-12  9:26                             ` Jakub Narebski
  0 siblings, 0 replies; 24+ messages in thread
From: Jakub Narebski @ 2008-12-12  9:26 UTC (permalink / raw)
  To: Sébastien Cevey
  Cc: git, Junio C Hamano, Petr Baudis, Gustavo Sverzut Barbieri

On Fri, 12 Dec 2008, Sébastien Cevey wrote:
> At Fri, 12 Dec 2008 03:03:55 +0100, Jakub Narebski wrote:
> 
> > And no, we don't need to sort by categories first.  Let me explain
> > in more detail a bit.
> 
> Thanks for the detailed explanation, I understand your preference.
> But as you said, it's a bit arbitrary, I think none is completely
> obvious.

First, I feel a bit bad for derailing this patch. Currently gitweb
does not do pagination of projects list; it is not even possible in
a sane way with current way project searching/selecting is implemented.
So the whole build_projlist_by_category() respecting $from and $to is
moot point.

So if we don't use it, even if it is nice to have for the future, we
don't need to pay cost of extra stable sorting.

> 
> I don't really have a strong opinion about which is best, but just to
> illustrate what made me go for the solution B, let me show another
> example:
> 
> name / date / cat
> 1      2006    A
> 2      2003    B
> 3      2005    B
> 4      2003    A
> 5      2000    A
> 6      2008    C
> 7      2007    C
> 8      2001    B
> 9      2005    A
> 
> We then sort by name and split in pages of N=3 (sorted by cat on each
> page):
> 
> A:sort(name) split(max=3) subsort(cat)
>   1  2006  A     4  2003  A     9  2005  A
>   2  2003  B     5  2000  A     8  2001  B
>   3  2005  B     6  2008  C     7  2007  C
> 
> B:sort(cat) subsort(name) split(max=3)
>   1  2006  A     9  2005  A     8  2001  B
>   4  2003  A     2  2003  B     6  2008  C
>   5  2000  A     3  2005  B     7  2007  C
> 
> With B, we have the second top-entry (2) relegated to the second page,
> which might be surprising because we ordered by name.  But what I find
> weird about A is that because of the per-page category sorting, the
> "top-sorted entries at the top" isn't true either (page 3).  We have
> "reshuffled" the result of 'sort(name) split(max=3)' on each page.

[...]
> It's perhaps even more apparent if we sort by date:
> 
> A:sort(year) split(max=3) subsort(cat)
>   1  2006  A     9  2005  A     4  2003  A
>   6  2008  C     3  2005  B     5  2000  A
>   7  2007  C     2  2003  B     8  2001  B
> 
> B:sort(cat) subsort(year) split(max=4)
>   1  2006  A     4  2003  A     3  2005  B
>   9  2005  A     8  2001  B     7  2007  C
>   5  2000  A     2  2003  B     6  2008  C
> 
> It feels kind of unnatural that not only projects are not sorted by
> date on each page (they are inside categories), but moreover
> categories are spread over all pages.
> 
> 
> I guess it depends on your use case, and whether categories are
> important or known by the user.  I personally don't really care (I
> never split stuff into pages in the gitweb I use), so I can do a new
> version of my patch that does A if you prefer, just let me know.  I
> just wanted to clarify that both solutions sort of suck :-)

Well, with version A you can (I think) simply change

  foreach my $cat (sort keys %categories) {

to

  foreach my $cat (sort 
  	{ cmp_cat($projlist, \%categories, $oi, $a, $b) } keys %categories) {

to have the following output (see the difference on page 3)

A':sort(name) split(max=3) subsort(sort(cat,name))
  1  2006  A     4  2003  A     7  2007  C
  2  2003  B     5  2000  A     8  2001  B
  3  2005  B     6  2008  C     9  2005  A

where sort_cat might be something like (we took advantage that
categories in %categories have at least one project):

  sub cmp_cat {
  	my ($projlist, $cats, $oi, $a, $b) = @_;
  	my ($aa, $bb);
  	# projects in categories are sorted, so we can compare first
  	# project from a category to sort categories in given ordering
  	$aa = $projlist->{$cats->{$a}[0]};
  	$bb = $projlist->{$cats->{$b}[0]};
  	if ($oi->{'type'} eq 'str') {
		return $aa->{$oi->{'key'}} cmp $bb->{$oi->{'key'}};
	} else {
		return $aa->{$oi->{'key'}} <=> $bb->{$oi->{'key'}};
	}
  }

-- 
Jakub Narebski
Poland

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2008-12-12  9:27 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-12-02 15:04 [PATCH] gitweb: Optional grouping of projects by category Sebastien Cevey
2008-12-02 23:36 ` Jakub Narebski
2008-12-03 14:22   ` [PATCH v2] " Sébastien Cevey
2008-12-03 22:51     ` Junio C Hamano
2008-12-03 23:12       ` Jakub Narebski
2008-12-04  0:39       ` [PATCH v3 0/3] " Sébastien Cevey
2008-12-04  0:43         ` Junio C Hamano
2008-12-04  0:42       ` [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic Sébastien Cevey
2008-12-04  0:44         ` [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions Sébastien Cevey
2008-12-04  0:46           ` [PATCH v3 3/3] gitweb: Optional grouping of projects by category Sébastien Cevey
2008-12-04 19:37             ` Junio C Hamano
2008-12-05  2:08             ` Jakub Narebski
2008-12-05  2:32               ` Sébastien Cevey
2008-12-05 10:45                 ` Jakub Narebski
2008-12-11 23:34                   ` Sébastien Cevey
2008-12-12  0:13                     ` Jakub Narebski
2008-12-12  0:40                       ` Sébastien Cevey
2008-12-12  2:03                         ` Jakub Narebski
2008-12-12  3:10                           ` Sébastien Cevey
2008-12-12  9:26                             ` Jakub Narebski
2008-12-05  1:52           ` [PATCH v3 2/3] gitweb: Split git_project_list_body in two functions Jakub Narebski
2008-12-05  1:38         ` [PATCH v3 1/3] gitweb: Modularized git_get_project_description to be more generic Jakub Narebski
2008-12-03 14:29   ` [PATCH] gitweb: Optional grouping of projects by category Sébastien Cevey
2008-12-03 21:14   ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).