On 22-08-12 12:24PM, Omar Polo wrote:
> On 2022/08/10 10:49:04 +1000, Mark Jamsek <mark@jamsek.com> wrote:
> > On 22-08-10 12:49AM, Mark Jamsek wrote:
> > > We incorrectly reassign when we should be incrementing the selected
> > > index. As a result, if the currently selected commit is less than
> > > nlines/2 from the last commit and we use 'd' to scroll down half
> > > a page, we jump up!
> > > 
> > > repro:
> > >   $ tog  # 80x24 in got.git
> > >   G
> > >   5k
> > >   d
> > >   *selection moves up 6 lines instead of moving to the last commit!*
> 
> it took me a while to understand what was wrong.  I guess it's one of
> those cases where it's easier to understad after trying the diff :D
> 
> > > Relatedly, I've simplified the page down logic a bit so that if the last
> > > commit is already on screen when in a hsplit, we move the selection
> > > cursor down rather than scroll. This required accounting for the border
> > > when drawing commits in a horizontal split, which was previously missed,
> > > to ensure last_displayed_entry->idx is correct.
> > 
> > Rebased and revised diff fixes a bug in the previous diff: make sure the
> > last displayed commit is the last commit in the log--not just the last
> > of the currently queued commits.
> > 
> > > ok?
> 
> the diff reads fine, however I'm a bit confused.  With your patch
> applied when I'm at the end of the logs `d' moves selection down, but
> otherwise it *scrolls* the page.  Before, it always scrolled.
> 
> Which one is the correct behaviour?

Yes, this change is deliberate and it was actually the previous
behaviour before we added hsplits. If you checkout
c71ed39a66ed8e3aeaa13e76fce179b7bf6efec6 you'll see it behaves the same
as this diff proposes. I also think it's a smoother experience too so am
a little biased :)

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68