fix(textarea): Up and Down land on the soft-wrapped row they target
A soft-wrapped row and the next row share a byte: the row's exclusive end is the next row's start. Up or Down with a goal column past the target row's width put the cursor on that byte, so it drew on the wrong row. Up made no progress and Down skipped a row. `row_with_end` tells a soft wrap from a hard break (byte-adjacent rows are soft). `move_to_display_col_on_line` takes the row and its `RowEnd` and clamps a soft row to its last grapheme, or to the start of an element that closes the row. A hard-break row still reaches its newline. `clamp_to_line` now cuts on graphemes, not chars, so a ZWJ cluster stays whole. Kigi differs from upstream's premise that elements wrap whole: with no display override in the buffer, `wrapped_lines` uses plain textwrap and an element can span rows. Snapping to such an element's start would leave the target row (Down from byte 0 stuck at 0), so an element that began on an earlier row keeps the boundary `display_col_to_buffer_pos` chose, as before this change. `wrapped_navigation_with_zwj_graphemes` asserted the bug: Up from the end stayed on the last row (byte 22). It now expects row 1's cluster start (byte 11), as upstream does. Tests: upstream's four cases, the row-spanning element, and a seeded property test (500 cases): each Up or Down moves exactly one row. The property test fails with the clamp disabled. Ported from upstream (grok-build ratatui-textarea/textarea.rs, 1.0.28).
This commit is contained in:
2 files changed
+191
-48
No files matched your search
@@ -267,6 +267,13 @@ struct WrapCache {
|
||||
lines: Vec<Range<usize>>,
|
||||
}
|
||||
|
||||
/// Whether a wrapped row's exclusive end is a cursor position on that row.
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
enum RowEnd {
|
||||
HardBreak,
|
||||
SoftWrap,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Clone, Copy)]
|
||||
pub struct TextAreaState {
|
||||
/// Index into wrapped lines of the first visible line.
|
||||
@@ -382,27 +389,13 @@ impl TextArea {
|
||||
}
|
||||
}
|
||||
|
||||
/// Clamp a buffer position so it stays within a wrapped line's range
|
||||
/// `[line_start, line_end)`. Without this, `display_col_to_buffer_pos`
|
||||
/// can return `line_end` when the column exceeds the line's display
|
||||
/// width — and `line_end` equals the *next* wrapped line's start,
|
||||
/// which confuses `effective_scroll` into thinking the cursor hasn't
|
||||
/// actually moved to the target line.
|
||||
///
|
||||
/// Uses `self.text` to find the last valid char boundary inside the line
|
||||
/// so we never land in the middle of a multi-byte character.
|
||||
/// Clamp `pos` to the start of the last grapheme in `[line_start, line_end)`.
|
||||
fn clamp_to_line(&self, pos: usize, line_start: usize, line_end: usize) -> usize {
|
||||
if line_end > line_start {
|
||||
// Find the start of the last character in the line.
|
||||
let last_char_start = self.text[line_start..line_end]
|
||||
.char_indices()
|
||||
.next_back()
|
||||
.map(|(i, _)| line_start + i)
|
||||
.unwrap_or(line_start);
|
||||
pos.min(last_char_start)
|
||||
} else {
|
||||
line_start
|
||||
}
|
||||
let last_grapheme_start = self.text[line_start..line_end]
|
||||
.grapheme_indices(true)
|
||||
.next_back()
|
||||
.map_or(line_start, |(i, _)| line_start + i);
|
||||
pos.min(last_grapheme_start)
|
||||
}
|
||||
|
||||
pub fn new() -> Self {
|
||||
@@ -1699,6 +1692,35 @@ impl TextArea {
|
||||
if idx == 0 { None } else { Some(idx - 1) }
|
||||
}
|
||||
|
||||
/// Soft-wrapped rows are byte-adjacent; a hard row leaves its newline between it and the next.
|
||||
fn row_with_end(lines: &[Range<usize>], idx: usize) -> Option<(Range<usize>, RowEnd)> {
|
||||
let row = lines.get(idx)?;
|
||||
let end_kind = match lines.get(idx + 1) {
|
||||
Some(next) if next.start == row.end => RowEnd::SoftWrap,
|
||||
_ => RowEnd::HardBreak,
|
||||
};
|
||||
Some((row.clone(), end_kind))
|
||||
}
|
||||
|
||||
fn clamp_to_row(&self, pos: usize, row: &Range<usize>, end_kind: RowEnd) -> usize {
|
||||
match end_kind {
|
||||
RowEnd::HardBreak => pos,
|
||||
RowEnd::SoftWrap => {
|
||||
let last_grapheme_start = self.clamp_to_line(pos, row.start, row.end);
|
||||
let Some(idx) = self.find_element_containing(last_grapheme_start) else {
|
||||
return last_grapheme_start;
|
||||
};
|
||||
let element_start = self.elements[idx].range.start;
|
||||
// A display-less element wraps as text; one that began on an earlier row keeps `pos`.
|
||||
if element_start < row.start {
|
||||
pos
|
||||
} else {
|
||||
element_start
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Map a display column to a buffer byte position on a given wrapped line.
|
||||
///
|
||||
/// Pure query — does not mutate any state. Handles elements (snapping to
|
||||
@@ -1791,13 +1813,12 @@ impl TextArea {
|
||||
|
||||
fn move_to_display_col_on_line(
|
||||
&mut self,
|
||||
line_start: usize,
|
||||
line_end: usize,
|
||||
row: Range<usize>,
|
||||
target_col: usize,
|
||||
end_kind: RowEnd,
|
||||
) {
|
||||
self.cursor_pos = self
|
||||
.display_col_to_buffer_pos(line_start, line_end, target_col)
|
||||
.0;
|
||||
let (pos, _hit_element) = self.display_col_to_buffer_pos(row.start, row.end, target_col);
|
||||
self.cursor_pos = self.clamp_to_row(pos, &row, end_kind);
|
||||
}
|
||||
|
||||
fn beginning_of_line(&self, pos: usize) -> usize {
|
||||
@@ -2652,14 +2673,10 @@ impl TextArea {
|
||||
let target_col = self.preferred_col.unwrap_or_else(|| {
|
||||
self.display_width_of_range(cur_range.start, self.cursor_pos)
|
||||
});
|
||||
if idx > 0 {
|
||||
let prev = &lines[idx - 1];
|
||||
let line_start = prev.start;
|
||||
let line_end = prev.end;
|
||||
Some((target_col, Some((line_start, line_end))))
|
||||
} else {
|
||||
Some((target_col, None))
|
||||
}
|
||||
let prev = idx
|
||||
.checked_sub(1)
|
||||
.and_then(|prev_idx| Self::row_with_end(lines, prev_idx));
|
||||
Some((target_col, prev))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
@@ -2669,11 +2686,11 @@ impl TextArea {
|
||||
} {
|
||||
// We had wrapping info. Apply movement accordingly.
|
||||
match maybe_line {
|
||||
Some((line_start, line_end)) => {
|
||||
Some((prev, end_kind)) => {
|
||||
if self.preferred_col.is_none() {
|
||||
self.preferred_col = Some(target_col);
|
||||
}
|
||||
self.move_to_display_col_on_line(line_start, line_end, target_col);
|
||||
self.move_to_display_col_on_line(prev, target_col, end_kind);
|
||||
return;
|
||||
}
|
||||
None => {
|
||||
@@ -2697,7 +2714,11 @@ impl TextArea {
|
||||
};
|
||||
let prev_line_start = self.text[..prev_nl].rfind('\n').map(|i| i + 1).unwrap_or(0);
|
||||
let prev_line_end = prev_nl;
|
||||
self.move_to_display_col_on_line(prev_line_start, prev_line_end, target_col);
|
||||
self.move_to_display_col_on_line(
|
||||
prev_line_start..prev_line_end,
|
||||
target_col,
|
||||
RowEnd::HardBreak,
|
||||
);
|
||||
} else {
|
||||
self.cursor_pos = 0;
|
||||
self.preferred_col = None;
|
||||
@@ -2716,14 +2737,7 @@ impl TextArea {
|
||||
let target_col = self.preferred_col.unwrap_or_else(|| {
|
||||
self.display_width_of_range(cur_range.start, self.cursor_pos)
|
||||
});
|
||||
if idx + 1 < lines.len() {
|
||||
let next = &lines[idx + 1];
|
||||
let line_start = next.start;
|
||||
let line_end = next.end;
|
||||
Some((target_col, Some((line_start, line_end))))
|
||||
} else {
|
||||
Some((target_col, None))
|
||||
}
|
||||
Some((target_col, Self::row_with_end(lines, idx + 1)))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
@@ -2732,11 +2746,11 @@ impl TextArea {
|
||||
}
|
||||
} {
|
||||
match move_to_last {
|
||||
Some((line_start, line_end)) => {
|
||||
Some((next, end_kind)) => {
|
||||
if self.preferred_col.is_none() {
|
||||
self.preferred_col = Some(target_col);
|
||||
}
|
||||
self.move_to_display_col_on_line(line_start, line_end, target_col);
|
||||
self.move_to_display_col_on_line(next, target_col, end_kind);
|
||||
return;
|
||||
}
|
||||
None => {
|
||||
@@ -2766,7 +2780,11 @@ impl TextArea {
|
||||
.find('\n')
|
||||
.map(|i| i + next_line_start)
|
||||
.unwrap_or(self.text.len());
|
||||
self.move_to_display_col_on_line(next_line_start, next_line_end, target_col);
|
||||
self.move_to_display_col_on_line(
|
||||
next_line_start..next_line_end,
|
||||
target_col,
|
||||
RowEnd::HardBreak,
|
||||
);
|
||||
} else {
|
||||
self.cursor_pos = self.text.len();
|
||||
self.preferred_col = None;
|
||||
@@ -3899,6 +3917,9 @@ fn truncate_line_display(line: &Line<'static>, max_width: usize) -> Line<'static
|
||||
Line::from(new_spans)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod soft_wrap_nav_tests;
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -6293,6 +6314,7 @@ mod tests {
|
||||
assert_eq!(t.cursor(), "👍👍".len());
|
||||
}
|
||||
|
||||
/// Each ZWJ cluster fills a 4-col row; Up from the end lands on row 1's cluster start.
|
||||
#[test]
|
||||
fn wrapped_navigation_with_zwj_graphemes() {
|
||||
let grapheme = "👩\u{200D}💻";
|
||||
@@ -6306,7 +6328,7 @@ mod tests {
|
||||
assert!(pos_after_down >= grapheme.len() * 2);
|
||||
|
||||
t.move_cursor_up();
|
||||
assert_eq!(t.cursor(), grapheme.len() * 2);
|
||||
assert_eq!(t.cursor(), grapheme.len());
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
//! Up/Down across soft-wrapped rows: a soft row's exclusive end is the next row's start.
|
||||
|
||||
use rand::prelude::*;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::text::Line;
|
||||
|
||||
use super::{ElementKind, TextArea};
|
||||
|
||||
fn ta_with(text: &str) -> TextArea {
|
||||
let mut t = TextArea::new();
|
||||
t.insert_str(text);
|
||||
t
|
||||
}
|
||||
|
||||
fn cursor_row(t: &TextArea, width: u16) -> usize {
|
||||
TextArea::wrapped_line_index_by_start(&t.wrapped_lines(width), t.cursor()).unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn up_from_wide_row_lands_on_last_char_of_narrower_soft_row() {
|
||||
let width = 8;
|
||||
let mut t = ta_with("aaa bb ccccccc");
|
||||
assert_eq!(vec![0..7, 7..14], *t.wrapped_lines(width));
|
||||
|
||||
t.move_cursor_up();
|
||||
assert_eq!(6, t.cursor());
|
||||
assert_eq!(0, cursor_row(&t, width));
|
||||
assert_eq!(Some((6, 0)), t.cursor_pos(Rect::new(0, 0, width, 5)));
|
||||
|
||||
t.move_cursor_down();
|
||||
assert_eq!(14, t.cursor());
|
||||
|
||||
t.move_cursor_up();
|
||||
assert_eq!(6, t.cursor());
|
||||
t.move_cursor_up();
|
||||
assert_eq!(0, t.cursor());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn up_onto_newline_terminated_row_lands_on_the_newline() {
|
||||
let mut t = ta_with("ab\ncdef");
|
||||
assert_eq!(vec![0..2, 3..7], *t.wrapped_lines(8));
|
||||
|
||||
t.move_cursor_up();
|
||||
assert_eq!(2, t.cursor());
|
||||
assert_eq!(0, cursor_row(&t, 8));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn down_from_wide_row_lands_on_last_char_of_narrower_soft_row() {
|
||||
let mut t = ta_with("ccccccc aaa bb ddddddd");
|
||||
assert_eq!(vec![0..8, 8..15, 15..22], *t.wrapped_lines(8));
|
||||
t.set_cursor(7);
|
||||
|
||||
t.move_cursor_down();
|
||||
assert_eq!(14, t.cursor());
|
||||
assert_eq!(1, cursor_row(&t, 8));
|
||||
|
||||
t.move_cursor_down();
|
||||
assert_eq!(22, t.cursor());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn up_onto_soft_row_closed_by_element_lands_before_the_element() {
|
||||
let mut t = TextArea::new();
|
||||
t.insert_element("xyz", ElementKind(0), Some(Line::from("[ELEM]")));
|
||||
t.insert_str("longword");
|
||||
assert_eq!(vec![0..3, 3..11], *t.wrapped_lines(8));
|
||||
|
||||
t.move_cursor_up();
|
||||
assert_eq!(0, t.cursor());
|
||||
assert_eq!(0, cursor_row(&t, 8));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn down_through_element_spanning_rows_reaches_the_end() {
|
||||
let mut t = TextArea::new();
|
||||
t.insert_element("abcdefgh", ElementKind(0), None);
|
||||
t.insert_str("z");
|
||||
assert_eq!(vec![0..3, 3..6, 6..9], *t.wrapped_lines(3));
|
||||
t.set_cursor(0);
|
||||
|
||||
t.move_cursor_down();
|
||||
assert_eq!(8, t.cursor());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn up_and_down_move_one_row_at_a_time() {
|
||||
let mut rng = StdRng::seed_from_u64(7);
|
||||
for _ in 0..500 {
|
||||
let words = rng.random_range(1..12);
|
||||
let text = (0..words)
|
||||
.map(|_| "x".repeat(rng.random_range(1..9)))
|
||||
.collect::<Vec<_>>()
|
||||
.join(" ");
|
||||
let width = rng.random_range(3..12);
|
||||
let mut t = ta_with(&text);
|
||||
let rows = t.wrapped_lines(width).len();
|
||||
t.set_cursor(rng.random_range(0..=text.len()));
|
||||
|
||||
let mut row = cursor_row(&t, width);
|
||||
while row + 1 < rows {
|
||||
t.move_cursor_down();
|
||||
assert_eq!(
|
||||
row + 1,
|
||||
cursor_row(&t, width),
|
||||
"down in {text:?} at width {width}"
|
||||
);
|
||||
row += 1;
|
||||
}
|
||||
while row > 0 {
|
||||
t.move_cursor_up();
|
||||
assert_eq!(
|
||||
row - 1,
|
||||
cursor_row(&t, width),
|
||||
"up in {text:?} at width {width}"
|
||||
);
|
||||
row -= 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user