From 037cf1393c6718bb56b1fd8117832a457355cc6f Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Mon, 19 Aug 2024 18:42:49 +0200 Subject: [PATCH] assistant: Undo workflow step when buffer is discarded (#16465) This fixes a weird bug: 1. Use `/workflow` in assistant 2. Have it generate a step that modifies a file 3. Either (a) select the step in the assistant and have it auto-insert newlines (b) select "Transform" to have the step applied 4. Close the modified file in the editor ("Discard") 5. Re-open the file 6. BUG: the changes made by assistant are still there! The reason for the bug is that the assistant keeps references to buffers and they're not closed/reloaded when closed/reopened. To fix the bug we now rollback the applied workflow steps when discarding a buffer. (This does *not* yet fix the issue where a workflow step inserts a new buffer into the project/worktree that does not show up on the file system yet but in `/file` and hangs around until Zed is closed.) Release Notes: - N/A Co-authored-by: Bennet --- crates/assistant/src/assistant_panel.rs | 12 ++++++++++++ crates/editor/src/items.rs | 6 ++++++ crates/language/src/buffer.rs | 8 ++++++++ crates/multi_buffer/src/multi_buffer.rs | 2 ++ crates/workspace/src/item.rs | 6 ++++++ crates/workspace/src/pane.rs | 9 +++++++-- 6 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/assistant/src/assistant_panel.rs b/crates/assistant/src/assistant_panel.rs index 8e6a7d364e8301c99239818c515b693a02c411b0..aa1044c7eea83918f264c1399536ac6d29ad382e 100644 --- a/crates/assistant/src/assistant_panel.rs +++ b/crates/assistant/src/assistant_panel.rs @@ -2479,6 +2479,18 @@ impl ContextEditor { }; let resolved_step = step.read(cx).resolution.clone(); + + if let Some(Ok(resolution)) = resolved_step.as_ref() { + for (buffer, _) in resolution.suggestion_groups.iter() { + let step_range = step_range.clone(); + cx.subscribe(buffer, move |this, _, event, cx| match event { + language::Event::Discarded => this.undo_workflow_step(step_range.clone(), cx), + _ => {} + }) + .detach(); + } + } + if let Some(existing_step) = self.workflow_steps.get_mut(&step_range) { existing_step.resolved_step = resolved_step; } else { diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 696e4b1fe3004786401c5c2c0e67fa2989f87e7c..64a94581e34feb0cd8ad3124ad6c5fe063d17272 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -680,6 +680,12 @@ impl Item for Editor { self.nav_history = Some(history); } + fn discarded(&self, _project: Model, cx: &mut ViewContext) { + for buffer in self.buffer().clone().read(cx).all_buffers() { + buffer.update(cx, |buffer, cx| buffer.discarded(cx)) + } + } + fn deactivated(&mut self, cx: &mut ViewContext) { let selection = self.selections.newest_anchor(); self.push_to_nav_history(selection.head(), None, cx); diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 3ee2d6ca55532c411dbd80c383e1e351c9053644..2aac8c3c39d6940052cd343f7f5dbe7e079da74e 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -332,6 +332,8 @@ pub enum Event { CapabilityChanged, /// The buffer was explicitly requested to close. Closed, + /// The buffer was discarded when closing. + Discarded, } /// The file associated with a buffer. @@ -827,6 +829,12 @@ impl Buffer { cx.notify(); } + /// This method is called to signal that the buffer has been discarded. + pub fn discarded(&mut self, cx: &mut ModelContext) { + cx.emit(Event::Discarded); + cx.notify(); + } + /// Reloads the contents of the buffer from disk. pub fn reload( &mut self, diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index a734f5c8625f2032bbc784cffbd9b43771670eb7..eb0bc0546c065a67451a11efabdf9f391649330d 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -106,6 +106,7 @@ pub enum Event { Saved, FileHandleChanged, Closed, + Discarded, DirtyChanged, DiagnosticsUpdated, } @@ -1691,6 +1692,7 @@ impl MultiBuffer { language::Event::Reparsed => Event::Reparsed(buffer.read(cx).remote_id()), language::Event::DiagnosticsUpdated => Event::DiagnosticsUpdated, language::Event::Closed => Event::Closed, + language::Event::Discarded => Event::Discarded, language::Event::CapabilityChanged => { self.capability = buffer.read(cx).capability(); Event::CapabilityChanged diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index 14535a72fbb909a7ffe39debbe816b567b5c7fbd..b06d8c435fd8c7f187f1d75c77ffdb13a06f7fd9 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -184,6 +184,7 @@ pub trait Item: FocusableView + EventEmitter { fn to_item_events(_event: &Self::Event, _f: impl FnMut(ItemEvent)) {} fn deactivated(&mut self, _: &mut ViewContext) {} + fn discarded(&self, _project: Model, _cx: &mut ViewContext) {} fn workspace_deactivated(&mut self, _: &mut ViewContext) {} fn navigate(&mut self, _: Box, _: &mut ViewContext) -> bool { false @@ -394,6 +395,7 @@ pub trait ItemHandle: 'static + Send { cx: &mut ViewContext, ); fn deactivated(&self, cx: &mut WindowContext); + fn discarded(&self, project: Model, cx: &mut WindowContext); fn workspace_deactivated(&self, cx: &mut WindowContext); fn navigate(&self, data: Box, cx: &mut WindowContext) -> bool; fn item_id(&self) -> EntityId; @@ -735,6 +737,10 @@ impl ItemHandle for View { }); } + fn discarded(&self, project: Model, cx: &mut WindowContext) { + self.update(cx, |this, cx| this.discarded(project, cx)); + } + fn deactivated(&self, cx: &mut WindowContext) { self.update(cx, |this, cx| this.deactivated(cx)); } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 393a47d13477d927d7b88a0b6e1489c3ffb7283c..c6d1241a4b523f06c7cbc143f4ef5dc2d89c3bfd 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1500,8 +1500,13 @@ impl Pane { })?; match answer { Ok(0) => {} - Ok(1) => return Ok(true), // Don't save this file - _ => return Ok(false), // Cancel + Ok(1) => { + // Don't save this file + pane.update(cx, |_, cx| item.discarded(project, cx)) + .log_err(); + return Ok(true); + } + _ => return Ok(false), // Cancel } } else { return Ok(false);