git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Lea Wiemann <lewiemann@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] gitweb: only display "next" links in logs if there is a next page
Date: Tue, 27 May 2008 17:04:20 -0700	[thread overview]
Message-ID: <7v1w3nz43f.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <483C99A2.90909@gmail.com> (Lea Wiemann's message of "Wed, 28 May 2008 01:30:42 +0200")

Lea Wiemann <lewiemann@gmail.com> writes:

> Lea Wiemann wrote:
>> 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.
>
> Oh, one more thing I forgot to mention: I've tested this with a small
> (single-page) log page and a long log page.  In both cases the "next"
> links get formatted correctly, and they stop linking to the next page
> on the correct (= last) page.  The only thing I haven't tested for is
> off-by-one errors, but I'm reasonably sure that $#commitlist >= 100 is
> right.

I do not trust people who rely on "tests" to catch counting errors
(although I am pretty bad at counting myself and often testsuite helps me
catch my own bugs).  Instead, let's follow the code to see if it is Ok.

You call format_paging_nav() from two places, both after calling
parse_commits() with $maxcount = 101.  So @commitlist could have 101
elements (in which case you would need to have next page because you will
show only 100 among them), or smaller than that (in which case you will
fully show them).  In other words, you would want to say 'next' iff

	(@commitlist > 100)

is true.  On the other hand, $#commitlist is the last index of @commitlist
array, which is one smaller than the number of elements in that array.
IOW, when it has 101 elements, it has $commitlist[0] thru $commitlist[100]
and $#commitlist == 100.  So the above condition is the same as

	($#commitlist >= 100)

So your counting looks correct to me.

I wonder if it is easier to read to use the more modern @array notation
than historic $#array notation (it looks very Perl 4 to me), but that is a
separate issue.  gitweb is littered with such anachronism ;-)

  reply	other threads:[~2008-05-28  0:05 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-27 22:31 [PATCH] gitweb: only display "next" links in logs if there is a next page Lea Wiemann
2008-05-27 23:08 ` Junio C Hamano
2008-05-27 23:23   ` Lea Wiemann
2008-05-27 23:25     ` Lea Wiemann
2008-05-27 23:30       ` Lea Wiemann
2008-05-28  0:04         ` Junio C Hamano [this message]
2008-05-28  0:10           ` Lea Wiemann
2008-05-28  9:01       ` Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7v1w3nz43f.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=lewiemann@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).