From 5e37973fc09824e251ff48fd489b9a0c703c9661 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Wed, 23 Sep 2026 07:51:50 -0700 Subject: [PATCH] The TUI help footer's keys come off the keymap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit improvise-17ak: render_footer hand-wrote l/Tab, h/S-Tab and q/Esc, a key table no check could read, and already short of the keymap (n, → and ? were bound and not advertised). It now draws the page indicator followed by hint_line(Help), truncated in cells. Derived rather than deleted: the status row carries the same hint, but draw_status prints status_msg instead of it whenever a message is standing, and nothing clears the message on entering Help, so deleting the footer's keys would leave a user who saved and then pressed ? with none. The guard renders the popup at 140 columns on every page and requires the footer row to contain hint_line(Help). Red at base; and a literal copy of today's string fails it as soon as the keymap drops a binding. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/test-evidence/tui.md | 34 +++++++++++++ src/ui/help.rs | 92 +++++++++++++++--------------------- 2 files changed, 72 insertions(+), 54 deletions(-) diff --git a/context/test-evidence/tui.md b/context/test-evidence/tui.md index dd6462b9..080a27b0 100644 --- a/context/test-evidence/tui.md +++ b/context/test-evidence/tui.md @@ -836,3 +836,37 @@ tests pass — including both of the ones *aimed* at the suppression — and onl this one fails. That is the shape design-principles §7 names: the assertion next door is satisfied by the wrong fix, and the discriminating input is the mode the change was not supposed to touch. + +### the_help_footer_names_the_keys_the_keymap_binds + +**What was wrong (improvise-17ak).** `src/ui/help.rs::render_footer` +hand-wrote the popup's paging keys: `l/Tab: next`, `h/S-Tab: prev`, +`q/Esc: close`. No check read them, since `every_hint_key_resolves_in_its_mode` +cannot see a string literal in a widget (the blind spot improvise-rr8n and +improvise-wk18 name). The literal was also already incomplete: the keymap binds +`help-page-next` to `n` and `→` as well, and `?` closes help, and the footer +advertised none of them. + +**Derive rather than delete.** rr8n's rule is to keep whichever table is +derived from the thing it describes, and the derived one here is the status +row's `hint_line(Help)`. The popup's margins always leave that row uncovered, +which argues for deleting the footer's keys. But `draw_status` prints +`status_msg` *instead of* `hint_text()` whenever the message is non-empty, and +nothing clears the message on entering Help. So after any message ("Saved to +…", an error), a user who presses `?` would see no paging keys at all. The +footer therefore now draws the page indicator followed by `hint_line(Help)` +itself, truncated in display cells (gotcha 50). + +**The test.** It renders the popup at 140×30, wide enough that the whole hint +fits (the rr8n lesson: an 80-column render can hide the very thing asserted), +on every page. It requires the footer row to contain `hint_line(&AppMode::Help)` +exactly, and first asserts that hint is non-empty and pages. + +**Red, and the rival that matters.** +- At base it fails on page 0: `"page 1 of 5 l/Tab: next q/Esc: close"` is + not `"Tab/l/n/→:next-page S-Tab/h/p/←:prev-page ?/Esc/q:close"`. +- A footer holding a literal copy of today's derived string passes today, so it + was injected together with a rebinding (`n` removed from `help-page-next` in + the protocol keymap). The test then fails: the copy still advertises `n`, + which the keymap no longer binds. That rebinding case is exactly the one the + ticket said the old footer would lie through. diff --git a/src/ui/help.rs b/src/ui/help.rs index 30fffcfd..86e5863b 100644 --- a/src/ui/help.rs +++ b/src/ui/help.rs @@ -6,6 +6,7 @@ use ratatui::{ }; use crate::protocol::help::{self, HelpLine}; +use crate::ui::text::truncate; /// Number of help pages available (defined alongside the help-page state). pub use crate::ui::app::HELP_PAGE_COUNT; @@ -228,66 +229,49 @@ fn render_tab_bar(buf: &mut Buffer, x: u16, y: u16, width: u16, active_page: usi } } -/// Renders the footer with page navigation hints. +/// The page indicator, then Help mode's own hint off the keymap: a footer that +/// spelled its keys was a second table nothing checked (improvise-17ak). fn render_footer(buf: &mut Buffer, x: u16, y: u16, width: u16, page: usize) { let dim = Style::default().fg(Color::DarkGray); - let key_style = Style::default().fg(Color::Cyan); + let hint = crate::protocol::hint_line(&crate::ui::app::AppMode::Help); + let line = format!(" page {} of {} {hint}", page + 1, HELP_PAGE_COUNT); + buf.set_string(x, y, truncate(&line, width as usize), dim); +} - let page_indicator = format!(" page {} of {} ", page + 1, HELP_PAGE_COUNT); +#[cfg(test)] +mod tests { + use super::*; + use crate::ui::app::AppMode; - let nav_parts: Vec<(&str, Style)> = if page == 0 && HELP_PAGE_COUNT > 1 { - vec![ - (" ", dim), - ("l", key_style), - ("/", dim), - ("Tab", key_style), - (": next", dim), - ] - } else if page >= HELP_PAGE_COUNT - 1 { - vec![ - (" ", dim), - ("h", key_style), - ("/", dim), - ("S-Tab", key_style), - (": prev", dim), - ] - } else { - vec![ - (" ", dim), - ("h", key_style), - (": prev ", dim), - ("l", key_style), - (": next", dim), - ] - }; - - let close_parts: Vec<(&str, Style)> = vec![ - (" ", dim), - ("q", key_style), - ("/", dim), - ("Esc", key_style), - (": close", dim), - ]; - - // Render page indicator on the left - buf.set_string(x, y, &page_indicator, dim); - - // Render navigation hints after the page indicator - let mut col = x + page_indicator.len() as u16; - for (text, style) in &nav_parts { - if col >= x + width { - break; - } - buf.set_string(col, y, text, *style); - col += text.len() as u16; + fn rows(buf: &Buffer) -> Vec { + let w = buf.area().width as usize; + buf.content() + .chunks(w) + .map(|row| row.iter().map(|c| c.symbol()).collect::()) + .collect() } - // Render close hint - for (text, style) in &close_parts { - if col >= x + width { - break; + /// The footer's keys are `hint_line(Help)`'s, not a table of its own + /// (improvise-17ak). Why the width and every page: `context/test-evidence/tui.md`. + #[test] + fn the_help_footer_names_the_keys_the_keymap_binds() { + let hint = crate::protocol::hint_line(&AppMode::Help); + assert!( + hint.contains("next-page"), + "the derived hint pages: {hint:?}" + ); + let area = Rect::new(0, 0, 140, 30); + for page in 0..HELP_PAGE_COUNT { + let mut buf = Buffer::empty(area); + HelpWidget::new(page).render(area, &mut buf); + let rows = rows(&buf); + let marker = format!("page {} of", page + 1); + let footer = rows.iter().find(|row| row.contains(&marker)); + let footer = footer.unwrap_or_else(|| panic!("page {page}: no footer row")); + assert!( + footer.contains(&hint), + "page {page}: the footer is not the keymap's hint {hint:?}: {footer:?}" + ); } - buf.set_string(col, y, text, *style); - col += text.len() as u16; } }