diff --git a/app/src/terminal/model/grid/grid_handler_tests.rs b/app/src/terminal/model/grid/grid_handler_tests.rs index da4b1cd043..6a24cd77b7 100644 --- a/app/src/terminal/model/grid/grid_handler_tests.rs +++ b/app/src/terminal/model/grid/grid_handler_tests.rs @@ -2495,3 +2495,45 @@ fn test_full_grid_clear_resize_then_bounds_to_string_does_not_panic() { ); } } + +/// Regression guard: resizing a terminal whose contents include wide +/// (full-width / CJK) characters must not panic. +/// +/// `resize_storage` converts each cursor to a flat-storage content offset via +/// `content_offset_at_point`, which can return `Err` for a cursor column that +/// does not map to tracked content. That result used to be unwrapped with +/// `.expect()`, which aborts the whole process — the crash users hit when a +/// terminal showing CJK text (e.g. a Korean directory name in the prompt) was +/// resized. The conversion now clamps to the nearest content cell instead. +/// +/// This drives mixed narrow/wide content through a reflow across many widths, +/// exercising the wide-char cursor-remapping path the fix hardens. +#[test] +fn test_resize_with_wide_chars_does_not_panic() { + let mut grid = GridHandler::new_for_test(4, 12); + + // A prompt-like first line mixing ASCII and full-width Korean syllables, + // a command, then more wide output — the kind of mixed-width content a + // Korean directory name produces. + for c in "~/한글폴더 $ ls".chars() { + grid.input(c); + } + grid.linefeed(); + grid.carriage_return(); + for c in "문서 사진 음악".chars() { + grid.input(c); + } + assert!( + grid.grid_storage()[VisibleRow(0)][2] + .flags + .intersects(Flags::WIDE_CHAR), + "expected a wide character in the prompt line" + ); + + // Reflow across a range of widths (including ones too narrow to hold a wide + // char in the trailing column) and back out. Every resize must complete + // without panicking. + for cols in [6, 3, 1, 5, 2, 8, 1, 4, 10, 2, 12] { + grid.resize(SizeInfo::new_without_font_metrics(4, cols)); + } +} diff --git a/app/src/terminal/model/grid/resize.rs b/app/src/terminal/model/grid/resize.rs index ad275da1ef..a16881f976 100644 --- a/app/src/terminal/model/grid/resize.rs +++ b/app/src/terminal/model/grid/resize.rs @@ -281,45 +281,125 @@ impl InitialCursorState { fn into_content_offset(self, grid: &GridHandler) -> CursorContentOffset { match self { Self::AtPoint(point) => { - let content_offset = grid - .flat_storage - .content_offset_at_point(point) - .expect("should have a content offset for point"); - CursorContentOffset::AtPoint(content_offset) + CursorContentOffset::AtPoint(content_offset_for_point(grid, point)) } Self::AtCellAfterPoint(point) => { - let content_offset = grid - .flat_storage - .content_offset_at_point(point) - .expect("should have a content offset for point"); - CursorContentOffset::AtCellAfterPoint(content_offset) + CursorContentOffset::AtCellAfterPoint(content_offset_for_point(grid, point)) } } } } +/// Resolves the flat-storage content offset for a cursor `point`, clamping the +/// column back toward the start of the row if the exact cell has no content +/// offset. +/// +/// The cursor `point` is derived from the live grid, whose per-column +/// accounting can diverge from flat storage's grapheme-run accounting for rows +/// that contain wide (full-width / CJK) characters: a wide char occupies two +/// columns but is a single grapheme, and a trailing wide-char spacer can push +/// `point.col` past the number of content cells flat storage tracks. In that +/// case `content_offset_at_point` returns `Err`, and unwrapping it used to +/// abort the whole app whenever a terminal containing wide characters was +/// resized. Instead, snap the cursor to the nearest preceding cell that does +/// have a content offset, mirroring the defensive clamping in +/// [`InitialCursorState::new`]. +/// +/// We walk backward over columns in the cursor's row first (the wide-char +/// case), and then, if even column 0 of that row is unresolvable, over earlier +/// rows. If nothing resolves (e.g. flat storage is empty), we return `None` +/// instead of fabricating a `ByteOffset::zero()`: content offsets count every +/// byte the grid has *ever* seen, so the earliest tracked offset is generally +/// greater than zero (and grows as scrollback is truncated from the front). A +/// zero offset would sit *before* the first stored row, which the later +/// `content_offset_to_point` conversion rejects — re-introducing the very panic +/// this function exists to prevent. The caller homes the cursor to the origin +/// when there is no anchor. +fn content_offset_for_point(grid: &GridHandler, point: Point) -> Option { + if let Ok(content_offset) = grid.flat_storage.content_offset_at_point(point) { + return Some(content_offset); + } + + log::warn!( + "no content offset for cursor point {point:?} during resize; \ + clamping to the nearest preceding content cell" + ); + + for row in (0..=point.row).rev() { + // For the cursor's own row, start just before the failing column; for + // earlier rows, start past the last possible column so the whole row is + // probed. Column 0 of a non-empty row always resolves, so as long as any + // row at or before the cursor holds content, the search terminates with + // a valid offset. + let start_col = if row == point.row { + point.col + } else { + grid.columns() + }; + for col in (0..start_col).rev() { + if let Ok(content_offset) = + grid.flat_storage.content_offset_at_point(Point { row, col }) + { + return Some(content_offset); + } + } + } + + // Flat storage tracks no resolvable cell at or before the cursor (e.g. it is + // empty). There is nothing to anchor the cursor to, so report the absence + // and let the caller home the cursor to the origin. + None +} + +/// Resolves a cursor content offset back to the grid point that anchors it, or +/// `None` when there is no anchor to map. +/// +/// [`content_offset_for_point`] yields `None` when flat storage holds no cell to +/// anchor the cursor to. Even a concrete offset can fail to convert if it falls +/// before the first stored row once scrollback has been truncated from the +/// front. Neither case has a meaningful prior position, so we return `None` and +/// let the caller home the cursor to the origin instead of unwrapping and +/// aborting the app mid-resize. Returning `None` (rather than the origin point) +/// keeps the no-anchor case distinguishable from a real anchor that happens to +/// land on the origin, so callers don't apply cell-after semantics to it. +fn resolve_content_offset(grid: &GridHandler, offset: Option) -> Option { + let offset = offset?; + grid.flat_storage + .content_offset_to_point(offset) + .map_err(|err| { + log::warn!( + "cursor content offset {offset:?} did not map to a point during \ + resize ({err:?}); homing cursor to origin" + ); + }) + .ok() +} + enum CursorContentOffset { - /// Cursor is at the location with the given byte offset in flat storage. - AtPoint(ByteOffset), - /// Cursor is at cell _after_ the location with the given byte offset in + /// Cursor is at the location with the given content offset in flat storage, + /// or `None` if flat storage held no cell to anchor the cursor to. + AtPoint(Option), + /// Cursor is at cell _after_ the location with the given content offset in /// flat storage. This helps ensure we properly handle `input_needs_wrap` - /// cases. - AtCellAfterPoint(ByteOffset), + /// cases. `None` carries the same no-anchor meaning as `AtPoint`. + AtCellAfterPoint(Option), } impl CursorContentOffset { fn into_cursor_point(self, new_cols: usize, grid: &GridHandler) -> FinalCursorState { + let origin = Point { row: 0, col: 0 }; match self { - Self::AtPoint(byte_offset) => FinalCursorState::AtPoint( - grid.flat_storage - .content_offset_to_point(byte_offset) - .expect("content offset should be valid"), + Self::AtPoint(offset) => FinalCursorState::AtPoint( + resolve_content_offset(grid, offset).unwrap_or(origin), ), - Self::AtCellAfterPoint(byte_offset) => { - let mut point = grid - .flat_storage - .content_offset_to_point(byte_offset) - .expect("content offset should be valid"); + Self::AtCellAfterPoint(offset) => { + // With no anchor there is nothing to advance past, so home the + // cursor to the origin rather than applying cell-after semantics + // (which would otherwise nudge the fabricated point to col 1 or + // flag `input_needs_wrap`). + let Some(mut point) = resolve_content_offset(grid, offset) else { + return FinalCursorState::AtPoint(origin); + }; // All data is in flat storage at the moment, so we need to // explicitly ask it about row wrapping. let input_needs_wrap =