Fix double lease panic in Quote Selection command (#12217)

Max Brunsfeld created 2 years ago

Release Notes:

- Fixed a panic that occurred when using the `assistant: quote
selection` command while signed out.

Change summary

crates/assistant/src/assistant_panel.rs | 58 +++++++-------------------
crates/gpui/src/app/entity_map.rs       | 22 ++++++---
2 files changed, 31 insertions(+), 49 deletions(-)

Detailed changes

crates/assistant/src/assistant_panel.rs

@@ -3271,52 +3271,26 @@ impl ConversationEditor {
         }
 
         if let Some(text) = text {
-            panel.update(cx, |panel, cx| {
-                if let Some(conversation) = panel
-                    .active_conversation_editor()
-                    .cloned()
-                    .or_else(|| panel.new_conversation(cx))
-                {
-                    conversation.update(cx, |conversation, cx| {
-                        conversation
-                            .editor
-                            .update(cx, |editor, cx| editor.insert(&text, cx))
-                    });
-                };
+            panel.update(cx, |_, cx| {
+                // Wait to create a new conversation until the workspace is no longer
+                // being updated.
+                cx.defer(move |panel, cx| {
+                    if let Some(conversation) = panel
+                        .active_conversation_editor()
+                        .cloned()
+                        .or_else(|| panel.new_conversation(cx))
+                    {
+                        conversation.update(cx, |conversation, cx| {
+                            conversation
+                                .editor
+                                .update(cx, |editor, cx| editor.insert(&text, cx))
+                        });
+                    };
+                });
             });
         }
     }
 
-    // fn insert_active_prompt(
-    //     workspace: &mut Workspace,
-    //     _: &InsertActivePrompt,
-    //     cx: &mut ViewContext<Workspace>,
-    // ) {
-    //     let Some(panel) = workspace.panel::<AssistantPanel>(cx) else {
-    //         return;
-    //     };
-
-    //     if !panel.focus_handle(cx).contains_focused(cx) {
-    //         workspace.toggle_panel_focus::<AssistantPanel>(cx);
-    //     }
-
-    //     if let Some(default_prompt) = panel.read(cx).prompt_library.clone().default_prompt() {
-    //         panel.update(cx, |panel, cx| {
-    //             if let Some(conversation) = panel
-    //                 .active_conversation_editor()
-    //                 .cloned()
-    //                 .or_else(|| panel.new_conversation(cx))
-    //             {
-    //                 conversation.update(cx, |conversation, cx| {
-    //                     conversation
-    //                         .editor
-    //                         .update(cx, |editor, cx| editor.insert(&default_prompt, cx))
-    //                 });
-    //             };
-    //         });
-    //     };
-    // }
-
     fn copy(&mut self, _: &editor::actions::Copy, cx: &mut ViewContext<Self>) {
         let editor = self.editor.read(cx);
         let conversation = self.conversation.read(cx);

crates/gpui/src/app/entity_map.rs

@@ -97,12 +97,11 @@ impl EntityMap {
     #[track_caller]
     pub fn lease<'a, T>(&mut self, model: &'a Model<T>) -> Lease<'a, T> {
         self.assert_valid_context(model);
-        let entity = Some(self.entities.remove(model.entity_id).unwrap_or_else(|| {
-            panic!(
-                "Circular entity lease of {}. Is it already being updated?",
-                std::any::type_name::<T>()
-            )
-        }));
+        let entity = Some(
+            self.entities
+                .remove(model.entity_id)
+                .unwrap_or_else(|| double_lease_panic::<T>("update")),
+        );
         Lease {
             model,
             entity,
@@ -118,7 +117,9 @@ impl EntityMap {
 
     pub fn read<T: 'static>(&self, model: &Model<T>) -> &T {
         self.assert_valid_context(model);
-        self.entities[model.entity_id].downcast_ref().unwrap()
+        self.entities[model.entity_id]
+            .downcast_ref()
+            .unwrap_or_else(|| double_lease_panic::<T>("read"))
     }
 
     fn assert_valid_context(&self, model: &AnyModel) {
@@ -149,6 +150,13 @@ impl EntityMap {
     }
 }
 
+fn double_lease_panic<T>(operation: &str) -> ! {
+    panic!(
+        "cannot {operation} {} while it is already being updated",
+        std::any::type_name::<T>()
+    )
+}
+
 pub(crate) struct Lease<'a, T> {
     entity: Option<Box<dyn Any>>,
     pub model: &'a Model<T>,