Open URLs with cmd-click (#7312)

Release Notes:

- Added ability to cmd-click on URLs in all buffers

---------

Co-authored-by: fdionisi <code@fdionisi.me>

Cargo.lock

@@ -2416,6 +2416,7 @@ dependencies = [
  "itertools 0.10.5",
  "language",
  "lazy_static",
+ "linkify",
  "log",
  "lsp",
  "multi_buffer",
@@ -4134,6 +4135,15 @@ dependencies = [
  "safemem",
 ]
 
+[[package]]
+name = "linkify"
+version = "0.10.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f1dfa36d52c581e9ec783a7ce2a5e0143da6237be5811a0b3153fedfdbe9f780"
+dependencies = [
+ "memchr",
+]
+
 [[package]]
 name = "linkme"
 version = "0.3.17"

crates/editor/Cargo.toml

@@ -20,7 +20,7 @@ test-support = [
     "util/test-support",
     "workspace/test-support",
     "tree-sitter-rust",
-    "tree-sitter-typescript"
+    "tree-sitter-typescript",
 ]
 
 [dependencies]
@@ -33,13 +33,14 @@ convert_case = "0.6.0"
 copilot = { path = "../copilot" }
 db = { path = "../db" }
 futures.workspace = true
-fuzzy = {  path = "../fuzzy" }
+fuzzy = { path = "../fuzzy" }
 git = { path = "../git" }
 gpui = { path = "../gpui" }
 indoc = "1.0.4"
 itertools = "0.10"
 language = { path = "../language" }
 lazy_static.workspace = true
+linkify = "0.10.0"
 log.workspace = true
 lsp = { path = "../lsp" }
 multi_buffer = { path = "../multi_buffer" }

crates/editor/src/display_map.rs

@@ -25,8 +25,8 @@ mod wrap_map;
 
 use crate::EditorStyle;
 use crate::{
-    link_go_to_definition::InlayHighlight, movement::TextLayoutDetails, Anchor, AnchorRangeExt,
-    InlayId, MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint,
+    hover_links::InlayHighlight, movement::TextLayoutDetails, Anchor, AnchorRangeExt, InlayId,
+    MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint,
 };
 pub use block_map::{BlockMap, BlockPoint};
 use collections::{BTreeMap, HashMap, HashSet};

crates/editor/src/display_map/inlay_map.rs

@@ -1168,7 +1168,7 @@ mod tests {
     use super::*;
     use crate::{
         display_map::{InlayHighlights, TextHighlights},
-        link_go_to_definition::InlayHighlight,
+        hover_links::InlayHighlight,
         InlayId, MultiBuffer,
     };
     use gpui::AppContext;

crates/editor/src/editor.rs

@@ -22,9 +22,9 @@ mod inlay_hint_cache;
 mod debounced_delay;
 mod git;
 mod highlight_matching_bracket;
+mod hover_links;
 mod hover_popover;
 pub mod items;
-mod link_go_to_definition;
 mod mouse_context_menu;
 pub mod movement;
 mod persistence;
@@ -77,7 +77,7 @@ use language::{
     Language, OffsetRangeExt, Point, Selection, SelectionGoal, TransactionId,
 };
 
-use link_go_to_definition::{GoToDefinitionLink, InlayHighlight, LinkGoToDefinitionState};
+use hover_links::{HoverLink, HoveredLinkState, InlayHighlight};
 use lsp::{DiagnosticSeverity, LanguageServerId};
 use mouse_context_menu::MouseContextMenu;
 use movement::TextLayoutDetails;
@@ -402,7 +402,7 @@ pub struct Editor {
     remote_id: Option<ViewId>,
     hover_state: HoverState,
     gutter_hovered: bool,
-    link_go_to_definition_state: LinkGoToDefinitionState,
+    hovered_link_state: Option<HoveredLinkState>,
     copilot_state: CopilotState,
     inlay_hint_cache: InlayHintCache,
     next_inlay_id: usize,
@@ -1477,7 +1477,7 @@ impl Editor {
             leader_peer_id: None,
             remote_id: None,
             hover_state: Default::default(),
-            link_go_to_definition_state: Default::default(),
+            hovered_link_state: Default::default(),
             copilot_state: Default::default(),
             inlay_hint_cache: InlayHintCache::new(inlay_hint_settings),
             gutter_hovered: false,
@@ -7243,11 +7243,8 @@ impl Editor {
         cx.spawn(|editor, mut cx| async move {
             let definitions = definitions.await?;
             editor.update(&mut cx, |editor, cx| {
-                editor.navigate_to_definitions(
-                    definitions
-                        .into_iter()
-                        .map(GoToDefinitionLink::Text)
-                        .collect(),
+                editor.navigate_to_hover_links(
+                    definitions.into_iter().map(HoverLink::Text).collect(),
                     split,
                     cx,
                 );
@@ -7257,9 +7254,9 @@ impl Editor {
         .detach_and_log_err(cx);
     }
 
-    pub fn navigate_to_definitions(
+    pub fn navigate_to_hover_links(
         &mut self,
-        mut definitions: Vec<GoToDefinitionLink>,
+        mut definitions: Vec<HoverLink>,
         split: bool,
         cx: &mut ViewContext<Editor>,
     ) {
@@ -7271,10 +7268,14 @@ impl Editor {
         if definitions.len() == 1 {
             let definition = definitions.pop().unwrap();
             let target_task = match definition {
-                GoToDefinitionLink::Text(link) => Task::Ready(Some(Ok(Some(link.target)))),
-                GoToDefinitionLink::InlayHint(lsp_location, server_id) => {
+                HoverLink::Text(link) => Task::Ready(Some(Ok(Some(link.target)))),
+                HoverLink::InlayHint(lsp_location, server_id) => {
                     self.compute_target_location(lsp_location, server_id, cx)
                 }
+                HoverLink::Url(url) => {
+                    cx.open_url(&url);
+                    Task::ready(Ok(None))
+                }
             };
             cx.spawn(|editor, mut cx| async move {
                 let target = target_task.await.context("target resolution task")?;
@@ -7325,29 +7326,27 @@ impl Editor {
                         let title = definitions
                             .iter()
                             .find_map(|definition| match definition {
-                                GoToDefinitionLink::Text(link) => {
-                                    link.origin.as_ref().map(|origin| {
-                                        let buffer = origin.buffer.read(cx);
-                                        format!(
-                                            "Definitions for {}",
-                                            buffer
-                                                .text_for_range(origin.range.clone())
-                                                .collect::<String>()
-                                        )
-                                    })
-                                }
-                                GoToDefinitionLink::InlayHint(_, _) => None,
+                                HoverLink::Text(link) => link.origin.as_ref().map(|origin| {
+                                    let buffer = origin.buffer.read(cx);
+                                    format!(
+                                        "Definitions for {}",
+                                        buffer
+                                            .text_for_range(origin.range.clone())
+                                            .collect::<String>()
+                                    )
+                                }),
+                                HoverLink::InlayHint(_, _) => None,
+                                HoverLink::Url(_) => None,
                             })
                             .unwrap_or("Definitions".to_string());
                         let location_tasks = definitions
                             .into_iter()
                             .map(|definition| match definition {
-                                GoToDefinitionLink::Text(link) => {
-                                    Task::Ready(Some(Ok(Some(link.target))))
-                                }
-                                GoToDefinitionLink::InlayHint(lsp_location, server_id) => {
+                                HoverLink::Text(link) => Task::Ready(Some(Ok(Some(link.target)))),
+                                HoverLink::InlayHint(lsp_location, server_id) => {
                                     editor.compute_target_location(lsp_location, server_id, cx)
                                 }
+                                HoverLink::Url(_) => Task::ready(Ok(None)),
                             })
                             .collect::<Vec<_>>();
                         (title, location_tasks)

crates/editor/src/element.rs

@@ -9,11 +9,6 @@ use crate::{
         self, hover_at, HOVER_POPOVER_GAP, MIN_POPOVER_CHARACTER_WIDTH, MIN_POPOVER_LINE_HEIGHT,
     },
     items::BufferSearchHighlights,
-    link_go_to_definition::{
-        go_to_fetched_definition, go_to_fetched_type_definition, show_link_definition,
-        update_go_to_definition_link, update_inlay_link_and_hover_points, GoToDefinitionTrigger,
-        LinkGoToDefinitionState,
-    },
     mouse_context_menu,
     scroll::scroll_amount::ScrollAmount,
     CursorShape, DisplayPoint, DocumentHighlightRead, DocumentHighlightWrite, Editor, EditorMode,
@@ -337,7 +332,14 @@ impl EditorElement {
         register_action(view, cx, Editor::display_cursor_names);
     }
 
-    fn register_key_listeners(&self, cx: &mut ElementContext) {
+    fn register_key_listeners(
+        &self,
+        cx: &mut ElementContext,
+        text_bounds: Bounds<Pixels>,
+        layout: &LayoutState,
+    ) {
+        let position_map = layout.position_map.clone();
+        let stacking_order = cx.stacking_order().clone();
         cx.on_key_event({
             let editor = self.editor.clone();
             move |event: &ModifiersChangedEvent, phase, cx| {
@@ -345,46 +347,41 @@ impl EditorElement {
                     return;
                 }
 
-                if editor.update(cx, |editor, cx| Self::modifiers_changed(editor, event, cx)) {
-                    cx.stop_propagation();
-                }
+                editor.update(cx, |editor, cx| {
+                    Self::modifiers_changed(
+                        editor,
+                        event,
+                        &position_map,
+                        text_bounds,
+                        &stacking_order,
+                        cx,
+                    )
+                })
             }
         });
     }
 
-    pub(crate) fn modifiers_changed(
+    fn modifiers_changed(
         editor: &mut Editor,
         event: &ModifiersChangedEvent,
+        position_map: &PositionMap,
+        text_bounds: Bounds<Pixels>,
+        stacking_order: &StackingOrder,
         cx: &mut ViewContext<Editor>,
-    ) -> bool {
-        let pending_selection = editor.has_pending_selection();
-
-        if let Some(point) = &editor.link_go_to_definition_state.last_trigger_point {
-            if event.command && !pending_selection {
-                let point = point.clone();
-                let snapshot = editor.snapshot(cx);
-                let kind = point.definition_kind(event.shift);
-
-                show_link_definition(kind, editor, point, snapshot, cx);
-                return false;
-            }
-        }
-
+    ) {
+        let mouse_position = cx.mouse_position();
+        if !text_bounds.contains(&mouse_position)
+            || !cx.was_top_layer(&mouse_position, stacking_order)
         {
-            if editor.link_go_to_definition_state.symbol_range.is_some()
-                || !editor.link_go_to_definition_state.definitions.is_empty()
-            {
-                editor.link_go_to_definition_state.symbol_range.take();
-                editor.link_go_to_definition_state.definitions.clear();
-                cx.notify();
-            }
-
-            editor.link_go_to_definition_state.task = None;
-
-            editor.clear_highlights::<LinkGoToDefinitionState>(cx);
+            return;
         }
 
-        false
+        editor.update_hovered_link(
+            position_map.point_for_position(text_bounds, mouse_position),
+            &position_map.snapshot,
+            event.modifiers,
+            cx,
+        )
     }
 
     fn mouse_left_down(
@@ -485,13 +482,7 @@ impl EditorElement {
             && cx.was_top_layer(&event.position, stacking_order)
         {
             let point = position_map.point_for_position(text_bounds, event.position);
-            let could_be_inlay = point.as_valid().is_none();
-            let split = event.modifiers.alt;
-            if event.modifiers.shift || could_be_inlay {
-                go_to_fetched_type_definition(editor, point, split, cx);
-            } else {
-                go_to_fetched_definition(editor, point, split, cx);
-            }
+            editor.handle_click_hovered_link(point, event.modifiers, cx);
 
             cx.stop_propagation();
         } else if end_selection {
@@ -564,31 +555,14 @@ impl EditorElement {
         if text_hovered && was_top {
             let point_for_position = position_map.point_for_position(text_bounds, event.position);
 
-            match point_for_position.as_valid() {
-                Some(point) => {
-                    update_go_to_definition_link(
-                        editor,
-                        Some(GoToDefinitionTrigger::Text(point)),
-                        modifiers.command,
-                        modifiers.shift,
-                        cx,
-                    );
-                    hover_at(editor, Some(point), cx);
-                    Self::update_visible_cursor(editor, point, position_map, cx);
-                }
-                None => {
-                    update_inlay_link_and_hover_points(
-                        &position_map.snapshot,
-                        point_for_position,
-                        editor,
-                        modifiers.command,
-                        modifiers.shift,
-                        cx,
-                    );
-                }
+            editor.update_hovered_link(point_for_position, &position_map.snapshot, modifiers, cx);
+
+            if let Some(point) = point_for_position.as_valid() {
+                hover_at(editor, Some(point), cx);
+                Self::update_visible_cursor(editor, point, position_map, cx);
             }
         } else {
-            update_go_to_definition_link(editor, None, modifiers.command, modifiers.shift, cx);
+            editor.hide_hovered_link(cx);
             hover_at(editor, None, cx);
             if gutter_hovered && was_top {
                 cx.stop_propagation();
@@ -930,13 +904,13 @@ impl EditorElement {
                     if self
                         .editor
                         .read(cx)
-                        .link_go_to_definition_state
-                        .definitions
-                        .is_empty()
+                        .hovered_link_state
+                        .as_ref()
+                        .is_some_and(|hovered_link_state| !hovered_link_state.links.is_empty())
                     {
-                        cx.set_cursor_style(CursorStyle::IBeam);
-                    } else {
                         cx.set_cursor_style(CursorStyle::PointingHand);
+                    } else {
+                        cx.set_cursor_style(CursorStyle::IBeam);
                     }
                 }
 
@@ -3105,9 +3079,9 @@ impl Element for EditorElement {
                     let key_context = self.editor.read(cx).key_context(cx);
                     cx.with_key_dispatch(Some(key_context), Some(focus_handle.clone()), |_, cx| {
                         self.register_actions(cx);
-                        self.register_key_listeners(cx);
 
                         cx.with_content_mask(Some(ContentMask { bounds }), |cx| {
+                            self.register_key_listeners(cx, text_bounds, &layout);
                             cx.handle_input(
                                 &focus_handle,
                                 ElementInputHandler::new(bounds, self.editor.clone()),
@@ -3224,16 +3198,6 @@ pub struct PointForPosition {
 }
 
 impl PointForPosition {
-    #[cfg(test)]
-    pub fn valid(valid: DisplayPoint) -> Self {
-        Self {
-            previous_valid: valid,
-            next_valid: valid,
-            exact_unclipped: valid,
-            column_overshoot_after_line_end: 0,
-        }
-    }
-
     pub fn as_valid(&self) -> Option<DisplayPoint> {
         if self.previous_valid == self.exact_unclipped && self.next_valid == self.exact_unclipped {
             Some(self.previous_valid)

crates/editor/src/hover_popover.rs

@@ -1,6 +1,6 @@
 use crate::{
     display_map::{InlayOffset, ToDisplayPoint},
-    link_go_to_definition::{InlayHighlight, RangeInEditor},
+    hover_links::{InlayHighlight, RangeInEditor},
     Anchor, AnchorRangeExt, DisplayPoint, Editor, EditorSettings, EditorSnapshot, EditorStyle,
     ExcerptId, Hover, RangeToAnchorExt,
 };
@@ -605,8 +605,8 @@ mod tests {
     use crate::{
         editor_tests::init_test,
         element::PointForPosition,
+        hover_links::update_inlay_link_and_hover_points,
         inlay_hint_cache::tests::{cached_hint_labels, visible_hint_labels},
-        link_go_to_definition::update_inlay_link_and_hover_points,
         test::editor_lsp_test_context::EditorLspTestContext,
         InlayId,
     };

crates/editor/src/items.rs

@@ -1,7 +1,7 @@
 use crate::{
-    editor_settings::SeedQuerySetting, link_go_to_definition::hide_link_definition,
-    persistence::DB, scroll::ScrollAnchor, Anchor, Autoscroll, Editor, EditorEvent, EditorSettings,
-    ExcerptId, ExcerptRange, MultiBuffer, MultiBufferSnapshot, NavigationData, ToPoint as _,
+    editor_settings::SeedQuerySetting, persistence::DB, scroll::ScrollAnchor, Anchor, Autoscroll,
+    Editor, EditorEvent, EditorSettings, ExcerptId, ExcerptRange, MultiBuffer, MultiBufferSnapshot,
+    NavigationData, ToPoint as _,
 };
 use anyhow::{anyhow, Context as _, Result};
 use collections::HashSet;
@@ -682,8 +682,7 @@ impl Item for Editor {
     }
 
     fn workspace_deactivated(&mut self, cx: &mut ViewContext<Self>) {
-        hide_link_definition(self, cx);
-        self.link_go_to_definition_state.last_trigger_point = None;
+        self.hide_hovered_link(cx);
     }
 
     fn is_dirty(&self, cx: &AppContext) -> bool {

crates/editor/src/test/editor_test_context.rs

@@ -4,7 +4,8 @@ use crate::{
 use collections::BTreeMap;
 use futures::Future;
 use gpui::{
-    AnyWindowHandle, AppContext, Keystroke, ModelContext, View, ViewContext, VisualTestContext,
+    AnyWindowHandle, AppContext, Keystroke, ModelContext, Pixels, Point, View, ViewContext,
+    VisualTestContext,
 };
 use indoc::indoc;
 use itertools::Itertools;
@@ -187,6 +188,31 @@ impl EditorTestContext {
         ranges[0].start.to_display_point(&snapshot)
     }
 
+    pub fn pixel_position(&mut self, marked_text: &str) -> Point<Pixels> {
+        let display_point = self.display_point(marked_text);
+        self.pixel_position_for(display_point)
+    }
+
+    pub fn pixel_position_for(&mut self, display_point: DisplayPoint) -> Point<Pixels> {
+        self.update_editor(|editor, cx| {
+            let newest_point = editor.selections.newest_display(cx).head();
+            let pixel_position = editor.pixel_position_of_newest_cursor.unwrap();
+            let line_height = editor
+                .style()
+                .unwrap()
+                .text
+                .line_height_in_pixels(cx.rem_size());
+            let snapshot = editor.snapshot(cx);
+            let details = editor.text_layout_details(cx);
+
+            let y = pixel_position.y
+                + line_height * (display_point.row() as f32 - newest_point.row() as f32);
+            let x = pixel_position.x + snapshot.x_for_display_point(display_point, &details)
+                - snapshot.x_for_display_point(newest_point, &details);
+            Point::new(x, y)
+        })
+    }
+
     // Returns anchors for the current buffer using `«` and `»`
     pub fn text_anchor_range(&mut self, marked_text: &str) -> Range<language::Anchor> {
         let ranges = self.ranges(marked_text);
@@ -343,7 +369,7 @@ impl EditorTestContext {
 }
 
 impl Deref for EditorTestContext {
-    type Target = gpui::TestAppContext;
+    type Target = gpui::VisualTestContext;
 
     fn deref(&self) -> &Self::Target {
         &self.cx

crates/gpui/src/app/test_context.rs

@@ -1,9 +1,10 @@
 use crate::{
     Action, AnyElement, AnyView, AnyWindowHandle, AppCell, AppContext, AsyncAppContext,
     AvailableSpace, BackgroundExecutor, Bounds, ClipboardItem, Context, Entity, EventEmitter,
-    ForegroundExecutor, Global, InputEvent, Keystroke, Model, ModelContext, Pixels, Platform,
-    Point, Render, Result, Size, Task, TestDispatcher, TestPlatform, TestWindow, TextSystem, View,
-    ViewContext, VisualContext, WindowContext, WindowHandle, WindowOptions,
+    ForegroundExecutor, Global, InputEvent, Keystroke, Model, ModelContext, Modifiers,
+    ModifiersChangedEvent, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels,
+    Platform, Point, Render, Result, Size, Task, TestDispatcher, TestPlatform, TestWindow,
+    TextSystem, View, ViewContext, VisualContext, WindowContext, WindowHandle, WindowOptions,
 };
 use anyhow::{anyhow, bail};
 use futures::{Stream, StreamExt};
@@ -236,6 +237,11 @@ impl TestAppContext {
         self.test_platform.has_pending_prompt()
     }
 
+    /// All the urls that have been opened with cx.open_url() during this test.
+    pub fn opened_url(&self) -> Option<String> {
+        self.test_platform.opened_url.borrow().clone()
+    }
+
     /// Simulates the user resizing the window to the new size.
     pub fn simulate_window_resize(&self, window_handle: AnyWindowHandle, size: Size<Pixels>) {
         self.test_window(window_handle).simulate_resize(size);
@@ -625,6 +631,36 @@ impl<'a> VisualTestContext {
         self.cx.simulate_input(self.window, input)
     }
 
+    /// Simulate a mouse move event to the given point
+    pub fn simulate_mouse_move(&mut self, position: Point<Pixels>, modifiers: Modifiers) {
+        self.simulate_event(MouseMoveEvent {
+            position,
+            modifiers,
+            pressed_button: None,
+        })
+    }
+
+    /// Simulate a primary mouse click at the given point
+    pub fn simulate_click(&mut self, position: Point<Pixels>, modifiers: Modifiers) {
+        self.simulate_event(MouseDownEvent {
+            position,
+            modifiers,
+            button: MouseButton::Left,
+            click_count: 1,
+        });
+        self.simulate_event(MouseUpEvent {
+            position,
+            modifiers,
+            button: MouseButton::Left,
+            click_count: 1,
+        });
+    }
+
+    /// Simulate a modifiers changed event
+    pub fn simulate_modifiers_change(&mut self, modifiers: Modifiers) {
+        self.simulate_event(ModifiersChangedEvent { modifiers })
+    }
+
     /// Simulates the user resizing the window to the new size.
     pub fn simulate_resize(&self, size: Size<Pixels>) {
         self.simulate_window_resize(self.window, size)

crates/gpui/src/platform/keystroke.rs

@@ -170,4 +170,34 @@ impl Modifiers {
     pub fn modified(&self) -> bool {
         self.control || self.alt || self.shift || self.command || self.function
     }
+
+    /// helper method for Modifiers with no modifiers
+    pub fn none() -> Modifiers {
+        Default::default()
+    }
+
+    /// helper method for Modifiers with just command
+    pub fn command() -> Modifiers {
+        Modifiers {
+            command: true,
+            ..Default::default()
+        }
+    }
+
+    /// helper method for Modifiers with just shift
+    pub fn shift() -> Modifiers {
+        Modifiers {
+            shift: true,
+            ..Default::default()
+        }
+    }
+
+    /// helper method for Modifiers with command + shift
+    pub fn command_shift() -> Modifiers {
+        Modifiers {
+            shift: true,
+            command: true,
+            ..Default::default()
+        }
+    }
 }

crates/gpui/src/platform/test/platform.rs

@@ -25,6 +25,7 @@ pub(crate) struct TestPlatform {
     active_cursor: Mutex<CursorStyle>,
     current_clipboard_item: Mutex<Option<ClipboardItem>>,
     pub(crate) prompts: RefCell<TestPrompts>,
+    pub opened_url: RefCell<Option<String>>,
     weak: Weak<Self>,
 }
 
@@ -45,6 +46,7 @@ impl TestPlatform {
             active_window: Default::default(),
             current_clipboard_item: Mutex::new(None),
             weak: weak.clone(),
+            opened_url: Default::default(),
         })
     }
 
@@ -188,8 +190,8 @@ impl Platform for TestPlatform {
 
     fn stop_display_link(&self, _display_id: DisplayId) {}
 
-    fn open_url(&self, _url: &str) {
-        unimplemented!()
+    fn open_url(&self, url: &str) {
+        *self.opened_url.borrow_mut() = Some(url.to_string())
     }
 
     fn on_open_urls(&self, _callback: Box<dyn FnMut(Vec<String>)>) {