diff --git a/crates/fresh-core/src/diff.rs b/crates/fresh-core/src/diff.rs index cf84abd403..38ebe461d4 100644 --- a/crates/fresh-core/src/diff.rs +++ b/crates/fresh-core/src/diff.rs @@ -53,6 +53,92 @@ pub fn compute_line_diff(old_text: &str, new_text: &str) -> Vec { diff_interned_lines(&old_ids, &new_ids) } +/// Map a byte offset in `old_text` to the corresponding byte offset in +/// `new_text`, so a cursor keeps pointing at the same logical text after +/// the buffer is rewritten (e.g. by format-on-save — issue #2777). +/// +/// Semantics (VS Code-like): +/// - An offset before/after every changed region shifts by the byte delta +/// of the preceding hunks, staying anchored to its text. +/// - An offset inside a changed region is refined by the region's common +/// prefix/suffix (so reindenting a line keeps the cursor at its column); +/// if its own text was rewritten it snaps to the end of that common +/// prefix — the start of the replacement. +/// +/// Cost is one line-level patience diff (`compute_line_diff`, which trims +/// the common prefix/suffix first) plus one O(lines) offset table per +/// side — no quadratic work on large files. +/// +/// The returned offset is always a char boundary in `new_text`, provided +/// `offset` is a char boundary in `old_text`. +pub fn map_offset_through_diff(old_text: &str, new_text: &str, offset: usize) -> usize { + let offset = offset.min(old_text.len()); + let old_starts = line_start_offsets(old_text); + let new_starts = line_start_offsets(new_text); + + let mut delta: isize = 0; + for h in compute_line_diff(old_text, new_text) { + let old_start = old_starts[h.old_start as usize]; + if offset < old_start { + // All remaining hunks are after the offset. + break; + } + let old_end = old_starts[(h.old_start + h.old_count) as usize]; + let new_start = new_starts[h.new_start as usize]; + let new_end = new_starts[(h.new_start + h.new_count) as usize]; + if offset < old_end { + return new_start + + map_offset_within_replacement( + &old_text[old_start..old_end], + &new_text[new_start..new_end], + offset - old_start, + ); + } + delta += (new_end - new_start) as isize - (old_end - old_start) as isize; + } + ((offset as isize + delta).max(0) as usize).min(new_text.len()) +} + +/// Byte offset of each line token per [`compute_line_diff`]'s +/// `split_inclusive('\n')` tokenization, with a trailing sentinel at +/// `text.len()` so `starts[i]..starts[i + count]` is a hunk's byte range. +fn line_start_offsets(text: &str) -> Vec { + let mut starts = vec![0]; + for line in text.split_inclusive('\n') { + starts.push(starts.last().unwrap() + line.len()); + } + starts +} + +/// Map char-boundary offset `rel` in `old` to an offset in `new`, where +/// `new` replaced `old` wholesale: keep it if it sits in the common +/// prefix, mirror it from the end if it sits in the (non-overlapping) +/// common suffix, else snap to the end of the common prefix. Prefix and +/// suffix are computed in whole chars, so the result is a char boundary. +fn map_offset_within_replacement(old: &str, new: &str, rel: usize) -> usize { + let mut prefix = 0; + for (a, b) in old.chars().zip(new.chars()) { + if a != b { + break; + } + prefix += a.len_utf8(); + } + if rel <= prefix { + return rel; + } + let mut suffix = 0; + for (a, b) in old[prefix..].chars().rev().zip(new[prefix..].chars().rev()) { + if a != b { + break; + } + suffix += a.len_utf8(); + } + if old.len() - rel <= suffix { + return new.len() - (old.len() - rel); + } + prefix +} + /// Interns lines to `u32` ids so every comparison in the diff is an /// integer compare, not a string compare. Callers that don't hold both /// sides as contiguous strings (e.g. a host service iterating two @@ -541,6 +627,101 @@ mod tests { ); } + // --- map_offset_through_diff (cursor mapping, issue #2777) --- + + /// The #2777 repro: a formatter deletes blank lines; an offset after + /// the deleted region must shift back and stay anchored to its text. + #[test] + fn map_offset_shifts_past_deleted_lines() { + let old = "alpha\n\n\n\nMARKER xyz\nomega\n"; + let new = "alpha\nMARKER xyz\nomega\n"; + // End of "MARKER xyz" (old line 5) -> end of "MARKER xyz" (new line 2). + assert_eq!(map_offset_through_diff(old, new, 19), 16); + // Start of "omega" tracks too. + assert_eq!(map_offset_through_diff(old, new, 20), 17); + } + + /// An offset entirely before every change must not move. + #[test] + fn map_offset_before_changes_is_identity() { + let old = "alpha\n\n\n\nMARKER xyz\nomega\n"; + let new = "alpha\nMARKER xyz\nomega\n"; + for offset in 0..=6 { + assert_eq!(map_offset_through_diff(old, new, offset), offset); + } + } + + /// Identical texts (formatter no-op): identity for every offset. + #[test] + fn map_offset_identity_on_equal_texts() { + let text = "a\nb\nc\n"; + for offset in 0..=text.len() { + assert_eq!(map_offset_through_diff(text, text, offset), offset); + } + } + + /// An offset inside a deleted region snaps to the start of the + /// replacement (VS Code semantics), not into unrelated text. + #[test] + fn map_offset_inside_deleted_region_snaps_to_replacement_start() { + let old = "alpha\n\n\n\nMARKER xyz\nomega\n"; + let new = "alpha\nMARKER xyz\nomega\n"; + // Offsets 6..9 are the three deleted blank lines. + for offset in 6..9 { + assert_eq!(map_offset_through_diff(old, new, offset), 6); + } + } + + /// The #2706 repro: reindentation rewrites the cursor's own line; the + /// common-suffix refinement keeps the cursor at the end of its text. + #[test] + fn map_offset_tracks_reindented_line_via_common_suffix() { + let old = "fn main() {\nlet a = 1;\nlet b = 2;\n}\n"; + let new = "fn main() {\n let a = 1;\n let b = 2;\n}\n"; + // End of "let b = 2;" (after the ';'). + let old_pos = old.find("2;").unwrap() + 2; + let new_pos = new.find("2;").unwrap() + 2; + assert_eq!(map_offset_through_diff(old, new, old_pos), new_pos); + } + + /// An offset between two hunks shifts only by the earlier hunk's + /// delta — it must not snap to either hunk. + #[test] + fn map_offset_between_hunks_shifts_by_earlier_delta_only() { + let old = "one\ntwo\nmiddle\nthree\nfour\n"; + let new = "ONE CHANGED\ntwo\nmiddle\nthree\nFOUR CHANGED\n"; + // Start of "middle": line 1 ("two\n") onward is common; the first + // hunk replaced "one\n" (4 bytes) with "ONE CHANGED\n" (12 bytes). + let old_pos = old.find("middle").unwrap(); + let new_pos = new.find("middle").unwrap(); + assert_eq!(map_offset_through_diff(old, new, old_pos), new_pos); + } + + /// Out-of-range input clamps to the new text's length. + #[test] + fn map_offset_clamps_to_new_len() { + assert_eq!(map_offset_through_diff("abc\ndef\n", "abc\n", 8), 4); + assert_eq!(map_offset_through_diff("abc\n", "abc\n", 100), 4); + } + + /// Multibyte content: results stay on char boundaries. + #[test] + fn map_offset_multibyte_stays_on_char_boundaries() { + let old = "héllo wörld\n\n\ntail é\n"; + let new = "héllo wörld\ntail é\n"; + // End of "tail é" tracks through the blank-line deletion. + let old_pos = old.len() - 1; // before final '\n' + let mapped = map_offset_through_diff(old, new, old_pos); + assert_eq!(mapped, new.len() - 1); + assert!(new.is_char_boundary(mapped)); + // An offset inside the rewritten region also lands on a boundary. + for offset in 0..=old.len() { + if old.is_char_boundary(offset) { + assert!(new.is_char_boundary(map_offset_through_diff(old, new, offset))); + } + } + } + /// Deterministic pseudo-random edit fuzzing: every generated pair /// must satisfy the reconstruction invariant. #[test] diff --git a/crates/fresh-editor/src/app/on_save_actions.rs b/crates/fresh-editor/src/app/on_save_actions.rs index 235f27610a..518e488bc2 100644 --- a/crates/fresh-editor/src/app/on_save_actions.rs +++ b/crates/fresh-editor/src/app/on_save_actions.rs @@ -513,6 +513,15 @@ impl Editor { let old_anchor = self.active_cursors().primary().anchor; let old_sticky_column = self.active_cursors().primary().sticky_column; + // Map the cursor (and any selection anchor) through a diff of the + // old vs. new content so each stays anchored to the same logical + // text — a raw byte offset lands in unrelated text whenever the + // rewrite changes lengths before it (issue #2777). + let new_cursor_pos = + fresh_core::diff::map_offset_through_diff(&buffer_content, output, old_cursor_pos); + let new_anchor = old_anchor + .map(|a| fresh_core::diff::map_offset_through_diff(&buffer_content, output, a)); + // Delete all content and insert new let delete_event = Event::Delete { range: 0..buffer_len, @@ -525,10 +534,9 @@ impl Editor { cursor_id, }; - // After delete+insert, cursor will be at output.len() - // Restore cursor to original position (or clamp to new buffer length) + // After delete+insert, the cursor sits at output.len(); a trailing + // MoveCursor restores it to the diff-mapped position. let new_buffer_len = output.len(); - let new_cursor_pos = old_cursor_pos.min(new_buffer_len); // Leading cursor-restore event. Applied forward this is a no-op (the // cursor is already at `old_cursor_pos`), but a `Batch` is undone by @@ -548,15 +556,16 @@ impl Editor { new_sticky_column: old_sticky_column, }; - // Only add MoveCursor event if position actually changes + // Only add MoveCursor event if there is something to restore (the + // delete+insert left the cursor at the buffer end with no selection) let mut events = vec![restore_cursor_event, delete_event, insert_event]; - if new_cursor_pos != new_buffer_len { + if new_cursor_pos != new_buffer_len || new_anchor.is_some() { let move_cursor_event = Event::MoveCursor { cursor_id, old_position: new_buffer_len, // Where cursor is after insert new_position: new_cursor_pos, old_anchor: None, - new_anchor: old_anchor.map(|a| a.min(new_buffer_len)), + new_anchor, old_sticky_column: None, new_sticky_column: old_sticky_column, }; diff --git a/crates/fresh-editor/tests/e2e/on_save_actions.rs b/crates/fresh-editor/tests/e2e/on_save_actions.rs index 832405ce01..f3e6b3fb4a 100644 --- a/crates/fresh-editor/tests/e2e/on_save_actions.rs +++ b/crates/fresh-editor/tests/e2e/on_save_actions.rs @@ -799,6 +799,154 @@ fn test_trim_on_save_preserves_crlf_and_no_revert() { ); } +/// Reproducer for #2777: when format-on-save rewrites the buffer, the +/// cursor must be mapped through the content change and stay anchored to +/// the same logical text — not restored as a raw byte offset into +/// different content. +/// +/// Repro from the issue: a formatter that deletes blank lines +/// (`grep -v '^$'`), cursor at the end of line 5 (`MARKER xyz`, +/// `Ln 5, Col 11`). After save, `MARKER xyz` becomes line 2 and the +/// cursor must follow it to `Ln 2, Col 11`. Before the fix the raw +/// offset landed inside `omega` (`Ln 3, Col 3`). +#[test] +#[cfg_attr(not(unix), ignore = "On-save actions require Unix-like environment")] +fn test_format_on_save_keeps_cursor_anchored_to_its_text() { + let temp_dir = TempDir::new().unwrap(); + let project_dir = temp_dir.path().join("project"); + std::fs::create_dir(&project_dir).unwrap(); + + let file_path = project_dir.join("marker.txt"); + std::fs::write(&file_path, "alpha\n\n\n\nMARKER xyz\nomega\n").unwrap(); + + // Formatter that deletes blank lines (stdin -> stdout). + let formatter = FormatterConfig { + command: "grep".to_string(), + args: vec!["-v".to_string(), "'^$'".to_string()], + stdin: true, + timeout_ms: 5000, + }; + + let mut config = Config::default(); + config.languages.insert( + "plaintext".to_string(), + LanguageConfig { + extensions: vec!["txt".to_string()], + filenames: vec![], + grammar: "plaintext".to_string(), + comment_prefix: None, + auto_indent: false, + auto_close: None, + auto_surround: None, + textmate_grammar: None, + show_whitespace_tabs: true, + line_wrap: None, + wrap_column: None, + page_view: None, + page_width: None, + use_tabs: None, + tab_size: None, + formatter: Some(formatter), + format_on_save: true, + on_save: vec![], + word_characters: None, + indentation_guide: None, + indent: None, + }, + ); + + let mut harness = + EditorTestHarness::with_config_and_working_dir(100, 24, config, project_dir).unwrap(); + + harness.open_file(&file_path).unwrap(); + harness.render().unwrap(); + + // Put the cursor at the end of line 5 ("MARKER xyz"). + for _ in 0..4 { + harness.send_key(KeyCode::Down, KeyModifiers::NONE).unwrap(); + } + harness.send_key(KeyCode::End, KeyModifiers::NONE).unwrap(); + harness.render().unwrap(); + harness.assert_screen_contains("Ln 5, Col 11"); + + // Save: the formatter removes the three blank lines. + harness + .send_key(KeyCode::Char('s'), KeyModifiers::CONTROL) + .unwrap(); + harness.render().unwrap(); + harness.assert_buffer_content("alpha\nMARKER xyz\nomega\n"); + + // The cursor must still sit at the end of "MARKER xyz", now line 2. + // Before the fix it read "Ln 3, Col 3" (inside "omega"). + harness.assert_screen_contains("Ln 2, Col 11"); +} + +/// Companion to the #2777 reproducer: a cursor entirely before the +/// formatted-away region must not move at all. +#[test] +#[cfg_attr(not(unix), ignore = "On-save actions require Unix-like environment")] +fn test_format_on_save_cursor_before_edit_stays_put() { + let temp_dir = TempDir::new().unwrap(); + let project_dir = temp_dir.path().join("project"); + std::fs::create_dir(&project_dir).unwrap(); + + let file_path = project_dir.join("marker.txt"); + std::fs::write(&file_path, "alpha\n\n\n\nMARKER xyz\nomega\n").unwrap(); + + let formatter = FormatterConfig { + command: "grep".to_string(), + args: vec!["-v".to_string(), "'^$'".to_string()], + stdin: true, + timeout_ms: 5000, + }; + + let mut config = Config::default(); + config.languages.insert( + "plaintext".to_string(), + LanguageConfig { + extensions: vec!["txt".to_string()], + filenames: vec![], + grammar: "plaintext".to_string(), + comment_prefix: None, + auto_indent: false, + auto_close: None, + auto_surround: None, + textmate_grammar: None, + show_whitespace_tabs: true, + line_wrap: None, + wrap_column: None, + page_view: None, + page_width: None, + use_tabs: None, + tab_size: None, + formatter: Some(formatter), + format_on_save: true, + on_save: vec![], + word_characters: None, + indentation_guide: None, + indent: None, + }, + ); + + let mut harness = + EditorTestHarness::with_config_and_working_dir(100, 24, config, project_dir).unwrap(); + + harness.open_file(&file_path).unwrap(); + harness.render().unwrap(); + + // End of line 1 ("alpha"), before every removed blank line. + harness.send_key(KeyCode::End, KeyModifiers::NONE).unwrap(); + harness.render().unwrap(); + harness.assert_screen_contains("Ln 1, Col 6"); + + harness + .send_key(KeyCode::Char('s'), KeyModifiers::CONTROL) + .unwrap(); + harness.render().unwrap(); + harness.assert_buffer_content("alpha\nMARKER xyz\nomega\n"); + harness.assert_screen_contains("Ln 1, Col 6"); +} + /// Test whitespace cleanup does nothing when file is already clean #[test] fn test_whitespace_cleanup_no_change_needed() {