From a49ed97891ff41b188bdfeb869d27bd4fee70e2e Mon Sep 17 00:00:00 2001 From: Derek Marshall Date: Thu, 15 Oct 2026 20:10:13 +0200 Subject: [PATCH] fix(list): count pages from the top so a jump never hides the cursor Pages are now fixed: the first starts at row 0 and each starts where the one before it ends. `ui::list_view::Pages` finds the starts by walking from the top, once per listing and list size and only as far as asked, and the settle places the column on the page holding its cursor. The settle used to snap `view_top` to a multiple of the page size it had just measured. Pages fit different numbers of columns, so it alternated between two starts and kept the lower one: after a `/` search no row was lit, and `l` off a page's last column landed in the next page's second column. PageDown / PageUp move to the same column and row on the next or previous page (its last row when shorter). PageDown from the last page wraps to the first; PageUp from the first goes to the last row, so opening a directory never measures past its second page. Closes #679 Co-Authored-By: Claude --- ARCHITECTURE.md | 12 +- src/app/bootstrap.rs | 6 +- src/app/render/grid_settle_tests.rs | 43 +++- src/app/render/mod.rs | 174 +++++----------- src/app/state/apply.rs | 5 +- src/app/state/commander.rs | 11 +- src/app/state/mod.rs | 6 +- src/app/state/navigation.rs | 35 ++-- src/app/state/tests/apply.rs | 5 + src/app/state/tests/navigation.rs | 58 ++++-- src/app/view_state.rs | 13 +- src/ui/list_view.rs | 297 +++++++++++++++++++++------- 12 files changed, 415 insertions(+), 250 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 75df812a..df6e52dc 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -277,9 +277,19 @@ Goal: 0 draws-per-second at idle. Implementation: buffer because the emoji arrives from flashes, file names and agent output alike. Its tests replay ratatui's real diff and backend bytes through libghostty-vt and compare the screen to the buffer. -- Caching: `build_rows()` and grid stabilization keyed by a +- Caching: `build_rows()` and the page settle keyed by a `list_generation` counter that increments on any listing / cursor / pick / mask change. +- Pages are counted from the top (#679): the first starts at row 0 + and each starts where the one before it ends. A page holds as many + columns as fit and a column is as wide as its longest name, so pages + differ in length, and `ui::list_view::Pages` finds their starts by + walking from the top, once per listing and list size and only as far + as asked. A row's page therefore never depends on how the cursor got + there, and PageDown / PageUp step through the same pages. Snapping + `view_top` to a multiple of the current page's length instead + alternated between two starts whenever neighbouring pages differed, + and could settle on one that didn't hold the cursor. The activity overlay (`A` toggle) reports dps and bytes/sec for ongoing tuning. diff --git a/src/app/bootstrap.rs b/src/app/bootstrap.rs index e102ff68..1c239638 100644 --- a/src/app/bootstrap.rs +++ b/src/app/bootstrap.rs @@ -139,10 +139,8 @@ impl App { view: View::Dir, cursor: Cursor::new(), rows: Vec::new(), - grid_dims: crate::ui::list_view::GridDims { - cols: 1, - rows_per_col: 1, - }, + grid_dims: crate::ui::list_view::GridDims::new(1, 1), + page: crate::ui::list_view::PageSpan::default(), list_generation: 0, // Per-column git: the startup branch string + (empty) markers, // and empty cache/worker plumbing until the first chdir + diff --git a/src/app/render/grid_settle_tests.rs b/src/app/render/grid_settle_tests.rs index 192356a2..e74be60e 100644 --- a/src/app/render/grid_settle_tests.rs +++ b/src/app/render/grid_settle_tests.rs @@ -112,7 +112,6 @@ fn page_cols(app: &App, view_top: usize, (w, h): (u16, u16)) -> u16 { /// it. The case: two pages could hold row 240, one from 192 (three columns, /// ending at 239) and one from 240 (four). #[test] -#[ignore = "red: the settle alternates between 192 and 240 and keeps 192, which ends before the cursor"] fn a_search_jump_draws_the_cursor_bar_on_the_match() { let (mut app, names) = listing(); draw(&mut app, BESIDE_PANE); @@ -139,7 +138,6 @@ fn a_search_jump_draws_the_cursor_bar_on_the_match() { /// the cursor's new one. The case: the page after 348-521 starts at 522 and /// fits two columns, and a page from 464 would fit three. #[test] -#[ignore = "red: the settle alternates between 522 and 464 and keeps 464, so the cursor lands in the second column"] fn l_off_the_last_column_opens_the_next_page_at_its_first_column() { let (mut app, names) = listing(); draw(&mut app, BARE); @@ -170,3 +168,44 @@ fn l_off_the_last_column_opens_the_next_page_at_its_first_column() { Some((first_column, row, names[522].clone())) ); } + +/// PageDown moves to the same column and row on the next page, or to its last +/// row when it has no such column. The case: from the last of three columns on +/// the page 348-521, the next page, 522-599, has two. +#[test] +fn page_down_to_a_narrower_page_lands_on_its_last_row() { + let (mut app, names) = listing(); + draw(&mut app, BARE); + search(&mut app, "n0464-", BARE); + assert_eq!( + app.state.left.cursor.view_top, 348, + "the cursor starts on the page 348-521" + ); + + press(&mut app, KeyCode::PageDown, BARE); + let buf = draw(&mut app, BARE); + let bar = cursor_bar(&app, &buf).map(|(_, _, name)| name); + assert_eq!(bar.as_deref(), Some(names[599].as_str())); +} + +/// PageUp moves to the same column and row on the page before. The case: from +/// the top of the two-column page at 522 to the top of the three-column page +/// at 348. +#[test] +fn page_up_from_a_pages_first_row_lands_on_the_previous_pages_first_row() { + let (mut app, names) = listing(); + draw(&mut app, BARE); + search(&mut app, "n0522-", BARE); + let buf = draw(&mut app, BARE); + let first_column = app.frame_layout(buf.area).list.x; + assert_eq!( + cursor_bar(&app, &buf).map(|(x, _, _)| x), + Some(first_column), + "the cursor starts at the top of its page", + ); + + press(&mut app, KeyCode::PageUp, BARE); + let buf = draw(&mut app, BARE); + let (x, _, name) = cursor_bar(&app, &buf).unwrap(); + assert_eq!((x, name.as_str()), (first_column, names[348].as_str())); +} diff --git a/src/app/render/mod.rs b/src/app/render/mod.rs index 406ea630..16b69b61 100644 --- a/src/app/render/mod.rs +++ b/src/app/render/mod.rs @@ -28,9 +28,9 @@ use ratatui::Frame; #[cfg(test)] use crate::config::StatusPosition; use crate::spyc_debug; -use crate::ui::list_view::{ListView, Row}; +use crate::ui::list_view::Row; -use super::{App, FrameLayout, View, state}; +use super::{App, FrameLayout, state}; mod activity_hud; mod agent_hud; @@ -434,36 +434,33 @@ impl App { self.build_rows_for(&self.state.left) } - /// Rebuild the list-rows cache and run the `view_top`↔grid stabilization. - /// Pure derived state: reads the listing/cursor + the list rect, writes - /// `cached_rows`, `grid_dims`, and `cursor.view_top`. No drawing, no IO — - /// extracted from the draw path (MVU Stage 2) so `render` stays - /// mutation-free. + /// Rebuild the list-rows cache and place each column on the page holding + /// its cursor. Pure derived state: reads the listing/cursor + the list rect, + /// writes `cached_rows`, the page caches, `grid_dims`, `page` and + /// `cursor.view_top`. No drawing, no IO — extracted from the draw path + /// (MVU Stage 2) so `render` stays mutation-free. fn settle_list_grid(&mut self, layout: &FrameLayout) { if self.view.cached_rows_gen != self.state.left.list_generation { self.view.cached_rows = self.build_rows(); self.view.cached_rows_gen = self.state.left.list_generation; } - let focused = !self.state.pane_focused(); - // Disjoint field borrows: cached_rows/theme (read) + cached_grid_key + // Disjoint field borrows: cached_rows (read) + pages/cached_grid_key // (write) on `view`, and `state.left` (write) — all distinct fields, so // this passes the borrow checker without an accessor that locks all of - // `self`. PR C settles the right column the same way against its own - // commander + caches. - stabilize_grid( + // `self`. + settle_page( &self.view.cached_rows, + self.view.cached_rows_gen, &mut self.state.left, layout.list, - focused, - &self.view.theme, + &mut self.view.pages, &mut self.view.cached_grid_key, ); // The right column's second commander (`state.right`) settles the same - // way against its own row cache + grid key + rect. No-op until `^s` + // way against its own row cache, pages and grid key. No-op until `^s` // opens one. if self.state.right.is_some() { - let right_focused = self.right_column_focused(); let right_gen = self .state .right @@ -476,12 +473,12 @@ impl App { self.view.right_cached_rows_gen = right_gen; } if let (Some(rect), Some(right)) = (layout.right, self.state.right.as_mut()) { - stabilize_grid( + settle_page( &self.view.right_cached_rows, + self.view.right_cached_rows_gen, right, rect, - right_focused, - &self.view.theme, + &mut self.view.right_pages, &mut self.view.right_cached_grid_key, ); } @@ -489,117 +486,50 @@ impl App { } } -/// The `view_top` ↔ grid stabilization for one commander column. The grid -/// depends on `view_top` (entry name lengths → column count → items-per-page) -/// and `view_top` depends on the grid, so this iterates to a fixpoint (≤4 -/// rounds), breaking a 2-cycle by picking the lower `view_top` (more context, -/// deterministic across frames). Skips the loop when nothing changed since the -/// last frame (the `cached_grid_key` guard). Pure derived state — mutates only -/// the commander's `grid_dims`/`cursor.view_top` and the caller's cache key, no -/// drawing or IO. Free fn (not an `App` method) so the borrows on the caller's -/// `view`/`state` fields stay disjoint; called once per visible column. -fn stabilize_grid( - rows: &[crate::ui::list_view::Row], +/// Put one commander column on the fixed page holding its cursor +/// ([`crate::ui::list_view::Pages`]), and record that page's grid and its +/// neighbours for PageDown / PageUp. A row's page depends only on the listing +/// and the list's size, never on where the view was before. +/// +/// Skips the work when nothing changed since the last frame (the +/// `cached_grid_key` guard). Pure derived state: mutates only the commander's +/// `cursor.view_top` / `grid_dims` / `page` and the caller's caches, no drawing +/// or IO. A free fn (not an `App` method) so the borrows on the caller's +/// `view` / `state` fields stay disjoint; called once per visible column. +fn settle_page( + rows: &[Row], + rows_gen: u64, commander: &mut state::Commander, rect: ratatui::layout::Rect, - focused: bool, - theme: &crate::ui::theme::Theme, + pages: &mut crate::ui::list_view::Pages, cached_grid_key: &mut (u64, usize, usize, u16, u16), ) { - let grid_key = ( - commander.list_generation, - commander.cursor.view_top, - commander.cursor.index, - rect.width, - rect.height, - ); - if grid_key == *cached_grid_key { + let key = |c: &state::Commander| { + ( + c.list_generation, + c.cursor.view_top, + c.cursor.index, + rect.width, + rect.height, + ) + }; + if key(commander) == *cached_grid_key { return; } - *cached_grid_key = grid_key; - let mut prev_vt: Option = None; // for 2-cycle detection - let mut settled = false; - for round in 0..4 { - let probe = ListView { - rows, - cursor: commander.cursor.index, - view_top: commander.cursor.view_top, - empty_marker: commander.view == View::Dir, - focused, - theme, - selection: None, - }; - commander.grid_dims = probe.grid(rect).dims(); - let old_vt = commander.cursor.view_top; - let pp = commander.grid_dims.items_per_page(); - commander.ensure_cursor_visible(); - if commander.cursor.view_top == old_vt { - spyc_debug!( - "grid settled round {}: vt={} cursor={} grid={}x{} pp={}", - round + 1, - old_vt, - commander.cursor.index, - commander.grid_dims.cols, - commander.grid_dims.rows_per_col, - pp, - ); - settled = true; - break; - } - spyc_debug!( - "grid unstable round {}: vt {} -> {} cursor={} grid={}x{} pp={}", - round + 1, - old_vt, - commander.cursor.view_top, - commander.cursor.index, - commander.grid_dims.cols, - commander.grid_dims.rows_per_col, - pp, - ); - // 2-cycle: new vt equals the vt from two rounds ago. - if Some(commander.cursor.view_top) == prev_vt { - // Always pick the lower vt — deterministic across frames. - let forced = old_vt.min(commander.cursor.view_top); - commander.cursor.view_top = forced; - // Recompute grid for the forced view_top. - let probe = ListView { - rows, - cursor: commander.cursor.index, - view_top: commander.cursor.view_top, - empty_marker: commander.view == View::Dir, - focused, - theme, - selection: None, - }; - commander.grid_dims = probe.grid(rect).dims(); - spyc_debug!( - "grid 2-cycle broken: forcing vt={} (cursor={} grid={}x{} pp={})", - forced, - commander.cursor.index, - commander.grid_dims.cols, - commander.grid_dims.rows_per_col, - commander.grid_dims.items_per_page(), - ); - settled = true; - break; - } - prev_vt = Some(old_vt); - } - if !settled { - spyc_debug!( - "grid did NOT settle after 4 rounds: vt={} cursor={}", - commander.cursor.view_top, - commander.cursor.index, - ); - } - // Update cache key in case the stabilization loop changed view_top. - *cached_grid_key = ( - commander.list_generation, - commander.cursor.view_top, + let page = pages.around(rows, rect, rows_gen, commander.cursor.index); + commander.cursor.view_top = page.start; + commander.page = page; + commander.grid_dims = crate::ui::list_view::grid_at(rows, page.start, rect).dims(); + spyc_debug!( + "page settled: vt={} cursor={} grid={}x{} prev={:?} next={:?}", + page.start, commander.cursor.index, - rect.width, - rect.height, + commander.grid_dims.cols, + commander.grid_dims.rows_per_col, + page.prev, + page.next, ); + *cached_grid_key = key(commander); } #[cfg(test)] diff --git a/src/app/state/apply.rs b/src/app/state/apply.rs index f474fa38..dcaf9867 100644 --- a/src/app/state/apply.rs +++ b/src/app/state/apply.rs @@ -32,7 +32,6 @@ impl AppState { pub fn apply(&mut self, action: &Action) -> ApplyResult { let len = self.cur().rows.len(); let rows_per_col = self.cur().grid_dims.rows_per_col as usize; - let per_page = self.cur().grid_dims.items_per_page(); match action { // -- Cursor motion -- @@ -56,8 +55,8 @@ impl AppState { self.flash_info("~"); } } - Action::PageUp => self.cursor_move_global(-(per_page as isize), len), - Action::PageDown => self.cursor_move_global(per_page as isize, len), + Action::PageUp => self.page_move(false), + Action::PageDown => self.page_move(true), Action::GotoFirst => self.goto_col_top(rows_per_col), Action::GotoLast => self.goto_col_bottom(rows_per_col, len), diff --git a/src/app/state/commander.rs b/src/app/state/commander.rs index 52446919..61bf4b41 100644 --- a/src/app/state/commander.rs +++ b/src/app/state/commander.rs @@ -4,7 +4,7 @@ use super::{GitCache, GitState, RowData, View}; use crate::config::Config; use crate::fs::Listing; use crate::state::{Cursor, IgnoreMasks, Picks}; -use crate::ui::list_view::GridDims; +use crate::ui::list_view::{GridDims, PageSpan}; use std::path::{Path, PathBuf}; /// One file-commander's worth of pure Model state — the per-browser fields a @@ -44,6 +44,9 @@ pub struct Commander { /// The geometry slice of this browser's last rendered grid (cols × /// rows-per-col), written by render and read by cursor/page-math. pub grid_dims: GridDims, + /// The cursor's page and its neighbours, as the last frame placed them: + /// what PageDown / PageUp step to. + pub page: PageSpan, /// Monotonic counter bumped whenever this browser's display row list /// changes. Used by App to skip redundant `build_rows()` calls. pub list_generation: u64, @@ -101,10 +104,8 @@ impl Commander { view: View::Dir, cursor: Cursor::new(), rows: Vec::new(), - grid_dims: GridDims { - cols: 1, - rows_per_col: 1, - }, + grid_dims: GridDims::new(1, 1), + page: PageSpan::default(), list_generation: 0, git: GitState::default(), git_cache: GitCache::default(), diff --git a/src/app/state/mod.rs b/src/app/state/mod.rs index 9b5fd345..a6eb56f8 100644 --- a/src/app/state/mod.rs +++ b/src/app/state/mod.rs @@ -999,10 +999,8 @@ impl AppState { view: View::Dir, cursor: Cursor::new(), rows: Vec::new(), - grid_dims: GridDims { - cols: 1, - rows_per_col: 20, - }, + grid_dims: GridDims::new(1, 20), + page: crate::ui::list_view::PageSpan::default(), list_generation: 0, git: GitState::default(), git_cache: GitCache::default(), diff --git a/src/app/state/navigation.rs b/src/app/state/navigation.rs index 04118915..1e150a9b 100644 --- a/src/app/state/navigation.rs +++ b/src/app/state/navigation.rs @@ -6,23 +6,7 @@ use anyhow::{Context, Result}; use crate::app::{Effect, Matcher}; -use super::{AppState, Commander, Side}; - -impl Commander { - /// Snap `view_top` to the page containing the cursor, given the last - /// rendered grid geometry. Per-browser (cursor/grid/rows all live here), so - /// each split column settles independently. Was an `AppState` method back - /// when there was one browser. - pub const fn ensure_cursor_visible(&mut self) { - let per_page = self.grid_dims.items_per_page(); - if per_page == 0 || self.rows.is_empty() { - self.cursor.view_top = 0; - return; - } - let page = self.cursor.index / per_page; - self.cursor.view_top = page * per_page; - } -} +use super::{AppState, Side}; impl AppState { /// j/k — move within the current column only. Wraps at column @@ -73,13 +57,22 @@ impl AppState { self.col_mut(side).cursor.index = (cur + delta).clamp(0, last) as usize; } - /// Move across the entire list (PageUp/PageDown). Wraps globally. - pub fn cursor_move_global(&mut self, delta: isize, len: usize) { + /// PageDown / PageUp: the same column and row on the next or previous page, + /// or that page's last row when it is shorter. Pages are the fixed ones the + /// last frame placed (`Commander::page`). PageDown from the last page wraps + /// to the first; PageUp from the first wraps to the last row. + pub fn page_move(&mut self, forward: bool) { + let len = self.cur().rows.len(); if len == 0 { return; } - let new_idx = (self.cur().cursor.index as isize + delta).rem_euclid(len as isize); - self.cur_mut().cursor.index = new_idx as usize; + let page = self.cur().page; + let offset = self.cur().cursor.index.saturating_sub(page.start); + let target = if forward { Some(page.next) } else { page.prev }; + let index = target.map_or(len - 1, |(start, page_len)| { + start + offset.min(page_len.saturating_sub(1)) + }); + self.cur_mut().cursor.index = index.min(len - 1); } /// `gg` — jump to the first entry of the current column. diff --git a/src/app/state/tests/apply.rs b/src/app/state/tests/apply.rs index 523c4085..1e161be0 100644 --- a/src/app/state/tests/apply.rs +++ b/src/app/state/tests/apply.rs @@ -30,6 +30,11 @@ fn apply_page_down() { cols: 1, rows_per_col: 3, }; + s.left.page = crate::ui::list_view::PageSpan { + start: 0, + prev: None, + next: (3, 3), + }; s.apply(&Action::PageDown); assert_eq!(s.left.cursor.index, 3); } diff --git a/src/app/state/tests/navigation.rs b/src/app/state/tests/navigation.rs index bde361e4..f6055845 100644 --- a/src/app/state/tests/navigation.rs +++ b/src/app/state/tests/navigation.rs @@ -120,30 +120,52 @@ fn column_move_single_column_noop() { assert_eq!(s.left.cursor.index, 0); // no-op } -// ── ensure_cursor_visible ───────────────────────────────────── +// ── page_move ───────────────────────────────────────────────── + +/// Ten rows on pages of three, three, two and two: the cursor's page starts +/// at 3, and the shorter page after it isn't the last. +fn on_the_middle_page(index: usize) -> AppState { + let mut s = state_with_rows(&["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"]); + s.left.page = crate::ui::list_view::PageSpan { + start: 3, + prev: Some((0, 3)), + next: (6, 2), + }; + s.left.cursor.index = index; + s +} + +#[test] +fn page_down_keeps_the_place_on_the_page() { + let mut s = on_the_middle_page(4); + s.page_move(true); + assert_eq!(s.left.cursor.index, 7); +} #[test] -fn ensure_visible_snaps_view_top() { - let mut s = state_with_rows(&["a", "b", "c", "d", "e", "f", "g", "h"]); - s.left.grid_dims = GridDims { - cols: 1, - rows_per_col: 3, - }; // 3 items per page - s.left.cursor.index = 5; // page 1 (items 3-5) - s.left.ensure_cursor_visible(); - assert_eq!(s.left.cursor.view_top, 3); +fn page_down_to_a_shorter_page_stops_at_its_last_row() { + let mut s = on_the_middle_page(5); + s.page_move(true); + assert_eq!(s.left.cursor.index, 7); } #[test] -fn ensure_visible_first_page() { - let mut s = state_with_rows(&["a", "b", "c", "d"]); - s.left.grid_dims = GridDims { - cols: 1, - rows_per_col: 3, +fn page_up_keeps_the_place_on_the_page() { + let mut s = on_the_middle_page(4); + s.page_move(false); + assert_eq!(s.left.cursor.index, 1); +} + +#[test] +fn page_up_from_the_first_page_wraps_to_the_last_row() { + let mut s = on_the_middle_page(1); + s.left.page = crate::ui::list_view::PageSpan { + start: 0, + prev: None, + next: (3, 3), }; - s.left.cursor.index = 1; - s.left.ensure_cursor_visible(); - assert_eq!(s.left.cursor.view_top, 0); + s.page_move(false); + assert_eq!(s.left.cursor.index, 9); } // ── find_match ──────────────────────────────────────────────── diff --git a/src/app/view_state.rs b/src/app/view_state.rs index c781bb02..0b825fd5 100644 --- a/src/app/view_state.rs +++ b/src/app/view_state.rs @@ -205,14 +205,17 @@ pub struct ViewState { /// Cached `build_rows()` output; invalidated by `list_generation`. pub(super) cached_rows: Vec, pub(super) cached_rows_gen: u64, - /// Grid stabilization cache key: (list_gen, view_top, cursor, width, height). + /// Page settle cache key: (list_gen, view_top, cursor, width, height). pub(super) cached_grid_key: (u64, usize, usize, u16, u16), - /// The same row cache + grid key for the **right** column's second - /// commander (`state.right`), settled independently. Unused (empty / `MAX` - /// gen) while no second commander is open. + /// Where the left column's fixed pages start, measured over `cached_rows`. + pub(super) pages: crate::ui::list_view::Pages, + /// The same row cache, grid key and page starts for the **right** column's + /// second commander (`state.right`), settled independently. Unused (empty / + /// `MAX` gen) while no second commander is open. pub(super) right_cached_rows: Vec, pub(super) right_cached_rows_gen: u64, pub(super) right_cached_grid_key: (u64, usize, usize, u16, u16), + pub(super) right_pages: crate::ui::list_view::Pages, /// Last terminal-window title emitted (OSC 2 dedup). `None` forces /// a re-emit on next draw. pub(super) last_term_title: Option, @@ -446,9 +449,11 @@ impl ViewState { cached_rows: Vec::new(), cached_rows_gen: u64::MAX, // force first build cached_grid_key: (u64::MAX, 0, 0, 0, 0), + pages: crate::ui::list_view::Pages::default(), right_cached_rows: Vec::new(), right_cached_rows_gen: u64::MAX, // force first build right_cached_grid_key: (u64::MAX, 0, 0, 0, 0), + right_pages: crate::ui::list_view::Pages::default(), last_term_title: None, harpoon_menu: None, chord_hint: None, diff --git a/src/ui/list_view.rs b/src/ui/list_view.rs index ec5c559f..2debcdd6 100644 --- a/src/ui/list_view.rs +++ b/src/ui/list_view.rs @@ -129,11 +129,96 @@ pub struct GridDims { } impl GridDims { + pub const fn new(cols: u16, rows_per_col: u16) -> Self { + Self { cols, rows_per_col } + } + pub const fn items_per_page(self) -> usize { self.cols as usize * self.rows_per_col as usize } } +/// The page holding the cursor and its neighbours, for update-time paging. +/// Pages are counted from the top (see [`Pages`]). +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct PageSpan { + /// The page's first row: the `view_top` drawn. + pub start: usize, + /// First row and length of the page before. `None` on the first page: + /// the last page, where it would wrap, is only known after measuring them + /// all, which opening a large directory shouldn't pay for. + pub prev: Option<(usize, usize)>, + /// First row and length of the page after, or of the first page from the + /// last, so a one-page listing is its own neighbour. + pub next: (usize, usize), +} + +/// Where the listing's fixed pages start. The first starts at row 0 and each +/// starts where the one before it ends. A page holds as many columns as fit, +/// and a column is as wide as its longest name, so pages differ in length and +/// their starts can only be found by walking from the top. +/// +/// A row's page therefore never depends on how the cursor reached it. The walk +/// runs once per listing and list size, and only as far as it is asked. +#[derive(Debug, Default)] +pub struct Pages { + /// The rows' generation and the list's rect the starts were measured for. + key: Option<(u64, Rect)>, + /// Ascending, from 0. + starts: Vec, + /// Where the last measured page ends. + end: usize, +} + +impl Pages { + /// The page holding `index`, with its neighbours. `generation` names the + /// rows: a new one, or a new `area`, restarts the walk. + pub fn around(&mut self, rows: &[Row], area: Rect, generation: u64, index: usize) -> PageSpan { + if self.key != Some((generation, area)) { + *self = Self { + key: Some((generation, area)), + ..Self::default() + }; + } + let len = rows.len(); + if len == 0 { + return PageSpan::default(); + } + let index = index.min(len - 1); + self.measure_while(rows, area, |pages| pages.end <= index); + let k = self.starts.partition_point(|&s| s <= index) - 1; + // The page after: the next one, or the first from the last. + self.measure_while(rows, area, |pages| pages.starts.len() <= k + 1); + let next = if k + 1 < self.starts.len() { + self.page(k + 1) + } else { + self.page(0) + }; + PageSpan { + start: self.starts[k], + prev: k.checked_sub(1).map(|k| self.page(k)), + next, + } + } + + /// First row and length of page `k`, which must be measured. + fn page(&self, k: usize) -> (usize, usize) { + let end = self.starts.get(k + 1).copied().unwrap_or(self.end); + (self.starts[k], end - self.starts[k]) + } + + /// Measure pages, one after another, while `more` holds and rows remain. + fn measure_while(&mut self, rows: &[Row], area: Rect, more: impl Fn(&Self) -> bool) { + while self.end < rows.len() && more(self) { + let start = self.end; + // `grid_at` always yields at least one row-tall column, so this ends. + let len = grid_at(rows, start, area).items_per_page(); + self.starts.push(start); + self.end = (start + len).min(rows.len()); + } + } +} + impl Grid { pub const fn items_per_page(&self) -> usize { self.cols as usize * self.rows as usize @@ -203,72 +288,7 @@ impl ListView<'_> { /// Compute the grid for the page that starts at `self.view_top`. pub fn grid(&self, area: Rect) -> Grid { - let height = area.height.max(1) as usize; - let width = area.width as usize; - - if self.rows.is_empty() { - return Grid { - cols: 1, - rows: height as u16, - col_widths: vec![MIN_NAME_WIDTH + MARKER_W], - }; - } - - // Columns are always R = screen_height rows deep. Walk columns - // starting at view_top, adding columns while the total width still - // fits on screen. What doesn't fit paginates. - let rows_per_col = height; - let tail_start = self.view_top.min(self.rows.len()); - let tail = &self.rows[tail_start..]; - let count = tail.len(); - - // Max display width of column `col`'s rows, measured lazily. Only the - // columns that actually fit on screen are measured below, so the work - // is O(items_per_page + one overflow column) — NOT O(rows to the end - // of the listing). The old code collected display_width for *every* - // tail row on *every* frame: a 100k-entry dir near the top meant 100k - // width calls + an ~800 KB Vec per frame (typing jank / agent-stream - // redraw cost that grew linearly with directory size). - let col_max_width = |col: usize| -> usize { - let start = col * rows_per_col; - let end = (start + rows_per_col).min(count); - tail[start..end] - .iter() - .map(|r| super::display_width(&r.display)) - .max() - .unwrap_or(0) - }; - - let mut col_widths: Vec = Vec::new(); - let mut total = 0usize; - let mut col_idx = 0usize; - loop { - let start = col_idx * rows_per_col; - if start >= count { - break; - } - let col_w = col_max_width(col_idx) + MARKER_W as usize; - let addition = col_w + if col_idx > 0 { COL_GAP as usize } else { 0 }; - if total + addition > width { - break; - } - total += addition; - col_widths.push(col_w as u16); - col_idx += 1; - } - if col_idx == 0 { - // Screen narrower than one cell. Render one clamped column — - // measured from the first column's rows only (what's drawn), not - // the whole tail. - let widest = col_max_width(0).max(MIN_NAME_WIDTH as usize); - col_widths.push((widest + MARKER_W as usize).min(width.max(1)) as u16); - col_idx = 1; - } - Grid { - cols: col_idx as u16, - rows: rows_per_col as u16, - col_widths, - } + grid_at(self.rows, self.view_top, area) } /// The bar a row gets: its background and whether it is bold, or `None` /// for a plain row. The cursor row forces the bright bar bg + fg over both @@ -311,6 +331,77 @@ impl ListView<'_> { } } +/// The grid for the page starting at `view_top`: the one geometry the +/// renderer, the hit-test and the page walk ([`Pages`]) all derive. +pub fn grid_at(rows: &[Row], view_top: usize, area: Rect) -> Grid { + let height = area.height.max(1) as usize; + let width = area.width as usize; + + if rows.is_empty() { + return Grid { + cols: 1, + rows: height as u16, + col_widths: vec![MIN_NAME_WIDTH + MARKER_W], + }; + } + + // Columns are always R = screen_height rows deep. Walk columns + // starting at view_top, adding columns while the total width still + // fits on screen. What doesn't fit paginates. + let rows_per_col = height; + let tail_start = view_top.min(rows.len()); + let tail = &rows[tail_start..]; + let count = tail.len(); + + // Max display width of column `col`'s rows, measured lazily. Only the + // columns that actually fit on screen are measured below, so the work + // is O(items_per_page + one overflow column) — NOT O(rows to the end + // of the listing). The old code collected display_width for *every* + // tail row on *every* frame: a 100k-entry dir near the top meant 100k + // width calls + an ~800 KB Vec per frame (typing jank / agent-stream + // redraw cost that grew linearly with directory size). + let col_max_width = |col: usize| -> usize { + let start = col * rows_per_col; + let end = (start + rows_per_col).min(count); + tail[start..end] + .iter() + .map(|r| super::display_width(&r.display)) + .max() + .unwrap_or(0) + }; + + let mut col_widths: Vec = Vec::new(); + let mut total = 0usize; + let mut col_idx = 0usize; + loop { + let start = col_idx * rows_per_col; + if start >= count { + break; + } + let col_w = col_max_width(col_idx) + MARKER_W as usize; + let addition = col_w + if col_idx > 0 { COL_GAP as usize } else { 0 }; + if total + addition > width { + break; + } + total += addition; + col_widths.push(col_w as u16); + col_idx += 1; + } + if col_idx == 0 { + // Screen narrower than one cell. Render one clamped column — + // measured from the first column's rows only (what's drawn), not + // the whole tail. + let widest = col_max_width(0).max(MIN_NAME_WIDTH as usize); + col_widths.push((widest + MARKER_W as usize).min(width.max(1)) as u16); + col_idx = 1; + } + Grid { + cols: col_idx as u16, + rows: rows_per_col as u16, + col_widths, + } +} + impl Widget for ListView<'_> { fn render(self, area: Rect, buf: &mut Buffer) { if self.rows.is_empty() { @@ -537,6 +628,80 @@ mod tests { }) } + /// A 20×2 list over rows on pages of 2, 2 and 8: a column holding a long + /// name is the only one its page fits. + fn uneven() -> Vec { + let long = "long-name-xxxxx"; + ["a", "b", long, long, "c", "d", "e", "f", "g", "h", "i", "j"] + .iter() + .map(|n| row(n)) + .collect() + } + + const UNEVEN_AREA: Rect = Rect { + x: 0, + y: 0, + width: 20, + height: 2, + }; + + #[test] + fn pages_are_counted_from_the_top_and_wrap() { + let rows = uneven(); + let mut pages = Pages::default(); + let mut span = |i| pages.around(&rows, UNEVEN_AREA, 0, i); + let page = |start, prev, next| PageSpan { start, prev, next }; + assert_eq!(span(0), page(0, None, (2, 2))); + assert_eq!(span(3), page(2, Some((0, 2)), (4, 8))); + assert_eq!(span(11), page(4, Some((2, 2)), (0, 2))); + } + + /// A row's page doesn't depend on which rows were asked about before it. + #[test] + fn a_rows_page_ignores_what_was_asked_before() { + let rows = uneven(); + let fresh = |i| Pages::default().around(&rows, UNEVEN_AREA, 0, i); + let mut warm = Pages::default(); + for i in [11, 0, 5, 2, 7, 1] { + assert_eq!(warm.around(&rows, UNEVEN_AREA, 0, i), fresh(i), "row {i}"); + } + } + + /// New rows, or a new list size, measure the pages again. + #[test] + fn new_rows_or_a_new_size_measure_again() { + let rows = uneven(); + let mut pages = Pages::default(); + assert_eq!(pages.around(&rows, UNEVEN_AREA, 0, 5).start, 4); + let wide = Rect { + width: 200, + ..UNEVEN_AREA + }; + let one_page = PageSpan { + start: 0, + prev: None, + next: (0, 12), + }; + assert_eq!(pages.around(&rows, wide, 0, 5), one_page); + let short: Vec = (0..12).map(|_| row("a")).collect(); + assert_eq!(pages.around(&short, UNEVEN_AREA, 1, 5).start, 0); + } + + /// Placing the first page measures it and the next, not the whole listing. + #[test] + fn the_first_page_measures_only_what_it_shows() { + let rows: Vec = (0..10_000).map(|_| row("a")).collect(); + let mut pages = Pages::default(); + pages.around(&rows, UNEVEN_AREA, 0, 0); + assert_eq!(pages.starts, [0, 8]); + } + + #[test] + fn an_empty_listing_has_no_pages() { + let span = Pages::default().around(&[], UNEVEN_AREA, 0, 0); + assert_eq!(span, PageSpan::default()); + } + #[test] fn rows_always_match_screen_height() { // 12 items on a 10-row screen → 2 columns of up to 10 each.