commit 4dc5f0c1fa330375b2287c1d64667a539f27b625
parent a3d6ea4087e8493d71b2e5154a04b1fc5a85827d
Author: Dan Callaghan <djc@djc.id.au>
Date: Sun, 5 Apr 2026 12:44:06 +1000
simplify and fix thread loading behaviour for page-down
The expected behaviour for page-down is that it scrolls the complete
buffer content height, with no overlapping lines. But this would not
behave correctly if you pressed page-down after spawning a thread
listing, because it was initially populated with exactly the buffer
content height. It meant there would never be a next page to
scroll to.
Change the initial load size to be the buffer content height plus 1,
so that if there is more than one page of threads, pressing page-down
can always scroll to the next page.
Delete the different special cases trying to guess whether there are
more pages of threads to load. There are many edge cases where this
logic would fall over (different combinations of scroll-down and
page-down) and the logic is not necessary. ThreadIndexMode already has
logic to return early from its load_more callback when all threads have
been loaded. Always invoke load_more callbacks when the bottom is
visible.
Diffstat:
2 files changed, 24 insertions(+), 66 deletions(-)
diff --git a/lib/sup/modes/line_cursor_mode.rb b/lib/sup/modes/line_cursor_mode.rb
@@ -31,7 +31,7 @@ class LineCursorMode < ScrollMode
super opts
end
- def spawned; call_load_more_callbacks buffer.content_height; end
+ def spawned; call_load_more_callbacks buffer.content_height + 1; end
def cleanup
@load_more_thread.kill
@@ -162,26 +162,11 @@ protected
end
end
- ## more complicated than one might think. three behaviors.
def page_down
- ## if we're on the last page, and it's not a full page, just move
- ## the cursor down to the bottom and assume we can't load anything
- ## else via the callbacks.
- if topline > lines - buffer.content_height
- set_cursor_pos(lines - 1)
-
- ## if we're on the last page, and it's a full page, try and load
- ## more lines via the callbacks and then shift the page down
- elsif topline == lines - buffer.content_height
- call_load_more_callbacks buffer.content_height
- super
-
- ## otherwise, just move down
- else
- relpos = @curpos - topline
- super
- set_cursor_pos [topline + relpos, lines - 1].min
- end
+ relpos = @curpos - topline
+ super
+ set_cursor_pos [topline + relpos, lines - 1].min
+ call_load_more_callbacks buffer.content_height if lines < topline + buffer.content_height
end
def jump_to_start
diff --git a/test/unit/test_line_cursor_mode.rb b/test/unit/test_line_cursor_mode.rb
@@ -51,21 +51,21 @@ class TestLineCursorMode < Minitest::Test
def test_cursor_down
mode = make_mode
- expect_load_more 40
+ expect_load_more 41
mode.draw # curpos gets messed up without this call, why?
- 20.times do
+ 21.times do
mode.handle_input Ncurses::CharCode.character('j')
end
- assert_equal 20, mode.curpos
+ assert_equal 21, mode.curpos
assert_equal 0, mode.topline
- ## One past halfway, load_more callbacks are triggered.
+ ## Two past halfway, load_more callbacks are triggered.
mode.handle_input Ncurses::CharCode.character('j')
- assert_equal 21, mode.curpos
+ assert_equal 22, mode.curpos
expect_load_more 40
- 18.times do
+ 17.times do
mode.handle_input Ncurses::CharCode.character('j')
end
assert_equal 39, mode.curpos
@@ -79,7 +79,7 @@ class TestLineCursorMode < Minitest::Test
def test_scroll_down
mode = make_mode
- expect_load_more 40
+ expect_load_more 41
## When the cursor is already at the top, it moves with the scroll.
assert_equal 0, mode.curpos
@@ -88,9 +88,6 @@ class TestLineCursorMode < Minitest::Test
assert_equal 1, mode.curpos
assert_equal 1, mode.topline
- ## It always loads 10 more if we would scroll past the bottom.
- expect_load_more 10
-
3.times do
mode.handle_input Ncurses::CharCode.character('j')
end
@@ -102,55 +99,31 @@ class TestLineCursorMode < Minitest::Test
mode.handle_input Ncurses::CharCode.character('J')
assert_equal 4, mode.curpos
assert_equal 2, mode.topline
+
+ ## It always loads 10 more if we would scroll past the bottom.
+ expect_load_more 10
end
def test_page_down
mode = make_mode
- expect_load_more 40
+ expect_load_more 41
- ## In theory, we should scroll a full page down. But because we always load
- ## exactly enough lines to fill one page, the first time we are off by one.
mode.handle_input Ncurses::CharCode.keycode(Ncurses::KEY_NPAGE)
- assert_equal 0, mode.curpos
- assert_equal 39, mode.topline
+ assert_equal 40, mode.curpos
+ assert_equal 40, mode.topline
expect_load_more 40
- ## ThreadIndexMode#update does this, which I think is the only reason curpos
- ## does not end up pointing to an invisible line...
- mode.send :set_cursor_pos, 39
- assert_equal 39, mode.curpos
- assert_equal 39, mode.topline
-
- ## Now we have 80 lines but the top is at line 39, which makes the next
- ## page down behaviour even more awkward...
- assert_equal 80, mode.lines
- mode.handle_input Ncurses::CharCode.keycode(Ncurses::KEY_NPAGE)
- assert_equal 79, mode.curpos
- assert_equal 79, mode.topline
- assert_equal 80, mode.lines
- assert @load_more.empty?
-
- ## Page down does not trigger load_more callbacks, which is not very nice.
- mode.handle_input Ncurses::CharCode.keycode(Ncurses::KEY_NPAGE)
mode.handle_input Ncurses::CharCode.keycode(Ncurses::KEY_NPAGE)
- mode.handle_input Ncurses::CharCode.keycode(Ncurses::KEY_NPAGE)
- assert_equal 80, mode.lines
- assert @load_more.empty?
-
- ## Sending cursor_down to ThreadIndexView when it's in this state *does*
- ## trigger load_more callbacks though. It doesn't happen in this test so
- ## it's probably a side effect of some interactions between the two classes
- ## like the #update method.
- mode.handle_input Ncurses::CharCode.character('j')
- #expect_load_more 40
- assert_equal 79, mode.curpos
- assert_equal 79, mode.topline
+ assert_equal 80, mode.curpos
+ assert_equal 80, mode.topline
+ assert_equal 81, mode.lines
+ expect_load_more 40
end
def test_page_down_when_fully_populated
mode = make_mode
- expect_load_more 40
- (0...120).map { |i| @lines << "more line #{i}" } # enough for 4 full pages
+ expect_load_more 41
+ (0...119).map { |i| @lines << "more line #{i}" } # enough for 4 full pages
mode.handle_input Ncurses::CharCode.keycode(Ncurses::KEY_NPAGE)
assert_equal 40, mode.curpos