Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions crates/fresh-editor/src/model/buffer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
84 changes: 40 additions & 44 deletions crates/fresh-editor/src/primitives/grapheme.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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.
Expand Down
54 changes: 23 additions & 31 deletions crates/fresh-editor/src/view/folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,41 +396,42 @@ 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`
/// (i.e. one byte past its terminating `\n`, or the buffer length if the
/// 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
}
Expand Down Expand Up @@ -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`.
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}