Merge branch 'main' into terminal-mouse

crates/editor/src/editor.rs

@@ -1789,15 +1789,15 @@ impl Editor {
         cx.notify();
     }
 
-    pub fn are_selections_empty(&self) -> bool {
-        let pending_empty = match self.selections.pending_anchor() {
-            Some(Selection { start, end, .. }) => start == end,
-            None => true,
+    pub fn has_pending_nonempty_selection(&self) -> bool {
+        let pending_nonempty_selection = match self.selections.pending_anchor() {
+            Some(Selection { start, end, .. }) => start != end,
+            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 selections_empty = view.are_selections_empty();
+        let end_selection = view.has_pending_selection();
+        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 {
-                pressed_button: Some(MouseButton::Left),
-                position,
-                ..
-            }) => self.mouse_dragged(*position, layout, paint, cx),
+            Event::MouseMoved(
+                event @ MouseMovedEvent {
+                    pressed_button: Some(MouseButton::Left),
+                    ..
+                },
+            ) => 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 {
-            hide_link_definition(editor, cx)
+            return;
         }
     }
+
+    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 target = &definition.target;
-                            if target.buffer == buffer {
-                                let range = &target.range;
-                                // Expand range by one character as lsp definition ranges include positions adjacent
-                                // but not contained by the symbol range
-                                let start = buffer_snapshot.clip_offset(
-                                    range.start.to_offset(&buffer_snapshot).saturating_sub(1),
-                                    Bias::Left,
-                                );
-                                let end = buffer_snapshot.clip_offset(
-                                    range.end.to_offset(&buffer_snapshot) + 1,
-                                    Bias::Right,
-                                );
-                                let offset = buffer_position.to_offset(&buffer_snapshot);
-                                !(start <= offset && end >= offset)
-                            } else {
-                                true
-                            }
-                        }) {
+                        let any_definition_does_not_contain_current_location =
+                            definitions.iter().any(|definition| {
+                                let target = &definition.target;
+                                if target.buffer == buffer {
+                                    let range = &target.range;
+                                    // Expand range by one character as lsp definition ranges include positions adjacent
+                                    // but not contained by the symbol range
+                                    let start = buffer_snapshot.clip_offset(
+                                        range.start.to_offset(&buffer_snapshot).saturating_sub(1),
+                                        Bias::Left,
+                                    );
+                                    let end = buffer_snapshot.clip_offset(
+                                        range.end.to_offset(&buffer_snapshot) + 1,
+                                        Bias::Right,
+                                    );
+                                    let offset = buffer_position.to_offset(&buffer_snapshot);
+                                    !(start <= offset && end >= offset)
+                                } 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);
                         }
@@ -806,5 +821,59 @@ mod tests {
             fn test() { do_work(); }
             fn «do_workˇ»() { test(); }
         "});
+
+        // 1. We have a pending selection, mouse point is over a symbol that we have a response for, hitting cmd and nothing happens
+        // 2. Selection is completed, hovering
+        let hover_point = cx.display_point(indoc! {"
+            fn test() { do_wˇork(); }
+            fn do_work() { test(); }
+        "});
+        let target_range = cx.lsp_range(indoc! {"
+            fn test() { do_work(); }
+            fn «do_work»() { test(); }
+        "});
+        let mut requests = cx.handle_request::<GotoDefinition, _, _>(move |url, _, _| async move {
+            Ok(Some(lsp::GotoDefinitionResponse::Link(vec![
+                lsp::LocationLink {
+                    origin_selection_range: None,
+                    target_uri: url,
+                    target_range,
+                    target_selection_range: target_range,
+                },
+            ])))
+        });
+
+        // create a pending selection
+        let selection_range = cx.ranges(indoc! {"
+            fn «test() { do_w»ork(); }
+            fn do_work() { test(); }
+        "})[0]
+            .clone();
+        cx.update_editor(|editor, cx| {
+            let snapshot = editor.buffer().read(cx).snapshot(cx);
+            let anchor_range = snapshot.anchor_before(selection_range.start)
+                ..snapshot.anchor_after(selection_range.end);
+            editor.change_selections(Some(crate::Autoscroll::Fit), cx, |s| {
+                s.set_pending_anchor_range(anchor_range, crate::SelectMode::Character)
+            });
+        });
+        cx.update_editor(|editor, cx| {
+            update_go_to_definition_link(
+                editor,
+                &UpdateGoToDefinitionLink {
+                    point: Some(hover_point),
+                    cmd_held: true,
+                    shift_held: false,
+                },
+                cx,
+            );
+        });
+        cx.foreground().run_until_parked();
+        assert!(requests.try_next().is_err());
+        cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
+            fn test() { do_work(); }
+            fn do_work() { test(); }
+        "});
+        cx.foreground().run_until_parked();
     }
 }

crates/editor/src/test.rs

@@ -183,7 +183,7 @@ impl<'a> EditorTestContext<'a> {
         }
     }
 
-    fn ranges(&self, marked_text: &str) -> Vec<Range<usize>> {
+    pub fn ranges(&self, marked_text: &str) -> Vec<Range<usize>> {
         let (unmarked_text, ranges) = marked_text_ranges(marked_text, false);
         assert_eq!(self.buffer_text(), unmarked_text);
         ranges

crates/gpui/src/platform/event.rs

@@ -64,7 +64,7 @@ impl Default for MouseButton {
     }
 }
 
-#[derive(Clone, Debug, Default)]
+#[derive(Clone, Copy, Debug, Default)]
 pub struct MouseButtonEvent {
     pub button: MouseButton,
     pub position: Vector2F,

crates/gpui/src/presenter.rs

@@ -31,7 +31,7 @@ pub struct Presenter {
     font_cache: Arc<FontCache>,
     text_layout_cache: TextLayoutCache,
     asset_cache: Arc<AssetCache>,
-    last_mouse_moved_event: Option<Event>,
+    last_mouse_moved_event: Option<MouseMovedEvent>,
     hovered_region_ids: HashSet<MouseRegionId>,
     clicked_region: Option<MouseRegion>,
     right_clicked_region: Option<MouseRegion>,
@@ -269,15 +269,27 @@ impl Presenter {
                         }
                     }
                 }
-                Event::MouseUp(e @ MouseButtonEvent { position, .. }) => {
+
+                &Event::MouseUp(
+                    e @ MouseButtonEvent {
+                        position, button, ..
+                    },
+                ) => {
                     self.prev_drag_position.take();
                     if let Some(region) = self.clicked_region.take() {
                         invalidated_views.push(region.view_id);
-                        if region.bounds.contains_point(*position) {
+                        if region.bounds.contains_point(position) {
                             clicked_region = Some((region, MouseRegionEvent::Click(e.clone())));
                         }
                     }
+
+                    if let Some(moved) = &mut self.last_mouse_moved_event {
+                        if moved.pressed_button == Some(button) {
+                            moved.pressed_button = None;
+                        }
+                    }
                 }
+
                 Event::MouseMoved(e @ MouseMovedEvent { position, .. }) => {
                     if let Some((clicked_region, prev_drag_position)) = self
                         .clicked_region
@@ -300,13 +312,17 @@ impl Presenter {
                         }
                     }
 
-                    self.last_mouse_moved_event = Some(event.clone());
+                    self.last_mouse_moved_event = Some(e.clone());
                 }
+
                 _ => {}
             }
 
-            let (mut handled, mut event_cx) =
-                self.handle_hover_events(&event, &mut invalidated_views, cx);
+            let (mut handled, mut event_cx) = if let Event::MouseMoved(e) = &event {
+                self.handle_hover_events(e, &mut invalidated_views, cx)
+            } else {
+                (false, self.build_event_context(cx))
+            };
 
             for (handler, view_id, region_event) in mouse_down_out_handlers {
                 event_cx.with_current_view(view_id, |event_cx| handler(region_event, event_cx))
@@ -374,51 +390,45 @@ impl Presenter {
 
     fn handle_hover_events<'a>(
         &'a mut self,
-        event: &Event,
+        e @ MouseMovedEvent {
+            position,
+            pressed_button,
+            ..
+        }: &MouseMovedEvent,
         invalidated_views: &mut Vec<usize>,
         cx: &'a mut MutableAppContext,
     ) -> (bool, EventContext<'a>) {
         let mut hover_regions = Vec::new();
-        if let Event::MouseMoved(
-            e @ MouseMovedEvent {
-                position,
-                pressed_button,
-                ..
-            },
-        ) = event
-        {
-            if pressed_button.is_none() {
-                let mut style_to_assign = CursorStyle::Arrow;
-                for region in self.cursor_regions.iter().rev() {
-                    if region.bounds.contains_point(*position) {
-                        style_to_assign = region.style;
-                        break;
-                    }
-                }
-                cx.platform().set_cursor_style(style_to_assign);
 
-                let mut hover_depth = None;
-                for (region, depth) in self.mouse_regions.iter().rev() {
-                    if region.bounds.contains_point(*position)
-                        && hover_depth.map_or(true, |hover_depth| hover_depth == *depth)
-                    {
-                        hover_depth = Some(*depth);
-                        if let Some(region_id) = region.id() {
-                            if !self.hovered_region_ids.contains(&region_id) {
-                                invalidated_views.push(region.view_id);
-                                hover_regions
-                                    .push((region.clone(), MouseRegionEvent::Hover(true, *e)));
-                                self.hovered_region_ids.insert(region_id);
-                            }
-                        }
-                    } else if let Some(region_id) = region.id() {
-                        if self.hovered_region_ids.contains(&region_id) {
+        if pressed_button.is_none() {
+            let mut style_to_assign = CursorStyle::Arrow;
+            for region in self.cursor_regions.iter().rev() {
+                if region.bounds.contains_point(*position) {
+                    style_to_assign = region.style;
+                    break;
+                }
+            }
+            cx.platform().set_cursor_style(style_to_assign);
+
+            let mut hover_depth = None;
+            for (region, depth) in self.mouse_regions.iter().rev() {
+                if region.bounds.contains_point(*position)
+                    && hover_depth.map_or(true, |hover_depth| hover_depth == *depth)
+                {
+                    hover_depth = Some(*depth);
+                    if let Some(region_id) = region.id() {
+                        if !self.hovered_region_ids.contains(&region_id) {
                             invalidated_views.push(region.view_id);
-                            hover_regions
-                                .push((region.clone(), MouseRegionEvent::Hover(false, *e)));
-                            self.hovered_region_ids.remove(&region_id);
+                            hover_regions.push((region.clone(), MouseRegionEvent::Hover(true, *e)));
+                            self.hovered_region_ids.insert(region_id);
                         }
                     }
+                } else if let Some(region_id) = region.id() {
+                    if self.hovered_region_ids.contains(&region_id) {
+                        invalidated_views.push(region.view_id);
+                        hover_regions.push((region.clone(), MouseRegionEvent::Hover(false, *e)));
+                        self.hovered_region_ids.remove(&region_id);
+                    }
                 }
             }
         }