diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index a8e5dfcd..a901a482 100644 --- a/context/test-evidence/clients.md +++ b/context/test-evidence/clients.md @@ -4659,3 +4659,21 @@ where. The first rival fails on `PgUp` (`page-scroll ["-1"]`) and, alone, on `Ctrl-u` (`scroll-rows ["-5"]`). A last assertion that each kind was pressed at least once guards the other direction — a binding vanishing — and is a named list on purpose, since that is the one thing no walk over bindings can see. + +**improvise-5kdr: the 3x3 fixture saw only a sign.** From the centre of a 3x3 +grid every step of one or more clamps to an edge, so the w7qd version stated +`page-scroll` as `1 + d` — one row, which is not what it does — and passed +because the true landing clamped to the same edge. Measured on #198 at 8772db2: +`page-scroll` ignoring the page size together with `scroll-rows` moving one row, +or `move-selection` moving `2d`, left the test green. The walk now stands at +(10,10) of a 21x21 grid with an 8x8 window, far from every edge, so a landing +shows magnitude: `10+d` for `move-selection` and `scroll-rows`, `10 + page·d` +for `page-scroll`, 0/20 for the jumps. The page size is not written into the +test: `grid_move`'s page rule moved into `page_rows(window_rows)` and the test +asks it, so the rule has one home. Rivals, each run: `move-selection` doubled +fails on `Up` (`move-selection ["-1","0"]`); the page-size-and-one-row pair +fails on `PgUp` (`page-scroll ["-1"]`); the one-row `scroll-rows` alone fails on +`Ctrl-u` (`scroll-rows ["-5"]`); the jump-table rival still fails naming every +jump. The residual, stated: because the test reads `page_rows`, a change to the +page rule itself moves the test with it — whether that rule is the TUI's is +`tests/lockstep.rs`'s question, not this one's. diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index 07b81414..d88b4e23 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -619,6 +619,11 @@ enum GridMove { Page(i32), } +/// Rows one `page-scroll` step moves: three quarters of the window, at least one. +fn page_rows(window_rows: usize) -> i32 { + (window_rows as i32 * 3 / 4).max(1) +} + fn grid_move(state: &ClientState, kind: GridMove) -> Option> { let GridExtent { rows, cols } = state.grid.extent; let (window_rows, window_cols) = state.window; @@ -641,7 +646,7 @@ fn grid_move(state: &ClientState, kind: GridMove) -> Option> { GridMove::ToEnd(true) => (row_max as usize, cursor.selected.1), GridMove::ToEnd(false) => (cursor.selected.0, col_max as usize), GridMove::Page(dir) => { - let delta = (window_rows as i32 * 3 / 4).max(1) * dir; + let delta = page_rows(window_rows) * dir; ((row + delta).clamp(0, row_max) as usize, cursor.selected.1) } }; @@ -2366,16 +2371,21 @@ mod tests { bound_commands(keymap, &mut bound); bound.sort(); bound.dedup(); - // From (1,1) of a 3x3 grid any step of at least one lands at 1+d clamped, - // so a delta's sign is visible; a jump's destination is its name. - let step = |d: &String| (1 + d.parse::().expect("a delta")).clamp(0, 2) as usize; + // Far from every edge of a grid far larger than any step, so a landing + // shows magnitude as well as sign (improvise-5kdr); a jump's is its name. + const START: (usize, usize) = (10, 10); + const WINDOW: (usize, usize) = (8, 8); + let d = |a: &String| a.parse::().expect("a delta"); + let at = |base: usize, by: isize| base.checked_add_signed(by).expect("inside the grid"); + let page = page_rows(WINDOW.0) as isize; let destination = |name: &str, args: &[String]| match name { - "move-selection" => Some((step(&args[0]), step(&args[1]))), - "scroll-rows" | "page-scroll" => Some((step(&args[0]), 1)), - "jump-first-row" => Some((0, 1)), - "jump-last-row" => Some((2, 1)), - "jump-first-col" => Some((1, 0)), - "jump-last-col" => Some((1, 2)), + "move-selection" => Some((at(START.0, d(&args[0])), at(START.1, d(&args[1])))), + "scroll-rows" => Some((at(START.0, d(&args[0])), START.1)), + "page-scroll" => Some((at(START.0, page * d(&args[0])), START.1)), + "jump-first-row" => Some((0, START.1)), + "jump-last-row" => Some((20, START.1)), + "jump-first-col" => Some((START.0, 0)), + "jump-last-col" => Some((START.0, 20)), _ => None, }; let mut unstated = Vec::new(); @@ -2389,6 +2399,13 @@ mod tests { } for sequence in sequences { let mut state = furnished(AppMode::Normal); + state.grid.extent = GridExtent { rows: 21, cols: 21 }; + state.window = WINDOW; + *state.view.cursor_mut() = crate::ViewCursor { + selected: START, + row_offset: 5, + col_offset: 5, + }; let view = state.view.active_view.clone(); for key in &sequence { if let KeyOutcome::Local(commands) = resolve(&state, *key) { @@ -2401,9 +2418,12 @@ mod tests { } match to { Some(to) => { - assert_eq!(landed, to, "{sequence:?} runs `{name} {args:?}` from (1,1)") + assert_eq!( + landed, to, + "{sequence:?} runs `{name} {args:?}` from {START:?}" + ) } - None if state.view.active_view == view && landed != (1, 1) => { + None if state.view.active_view == view && landed != START => { unstated.push(format!("{name} {args:?} -> {landed:?}")) } None => {}