Max Brunsfeld created
This PR adds some more functionality to the Proposed Changes Editor
view, which we'll be using in
https://github.com/zed-industries/zed/pull/18240 for allowing the
assistant to propose changes to a set of buffers.
* Add an `Apply All` button, and fully implement applying of changes to
the base buffer
* Make the proposed changes editor searchable
* Fix a bug in branch buffers' diff state management
Release Notes:
- N/Acrates/editor/src/editor.rs | 4
crates/editor/src/proposed_changes_editor.rs | 84 +++++++++++++
crates/language/src/buffer.rs | 130 ++++++++++++++++-----
crates/language/src/buffer_tests.rs | 96 +++++++++------
crates/zed/src/zed.rs | 3
5 files changed, 241 insertions(+), 76 deletions(-)
@@ -98,7 +98,9 @@ use language::{
};
use language::{point_to_lsp, BufferRow, CharClassifier, Runnable, RunnableRange};
use linked_editing_ranges::refresh_linked_ranges;
-use proposed_changes_editor::{ProposedChangesBuffer, ProposedChangesEditor};
+pub use proposed_changes_editor::{
+ ProposedChangesBuffer, ProposedChangesEditor, ProposedChangesEditorToolbar,
+};
use similar::{ChangeTag, TextDiff};
use task::{ResolvedTask, TaskTemplate, TaskVariables};
@@ -6,10 +6,13 @@ use language::{Buffer, BufferEvent, Capability};
use multi_buffer::{ExcerptRange, MultiBuffer};
use project::Project;
use smol::stream::StreamExt;
-use std::{ops::Range, time::Duration};
+use std::{any::TypeId, ops::Range, time::Duration};
use text::ToOffset;
use ui::prelude::*;
-use workspace::Item;
+use workspace::{
+ searchable::SearchableItemHandle, Item, ItemHandle as _, ToolbarItemEvent, ToolbarItemLocation,
+ ToolbarItemView,
+};
pub struct ProposedChangesEditor {
editor: View<Editor>,
@@ -23,6 +26,10 @@ pub struct ProposedChangesBuffer<T> {
pub ranges: Vec<Range<T>>,
}
+pub struct ProposedChangesEditorToolbar {
+ current_editor: Option<View<ProposedChangesEditor>>,
+}
+
impl ProposedChangesEditor {
pub fn new<T: ToOffset>(
buffers: Vec<ProposedChangesBuffer<T>>,
@@ -96,6 +103,17 @@ impl ProposedChangesEditor {
self.recalculate_diffs_tx.unbounded_send(buffer).ok();
}
}
+
+ fn apply_all_changes(&self, cx: &mut ViewContext<Self>) {
+ let buffers = self.editor.read(cx).buffer.read(cx).all_buffers();
+ for branch_buffer in buffers {
+ if let Some(base_buffer) = branch_buffer.read(cx).diff_base_buffer() {
+ base_buffer.update(cx, |base_buffer, cx| {
+ base_buffer.merge(&branch_buffer, None, cx)
+ });
+ }
+ }
+ }
}
impl Render for ProposedChangesEditor {
@@ -122,4 +140,66 @@ impl Item for ProposedChangesEditor {
fn tab_content_text(&self, _cx: &WindowContext) -> Option<SharedString> {
Some("Proposed changes".into())
}
+
+ fn as_searchable(&self, _: &View<Self>) -> Option<Box<dyn SearchableItemHandle>> {
+ Some(Box::new(self.editor.clone()))
+ }
+
+ fn act_as_type<'a>(
+ &'a self,
+ type_id: TypeId,
+ self_handle: &'a View<Self>,
+ _: &'a AppContext,
+ ) -> Option<gpui::AnyView> {
+ if type_id == TypeId::of::<Self>() {
+ Some(self_handle.to_any())
+ } else if type_id == TypeId::of::<Editor>() {
+ Some(self.editor.to_any())
+ } else {
+ None
+ }
+ }
+}
+
+impl ProposedChangesEditorToolbar {
+ pub fn new() -> Self {
+ Self {
+ current_editor: None,
+ }
+ }
+
+ fn get_toolbar_item_location(&self) -> ToolbarItemLocation {
+ if self.current_editor.is_some() {
+ ToolbarItemLocation::PrimaryRight
+ } else {
+ ToolbarItemLocation::Hidden
+ }
+ }
+}
+
+impl Render for ProposedChangesEditorToolbar {
+ fn render(&mut self, _cx: &mut ViewContext<Self>) -> impl IntoElement {
+ let editor = self.current_editor.clone();
+ Button::new("apply-changes", "Apply All").on_click(move |_, cx| {
+ if let Some(editor) = &editor {
+ editor.update(cx, |editor, cx| {
+ editor.apply_all_changes(cx);
+ });
+ }
+ })
+ }
+}
+
+impl EventEmitter<ToolbarItemEvent> for ProposedChangesEditorToolbar {}
+
+impl ToolbarItemView for ProposedChangesEditorToolbar {
+ fn set_active_pane_item(
+ &mut self,
+ active_pane_item: Option<&dyn workspace::ItemHandle>,
+ _cx: &mut ViewContext<Self>,
+ ) -> workspace::ToolbarItemLocation {
+ self.current_editor =
+ active_pane_item.and_then(|item| item.downcast::<ProposedChangesEditor>());
+ self.get_toolbar_item_location()
+ }
}
@@ -87,7 +87,11 @@ pub type BufferRow = u32;
#[derive(Clone)]
enum BufferDiffBase {
Git(Rope),
- PastBufferVersion(Model<Buffer>, BufferSnapshot),
+ PastBufferVersion {
+ buffer: Model<Buffer>,
+ rope: Rope,
+ operations_to_ignore: Vec<clock::Lamport>,
+ },
}
/// An in-memory representation of a source code file, including its text,
@@ -795,19 +799,15 @@ impl Buffer {
let this = cx.handle();
cx.new_model(|cx| {
let mut branch = Self {
- diff_base: Some(BufferDiffBase::PastBufferVersion(
- this.clone(),
- self.snapshot(),
- )),
+ diff_base: Some(BufferDiffBase::PastBufferVersion {
+ buffer: this.clone(),
+ rope: self.as_rope().clone(),
+ operations_to_ignore: Vec::new(),
+ }),
language: self.language.clone(),
has_conflict: self.has_conflict,
has_unsaved_edits: Cell::new(self.has_unsaved_edits.get_mut().clone()),
- _subscriptions: vec![cx.subscribe(&this, |branch: &mut Self, _, event, cx| {
- if let BufferEvent::Operation { operation, .. } = event {
- branch.apply_ops([operation.clone()], cx);
- branch.diff_base_version += 1;
- }
- })],
+ _subscriptions: vec![cx.subscribe(&this, Self::on_base_buffer_event)],
..Self::build(
self.text.branch(),
None,
@@ -823,18 +823,74 @@ impl Buffer {
})
}
- pub fn merge(&mut self, branch: &Model<Self>, cx: &mut ModelContext<Self>) {
- let branch = branch.read(cx);
- let edits = branch
- .edits_since::<usize>(&self.version)
- .map(|edit| {
- (
- edit.old,
- branch.text_for_range(edit.new).collect::<String>(),
+ /// Applies all of the changes in `branch` buffer that intersect the given `range`
+ /// to this buffer.
+ pub fn merge(
+ &mut self,
+ branch: &Model<Self>,
+ range: Option<Range<Anchor>>,
+ cx: &mut ModelContext<Self>,
+ ) {
+ let edits = branch.read_with(cx, |branch, _| {
+ branch
+ .edits_since_in_range::<usize>(
+ &self.version,
+ range.unwrap_or(Anchor::MIN..Anchor::MAX),
)
- })
- .collect::<Vec<_>>();
- self.edit(edits, None, cx);
+ .map(|edit| {
+ (
+ edit.old,
+ branch.text_for_range(edit.new).collect::<String>(),
+ )
+ })
+ .collect::<Vec<_>>()
+ });
+ let operation = self.edit(edits, None, cx);
+
+ // Prevent this operation from being reapplied to the branch.
+ branch.update(cx, |branch, cx| {
+ if let Some(BufferDiffBase::PastBufferVersion {
+ operations_to_ignore,
+ ..
+ }) = &mut branch.diff_base
+ {
+ operations_to_ignore.extend(operation);
+ }
+ cx.emit(BufferEvent::Edited)
+ });
+ }
+
+ fn on_base_buffer_event(
+ &mut self,
+ _: Model<Buffer>,
+ event: &BufferEvent,
+ cx: &mut ModelContext<Self>,
+ ) {
+ if let BufferEvent::Operation { operation, .. } = event {
+ if let Some(BufferDiffBase::PastBufferVersion {
+ operations_to_ignore,
+ ..
+ }) = &mut self.diff_base
+ {
+ let mut is_ignored = false;
+ if let Operation::Buffer(text::Operation::Edit(buffer_operation)) = &operation {
+ operations_to_ignore.retain(|operation_to_ignore| {
+ match buffer_operation.timestamp.cmp(&operation_to_ignore) {
+ Ordering::Less => true,
+ Ordering::Equal => {
+ is_ignored = true;
+ false
+ }
+ Ordering::Greater => false,
+ }
+ });
+ }
+ if !is_ignored {
+ self.apply_ops([operation.clone()], cx);
+ self.diff_base_version += 1;
+ }
+ }
+ }
}
#[cfg(test)]
@@ -1017,9 +1073,8 @@ impl Buffer {
/// Returns the current diff base, see [Buffer::set_diff_base].
pub fn diff_base(&self) -> Option<&Rope> {
match self.diff_base.as_ref()? {
- BufferDiffBase::Git(rope) => Some(rope),
- BufferDiffBase::PastBufferVersion(_, buffer_snapshot) => {
- Some(buffer_snapshot.as_rope())
+ BufferDiffBase::Git(rope) | BufferDiffBase::PastBufferVersion { rope, .. } => {
+ Some(rope)
}
}
}
@@ -1050,29 +1105,36 @@ impl Buffer {
self.diff_base_version
}
+ pub fn diff_base_buffer(&self) -> Option<Model<Self>> {
+ match self.diff_base.as_ref()? {
+ BufferDiffBase::Git(_) => None,
+ BufferDiffBase::PastBufferVersion { buffer, .. } => Some(buffer.clone()),
+ }
+ }
+
/// Recomputes the diff.
pub fn recalculate_diff(&mut self, cx: &mut ModelContext<Self>) -> Option<Task<()>> {
- let diff_base_rope = match self.diff_base.as_mut()? {
+ let diff_base_rope = match self.diff_base.as_ref()? {
BufferDiffBase::Git(rope) => rope.clone(),
- BufferDiffBase::PastBufferVersion(base_buffer, base_buffer_snapshot) => {
- let new_base_snapshot = base_buffer.read(cx).snapshot();
- *base_buffer_snapshot = new_base_snapshot;
- base_buffer_snapshot.as_rope().clone()
- }
+ BufferDiffBase::PastBufferVersion { buffer, .. } => buffer.read(cx).as_rope().clone(),
};
- let snapshot = self.snapshot();
+ let snapshot = self.snapshot();
let mut diff = self.git_diff.clone();
let diff = cx.background_executor().spawn(async move {
diff.update(&diff_base_rope, &snapshot).await;
- diff
+ (diff, diff_base_rope)
});
Some(cx.spawn(|this, mut cx| async move {
- let buffer_diff = diff.await;
+ let (buffer_diff, diff_base_rope) = diff.await;
this.update(&mut cx, |this, cx| {
this.git_diff = buffer_diff;
this.non_text_state_update_count += 1;
+ if let Some(BufferDiffBase::PastBufferVersion { rope, .. }) = &mut this.diff_base {
+ *rope = diff_base_rope;
+ cx.emit(BufferEvent::DiffBaseChanged);
+ }
cx.emit(BufferEvent::DiffUpdated);
})
.ok();
@@ -2413,80 +2413,98 @@ fn test_branch_and_merge(cx: &mut TestAppContext) {
});
// Edits to the branch are not applied to the base.
- branch_buffer.update(cx, |buffer, cx| {
- buffer.edit(
- [(Point::new(1, 0)..Point::new(1, 0), "ONE_POINT_FIVE\n")],
+ branch_buffer.update(cx, |branch_buffer, cx| {
+ branch_buffer.edit(
+ [
+ (Point::new(1, 0)..Point::new(1, 0), "1.5\n"),
+ (Point::new(2, 0)..Point::new(2, 5), "THREE"),
+ ],
None,
cx,
)
});
branch_buffer.read_with(cx, |branch_buffer, cx| {
assert_eq!(base_buffer.read(cx).text(), "one\ntwo\nthree\n");
- assert_eq!(branch_buffer.text(), "one\nONE_POINT_FIVE\ntwo\nthree\n");
+ assert_eq!(branch_buffer.text(), "one\n1.5\ntwo\nTHREE\n");
});
+ // The branch buffer maintains a diff with respect to its base buffer.
+ start_recalculating_diff(&branch_buffer, cx);
+ cx.run_until_parked();
+ assert_diff_hunks(
+ &branch_buffer,
+ cx,
+ &[(1..2, "", "1.5\n"), (3..4, "three\n", "THREE\n")],
+ );
+
// Edits to the base are applied to the branch.
base_buffer.update(cx, |buffer, cx| {
buffer.edit([(Point::new(0, 0)..Point::new(0, 0), "ZERO\n")], None, cx)
});
branch_buffer.read_with(cx, |branch_buffer, cx| {
assert_eq!(base_buffer.read(cx).text(), "ZERO\none\ntwo\nthree\n");
- assert_eq!(
- branch_buffer.text(),
- "ZERO\none\nONE_POINT_FIVE\ntwo\nthree\n"
- );
+ assert_eq!(branch_buffer.text(), "ZERO\none\n1.5\ntwo\nTHREE\n");
});
- assert_diff_hunks(&branch_buffer, cx, &[(2..3, "", "ONE_POINT_FIVE\n")]);
+ // Until the git diff recalculation is complete, the git diff references
+ // the previous content of the base buffer, so that it stays in sync.
+ start_recalculating_diff(&branch_buffer, cx);
+ assert_diff_hunks(
+ &branch_buffer,
+ cx,
+ &[(2..3, "", "1.5\n"), (4..5, "three\n", "THREE\n")],
+ );
+ cx.run_until_parked();
+ assert_diff_hunks(
+ &branch_buffer,
+ cx,
+ &[(2..3, "", "1.5\n"), (4..5, "three\n", "THREE\n")],
+ );
// Edits to any replica of the base are applied to the branch.
base_buffer_replica.update(cx, |buffer, cx| {
- buffer.edit(
- [(Point::new(2, 0)..Point::new(2, 0), "TWO_POINT_FIVE\n")],
- None,
- cx,
- )
+ buffer.edit([(Point::new(2, 0)..Point::new(2, 0), "2.5\n")], None, cx)
});
branch_buffer.read_with(cx, |branch_buffer, cx| {
- assert_eq!(
- base_buffer.read(cx).text(),
- "ZERO\none\ntwo\nTWO_POINT_FIVE\nthree\n"
- );
- assert_eq!(
- branch_buffer.text(),
- "ZERO\none\nONE_POINT_FIVE\ntwo\nTWO_POINT_FIVE\nthree\n"
- );
+ assert_eq!(base_buffer.read(cx).text(), "ZERO\none\ntwo\n2.5\nthree\n");
+ assert_eq!(branch_buffer.text(), "ZERO\none\n1.5\ntwo\n2.5\nTHREE\n");
});
// Merging the branch applies all of its changes to the base.
base_buffer.update(cx, |base_buffer, cx| {
- base_buffer.merge(&branch_buffer, cx);
+ base_buffer.merge(&branch_buffer, None, cx);
+ });
+
+ branch_buffer.update(cx, |branch_buffer, cx| {
assert_eq!(
- base_buffer.text(),
- "ZERO\none\nONE_POINT_FIVE\ntwo\nTWO_POINT_FIVE\nthree\n"
+ base_buffer.read(cx).text(),
+ "ZERO\none\n1.5\ntwo\n2.5\nTHREE\n"
);
+ assert_eq!(branch_buffer.text(), "ZERO\none\n1.5\ntwo\n2.5\nTHREE\n");
});
}
+fn start_recalculating_diff(buffer: &Model<Buffer>, cx: &mut TestAppContext) {
+ buffer
+ .update(cx, |buffer, cx| buffer.recalculate_diff(cx).unwrap())
+ .detach();
+}
+
+#[track_caller]
fn assert_diff_hunks(
buffer: &Model<Buffer>,
cx: &mut TestAppContext,
expected_hunks: &[(Range<u32>, &str, &str)],
) {
- buffer
- .update(cx, |buffer, cx| buffer.recalculate_diff(cx).unwrap())
- .detach();
- cx.executor().run_until_parked();
-
- buffer.read_with(cx, |buffer, _| {
- let snapshot = buffer.snapshot();
- assert_hunks(
- snapshot.git_diff_hunks_intersecting_range(Anchor::MIN..Anchor::MAX),
- &snapshot,
- &buffer.diff_base().unwrap().to_string(),
- expected_hunks,
- );
- });
+ let (snapshot, diff_base) = buffer.read_with(cx, |buffer, _| {
+ (buffer.snapshot(), buffer.diff_base().unwrap().to_string())
+ });
+ assert_hunks(
+ snapshot.git_diff_hunks_intersecting_range(Anchor::MIN..Anchor::MAX),
+ &snapshot,
+ &diff_base,
+ expected_hunks,
+ );
}
#[gpui::test(iterations = 100)]
@@ -14,6 +14,7 @@ use breadcrumbs::Breadcrumbs;
use client::ZED_URL_SCHEME;
use collections::VecDeque;
use command_palette_hooks::CommandPaletteFilter;
+use editor::ProposedChangesEditorToolbar;
use editor::{scroll::Autoscroll, Editor, MultiBuffer};
use feature_flags::FeatureFlagAppExt;
use gpui::{
@@ -582,6 +583,8 @@ fn initialize_pane(workspace: &mut Workspace, pane: &View<Pane>, cx: &mut ViewCo
let buffer_search_bar = cx.new_view(search::BufferSearchBar::new);
toolbar.add_item(buffer_search_bar.clone(), cx);
+ let proposed_change_bar = cx.new_view(|_| ProposedChangesEditorToolbar::new());
+ toolbar.add_item(proposed_change_bar, cx);
let quick_action_bar =
cx.new_view(|cx| QuickActionBar::new(buffer_search_bar, workspace, cx));
toolbar.add_item(quick_action_bar, cx);