Re: [PATCH 1/2] gitweb: Make it possible to paginate projects

From: Jakub Narebski <jnareb@gmail.com>
Date: Sep 10, 2010, 19:10 UTC
Message-ID: <m3zkvpl8jf.fsf@localhost.localdomain>
In-Reply-To: <1284135442-10971-2-git-send-email-lkundrak@v3.sk>

Lubomir Rintel <lkundrak@v3.sk> writes:

> This adds simple pagination (next and prev links), to project lists,
> analogous to what is done for commit history lists.

Lack signoff (see Documentation/SubmittingPatches).

Show 15 quoted lines

> ---
>  gitweb/gitweb.perl |   26 ++++++++++++++++++++++++++
>  1 files changed, 26 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a85e2f6..8dc7f29 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -255,6 +255,9 @@ our %highlight_ext = (
>  	map { $_ => 'xml' } qw(xhtml html htm),
>  );
>  
> +# Set this to non-zero to enable project list pagination
> +our $projects_per_page = 0;
> +

Good idea of being able to enable or disable pagination of projects list.

I am not sure though if this is the correct solution. First, pagination in all other places is hardcoded to 100 items per page; IMHO for consistency it would be good to use the same page size everywhere.

Second, perhaps instead of yet another global variable a better solution would be non-verridable %feature, like 'pathinfo' or 'forks' features?

Show 12 quoted lines

>  # You define site-wide feature defaults here; override them with
>  # $GITWEB_CONFIG as necessary.
>  our %feature = (
> @@ -4613,9 +4616,19 @@ sub git_project_list_body {
>  	my @projects = fill_project_list_info($projlist, $check_forks);
>  
>  	$order ||= $default_projects_order;
> +	$page ||= 0;
> +	if ($projects_per_page) {
> +		$from = $page * $projects_per_page unless defined $from;
> +		$to = $from + $projects_per_page - 1 unless defined $to;
> +	}

Hmmm...

Show 8 quoted lines

>  	$from = 0 unless defined $from;
>  	$to = $#projects if (!defined $to || $#projects < $to);
>  
> +	my $prev_link = $cgi->a({-href => href(-replay=>1, page=>$page-1),
> +		 -accesskey => "p", -title => "Alt-p"}, "prev") if ($page > 0);
> +	my $next_link = $cgi->a({-href => href(-replay=>1, page=>$page+1),
> +		 -accesskey => "n", -title => "Alt-n"}, "next") if ($#$projlist > $to);
> +

In other places we have 'first' &sdot; 'prev' &sdot; 'next'...

Show 19 quoted lines

>  	my %order_info = (
>  		project => { key => 'path', type => 'str' },
>  		descr => { key => 'descr_long', type => 'str' },
> @@ -4709,6 +4722,19 @@ sub git_project_list_body {
>  		print "<td colspan=\"5\">$extra</td>\n" .
>  		      "</tr>\n";
>  	}
> +
> +	if ($prev_link or $next_link) {
> +		print "<tr>\n";
> +		if ($check_forks) {
> +			print "<td></td>\n";
> +		}
> +		print "<td colspan=\"5\">";
> +		print $prev_link if $prev_link;
> +		print " &sdot; " if $prev_link and $next_link;
> +		print $next_link if $next_link;
> +		print "</td>\n</tr>\n";
> +	}

... is there a reason to not use format_paging_nav() subroutine?

Show 7 quoted lines

> +
>  	print "</table>\n";
>  }
>  
> -- 
> 1.7.2.1
>

See also comments to next patch in series

-- 
Jakub Narebski
Poland
ShadeHawk on #git