gpui: Prevent the same action name from being registered multiple times (#33359)

Michael Sloan created 5 months ago

Also removes duplicate `editor::RevertFile` and `vim::HelixDelete`
actions

Release Notes:

- N/A

Change summary

crates/editor/src/actions.rs      |  1 -
crates/gpui/src/action.rs         | 26 ++++++++++++++++++++------
crates/gpui/src/keymap/context.rs |  2 +-
crates/vim/src/helix.rs           | 25 +------------------------
4 files changed, 22 insertions(+), 32 deletions(-)

Detailed changes

crates/editor/src/actions.rs

@@ -388,7 +388,6 @@ actions!(
         RestartLanguageServer,
         RevealInFileManager,
         ReverseLines,
-        RevertFile,
         ReloadFile,
         Rewrap,
         RunFlycheck,

crates/gpui/src/action.rs

@@ -48,6 +48,8 @@ macro_rules! actions {
 /// actions!(editor, [MoveUp, MoveDown, MoveLeft, MoveRight, Newline]);
 /// ```
 ///
+/// Registering the actions with the same name will result in a panic during  `App` creation.
+///
 /// # Derive Macro
 ///
 /// More complex data types can also be actions, by using the derive macro for `Action`:
@@ -280,14 +282,27 @@ impl ActionRegistry {
     }
 
     fn insert_action(&mut self, action: MacroActionData) {
+        let name = action.name;
+        if self.by_name.contains_key(name) {
+            panic!(
+                "Action with name `{name}` already registered \
+                (might be registered in `#[action(deprecated_aliases = [...])]`."
+            );
+        }
         self.by_name.insert(
-            action.name,
+            name,
             ActionData {
                 build: action.build,
                 json_schema: action.json_schema,
             },
         );
         for &alias in action.deprecated_aliases {
+            if self.by_name.contains_key(alias) {
+                panic!(
+                    "Action with name `{alias}` already registered. \
+                    `{alias}` is specified in `#[action(deprecated_aliases = [...])]` for action `{name}`."
+                );
+            }
             self.by_name.insert(
                 alias,
                 ActionData {
@@ -295,14 +310,13 @@ impl ActionRegistry {
                     json_schema: action.json_schema,
                 },
             );
-            self.deprecated_aliases.insert(alias, action.name);
+            self.deprecated_aliases.insert(alias, name);
             self.all_names.push(alias);
         }
-        self.names_by_type_id.insert(action.type_id, action.name);
-        self.all_names.push(action.name);
+        self.names_by_type_id.insert(action.type_id, name);
+        self.all_names.push(name);
         if let Some(deprecation_msg) = action.deprecation_message {
-            self.deprecation_messages
-                .insert(action.name, deprecation_msg);
+            self.deprecation_messages.insert(name, deprecation_msg);
         }
     }
 

crates/gpui/src/keymap/context.rs

@@ -432,7 +432,7 @@ mod tests {
             actions!(
                 test_only,
                 [
-                    A, B, C, D, E, F, G, // Don't wrap, test the trailing comma
+                    H, I, J, K, L, M, N, // Don't wrap, test the trailing comma
                 ]
             );
         }

crates/vim/src/helix.rs

@@ -3,14 +3,12 @@ use gpui::{Action, actions};
 use gpui::{Context, Window};
 use language::{CharClassifier, CharKind};
 
-use crate::motion::MotionKind;
 use crate::{Vim, motion::Motion, state::Mode};
 
-actions!(vim, [HelixNormalAfter, HelixDelete]);
+actions!(vim, [HelixNormalAfter]);
 
 pub fn register(editor: &mut Editor, cx: &mut Context<Vim>) {
     Vim::action(editor, cx, Vim::helix_normal_after);
-    Vim::action(editor, cx, Vim::helix_delete);
 }
 
 impl Vim {
@@ -292,27 +290,6 @@ impl Vim {
             _ => self.helix_move_and_collapse(motion, times, window, cx),
         }
     }
-
-    pub fn helix_delete(&mut self, _: &HelixDelete, window: &mut Window, cx: &mut Context<Self>) {
-        self.store_visual_marks(window, cx);
-        self.update_editor(window, cx, |vim, editor, window, cx| {
-            // Fixup selections so they have helix's semantics.
-            // Specifically:
-            //  - Make sure that each cursor acts as a 1 character wide selection
-            editor.transact(window, cx, |editor, window, cx| {
-                editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| {
-                    s.move_with(|map, selection| {
-                        if selection.is_empty() && !selection.reversed {
-                            selection.end = movement::right(map, selection.end);
-                        }
-                    });
-                });
-            });
-
-            vim.copy_selections_content(editor, MotionKind::Exclusive, window, cx);
-            editor.insert("", window, cx);
-        });
-    }
 }
 
 #[cfg(test)]