working jump to definition with tests

crates/editor/src/display_map.rs

@@ -474,6 +474,14 @@ impl DisplaySnapshot {
     pub fn longest_row(&self) -> u32 {
         self.blocks_snapshot.longest_row()
     }
+
+    #[cfg(any(test, feature = "test-support"))]
+    pub fn highlight_ranges<Tag: ?Sized + 'static>(
+        &self,
+    ) -> Option<Arc<(HighlightStyle, Vec<Range<Anchor>>)>> {
+        let type_id = TypeId::of::<Tag>();
+        self.text_highlights.get(&Some(type_id)).cloned()
+    }
 }
 
 #[derive(Copy, Clone, Default, Eq, Ord, PartialOrd, PartialEq)]

crates/editor/src/editor.rs

@@ -45,7 +45,7 @@ pub use multi_buffer::{
     ToPoint,
 };
 use ordered_float::OrderedFloat;
-use project::{Project, ProjectPath, ProjectTransaction};
+use project::{LocationLink, Project, ProjectPath, ProjectTransaction};
 use selections_collection::{resolve_multiple, MutableSelectionsCollection, SelectionsCollection};
 use serde::{Deserialize, Serialize};
 use settings::Settings;
@@ -4602,28 +4602,7 @@ impl Editor {
         cx.spawn(|workspace, mut cx| async move {
             let definitions = definitions.await?;
             workspace.update(&mut cx, |workspace, cx| {
-                let nav_history = workspace.active_pane().read(cx).nav_history().clone();
-                for definition in definitions {
-                    let range = definition
-                        .target
-                        .range
-                        .to_offset(definition.target.buffer.read(cx));
-
-                    let target_editor_handle =
-                        workspace.open_project_item(definition.target.buffer, cx);
-                    target_editor_handle.update(cx, |target_editor, cx| {
-                        // When selecting a definition in a different buffer, disable the nav history
-                        // to avoid creating a history entry at the previous cursor location.
-                        if editor_handle != target_editor_handle {
-                            nav_history.borrow_mut().disable();
-                        }
-                        target_editor.change_selections(Some(Autoscroll::Center), cx, |s| {
-                            s.select_ranges([range]);
-                        });
-
-                        nav_history.borrow_mut().enable();
-                    });
-                }
+                Editor::navigate_to_definitions(workspace, editor_handle, definitions, cx);
             });
 
             Ok::<(), anyhow::Error>(())
@@ -4631,6 +4610,35 @@ impl Editor {
         .detach_and_log_err(cx);
     }
 
+    pub fn navigate_to_definitions(
+        workspace: &mut Workspace,
+        editor_handle: ViewHandle<Editor>,
+        definitions: Vec<LocationLink>,
+        cx: &mut ViewContext<Workspace>,
+    ) {
+        let nav_history = workspace.active_pane().read(cx).nav_history().clone();
+        for definition in definitions {
+            let range = definition
+                .target
+                .range
+                .to_offset(definition.target.buffer.read(cx));
+
+            let target_editor_handle = workspace.open_project_item(definition.target.buffer, cx);
+            target_editor_handle.update(cx, |target_editor, cx| {
+                // When selecting a definition in a different buffer, disable the nav history
+                // to avoid creating a history entry at the previous cursor location.
+                if editor_handle != target_editor_handle {
+                    nav_history.borrow_mut().disable();
+                }
+                target_editor.change_selections(Some(Autoscroll::Center), cx, |s| {
+                    s.select_ranges([range]);
+                });
+
+                nav_history.borrow_mut().enable();
+            });
+        }
+    }
+
     pub fn find_all_references(
         workspace: &mut Workspace,
         _: &FindAllReferences,

crates/editor/src/element.rs

@@ -6,9 +6,9 @@ use super::{
 use crate::{
     display_map::{BlockStyle, DisplaySnapshot, TransformBlock},
     hover_popover::HoverAt,
+    link_go_to_definition::{CmdChanged, GoToFetchedDefinition, UpdateGoToDefinitionLink},
     EditorStyle,
 };
-use crate::{hover_popover::HoverAt, link_go_to_definition::FetchDefinition};
 use clock::ReplicaId;
 use collections::{BTreeMap, HashMap};
 use gpui::{
@@ -105,7 +105,7 @@ impl EditorElement {
     fn mouse_down(
         &self,
         position: Vector2F,
-        _: bool,
+        cmd: bool,
         alt: bool,
         shift: bool,
         mut click_count: usize,
@@ -113,6 +113,14 @@ impl EditorElement {
         paint: &mut PaintState,
         cx: &mut EventContext,
     ) -> bool {
+        if cmd && paint.text_bounds.contains_point(position) {
+            let (point, overshoot) = paint.point_for_position(&self.snapshot(cx), layout, position);
+            if overshoot.is_zero() {
+                cx.dispatch_action(GoToFetchedDefinition { point });
+                return true;
+            }
+        }
+
         if paint.gutter_bounds.contains_point(position) {
             click_count = 3; // Simulate triple-click when clicking the gutter to select lines
         } else if !paint.text_bounds.contains_point(position) {
@@ -204,6 +212,52 @@ impl EditorElement {
         }
     }
 
+    fn mouse_moved(
+        &self,
+        position: Vector2F,
+        cmd: bool,
+        layout: &LayoutState,
+        paint: &PaintState,
+        cx: &mut EventContext,
+    ) -> bool {
+        // 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, overshoot) = paint.point_for_position(&self.snapshot(cx), layout, position);
+            if overshoot.is_zero() {
+                Some(point)
+            } else {
+                None
+            }
+        } else {
+            None
+        };
+
+        cx.dispatch_action(UpdateGoToDefinitionLink {
+            point,
+            cmd_held: cmd,
+        });
+
+        if paint
+            .context_menu_bounds
+            .map_or(false, |context_menu_bounds| {
+                context_menu_bounds.contains_point(*position)
+            })
+        {
+            return false;
+        }
+
+        if paint
+            .hover_bounds
+            .map_or(false, |hover_bounds| hover_bounds.contains_point(position))
+        {
+            return false;
+        }
+
+        cx.dispatch_action(HoverAt { point });
+        true
+    }
+
     fn key_down(&self, input: Option<&str>, cx: &mut EventContext) -> bool {
         let view = self.view.upgrade(cx.app).unwrap();
 
@@ -219,6 +273,11 @@ impl EditorElement {
         }
     }
 
+    fn modifiers_changed(&self, cmd: bool, cx: &mut EventContext) -> bool {
+        cx.dispatch_action(CmdChanged { cmd_down: cmd });
+        false
+    }
+
     fn scroll(
         &self,
         position: Vector2F,
@@ -1427,45 +1486,11 @@ impl Element for EditorElement {
                 precise,
             } => self.scroll(*position, *delta, *precise, layout, paint, cx),
             Event::KeyDown { input, .. } => self.key_down(input.as_deref(), cx),
+            Event::ModifiersChanged { cmd, .. } => self.modifiers_changed(*cmd, cx),
             Event::MouseMoved { position, cmd, .. } => {
-                // 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, overshoot) =
-                        paint.point_for_position(&self.snapshot(cx), layout, *position);
-                    if overshoot.is_zero() {
-                        Some(point)
-                    } else {
-                        None
-                    }
-                } else {
-                    None
-                };
-
-                if *cmd {
-                    cx.dispatch_action(FetchDefinition { point });
-                }
-
-                if paint
-                    .context_menu_bounds
-                    .map_or(false, |context_menu_bounds| {
-                        context_menu_bounds.contains_point(*position)
-                    })
-                {
-                    return false;
-                }
-
-                if paint
-                    .hover_bounds
-                    .map_or(false, |hover_bounds| hover_bounds.contains_point(*position))
-                {
-                    return false;
-                }
-
-                cx.dispatch_action(HoverAt { point });
-                true
+                self.mouse_moved(*position, *cmd, layout, paint, cx)
             }
+
             _ => false,
         }
     }

crates/editor/src/hover_popover.rs

@@ -1,5 +1,3 @@
-use std::{ops::Range, time::Duration};
-
 use gpui::{
     actions,
     elements::{Flex, MouseEventHandler, Padding, Text},
@@ -9,6 +7,7 @@ use gpui::{
 };
 use language::Bias;
 use project::{HoverBlock, Project};
+use std::{ops::Range, time::Duration};
 use util::TryFutureExt;
 
 use crate::{

crates/editor/src/link_go_to_definition.rs

@@ -1,60 +1,77 @@
-use std::{
-    ops::Range,
-    time::{Duration, Instant},
-};
+use std::ops::Range;
 
-use gpui::{
-    actions,
-    color::Color,
-    elements::{Flex, MouseEventHandler, Padding, Text},
-    fonts::{HighlightStyle, Underline},
-    impl_internal_actions,
-    platform::CursorStyle,
-    Axis, Element, ElementBox, ModelHandle, MutableAppContext, RenderContext, Task, ViewContext,
-};
-use language::Bias;
-use project::{HoverBlock, Project};
+use gpui::{impl_internal_actions, MutableAppContext, Task, ViewContext};
+use language::{Bias, ToOffset};
+use project::LocationLink;
+use settings::Settings;
 use util::TryFutureExt;
+use workspace::Workspace;
 
-use crate::{
-    display_map::ToDisplayPoint, Anchor, AnchorRangeExt, DisplayPoint, Editor, EditorSnapshot,
-    EditorStyle,
-};
+use crate::{Anchor, DisplayPoint, Editor, GoToDefinition};
 
 #[derive(Clone, PartialEq)]
-pub struct FetchDefinition {
+pub struct UpdateGoToDefinitionLink {
     pub point: Option<DisplayPoint>,
+    pub cmd_held: bool,
+}
+
+#[derive(Clone, PartialEq)]
+pub struct CmdChanged {
+    pub cmd_down: bool,
 }
 
 #[derive(Clone, PartialEq)]
 pub struct GoToFetchedDefinition {
-    pub point: Option<DisplayPoint>,
+    pub point: DisplayPoint,
 }
 
-impl_internal_actions!(edtior, [FetchDefinition, GoToFetchedDefinition]);
+impl_internal_actions!(
+    editor,
+    [UpdateGoToDefinitionLink, CmdChanged, GoToFetchedDefinition]
+);
 
 pub fn init(cx: &mut MutableAppContext) {
-    cx.add_action(fetch_definition);
+    cx.add_action(update_go_to_definition_link);
+    cx.add_action(cmd_changed);
     cx.add_action(go_to_fetched_definition);
 }
 
 #[derive(Default)]
 pub struct LinkGoToDefinitionState {
+    pub last_mouse_point: Option<DisplayPoint>,
     pub triggered_from: Option<Anchor>,
     pub symbol_range: Option<Range<Anchor>>,
+    pub definitions: Vec<LocationLink>,
     pub task: Option<Task<Option<()>>>,
 }
 
-pub fn fetch_definition(
+pub fn update_go_to_definition_link(
     editor: &mut Editor,
-    &FetchDefinition { point }: &FetchDefinition,
+    &UpdateGoToDefinitionLink { point, cmd_held }: &UpdateGoToDefinitionLink,
     cx: &mut ViewContext<Editor>,
 ) {
-    if let Some(point) = point {
-        show_link_definition(editor, point, cx);
-    } else {
-        //TODO: Also needs to be dispatched when cmd modifier is released
-        hide_link_definition(editor, cx);
+    editor.link_go_to_definition_state.last_mouse_point = point;
+    if cmd_held {
+        if let Some(point) = point {
+            show_link_definition(editor, point, cx);
+            return;
+        }
+    }
+
+    hide_link_definition(editor, cx);
+}
+
+pub fn cmd_changed(
+    editor: &mut Editor,
+    &CmdChanged { cmd_down }: &CmdChanged,
+    cx: &mut ViewContext<Editor>,
+) {
+    if let Some(point) = editor.link_go_to_definition_state.last_mouse_point {
+        if cmd_down {
+            show_link_definition(editor, point, cx);
+        } else {
+            hide_link_definition(editor, cx)
+        }
     }
 }
 
@@ -101,7 +118,22 @@ pub fn show_link_definition(
         .buffer_snapshot
         .anchor_at(multibuffer_offset, Bias::Left);
 
-    // Don't request again if the location is the same as the previous request
+    // Don't request again if the location is within the symbol region of a previous request
+    if let Some(symbol_range) = &editor.link_go_to_definition_state.symbol_range {
+        if symbol_range
+            .start
+            .cmp(&anchor, &snapshot.buffer_snapshot)
+            .is_le()
+            && symbol_range
+                .end
+                .cmp(&anchor, &snapshot.buffer_snapshot)
+                .is_ge()
+        {
+            return;
+        }
+    }
+
+    // Don't request from the exact same location again
     if let Some(triggered_from) = &editor.link_go_to_definition_state.triggered_from {
         if triggered_from
             .cmp(&anchor, &snapshot.buffer_snapshot)
@@ -120,11 +152,10 @@ pub fn show_link_definition(
                 })
             });
 
-            let origin_range = definition_request.await.ok().and_then(|definition_result| {
-                definition_result
-                    .into_iter()
-                    .filter_map(|link| {
-                        link.origin.map(|origin| {
+            let result = definition_request.await.ok().map(|definition_result| {
+                (
+                    definition_result.iter().find_map(|link| {
+                        link.origin.as_ref().map(|origin| {
                             let start = snapshot
                                 .buffer_snapshot
                                 .anchor_in_excerpt(excerpt_id.clone(), origin.range.start);
@@ -134,25 +165,65 @@ pub fn show_link_definition(
 
                             start..end
                         })
-                    })
-                    .next()
+                    }),
+                    definition_result,
+                )
             });
 
             if let Some(this) = this.upgrade(&cx) {
                 this.update(&mut cx, |this, cx| {
-                    if let Some(origin_range) = origin_range {
-                        this.highlight_text::<LinkGoToDefinitionState>(
-                            vec![origin_range],
-                            HighlightStyle {
-                                underline: Some(Underline {
-                                    color: Some(Color::red()),
-                                    thickness: 1.0.into(),
-                                    squiggly: false,
-                                }),
-                                ..Default::default()
-                            },
-                            cx,
-                        )
+                    // Clear any existing highlights
+                    this.clear_text_highlights::<LinkGoToDefinitionState>(cx);
+                    this.link_go_to_definition_state.triggered_from = Some(anchor.clone());
+                    this.link_go_to_definition_state.symbol_range = result
+                        .as_ref()
+                        .and_then(|(symbol_range, _)| symbol_range.clone());
+
+                    if let Some((symbol_range, definitions)) = result {
+                        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
+                            }
+                        }) {
+                            // 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;
+                                let (offset_range, _) = snapshot.surrounding_word(anchor);
+
+                                snapshot.anchor_before(offset_range.start)
+                                    ..snapshot.anchor_after(offset_range.end)
+                            });
+
+                            // Highlight symbol using theme link definition highlight style
+                            let style = cx.global::<Settings>().theme.editor.link_definition;
+                            this.highlight_text::<LinkGoToDefinitionState>(
+                                vec![highlight_range],
+                                style,
+                                cx,
+                            )
+                        } else {
+                            hide_link_definition(this, cx);
+                        }
                     }
                 })
             }
@@ -166,21 +237,363 @@ pub fn show_link_definition(
 }
 
 pub fn hide_link_definition(editor: &mut Editor, cx: &mut ViewContext<Editor>) {
-    // only notify the context once
     if editor.link_go_to_definition_state.symbol_range.is_some() {
         editor.link_go_to_definition_state.symbol_range.take();
         cx.notify();
     }
 
-    editor.link_go_to_definition_state.task = None;
     editor.link_go_to_definition_state.triggered_from = None;
+    editor.link_go_to_definition_state.task = None;
 
     editor.clear_text_highlights::<LinkGoToDefinitionState>(cx);
 }
 
 pub fn go_to_fetched_definition(
-    editor: &mut Editor,
+    workspace: &mut Workspace,
     GoToFetchedDefinition { point }: &GoToFetchedDefinition,
-    cx: &mut ViewContext<Editor>,
+    cx: &mut ViewContext<Workspace>,
 ) {
+    let active_item = workspace.active_item(cx);
+    let editor_handle = if let Some(editor) = active_item
+        .as_ref()
+        .and_then(|item| item.act_as::<Editor>(cx))
+    {
+        editor
+    } else {
+        return;
+    };
+
+    let mut definitions = Vec::new();
+
+    editor_handle.update(cx, |editor, cx| {
+        hide_link_definition(editor, cx);
+        std::mem::swap(
+            &mut editor.link_go_to_definition_state.definitions,
+            &mut definitions,
+        );
+    });
+
+    if !definitions.is_empty() {
+        Editor::navigate_to_definitions(workspace, editor_handle, definitions, cx);
+    } else {
+        editor_handle.update(cx, |editor, cx| {
+            editor.change_selections(None, cx, |s| s.select_display_ranges(vec![*point..*point]));
+        });
+
+        Editor::go_to_definition(workspace, &GoToDefinition, cx);
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use futures::StreamExt;
+    use indoc::indoc;
+
+    use crate::test::EditorLspTestContext;
+
+    use super::*;
+
+    #[gpui::test]
+    async fn test_link_go_to_definition(cx: &mut gpui::TestAppContext) {
+        let mut cx = EditorLspTestContext::new_rust(
+            lsp::ServerCapabilities {
+                hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
+                ..Default::default()
+            },
+            cx,
+        )
+        .await;
+
+        cx.set_state(indoc! {"
+            fn |test()
+                do_work();
+            
+            fn do_work()
+                test();"});
+
+        // Basic hold cmd, expect highlight in region if response contains definition
+        let hover_point = cx.display_point(indoc! {"
+            fn test()
+                do_w|ork();
+            
+            fn do_work()
+                test();"});
+
+        let symbol_range = cx.lsp_range(indoc! {"
+            fn test()
+                [do_work]();
+            
+            fn do_work()
+                test();"});
+        let target_range = cx.lsp_range(indoc! {"
+            fn test()
+                do_work();
+            
+            fn [do_work]()
+                test();"});
+
+        let mut requests =
+            cx.lsp
+                .handle_request::<lsp::request::GotoDefinition, _, _>(move |_, _| async move {
+                    Ok(Some(lsp::GotoDefinitionResponse::Link(vec![
+                        lsp::LocationLink {
+                            origin_selection_range: Some(symbol_range),
+                            target_uri: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
+                            target_range,
+                            target_selection_range: target_range,
+                        },
+                    ])))
+                });
+        cx.update_editor(|editor, cx| {
+            update_go_to_definition_link(
+                editor,
+                &UpdateGoToDefinitionLink {
+                    point: Some(hover_point),
+                    cmd_held: true,
+                },
+                cx,
+            );
+        });
+        requests.next().await;
+        cx.foreground().run_until_parked();
+        cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
+            fn test()
+                [do_work]();
+            
+            fn do_work()
+                test();"});
+
+        // Unpress cmd causes highlight to go away
+        cx.update_editor(|editor, cx| {
+            cmd_changed(editor, &CmdChanged { cmd_down: false }, cx);
+        });
+        // Assert no link highlights
+        cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
+            fn test()
+                do_work();
+            
+            fn do_work()
+                test();"});
+
+        // Response without source range still highlights word
+        let mut requests =
+            cx.lsp
+                .handle_request::<lsp::request::GotoDefinition, _, _>(move |_, _| async move {
+                    Ok(Some(lsp::GotoDefinitionResponse::Link(vec![
+                        lsp::LocationLink {
+                            // No origin range
+                            origin_selection_range: None,
+                            target_uri: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
+                            target_range,
+                            target_selection_range: target_range,
+                        },
+                    ])))
+                });
+        cx.update_editor(|editor, cx| {
+            update_go_to_definition_link(
+                editor,
+                &UpdateGoToDefinitionLink {
+                    point: Some(hover_point),
+                    cmd_held: true,
+                },
+                cx,
+            );
+        });
+        requests.next().await;
+        cx.foreground().run_until_parked();
+        cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
+            fn test()
+                [do_work]();
+            
+            fn do_work()
+                test();"});
+
+        // Moving mouse to location with no response dismisses highlight
+        let hover_point = cx.display_point(indoc! {"
+            f|n test()
+                do_work();
+            
+            fn do_work()
+                test();"});
+        let mut requests =
+            cx.lsp
+                .handle_request::<lsp::request::GotoDefinition, _, _>(move |_, _| async move {
+                    // No definitions returned
+                    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,
+                },
+                cx,
+            );
+        });
+        requests.next().await;
+        cx.foreground().run_until_parked();
+
+        // Assert no link highlights
+        cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
+            fn test()
+                do_work();
+            
+            fn do_work()
+                test();"});
+
+        // Move mouse without cmd and then pressing cmd triggers highlight
+        let hover_point = cx.display_point(indoc! {"
+            fn test()
+                do_work();
+            
+            fn do_work()
+                te|st();"});
+        cx.update_editor(|editor, cx| {
+            update_go_to_definition_link(
+                editor,
+                &UpdateGoToDefinitionLink {
+                    point: Some(hover_point),
+                    cmd_held: false,
+                },
+                cx,
+            );
+        });
+        cx.foreground().run_until_parked();
+
+        // Assert no link highlights
+        cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
+            fn test()
+                do_work();
+            
+            fn do_work()
+                test();"});
+
+        let symbol_range = cx.lsp_range(indoc! {"
+            fn test()
+                do_work();
+            
+            fn do_work()
+                [test]();"});
+        let target_range = cx.lsp_range(indoc! {"
+            fn [test]()
+                do_work();
+            
+            fn do_work()
+                test();"});
+
+        let mut requests =
+            cx.lsp
+                .handle_request::<lsp::request::GotoDefinition, _, _>(move |_, _| async move {
+                    Ok(Some(lsp::GotoDefinitionResponse::Link(vec![
+                        lsp::LocationLink {
+                            origin_selection_range: Some(symbol_range),
+                            target_uri: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
+                            target_range,
+                            target_selection_range: target_range,
+                        },
+                    ])))
+                });
+        cx.update_editor(|editor, cx| {
+            cmd_changed(editor, &CmdChanged { cmd_down: true }, cx);
+        });
+        requests.next().await;
+        cx.foreground().run_until_parked();
+
+        cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
+            fn test()
+                do_work();
+            
+            fn do_work()
+                [test]();"});
+
+        // Moving within symbol range doesn't re-request
+        let hover_point = cx.display_point(indoc! {"
+            fn test()
+                do_work();
+            
+            fn do_work()
+                tes|t();"});
+        cx.update_editor(|editor, cx| {
+            update_go_to_definition_link(
+                editor,
+                &UpdateGoToDefinitionLink {
+                    point: Some(hover_point),
+                    cmd_held: true,
+                },
+                cx,
+            );
+        });
+        cx.foreground().run_until_parked();
+        cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
+            fn test()
+                do_work();
+            
+            fn do_work()
+                [test]();"});
+
+        // 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);
+        });
+        // Assert selection moved to to definition
+        cx.lsp
+            .handle_request::<lsp::request::GotoDefinition, _, _>(move |_, _| async move {
+                // Empty definition response to make sure we aren't hitting the lsp and using
+                // the cached location instead
+                Ok(Some(lsp::GotoDefinitionResponse::Link(vec![])))
+            });
+        cx.assert_editor_state(indoc! {"
+            fn [test}()
+                do_work();
+            
+            fn do_work()
+                test();"});
+        // Assert no link highlights after jump
+        cx.assert_editor_text_highlights::<LinkGoToDefinitionState>(indoc! {"
+            fn test()
+                do_work();
+            
+            fn do_work()
+                test();"});
+
+        // Cmd click without existing definition requests and jumps
+        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.lsp
+                .handle_request::<lsp::request::GotoDefinition, _, _>(move |_, _| async move {
+                    Ok(Some(lsp::GotoDefinitionResponse::Link(vec![
+                        lsp::LocationLink {
+                            origin_selection_range: None,
+                            target_uri: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
+                            target_range,
+                            target_selection_range: target_range,
+                        },
+                    ])))
+                });
+        cx.update_workspace(|workspace, cx| {
+            go_to_fetched_definition(workspace, &GoToFetchedDefinition { point: hover_point }, cx);
+        });
+        requests.next().await;
+        cx.foreground().run_until_parked();
+
+        cx.assert_editor_state(indoc! {"
+            fn test()
+                do_work();
+            
+            fn [do_work}()
+                test();"});
+    }
 }

crates/editor/src/selections_collection.rs

@@ -536,7 +536,6 @@ impl<'a> MutableSelectionsCollection<'a> {
         self.select_anchors(selections)
     }
 
-    #[cfg(any(test, feature = "test-support"))]
     pub fn select_display_ranges<T>(&mut self, ranges: T)
     where
         T: IntoIterator<Item = Range<DisplayPoint>>,

crates/editor/src/test.rs

@@ -7,19 +7,20 @@ use futures::StreamExt;
 use indoc::indoc;
 
 use collections::BTreeMap;
-use gpui::{keymap::Keystroke, AppContext, ModelHandle, ViewContext, ViewHandle};
+use gpui::{json, keymap::Keystroke, AppContext, ModelHandle, ViewContext, ViewHandle};
 use language::{point_to_lsp, FakeLspAdapter, Language, LanguageConfig, Selection};
-use project::{FakeFs, Project};
+use project::Project;
 use settings::Settings;
 use util::{
-    set_eq,
+    assert_set_eq, set_eq,
     test::{marked_text, marked_text_ranges, marked_text_ranges_by, SetEqError},
 };
+use workspace::{pane, AppState, Workspace, WorkspaceHandle};
 
 use crate::{
     display_map::{DisplayMap, DisplaySnapshot, ToDisplayPoint},
     multi_buffer::ToPointUtf16,
-    Autoscroll, DisplayPoint, Editor, EditorMode, MultiBuffer, ToPoint,
+    AnchorRangeExt, Autoscroll, DisplayPoint, Editor, EditorMode, MultiBuffer, ToPoint,
 };
 
 #[cfg(test)]
@@ -215,6 +216,24 @@ impl<'a> EditorTestContext<'a> {
         )
     }
 
+    pub fn assert_editor_text_highlights<Tag: ?Sized + 'static>(&mut self, marked_text: &str) {
+        let (unmarked, mut ranges) = marked_text_ranges_by(marked_text, vec![('[', ']').into()]);
+        assert_eq!(unmarked, self.buffer_text());
+
+        let asserted_ranges = ranges.remove(&('[', ']').into()).unwrap();
+        let snapshot = self.update_editor(|editor, cx| editor.snapshot(cx));
+        let actual_ranges: Vec<Range<usize>> = snapshot
+            .display_snapshot
+            .highlight_ranges::<Tag>()
+            .map(|ranges| ranges.as_ref().clone().1)
+            .unwrap_or_default()
+            .into_iter()
+            .map(|range| range.to_offset(&snapshot.buffer_snapshot))
+            .collect();
+
+        assert_set_eq!(asserted_ranges, actual_ranges);
+    }
+
     pub fn assert_editor_selections(&mut self, expected_selections: Vec<Selection<usize>>) {
         let mut empty_selections = Vec::new();
         let mut reverse_selections = Vec::new();
@@ -390,6 +409,7 @@ impl<'a> DerefMut for EditorTestContext<'a> {
 pub struct EditorLspTestContext<'a> {
     pub cx: EditorTestContext<'a>,
     pub lsp: lsp::FakeLanguageServer,
+    pub workspace: ViewHandle<Workspace>,
 }
 
 impl<'a> EditorLspTestContext<'a> {
@@ -398,8 +418,17 @@ impl<'a> EditorLspTestContext<'a> {
         capabilities: lsp::ServerCapabilities,
         cx: &'a mut gpui::TestAppContext,
     ) -> EditorLspTestContext<'a> {
+        use json::json;
+
+        cx.update(|cx| {
+            crate::init(cx);
+            pane::init(cx);
+        });
+
+        let params = cx.update(AppState::test);
+
         let file_name = format!(
-            "/file.{}",
+            "file.{}",
             language
                 .path_suffixes()
                 .first()
@@ -411,30 +440,36 @@ impl<'a> EditorLspTestContext<'a> {
             ..Default::default()
         });
 
-        let fs = FakeFs::new(cx.background().clone());
-        fs.insert_file(file_name.clone(), "".to_string()).await;
-
-        let project = Project::test(fs, [file_name.as_ref()], cx).await;
+        let project = Project::test(params.fs.clone(), [], cx).await;
         project.update(cx, |project, _| project.languages().add(Arc::new(language)));
-        let buffer = project
-            .update(cx, |project, cx| project.open_local_buffer(file_name, cx))
+
+        params
+            .fs
+            .as_fake()
+            .insert_tree("/root", json!({ "dir": { file_name: "" }}))
+            .await;
+
+        let (window_id, workspace) = cx.add_window(|cx| Workspace::new(project.clone(), cx));
+        project
+            .update(cx, |project, cx| {
+                project.find_or_create_local_worktree("/root", true, cx)
+            })
             .await
             .unwrap();
+        cx.read(|cx| workspace.read(cx).worktree_scans_complete(cx))
+            .await;
 
-        let (window_id, editor) = cx.update(|cx| {
-            cx.set_global(Settings::test(cx));
-            crate::init(cx);
+        let file = cx.read(|cx| workspace.file_project_paths(cx)[0].clone());
+        let item = workspace
+            .update(cx, |workspace, cx| workspace.open_path(file, true, cx))
+            .await
+            .expect("Could not open test file");
 
-            let (window_id, editor) = cx.add_window(Default::default(), |cx| {
-                let buffer = cx.add_model(|cx| MultiBuffer::singleton(buffer, cx));
-
-                Editor::new(EditorMode::Full, buffer, Some(project), None, cx)
-            });
-
-            editor.update(cx, |_, cx| cx.focus_self());
-
-            (window_id, editor)
+        let editor = cx.update(|cx| {
+            item.act_as::<Editor>(cx)
+                .expect("Opened test file wasn't an editor")
         });
+        editor.update(cx, |_, cx| cx.focus_self());
 
         let lsp = fake_servers.next().await.unwrap();
 
@@ -445,6 +480,7 @@ impl<'a> EditorLspTestContext<'a> {
                 editor,
             },
             lsp,
+            workspace,
         }
     }
 
@@ -493,6 +529,13 @@ impl<'a> EditorLspTestContext<'a> {
             lsp::Range { start, end }
         })
     }
+
+    pub fn update_workspace<F, T>(&mut self, update: F) -> T
+    where
+        F: FnOnce(&mut Workspace, &mut ViewContext<Workspace>) -> T,
+    {
+        self.workspace.update(self.cx.cx, update)
+    }
 }
 
 impl<'a> Deref for EditorLspTestContext<'a> {