Re: [PATCH v2 1/2] gitweb: add project_filter to limit project list to a subdirectory
From: Jakub Narebski <hidden>
Date: 2016-06-15 22:52:52
"Bernhard R. Link" [off-list ref] writes:
This commit changes the project listings (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.
"project listings" to "projects listing views", isn't it?
The change is quite minimal as git_get_projects_list already can limit itself to a subdirectory (though that was previously only used for 'forks').
Nice description, and more clear than before.
If strict_export is enabled and there is no projects_list, it still traverses the full tree and only filters afterwards to avoid anything getting visible by this. Otherwise only the subtree needs to be traversed, significantly reducing load times.
I still don't understand interaction between project_filter ('pf'),
$strict_export and $projects_list being either directory or a file
with a list of projects.
Does it mean, that when $projects_list is a file with a list of projects,
and we use project_filter, then:
* if $strict_export is false, then $project_list is ignored, and the
filtered list of projects is created by scanning
"$projectroot/$project_filter"
* if $strict_export is true, then $project_list file is read in full,
and then filtered to project with $project_filter as prefix
Is it correct? Is it sane, stated this way?
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.
O.K. Anyway PATH_INFO support can be added in the future, by special casing situation where project list action is stated using PATH_INFO... I think. A few nitpicks with respect to patch itself.
quoted hunk ↗ jump to hunk
@@ -2827,6 +2835,7 @@ sub git_get_project_url_list { sub git_get_projects_list { my $filter = shift || ''; + my $paranoid = shift || 0; my @list;
First, undefined value is false in Perl, so there is no need for " || 0" in setting $paranoid variable. Second, why not use global variable $strict_export instead of adding another parameter to git_get_projects_list()?
quoted hunk ↗ jump to hunk
@@ -5979,7 +5994,7 @@ sub git_project_list { die_error(400, "Unknown order parameter"); } - my @list = git_get_projects_list(); + my @list = git_get_projects_list($project_filter, $strict_export); if (!@list) { die_error(404, "No projects found"); }
[...]
quoted hunk ↗ jump to hunk
@@ -3963,9 +3976,11 @@ sub git_footer_html { } } else { - print $cgi->a({-href => href(project=>undef, action=>"opml"), + print $cgi->a({-href => href(project=>undef, action=>"opml", + project_filter => $project_filter), -class => $feed_class}, "OPML") . " "; - print $cgi->a({-href => href(project=>undef, action=>"project_index"), + print $cgi->a({-href => href(project=>undef, action=>"project_index", + project_filter => $project_filter), -class => $feed_class}, "TXT") . "\n"; }
O.K. -- Jakub Narebski