Hi,
Jakub Narebski wrote:
> Extract filtering out forks (which is done if 'forks' feature is
> enabled) into filter_forks_from_projects_list subroutine
Context (to remind myself): the git_get_projects_list function decides
which repositories to show on the homepage. git_project_list_body
takes care of actually showing them in a table; it displays a "+" sign
in front of projects with forks and generally the project list it is
passed omits the forks themselves.
git_get_projects_list needs to be reasonably fast, since it is used
to validate the "project" parameter in every request.
[...]
> Both are now run _before_ displaying projects, and not while printing;
> this allow to know upfront if there were any found projects. Gitweb
> now can and do print 'No such projects found' if user searches for
> phrase which does not correspond to any project (any repository).
Aren't forks filtered out by git-get-projects-list already?
[...]
> Filtering out forks and marking repository (project) as having forks
> is now consolidated into one subroutine (special case of handling
> forks in git_get_projects_list only for $projects_list being file is
> now removed).
Ah, no, they aren't. That's a good change for consistency already.
> Forks handling is also cleaned up and simplified, which
> might mean some changes in output.
(Not having read the code yet) this part is not obvious to me. Maybe
an example could help.
[...]
> The interaction between forks, content tags and searching is now made
> more explicit: searching whether by tag, or via search form turns off
> fork filtering (gitweb searches also forks, and will show all
> results).
Hm, or maybe this is it.
> IMVHO the code is
> cleaner.
So some general code tidying is intermixed.
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2646,18 +2646,20 @@ sub git_get_projects_list {
[...]
> - my $dir = $projects_list . ($filter ? "/$filter" : '');
> + my $dir = $projects_list;
> # remove the trailing "/"
> $dir =~ s!/+$!!;
> - my $pfxlen = length("$dir");
> - my $pfxdepth = ($dir =~ tr!/!!);
> + my $pfxlen = length("$projects_list");
> + my $pfxdepth = ($projects_list =~ tr!/!!);
[...]
$dir is the same as before except that trailing slashes are removed,
and meanwhile the stripped names of directories returned from find()
will include $filter. Should be almost a no-op (i.e., a nice
simplification).
> @@ -2672,14 +2674,14 @@ sub git_get_projects_list {
> # only directories can be git repositories
> return unless (-d $_);
> # don't traverse too deep (Find is super slow on os x)
> + # $project_maxdepth excludes depth of $projectroot
> if (($File::Find::name =~ tr!/!!) - $pfxdepth > $project_maxdepth) {
A bugfix. $project_maxdepth is advertised to be relative to the
projectroot but it was relative to $projects_list/$filter.
[...]
> @@ -2703,32 +2704,9 @@ sub git_get_projects_list {
> if (!defined $path) {
> next;
> }
> - if ($filter ne '') {
> - # looking for forks;
> - my $pfx = substr($path, 0, length($filter));
> - if ($pfx ne $filter) {
> - next PROJECT;
> - }
> - my $sfx = substr($path, length($filter));
> - if ($sfx !~ /^\/.*\.git$/) {
> - next PROJECT;
> - }
[...]
> + # if $filter is rpovided, check if $path begins with $filter
s/rpovided/provided/ :)
> + if ($filter && $path !~ m!^\Q$filter\E/!) {
> + next;
Why did the code check for \Q.git\E$ before? If I'm not missing
something, I'm happy to see the condition go.
What if $filter is 0? Silly question, I guess.
[...]
> @@ -2745,6 +2721,67 @@ sub git_get_projects_list {
> return @list;
> }
>
> +# this assumes that forks follow immediately forked projects:
> +# [ ..., 'repo.git', 'repo/fork.git', ...]; if not, set second
> +# parameter to true value to sort projects by path first.
> +sub filter_forks_from_projects_list {
> + my ($projects, $need_sorting) = @_;
> +
> + @$projects = sort { $a->{'path'} cmp $b->{'path'} } @$projects
> + if ($need_sorting);
What happens in this scenario?
git.git
git.related.project.git
git/platforms.git
[...]
> @@ -4747,23 +4784,36 @@ sub fill_project_list_info {
> if (!defined $pr->{'owner'}) {
> $pr->{'owner'} = git_get_project_owner("$pr->{'path'}") || "";
> }
> - if ($check_forks) {
> - my $pname = $pr->{'path'};
> - if (($pname =~ s/\.git$//) &&
> - ($pname !~ /\/$/) &&
> - (-d "$projectroot/$pname")) {
> - $pr->{'forks'} = "-d $projectroot/$pname";
So currently we treat project.git as having forks when the
corresponding project/ directory exists, even when the latter is
empty. The proposed new rule is that project.git has forks if and
only if there is at least one project/foo.git repository. Sensible.
Thanks for a pleasant read.
Hope that helps,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html