Subject: Re: [PATCH v5 2/5] gitweb: add project_filter to limit project list to a subdirectory

From: Jakub Narebski <[email-masked]>
Date: Mon, 30 Jan 2012 16:57:12 +0100

On Mon, 30 Jan 2012, Bernhard R. Link wrote:

> This commit changes the project listing views (project_list,
> project_index and opml) to limit the output to only projects in a
> subdirectory if the new optional parameter ?pf=directory name is
> used.

It would be nice to have in this or in a separate commit an update
to get_page_title() for HTML output, and to git_opml() updating
<title> element in OPML output, so that it mentions that project
list is limitied to $project_filter subdirectory.

For plain text output of git_project_index() nothing really can be
done -- there is no title.  Well, we could fiddle with 'filename'
part of Content-Disposition HTTP header...
 
> The implementation of the filter reuses the implementation used for
> the 'forks' action (i.e. listing all projects within that directory
> from the projects list file (GITWEB_LIST) or only projects in the
> given subdirectory of the project root directory without a projects
> list file).

O.K., more detailed description of $strict_export interaction is in
that other commit.

> Reusing $project instead of adding a new parameter would have been
> nicer from a UI point-of-view (including PATH_INFO support) but
> would complicate the $project validating code that is currently
> being used to ensure nothing is exported that should not be viewable.

Nb. I wonder if we should make it invalid to have both 'project' and
'project_filter' parameters...
 
> @@ -994,6 +995,13 @@ sub evaluate_and_validate_params {
>  		}
>  	}
>  
> +	our $project_filter = $input_params{'project_filter'};
> +	if (defined $project_filter) {
> +		if (!validate_pathname($project_filter)) {
> +			die_error(404, "Invalid project_filter parameter");
> +		}
> +	}
> +

That accidentally makes "pf=foo/" (with trailing slash) invalid.
On the other hand being able to assume that $project_filter doesn't
end in '/' simplifies code a bit.

> @@ -5984,7 +5992,7 @@ sub git_project_list {
> -	my @list = git_get_projects_list();
> +	my @list = git_get_projects_list($project_filter, $strict_export);

>  sub git_project_index {
> -	my @projects = git_get_projects_list();
> +	my @projects = git_get_projects_list($project_filter, $strict_export);

>  sub git_opml {
> -	my @list = git_get_projects_list();
> +	my @list = git_get_projects_list($project_filter, $strict_export);

Nice!

-- 
Jakub Narebski
Poland