On 2024/07/03 00:14:04 +1000, Mark Jamsek <mark@jamsek.com> wrote:
> This is a fix of another diff view J keymap case that presents when
> selecting the last visible commit in the horizontally split log view and
> then toggling fullscreen. The view fails to load the next commit when
> entering the J keymap due to too broad a predicate; the line selection
> clamp should only be applied when scrolling from a focused, horizontally
> split log view and not when scrolling from the diff view with J.
> 
> This can be seen by running the added regress without the fix applied,
> although it might be best observed with a manual test by running tog in
> an 80x24 xterm and entering:
> 
> 	S	# toggle horizontal split
> 	4j	# jump to 5th commit
> 	return	# open diff view in hsplit
> 	F	# toggle fullscreen diff view
> 	J	# diff view won't load the next commit
> 
> There's also an off-by-one in the calculation to determine how many
> commits we need to populate the log, which is revealed if that part of
> this diff (i.e., first hunk) is reverted. By entering 20J instead of J
> in the above manual test case to continue the scroll beyond the last
> loaded (i.e., 24th) commit, the 25th commit will not be loaded--it gets
> stuck on the 24th. There's a test added to cover this too.
> 
> ok?

I can reproduce and confirm that the diff fixes it.  Have to admit that
the off-by-one is a bit over my head, I'm not sure exactly why it works,
the rest of the diff looks fine to me.

So, fwiw, ok op@ :)


Thanks,

Omar Polo