Lea Wiemann <lewiemann@xxxxxxxxx> writes:
> There was a bug in the implementation of the "next" links in
> format_paging_nav (for log and shortlog), which caused the next links
> to always be displayed, even if there is no next page. This fixes it.
Thanks for correcting this.
> sub format_paging_nav {
> - my ($action, $hash, $head, $page, $nrevs) = @_;
> + my ($action, $hash, $head, $page, $has_next_link) = @_;
> my $paging_nav;
>
>
> @@ -2774,7 +2774,7 @@ sub format_paging_nav {
> $paging_nav .= " ⋅ prev";
> }
>
> - if ($nrevs >= (100 * ($page+1)-1)) {
> + if ($has_next_link) {
> $paging_nav .= " ⋅ " .
> $cgi->a({-href => href(-replay=>1, page=>$page+1),
> -accesskey => "n", -title => "Alt-n"}, "next");
This makes logic much simpler. Nice change.
> @@ -4665,7 +4665,7 @@ sub git_log {
>
> my @commitlist = parse_commits($hash, 101, (100 * $page));
Here I have realized the source of this bug. Some time ago
git-rev-list acquired '--skip=<number>' option to have _git_ skip
commits and not _gitweb_, which improves performance a bit. It was
required to implement huge performance improvement, namely getting
details for all commits from a single command, otherwise the
performance improvement of calling one git command instead of
$page_size git commands would be much reduced by generating large
amount of data which would be skipped (wound't be used by gitweb).
Unfortunately this change wasn't reviewed carefully enough; old logic
to decide whether to add 'next' link compared (tried to compare)
number of commits receivied with number of commits requested (via
'--max-count=<number>' option). I guess that having format_paging_nav
decide whether to add "next" link was a bad idea...
> - my $paging_nav = format_paging_nav('log', $hash, $head, $page, (100 * ($page+1)));
> + my $paging_nav = format_paging_nav('log', $hash, $head, $page, $#commitlist >= 100);
>
I would agree with Junio here that @commitlist > 100 would be more
readable.
Logic goes as the following: we request ($page_size+1) revisions to
know if there are additional revisions, skipping ($page_size * $page)
revisions; gitweb adds 'next' link if it got more than $page_size
revisions.
> git_header_html();
> git_print_page_nav('log','', $hash,undef,undef, $paging_nav);
> @@ -5585,7 +5585,7 @@ sub git_shortlog {
>
> my @commitlist = parse_commits($hash, 101, (100 * $page));
>
> - my $paging_nav = format_paging_nav('shortlog', $hash, $head, $page, (100 * ($page+1)));
> + my $paging_nav = format_paging_nav('shortlog', $hash, $head, $page, $#commitlist >= 100);
> my $next_link = '';
> if ($#commitlist >= 100) {
> $next_link =
What about git_history()... oh, I see, it generates paging itself, and
soes not use format_paging_nav() subroutine. But I think it does not
exhibit mentioned (and corrected) error. BTW. I *guess* that with
href(-replay=>1, ...) gitweb could use format_paging_nav() also for
other pages...
--
Jakub Narebski
Poland
ShadeHawk on #git
--
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
