From b61e9a940ea560b0d99da3737e53ec086cf7de6d Mon Sep 17 00:00:00 2001 From: ForLoveOfCats Date: Thu, 18 Aug 2022 11:46:11 -0400 Subject: [PATCH] Avoid triggering goto-definition links while with a pending selection Co-Authored-By: Antonio Scandurra --- crates/editor/src/editor.rs | 12 ++--- crates/editor/src/element.rs | 50 +++++++++++++---- crates/editor/src/link_go_to_definition.rs | 63 +++++++++++++--------- 3 files changed, 84 insertions(+), 41 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 2fd3c0653801074f6112569deb2f6cfb79ed6cbb..9e5a6329bf3099f64c074acfa1483a0aa9836018 100644 --- a/crates/editor/src/editor.rs +++ b/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() } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 6140731579455a9a98887bf524a5372edb5bad24..0c899cd4214b8120b6d7f0f1a9ae66417676bef7 100644 --- a/crates/editor/src/element.rs +++ b/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 + }; + + cx.dispatch_action(UpdateGoToDefinitionLink { + point, + cmd_held: cmd, + shift_held: shift, + }); - if view.is_selecting() { + 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, diff --git a/crates/editor/src/link_go_to_definition.rs b/crates/editor/src/link_go_to_definition.rs index 6eb38df75818050d342420e42b343ea37cb8b6ad..79b2e950cb477798addd8ae26709c39dc2fa74b7 100644 --- a/crates/editor/src/link_go_to_definition.rs +++ b/crates/editor/src/link_go_to_definition.rs @@ -70,6 +70,8 @@ pub fn update_go_to_definition_link( }: &UpdateGoToDefinitionLink, cx: &mut ViewContext, ) { + 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, ) { + 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); }