[PATCHv3 2/3] gitweb: Option for filling only specified info in fill_project_list_info

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Enhance fill_project_list_info() subroutine to accept optional
parameters that specify which fields in project information needs to
be filled.  If none are specified then fill_project_list_info()
behaves as it used to, and ensure that all project info is filled.

This is in preparation of future lazy filling of project info in
project search and pagination of sorted list of projects.

Signed-off-by: Jakub Narebski <jnareb@xxxxxxxxx>
---
This could have been squashed with the next commit, but this way it is
pure refactoring that shouldn't change gitweb behavior, and hopefully
makes this patch easier to review.

Changes from v2:
* Instead of overloading project_info_needs_filling() to do both
  handling of which fields need to be filled, and which fields
  we want to have filled, just make its caller filter its arguments.

  This means that in this version of patch there are no changes to
  project_info_needs_filling() subroutine.

# Instead of handling both cases (the one with set of wanted keys,
  and the one where we want to fill all remaining info) in a single
  filtering subroutine, use two different anonymous subroutines
  to filter or not filter keys.

* Expanded comment about fill_project_list_info to include detailed
  description of when it does remove projects from returned list (and
  why it is), and examples for both types of usage of this subroutine.

* Rewritten major part of commit message, which in v2 was not updated
  to reflect new development, and referred to behavior in v1.

[Should I include changes from v1 here, in the future?]


On Mon, 20 Feb 2012, Junio C Hamano wrote
in http://thread.gmane.org/gmane.comp.version-control.git/190852/focus=191046
JH>
JH> I must say that the approach to put the set filtering logic inside
JH> project_info_needs_filling function smells like a bad API design.
JH>
JH> In other words, wouldn't a callchain that uses a more natural set of API
JH> functions be like this?
JH> 
JH>       # the top-level caller that is interested only in these two fields
JH>       fill_project_list_info($projlist, 'age', 'owner');
JH> 
JH>       # in fill_project_list_info()
JH>       my $projlist = shift;
JH>       my %wanted = map { $_ => 1 } @_;
JH> 
JH>       foreach my $pr (@$projlist) {
JH>               if (project_info_needs_filling($pr,
JH>                       filter_set(\%wanted, 'age', 'age_string'))) {

I think this might be an even better solution, instead of handling all
keys / selected keys in filter_set(), provide different $filter_set
subroutines:

        foreach my $pr (@$projlist) {
                  if (project_info_needs_filling($pr,
                          $filter_set->('age', 'age_string'))) {


 gitweb/gitweb.perl |   33 +++++++++++++++++++++++++--------
 1 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cdd84c7..7fb7a55 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5209,39 +5209,56 @@ sub project_info_needs_filling {
 	return;
 }
 
-# fills project list info (age, description, owner, category, forks)
+# fills project list info (age, description, owner, category, forks, etc.)
 # for each project in the list, removing invalid projects from
-# returned list
+# returned list, or fill only specified info.
+#
+# Invalid projects are removed from the returned list if and only if you
+# ask 'age' or 'age_string' to be filled, because they are the only fields
+# that run unconditionally git command that requires repository, and
+# therefore do always check if project repository is invalid.
+#
+# USAGE:
+# * fill_project_list_info(\@project_list, 'descr_long', 'ctags')
+#   ensures that 'descr_long' and 'ctags' fields are filled
+# * @project_list = fill_project_list_info(\@project_list)
+#   ensures that all fields are filled (and invalid projects removed)
+#
 # NOTE: modifies $projlist, but does not remove entries from it
 sub fill_project_list_info {
-	my $projlist = shift;
+	my ($projlist, @wanted_keys) = @_;
 	my @projects;
+	my $filter_set = sub { return @_; };
+	if (@wanted_keys) {
+		my %wanted_keys = map { $_ => 1 } @wanted_keys;
+		$filter_set = sub { return grep { $wanted_keys{$_} } @_; };
+	}
 
 	my $show_ctags = gitweb_check_feature('ctags');
  PROJECT:
 	foreach my $pr (@$projlist) {
-		if (project_info_needs_filling($pr, 'age', 'age_string')) {
+		if (project_info_needs_filling($pr, $filter_set->('age', 'age_string'))) {
 			my (@activity) = git_get_last_activity($pr->{'path'});
 			unless (@activity) {
 				next PROJECT;
 			}
 			($pr->{'age'}, $pr->{'age_string'}) = @activity;
 		}
-		if (project_info_needs_filling($pr, 'descr', 'descr_long')) {
+		if (project_info_needs_filling($pr, $filter_set->('descr', 'descr_long'))) {
 			my $descr = git_get_project_description($pr->{'path'}) || "";
 			$descr = to_utf8($descr);
 			$pr->{'descr_long'} = $descr;
 			$pr->{'descr'} = chop_str($descr, $projects_list_description_width, 5);
 		}
-		if (project_info_needs_filling($pr, 'owner')) {
+		if (project_info_needs_filling($pr, $filter_set->('owner'))) {
 			$pr->{'owner'} = git_get_project_owner("$pr->{'path'}") || "";
 		}
 		if ($show_ctags &&
-		    project_info_needs_filling($pr, 'ctags')) {
+		    project_info_needs_filling($pr, $filter_set->('ctags'))) {
 			$pr->{'ctags'} = git_get_project_ctags($pr->{'path'});
 		}
 		if ($projects_list_group_categories &&
-		    project_info_needs_filling($pr, 'category')) {
+		    project_info_needs_filling($pr, $filter_set->('category'))) {
 			my $cat = git_get_project_category($pr->{'path'}) ||
 			                                   $project_list_default_category;
 			$pr->{'category'} = to_utf8($cat);
-- 
1.7.9

--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]