review(p9-fb-10-final): extract place_cursor_x helper + filter overlay render test
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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));
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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));
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user