Avoid triggering goto-definition links while with a pending selection

Co-Authored-By: Antonio Scandurra <me@as-cii.com>

crates/editor/src/editor.rs

@@ -1789,15 +1789,15 @@ impl Editor {
         cx.notify();
     }
 
-    pub fn are_selections_empty(&self) -> bool {
+    pub fn has_pending_nonempty_selection(&self) -> bool {
-        let pending_empty = match self.selections.pending_anchor() {
+        let pending_nonempty_selection = match self.selections.pending_anchor() {
-            Some(Selection { start, end, .. }) => start == end,
+            Some(Selection { start, end, .. }) => start != end,
-            None => true,
+            None => false,
         };
-        pending_empty && self.columnar_selection_tail.is_none()
+        pending_nonempty_selection || self.columnar_selection_tail.is_some()
     }
 
-    pub fn is_selecting(&self) -> bool {
+    pub fn has_pending_selection(&self) -> bool {
         self.selections.pending_anchor().is_some() || self.columnar_selection_tail.is_some()
     }
 

crates/editor/src/element.rs

@@ -179,14 +179,14 @@ impl EditorElement {
         cx: &mut EventContext,
     ) -> bool {
         let view = self.view(cx.app.as_ref());
-        let end_selection = view.is_selecting();
+        let end_selection = view.has_pending_selection();
-        let selections_empty = view.are_selections_empty();
+        let pending_nonempty_selections = view.has_pending_nonempty_selection();
 
         if end_selection {
             cx.dispatch_action(Select(SelectPhase::End));
         }
 
-        if selections_empty && cmd && paint.text_bounds.contains_point(position) {
+        if !pending_nonempty_selections && cmd && paint.text_bounds.contains_point(position) {
             let (point, target_point) =
                 paint.point_for_position(&self.snapshot(cx), layout, position);
 
@@ -206,14 +206,38 @@ impl EditorElement {
 
     fn mouse_dragged(
         &self,
-        position: Vector2F,
+        MouseMovedEvent {
+            cmd,
+            shift,
+            position,
+            ..
+        }: MouseMovedEvent,
         layout: &mut LayoutState,
         paint: &mut PaintState,
         cx: &mut EventContext,
     ) -> bool {
-        let view = self.view(cx.app.as_ref());
+        // This will be handled more correctly once https://github.com/zed-industries/zed/issues/1218 is completed
+        // Don't trigger hover popover if mouse is hovering over context menu
+        let point = if paint.text_bounds.contains_point(position) {
+            let (point, target_point) =
+                paint.point_for_position(&self.snapshot(cx), layout, position);
+            if point == target_point {
+                Some(point)
+            } else {
+                None
+            }
+        } else {
+            None
+        };
 
-        if view.is_selecting() {
+        cx.dispatch_action(UpdateGoToDefinitionLink {
+            point,
+            cmd_held: cmd,
+            shift_held: shift,
+        });
+
+        let view = self.view(cx.app);
+        if view.has_pending_selection() {
             let rect = paint.text_bounds;
             let mut scroll_delta = Vector2F::zero();
 
@@ -250,8 +274,11 @@ impl EditorElement {
                 scroll_position: (snapshot.scroll_position() + scroll_delta)
                     .clamp(Vector2F::zero(), layout.scroll_max),
             }));
+
+            cx.dispatch_action(HoverAt { point });
             true
         } else {
+            cx.dispatch_action(HoverAt { point });
             false
         }
     }
@@ -1572,11 +1599,12 @@ impl Element for EditorElement {
                 ..
             }) => self.mouse_up(position, cmd, shift, layout, paint, cx),
 
-            Event::MouseMoved(MouseMovedEvent {
+            Event::MouseMoved(
-                pressed_button: Some(MouseButton::Left),
+                event @ MouseMovedEvent {
-                position,
+                    pressed_button: Some(MouseButton::Left),
-                ..
+                    ..
-            }) => self.mouse_dragged(*position, layout, paint, cx),
+                },
+            ) => self.mouse_dragged(*event, layout, paint, cx),
 
             Event::ScrollWheel(ScrollWheelEvent {
                 position,

crates/editor/src/link_go_to_definition.rs

@@ -70,6 +70,8 @@ pub fn update_go_to_definition_link(
     }: &UpdateGoToDefinitionLink,
     cx: &mut ViewContext<Editor>,
 ) {
+    let pending_nonempty_selection = editor.has_pending_nonempty_selection();
+
     // Store new mouse point as an anchor
     let snapshot = editor.snapshot(cx);
     let point = point.map(|point| {
@@ -89,6 +91,12 @@ pub fn update_go_to_definition_link(
     }
 
     editor.link_go_to_definition_state.last_mouse_location = point.clone();
+
+    if pending_nonempty_selection {
+        hide_link_definition(editor, cx);
+        return;
+    }
+
     if cmd_held {
         if let Some(point) = point {
             let kind = if shift_held {
@@ -113,12 +121,14 @@ pub fn cmd_shift_changed(
     }: &CmdShiftChanged,
     cx: &mut ViewContext<Editor>,
 ) {
+    let pending_nonempty_selection = editor.has_pending_nonempty_selection();
+
     if let Some(point) = editor
         .link_go_to_definition_state
         .last_mouse_location
         .clone()
     {
-        if cmd_down {
+        if cmd_down && !pending_nonempty_selection {
             let snapshot = editor.snapshot(cx);
             let kind = if shift_down {
                 LinkDefinitionKind::Type
@@ -127,10 +137,11 @@ pub fn cmd_shift_changed(
             };
 
             show_link_definition(kind, editor, point, snapshot, cx);
-        } else {
+            return;
-            hide_link_definition(editor, cx)
         }
     }
+
+    hide_link_definition(editor, cx)
 }
 
 #[derive(Debug, Clone, Copy, PartialEq)]
@@ -243,28 +254,32 @@ pub fn show_link_definition(
                         this.link_go_to_definition_state.definitions = definitions.clone();
 
                         let buffer_snapshot = buffer.read(cx).snapshot();
+
                         // Only show highlight if there exists a definition to jump to that doesn't contain
                         // the current location.
-                        if definitions.iter().any(|definition| {
+                        let any_definition_does_not_contain_current_location =
-                            let target = &definition.target;
+                            definitions.iter().any(|definition| {
-                            if target.buffer == buffer {
+                                let target = &definition.target;
-                                let range = &target.range;
+                                if target.buffer == buffer {
-                                // Expand range by one character as lsp definition ranges include positions adjacent
+                                    let range = &target.range;
-                                // but not contained by the symbol range
+                                    // Expand range by one character as lsp definition ranges include positions adjacent
-                                let start = buffer_snapshot.clip_offset(
+                                    // but not contained by the symbol range
-                                    range.start.to_offset(&buffer_snapshot).saturating_sub(1),
+                                    let start = buffer_snapshot.clip_offset(
-                                    Bias::Left,
+                                        range.start.to_offset(&buffer_snapshot).saturating_sub(1),
-                                );
+                                        Bias::Left,
-                                let end = buffer_snapshot.clip_offset(
+                                    );
-                                    range.end.to_offset(&buffer_snapshot) + 1,
+                                    let end = buffer_snapshot.clip_offset(
-                                    Bias::Right,
+                                        range.end.to_offset(&buffer_snapshot) + 1,
-                                );
+                                        Bias::Right,
-                                let offset = buffer_position.to_offset(&buffer_snapshot);
+                                    );
-                                !(start <= offset && end >= offset)
+                                    let offset = buffer_position.to_offset(&buffer_snapshot);
-                            } else {
+                                    !(start <= offset && end >= offset)
-                                true
+                                } else {
-                            }
+                                    true
-                        }) {
+                                }
+                            });
+
+                        if any_definition_does_not_contain_current_location {
                             // If no symbol range returned from language server, use the surrounding word.
                             let highlight_range = symbol_range.unwrap_or_else(|| {
                                 let snapshot = &snapshot.buffer_snapshot;
@@ -280,7 +295,7 @@ pub fn show_link_definition(
                                 vec![highlight_range],
                                 style,
                                 cx,
-                            )
+                            );
                         } else {
                             hide_link_definition(this, cx);
                         }