From 1b99000e998f2ff993e068ce3242a9fcd4d5bb19 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Thu, 23 Apr 2026 23:51:49 +0530 Subject: [PATCH 1/3] feat(viewer): add h and ? as help keys alongside F1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported in issue #44: Terminator (the terminal emulator) intercepts F1 for its own help menu, so mdterm's help was effectively unreachable there. Users had exactly one way to open the help overlay, and that way was being stolen by the terminal. Let's fix that. Bind `h` and `H` to open help from markdown Normal mode, matching the convention most vim-adjacent TUIs use. That alone wasn't enough though — `h` is already bound in JSON view (collapse node) and slide mode (previous slide), so those users would still be stuck. So also wire `?` into the global help toggle next to F1. `?` is the universal TUI help key (vim, lazygit, tig, k9s, fzf, take your pick) and it wasn't bound anywhere, which made it the obvious choice. The `?` handler is gated out of Search and FuzzyHeading modes since those are text-input contexts where `?` is a perfectly legitimate character to type. Confusion would otherwise ensue. Status bar advertises `?` (works everywhere); the help overlay lists all three keys for discoverability. --- src/viewer.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/viewer.rs b/src/viewer.rs index 20ecba2..76649f4 100644 --- a/src/viewer.rs +++ b/src/viewer.rs @@ -954,8 +954,13 @@ fn handle_event(state: &mut ViewerState, ev: Event) -> bool { if ke.code == KeyCode::Char('c') && ke.modifiers.contains(KeyModifiers::CONTROL) { return true; } - // F1 opens help from any mode; Esc/F1 closes it - if ke.code == KeyCode::F(1) { + // F1 / ? open help from any mode; Esc/F1/? close it. + // `?` is skipped in text-input modes (Search, FuzzyHeading) where + // the user may legitimately type it. + let is_help_toggle = ke.code == KeyCode::F(1) + || (ke.code == KeyCode::Char('?') + && !matches!(state.mode, ViewMode::Search | ViewMode::FuzzyHeading)); + if is_help_toggle { if state.mode == ViewMode::Help { state.mode = ViewMode::Normal; } else { @@ -970,7 +975,7 @@ fn handle_event(state: &mut ViewerState, ev: Event) -> bool { let prev_scroll = state.help_scroll; let prev_mode = state.mode; match ke.code { - KeyCode::Esc | KeyCode::Char('q') => { + KeyCode::Esc | KeyCode::Char('q') | KeyCode::Char('h') | KeyCode::Char('H') => { state.mode = ViewMode::Normal; } KeyCode::Down | KeyCode::Char('j') => { @@ -1579,6 +1584,13 @@ fn handle_normal(state: &mut ViewerState, code: KeyCode, mods: KeyModifiers) -> } } + // Help + KeyCode::Char('h') | KeyCode::Char('H') => { + reset_cursor_shape(state); + state.help_scroll = 0; + state.mode = ViewMode::Help; + } + // Theme toggle KeyCode::Char('t') => { state.theme = state.theme.toggle(); @@ -2928,7 +2940,7 @@ fn render_status_bar(stdout: &mut io::Stdout, state: &ViewerState) -> io::Result }; let loading_len = loading_label.chars().count(); - let hint = " / search · o toc · f links · t theme · F1 help "; + let hint = " / search · o toc · f links · t theme · ? help "; let hint_len = hint.chars().count(); let needed = 4 + hint_len + loading_len + pos_len; let (show_hint, fill) = if width > needed { @@ -3590,7 +3602,7 @@ pub(crate) fn help_sections() -> &'static [HelpSection] { ("o", "Table of contents"), ("f", "Link picker (open URLs)"), (":", "Fuzzy heading jump"), - ("F1", "This help screen"), + ("h / ? / F1", "This help screen"), ], }, HelpSection { From 789f1538b4cd332096b848ebfbce46a843a8bc6d Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Fri, 24 Apr 2026 00:03:09 +0530 Subject: [PATCH 2/3] fix(viewer): guard help keys against Ctrl and sync docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first pass at the h/?/F1 help bindings had two holes worth closing before anyone notices. First, `Ctrl+H` emits as `Char('h')` on most terminals — it's the traditional backspace byte — and the new bindings were matching bare `KeyCode::Char('h')` without looking at modifiers. So Ctrl+H from Normal mode would pop the help overlay, and from inside Help it would close it. Not what anyone asked for. Gate both match arms on `!CONTROL`. Second, the README and the help overlay's own footer were still advertising only F1. The help *body* got updated, the footer and docs did not. Bring them in sync so the overlay doesn't contradict itself and the README doesn't lie. Small stuff, but the kind of inconsistency that rots quickly if you leave it alone. --- README.md | 2 +- src/viewer.rs | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 02e5f6a..4f2efd3 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,7 @@ mdterm README.md | less -R | `Y` | Copy entire document to clipboard | | `c` | Copy nearest code block to clipboard | | `Tab` / `Shift+Tab` | Switch between files | -| `F1` | Help screen | +| `h` / `?` / `F1` | Help screen | | `q` / `Ctrl+C` | Quit | ### Slide Mode (`--slides`) diff --git a/src/viewer.rs b/src/viewer.rs index 76649f4..c767592 100644 --- a/src/viewer.rs +++ b/src/viewer.rs @@ -975,7 +975,12 @@ fn handle_event(state: &mut ViewerState, ev: Event) -> bool { let prev_scroll = state.help_scroll; let prev_mode = state.mode; match ke.code { - KeyCode::Esc | KeyCode::Char('q') | KeyCode::Char('h') | KeyCode::Char('H') => { + KeyCode::Esc | KeyCode::Char('q') => { + state.mode = ViewMode::Normal; + } + KeyCode::Char('h') | KeyCode::Char('H') + if !ke.modifiers.contains(KeyModifiers::CONTROL) => + { state.mode = ViewMode::Normal; } KeyCode::Down | KeyCode::Char('j') => { @@ -1585,7 +1590,7 @@ fn handle_normal(state: &mut ViewerState, code: KeyCode, mods: KeyModifiers) -> } // Help - KeyCode::Char('h') | KeyCode::Char('H') => { + KeyCode::Char('h') | KeyCode::Char('H') if !mods.contains(KeyModifiers::CONTROL) => { reset_cursor_shape(state); state.help_scroll = 0; state.mode = ViewMode::Help; @@ -3783,7 +3788,7 @@ fn render_help_overlay(stdout: &mut io::Stdout, state: &ViewerState) -> io::Resu // Footer let scroll_hint = if can_scroll_down { " ▼ more " } else { "" }; - let footer = " F1 / Esc / q close "; + let footer = " ? / F1 / Esc / q close "; let footer_len = footer.chars().count() + scroll_hint.chars().count(); let bot_dashes = box_w.saturating_sub(3 + footer_len); queue!( From ec9b03722ae0731f18fc26b77091242e694ebc4a Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Fri, 24 Apr 2026 00:20:52 +0530 Subject: [PATCH 3/3] refactor(viewer): consolidate help-toggle logic into one predicate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The help overlay's open/close behavior was spread across three places — an `is_help_toggle` for F1/`?`, a separate `h`/`H` close branch inside the Help-mode handler, and yet another `h`/`H` open branch in `handle_normal`. Three rules, three contexts, three sets of guards. Figuring out when a key actually toggled help required reading all three and mentally assembling the answer. While the split was working, it was also inconsistent. `h`/`H` got a Ctrl guard in the follow-up fix; `?` did not, so Ctrl+? still popped help for no reason. And the help-overlay footer advertised `" ? / F1 / Esc / q close "` even though `h` would close it too. The code and the UI disagreed about the close set. Collapse the whole thing into a single `is_help_toggle(code, mods, mode, slide_mode, json_nav)` predicate and delete the two duplicate branches. Same predicate now Ctrl-guards `?` symmetrically and encodes the slide-mode / JSON-nav yielding rules that used to be implicit in call-site ordering. Footer updated to match reality. Add a testable `ViewMode::accepts_text_input()` method so the text-input-mode check isn't an inline denylist that someone has to remember to update when a new mode shows up. And eight unit tests over the predicate matrix, so the next person who adds a mode finds out at `cargo test` time instead of in the wild. While at it, clear out all 21 pre-existing clippy warnings — collapsible match arms that were one-line-ifs hiding as arm bodies, redundant `|l| line_text(l)` closures, a useless `format!` with no interpolation, a `for stream in incoming() { if let Ok(...) }` that `flatten()` exists to kill, and a manual `if divisor > 0 { a / b }` that's exactly what `checked_div` is for. None of it was broken. None of it was pretty either. --- src/image.rs | 11 +- src/markdown.rs | 10 +- src/style.rs | 8 +- src/viewer.rs | 332 +++++++++++++++++++++++++++++++++++------------- 4 files changed, 260 insertions(+), 101 deletions(-) diff --git a/src/image.rs b/src/image.rs index 26f34a2..97722ec 100644 --- a/src/image.rs +++ b/src/image.rs @@ -2471,10 +2471,8 @@ mod tests { let addr = listener.local_addr().ok()?; let base = format!("http://127.0.0.1:{}", addr.port()); std::thread::spawn(move || { - for stream in listener.incoming() { - if let Ok(stream) = stream { - let _ = (handler)(stream); - } + for stream in listener.incoming().flatten() { + (handler)(stream); } }); Some((base, addr)) @@ -2511,9 +2509,8 @@ mod tests { let n = stream.read(&mut buf).unwrap_or(0); let req = String::from_utf8_lossy(&buf[..n]); if req.contains("GET /redirect") { - let resp = format!( - "HTTP/1.1 302 Found\r\nLocation: /image.png\r\nContent-Length: 0\r\n\r\n" - ); + let resp = + "HTTP/1.1 302 Found\r\nLocation: /image.png\r\nContent-Length: 0\r\n\r\n"; let _ = stream.write_all(resp.as_bytes()); } else { let body = png.clone(); diff --git a/src/markdown.rs b/src/markdown.rs index cc779ae..92daf20 100644 --- a/src/markdown.rs +++ b/src/markdown.rs @@ -574,8 +574,8 @@ impl<'a> Renderer<'a> { flex_remaining -= 1; if flex_remaining == 0 { *w = remaining; - } else if flex_natural > 0 { - let share = (*w * flex_available / flex_natural).max(3); + } else if let Some(share) = (*w * flex_available).checked_div(flex_natural) { + let share = share.max(3); *w = share; remaining = remaining.saturating_sub(share); } @@ -1733,7 +1733,7 @@ mod tests { fn unordered_list_has_bullets() { let input = "- item one\n- item two"; let (lines, _) = render_test(input); - let texts: Vec = lines.iter().map(|l| line_text(l)).collect(); + let texts: Vec = lines.iter().map(line_text).collect(); let bullet_lines: Vec<_> = texts.iter().filter(|t| t.contains('•')).collect(); assert_eq!(bullet_lines.len(), 2); } @@ -1742,7 +1742,7 @@ mod tests { fn ordered_list_has_numbers() { let input = "1. first\n2. second"; let (lines, _) = render_test(input); - let texts: Vec = lines.iter().map(|l| line_text(l)).collect(); + let texts: Vec = lines.iter().map(line_text).collect(); assert!(texts.iter().any(|t| t.contains("1."))); assert!(texts.iter().any(|t| t.contains("2."))); } @@ -1753,7 +1753,7 @@ mod tests { fn blockquote_produces_styled_output() { let input = "> quoted text"; let (lines, _) = render_test(input); - let texts: Vec = lines.iter().map(|l| line_text(l)).collect(); + let texts: Vec = lines.iter().map(line_text).collect(); assert!(texts.iter().any(|t| t.contains("quoted text"))); } diff --git a/src/style.rs b/src/style.rs index b241778..e76517a 100644 --- a/src/style.rs +++ b/src/style.rs @@ -311,7 +311,7 @@ mod tests { assert!(line.display_width() <= 4); } // All characters preserved - let all: String = wrapped.iter().map(|l| line_text(l)).collect(); + let all: String = wrapped.iter().map(line_text).collect(); assert_eq!(all, "abcdefghij"); } @@ -419,7 +419,7 @@ mod tests { line.display_width() ); } - let all: String = wrapped.iter().map(|l| line_text(l)).collect(); + let all: String = wrapped.iter().map(line_text).collect(); assert_eq!(all, "你好世界测试"); } @@ -435,7 +435,7 @@ mod tests { for line in &wrapped { assert!(line.display_width() <= 4); } - let all: String = wrapped.iter().map(|l| line_text(l)).collect(); + let all: String = wrapped.iter().map(line_text).collect(); assert_eq!(all, "🎉🎊🎈"); } @@ -467,7 +467,7 @@ mod tests { "first span should preserve bold style" ); // All text should be preserved across wrapped lines - let all_text: String = wrapped.iter().map(|l| line_text(l)).collect(); + let all_text: String = wrapped.iter().map(line_text).collect(); assert!(all_text.contains("bold")); assert!(all_text.contains("normal")); assert!(all_text.contains("text")); diff --git a/src/viewer.rs b/src/viewer.rs index c767592..caf14dd 100644 --- a/src/viewer.rs +++ b/src/viewer.rs @@ -226,7 +226,7 @@ impl Drop for TerminalGuard { // ── View modes ────────────────────────────────────────────────────────────── -#[derive(PartialEq, Copy, Clone)] +#[derive(PartialEq, Copy, Clone, Debug)] enum ViewMode { Normal, Search, @@ -236,6 +236,48 @@ enum ViewMode { Help, } +impl ViewMode { + /// Modes where the user is typing free-form text; single-letter bindings + /// like `?` or `h` must be passed through as input, not intercepted. + fn accepts_text_input(self) -> bool { + matches!(self, ViewMode::Search | ViewMode::FuzzyHeading) + } +} + +/// Returns true when JSON navigation would consume letter keys (`h`/`H`). +fn json_nav_active(state: &ViewerState) -> bool { + state + .json_view + .as_ref() + .is_some_and(|jv| !jv.navigable.is_empty()) +} + +/// Single source of truth for "does this key toggle the help overlay?". +/// +/// - `F1` — from any mode. +/// - `?` — from any mode except text-input modes. Ctrl-guarded. +/// - `h` / `H` — only from Normal (to open) or Help (to close), Ctrl-guarded, +/// and yields to slide-mode and JSON navigation which bind `h` themselves. +fn is_help_toggle( + code: KeyCode, + modifiers: KeyModifiers, + mode: ViewMode, + slide_mode: bool, + json_nav: bool, +) -> bool { + let ctrl = modifiers.contains(KeyModifiers::CONTROL); + match code { + KeyCode::F(1) => true, + KeyCode::Char('?') if !ctrl => !mode.accepts_text_input(), + KeyCode::Char('h') | KeyCode::Char('H') if !ctrl => match mode { + ViewMode::Help => true, + ViewMode::Normal => !slide_mode && !json_nav, + _ => false, + }, + _ => false, + } +} + // ── Viewer state ──────────────────────────────────────────────────────────── struct ViewerState { @@ -954,13 +996,13 @@ fn handle_event(state: &mut ViewerState, ev: Event) -> bool { if ke.code == KeyCode::Char('c') && ke.modifiers.contains(KeyModifiers::CONTROL) { return true; } - // F1 / ? open help from any mode; Esc/F1/? close it. - // `?` is skipped in text-input modes (Search, FuzzyHeading) where - // the user may legitimately type it. - let is_help_toggle = ke.code == KeyCode::F(1) - || (ke.code == KeyCode::Char('?') - && !matches!(state.mode, ViewMode::Search | ViewMode::FuzzyHeading)); - if is_help_toggle { + if is_help_toggle( + ke.code, + ke.modifiers, + state.mode, + state.slide_mode, + json_nav_active(state), + ) { if state.mode == ViewMode::Help { state.mode = ViewMode::Normal; } else { @@ -978,11 +1020,6 @@ fn handle_event(state: &mut ViewerState, ev: Event) -> bool { KeyCode::Esc | KeyCode::Char('q') => { state.mode = ViewMode::Normal; } - KeyCode::Char('h') | KeyCode::Char('H') - if !ke.modifiers.contains(KeyModifiers::CONTROL) => - { - state.mode = ViewMode::Normal; - } KeyCode::Down | KeyCode::Char('j') => { let total = help_total_rows(); let (_, _, _, _, visible) = @@ -1589,13 +1626,6 @@ fn handle_normal(state: &mut ViewerState, code: KeyCode, mods: KeyModifiers) -> } } - // Help - KeyCode::Char('h') | KeyCode::Char('H') if !mods.contains(KeyModifiers::CONTROL) => { - reset_cursor_shape(state); - state.help_scroll = 0; - state.mode = ViewMode::Help; - } - // Theme toggle KeyCode::Char('t') => { state.theme = state.theme.toggle(); @@ -1649,48 +1679,42 @@ fn handle_normal(state: &mut ViewerState, code: KeyCode, mods: KeyModifiers) -> } // TOC - KeyCode::Char('o') => { - if !state.toc_entries.is_empty() { - reset_cursor_shape(state); - state.toc_selected = 0; - state.toc_scroll = 0; - // Try to select the heading closest to current offset - for (i, entry) in state.toc_entries.iter().enumerate() { - if entry.line_idx <= state.offset { - state.toc_selected = i; - } - } - // Ensure scroll shows the selected entry - let viewport = state.viewport(); - let count = state.toc_entries.len(); - let box_h = (count + 2).min(viewport.saturating_sub(4)); - let visible_entries = box_h.saturating_sub(2); - if visible_entries > 0 && state.toc_selected >= visible_entries { - state.toc_scroll = state.toc_selected - visible_entries + 1; + KeyCode::Char('o') if !state.toc_entries.is_empty() => { + reset_cursor_shape(state); + state.toc_selected = 0; + state.toc_scroll = 0; + // Try to select the heading closest to current offset + for (i, entry) in state.toc_entries.iter().enumerate() { + if entry.line_idx <= state.offset { + state.toc_selected = i; } - state.mode = ViewMode::Toc; } + // Ensure scroll shows the selected entry + let viewport = state.viewport(); + let count = state.toc_entries.len(); + let box_h = (count + 2).min(viewport.saturating_sub(4)); + let visible_entries = box_h.saturating_sub(2); + if visible_entries > 0 && state.toc_selected >= visible_entries { + state.toc_scroll = state.toc_selected - visible_entries + 1; + } + state.mode = ViewMode::Toc; } // Link picker - KeyCode::Char('f') => { - if !state.link_entries.is_empty() { - reset_cursor_shape(state); - state.link_selected = 0; - state.link_scroll = 0; - state.mode = ViewMode::LinkPicker; - } + KeyCode::Char('f') if !state.link_entries.is_empty() => { + reset_cursor_shape(state); + state.link_selected = 0; + state.link_scroll = 0; + state.mode = ViewMode::LinkPicker; } // Fuzzy heading search - KeyCode::Char(':') => { - if !state.toc_entries.is_empty() { - reset_cursor_shape(state); - state.fuzzy_input.clear(); - state.fuzzy_selected = 0; - state.fuzzy_scroll = 0; - state.mode = ViewMode::FuzzyHeading; - } + KeyCode::Char(':') if !state.toc_entries.is_empty() => { + reset_cursor_shape(state); + state.fuzzy_input.clear(); + state.fuzzy_selected = 0; + state.fuzzy_scroll = 0; + state.mode = ViewMode::FuzzyHeading; } // Copy full document @@ -1726,21 +1750,17 @@ fn handle_normal(state: &mut ViewerState, code: KeyCode, mods: KeyModifiers) -> } // File switching - KeyCode::Tab => { - if state.files.len() > 1 { - let next = (state.current_file_idx + 1) % state.files.len(); - state.switch_file(next); - } + KeyCode::Tab if state.files.len() > 1 => { + let next = (state.current_file_idx + 1) % state.files.len(); + state.switch_file(next); } - KeyCode::BackTab => { - if state.files.len() > 1 { - let prev = if state.current_file_idx == 0 { - state.files.len() - 1 - } else { - state.current_file_idx - 1 - }; - state.switch_file(prev); - } + KeyCode::BackTab if state.files.len() > 1 => { + let prev = if state.current_file_idx == 0 { + state.files.len() - 1 + } else { + state.current_file_idx - 1 + }; + state.switch_file(prev); } KeyCode::Backspace => { if let Some((file_idx, offset)) = state.nav_history.pop() { @@ -1789,10 +1809,10 @@ fn handle_slide_keys(state: &mut ViewerState, code: KeyCode) -> bool { | KeyCode::Char('l') | KeyCode::Char('j') | KeyCode::Down - | KeyCode::PageDown => { - if state.current_slide + 1 < num_slides { - state.current_slide += 1; - } + | KeyCode::PageDown + if state.current_slide + 1 < num_slides => + { + state.current_slide += 1; } KeyCode::Left | KeyCode::Char('h') @@ -1861,10 +1881,8 @@ fn handle_toc(state: &mut ViewerState, code: KeyCode) { KeyCode::Up | KeyCode::Char('k') => { state.toc_selected = state.toc_selected.saturating_sub(1); } - KeyCode::Down | KeyCode::Char('j') => { - if state.toc_selected + 1 < count { - state.toc_selected += 1; - } + KeyCode::Down | KeyCode::Char('j') if state.toc_selected + 1 < count => { + state.toc_selected += 1; } KeyCode::PageUp => { state.toc_selected = state.toc_selected.saturating_sub(visible_entries); @@ -2031,10 +2049,8 @@ fn handle_link_picker(state: &mut ViewerState, code: KeyCode) { KeyCode::Up | KeyCode::Char('k') => { state.link_selected = state.link_selected.saturating_sub(1); } - KeyCode::Down | KeyCode::Char('j') => { - if state.link_selected + 1 < count { - state.link_selected += 1; - } + KeyCode::Down | KeyCode::Char('j') if state.link_selected + 1 < count => { + state.link_selected += 1; } KeyCode::PageUp => { state.link_selected = state.link_selected.saturating_sub(visible_entries); @@ -3788,7 +3804,7 @@ fn render_help_overlay(stdout: &mut io::Stdout, state: &ViewerState) -> io::Resu // Footer let scroll_hint = if can_scroll_down { " ▼ more " } else { "" }; - let footer = " ? / F1 / Esc / q close "; + let footer = " h / ? / F1 / Esc / q close "; let footer_len = footer.chars().count() + scroll_hint.chars().count(); let bot_dashes = box_w.saturating_sub(3 + footer_len); queue!( @@ -3988,6 +4004,152 @@ mod tests { } } + // ── is_help_toggle matrix ───────────────────────────────────────────── + + const ALL_MODES: &[ViewMode] = &[ + ViewMode::Normal, + ViewMode::Search, + ViewMode::Toc, + ViewMode::LinkPicker, + ViewMode::FuzzyHeading, + ViewMode::Help, + ]; + + fn toggle(code: KeyCode, mods: KeyModifiers, mode: ViewMode) -> bool { + is_help_toggle(code, mods, mode, false, false) + } + + #[test] + fn f1_toggles_from_every_mode() { + for &m in ALL_MODES { + assert!( + toggle(KeyCode::F(1), KeyModifiers::NONE, m), + "F1 should toggle help from {:?}", + m + ); + } + } + + #[test] + fn question_mark_toggles_except_text_input() { + for &m in ALL_MODES { + let expected = !m.accepts_text_input(); + assert_eq!( + toggle(KeyCode::Char('?'), KeyModifiers::NONE, m), + expected, + "`?` toggle in {:?} should be {}", + m, + expected + ); + } + } + + #[test] + fn ctrl_question_mark_never_toggles() { + for &m in ALL_MODES { + assert!( + !toggle(KeyCode::Char('?'), KeyModifiers::CONTROL, m), + "Ctrl+? must not toggle help (mode {:?})", + m + ); + } + } + + #[test] + fn h_opens_from_normal_and_closes_from_help_only() { + for &m in ALL_MODES { + let expected = matches!(m, ViewMode::Normal | ViewMode::Help); + for code in [KeyCode::Char('h'), KeyCode::Char('H')] { + assert_eq!( + toggle(code, KeyModifiers::NONE, m), + expected, + "{:?} in {:?} should be {}", + code, + m, + expected + ); + } + } + } + + #[test] + fn ctrl_h_never_toggles() { + for &m in ALL_MODES { + for code in [KeyCode::Char('h'), KeyCode::Char('H')] { + assert!( + !is_help_toggle(code, KeyModifiers::CONTROL, m, false, false), + "Ctrl+{:?} must not toggle help (mode {:?})", + code, + m + ); + } + } + } + + #[test] + fn h_yields_to_slide_and_json_nav_but_question_mark_and_f1_do_not() { + // h/H must not steal focus from slide-mode prev-slide or JSON navigation + for &(slide, json) in &[(true, false), (false, true), (true, true)] { + for code in [KeyCode::Char('h'), KeyCode::Char('H')] { + assert!( + !is_help_toggle(code, KeyModifiers::NONE, ViewMode::Normal, slide, json), + "h/H must yield when slide={} json_nav={}", + slide, + json + ); + } + // But ? and F1 remain universal escape hatches. + assert!(is_help_toggle( + KeyCode::Char('?'), + KeyModifiers::NONE, + ViewMode::Normal, + slide, + json + )); + assert!(is_help_toggle( + KeyCode::F(1), + KeyModifiers::NONE, + ViewMode::Normal, + slide, + json + )); + } + } + + #[test] + fn h_still_closes_help_even_when_slide_or_json_nav_context_remains() { + // Closing help from Help mode must work regardless of the backdrop state. + for &(slide, json) in &[(false, false), (true, false), (false, true), (true, true)] { + for code in [KeyCode::Char('h'), KeyCode::Char('H')] { + assert!( + is_help_toggle(code, KeyModifiers::NONE, ViewMode::Help, slide, json), + "h/H must close Help regardless of slide/json context" + ); + } + } + } + + #[test] + fn unrelated_keys_never_toggle() { + for &m in ALL_MODES { + for code in [ + KeyCode::Char('j'), + KeyCode::Char('k'), + KeyCode::Char('q'), + KeyCode::Esc, + KeyCode::Enter, + KeyCode::F(2), + ] { + assert!( + !toggle(code, KeyModifiers::NONE, m), + "{:?} must not toggle help (mode {:?})", + code, + m + ); + } + } + } + #[test] fn help_box_dimensions_reasonable_80x24() { let (key_col, desc_col, box_w, box_h, visible_rows) = help_box_dimensions(80, 24); @@ -4141,8 +4303,8 @@ mod tests { span("Hello ", None), span("click me", Some("https://example.com")), ])]); - // Click on "Hello " (no link) - assert_eq!(state.link_at_position(1, 2 + 0), None); + // Gutter is 2 cols; click on "Hello " (no link) at content cols 0 and 5. + assert_eq!(state.link_at_position(1, 2), None); assert_eq!(state.link_at_position(1, 2 + 5), None); } @@ -4184,8 +4346,8 @@ mod tests { span(" ", None), span("bb", Some("https://b.com")), ])]); - // "aa" at cols 0..2, " " at 2..3, "bb" at 3..5 - assert_eq!(state.link_at_position(1, 2 + 0), Some("https://a.com")); + // "aa" at cols 0..2, " " at 2..3, "bb" at 3..5 (offset by 2-col gutter) + assert_eq!(state.link_at_position(1, 2), Some("https://a.com")); assert_eq!(state.link_at_position(1, 2 + 1), Some("https://a.com")); assert_eq!(state.link_at_position(1, 2 + 2), None); // space assert_eq!(state.link_at_position(1, 2 + 3), Some("https://b.com"));