Remove internal actions from `link_go_to_definition`

Antonio Scandurra created

Change summary

crates/editor/src/editor.rs                |   1 
crates/editor/src/element.rs               |  40 +++---
crates/editor/src/link_go_to_definition.rs | 136 +++---------------------
3 files changed, 37 insertions(+), 140 deletions(-)

Detailed changes

crates/editor/src/editor.rs ๐Ÿ”—

@@ -386,7 +386,6 @@ pub fn init(cx: &mut AppContext) {
     cx.add_action(Editor::copilot_suggest);
 
     hover_popover::init(cx);
-    link_go_to_definition::init(cx);
     mouse_context_menu::init(cx);
     scroll::actions::init(cx);
 

crates/editor/src/element.rs ๐Ÿ”—

@@ -11,7 +11,7 @@ use crate::{
         MIN_POPOVER_LINE_HEIGHT,
     },
     link_go_to_definition::{
-        GoToFetchedDefinition, GoToFetchedTypeDefinition, UpdateGoToDefinitionLink,
+        go_to_fetched_definition, go_to_fetched_type_definition, update_go_to_definition_link,
     },
     mouse_context_menu::DeployMouseContextMenu,
     scroll::actions::Scroll,
@@ -309,17 +309,25 @@ impl EditorElement {
             editor.select(SelectPhase::End, cx);
         }
 
-        if !pending_nonempty_selections && cmd && text_bounds.contains_point(position) {
-            let (point, target_point) = position_map.point_for_position(text_bounds, position);
+        if let Some(workspace) = editor
+            .workspace
+            .as_ref()
+            .and_then(|(workspace, _)| workspace.upgrade(cx))
+        {
+            if !pending_nonempty_selections && cmd && text_bounds.contains_point(position) {
+                let (point, target_point) = position_map.point_for_position(text_bounds, position);
 
-            if point == target_point {
-                if shift {
-                    cx.dispatch_action(GoToFetchedTypeDefinition { point });
-                } else {
-                    cx.dispatch_action(GoToFetchedDefinition { point });
-                }
+                if point == target_point {
+                    workspace.update(cx, |workspace, cx| {
+                        if shift {
+                            go_to_fetched_type_definition(workspace, point, cx);
+                        } else {
+                            go_to_fetched_definition(workspace, point, cx);
+                        }
+                    });
 
-                return true;
+                    return true;
+                }
             }
         }
 
@@ -350,11 +358,7 @@ impl EditorElement {
             None
         };
 
-        cx.dispatch_action(UpdateGoToDefinitionLink {
-            point,
-            cmd_held: cmd,
-            shift_held: shift,
-        });
+        update_go_to_definition_link(editor, point, cmd, shift, cx);
 
         if editor.has_pending_selection() {
             let mut scroll_delta = Vector2F::zero();
@@ -418,11 +422,7 @@ impl EditorElement {
         // Don't trigger hover popover if mouse is hovering over context menu
         let point = position_to_display_point(position, text_bounds, position_map);
 
-        cx.dispatch_action(UpdateGoToDefinitionLink {
-            point,
-            cmd_held: cmd,
-            shift_held: shift,
-        });
+        update_go_to_definition_link(editor, point, cmd, shift, cx);
         hover_at(editor, point, cx);
 
         true
@@ -1,6 +1,6 @@
 use std::ops::Range;
 
-use gpui::{impl_internal_actions, AppContext, Task, ViewContext};
+use gpui::{Task, ViewContext};
 use language::{Bias, ToOffset};
 use project::LocationLink;
 use settings::Settings;
@@ -11,38 +11,6 @@ use crate::{
     Anchor, DisplayPoint, Editor, EditorSnapshot, GoToDefinition, GoToTypeDefinition, SelectPhase,
 };
 
-#[derive(Clone, PartialEq)]
-pub struct UpdateGoToDefinitionLink {
-    pub point: Option<DisplayPoint>,
-    pub cmd_held: bool,
-    pub shift_held: bool,
-}
-
-#[derive(Clone, PartialEq)]
-pub struct GoToFetchedDefinition {
-    pub point: DisplayPoint,
-}
-
-#[derive(Clone, PartialEq)]
-pub struct GoToFetchedTypeDefinition {
-    pub point: DisplayPoint,
-}
-
-impl_internal_actions!(
-    editor,
-    [
-        UpdateGoToDefinitionLink,
-        GoToFetchedDefinition,
-        GoToFetchedTypeDefinition
-    ]
-);
-
-pub fn init(cx: &mut AppContext) {
-    cx.add_action(update_go_to_definition_link);
-    cx.add_action(go_to_fetched_definition);
-    cx.add_action(go_to_fetched_type_definition);
-}
-
 #[derive(Debug, Default)]
 pub struct LinkGoToDefinitionState {
     pub last_mouse_location: Option<Anchor>,
@@ -54,11 +22,9 @@ pub struct LinkGoToDefinitionState {
 
 pub fn update_go_to_definition_link(
     editor: &mut Editor,
-    &UpdateGoToDefinitionLink {
-        point,
-        cmd_held,
-        shift_held,
-    }: &UpdateGoToDefinitionLink,
+    point: Option<DisplayPoint>,
+    cmd_held: bool,
+    shift_held: bool,
     cx: &mut ViewContext<Editor>,
 ) {
     let pending_nonempty_selection = editor.has_pending_nonempty_selection();
@@ -285,7 +251,7 @@ pub fn hide_link_definition(editor: &mut Editor, cx: &mut ViewContext<Editor>) {
 
 pub fn go_to_fetched_definition(
     workspace: &mut Workspace,
-    &GoToFetchedDefinition { point }: &GoToFetchedDefinition,
+    point: DisplayPoint,
     cx: &mut ViewContext<Workspace>,
 ) {
     go_to_fetched_definition_of_kind(LinkDefinitionKind::Symbol, workspace, point, cx);
@@ -293,7 +259,7 @@ pub fn go_to_fetched_definition(
 
 pub fn go_to_fetched_type_definition(
     workspace: &mut Workspace,
-    &GoToFetchedTypeDefinition { point }: &GoToFetchedTypeDefinition,
+    point: DisplayPoint,
     cx: &mut ViewContext<Workspace>,
 ) {
     go_to_fetched_definition_of_kind(LinkDefinitionKind::Type, workspace, point, cx);
@@ -410,15 +376,7 @@ mod tests {
 
         // Press cmd+shift to trigger highlight
         cx.update_editor(|editor, cx| {
-            update_go_to_definition_link(
-                editor,
-                &UpdateGoToDefinitionLink {
-                    point: Some(hover_point),
-                    cmd_held: true,
-                    shift_held: true,
-                },
-                cx,
-            );
+            update_go_to_definition_link(editor, Some(hover_point), true, true, cx);
         });
         requests.next().await;
         cx.foreground().run_until_parked();
@@ -469,11 +427,7 @@ mod tests {
             });
 
         cx.update_workspace(|workspace, cx| {
-            go_to_fetched_type_definition(
-                workspace,
-                &GoToFetchedTypeDefinition { point: hover_point },
-                cx,
-            );
+            go_to_fetched_type_definition(workspace, hover_point, cx);
         });
         requests.next().await;
         cx.foreground().run_until_parked();
@@ -526,15 +480,7 @@ mod tests {
         });
 
         cx.update_editor(|editor, cx| {
-            update_go_to_definition_link(
-                editor,
-                &UpdateGoToDefinitionLink {
-                    point: Some(hover_point),
-                    cmd_held: true,
-                    shift_held: false,
-                },
-                cx,
-            );
+            update_go_to_definition_link(editor, Some(hover_point), true, false, cx);
         });
         requests.next().await;
         cx.foreground().run_until_parked();
@@ -568,15 +514,7 @@ mod tests {
             ])))
         });
         cx.update_editor(|editor, cx| {
-            update_go_to_definition_link(
-                editor,
-                &UpdateGoToDefinitionLink {
-                    point: Some(hover_point),
-                    cmd_held: true,
-                    shift_held: false,
-                },
-                cx,
-            );
+            update_go_to_definition_link(editor, Some(hover_point), true, false, cx);
         });
         requests.next().await;
         cx.foreground().run_until_parked();
@@ -598,15 +536,7 @@ mod tests {
                 Ok(Some(lsp::GotoDefinitionResponse::Link(vec![])))
             });
         cx.update_editor(|editor, cx| {
-            update_go_to_definition_link(
-                editor,
-                &UpdateGoToDefinitionLink {
-                    point: Some(hover_point),
-                    cmd_held: true,
-                    shift_held: false,
-                },
-                cx,
-            );
+            update_go_to_definition_link(editor, Some(hover_point), true, false, cx);
         });
         requests.next().await;
         cx.foreground().run_until_parked();
@@ -623,15 +553,7 @@ mod tests {
             fn do_work() { teห‡st(); }
         "});
         cx.update_editor(|editor, cx| {
-            update_go_to_definition_link(
-                editor,
-                &UpdateGoToDefinitionLink {
-                    point: Some(hover_point),
-                    cmd_held: false,
-                    shift_held: false,
-                },
-                cx,
-            );
+            update_go_to_definition_link(editor, Some(hover_point), false, false, cx);
         });
         cx.foreground().run_until_parked();
 
@@ -690,15 +612,7 @@ mod tests {
 
         // Moving the mouse restores the highlights.
         cx.update_editor(|editor, cx| {
-            update_go_to_definition_link(
-                editor,
-                &UpdateGoToDefinitionLink {
-                    point: Some(hover_point),
-                    cmd_held: true,
-                    shift_held: false,
-                },
-                cx,
-            );
+            update_go_to_definition_link(editor, Some(hover_point), true, false, cx);
         });
         cx.foreground().run_until_parked();
         cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
@@ -712,15 +626,7 @@ mod tests {
             fn do_work() { tesห‡t(); }
         "});
         cx.update_editor(|editor, cx| {
-            update_go_to_definition_link(
-                editor,
-                &UpdateGoToDefinitionLink {
-                    point: Some(hover_point),
-                    cmd_held: true,
-                    shift_held: false,
-                },
-                cx,
-            );
+            update_go_to_definition_link(editor, Some(hover_point), true, false, cx);
         });
         cx.foreground().run_until_parked();
         cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
@@ -730,7 +636,7 @@ mod tests {
 
         // Cmd click with existing definition doesn't re-request and dismisses highlight
         cx.update_workspace(|workspace, cx| {
-            go_to_fetched_definition(workspace, &GoToFetchedDefinition { point: hover_point }, cx);
+            go_to_fetched_definition(workspace, hover_point, cx);
         });
         // Assert selection moved to to definition
         cx.lsp
@@ -771,7 +677,7 @@ mod tests {
             ])))
         });
         cx.update_workspace(|workspace, cx| {
-            go_to_fetched_definition(workspace, &GoToFetchedDefinition { point: hover_point }, cx);
+            go_to_fetched_definition(workspace, hover_point, cx);
         });
         requests.next().await;
         cx.foreground().run_until_parked();
@@ -816,15 +722,7 @@ mod tests {
             });
         });
         cx.update_editor(|editor, cx| {
-            update_go_to_definition_link(
-                editor,
-                &UpdateGoToDefinitionLink {
-                    point: Some(hover_point),
-                    cmd_held: true,
-                    shift_held: false,
-                },
-                cx,
-            );
+            update_go_to_definition_link(editor, Some(hover_point), true, false, cx);
         });
         cx.foreground().run_until_parked();
         assert!(requests.try_next().is_err());