Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Use line-based and word-based diff when reloading and formatting buffers (#25129)

Browse source 
Closes https://github.com/zed-industries/zed/issues/10122
Closes https://github.com/zed-industries/zed/issues/25034

When formatting buffers or reloading them after they change on disk, we
performed a diff between the buffer's current contents and the new
content. We need this diff in order preserve the positions of cursors
and other decorations when updating the buffer's text.

In order to handle changes within lines, we would previously compute a
*character-wise* diff. This was extremely expensive for large files.

This PR gets rid of the character-wise diff, and instead performs a
normal line-wise diff. Then, for certain replace hunks, we compute a
secondary word-based diff. Also, I've switched to the
[`imara-diff`](https://github.com/pascalkuthe/imara-diff) crate, instead
of `similar`.

Release Notes:

- Fixed a hang that could occur when large files were changed on disk or
formatted.
This commit is contained in:
Max Brunsfeld 2025-02-19 16:56:01 -08:00 • committed by GitHub
parent 1087e05da4
commit 0fdad0c0d6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 429 additions and 343 deletions
Download patch file Download diff file Expand all files Collapse all files

1
crates/project/Cargo.toml
View file

@ -64,7 +64,6 @@ settings.workspace = true
sha2.workspace = true
shellexpand.workspace = true
shlex.workspace = true
similar = "1.3"
smol.workspace = true
snippet.workspace = true
snippet_provider.workspace = true

58
crates/project/src/lsp_store.rs
View file

@ -63,7 +63,6 @@ use rpc::{
use serde::Serialize;
use settings::{Settings, SettingsLocation, SettingsStore};
use sha2::{Digest, Sha256};
use similar::{ChangeTag, TextDiff};
use smol::channel::Sender;
use snippet::Snippet;
use std::{
@ -110,15 +109,13 @@ pub enum LspFormatTarget {
Ranges(BTreeMap<BufferId, Vec<Range<Anchor>>>),
}
// proto::RegisterBufferWithLanguageServer {}
pub type OpenLspBufferHandle = Entity<Entity<Buffer>>;
// Currently, formatting operations are represented differently depending on
// whether they come from a language server or an external command.
#[derive(Debug)]
pub enum FormatOperation {
Lsp(Vec<(Range<Anchor>, String)>),
Lsp(Vec<(Range<Anchor>, Arc<str>)>),
External(Diff),
Prettier(Diff),
}
@ -1388,7 +1385,7 @@ impl LocalLspStore {
language_server: &Arc<LanguageServer>,
settings: &LanguageSettings,
cx: &mut AsyncApp,
) -> Result<Vec<(Range<Anchor>, String)>> {
) -> Result<Vec<(Range<Anchor>, Arc<str>)>> {
let capabilities = &language_server.capabilities();
let range_formatting_provider = capabilities.document_range_formatting_provider.as_ref();
if range_formatting_provider.map_or(false, |provider| provider == &OneOf::Left(false)) {
@ -1459,7 +1456,7 @@ impl LocalLspStore {
language_server: &Arc<LanguageServer>,
settings: &LanguageSettings,
cx: &mut AsyncApp,
) -> Result<Vec<(Range<Anchor>, String)>> {
) -> Result<Vec<(Range<Anchor>, Arc<str>)>> {
let uri = lsp::Url::from_file_path(abs_path)
.map_err(|_| anyhow!("failed to convert abs path to uri"))?;
let text_document = lsp::TextDocumentIdentifier::new(uri);
@ -2144,7 +2141,7 @@ impl LocalLspStore {
server_id: LanguageServerId,
version: Option<i32>,
cx: &mut Context<LspStore>,
) -> Task<Result<Vec<(Range<Anchor>, String)>>> {
) -> Task<Result<Vec<(Range<Anchor>, Arc<str>)>>> {
let snapshot = self.buffer_snapshot_for_lsp_version(buffer, server_id, version, cx);
cx.background_spawn(async move {
let snapshot = snapshot?;
@ -2192,48 +2189,23 @@ impl LocalLspStore {
// we can identify the changes more precisely, preserving the locations
// of any anchors positioned in the unchanged regions.
if range.end.row > range.start.row {
let mut offset = range.start.to_offset(&snapshot);
let offset = range.start.to_offset(&snapshot);
let old_text = snapshot.text_for_range(range).collect::<String>();
let diff = TextDiff::from_lines(old_text.as_str(), &new_text);
let mut moved_since_edit = true;
for change in diff.iter_all_changes() {
let tag = change.tag();
let value = change.value();
match tag {
ChangeTag::Equal => {
offset += value.len();
moved_since_edit = true;
}
ChangeTag::Delete => {
let start = snapshot.anchor_after(offset);
let end = snapshot.anchor_before(offset + value.len());
if moved_since_edit {
edits.push((start..end, String::new()));
} else {
edits.last_mut().unwrap().0.end = end;
}
offset += value.len();
moved_since_edit = false;
}
ChangeTag::Insert => {
if moved_since_edit {
let anchor = snapshot.anchor_after(offset);
edits.push((anchor..anchor, value.to_string()));
} else {
edits.last_mut().unwrap().1.push_str(value);
}
moved_since_edit = false;
}
}
}
let range_edits = language::text_diff(old_text.as_str(), &new_text);
edits.extend(range_edits.into_iter().map(|(range, replacement)| {
(
snapshot.anchor_after(offset + range.start)
..snapshot.anchor_before(offset + range.end),
replacement,
)
}));
} else if range.end == range.start {
let anchor = snapshot.anchor_after(range.start);
edits.push((anchor..anchor, new_text));
edits.push((anchor..anchor, new_text.into()));
} else {
let edit_start = snapshot.anchor_after(range.start);
let edit_end = snapshot.anchor_before(range.end);
edits.push((edit_start..edit_end, new_text));
edits.push((edit_start..edit_end, new_text.into()));
}
}

30
crates/project/src/project_tests.rs
View file

@ -25,7 +25,11 @@ use std::{mem, num::NonZeroU32, ops::Range, task::Poll};
use task::{ResolvedTask, TaskContext};
use unindent::Unindent as _;
use util::{
assert_set_eq, path, paths::PathMatcher, separator, test::TempTree, uri, TryFutureExt as _,
assert_set_eq, path,
paths::PathMatcher,
separator,
test::{marked_text_offsets, TempTree},
uri, TryFutureExt as _,
};
#[gpui::test]
@ -3625,7 +3629,8 @@ async fn test_buffer_is_dirty(cx: &mut gpui::TestAppContext) {
async fn test_buffer_file_changes_on_disk(cx: &mut gpui::TestAppContext) {
init_test(cx);
let initial_contents = "aaa\nbbbbb\nc\n";
let (initial_contents, initial_offsets) =
marked_text_offsets("one twoˇ\nthree ˇfourˇ five\nsixˇ seven\n");
let fs = FakeFs::new(cx.executor());
fs.insert_tree(
path!("/dir"),
@ -3640,8 +3645,9 @@ async fn test_buffer_file_changes_on_disk(cx: &mut gpui::TestAppContext) {
.await
.unwrap();
let anchors = (0..3)
.map(|row| buffer.update(cx, |b, _| b.anchor_before(Point::new(row, 1))))
let anchors = initial_offsets
.iter()
.map(|offset| buffer.update(cx, |b, _| b.anchor_before(offset)))
.collect::<Vec<_>>();
// Change the file on disk, adding two new lines of text, and removing
@ -3650,10 +3656,12 @@ async fn test_buffer_file_changes_on_disk(cx: &mut gpui::TestAppContext) {
assert!(!buffer.is_dirty());
assert!(!buffer.has_conflict());
});
let new_contents = "AAAA\naaa\nBB\nbbbbb\n";
let (new_contents, new_offsets) =
marked_text_offsets("oneˇ\nthree ˇFOURˇ five\nsixtyˇ seven\n");
fs.save(
path!("/dir/the-file").as_ref(),
&new_contents.into(),
&new_contents.as_str().into(),
LineEnding::Unix,
)
.await
@ -3668,14 +3676,11 @@ async fn test_buffer_file_changes_on_disk(cx: &mut gpui::TestAppContext) {
assert!(!buffer.is_dirty());
assert!(!buffer.has_conflict());
let anchor_positions = anchors
let anchor_offsets = anchors
.iter()
.map(|anchor| anchor.to_point(&*buffer))
.map(|anchor| anchor.to_offset(&*buffer))
.collect::<Vec<_>>();
assert_eq!(
anchor_positions,
[Point::new(1, 1), Point::new(3, 1), Point::new(3, 5)]
);
assert_eq!(anchor_offsets, new_offsets);
});
// Modify the buffer
@ -3698,6 +3703,7 @@ async fn test_buffer_file_changes_on_disk(cx: &mut gpui::TestAppContext) {
// marked as having a conflict.
cx.executor().run_until_parked();
buffer.update(cx, |buffer, _| {
assert_eq!(buffer.text(), " ".to_string() + &new_contents);
assert!(buffer.has_conflict());
});
}