fix(editor): keep the caret and view when a file changes in distant places (#1329)

Document::diff only trimmed the common prefix and suffix, so two distant
edits on disk became one change spanning everything between them. The
caret and scroll position inside that span were mapped to its start,
which made the editor jump to the first edit after an agent, formatter
or git operation rewrote the file.

Diff the changed middle line by line (Myers via similar, 50 ms deadline
with a coarser fallback) and emit one change per edited region, trimmed
to its differing characters. The frontend DocSync path produces the same
per-region changes, so external reloads, local history restores and
formatting keep the caret and view in place.

Closes #1326

Co-authored-by: ntbang0901 <[email protected]>
This commit is contained in:
ntbang0901 authored and GitHub committed 2026-11-05 09:04:48 +08:00
1 parent 01e09565b7
commit 02d30b1609
11 files changed
+229 -58

No files matched your search

Generated
+1
View File
@@ -4634,6 +4634,7 @@ dependencies = [
"proptest",
"ropey",
"serde",
"similar",
"specta",
]
+1
View File
@@ -12,6 +12,7 @@ ide-core.workspace = true
# Hence no `unicode_lines` / `cr_lines` (a test guards against feature unification).
ropey = { version = "1.6", default-features = false, features = ["simd"] }
serde.workspace = true
similar = { version = "2.7", default-features = false }
specta.workspace = true
[dev-dependencies]
+4
View File
@@ -73,6 +73,10 @@ fn bench(c: &mut Criterion) {
BatchSize::SmallInput,
)
});
let two_edits = text.replacen("value_100 ", "renamed_100 ", 1).replacen("value_9000 ", "renamed_9000 ", 1);
c.bench_function("text/diff_two_edits_1mb", |b| b.iter(|| base.diff(black_box(&two_edits))));
let every_line = text.replace("compute", "calculate");
c.bench_function("text/diff_every_line_1mb", |b| b.iter(|| base.diff(black_box(&every_line))));
c.bench_function("text/offset_to_pos_round_trip", |b| {
let mut rng = Lcg(3);
let len = base.len_utf16();
+124 -31
View File
@@ -14,6 +14,7 @@ use ide_core::{ChangeSet, IdeError, IdeResult, Pos, Range, TextChange, TextEdit}
use serde::{Deserialize, Serialize};
use specta::Type;
use std::borrow::Cow;
use std::time::{Duration, Instant};
pub use ropey;
@@ -66,6 +67,36 @@ impl LineEnding {
}
}
/// How long [`Document::diff`] may look for separate regions, so a reload never stalls on a huge file.
const DIFF_BUDGET: Duration = Duration::from_millis(50);
type LineRange = std::ops::Range<usize>;
/// Byte lengths of the common prefix and suffix; they never overlap.
fn common_ends(a: &str, b: &str) -> (usize, usize) {
let shared = |a: &mut dyn Iterator<Item = char>, b: &mut dyn Iterator<Item = char>| -> usize {
a.zip(b).take_while(|(x, y)| x == y).map(|(x, _)| x.len_utf8()).sum()
};
let prefix = shared(&mut a.chars(), &mut b.chars());
let suffix = shared(&mut a[prefix..].chars().rev(), &mut b[prefix..].chars().rev());
(prefix, suffix)
}
/// Byte offset of each line, then the total length.
fn line_starts(lines: &[&str]) -> Vec<usize> {
let mut at = 0;
let mut starts = vec![0];
starts.extend(lines.iter().map(|line| {
at += line.len();
at
}));
starts
}
fn utf16_len(text: &str) -> u32 {
text.chars().map(|c| c.len_utf16() as u32).sum()
}
/// Converts `\r\n` and lone `\r` to `\n`. Borrows when there is nothing to do.
pub fn normalize_line_endings(text: &str) -> Cow<'_, str> {
if !text.contains('\r') {
@@ -334,42 +365,63 @@ impl Document {
cs
}
/// The minimal single-change (common prefix/suffix) change set turning the
/// current text into `text` (after line-ending normalization). Empty when equal.
/// The change set turning the current text into `text` (after line-ending
/// normalization), one change per changed region. Empty when equal. A single change
/// spanning two distant edits would drag the editor's caret and view to the first one.
pub fn diff(&self, text: &str) -> ChangeSet {
self.diff_until(text, Instant::now() + DIFF_BUDGET)
}
/// Past `deadline` the regions get coarser, never wrong.
fn diff_until(&self, text: &str, deadline: Instant) -> ChangeSet {
let new = normalize_line_endings(text);
let old_chars = self.rope.len_chars();
let old = self.rope.to_string();
let (prefix, suffix) = common_ends(&old, &new);
let old_rest = &old[prefix..old.len() - suffix];
let new_rest = &new[prefix..new.len() - suffix];
let mut prefix_chars = 0usize;
let mut prefix_bytes = 0usize;
for (a, b) in self.rope.chars().zip(new.chars()) {
if a != b {
break;
let old_lines: Vec<&str> = old_rest.split_inclusive('\n').collect();
let new_lines: Vec<&str> = new_rest.split_inclusive('\n').collect();
let old_starts = line_starts(&old_lines);
let new_starts = line_starts(&new_lines);
let mut changes = Vec::new();
let mut utf16_at = utf16_len(&old[..prefix]);
let mut old_at = 0;
let mut region: Option<(LineRange, LineRange)> = None;
let ops =
similar::capture_diff_slices_deadline(similar::Algorithm::Myers, &old_lines, &new_lines, Some(deadline));
let mut flush = |region: Option<(LineRange, LineRange)>| {
let Some((old_range, new_range)) = region else { return };
let region_old = &old_rest[old_starts[old_range.start]..old_starts[old_range.end]];
let region_new = &new_rest[new_starts[new_range.start]..new_starts[new_range.end]];
let (head, tail) = common_ends(region_old, region_new);
let start = old_starts[old_range.start] + head;
let end = old_starts[old_range.end] - tail;
let from = utf16_at + utf16_len(&old_rest[old_at..start]);
let to = from + utf16_len(&old_rest[start..end]);
changes.push(TextChange { from, to, insert: region_new[head..region_new.len() - tail].to_owned() });
utf16_at = to;
old_at = end;
};
// Positions come from the lengths: `similar` can report a stale start for the empty
// side of an op (an insert after an equal run still names the run's first old line).
let (mut old_line, mut new_line) = (0, 0);
for op in ops {
let old_end = old_line + op.old_range().len();
let new_end = new_line + op.new_range().len();
if op.tag() == similar::DiffTag::Equal {
flush(region.take());
} else {
region = Some(match region.take() {
Some((o, n)) => (o.start..old_end, n.start..new_end),
None => (old_line..old_end, new_line..new_end),
});
}
prefix_chars += 1;
prefix_bytes += b.len_utf8();
(old_line, new_line) = (old_end, new_end);
}
let new_rest = &new[prefix_bytes..];
let new_rest_chars = new_rest.chars().count();
if prefix_chars == old_chars && new_rest_chars == 0 {
return ChangeSet::default();
}
let max_suffix = (old_chars - prefix_chars).min(new_rest_chars);
let mut suffix_chars = 0usize;
let mut suffix_bytes = 0usize;
for (a, b) in self.rope.chars_at(old_chars).reversed().zip(new_rest.chars().rev()).take(max_suffix) {
if a != b {
break;
}
suffix_chars += 1;
suffix_bytes += b.len_utf8();
}
let from = self.rope.char_to_utf16_cu(prefix_chars) as u32;
let to = self.rope.char_to_utf16_cu(old_chars - suffix_chars) as u32;
let insert = new_rest[..new_rest.len() - suffix_bytes].to_owned();
ChangeSet { changes: vec![TextChange { from, to, insert }] }
flush(region);
ChangeSet { changes }
}
// ---- coordinate conversion (UTF-16 offset ⇄ Pos ⇄ byte offset) ----
@@ -606,4 +658,45 @@ mod tests {
assert_eq!(set.changes, vec![tc(2, 2, "x")]);
assert_eq!(doc.text(), "😀x😀");
}
fn numbered_lines(n: usize) -> String {
(0..n).map(|i| format!("line {i}\n")).collect()
}
#[test]
fn diff_keeps_distant_edits_apart() {
let old = numbered_lines(400);
let new = old.replace("line 20\n", "line 20x\n").replace("line 299\n", "line 299x\n");
let at = |line: &str| (old.find(line).unwrap() + line.len() - 1) as u32;
let set = Document::new(&old).diff(&new);
assert_eq!(
set.changes,
vec![tc(at("line 20\n"), at("line 20\n"), "x"), tc(at("line 299\n"), at("line 299\n"), "x")]
);
}
#[test]
fn diff_handles_added_and_removed_lines() {
let old = "a\nb\nc\nd\ne\nf\n";
let set = Document::new(old).diff("a\nnew\nb\nc\ne\nf\n");
assert_eq!(set.changes, vec![tc(2, 2, "new\n"), tc(5, 7, "")]);
}
#[test]
fn diff_counts_utf16_past_astral_text() {
let old = "😀 one\nsame\n😀 two\n";
let set = Document::new(old).diff("😀 ONE\nsame\n😀 TWO\n");
assert_eq!(set.changes, vec![tc(3, 6, "ONE"), tc(15, 18, "TWO")]);
}
#[test]
fn diff_past_its_deadline_is_coarser_but_still_correct() {
let old = numbered_lines(5000);
let new = old.replace("line 20\n", "line 20x\n").replace("line 4000\n", "line 4000x\n");
let doc = Document::new(&old);
let set = doc.diff_until(&new, std::time::Instant::now());
let mut replayed = Document::new(&old);
replayed.apply(0, &set).unwrap();
assert_eq!(replayed.text(), new);
}
}
@@ -5,3 +5,4 @@
# It is recommended to check this file in to source control so that
# everyone who runs the test benefits from these saved cases.
cc afa0785335731ca8adc050b09312843aeb3ff05dc895963d09d9f728c553717c # shrinks to doc_text = "a", picks = [(0, 0, ""), (0, 0, ""), (658842129343018477, 1922606258417419361, ""), (0, 4297423518677873183, ""), (953525717999286983, 557348908835472853, "a"), (0, 0, "b"), (0, 1653396554766652711, "")], seed = 7072935092038531240
cc dd6ec32be77e8ad96d0ce3ef14ad9d72d5ba2a06f227ea693bfdee62debf08b7 # shrinks to a = "中é\n\n", b = "\n\na"
+1 -1
View File
@@ -176,7 +176,7 @@ proptest! {
let set = doc.replace_all(&b);
prop_assert_eq!(doc.text(), b.clone());
prop_assert_eq!(naive_apply(&a, &set), b.clone());
prop_assert!(set.changes.len() <= 1);
prop_assert!(set.changes.windows(2).all(|w| w[0].to < w[1].from));
prop_assert_eq!(set.changes.is_empty(), a == b);
}
+22
View File
@@ -344,6 +344,28 @@ fn backend_edits_emit_patches() {
assert_eq!(fs::read_to_string(&path).unwrap(), "ONE\r\n2\r\nTHREE\r\n");
}
#[test]
fn external_change_in_two_places_patches_each() {
let f = fixture();
let path = f.root.join("two.txt");
let old: String = (0..400).map(|i| format!("line {i}\n")).collect();
fs::write(&path, &old).unwrap();
let id = f.docs.open(&path).unwrap().id;
std::thread::sleep(std::time::Duration::from_millis(10));
let new = old.replace("line 20\n", "line 20x\n").replace("line 299\n", "line 299x\n");
fs::write(&path, &new).unwrap();
f.docs.on_disk_changed(std::slice::from_ref(&path));
assert_eq!(f.docs.snapshot(id).unwrap().text, new);
let events = f.events.lock();
let changes = events.iter().find_map(|e| match e {
IdeEvent::DocPatched { changes, .. } => Some(changes),
_ => None,
});
assert_eq!(changes.map(Vec::len), Some(2));
}
#[test]
fn external_changes() {
let f = fixture();
+2
View File
@@ -62,6 +62,8 @@
"text/open_1mb": 2000,
"text/apply_keystroke_1mb": 20,
"text/apply_100_changes_1mb": 1000,
"text/diff_two_edits_1mb": 10000,
"text/diff_every_line_1mb": 100000,
"text/offset_to_pos_round_trip": 5,
"text/pos_to_byte": 5,
"text/text_for_save_crlf_1mb": 1000,
+61 -5
View File
@@ -2,7 +2,7 @@ import { describe, expect, it, vi } from "vitest";
import { ChangeSet, Text } from "@codemirror/state";
import type { TextChange } from "@/platform/ipc";
import { DocSync, toTextChanges } from "./DocSync";
import { minimalChange } from "./diff";
import { minimalChanges } from "./diff";
function apply(text: string, changes: TextChange[]) {
let out = "";
@@ -14,6 +14,10 @@ function apply(text: string, changes: TextChange[]) {
return out + text.slice(pos);
}
function numberedLines(count: number) {
return Array.from({ length: count }, (_, i) => `line ${i}\n`).join("");
}
class OutOfSync extends Error {
kind = "outOfSync";
}
@@ -226,6 +230,46 @@ describe("DocSync", () => {
expect(sync.idle).toBe(true);
});
it("a hard resync touches only the regions that differ", async () => {
const initial = numberedLines(40);
const { server, view, type, frame, host } = setup(initial);
server.failNext = { kind: "permissionDenied", message: "read-only" };
type(initial.indexOf("line 2\n"), initial.indexOf("line 2\n"), "#");
type(view.text.toString().indexOf("line 37\n"), view.text.toString().indexOf("line 37\n"), "#");
frame();
await server.processAll();
await flushMicrotasks();
expect(view.text.toString()).toBe(initial);
const touched: number[] = [];
host.applyRemote.mock.calls[0]![0].iterChanges((fromA) => touched.push(fromA));
expect(touched).toHaveLength(2);
});
it("a snapshot resync patches each region a lost event changed", async () => {
vi.useFakeTimers();
try {
const initial = numberedLines(40);
const { server, view, type, frame, host } = setup(initial);
type(initial.length, initial.length, "tail\n");
frame();
const lost = [
{ from: initial.indexOf("line 2\n"), to: initial.indexOf("line 2\n"), insert: "#" },
{ from: initial.indexOf("line 37\n"), to: initial.indexOf("line 37\n"), insert: "#" },
];
server.patch(lost);
await server.processAll();
await vi.advanceTimersByTimeAsync(100);
await server.processAll();
await flushMicrotasks();
expect(view.text.toString()).toBe(server.text);
const touched: number[] = [];
host.applyRemote.mock.calls[0]![0].iterChanges((fromA) => touched.push(fromA));
expect(touched).toHaveLength(2);
} finally {
vi.useRealTimers();
}
});
it("whenIdle resolves after everything is acked", async () => {
const { server, sync, type } = setup("");
type(0, 0, "a");
@@ -279,10 +323,22 @@ describe("DocSync", () => {
});
});
describe("minimalChange", () => {
describe("minimalChanges", () => {
it("trims common prefix and suffix", () => {
expect(minimalChange("hello world", "hello brave world")).toEqual({ from: 6, to: 6, insert: "brave " });
expect(minimalChange("abc", "abc")).toBeNull();
expect(minimalChange("aaa", "a")).toEqual({ from: 1, to: 3, insert: "" });
expect(minimalChanges("hello world", "hello brave world")).toEqual([{ from: 6, to: 6, insert: "brave " }]);
expect(minimalChanges("abc", "abc")).toEqual([]);
expect(apply("aaa", minimalChanges("aaa", "a"))).toBe("a");
});
it("keeps distant edits apart", () => {
const before = numberedLines(40);
const after = before.replace("line 2\n", "line two\n").replace("line 37\n", "line thirty-seven\n");
const changes = minimalChanges(before, after);
expect(changes).toHaveLength(2);
expect(apply(before, changes)).toBe(after);
});
it("rebuilds text across astral characters", () => {
expect(apply("a😀b\n😁c", minimalChanges("a😀b\n😁c", "a😁b\n😀c"))).toBe("a😁b\n😀c");
});
});
+6 -6
View File
@@ -26,7 +26,7 @@
*/
import { ChangeSet, Text } from "@codemirror/state";
import type { TextChange } from "@/platform/ipc";
import { minimalChange } from "./diff";
import { minimalChanges } from "./diff";
export interface SyncTransport {
edit(baseVersion: number, changes: TextChange[]): Promise<number>;
@@ -258,9 +258,9 @@ export class DocSync {
}
if (gen !== this.generation || this.disposed) return;
if (snap.version > this.serverVersion) {
const change = minimalChange(this.confirmed.toString(), snap.text);
const changes = minimalChanges(this.confirmed.toString(), snap.text);
this.pendingRemote = this.pendingRemote.filter((p) => p.version > snap.version);
this.applyRemoteNow({ baseVersion: this.serverVersion, version: snap.version, changes: change ? [change] : [] });
this.applyRemoteNow({ baseVersion: this.serverVersion, version: snap.version, changes });
this.drainRemote();
}
this.resend();
@@ -329,9 +329,9 @@ export class DocSync {
this.pendingRemote = this.pendingRemote.filter((p) => p.version > snap.version);
this.confirmed = Text.of(snap.text.split("\n"));
this.serverVersion = snap.version;
const change = minimalChange(local, snap.text);
if (change) {
const cs = ChangeSet.of([change], local.length);
const changes = minimalChanges(local, snap.text);
if (changes.length) {
const cs = ChangeSet.of(changes, local.length);
this.localText = this.confirmed;
this.host.applyRemote(cs);
}
+6 -15
View File
@@ -1,20 +1,11 @@
import { diff } from "@codemirror/merge";
import type { TextChange } from "@/platform/ipc";
/**
* The single replacement turning `a` into `b` (common prefix / suffix
* trimmed), or null when equal. Linear time; used for resyncs, where the
* backend's own patches are already minimal.
* The changes turning `a` into `b`, one per changed region; empty when equal. Used for resyncs:
* a single change spanning two distant edits would drag the caret and view to the first one.
* The timeout keeps a huge file from stalling the UI; past it the regions only get coarser.
*/
export function minimalChange(a: string, b: string): TextChange | null {
if (a === b) return null;
const max = Math.min(a.length, b.length);
let start = 0;
while (start < max && a.charCodeAt(start) === b.charCodeAt(start)) start++;
let endA = a.length;
let endB = b.length;
while (endA > start && endB > start && a.charCodeAt(endA - 1) === b.charCodeAt(endB - 1)) {
endA--;
endB--;
}
return { from: start, to: endA, insert: b.slice(start, endB) };
export function minimalChanges(a: string, b: string): TextChange[] {
return diff(a, b, { timeout: 50 }).map((c) => ({ from: c.fromA, to: c.toA, insert: b.slice(c.fromB, c.toB) }));
}