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

["Sébastien Cevey" <seb@cine7.net>]

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