Fix crash due to mutating command palette's candidates on confirmation (#3575)

Max Brunsfeld created 2 years ago

I was seeing a crash when confirming the command palette. It was caused
by getting the palette's `commands` (match candidates) and `matches`
getting out of sync because we mutated `commands` when removing the
selected command.

Change summary

crates/command_palette2/src/command_palette.rs | 22 +++++++------------
1 file changed, 8 insertions(+), 14 deletions(-)

Detailed changes

crates/command_palette2/src/command_palette.rs

@@ -7,7 +7,7 @@ use collections::{CommandPaletteFilter, HashMap};
 use fuzzy::{StringMatch, StringMatchCandidate};
 use gpui::{
     actions, Action, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, FocusableView,
-    Keystroke, ParentElement, Render, Styled, View, ViewContext, VisualContext, WeakView,
+    ParentElement, Render, Styled, View, ViewContext, VisualContext, WeakView,
 };
 use picker::{Picker, PickerDelegate};
 
@@ -61,7 +61,6 @@ impl CommandPalette {
                 Some(Command {
                     name: humanize_action_name(&name),
                     action,
-                    keystrokes: vec![], // todo!()
                 })
             })
             .collect();
@@ -110,7 +109,6 @@ pub struct CommandPaletteDelegate {
 struct Command {
     name: String,
     action: Box<dyn Action>,
-    keystrokes: Vec<Keystroke>,
 }
 
 impl Clone for Command {
@@ -118,7 +116,6 @@ impl Clone for Command {
         Self {
             name: self.name.clone(),
             action: self.action.boxed_clone(),
-            keystrokes: self.keystrokes.clone(),
         }
     }
 }
@@ -229,6 +226,7 @@ impl PickerDelegate for CommandPaletteDelegate {
                     })
                 }
             }
+
             if let Some(CommandInterceptResult {
                 action,
                 string,
@@ -244,7 +242,6 @@ impl PickerDelegate for CommandPaletteDelegate {
                 commands.push(Command {
                     name: string.clone(),
                     action,
-                    keystrokes: vec![],
                 });
                 matches.insert(
                     0,
@@ -256,6 +253,7 @@ impl PickerDelegate for CommandPaletteDelegate {
                     },
                 )
             }
+
             picker
                 .update(&mut cx, |picker, _| {
                     let delegate = &mut picker.delegate;
@@ -285,6 +283,8 @@ impl PickerDelegate for CommandPaletteDelegate {
         }
         let action_ix = self.matches[self.selected_ix].candidate_id;
         let command = self.commands.swap_remove(action_ix);
+        self.matches.clear();
+        self.commands.clear();
         cx.update_global(|hit_counts: &mut HitCounts, _| {
             *hit_counts.0.entry(command.name).or_default() += 1;
         });
@@ -300,13 +300,8 @@ impl PickerDelegate for CommandPaletteDelegate {
         selected: bool,
         cx: &mut ViewContext<Picker<Self>>,
     ) -> Option<Self::ListItem> {
-        let Some(r#match) = self.matches.get(ix) else {
-            return None;
-        };
-        let Some(command) = self.commands.get(r#match.candidate_id) else {
-            return None;
-        };
-
+        let r#match = self.matches.get(ix)?;
+        let command = self.commands.get(r#match.candidate_id)?;
         Some(
             ListItem::new(ix).inset(true).selected(selected).child(
                 h_stack()
@@ -354,8 +349,7 @@ impl std::fmt::Debug for Command {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("Command")
             .field("name", &self.name)
-            .field("keystrokes", &self.keystrokes)
-            .finish()
+            .finish_non_exhaustive()
     }
 }