Merge pull request #220 from fiddlerwoaroof/fix/17ak-help-footer

The TUI help footer's keys come off the keymap (17ak)

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.

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<String> {
+        let w = buf.area().width as usize;
+        buf.content()
+            .chunks(w)
+            .map(|row| row.iter().map(|c| c.symbol()).collect::<String>())
+            .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;
     }
 }