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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 30, 2012, 20:34 UTC
Message-ID: <7v39axaq0v.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120130200355.GA2584@server.brlink.eu>

"Bernhard R. Link" <brl@mail.brlink.eu> writes:

> I'll resend the series as replies to this mail.

Thanks; I'll queue them in 'pu' for now (if Jakub wants to Ack the pieces, I'll amend them).

Regarding the first patch in the series, while it may be a valid perl to introduce a new variable, assign to it and then munge its contents with s///, all inside a parameter list of a function call, it is doing a bit too much and makes it hard to see if the variable may or may not later be used in the same scope (in this case, it is not).

I am tempted to squash the following in.

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index b764d51..f215eaa 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6003,7 +6003,8 @@ sub git_forks {
 		die_error(400, "Unknown order parameter");
 	}
 
-	my @list = git_get_projects_list((my $filter = $project) =~ s/\.git$//);
+	my ($filter = $project) =~ s/\.git$//;
+	my @list = git_get_projects_list($filter);
 	if (!@list) {
 		die_error(404, "No forks found");
 	}
@@ -6062,7 +6063,8 @@ sub git_summary {
 
 	if ($check_forks) {
 		# find forks of a project
-		@forklist = git_get_projects_list((my $filter = $project) =~ s/\.git$//);
+		my ($filter = $project) =~ s/\.git$//;
+		@forklist = git_get_projects_list($filter);
 		# filter out forks of forks
 		@forklist = filter_forks_from_projects_list(\@forklist)
 			if (@forklist);

-- 
1.7.9.154.g413bff