From b96d8f9a67ee08fb59b5f2c3305ba0701244ba2e Mon Sep 17 00:00:00 2001 From: altair823 Date: Sun, 3 May 2026 10:29:48 +0000 Subject: [PATCH] review(p9-fb-10-final): extract place_cursor_x helper + filter overlay render test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add `place_cursor_x(inner_x, inner_width, prompt_w, cursor_col) -> u16` to `input.rs`: sums in `usize` (no u16 wrap), clamps to inner right edge, tries_into with u16::MAX fallback. Two unit tests pin the clamp and the in-bounds path. - Re-export from `lib.rs` alongside `InputBuffer`, `display_width`, `truncate_to_display_width`. - Replace the open-coded 2-line `raw_x`/`cursor_x` blocks in Search, Ask, and Library with a single `place_cursor_x` call each — consistent usize arithmetic across all three panes. - Add `filter_overlay_render_places_cursor_on_focused_field` integration test in `tests/library.rs`: opens the filter overlay, renders through `TestBackend`, asserts `terminal.get_cursor_position().x > 0` (label offset > 0 proves `set_cursor_position` was called with a meaningful coordinate, not stuck at origin). Co-Authored-By: Claude Sonnet 4.6 --- crates/kebab-tui/src/ask.rs | 7 +++--- crates/kebab-tui/src/input.rs | 35 +++++++++++++++++++++++++++++ crates/kebab-tui/src/lib.rs | 2 +- crates/kebab-tui/src/library.rs | 7 +++--- crates/kebab-tui/src/search.rs | 11 ++++----- crates/kebab-tui/tests/library.rs | 37 +++++++++++++++++++++++++++++++ 6 files changed, 85 insertions(+), 14 deletions(-) diff --git a/crates/kebab-tui/src/ask.rs b/crates/kebab-tui/src/ask.rs index 38f5696..1a02bf9 100644 --- a/crates/kebab-tui/src/ask.rs +++ b/crates/kebab-tui/src/ask.rs @@ -83,9 +83,10 @@ fn render_input(f: &mut Frame, area: Rect, s: &AskState, theme: &crate::theme::T // omits set_cursor_position (Library/Inspect), ratatui calls // hide_cursor instead. So this single call both positions and // unhides the caret for the Ask input column. - let prompt_w = crate::input::display_width(PROMPT) as u16; - let raw_x = inner.x + prompt_w + s.input.cursor_col() as u16; - let cursor_x = raw_x.min(inner.x + inner.width.saturating_sub(1)); + // place_cursor_x sums in usize (avoiding u16 wrap) and clamps to + // the right edge of the inner area. + let prompt_w = crate::input::display_width(PROMPT); + let cursor_x = crate::input::place_cursor_x(inner.x, inner.width, prompt_w, s.input.cursor_col()); f.set_cursor_position((cursor_x, inner.y)); } diff --git a/crates/kebab-tui/src/input.rs b/crates/kebab-tui/src/input.rs index 2e52ce1..6f9cf73 100644 --- a/crates/kebab-tui/src/input.rs +++ b/crates/kebab-tui/src/input.rs @@ -32,6 +32,27 @@ use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; +/// Compute the cursor column for a text-input pane: prompt width + +/// content cursor, summed in `usize` to avoid `u16` overflow, then +/// clamped to fit within `inner_width` columns from `inner_x`. +/// +/// Use as: +/// ```ignore +/// f.set_cursor_position((place_cursor_x(inner.x, inner.width, prompt_w, buf.cursor_col()), inner.y)); +/// ``` +/// +/// If a fourth input pane is added, use this helper rather than +/// open-coding the arithmetic — one place to fix if the clamping +/// policy ever changes. +pub fn place_cursor_x(inner_x: u16, inner_width: u16, prompt_w: usize, cursor_col: usize) -> u16 { + let raw = (inner_x as usize) + .saturating_add(prompt_w) + .saturating_add(cursor_col); + let max = (inner_x as usize) + .saturating_add(inner_width.saturating_sub(1) as usize); + raw.min(max).try_into().unwrap_or(u16::MAX) +} + /// Display width of `s` in terminal columns. CJK / fullwidth = 2 /// per char, ASCII = 1, combining marks = 0. Sums every char's /// `unicode-width` reading — same calculation Ratatui uses @@ -317,4 +338,18 @@ mod tests { assert!(b.is_empty()); assert_eq!(b.cursor_col(), 0); } + + /// p9-fb-10: place_cursor_x clamps within the inner area. + #[test] + fn place_cursor_x_clamps_to_inner_right_edge() { + // inner.x=10, width=20, so the rightmost column is 10+20-1 = 29. + // prompt_w=2, cursor_col=100 (overflow) → clamped to 29. + assert_eq!(place_cursor_x(10, 20, 2, 100), 29); + } + + /// p9-fb-10: place_cursor_x preserves position when within bounds. + #[test] + fn place_cursor_x_keeps_position_when_within_bounds() { + assert_eq!(place_cursor_x(10, 20, 2, 5), 17); // 10 + 2 + 5 + } } diff --git a/crates/kebab-tui/src/lib.rs b/crates/kebab-tui/src/lib.rs index f9cccd5..0853b51 100644 --- a/crates/kebab-tui/src/lib.rs +++ b/crates/kebab-tui/src/lib.rs @@ -27,7 +27,7 @@ mod search; mod terminal; mod theme; -pub use input::{InputBuffer, display_width, truncate_to_display_width}; +pub use input::{InputBuffer, display_width, place_cursor_x, truncate_to_display_width}; pub use theme::{Palette, Role, Theme}; pub use app::{ App, AskState, IngestState, InspectState, InspectTarget, KeyOutcome, LibraryState, Mode, diff --git a/crates/kebab-tui/src/library.rs b/crates/kebab-tui/src/library.rs index d46165d..4863caf 100644 --- a/crates/kebab-tui/src/library.rs +++ b/crates/kebab-tui/src/library.rs @@ -153,13 +153,14 @@ fn render_filter_overlay(f: &mut Frame, area: Rect, edit: &FilterEdit, theme: &c // omits set_cursor_position (Library/Inspect main view), ratatui // calls hide_cursor instead. So this single call positions the // caret on the focused field of the filter overlay. + // place_cursor_x sums in usize (avoiding u16 wrap) and clamps to + // the right edge of the inner area. let (label, focused_buf, row_offset) = match edit.field { FilterField::Tags => (LABEL_TAGS, &edit.tags_buf, 0u16), FilterField::Lang => (LABEL_LANG, &edit.lang_buf, 1u16), }; - let label_w = display_width(label) as u16; - let raw_x = inner.x + label_w + focused_buf.cursor_col() as u16; - let cursor_x = raw_x.min(inner.x + inner.width.saturating_sub(1)); + let label_w = display_width(label); + let cursor_x = crate::input::place_cursor_x(inner.x, inner.width, label_w, focused_buf.cursor_col()); f.set_cursor_position((cursor_x, inner.y + row_offset)); } diff --git a/crates/kebab-tui/src/search.rs b/crates/kebab-tui/src/search.rs index a13a255..aee842f 100644 --- a/crates/kebab-tui/src/search.rs +++ b/crates/kebab-tui/src/search.rs @@ -91,13 +91,10 @@ fn render_input_bar(f: &mut Frame, area: Rect, s: &SearchState, theme: &crate::t // omits set_cursor_position (Library/Inspect), ratatui calls // hide_cursor instead. So this single call both positions and // unhides the caret for the Search input column. - let raw_x = inner.x + (prompt_w + s.input.cursor_col()) as u16; - // Clamp to the right edge of the inner area — a long CJK query - // in a narrow terminal could otherwise place the caret beyond - // the box; crossterm passes coords through verbatim. - let cursor_x = raw_x.min(inner.x + inner.width.saturating_sub(1)); - let cursor_y = inner.y; - f.set_cursor_position((cursor_x, cursor_y)); + // place_cursor_x sums in usize (avoiding u16 wrap) and clamps to + // the right edge of the inner area. + let cursor_x = crate::input::place_cursor_x(inner.x, inner.width, prompt_w, s.input.cursor_col()); + f.set_cursor_position((cursor_x, inner.y)); } fn mode_label(m: SearchMode) -> &'static str { diff --git a/crates/kebab-tui/tests/library.rs b/crates/kebab-tui/tests/library.rs index c971a49..1cbfff3 100644 --- a/crates/kebab-tui/tests/library.rs +++ b/crates/kebab-tui/tests/library.rs @@ -249,6 +249,43 @@ fn filter_overlay_accepts_hangul_tags() { ); } +/// p9-fb-10: filter overlay calls f.set_cursor_position so ratatui +/// shows the caret on the focused field. Pin: after opening the +/// overlay, render → terminal cursor is set + has non-zero x +/// (the label offset > 0). +#[test] +fn filter_overlay_render_places_cursor_on_focused_field() { + let mut app = app_with_docs(vec![make_doc("a.md", "A", vec![])]); + // Open filter. + let _ = kebab_tui::handle_key_library( + &mut app, + KeyEvent::new(KeyCode::Char('f'), KeyModifiers::NONE), + ); + let backend = TestBackend::new(80, 20); + let mut terminal = Terminal::new(backend).unwrap(); + terminal + .draw(|f| { + let area = Rect::new(0, 0, 80, 20); + render_library(f, area, &app); + }) + .expect("render must not panic"); + // After draw, ratatui calls backend.set_cursor_position when the + // frame's cursor_position is Some. The terminal's + // get_cursor_position proxies to the backend. + let pos = terminal.get_cursor_position().expect( + "filter overlay must call set_cursor_position, so cursor pos must be readable", + ); + // The Tags label ("tags_any (csv): ") has display_width 16; inner.x + // is 1 (inside border). With empty input cursor_col=0, expected x=17. + // We assert x>0 to avoid hardcoding the exact layout geometry while + // still confirming set_cursor_position was called with a meaningful + // offset (not stuck at origin). + assert!( + pos.x > 0, + "cursor x should be positive (label offset > 0): {pos:?}" + ); +} + /// p9-fb-10: Library renders Hangul / CJK titles without overflowing /// the title column. Smoke pin — render with a mixed Korean fixture /// and confirm no panic + the truncated width fits the column.