From 4a252515b1e3dd9d3ec5a6e3f4c17acece774205 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 2 Apr 2025 00:13:28 +0200 Subject: [PATCH] Improve tracking for agent edits (#27857) Release Notes: - N/A --------- Co-authored-by: Nathan Sobo Co-authored-by: Max Brunsfeld --- Cargo.lock | 8 +- assets/keymaps/default-linux.json | 2 +- assets/keymaps/default-macos.json | 2 +- crates/assistant2/src/assistant.rs | 2 +- crates/assistant2/src/assistant_diff.rs | 133 +- crates/assistant2/src/message_editor.rs | 19 +- crates/assistant2/src/thread.rs | 9 +- crates/assistant_tool/Cargo.toml | 8 +- crates/assistant_tool/src/action_log.rs | 1173 +++++++++-------- crates/assistant_tools/Cargo.toml | 2 - .../assistant_tools/src/create_file_tool.rs | 9 +- crates/assistant_tools/src/edit_files_tool.rs | 23 +- .../src/find_replace_file_tool.rs | 19 +- crates/assistant_tools/src/replace.rs | 2 +- crates/git/src/repository.rs | 97 -- crates/language/src/buffer.rs | 23 +- crates/language/src/buffer_tests.rs | 6 +- crates/project/src/lsp_store.rs | 6 +- crates/text/src/patch.rs | 9 + 19 files changed, 757 insertions(+), 795 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 81ea8f3865..171e3022a3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -693,17 +693,22 @@ name = "assistant_tool" version = "0.1.0" dependencies = [ "anyhow", - "async-watch", "buffer_diff", "clock", "collections", + "ctor", "derive_more", + "env_logger 0.11.7", + "futures 0.3.31", "gpui", "icons", "language", "language_model", + "log", "parking_lot", + "pretty_assertions", "project", + "rand 0.8.5", "serde", "serde_json", "settings", @@ -718,7 +723,6 @@ dependencies = [ "anyhow", "assistant_tool", "chrono", - "clock", "collections", "feature_flags", "futures 0.3.31", diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 72916952a2..cedf508183 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -149,7 +149,7 @@ { "context": "AssistantDiff", "bindings": { - "ctrl-y": "agent::ToggleKeep", + "ctrl-y": "agent::Keep", "ctrl-k ctrl-r": "agent::Reject" } }, diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 1197d74221..83e0ef3637 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -241,7 +241,7 @@ "context": "AssistantDiff", "use_key_equivalents": true, "bindings": { - "cmd-y": "agent::ToggleKeep", + "cmd-y": "agent::Keep", "cmd-alt-z": "agent::Reject" } }, diff --git a/crates/assistant2/src/assistant.rs b/crates/assistant2/src/assistant.rs index 3a2fb29659..b34986d33f 100644 --- a/crates/assistant2/src/assistant.rs +++ b/crates/assistant2/src/assistant.rs @@ -66,7 +66,7 @@ actions!( AcceptSuggestedContext, OpenActiveThreadAsMarkdown, OpenAssistantDiff, - ToggleKeep, + Keep, Reject, RejectAll, KeepAll diff --git a/crates/assistant2/src/assistant_diff.rs b/crates/assistant2/src/assistant_diff.rs index d000ed10e1..62485a66db 100644 --- a/crates/assistant2/src/assistant_diff.rs +++ b/crates/assistant2/src/assistant_diff.rs @@ -1,4 +1,4 @@ -use crate::{Thread, ThreadEvent, ToggleKeep}; +use crate::{Thread, ThreadEvent}; use anyhow::Result; use buffer_diff::DiffHunkStatus; use collections::HashSet; @@ -78,7 +78,7 @@ impl AssistantDiff { hunk_range, is_created_file, line_height, - _editor: &Entity, + editor: &Entity, window: &mut Window, cx: &mut App| { render_diff_hunk_controls( @@ -88,6 +88,7 @@ impl AssistantDiff { is_created_file, line_height, &assistant_diff, + editor, window, cx, ) @@ -130,7 +131,7 @@ impl AssistantDiff { let changed_buffers = thread.action_log().read(cx).changed_buffers(cx); let mut paths_to_delete = self.multibuffer.read(cx).paths().collect::>(); - for (buffer, changed) in changed_buffers { + for (buffer, diff_handle) in changed_buffers { let Some(file) = buffer.read(cx).file().cloned() else { continue; }; @@ -139,7 +140,7 @@ impl AssistantDiff { paths_to_delete.remove(&path_key); let snapshot = buffer.read(cx).snapshot(); - let diff = changed.diff.read(cx); + let diff = diff_handle.read(cx); let diff_hunk_ranges = diff .hunks_intersecting_range( language::Anchor::MIN..language::Anchor::MAX, @@ -159,7 +160,7 @@ impl AssistantDiff { editor::DEFAULT_MULTIBUFFER_CONTEXT, cx, ); - multibuffer.add_diff(changed.diff.clone(), cx); + multibuffer.add_diff(diff_handle, cx); (was_empty, is_excerpt_newly_added) }); @@ -221,7 +222,7 @@ impl AssistantDiff { } } - fn toggle_keep(&mut self, _: &crate::ToggleKeep, _window: &mut Window, cx: &mut Context) { + fn keep(&mut self, _: &crate::Keep, _window: &mut Window, cx: &mut Context) { let ranges = self .editor .read(cx) @@ -240,8 +241,7 @@ impl AssistantDiff { let buffer = self.multibuffer.read(cx).buffer(hunk.buffer_id); if let Some(buffer) = buffer { self.thread.update(cx, |thread, cx| { - let accept = hunk.status().has_secondary_hunk(); - thread.review_edits_in_range(buffer, hunk.buffer_range, accept, cx) + thread.keep_edits_in_range(buffer, hunk.buffer_range, cx) }); } } @@ -268,10 +268,9 @@ impl AssistantDiff { .update(cx, |thread, cx| thread.keep_all_edits(cx)); } - fn review_diff_hunks( + fn keep_edits_in_ranges( &mut self, hunk_ranges: Vec>, - accept: bool, cx: &mut Context, ) { let snapshot = self.multibuffer.read(cx).snapshot(cx); @@ -285,7 +284,7 @@ impl AssistantDiff { let buffer = self.multibuffer.read(cx).buffer(hunk.buffer_id); if let Some(buffer) = buffer { self.thread.update(cx, |thread, cx| { - thread.review_edits_in_range(buffer, hunk.buffer_range, accept, cx) + thread.keep_edits_in_range(buffer, hunk.buffer_range, cx) }); } } @@ -479,7 +478,7 @@ impl Render for AssistantDiff { } else { "AssistantDiff" }) - .on_action(cx.listener(Self::toggle_keep)) + .on_action(cx.listener(Self::keep)) .on_action(cx.listener(Self::reject)) .on_action(cx.listener(Self::reject_all)) .on_action(cx.listener(Self::keep_all)) @@ -495,16 +494,16 @@ impl Render for AssistantDiff { fn render_diff_hunk_controls( row: u32, - status: &DiffHunkStatus, + _status: &DiffHunkStatus, hunk_range: Range, is_created_file: bool, line_height: Pixels, assistant_diff: &Entity, + editor: &Entity, window: &mut Window, cx: &mut App, ) -> AnyElement { - let editor = assistant_diff.read(cx).editor.clone(); - + let editor = editor.clone(); h_flex() .h(line_height) .mr_0p5() @@ -519,75 +518,47 @@ fn render_diff_hunk_controls( .gap_1() .occlude() .shadow_md() - .children(if status.has_secondary_hunk() { - vec![ - Button::new("reject", "Reject") - .disabled(is_created_file) - .key_binding( - KeyBinding::for_action_in( - &crate::Reject, - &editor.read(cx).focus_handle(cx), - window, - cx, - ) - .map(|kb| kb.size(rems_from_px(12.))), - ) - .on_click({ - let editor = editor.clone(); - move |_event, window, cx| { - editor.update(cx, |editor, cx| { - let snapshot = editor.snapshot(window, cx); - let point = hunk_range.start.to_point(&snapshot.buffer_snapshot); - editor.restore_hunks_in_ranges(vec![point..point], window, cx); - }); - } - }), - Button::new(("keep", row as u64), "Keep") - .key_binding( - KeyBinding::for_action_in( - &crate::ToggleKeep, - &editor.read(cx).focus_handle(cx), - window, - cx, - ) - .map(|kb| kb.size(rems_from_px(12.))), - ) - .on_click({ - let assistant_diff = assistant_diff.clone(); - move |_event, _window, cx| { - assistant_diff.update(cx, |diff, cx| { - diff.review_diff_hunks( - vec![hunk_range.start..hunk_range.start], - true, - cx, - ); - }); - } - }), - ] - } else { - vec![ - Button::new(("review", row as u64), "Review") - .key_binding(KeyBinding::for_action_in( - &ToggleKeep, + .children(vec![ + Button::new("reject", "Reject") + .disabled(is_created_file) + .key_binding( + KeyBinding::for_action_in( + &crate::Reject, &editor.read(cx).focus_handle(cx), window, cx, - )) - .on_click({ - let assistant_diff = assistant_diff.clone(); - move |_event, _window, cx| { - assistant_diff.update(cx, |diff, cx| { - diff.review_diff_hunks( - vec![hunk_range.start..hunk_range.start], - false, - cx, - ); - }); - } - }), - ] - }) + ) + .map(|kb| kb.size(rems_from_px(12.))), + ) + .on_click({ + let editor = editor.clone(); + move |_event, window, cx| { + editor.update(cx, |editor, cx| { + let snapshot = editor.snapshot(window, cx); + let point = hunk_range.start.to_point(&snapshot.buffer_snapshot); + editor.restore_hunks_in_ranges(vec![point..point], window, cx); + }); + } + }), + Button::new(("keep", row as u64), "Keep") + .key_binding( + KeyBinding::for_action_in( + &crate::Keep, + &editor.read(cx).focus_handle(cx), + window, + cx, + ) + .map(|kb| kb.size(rems_from_px(12.))), + ) + .on_click({ + let assistant_diff = assistant_diff.clone(); + move |_event, _window, cx| { + assistant_diff.update(cx, |diff, cx| { + diff.keep_edits_in_ranges(vec![hunk_range.start..hunk_range.start], cx); + }); + } + }), + ]) .when( !editor.read(cx).buffer().read(cx).all_diff_hunks_expanded(), |el| { diff --git a/crates/assistant2/src/message_editor.rs b/crates/assistant2/src/message_editor.rs index 7bef4a83dd..b1733edd3f 100644 --- a/crates/assistant2/src/message_editor.rs +++ b/crates/assistant2/src/message_editor.rs @@ -245,9 +245,6 @@ impl MessageEditor { thread .update(cx, |thread, cx| { let context = context_store.read(cx).context().clone(); - thread.action_log().update(cx, |action_log, cx| { - action_log.clear_reviewed_changes(cx); - }); thread.insert_user_message(user_message, context, checkpoint, cx); }) .ok(); @@ -546,7 +543,7 @@ impl Render for MessageEditor { parent.child( v_flex().bg(cx.theme().colors().editor_background).children( changed_buffers.into_iter().enumerate().flat_map( - |(index, (buffer, changed))| { + |(index, (buffer, _diff))| { let file = buffer.read(cx).file()?; let path = file.path(); @@ -619,25 +616,13 @@ impl Render for MessageEditor { .color(Color::Deleted), ), ) - .when(!changed.needs_review, |parent| { - parent.child( - Icon::new(IconName::Check) - .color(Color::Success), - ) - }) .child( div() .h_full() .absolute() .w_8() .bottom_0() - .map(|this| { - if !changed.needs_review { - this.right_4() - } else { - this.right_0() - } - }) + .right_0() .bg(linear_gradient( 90., linear_color_stop( diff --git a/crates/assistant2/src/thread.rs b/crates/assistant2/src/thread.rs index e84ffd07f4..5bca86db12 100644 --- a/crates/assistant2/src/thread.rs +++ b/crates/assistant2/src/thread.rs @@ -1679,23 +1679,20 @@ impl Thread { Ok(String::from_utf8_lossy(&markdown).to_string()) } - pub fn review_edits_in_range( + pub fn keep_edits_in_range( &mut self, buffer: Entity, buffer_range: Range, - accept: bool, cx: &mut Context, ) { self.action_log.update(cx, |action_log, cx| { - action_log.review_edits_in_range(buffer, buffer_range, accept, cx) + action_log.keep_edits_in_range(buffer, buffer_range, cx) }); } - /// Keeps all edits across all buffers at once. - /// This provides a more performant alternative to calling review_edits_in_range for each buffer. pub fn keep_all_edits(&mut self, cx: &mut Context) { self.action_log - .update(cx, |action_log, _cx| action_log.keep_all_edits()); + .update(cx, |action_log, cx| action_log.keep_all_edits(cx)); } pub fn action_log(&self) -> &Entity { diff --git a/crates/assistant_tool/Cargo.toml b/crates/assistant_tool/Cargo.toml index 6b36fb416b..cd257c615d 100644 --- a/crates/assistant_tool/Cargo.toml +++ b/crates/assistant_tool/Cargo.toml @@ -13,11 +13,11 @@ path = "src/assistant_tool.rs" [dependencies] anyhow.workspace = true -async-watch.workspace = true buffer_diff.workspace = true clock.workspace = true collections.workspace = true derive_more.workspace = true +futures.workspace = true gpui.workspace = true icons.workspace = true language.workspace = true @@ -27,15 +27,21 @@ project.workspace = true serde.workspace = true serde_json.workspace = true text.workspace = true +util.workspace = true [dev-dependencies] buffer_diff = { workspace = true, features = ["test-support"] } collections = { workspace = true, features = ["test-support"] } clock = { workspace = true, features = ["test-support"] } +ctor.workspace = true +env_logger.workspace = true gpui = { workspace = true, features = ["test-support"] } language = { workspace = true, features = ["test-support"] } language_model = { workspace = true, features = ["test-support"] } +log.workspace = true +pretty_assertions.workspace = true project = { workspace = true, features = ["test-support"] } +rand.workspace = true settings = { workspace = true, features = ["test-support"] } text = { workspace = true, features = ["test-support"] } util = { workspace = true, features = ["test-support"] } diff --git a/crates/assistant_tool/src/action_log.rs b/crates/assistant_tool/src/action_log.rs index 099378464d..535ef4c930 100644 --- a/crates/assistant_tool/src/action_log.rs +++ b/crates/assistant_tool/src/action_log.rs @@ -1,11 +1,12 @@ use anyhow::{Context as _, Result}; use buffer_diff::BufferDiff; -use collections::{BTreeMap, HashMap, HashSet}; +use collections::{BTreeMap, HashSet}; +use futures::{StreamExt, channel::mpsc}; use gpui::{App, AppContext, AsyncApp, Context, Entity, Subscription, Task, WeakEntity}; -use language::{ - Buffer, BufferEvent, DiskState, OffsetRangeExt, Operation, TextBufferSnapshot, ToOffset, -}; -use std::{ops::Range, sync::Arc}; +use language::{Buffer, BufferEvent, DiskState, Point}; +use std::{cmp, ops::Range, sync::Arc}; +use text::{Edit, Patch, Rope}; +use util::RangeExt; /// Tracks actions performed by tools in a thread pub struct ActionLog { @@ -28,21 +29,6 @@ impl ActionLog { } } - pub fn clear_reviewed_changes(&mut self, cx: &mut Context) { - self.tracked_buffers - .retain(|_buffer, tracked_buffer| match &mut tracked_buffer.change { - Change::Edited { - accepted_edit_ids, .. - } => { - accepted_edit_ids.clear(); - tracked_buffer.schedule_diff_update(); - true - } - Change::Deleted { reviewed, .. } => !*reviewed, - }); - cx.notify(); - } - /// Notifies a diagnostics check pub fn checked_project_diagnostics(&mut self) { self.edited_since_project_diagnostics_check = false; @@ -64,27 +50,31 @@ impl ActionLog { .entry(buffer.clone()) .or_insert_with(|| { let text_snapshot = buffer.read(cx).text_snapshot(); - let unreviewed_diff = cx.new(|cx| BufferDiff::new(&text_snapshot, cx)); - let diff = cx.new(|cx| { - let mut diff = BufferDiff::new(&text_snapshot, cx); - diff.set_secondary_diff(unreviewed_diff.clone()); - diff - }); - let (diff_update_tx, diff_update_rx) = async_watch::channel(()); + let diff = cx.new(|cx| BufferDiff::new(&text_snapshot, cx)); + let (diff_update_tx, diff_update_rx) = mpsc::unbounded(); + let base_text; + let status; + let unreviewed_changes; + if created { + base_text = Rope::default(); + status = TrackedBufferStatus::Created; + unreviewed_changes = Patch::new(vec![Edit { + old: 0..1, + new: 0..text_snapshot.max_point().row + 1, + }]) + } else { + base_text = buffer.read(cx).as_rope().clone(); + status = TrackedBufferStatus::Modified; + unreviewed_changes = Patch::default(); + } TrackedBuffer { buffer: buffer.clone(), - change: Change::Edited { - unreviewed_edit_ids: HashSet::default(), - accepted_edit_ids: HashSet::default(), - initial_content: if created { - None - } else { - Some(text_snapshot.clone()) - }, - }, + base_text, + unreviewed_changes, + snapshot: text_snapshot.clone(), + status, version: buffer.read(cx).version(), diff, - secondary_diff: unreviewed_diff, diff_update: diff_update_tx, _maintain_diff: cx.spawn({ let buffer = buffer.clone(); @@ -108,9 +98,7 @@ impl ActionLog { cx: &mut Context, ) { match event { - BufferEvent::Operation { operation, .. } => { - self.handle_buffer_operation(buffer, operation, cx) - } + BufferEvent::Edited { .. } => self.handle_buffer_edited(buffer, cx), BufferEvent::FileHandleChanged => { self.handle_buffer_file_changed(buffer, cx); } @@ -118,62 +106,11 @@ impl ActionLog { }; } - fn handle_buffer_operation( - &mut self, - buffer: Entity, - operation: &Operation, - cx: &mut Context, - ) { + fn handle_buffer_edited(&mut self, buffer: Entity, cx: &mut Context) { let Some(tracked_buffer) = self.tracked_buffers.get_mut(&buffer) else { return; }; - let Operation::Buffer(text::Operation::Edit(operation)) = operation else { - return; - }; - let Change::Edited { - unreviewed_edit_ids, - accepted_edit_ids, - .. - } = &mut tracked_buffer.change - else { - return; - }; - - if unreviewed_edit_ids.contains(&operation.timestamp) - || accepted_edit_ids.contains(&operation.timestamp) - { - return; - } - - let buffer = buffer.read(cx); - let operation_edit_ranges = buffer - .edited_ranges_for_edit_ids::([&operation.timestamp]) - .collect::>(); - let intersects_unreviewed_edits = ranges_intersect( - operation_edit_ranges.iter().cloned(), - buffer.edited_ranges_for_edit_ids::(unreviewed_edit_ids.iter()), - ); - let mut intersected_accepted_edits = HashSet::default(); - for accepted_edit_id in accepted_edit_ids.iter() { - let intersects_accepted_edit = ranges_intersect( - operation_edit_ranges.iter().cloned(), - buffer.edited_ranges_for_edit_ids::([accepted_edit_id]), - ); - if intersects_accepted_edit { - intersected_accepted_edits.insert(*accepted_edit_id); - } - } - - // If the buffer operation overlaps with any tracked edits, mark it as unreviewed. - // If it intersects an already-accepted id, mark that edit as unreviewed again. - if intersects_unreviewed_edits || !intersected_accepted_edits.is_empty() { - unreviewed_edit_ids.insert(operation.timestamp); - for accepted_edit_id in intersected_accepted_edits { - unreviewed_edit_ids.insert(accepted_edit_id); - accepted_edit_ids.remove(&accepted_edit_id); - } - tracked_buffer.schedule_diff_update(); - } + tracked_buffer.schedule_diff_update(ChangeAuthor::User, cx); } fn handle_buffer_file_changed(&mut self, buffer: Entity, cx: &mut Context) { @@ -181,25 +118,8 @@ impl ActionLog { return; }; - match tracked_buffer.change { - Change::Deleted { .. } => { - if buffer - .read(cx) - .file() - .map_or(false, |file| file.disk_state() != DiskState::Deleted) - { - // If the buffer had been deleted by a tool, but it got - // resurrected externally, we want to clear the changes we - // were tracking and reset the buffer's state. - tracked_buffer.change = Change::Edited { - unreviewed_edit_ids: HashSet::default(), - accepted_edit_ids: HashSet::default(), - initial_content: Some(buffer.read(cx).text_snapshot()), - }; - } - tracked_buffer.schedule_diff_update(); - } - Change::Edited { .. } => { + match tracked_buffer.status { + TrackedBufferStatus::Created | TrackedBufferStatus::Modified => { if buffer .read(cx) .file() @@ -208,9 +128,22 @@ impl ActionLog { // If the buffer had been edited by a tool, but it got // deleted externally, we want to stop tracking it. self.tracked_buffers.remove(&buffer); - } else { - tracked_buffer.schedule_diff_update(); } + cx.notify(); + } + TrackedBufferStatus::Deleted => { + if buffer + .read(cx) + .file() + .map_or(false, |file| file.disk_state() != DiskState::Deleted) + { + // If the buffer had been deleted by a tool, but it got + // resurrected externally, we want to clear the changes we + // were tracking and reset the buffer's state. + self.tracked_buffers.remove(&buffer); + self.track_buffer(buffer, false, cx); + } + cx.notify(); } } } @@ -218,19 +151,77 @@ impl ActionLog { async fn maintain_diff( this: WeakEntity, buffer: Entity, - mut diff_update: async_watch::Receiver<()>, + mut diff_update: mpsc::UnboundedReceiver<(ChangeAuthor, text::BufferSnapshot)>, cx: &mut AsyncApp, ) -> Result<()> { - while let Some(_) = diff_update.recv().await.ok() { - let update = this.update(cx, |this, cx| { + while let Some((author, buffer_snapshot)) = diff_update.next().await { + let (rebase, diff, language, language_registry) = + this.read_with(cx, |this, cx| { + let tracked_buffer = this + .tracked_buffers + .get(&buffer) + .context("buffer not tracked")?; + + let rebase = cx.background_spawn({ + let mut base_text = tracked_buffer.base_text.clone(); + let old_snapshot = tracked_buffer.snapshot.clone(); + let new_snapshot = buffer_snapshot.clone(); + let unreviewed_changes = tracked_buffer.unreviewed_changes.clone(); + async move { + let edits = diff_snapshots(&old_snapshot, &new_snapshot); + let unreviewed_changes = match author { + ChangeAuthor::User => rebase_patch( + &unreviewed_changes, + edits, + &mut base_text, + new_snapshot.as_rope(), + ), + ChangeAuthor::Agent => unreviewed_changes.compose(edits), + }; + ( + Arc::new(base_text.to_string()), + base_text, + unreviewed_changes, + ) + } + }); + + anyhow::Ok(( + rebase, + tracked_buffer.diff.clone(), + tracked_buffer.buffer.read(cx).language().cloned(), + tracked_buffer.buffer.read(cx).language_registry(), + )) + })??; + + let (new_base_text, new_base_text_rope, unreviewed_changes) = rebase.await; + let diff_snapshot = BufferDiff::update_diff( + diff.clone(), + buffer_snapshot.clone(), + Some(new_base_text), + true, + false, + language, + language_registry, + cx, + ) + .await; + if let Ok(diff_snapshot) = diff_snapshot { + diff.update(cx, |diff, cx| { + diff.set_snapshot(diff_snapshot, &buffer_snapshot, None, cx) + })?; + } + this.update(cx, |this, cx| { let tracked_buffer = this .tracked_buffers .get_mut(&buffer) .context("buffer not tracked")?; - anyhow::Ok(tracked_buffer.update_diff(cx)) + tracked_buffer.base_text = new_base_text_rope; + tracked_buffer.snapshot = buffer_snapshot; + tracked_buffer.unreviewed_changes = unreviewed_changes; + cx.notify(); + anyhow::Ok(()) })??; - update.await; - this.update(cx, |_this, cx| cx.notify())?; } Ok(()) @@ -242,159 +233,126 @@ impl ActionLog { } /// Track a buffer as read, so we can notify the model about user edits. - pub fn will_create_buffer( - &mut self, - buffer: Entity, - edit_id: Option, - cx: &mut Context, - ) { + pub fn will_create_buffer(&mut self, buffer: Entity, cx: &mut Context) { self.track_buffer(buffer.clone(), true, cx); - self.buffer_edited(buffer, edit_id.into_iter().collect(), cx) + self.buffer_edited(buffer, cx) } /// Mark a buffer as edited, so we can refresh it in the context - pub fn buffer_edited( - &mut self, - buffer: Entity, - mut edit_ids: Vec, - cx: &mut Context, - ) { + pub fn buffer_edited(&mut self, buffer: Entity, cx: &mut Context) { self.edited_since_project_diagnostics_check = true; self.stale_buffers_in_context.insert(buffer.clone()); let tracked_buffer = self.track_buffer(buffer.clone(), false, cx); - - match &mut tracked_buffer.change { - Change::Edited { - unreviewed_edit_ids, - .. - } => { - unreviewed_edit_ids.extend(edit_ids.iter().copied()); - } - Change::Deleted { - deleted_content, - deletion_id, - .. - } => { - edit_ids.extend(*deletion_id); - tracked_buffer.change = Change::Edited { - unreviewed_edit_ids: edit_ids.into_iter().collect(), - accepted_edit_ids: HashSet::default(), - initial_content: Some(deleted_content.clone()), - }; - } + if let TrackedBufferStatus::Deleted = tracked_buffer.status { + tracked_buffer.status = TrackedBufferStatus::Modified; } - - tracked_buffer.schedule_diff_update(); + tracked_buffer.schedule_diff_update(ChangeAuthor::Agent, cx); } pub fn will_delete_buffer(&mut self, buffer: Entity, cx: &mut Context) { let tracked_buffer = self.track_buffer(buffer.clone(), false, cx); - if let Change::Edited { - initial_content, .. - } = &tracked_buffer.change - { - if let Some(initial_content) = initial_content { - let deletion_id = buffer.update(cx, |buffer, cx| buffer.set_text("", cx)); - tracked_buffer.change = Change::Deleted { - reviewed: false, - deleted_content: initial_content.clone(), - deletion_id, - }; - tracked_buffer.schedule_diff_update(); - } else { + match tracked_buffer.status { + TrackedBufferStatus::Created => { self.tracked_buffers.remove(&buffer); cx.notify(); } + TrackedBufferStatus::Modified => { + buffer.update(cx, |buffer, cx| buffer.set_text("", cx)); + tracked_buffer.status = TrackedBufferStatus::Deleted; + tracked_buffer.schedule_diff_update(ChangeAuthor::Agent, cx); + } + TrackedBufferStatus::Deleted => {} } + cx.notify(); } - /// Accepts edits in a given range within a buffer. - pub fn review_edits_in_range( + pub fn keep_edits_in_range( &mut self, buffer: Entity, buffer_range: Range, - accept: bool, cx: &mut Context, - ) { + ) where + T: 'static + language::ToPoint, // + Clone + // + Copy + // + Ord + // + Sub + // + Add + // + AddAssign + // + Default + // + PartialEq, + { let Some(tracked_buffer) = self.tracked_buffers.get_mut(&buffer) else { return; }; - let buffer = buffer.read(cx); - let buffer_range = buffer_range.to_offset(buffer); - - match &mut tracked_buffer.change { - Change::Deleted { reviewed, .. } => { - *reviewed = accept; + match tracked_buffer.status { + TrackedBufferStatus::Deleted => { + self.tracked_buffers.remove(&buffer); + cx.notify(); } - Change::Edited { - unreviewed_edit_ids, - accepted_edit_ids, - .. - } => { - let (source, destination) = if accept { - (unreviewed_edit_ids, accepted_edit_ids) - } else { - (accepted_edit_ids, unreviewed_edit_ids) - }; - source.retain(|edit_id| { - for range in buffer.edited_ranges_for_edit_ids::([edit_id]) { - if buffer_range.end >= range.start && buffer_range.start <= range.end { - destination.insert(*edit_id); - return false; - } + _ => { + let buffer = buffer.read(cx); + let buffer_range = + buffer_range.start.to_point(buffer)..buffer_range.end.to_point(buffer); + let buffer_row_range = buffer_range.start.row..buffer_range.end.row + 1; + let mut delta = 0i32; + tracked_buffer.unreviewed_changes.retain_mut(|edit| { + edit.old.start = (edit.old.start as i32 + delta) as u32; + edit.old.end = (edit.old.end as i32 + delta) as u32; + if edit.new.overlaps(&buffer_row_range) { + let old_bytes = tracked_buffer + .base_text + .point_to_offset(Point::new(edit.old.start, 0)) + ..tracked_buffer.base_text.point_to_offset(cmp::min( + Point::new(edit.old.end, 0), + tracked_buffer.base_text.max_point(), + )); + let new_bytes = tracked_buffer + .snapshot + .point_to_offset(Point::new(edit.new.start, 0)) + ..tracked_buffer.snapshot.point_to_offset(cmp::min( + Point::new(edit.new.end, 0), + tracked_buffer.snapshot.max_point(), + )); + tracked_buffer.base_text.replace( + old_bytes, + &tracked_buffer + .snapshot + .text_for_range(new_bytes) + .collect::(), + ); + delta += edit.new_len() as i32 - edit.old_len() as i32; + false + } else { + true } - true }); + tracked_buffer.schedule_diff_update(ChangeAuthor::User, cx); } } - - tracked_buffer.schedule_diff_update(); } - /// Keep all edits across all buffers. - /// This is a more performant alternative to calling review_edits_in_range for each buffer. - pub fn keep_all_edits(&mut self) { - // Process all tracked buffers - for (_, tracked_buffer) in self.tracked_buffers.iter_mut() { - match &mut tracked_buffer.change { - Change::Deleted { reviewed, .. } => { - *reviewed = true; + pub fn keep_all_edits(&mut self, cx: &mut Context) { + self.tracked_buffers + .retain(|_buffer, tracked_buffer| match tracked_buffer.status { + TrackedBufferStatus::Deleted => false, + _ => { + tracked_buffer.unreviewed_changes.clear(); + tracked_buffer.base_text = tracked_buffer.snapshot.as_rope().clone(); + tracked_buffer.schedule_diff_update(ChangeAuthor::User, cx); + true } - Change::Edited { - unreviewed_edit_ids, - accepted_edit_ids, - .. - } => { - accepted_edit_ids.extend(unreviewed_edit_ids.drain()); - } - } - - tracked_buffer.schedule_diff_update(); - } + }); + cx.notify(); } /// Returns the set of buffers that contain changes that haven't been reviewed by the user. - pub fn changed_buffers(&self, cx: &App) -> BTreeMap, ChangedBuffer> { + pub fn changed_buffers(&self, cx: &App) -> BTreeMap, Entity> { self.tracked_buffers .iter() .filter(|(_, tracked)| tracked.has_changes(cx)) - .map(|(buffer, tracked)| { - ( - buffer.clone(), - ChangedBuffer { - diff: tracked.diff.clone(), - needs_review: match &tracked.change { - Change::Edited { - unreviewed_edit_ids, - .. - } => !unreviewed_edit_ids.is_empty(), - Change::Deleted { reviewed, .. } => !reviewed, - }, - }, - ) - }) + .map(|(buffer, tracked)| (buffer.clone(), tracked.diff.clone())) .collect() } @@ -419,48 +377,169 @@ impl ActionLog { } } -fn ranges_intersect( - ranges_a: impl IntoIterator>, - ranges_b: impl IntoIterator>, -) -> bool { - let mut ranges_a_iter = ranges_a.into_iter().peekable(); - let mut ranges_b_iter = ranges_b.into_iter().peekable(); - while let (Some(range_a), Some(range_b)) = (ranges_a_iter.peek(), ranges_b_iter.peek()) { - if range_a.end < range_b.start { - ranges_a_iter.next(); - } else if range_b.end < range_a.start { - ranges_b_iter.next(); +fn rebase_patch( + patch: &Patch, + edits: Vec>, + old_text: &mut Rope, + new_text: &Rope, +) -> Patch { + let mut translated_unreviewed_edits = Patch::default(); + let mut conflicting_edits = Vec::new(); + + let mut old_edits = patch.edits().iter().cloned().peekable(); + let mut new_edits = edits.into_iter().peekable(); + let mut applied_delta = 0i32; + let mut rebased_delta = 0i32; + + while let Some(mut new_edit) = new_edits.next() { + let mut conflict = false; + + // Push all the old edits that are before this new edit or that intersect with it. + while let Some(old_edit) = old_edits.peek() { + if new_edit.old.end <= old_edit.new.start { + break; + } else if new_edit.old.start >= old_edit.new.end { + let mut old_edit = old_edits.next().unwrap(); + old_edit.old.start = (old_edit.old.start as i32 + applied_delta) as u32; + old_edit.old.end = (old_edit.old.end as i32 + applied_delta) as u32; + old_edit.new.start = (old_edit.new.start as i32 + applied_delta) as u32; + old_edit.new.end = (old_edit.new.end as i32 + applied_delta) as u32; + rebased_delta += old_edit.new_len() as i32 - old_edit.old_len() as i32; + translated_unreviewed_edits.push(old_edit); + } else { + conflict = true; + if new_edits + .peek() + .map_or(false, |next_edit| next_edit.old.overlaps(&old_edit.new)) + { + new_edit.old.start = (new_edit.old.start as i32 + applied_delta) as u32; + new_edit.old.end = (new_edit.old.end as i32 + applied_delta) as u32; + conflicting_edits.push(new_edit); + new_edit = new_edits.next().unwrap(); + } else { + let mut old_edit = old_edits.next().unwrap(); + old_edit.old.start = (old_edit.old.start as i32 + applied_delta) as u32; + old_edit.old.end = (old_edit.old.end as i32 + applied_delta) as u32; + old_edit.new.start = (old_edit.new.start as i32 + applied_delta) as u32; + old_edit.new.end = (old_edit.new.end as i32 + applied_delta) as u32; + rebased_delta += old_edit.new_len() as i32 - old_edit.old_len() as i32; + translated_unreviewed_edits.push(old_edit); + } + } + } + + if conflict { + new_edit.old.start = (new_edit.old.start as i32 + applied_delta) as u32; + new_edit.old.end = (new_edit.old.end as i32 + applied_delta) as u32; + conflicting_edits.push(new_edit); } else { - return true; + // This edit doesn't intersect with any old edit, so we can apply it to the old text. + new_edit.old.start = (new_edit.old.start as i32 + applied_delta - rebased_delta) as u32; + new_edit.old.end = (new_edit.old.end as i32 + applied_delta - rebased_delta) as u32; + let old_bytes = old_text.point_to_offset(Point::new(new_edit.old.start, 0)) + ..old_text.point_to_offset(cmp::min( + Point::new(new_edit.old.end, 0), + old_text.max_point(), + )); + let new_bytes = new_text.point_to_offset(Point::new(new_edit.new.start, 0)) + ..new_text.point_to_offset(cmp::min( + Point::new(new_edit.new.end, 0), + new_text.max_point(), + )); + + old_text.replace( + old_bytes, + &new_text.chunks_in_range(new_bytes).collect::(), + ); + applied_delta += new_edit.new_len() as i32 - new_edit.old_len() as i32; } } - false + + // Push all the outstanding old edits. + for mut old_edit in old_edits { + old_edit.old.start = (old_edit.old.start as i32 + applied_delta) as u32; + old_edit.old.end = (old_edit.old.end as i32 + applied_delta) as u32; + old_edit.new.start = (old_edit.new.start as i32 + applied_delta) as u32; + old_edit.new.end = (old_edit.new.end as i32 + applied_delta) as u32; + translated_unreviewed_edits.push(old_edit); + } + + translated_unreviewed_edits.compose(conflicting_edits) +} + +fn diff_snapshots( + old_snapshot: &text::BufferSnapshot, + new_snapshot: &text::BufferSnapshot, +) -> Vec> { + let mut edits = new_snapshot + .edits_since::(&old_snapshot.version) + .map(|edit| { + if edit.old.start.column == old_snapshot.line_len(edit.old.start.row) + && new_snapshot.chars_at(edit.new.start).next() == Some('\n') + && edit.old.start != old_snapshot.max_point() + { + Edit { + old: edit.old.start.row + 1..edit.old.end.row + 1, + new: edit.new.start.row + 1..edit.new.end.row + 1, + } + } else if edit.old.start.column == 0 + && edit.old.end.column == 0 + && edit.new.end.column == 0 + && edit.old.end != old_snapshot.max_point() + { + Edit { + old: edit.old.start.row..edit.old.end.row, + new: edit.new.start.row..edit.new.end.row, + } + } else { + Edit { + old: edit.old.start.row..edit.old.end.row + 1, + new: edit.new.start.row..edit.new.end.row + 1, + } + } + }) + .peekable(); + let mut row_edits = Vec::new(); + while let Some(mut edit) = edits.next() { + while let Some(next_edit) = edits.peek() { + if edit.old.end >= next_edit.old.start { + edit.old.end = next_edit.old.end; + edit.new.end = next_edit.new.end; + edits.next(); + } else { + break; + } + } + row_edits.push(edit); + } + row_edits +} + +enum ChangeAuthor { + User, + Agent, +} + +#[derive(Copy, Clone, Eq, PartialEq)] +enum TrackedBufferStatus { + Created, + Modified, + Deleted, } struct TrackedBuffer { buffer: Entity, - change: Change, + base_text: Rope, + unreviewed_changes: Patch, + status: TrackedBufferStatus, version: clock::Global, diff: Entity, - secondary_diff: Entity, - diff_update: async_watch::Sender<()>, + snapshot: text::BufferSnapshot, + diff_update: mpsc::UnboundedSender<(ChangeAuthor, text::BufferSnapshot)>, _maintain_diff: Task<()>, _subscription: Subscription, } -enum Change { - Edited { - unreviewed_edit_ids: HashSet, - accepted_edit_ids: HashSet, - initial_content: Option, - }, - Deleted { - reviewed: bool, - deleted_content: TextBufferSnapshot, - deletion_id: Option, - }, -} - impl TrackedBuffer { fn has_changes(&self, cx: &App) -> bool { self.diff @@ -470,165 +549,62 @@ impl TrackedBuffer { .is_some() } - fn schedule_diff_update(&self) { - self.diff_update.send(()).ok(); - } - - fn update_diff(&mut self, cx: &mut App) -> Task<()> { - match &self.change { - Change::Edited { - unreviewed_edit_ids, - accepted_edit_ids, - .. - } => { - let edits_to_undo = unreviewed_edit_ids - .iter() - .chain(accepted_edit_ids) - .map(|edit_id| (*edit_id, u32::MAX)) - .collect::>(); - let buffer_without_edits = self.buffer.update(cx, |buffer, cx| buffer.branch(cx)); - buffer_without_edits - .update(cx, |buffer, cx| buffer.undo_operations(edits_to_undo, cx)); - let primary_diff_update = self.diff.update(cx, |diff, cx| { - diff.set_base_text_buffer( - buffer_without_edits, - self.buffer.read(cx).text_snapshot(), - cx, - ) - }); - - let unreviewed_edits_to_undo = unreviewed_edit_ids - .iter() - .map(|edit_id| (*edit_id, u32::MAX)) - .collect::>(); - let buffer_without_unreviewed_edits = - self.buffer.update(cx, |buffer, cx| buffer.branch(cx)); - buffer_without_unreviewed_edits.update(cx, |buffer, cx| { - buffer.undo_operations(unreviewed_edits_to_undo, cx) - }); - let secondary_diff_update = self.secondary_diff.update(cx, |diff, cx| { - diff.set_base_text_buffer( - buffer_without_unreviewed_edits.clone(), - self.buffer.read(cx).text_snapshot(), - cx, - ) - }); - - cx.background_spawn(async move { - _ = primary_diff_update.await; - _ = secondary_diff_update.await; - }) - } - Change::Deleted { - reviewed, - deleted_content, - .. - } => { - let reviewed = *reviewed; - let deleted_content = deleted_content.clone(); - - let primary_diff = self.diff.clone(); - let secondary_diff = self.secondary_diff.clone(); - let buffer_snapshot = self.buffer.read(cx).text_snapshot(); - let language = self.buffer.read(cx).language().cloned(); - let language_registry = self.buffer.read(cx).language_registry().clone(); - - cx.spawn(async move |cx| { - let base_text = Arc::new(deleted_content.text()); - - let primary_diff_snapshot = BufferDiff::update_diff( - primary_diff.clone(), - buffer_snapshot.clone(), - Some(base_text.clone()), - true, - false, - language.clone(), - language_registry.clone(), - cx, - ) - .await; - let secondary_diff_snapshot = BufferDiff::update_diff( - secondary_diff.clone(), - buffer_snapshot.clone(), - if reviewed { - None - } else { - Some(base_text.clone()) - }, - true, - false, - language.clone(), - language_registry.clone(), - cx, - ) - .await; - - if let Ok(primary_diff_snapshot) = primary_diff_snapshot { - primary_diff - .update(cx, |diff, cx| { - diff.set_snapshot(primary_diff_snapshot, &buffer_snapshot, None, cx) - }) - .ok(); - } - - if let Ok(secondary_diff_snapshot) = secondary_diff_snapshot { - secondary_diff - .update(cx, |diff, cx| { - diff.set_snapshot( - secondary_diff_snapshot, - &buffer_snapshot, - None, - cx, - ) - }) - .ok(); - } - }) - } - } + fn schedule_diff_update(&self, author: ChangeAuthor, cx: &App) { + self.diff_update + .unbounded_send((author, self.buffer.read(cx).text_snapshot())) + .ok(); } } pub struct ChangedBuffer { pub diff: Entity, - pub needs_review: bool, } #[cfg(test)] mod tests { + use std::env; + use super::*; use buffer_diff::DiffHunkStatusKind; use gpui::TestAppContext; use language::Point; use project::{FakeFs, Fs, Project, RemoveOptions}; + use rand::prelude::*; use serde_json::json; use settings::SettingsStore; - use util::path; + use util::{RandomCharIter, path, post_inc}; + + #[ctor::ctor] + fn init_logger() { + if std::env::var("RUST_LOG").is_ok() { + env_logger::init(); + } + } #[gpui::test(iterations = 10)] async fn test_edit_review(cx: &mut TestAppContext) { let action_log = cx.new(|_| ActionLog::new()); let buffer = cx.new(|cx| Buffer::local("abc\ndef\nghi\njkl\nmno", cx)); - let edit1 = buffer.update(cx, |buffer, cx| { - buffer - .edit([(Point::new(1, 1)..Point::new(1, 2), "E")], None, cx) - .unwrap() - }); - let edit2 = buffer.update(cx, |buffer, cx| { - buffer - .edit([(Point::new(4, 2)..Point::new(4, 3), "O")], None, cx) - .unwrap() + cx.update(|cx| { + action_log.update(cx, |log, cx| log.buffer_read(buffer.clone(), cx)); + buffer.update(cx, |buffer, cx| { + buffer + .edit([(Point::new(1, 1)..Point::new(1, 2), "E")], None, cx) + .unwrap() + }); + buffer.update(cx, |buffer, cx| { + buffer + .edit([(Point::new(4, 2)..Point::new(4, 3), "O")], None, cx) + .unwrap() + }); + action_log.update(cx, |log, cx| log.buffer_edited(buffer.clone(), cx)); }); + cx.run_until_parked(); assert_eq!( buffer.read_with(cx, |buffer, _| buffer.text()), "abc\ndEf\nghi\njkl\nmnO" ); - - action_log.update(cx, |log, cx| { - log.buffer_edited(buffer.clone(), vec![edit1, edit2], cx) - }); - cx.run_until_parked(); assert_eq!( unreviewed_hunks(&action_log, cx), vec![( @@ -636,13 +612,11 @@ mod tests { vec![ HunkStatus { range: Point::new(1, 0)..Point::new(2, 0), - review_status: ReviewStatus::Unreviewed, diff_status: DiffHunkStatusKind::Modified, old_text: "def\n".into(), }, HunkStatus { range: Point::new(4, 0)..Point::new(4, 3), - review_status: ReviewStatus::Unreviewed, diff_status: DiffHunkStatusKind::Modified, old_text: "mno".into(), } @@ -651,84 +625,26 @@ mod tests { ); action_log.update(cx, |log, cx| { - log.review_edits_in_range(buffer.clone(), Point::new(3, 0)..Point::new(4, 3), true, cx) + log.keep_edits_in_range(buffer.clone(), Point::new(3, 0)..Point::new(4, 3), cx) }); cx.run_until_parked(); assert_eq!( unreviewed_hunks(&action_log, cx), vec![( buffer.clone(), - vec![ - HunkStatus { - range: Point::new(1, 0)..Point::new(2, 0), - review_status: ReviewStatus::Unreviewed, - diff_status: DiffHunkStatusKind::Modified, - old_text: "def\n".into(), - }, - HunkStatus { - range: Point::new(4, 0)..Point::new(4, 3), - review_status: ReviewStatus::Reviewed, - diff_status: DiffHunkStatusKind::Modified, - old_text: "mno".into(), - } - ], + vec![HunkStatus { + range: Point::new(1, 0)..Point::new(2, 0), + diff_status: DiffHunkStatusKind::Modified, + old_text: "def\n".into(), + }], )] ); action_log.update(cx, |log, cx| { - log.review_edits_in_range( - buffer.clone(), - Point::new(3, 0)..Point::new(4, 3), - false, - cx, - ) + log.keep_edits_in_range(buffer.clone(), Point::new(0, 0)..Point::new(4, 3), cx) }); cx.run_until_parked(); - assert_eq!( - unreviewed_hunks(&action_log, cx), - vec![( - buffer.clone(), - vec![ - HunkStatus { - range: Point::new(1, 0)..Point::new(2, 0), - review_status: ReviewStatus::Unreviewed, - diff_status: DiffHunkStatusKind::Modified, - old_text: "def\n".into(), - }, - HunkStatus { - range: Point::new(4, 0)..Point::new(4, 3), - review_status: ReviewStatus::Unreviewed, - diff_status: DiffHunkStatusKind::Modified, - old_text: "mno".into(), - } - ], - )] - ); - - action_log.update(cx, |log, cx| { - log.review_edits_in_range(buffer.clone(), Point::new(0, 0)..Point::new(4, 3), true, cx) - }); - cx.run_until_parked(); - assert_eq!( - unreviewed_hunks(&action_log, cx), - vec![( - buffer.clone(), - vec![ - HunkStatus { - range: Point::new(1, 0)..Point::new(2, 0), - review_status: ReviewStatus::Reviewed, - diff_status: DiffHunkStatusKind::Modified, - old_text: "def\n".into(), - }, - HunkStatus { - range: Point::new(4, 0)..Point::new(4, 3), - review_status: ReviewStatus::Reviewed, - diff_status: DiffHunkStatusKind::Modified, - old_text: "mno".into(), - } - ], - )] - ); + assert_eq!(unreviewed_hunks(&action_log, cx), vec![]); } #[gpui::test(iterations = 10)] @@ -736,85 +652,150 @@ mod tests { let action_log = cx.new(|_| ActionLog::new()); let buffer = cx.new(|cx| Buffer::local("abc\ndef\nghi\njkl\nmno", cx)); - let tool_edit = buffer.update(cx, |buffer, cx| { - buffer - .edit( - [(Point::new(0, 2)..Point::new(2, 3), "C\nDEF\nGHI")], - None, - cx, - ) - .unwrap() + cx.update(|cx| { + action_log.update(cx, |log, cx| log.buffer_read(buffer.clone(), cx)); + buffer.update(cx, |buffer, cx| { + buffer + .edit([(Point::new(1, 2)..Point::new(2, 3), "F\nGHI")], None, cx) + .unwrap() + }); + action_log.update(cx, |log, cx| log.buffer_edited(buffer.clone(), cx)); }); + cx.run_until_parked(); assert_eq!( buffer.read_with(cx, |buffer, _| buffer.text()), - "abC\nDEF\nGHI\njkl\nmno" + "abc\ndeF\nGHI\njkl\nmno" ); - - action_log.update(cx, |log, cx| { - log.buffer_edited(buffer.clone(), vec![tool_edit], cx) - }); - cx.run_until_parked(); assert_eq!( unreviewed_hunks(&action_log, cx), vec![( buffer.clone(), vec![HunkStatus { - range: Point::new(0, 0)..Point::new(3, 0), - review_status: ReviewStatus::Unreviewed, + range: Point::new(1, 0)..Point::new(3, 0), diff_status: DiffHunkStatusKind::Modified, - old_text: "abc\ndef\nghi\n".into(), - }], - )] - ); - - action_log.update(cx, |log, cx| { - log.review_edits_in_range(buffer.clone(), Point::new(0, 0)..Point::new(1, 0), true, cx) - }); - cx.run_until_parked(); - assert_eq!( - unreviewed_hunks(&action_log, cx), - vec![( - buffer.clone(), - vec![HunkStatus { - range: Point::new(0, 0)..Point::new(3, 0), - review_status: ReviewStatus::Reviewed, - diff_status: DiffHunkStatusKind::Modified, - old_text: "abc\ndef\nghi\n".into(), + old_text: "def\nghi\n".into(), }], )] ); buffer.update(cx, |buffer, cx| { - buffer.edit([(Point::new(0, 2)..Point::new(0, 2), "X")], None, cx) + buffer.edit( + [ + (Point::new(0, 2)..Point::new(0, 2), "X"), + (Point::new(3, 0)..Point::new(3, 0), "Y"), + ], + None, + cx, + ) }); cx.run_until_parked(); + assert_eq!( + buffer.read_with(cx, |buffer, _| buffer.text()), + "abXc\ndeF\nGHI\nYjkl\nmno" + ); assert_eq!( unreviewed_hunks(&action_log, cx), vec![( buffer.clone(), vec![HunkStatus { - range: Point::new(0, 0)..Point::new(3, 0), - review_status: ReviewStatus::Unreviewed, + range: Point::new(1, 0)..Point::new(3, 0), diff_status: DiffHunkStatusKind::Modified, - old_text: "abc\ndef\nghi\n".into(), + old_text: "def\nghi\n".into(), }], )] ); - action_log.update(cx, |log, cx| log.clear_reviewed_changes(cx)); + buffer.update(cx, |buffer, cx| { + buffer.edit([(Point::new(1, 1)..Point::new(1, 1), "Z")], None, cx) + }); + cx.run_until_parked(); + assert_eq!( + buffer.read_with(cx, |buffer, _| buffer.text()), + "abXc\ndZeF\nGHI\nYjkl\nmno" + ); + assert_eq!( + unreviewed_hunks(&action_log, cx), + vec![( + buffer.clone(), + vec![HunkStatus { + range: Point::new(1, 0)..Point::new(3, 0), + diff_status: DiffHunkStatusKind::Modified, + old_text: "def\nghi\n".into(), + }], + )] + ); + + action_log.update(cx, |log, cx| { + log.keep_edits_in_range(buffer.clone(), Point::new(0, 0)..Point::new(1, 0), cx) + }); + cx.run_until_parked(); + assert_eq!(unreviewed_hunks(&action_log, cx), vec![]); + } + + #[gpui::test(iterations = 10)] + async fn test_creation(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + language::init(cx); + Project::init_settings(cx); + }); + + let action_log = cx.new(|_| ActionLog::new()); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({})).await; + + let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await; + let file_path = project + .read_with(cx, |project, cx| project.find_project_path("dir/file1", cx)) + .unwrap(); + + // Simulate file2 being recreated by a tool. + let buffer = project + .update(cx, |project, cx| project.open_buffer(file_path, cx)) + .await + .unwrap(); + cx.update(|cx| { + buffer.update(cx, |buffer, cx| buffer.set_text("lorem", cx)); + action_log.update(cx, |log, cx| log.will_create_buffer(buffer.clone(), cx)); + }); + project + .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx)) + .await + .unwrap(); cx.run_until_parked(); assert_eq!( unreviewed_hunks(&action_log, cx), vec![( buffer.clone(), vec![HunkStatus { - range: Point::new(0, 0)..Point::new(3, 0), - review_status: ReviewStatus::Unreviewed, - diff_status: DiffHunkStatusKind::Modified, - old_text: "abc\ndef\nghi\n".into(), + range: Point::new(0, 0)..Point::new(0, 5), + diff_status: DiffHunkStatusKind::Added, + old_text: "".into(), }], )] ); + + buffer.update(cx, |buffer, cx| buffer.edit([(0..0, "X")], None, cx)); + cx.run_until_parked(); + assert_eq!( + unreviewed_hunks(&action_log, cx), + vec![( + buffer.clone(), + vec![HunkStatus { + range: Point::new(0, 0)..Point::new(0, 6), + diff_status: DiffHunkStatusKind::Added, + old_text: "".into(), + }], + )] + ); + + action_log.update(cx, |log, cx| { + log.keep_edits_in_range(buffer.clone(), 0..5, cx) + }); + cx.run_until_parked(); + assert_eq!(unreviewed_hunks(&action_log, cx), vec![]); } #[gpui::test(iterations = 10)] @@ -879,7 +860,6 @@ mod tests { buffer1.clone(), vec![HunkStatus { range: Point::new(0, 0)..Point::new(0, 0), - review_status: ReviewStatus::Unreviewed, diff_status: DiffHunkStatusKind::Deleted, old_text: "lorem\n".into(), }] @@ -888,7 +868,6 @@ mod tests { buffer2.clone(), vec![HunkStatus { range: Point::new(0, 0)..Point::new(0, 0), - review_status: ReviewStatus::Unreviewed, diff_status: DiffHunkStatusKind::Deleted, old_text: "ipsum\n".into(), }], @@ -899,20 +878,19 @@ mod tests { // Simulate file1 being recreated externally. fs.insert_file(path!("/dir/file1"), "LOREM".as_bytes().to_vec()) .await; + + // Simulate file2 being recreated by a tool. let buffer2 = project .update(cx, |project, cx| project.open_buffer(file2_path, cx)) .await .unwrap(); - cx.run_until_parked(); - // Simulate file2 being recreated by a tool. - let edit_id = buffer2.update(cx, |buffer, cx| buffer.set_text("IPSUM", cx)); - action_log.update(cx, |log, cx| { - log.will_create_buffer(buffer2.clone(), edit_id, cx) - }); + buffer2.update(cx, |buffer, cx| buffer.set_text("IPSUM", cx)); + action_log.update(cx, |log, cx| log.will_create_buffer(buffer2.clone(), cx)); project .update(cx, |project, cx| project.save_buffer(buffer2.clone(), cx)) .await .unwrap(); + cx.run_until_parked(); assert_eq!( unreviewed_hunks(&action_log, cx), @@ -920,7 +898,6 @@ mod tests { buffer2.clone(), vec![HunkStatus { range: Point::new(0, 0)..Point::new(0, 5), - review_status: ReviewStatus::Unreviewed, diff_status: DiffHunkStatusKind::Modified, old_text: "ipsum\n".into(), }], @@ -935,20 +912,162 @@ mod tests { assert_eq!(unreviewed_hunks(&action_log, cx), vec![]); } + #[gpui::test(iterations = 100)] + async fn test_random_diffs(mut rng: StdRng, cx: &mut TestAppContext) { + let operations = env::var("OPERATIONS") + .map(|i| i.parse().expect("invalid `OPERATIONS` variable")) + .unwrap_or(20); + + let action_log = cx.new(|_| ActionLog::new()); + let text = RandomCharIter::new(&mut rng).take(50).collect::(); + let buffer = cx.new(|cx| Buffer::local(text, cx)); + action_log.update(cx, |log, cx| log.buffer_read(buffer.clone(), cx)); + + for _ in 0..operations { + match rng.gen_range(0..100) { + 0..25 => { + action_log.update(cx, |log, cx| { + let range = buffer.read(cx).random_byte_range(0, &mut rng); + log::info!("keeping all edits in range {:?}", range); + log.keep_edits_in_range(buffer.clone(), range, cx) + }); + } + _ => { + let is_agent_change = rng.gen_bool(0.5); + if is_agent_change { + log::info!("agent edit"); + } else { + log::info!("user edit"); + } + cx.update(|cx| { + buffer.update(cx, |buffer, cx| buffer.randomly_edit(&mut rng, 1, cx)); + if is_agent_change { + action_log.update(cx, |log, cx| log.buffer_edited(buffer.clone(), cx)); + } + }); + } + } + + if rng.gen_bool(0.2) { + quiesce(&action_log, &buffer, cx); + } + } + + quiesce(&action_log, &buffer, cx); + + fn quiesce( + action_log: &Entity, + buffer: &Entity, + cx: &mut TestAppContext, + ) { + log::info!("quiescing..."); + cx.run_until_parked(); + action_log.update(cx, |log, cx| { + let tracked_buffer = log.track_buffer(buffer.clone(), false, cx); + let mut old_text = tracked_buffer.base_text.clone(); + let new_text = buffer.read(cx).as_rope(); + for edit in tracked_buffer.unreviewed_changes.edits() { + let old_start = old_text.point_to_offset(Point::new(edit.new.start, 0)); + let old_end = old_text.point_to_offset(cmp::min( + Point::new(edit.new.start + edit.old_len(), 0), + old_text.max_point(), + )); + old_text.replace( + old_start..old_end, + &new_text.slice_rows(edit.new.clone()).to_string(), + ); + } + pretty_assertions::assert_eq!(old_text.to_string(), new_text.to_string()); + }) + } + } + + #[gpui::test(iterations = 100)] + fn test_rebase_random(mut rng: StdRng) { + let operations = env::var("OPERATIONS") + .map(|i| i.parse().expect("invalid `OPERATIONS` variable")) + .unwrap_or(20); + + let mut next_line_id = 0; + let base_lines = (0..rng.gen_range(1..=20)) + .map(|_| post_inc(&mut next_line_id).to_string()) + .collect::>(); + log::info!("base lines: {:?}", base_lines); + + let (new_lines, patch_1) = + build_edits(&base_lines, operations, &mut rng, &mut next_line_id); + log::info!("agent edits: {:#?}", patch_1); + let (new_lines, patch_2) = build_edits(&new_lines, operations, &mut rng, &mut next_line_id); + log::info!("user edits: {:#?}", patch_2); + + let mut old_text = Rope::from(base_lines.join("\n")); + let new_text = Rope::from(new_lines.join("\n")); + let patch = rebase_patch(&patch_1, patch_2.into_inner(), &mut old_text, &new_text); + log::info!("rebased edits: {:#?}", patch.edits()); + + for edit in patch.edits() { + let old_start = old_text.point_to_offset(Point::new(edit.new.start, 0)); + let old_end = old_text.point_to_offset(cmp::min( + Point::new(edit.new.start + edit.old_len(), 0), + old_text.max_point(), + )); + old_text.replace( + old_start..old_end, + &new_text.slice_rows(edit.new.clone()).to_string(), + ); + } + pretty_assertions::assert_eq!(old_text.to_string(), new_text.to_string()); + } + + fn build_edits( + lines: &Vec, + count: usize, + rng: &mut StdRng, + next_line_id: &mut usize, + ) -> (Vec, Patch) { + let mut delta = 0i32; + let mut last_edit_end = 0; + let mut edits = Patch::default(); + let mut edited_lines = lines.clone(); + for _ in 0..count { + if last_edit_end >= lines.len() { + break; + } + + let end = rng.gen_range(last_edit_end..lines.len()); + let start = rng.gen_range(last_edit_end..=end); + let old_len = end - start; + + let mut new_len: usize = rng.gen_range(0..=3); + if start == end && new_len == 0 { + new_len += 1; + } + + last_edit_end = end + 1; + + let new_lines = (0..new_len) + .map(|_| post_inc(next_line_id).to_string()) + .collect::>(); + log::info!(" editing {:?}: {:?}", start..end, new_lines); + let old = start as u32..end as u32; + let new = (start as i32 + delta) as u32..(start as i32 + delta + new_len as i32) as u32; + edited_lines.splice( + new.start as usize..new.start as usize + old.len(), + new_lines, + ); + edits.push(Edit { old, new }); + delta += new_len as i32 - old_len as i32; + } + (edited_lines, edits) + } + #[derive(Debug, Clone, PartialEq, Eq)] struct HunkStatus { range: Range, - review_status: ReviewStatus, diff_status: DiffHunkStatusKind, old_text: String, } - #[derive(Copy, Clone, Debug, PartialEq, Eq)] - enum ReviewStatus { - Unreviewed, - Reviewed, - } - fn unreviewed_hunks( action_log: &Entity, cx: &TestAppContext, @@ -958,24 +1077,16 @@ mod tests { .read(cx) .changed_buffers(cx) .into_iter() - .map(|(buffer, tracked_buffer)| { + .map(|(buffer, diff)| { let snapshot = buffer.read(cx).snapshot(); ( buffer, - tracked_buffer - .diff - .read(cx) + diff.read(cx) .hunks(&snapshot, cx) .map(|hunk| HunkStatus { - review_status: if hunk.status().has_secondary_hunk() { - ReviewStatus::Unreviewed - } else { - ReviewStatus::Reviewed - }, diff_status: hunk.status().kind, range: hunk.range, - old_text: tracked_buffer - .diff + old_text: diff .read(cx) .base_text() .text_for_range(hunk.diff_base_byte_range) diff --git a/crates/assistant_tools/Cargo.toml b/crates/assistant_tools/Cargo.toml index a2f4df4cb0..55cb7566a5 100644 --- a/crates/assistant_tools/Cargo.toml +++ b/crates/assistant_tools/Cargo.toml @@ -14,7 +14,6 @@ path = "src/assistant_tools.rs" [dependencies] anyhow.workspace = true assistant_tool.workspace = true -clock.workspace = true chrono.workspace = true collections.workspace = true feature_flags.workspace = true @@ -42,7 +41,6 @@ worktree.workspace = true open = { workspace = true } [dev-dependencies] -clock = { workspace = true, features = ["test-support"] } collections = { workspace = true, features = ["test-support"] } gpui = { workspace = true, features = ["test-support"] } language = { workspace = true, features = ["test-support"] } diff --git a/crates/assistant_tools/src/create_file_tool.rs b/crates/assistant_tools/src/create_file_tool.rs index d4f0f77e64..64dfbdbfbd 100644 --- a/crates/assistant_tools/src/create_file_tool.rs +++ b/crates/assistant_tools/src/create_file_tool.rs @@ -92,10 +92,11 @@ impl Tool for CreateFileTool { })? .await .map_err(|err| anyhow!("Unable to open buffer for {destination_path}: {err}"))?; - let edit_id = buffer.update(cx, |buffer, cx| buffer.set_text(contents, cx))?; - - action_log.update(cx, |action_log, cx| { - action_log.will_create_buffer(buffer.clone(), edit_id, cx) + cx.update(|cx| { + buffer.update(cx, |buffer, cx| buffer.set_text(contents, cx)); + action_log.update(cx, |action_log, cx| { + action_log.will_create_buffer(buffer.clone(), cx) + }); })?; project diff --git a/crates/assistant_tools/src/edit_files_tool.rs b/crates/assistant_tools/src/edit_files_tool.rs index 579bd6e9b3..5470045918 100644 --- a/crates/assistant_tools/src/edit_files_tool.rs +++ b/crates/assistant_tools/src/edit_files_tool.rs @@ -174,7 +174,6 @@ enum EditorResponse { struct AppliedAction { source: String, buffer: Entity, - edit_ids: Vec, } #[derive(Debug)] @@ -339,18 +338,17 @@ impl EditToolRequest { self.push_search_error(error); } DiffResult::Diff(diff) => { - let edit_ids = buffer.update(cx, |buffer, cx| { - buffer.finalize_last_transaction(); - buffer.apply_diff(diff, false, cx); - let transaction = buffer.finalize_last_transaction(); - transaction.map_or(Vec::new(), |transaction| transaction.edit_ids.clone()) + cx.update(|cx| { + buffer.update(cx, |buffer, cx| { + buffer.finalize_last_transaction(); + buffer.apply_diff(diff, cx); + buffer.finalize_last_transaction(); + }); + self.action_log + .update(cx, |log, cx| log.buffer_edited(buffer.clone(), cx)); })?; - self.push_applied_action(AppliedAction { - source, - buffer, - edit_ids, - }); + self.push_applied_action(AppliedAction { source, buffer }); } } @@ -473,9 +471,6 @@ impl EditToolRequest { for action in applied { changed_buffers.insert(action.buffer.clone()); - self.action_log.update(cx, |log, cx| { - log.buffer_edited(action.buffer, action.edit_ids, cx) - })?; write!(&mut output, "\n\n{}", action.source)?; } diff --git a/crates/assistant_tools/src/find_replace_file_tool.rs b/crates/assistant_tools/src/find_replace_file_tool.rs index da2e5e155e..fb05170bad 100644 --- a/crates/assistant_tools/src/find_replace_file_tool.rs +++ b/crates/assistant_tools/src/find_replace_file_tool.rs @@ -1,7 +1,7 @@ use crate::{replace::replace_with_flexible_indent, schema::json_schema_for}; use anyhow::{Context as _, Result, anyhow}; use assistant_tool::{ActionLog, Tool}; -use gpui::{App, AppContext, Entity, Task}; +use gpui::{App, AppContext, AsyncApp, Entity, Task}; use language_model::{LanguageModelRequestMessage, LanguageModelToolSchemaFormat}; use project::Project; use schemars::JsonSchema; @@ -165,7 +165,7 @@ impl Tool for FindReplaceFileTool { Err(err) => return Task::ready(Err(anyhow!(err))), }; - cx.spawn(async move |cx| { + cx.spawn(async move |cx: &mut AsyncApp| { let project_path = project.read_with(cx, |project, cx| { project .find_project_path(&input.path, cx) @@ -225,20 +225,18 @@ impl Tool for FindReplaceFileTool { return Err(err) }; - let (edit_ids, snapshot) = buffer.update(cx, |buffer, cx| { + let snapshot = buffer.update(cx, |buffer, cx| { buffer.finalize_last_transaction(); - buffer.apply_diff(diff, false, cx); - let transaction = buffer.finalize_last_transaction(); - let edit_ids = transaction.map_or(Vec::new(), |transaction| transaction.edit_ids.clone()); - - (edit_ids, buffer.snapshot()) + buffer.apply_diff(diff, cx); + buffer.finalize_last_transaction(); + buffer.snapshot() })?; action_log.update(cx, |log, cx| { - log.buffer_edited(buffer.clone(), edit_ids, cx) + log.buffer_edited(buffer.clone(), cx) })?; - project.update(cx, |project, cx| { + project.update( cx, |project, cx| { project.save_buffer(buffer, cx) })?.await?; @@ -249,6 +247,7 @@ impl Tool for FindReplaceFileTool { Ok(format!("Edited {}:\n\n```diff\n{}\n```", input.path.display(), diff_str)) + }) } } diff --git a/crates/assistant_tools/src/replace.rs b/crates/assistant_tools/src/replace.rs index bbfadf981e..17243b4a12 100644 --- a/crates/assistant_tools/src/replace.rs +++ b/crates/assistant_tools/src/replace.rs @@ -518,7 +518,7 @@ mod tests { // Call replace_flexible and transform the result replace_with_flexible_indent(old, new, &buffer_snapshot).map(|diff| { buffer.update(cx, |buffer, cx| { - let _ = buffer.apply_diff(diff, false, cx); + let _ = buffer.apply_diff(diff, cx); buffer.text() }) }) diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index ebd9ea411f..17f853589d 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -377,7 +377,6 @@ impl RealGitRepository { #[derive(Clone, Debug)] pub struct GitRepositoryCheckpoint { ref_name: String, - head_sha: Option, commit_sha: Oid, } @@ -1213,11 +1212,6 @@ impl GitRepository for RealGitRepository { Ok(GitRepositoryCheckpoint { ref_name, - head_sha: if let Some(head_sha) = head_sha { - Some(head_sha.parse()?) - } else { - None - }, commit_sha: checkpoint_sha.parse()?, }) }) @@ -1252,13 +1246,6 @@ impl GitRepository for RealGitRepository { }) .await?; - if let Some(head_sha) = checkpoint.head_sha { - git.run(&["reset", "--mixed", &head_sha.to_string()]) - .await?; - } else { - git.run(&["update-ref", "-d", "HEAD"]).await?; - } - Ok(()) }) .boxed() @@ -1269,10 +1256,6 @@ impl GitRepository for RealGitRepository { left: GitRepositoryCheckpoint, right: GitRepositoryCheckpoint, ) -> BoxFuture> { - if left.head_sha != right.head_sha { - return future::ready(Ok(false)).boxed(); - } - let working_directory = self.working_directory(); let git_binary_path = self.git_binary_path.clone(); @@ -1768,7 +1751,6 @@ fn checkpoint_author_envs() -> HashMap { #[cfg(test)] mod tests { use super::*; - use crate::status::FileStatus; use gpui::TestAppContext; #[gpui::test] @@ -1803,7 +1785,6 @@ mod tests { smol::fs::write(repo_dir.path().join("new_file_before_checkpoint"), "1") .await .unwrap(); - let sha_before_checkpoint = repo.head_sha().unwrap(); let checkpoint = repo.checkpoint().await.unwrap(); // Ensure the user can't see any branches after creating a checkpoint. @@ -1837,7 +1818,6 @@ mod tests { repo.gc().await.unwrap(); repo.restore_checkpoint(checkpoint.clone()).await.unwrap(); - assert_eq!(repo.head_sha().unwrap(), sha_before_checkpoint); assert_eq!( smol::fs::read_to_string(&file_path).await.unwrap(), "modified before checkpoint" @@ -1901,83 +1881,6 @@ mod tests { ); } - #[gpui::test] - async fn test_undoing_commit_via_checkpoint(cx: &mut TestAppContext) { - cx.executor().allow_parking(); - - let repo_dir = tempfile::tempdir().unwrap(); - - git2::Repository::init(repo_dir.path()).unwrap(); - let file_path = repo_dir.path().join("file"); - smol::fs::write(&file_path, "initial").await.unwrap(); - - let repo = - RealGitRepository::new(&repo_dir.path().join(".git"), None, cx.executor()).unwrap(); - repo.stage_paths( - vec![RepoPath::from_str("file")], - Arc::new(HashMap::default()), - ) - .await - .unwrap(); - repo.commit( - "Initial commit".into(), - None, - Arc::new(checkpoint_author_envs()), - ) - .await - .unwrap(); - - let initial_commit_sha = repo.head_sha().unwrap(); - - smol::fs::write(repo_dir.path().join("new_file1"), "content1") - .await - .unwrap(); - smol::fs::write(repo_dir.path().join("new_file2"), "content2") - .await - .unwrap(); - - let checkpoint = repo.checkpoint().await.unwrap(); - - repo.stage_paths( - vec![ - RepoPath::from_str("new_file1"), - RepoPath::from_str("new_file2"), - ], - Arc::new(HashMap::default()), - ) - .await - .unwrap(); - repo.commit( - "Commit new files".into(), - None, - Arc::new(checkpoint_author_envs()), - ) - .await - .unwrap(); - - repo.restore_checkpoint(checkpoint).await.unwrap(); - assert_eq!(repo.head_sha().unwrap(), initial_commit_sha); - assert_eq!( - smol::fs::read_to_string(repo_dir.path().join("new_file1")) - .await - .unwrap(), - "content1" - ); - assert_eq!( - smol::fs::read_to_string(repo_dir.path().join("new_file2")) - .await - .unwrap(), - "content2" - ); - assert_eq!( - repo.status(&[]).await.unwrap().entries.as_ref(), - &[ - (RepoPath::from_str("new_file1"), FileStatus::Untracked), - (RepoPath::from_str("new_file2"), FileStatus::Untracked) - ] - ); - } - #[gpui::test] async fn test_compare_checkpoints(cx: &mut TestAppContext) { cx.executor().allow_parking(); diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 8ec6a9c47e..8761d2adbc 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -1321,7 +1321,7 @@ impl Buffer { this.update(cx, |this, cx| { if this.version() == diff.base_version { this.finalize_last_transaction(); - this.apply_diff(diff, true, cx); + this.apply_diff(diff, cx); tx.send(this.finalize_last_transaction().cloned()).ok(); this.has_conflict = false; this.did_reload(this.version(), this.line_ending(), new_mtime, cx); @@ -1882,14 +1882,7 @@ impl Buffer { /// Applies a diff to the buffer. If the buffer has changed since the given diff was /// calculated, then adjust the diff to account for those changes, and discard any /// parts of the diff that conflict with those changes. - /// - /// If `atomic` is true, the diff will be applied as a single edit. - pub fn apply_diff( - &mut self, - diff: Diff, - atomic: bool, - cx: &mut Context, - ) -> Option { + pub fn apply_diff(&mut self, diff: Diff, cx: &mut Context) -> Option { let snapshot = self.snapshot(); let mut edits_since = snapshot.edits_since::(&diff.base_version).peekable(); let mut delta = 0; @@ -1919,17 +1912,7 @@ impl Buffer { self.start_transaction(); self.text.set_line_ending(diff.line_ending); - if atomic { - self.edit(adjusted_edits, None, cx); - } else { - let mut delta = 0isize; - for (range, new_text) in adjusted_edits { - let adjusted_range = - (range.start as isize + delta) as usize..(range.end as isize + delta) as usize; - delta += new_text.len() as isize - range.len() as isize; - self.edit([(adjusted_range, new_text)], None, cx); - } - } + self.edit(adjusted_edits, None, cx); self.end_transaction(cx) } diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 6f0f25a897..76316b2640 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -376,7 +376,7 @@ async fn test_apply_diff(cx: &mut TestAppContext) { let diff = buffer.update(cx, |b, cx| b.diff(text.clone(), cx)).await; buffer.update(cx, |buffer, cx| { - buffer.apply_diff(diff, true, cx).unwrap(); + buffer.apply_diff(diff, cx).unwrap(); assert_eq!(buffer.text(), text); let actual_offsets = anchors .iter() @@ -390,7 +390,7 @@ async fn test_apply_diff(cx: &mut TestAppContext) { let diff = buffer.update(cx, |b, cx| b.diff(text.clone(), cx)).await; buffer.update(cx, |buffer, cx| { - buffer.apply_diff(diff, true, cx).unwrap(); + buffer.apply_diff(diff, cx).unwrap(); assert_eq!(buffer.text(), text); let actual_offsets = anchors .iter() @@ -435,7 +435,7 @@ async fn test_normalize_whitespace(cx: &mut gpui::TestAppContext) { let format_diff = format.await; buffer.update(cx, |buffer, cx| { let version_before_format = format_diff.base_version.clone(); - buffer.apply_diff(format_diff, true, cx); + buffer.apply_diff(format_diff, cx); // The outcome depends on the order of concurrent tasks. // diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 424163b346..362d5d57b3 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1230,7 +1230,7 @@ impl LocalLspStore { .await; buffer.handle.update(cx, |buffer, cx| { buffer.start_transaction(); - buffer.apply_diff(diff, true, cx); + buffer.apply_diff(diff, cx); transaction_id_format = transaction_id_format.or(buffer.end_transaction(cx)); if let Some(transaction_id) = transaction_id_format { @@ -1364,7 +1364,7 @@ impl LocalLspStore { zlog::trace!(logger => "Applying changes"); buffer.handle.update(cx, |buffer, cx| { buffer.start_transaction(); - buffer.apply_diff(diff, true, cx); + buffer.apply_diff(diff, cx); transaction_id_format = transaction_id_format.or(buffer.end_transaction(cx)); if let Some(transaction_id) = transaction_id_format { @@ -1407,7 +1407,7 @@ impl LocalLspStore { zlog::trace!(logger => "Applying changes"); buffer.handle.update(cx, |buffer, cx| { buffer.start_transaction(); - buffer.apply_diff(diff, true, cx); + buffer.apply_diff(diff, cx); transaction_id_format = transaction_id_format.or(buffer.end_transaction(cx)); if let Some(transaction_id) = transaction_id_format { diff --git a/crates/text/src/patch.rs b/crates/text/src/patch.rs index ff1d0c2e9a..96fed17571 100644 --- a/crates/text/src/patch.rs +++ b/crates/text/src/patch.rs @@ -224,6 +224,15 @@ where } } +impl Patch { + pub fn retain_mut(&mut self, f: F) + where + F: FnMut(&mut Edit) -> bool, + { + self.0.retain_mut(f); + } +} + impl IntoIterator for Patch { type Item = Edit; type IntoIter = std::vec::IntoIter>;