diff --git a/crates/fresh-editor/src/model/buffer/mod.rs b/crates/fresh-editor/src/model/buffer/mod.rs index 82e6de63be..2f76912a48 100644 --- a/crates/fresh-editor/src/model/buffer/mod.rs +++ b/crates/fresh-editor/src/model/buffer/mod.rs @@ -1717,6 +1717,13 @@ impl TextBuffer { line_feed_cnt, &self.buffers, ); + + // Content changed: layout caches (line-wrap cache, visual-row + // index) key on the version and would otherwise serve + // pre-append state. Deliberately not `mark_content_modified` + // — the buffer still matches the on-disk stream, so it isn't + // dirty. + self.bump_version(); } /// Check if the buffer has been modified since last save diff --git a/crates/fresh-editor/src/primitives/grapheme.rs b/crates/fresh-editor/src/primitives/grapheme.rs index d90a31b211..bdb72f8953 100644 --- a/crates/fresh-editor/src/primitives/grapheme.rs +++ b/crates/fresh-editor/src/primitives/grapheme.rs @@ -9,7 +9,20 @@ //! A grapheme cluster is what a user perceives as a single character. //! For example, Thai "ที่" looks like one character but is 3 Unicode code points. -use unicode_segmentation::UnicodeSegmentation; +use unicode_segmentation::{GraphemeCursor, UnicodeSegmentation}; + +/// Snap `pos` down to a UTF-8 code-point boundary so it can be handed to +/// [`GraphemeCursor`], which requires boundary-aligned offsets. A position +/// inside a code point can never be a grapheme boundary, so snapping down +/// preserves the semantics of every caller below. +#[inline] +fn snap_to_char_boundary(s: &str, mut pos: usize) -> usize { + pos = pos.min(s.len()); + while pos > 0 && !s.is_char_boundary(pos) { + pos -= 1; + } + pos +} /// Find the byte position of the previous grapheme cluster boundary. /// @@ -28,18 +41,19 @@ pub fn prev_grapheme_boundary(s: &str, pos: usize) -> usize { return 0; } - let pos = pos.min(s.len()); - - // Find all grapheme boundaries up to our position - let mut last_boundary = 0; - for (idx, _) in s.grapheme_indices(true) { - if idx >= pos { - break; - } - last_boundary = idx; + // A position INSIDE a code point is inside that code point's + // cluster: the boundary strictly before it is the containing + // cluster's START, which the snapped char boundary may already be + // (single-code-point cluster). Stepping `prev_boundary` from the + // snapped position instead would overshoot one cluster left. + let snapped = snap_to_char_boundary(s, pos); + if snapped < pos { + return snap_to_grapheme_boundary(s, snapped); } - - last_boundary + let mut cursor = GraphemeCursor::new(snapped, s.len(), true); + // With the whole string as a single chunk (chunk_start == 0) the cursor + // never needs pre-context, so this cannot fail. + cursor.prev_boundary(s, 0).ok().flatten().unwrap_or(0) } /// Find the byte position of the next grapheme cluster boundary. @@ -59,20 +73,11 @@ pub fn next_grapheme_boundary(s: &str, pos: usize) -> usize { return s.len(); } - // Find the grapheme that contains our position, then return its end - for (idx, grapheme) in s.grapheme_indices(true) { - let end = idx + grapheme.len(); - if idx >= pos { - // This grapheme starts at or after our position - return end; - } - if end > pos { - // Our position is within this grapheme - return end; - } - } - - s.len() + // A mid-cluster position (snapped or not) advances to the end of the + // containing cluster — the next boundary after it. + let pos = snap_to_char_boundary(s, pos); + let mut cursor = GraphemeCursor::new(pos, s.len(), true); + cursor.next_boundary(s, 0).ok().flatten().unwrap_or(s.len()) } /// Get the grapheme cluster at the given position. @@ -87,14 +92,9 @@ pub fn grapheme_at(s: &str, pos: usize) -> Option<(&str, usize, usize)> { return None; } - for (idx, grapheme) in s.grapheme_indices(true) { - let end = idx + grapheme.len(); - if idx <= pos && pos < end { - return Some((grapheme, idx, end)); - } - } - - None + let start = snap_to_grapheme_boundary(s, pos); + let end = next_grapheme_boundary(s, start); + Some((&s[start..end], start, end)) } /// Snap a byte position **down** to the nearest grapheme-cluster @@ -120,17 +120,13 @@ pub fn snap_to_grapheme_boundary(s: &str, pos: usize) -> usize { if pos >= s.len() { return s.len(); } - let mut last_boundary = 0; - for (idx, _) in s.grapheme_indices(true) { - if idx == pos { - return pos; - } - if idx > pos { - break; - } - last_boundary = idx; + let pos = snap_to_char_boundary(s, pos); + let mut cursor = GraphemeCursor::new(pos, s.len(), true); + match cursor.is_boundary(s, 0) { + Ok(true) => pos, + // Not a boundary → start of the containing cluster. + _ => cursor.prev_boundary(s, 0).ok().flatten().unwrap_or(0), } - last_boundary } /// Count the number of grapheme clusters in a string. diff --git a/crates/fresh-editor/src/view/folding.rs b/crates/fresh-editor/src/view/folding.rs index 9d0fbe49a5..da38abe348 100644 --- a/crates/fresh-editor/src/view/folding.rs +++ b/crates/fresh-editor/src/view/folding.rs @@ -396,27 +396,27 @@ impl Default for FoldManager { /// ([`PatternIndentCalculator::count_leading_indent`]). pub mod indent_folding { use crate::model::buffer::Buffer; - use crate::primitives::indent_pattern::PatternIndentCalculator; + + /// Chunk size for the line-boundary scans below. Scanning via + /// per-byte `byte_at` calls costs a full `slice_bytes` round-trip + /// per byte, which is what made cursor movement inside a very long + /// line slow (these scans run per rendered frame); block reads make + /// the scans effectively memchr-speed. + const LINE_SCAN_CHUNK: usize = 4096; /// Find the byte offset of the start of the line containing `pos`. /// Scans backward for `\n` (or returns 0). pub fn find_line_start_byte(buffer: &Buffer, pos: usize) -> usize { - if pos == 0 { - return 0; - } - let mut p = pos.min(buffer.len()).saturating_sub(1); - loop { - match PatternIndentCalculator::byte_at(buffer, p) { - Some(b'\n') => return p + 1, - None => return 0, - _ => { - if p == 0 { - return 0; - } - p -= 1; - } + let mut end = pos.min(buffer.len()); + while end > 0 { + let start = end.saturating_sub(LINE_SCAN_CHUNK); + let bytes = buffer.slice_bytes(start..end); + if let Some(i) = bytes.iter().rposition(|&b| b == b'\n') { + return start + i + 1; } + end = start; } + 0 } /// Find the exclusive byte offset just past the line containing `pos` @@ -424,13 +424,14 @@ pub mod indent_folding { /// line has no trailing newline). Scans forward for `\n`. pub fn find_line_end_byte(buffer: &Buffer, pos: usize) -> usize { let buf_len = buffer.len(); - let mut p = pos; - while p < buf_len { - match PatternIndentCalculator::byte_at(buffer, p) { - Some(b'\n') => return p + 1, - None => return buf_len, - _ => p += 1, + let mut start = pos.min(buf_len); + while start < buf_len { + let end = (start + LINE_SCAN_CHUNK).min(buf_len); + let bytes = buffer.slice_bytes(start..end); + if let Some(i) = bytes.iter().position(|&b| b == b'\n') { + return start + i + 1; } + start = end; } buf_len } @@ -582,16 +583,7 @@ pub mod indent_folding { /// Scans forward for `\n` and returns the byte after it. If no `\n` is /// found, returns `buffer.len()`. pub fn find_next_line_start_byte(buffer: &Buffer, pos: usize) -> usize { - let mut p = pos; - let len = buffer.len(); - while p < len { - match PatternIndentCalculator::byte_at(buffer, p) { - Some(b'\n') => return p + 1, - None => return len, - _ => p += 1, - } - } - len + find_line_end_byte(buffer, pos) } /// Byte-range of a fold that contains `target_byte`. diff --git a/crates/fresh-editor/tests/streaming_and_grapheme_regression_tests.rs b/crates/fresh-editor/tests/streaming_and_grapheme_regression_tests.rs new file mode 100644 index 0000000000..d2b0146816 --- /dev/null +++ b/crates/fresh-editor/tests/streaming_and_grapheme_regression_tests.rs @@ -0,0 +1,77 @@ +//! Regression tests for two fixes that came out of the wrapped-line +//! navigation performance review: +//! +//! * `extend_streaming` mutates buffer content (appends bytes) but did +//! not bump `buffer.version()`, so version-keyed layout caches +//! (visual-row index, line-wrap cache) could remain reachable with +//! pre-append state. +//! +//! * `prev_grapheme_boundary` stepped one cluster too far when given a +//! position INSIDE a multi-byte code point whose cluster is a single +//! code point (accented latin, CJK): the snapped char boundary IS the +//! containing cluster's start, and stepping `prev_boundary` from it +//! overshoots. + +mod common; + +use common::harness::EditorTestHarness; + +/// `extend_streaming` appends content and must bump `buffer.version()` +/// like every other mutation path — layout caches key on the version, +/// so an unchanged version can serve pre-append layout. +#[test] +fn extend_streaming_bumps_buffer_version() { + use std::io::Write; + + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("stream.txt"); + std::fs::write(&path, "hello\n").unwrap(); + + let mut harness = EditorTestHarness::new(80, 24).unwrap(); + harness.open_file(&path).unwrap(); + harness.render().unwrap(); + + let before = harness.editor().active_state().buffer.version(); + + // The stream grows on disk; the editor is told to extend. + let mut f = std::fs::OpenOptions::new() + .append(true) + .open(&path) + .unwrap(); + f.write_all(b"appended\n").unwrap(); + f.flush().unwrap(); + let new_size = std::fs::metadata(&path).unwrap().len() as usize; + harness + .editor_mut() + .active_state_mut() + .buffer + .extend_streaming(&path, new_size); + + let after = harness.editor().active_state().buffer.version(); + assert_ne!( + before, + after, + "extend_streaming appended {} bytes but buffer.version() did not change — \ + version-keyed layout caches will serve pre-append content", + new_size - 6, + ); +} + +/// For a position INSIDE a multi-byte code point, +/// `prev_grapheme_boundary` must return the start of the CONTAINING +/// cluster, not the boundary one cluster earlier. +#[test] +fn prev_grapheme_boundary_mid_code_point_returns_containing_cluster_start() { + use fresh::primitives::grapheme::prev_grapheme_boundary; + + // "aé": 'a' at 0, 'é' occupies bytes 1..3. Position 2 is inside + // 'é'; its containing cluster starts at 1. + assert_eq!(prev_grapheme_boundary("aé", 2), 1); + + // Sanity: a position on a char boundary INSIDE a multi-code-point + // cluster still resolves to the cluster start... + assert_eq!(prev_grapheme_boundary("ที่", 3), 0); + // ...and a position on a cluster boundary steps to the previous one. + assert_eq!(prev_grapheme_boundary("aé", 1), 0); + assert_eq!(prev_grapheme_boundary("aé", 3), 1); +}