
We had a frequent panic when the agent was using our edit file tool. The root cause was that we were constructing a `BufferDiff` with `BufferDiff::new`, then calling `set_base_text`, but not waiting for that asynchronous operation to finish. This means there was a window of time where the diff's base text was set to the initial value of `""`--that's not a problem in itself, but it was possible for us to call `PendingDiff::update` during that window, which calls `BufferDiff::update_diff`, which calls `BufferDiffSnapshot::new_with_base_buffer`, which takes two arguments `base_text` and `base_text_snapshot` that are supposed to represent the same text. We were getting the first of those arguments from the `base_text` field of `PendingDiff`, which is set immediately to the target base text without waiting for `BufferDiff::set_base_text` to run to completion; and the second from the `BufferDiff` itself, which still has the empty base text during that window. As a result of that mismatch, we could end up adding `DeletedHunk` diff transforms to the multibuffer for the diff card even though the multibuffer's base text was empty, ultimately leading to a panic very far away in rendering code. I've fixed this by adding a new `BufferDiff` constructor for the case where the buffer contents and the base text are (initially) the same, like for the diff cards, and so we don't need an async diff calculation. I also added a debug assertion to catch the basic issue here earlier, when `BufferDiffSnapshot::new_with_base_buffer` is called with two base texts that don't match. Release Notes: - N/A --------- Co-authored-by: Conrad <conrad@zed.dev>
424 lines
14 KiB
Rust
424 lines
14 KiB
Rust
use anyhow::Result;
|
|
use buffer_diff::{BufferDiff, BufferDiffSnapshot};
|
|
use editor::{MultiBuffer, PathKey};
|
|
use gpui::{App, AppContext, AsyncApp, Context, Entity, Subscription, Task};
|
|
use itertools::Itertools;
|
|
use language::{
|
|
Anchor, Buffer, Capability, LanguageRegistry, OffsetRangeExt as _, Point, Rope, TextBuffer,
|
|
};
|
|
use std::{
|
|
cmp::Reverse,
|
|
ops::Range,
|
|
path::{Path, PathBuf},
|
|
sync::Arc,
|
|
};
|
|
use util::ResultExt;
|
|
|
|
pub enum Diff {
|
|
Pending(PendingDiff),
|
|
Finalized(FinalizedDiff),
|
|
}
|
|
|
|
impl Diff {
|
|
pub fn finalized(
|
|
path: PathBuf,
|
|
old_text: Option<String>,
|
|
new_text: String,
|
|
language_registry: Arc<LanguageRegistry>,
|
|
cx: &mut Context<Self>,
|
|
) -> Self {
|
|
let multibuffer = cx.new(|_cx| MultiBuffer::without_headers(Capability::ReadOnly));
|
|
let new_buffer = cx.new(|cx| Buffer::local(new_text, cx));
|
|
let base_text = old_text.clone().unwrap_or(String::new()).into();
|
|
let task = cx.spawn({
|
|
let multibuffer = multibuffer.clone();
|
|
let path = path.clone();
|
|
let buffer = new_buffer.clone();
|
|
async move |_, cx| {
|
|
let language = language_registry
|
|
.language_for_file_path(&path)
|
|
.await
|
|
.log_err();
|
|
|
|
buffer.update(cx, |buffer, cx| buffer.set_language(language.clone(), cx))?;
|
|
|
|
let diff = build_buffer_diff(
|
|
old_text.unwrap_or("".into()).into(),
|
|
&buffer,
|
|
Some(language_registry.clone()),
|
|
cx,
|
|
)
|
|
.await?;
|
|
|
|
multibuffer
|
|
.update(cx, |multibuffer, cx| {
|
|
let hunk_ranges = {
|
|
let buffer = buffer.read(cx);
|
|
let diff = diff.read(cx);
|
|
diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, buffer, cx)
|
|
.map(|diff_hunk| diff_hunk.buffer_range.to_point(buffer))
|
|
.collect::<Vec<_>>()
|
|
};
|
|
|
|
multibuffer.set_excerpts_for_path(
|
|
PathKey::for_buffer(&buffer, cx),
|
|
buffer.clone(),
|
|
hunk_ranges,
|
|
editor::DEFAULT_MULTIBUFFER_CONTEXT,
|
|
cx,
|
|
);
|
|
multibuffer.add_diff(diff, cx);
|
|
})
|
|
.log_err();
|
|
|
|
anyhow::Ok(())
|
|
}
|
|
});
|
|
|
|
Self::Finalized(FinalizedDiff {
|
|
multibuffer,
|
|
path,
|
|
base_text,
|
|
new_buffer,
|
|
_update_diff: task,
|
|
})
|
|
}
|
|
|
|
pub fn new(buffer: Entity<Buffer>, cx: &mut Context<Self>) -> Self {
|
|
let buffer_text_snapshot = buffer.read(cx).text_snapshot();
|
|
let base_text_snapshot = buffer.read(cx).snapshot();
|
|
let base_text = base_text_snapshot.text();
|
|
debug_assert_eq!(buffer_text_snapshot.text(), base_text);
|
|
let buffer_diff = cx.new(|cx| {
|
|
let mut diff = BufferDiff::new_unchanged(&buffer_text_snapshot, base_text_snapshot);
|
|
let snapshot = diff.snapshot(cx);
|
|
let secondary_diff = cx.new(|cx| {
|
|
let mut diff = BufferDiff::new(&buffer_text_snapshot, cx);
|
|
diff.set_snapshot(snapshot, &buffer_text_snapshot, cx);
|
|
diff
|
|
});
|
|
diff.set_secondary_diff(secondary_diff);
|
|
diff
|
|
});
|
|
|
|
let multibuffer = cx.new(|cx| {
|
|
let mut multibuffer = MultiBuffer::without_headers(Capability::ReadOnly);
|
|
multibuffer.add_diff(buffer_diff.clone(), cx);
|
|
multibuffer
|
|
});
|
|
|
|
Self::Pending(PendingDiff {
|
|
multibuffer,
|
|
base_text: Arc::new(base_text),
|
|
_subscription: cx.observe(&buffer, |this, _, cx| {
|
|
if let Diff::Pending(diff) = this {
|
|
diff.update(cx);
|
|
}
|
|
}),
|
|
new_buffer: buffer,
|
|
diff: buffer_diff,
|
|
revealed_ranges: Vec::new(),
|
|
update_diff: Task::ready(Ok(())),
|
|
})
|
|
}
|
|
|
|
pub fn reveal_range(&mut self, range: Range<Anchor>, cx: &mut Context<Self>) {
|
|
if let Self::Pending(diff) = self {
|
|
diff.reveal_range(range, cx);
|
|
}
|
|
}
|
|
|
|
pub fn finalize(&mut self, cx: &mut Context<Self>) {
|
|
if let Self::Pending(diff) = self {
|
|
*self = Self::Finalized(diff.finalize(cx));
|
|
}
|
|
}
|
|
|
|
pub fn multibuffer(&self) -> &Entity<MultiBuffer> {
|
|
match self {
|
|
Self::Pending(PendingDiff { multibuffer, .. }) => multibuffer,
|
|
Self::Finalized(FinalizedDiff { multibuffer, .. }) => multibuffer,
|
|
}
|
|
}
|
|
|
|
pub fn to_markdown(&self, cx: &App) -> String {
|
|
let buffer_text = self
|
|
.multibuffer()
|
|
.read(cx)
|
|
.all_buffers()
|
|
.iter()
|
|
.map(|buffer| buffer.read(cx).text())
|
|
.join("\n");
|
|
let path = match self {
|
|
Diff::Pending(PendingDiff {
|
|
new_buffer: buffer, ..
|
|
}) => buffer.read(cx).file().map(|file| file.path().as_ref()),
|
|
Diff::Finalized(FinalizedDiff { path, .. }) => Some(path.as_path()),
|
|
};
|
|
format!(
|
|
"Diff: {}\n```\n{}\n```\n",
|
|
path.unwrap_or(Path::new("untitled")).display(),
|
|
buffer_text
|
|
)
|
|
}
|
|
|
|
pub fn has_revealed_range(&self, cx: &App) -> bool {
|
|
self.multibuffer().read(cx).excerpt_paths().next().is_some()
|
|
}
|
|
|
|
pub fn needs_update(&self, old_text: &str, new_text: &str, cx: &App) -> bool {
|
|
match self {
|
|
Diff::Pending(PendingDiff {
|
|
base_text,
|
|
new_buffer,
|
|
..
|
|
}) => {
|
|
base_text.as_str() != old_text
|
|
|| !new_buffer.read(cx).as_rope().chunks().equals_str(new_text)
|
|
}
|
|
Diff::Finalized(FinalizedDiff {
|
|
base_text,
|
|
new_buffer,
|
|
..
|
|
}) => {
|
|
base_text.as_str() != old_text
|
|
|| !new_buffer.read(cx).as_rope().chunks().equals_str(new_text)
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
pub struct PendingDiff {
|
|
multibuffer: Entity<MultiBuffer>,
|
|
base_text: Arc<String>,
|
|
new_buffer: Entity<Buffer>,
|
|
diff: Entity<BufferDiff>,
|
|
revealed_ranges: Vec<Range<Anchor>>,
|
|
_subscription: Subscription,
|
|
update_diff: Task<Result<()>>,
|
|
}
|
|
|
|
impl PendingDiff {
|
|
pub fn update(&mut self, cx: &mut Context<Diff>) {
|
|
let buffer = self.new_buffer.clone();
|
|
let buffer_diff = self.diff.clone();
|
|
let base_text = self.base_text.clone();
|
|
self.update_diff = cx.spawn(async move |diff, cx| {
|
|
let text_snapshot = buffer.read_with(cx, |buffer, _| buffer.text_snapshot())?;
|
|
let diff_snapshot = BufferDiff::update_diff(
|
|
buffer_diff.clone(),
|
|
text_snapshot.clone(),
|
|
Some(base_text),
|
|
false,
|
|
false,
|
|
None,
|
|
None,
|
|
cx,
|
|
)
|
|
.await?;
|
|
buffer_diff.update(cx, |diff, cx| {
|
|
diff.set_snapshot(diff_snapshot.clone(), &text_snapshot, cx);
|
|
diff.secondary_diff().unwrap().update(cx, |diff, cx| {
|
|
diff.set_snapshot(diff_snapshot.clone(), &text_snapshot, cx);
|
|
});
|
|
})?;
|
|
diff.update(cx, |diff, cx| {
|
|
if let Diff::Pending(diff) = diff {
|
|
diff.update_visible_ranges(cx);
|
|
}
|
|
})
|
|
});
|
|
}
|
|
|
|
pub fn reveal_range(&mut self, range: Range<Anchor>, cx: &mut Context<Diff>) {
|
|
self.revealed_ranges.push(range);
|
|
self.update_visible_ranges(cx);
|
|
}
|
|
|
|
fn finalize(&self, cx: &mut Context<Diff>) -> FinalizedDiff {
|
|
let ranges = self.excerpt_ranges(cx);
|
|
let base_text = self.base_text.clone();
|
|
let language_registry = self.new_buffer.read(cx).language_registry();
|
|
|
|
let path = self
|
|
.new_buffer
|
|
.read(cx)
|
|
.file()
|
|
.map(|file| file.path().as_ref())
|
|
.unwrap_or(Path::new("untitled"))
|
|
.into();
|
|
|
|
// Replace the buffer in the multibuffer with the snapshot
|
|
let buffer = cx.new(|cx| {
|
|
let language = self.new_buffer.read(cx).language().cloned();
|
|
let buffer = TextBuffer::new_normalized(
|
|
0,
|
|
cx.entity_id().as_non_zero_u64().into(),
|
|
self.new_buffer.read(cx).line_ending(),
|
|
self.new_buffer.read(cx).as_rope().clone(),
|
|
);
|
|
let mut buffer = Buffer::build(buffer, None, Capability::ReadWrite);
|
|
buffer.set_language(language, cx);
|
|
buffer
|
|
});
|
|
|
|
let buffer_diff = cx.spawn({
|
|
let buffer = buffer.clone();
|
|
async move |_this, cx| {
|
|
build_buffer_diff(base_text, &buffer, language_registry, cx).await
|
|
}
|
|
});
|
|
|
|
let update_diff = cx.spawn(async move |this, cx| {
|
|
let buffer_diff = buffer_diff.await?;
|
|
this.update(cx, |this, cx| {
|
|
this.multibuffer().update(cx, |multibuffer, cx| {
|
|
let path_key = PathKey::for_buffer(&buffer, cx);
|
|
multibuffer.clear(cx);
|
|
multibuffer.set_excerpts_for_path(
|
|
path_key,
|
|
buffer,
|
|
ranges,
|
|
editor::DEFAULT_MULTIBUFFER_CONTEXT,
|
|
cx,
|
|
);
|
|
multibuffer.add_diff(buffer_diff.clone(), cx);
|
|
});
|
|
|
|
cx.notify();
|
|
})
|
|
});
|
|
|
|
FinalizedDiff {
|
|
path,
|
|
base_text: self.base_text.clone(),
|
|
multibuffer: self.multibuffer.clone(),
|
|
new_buffer: self.new_buffer.clone(),
|
|
_update_diff: update_diff,
|
|
}
|
|
}
|
|
|
|
fn update_visible_ranges(&mut self, cx: &mut Context<Diff>) {
|
|
let ranges = self.excerpt_ranges(cx);
|
|
self.multibuffer.update(cx, |multibuffer, cx| {
|
|
multibuffer.set_excerpts_for_path(
|
|
PathKey::for_buffer(&self.new_buffer, cx),
|
|
self.new_buffer.clone(),
|
|
ranges,
|
|
editor::DEFAULT_MULTIBUFFER_CONTEXT,
|
|
cx,
|
|
);
|
|
let end = multibuffer.len(cx);
|
|
Some(multibuffer.snapshot(cx).offset_to_point(end).row + 1)
|
|
});
|
|
cx.notify();
|
|
}
|
|
|
|
fn excerpt_ranges(&self, cx: &App) -> Vec<Range<Point>> {
|
|
let buffer = self.new_buffer.read(cx);
|
|
let diff = self.diff.read(cx);
|
|
let mut ranges = diff
|
|
.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, buffer, cx)
|
|
.map(|diff_hunk| diff_hunk.buffer_range.to_point(buffer))
|
|
.collect::<Vec<_>>();
|
|
ranges.extend(
|
|
self.revealed_ranges
|
|
.iter()
|
|
.map(|range| range.to_point(buffer)),
|
|
);
|
|
ranges.sort_unstable_by_key(|range| (range.start, Reverse(range.end)));
|
|
|
|
// Merge adjacent ranges
|
|
let mut ranges = ranges.into_iter().peekable();
|
|
let mut merged_ranges = Vec::new();
|
|
while let Some(mut range) = ranges.next() {
|
|
while let Some(next_range) = ranges.peek() {
|
|
if range.end >= next_range.start {
|
|
range.end = range.end.max(next_range.end);
|
|
ranges.next();
|
|
} else {
|
|
break;
|
|
}
|
|
}
|
|
|
|
merged_ranges.push(range);
|
|
}
|
|
merged_ranges
|
|
}
|
|
}
|
|
|
|
pub struct FinalizedDiff {
|
|
path: PathBuf,
|
|
base_text: Arc<String>,
|
|
new_buffer: Entity<Buffer>,
|
|
multibuffer: Entity<MultiBuffer>,
|
|
_update_diff: Task<Result<()>>,
|
|
}
|
|
|
|
async fn build_buffer_diff(
|
|
old_text: Arc<String>,
|
|
buffer: &Entity<Buffer>,
|
|
language_registry: Option<Arc<LanguageRegistry>>,
|
|
cx: &mut AsyncApp,
|
|
) -> Result<Entity<BufferDiff>> {
|
|
let buffer = cx.update(|cx| buffer.read(cx).snapshot())?;
|
|
|
|
let old_text_rope = cx
|
|
.background_spawn({
|
|
let old_text = old_text.clone();
|
|
async move { Rope::from(old_text.as_str()) }
|
|
})
|
|
.await;
|
|
let base_buffer = cx
|
|
.update(|cx| {
|
|
Buffer::build_snapshot(
|
|
old_text_rope,
|
|
buffer.language().cloned(),
|
|
language_registry,
|
|
cx,
|
|
)
|
|
})?
|
|
.await;
|
|
|
|
let diff_snapshot = cx
|
|
.update(|cx| {
|
|
BufferDiffSnapshot::new_with_base_buffer(
|
|
buffer.text.clone(),
|
|
Some(old_text),
|
|
base_buffer,
|
|
cx,
|
|
)
|
|
})?
|
|
.await;
|
|
|
|
let secondary_diff = cx.new(|cx| {
|
|
let mut diff = BufferDiff::new(&buffer, cx);
|
|
diff.set_snapshot(diff_snapshot.clone(), &buffer, cx);
|
|
diff
|
|
})?;
|
|
|
|
cx.new(|cx| {
|
|
let mut diff = BufferDiff::new(&buffer.text, cx);
|
|
diff.set_snapshot(diff_snapshot, &buffer, cx);
|
|
diff.set_secondary_diff(secondary_diff);
|
|
diff
|
|
})
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use gpui::{AppContext as _, TestAppContext};
|
|
use language::Buffer;
|
|
|
|
use crate::Diff;
|
|
|
|
#[gpui::test]
|
|
async fn test_pending_diff(cx: &mut TestAppContext) {
|
|
let buffer = cx.new(|cx| Buffer::local("hello!", cx));
|
|
let _diff = cx.new(|cx| Diff::new(buffer.clone(), cx));
|
|
buffer.update(cx, |buffer, cx| {
|
|
buffer.set_text("HELLO!", cx);
|
|
});
|
|
cx.run_until_parked();
|
|
}
|
|
}
|