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

On Fri, 12 Dec 2008, Sébastien Cevey wrote:

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

quoted

quoted

quoted

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