Improve terminal hover tooltips (#26487)

Kirill Bulatov created

Follow-up of https://github.com/zed-industries/zed/pull/26174

* Fixes `./path/foo.bar` not properly parsed as valid open target
* Shows full open target's path in cmd-hover tooltips

Before:

<img width="864" alt="before_1"
src="https://github.com/user-attachments/assets/2575b887-6c4d-486e-8e92-dd76aedf8103"
/>
<img width="864" alt="before_2"
src="https://github.com/user-attachments/assets/ded1f203-523c-4b75-afe9-fe541c785798"
/>

After:

<img width="864" alt="after_1"
src="https://github.com/user-attachments/assets/c50d9ba3-5dfb-4cfb-aed6-00e6fa6f088e"
/>
<img width="864" alt="after_2"
src="https://github.com/user-attachments/assets/0cdc8f34-7faa-4aab-87f3-dc0c8b499842"
/>

Release Notes:



- N/A

Change summary

crates/terminal_view/src/terminal_element.rs |  28 ++--
crates/terminal_view/src/terminal_view.rs    | 104 +++++++++++++--------
2 files changed, 79 insertions(+), 53 deletions(-)

Detailed changes

crates/terminal_view/src/terminal_element.rs 🔗

@@ -21,7 +21,7 @@ use terminal::{
         },
     },
     terminal_settings::TerminalSettings,
-    HoveredWord, IndexedCell, Terminal, TerminalBounds, TerminalContent,
+    IndexedCell, Terminal, TerminalBounds, TerminalContent,
 };
 use theme::{ActiveTheme, Theme, ThemeSettings};
 use ui::{ParentElement, Tooltip};
@@ -45,7 +45,6 @@ pub struct LayoutState {
     display_offset: usize,
     hyperlink_tooltip: Option<AnyElement>,
     gutter: Pixels,
-    last_hovered_word: Option<HoveredWord>,
     block_below_cursor_element: Option<AnyElement>,
 }
 
@@ -157,7 +156,6 @@ pub struct TerminalElement {
     focus: FocusHandle,
     focused: bool,
     cursor_visible: bool,
-    can_navigate_to_selected_word: bool,
     interactivity: Interactivity,
     block_below_cursor: Option<Rc<BlockProperties>>,
 }
@@ -178,7 +176,6 @@ impl TerminalElement {
         focus: FocusHandle,
         focused: bool,
         cursor_visible: bool,
-        can_navigate_to_selected_word: bool,
         block_below_cursor: Option<Rc<BlockProperties>>,
     ) -> TerminalElement {
         TerminalElement {
@@ -188,7 +185,6 @@ impl TerminalElement {
             focused,
             focus: focus.clone(),
             cursor_visible,
-            can_navigate_to_selected_word,
             block_below_cursor,
             interactivity: Default::default(),
         }
@@ -695,27 +691,29 @@ impl Element for TerminalElement {
 
                 let background_color = theme.colors().terminal_background;
 
-                let last_hovered_word = self.terminal.update(cx, |terminal, cx| {
+                let (last_hovered_word, hover_target) = self.terminal.update(cx, |terminal, cx| {
                     terminal.set_size(dimensions);
                     terminal.sync(window, cx);
 
-                    if self.can_navigate_to_selected_word
-                        && window.modifiers().secondary()
+                    if window.modifiers().secondary()
                         && bounds.contains(&window.mouse_position())
+                        && self.terminal_view.read(cx).hover_target_tooltip.is_some()
                     {
-                        terminal.last_content.last_hovered_word.clone()
+                        let hover_target = self.terminal_view.read(cx).hover_target_tooltip.clone();
+                        let last_hovered_word = terminal.last_content.last_hovered_word.clone();
+                        (last_hovered_word, hover_target)
                     } else {
-                        None
+                        (None, None)
                     }
                 });
 
                 let scroll_top = self.terminal_view.read(cx).scroll_top;
-                let hyperlink_tooltip = last_hovered_word.clone().map(|hovered_word| {
+                let hyperlink_tooltip = hover_target.as_ref().map(|hover_target| {
                     let offset = bounds.origin + point(gutter, px(0.)) - point(px(0.), scroll_top);
                     let mut element = div()
                         .size_full()
                         .id("terminal-element")
-                        .tooltip(Tooltip::text(hovered_word.word.clone()))
+                        .tooltip(Tooltip::text(hover_target.clone()))
                         .into_any_element();
                     element.prepaint_as_root(offset, bounds.size.into(), window, cx);
                     element
@@ -851,7 +849,6 @@ impl Element for TerminalElement {
                     display_offset,
                     hyperlink_tooltip,
                     gutter,
-                    last_hovered_word,
                     block_below_cursor_element,
                 }
             },
@@ -884,7 +881,10 @@ impl Element for TerminalElement {
             };
 
             self.register_mouse_listeners(layout.mode, &layout.hitbox, window);
-            if self.can_navigate_to_selected_word && layout.last_hovered_word.is_some() {
+            if window.modifiers().secondary()
+                && bounds.contains(&window.mouse_position())
+                && self.terminal_view.read(cx).hover_target_tooltip.is_some()
+            {
                 window.set_cursor_style(gpui::CursorStyle::PointingHand, &layout.hitbox);
             } else {
                 window.set_cursor_style(gpui::CursorStyle::IBeam, &layout.hitbox);

crates/terminal_view/src/terminal_view.rs 🔗

@@ -114,7 +114,7 @@ pub struct TerminalView {
     blinking_terminal_enabled: bool,
     blinking_paused: bool,
     blink_epoch: usize,
-    can_navigate_to_selected_word: bool,
+    hover_target_tooltip: Option<String>,
     workspace_id: Option<WorkspaceId>,
     show_breadcrumbs: bool,
     block_below_cursor: Option<Rc<BlockProperties>>,
@@ -196,7 +196,7 @@ impl TerminalView {
             blinking_terminal_enabled: false,
             blinking_paused: false,
             blink_epoch: 0,
-            can_navigate_to_selected_word: false,
+            hover_target_tooltip: None,
             workspace_id,
             show_breadcrumbs: TerminalSettings::get_global(cx).toolbar.breadcrumbs,
             block_below_cursor: None,
@@ -869,19 +869,25 @@ fn subscribe_for_terminal_events(
             }
 
             Event::NewNavigationTarget(maybe_navigation_target) => {
-                this.can_navigate_to_selected_word = match maybe_navigation_target {
-                    Some(MaybeNavigationTarget::Url(_)) => true,
-                    Some(MaybeNavigationTarget::PathLike(path_like_target)) => {
-                        let valid_files_to_open_task = possible_open_target(
-                            &workspace,
-                            &path_like_target.terminal_dir,
-                            &path_like_target.maybe_path,
-                            cx,
-                        );
-                        smol::block_on(valid_files_to_open_task).is_some()
-                    }
-                    None => false,
-                };
+                this.hover_target_tooltip =
+                    maybe_navigation_target
+                        .as_ref()
+                        .and_then(|navigation_target| match navigation_target {
+                            MaybeNavigationTarget::Url(url) => Some(url.clone()),
+                            MaybeNavigationTarget::PathLike(path_like_target) => {
+                                let valid_files_to_open_task = possible_open_target(
+                                    &workspace,
+                                    &path_like_target.terminal_dir,
+                                    &path_like_target.maybe_path,
+                                    cx,
+                                );
+                                Some(match smol::block_on(valid_files_to_open_task)? {
+                                    OpenTarget::File(path, _) | OpenTarget::Worktree(path, _) => {
+                                        path.to_string(|path| path.to_string_lossy().to_string())
+                                    }
+                                })
+                            }
+                        });
                 cx.notify()
             }
 
@@ -889,7 +895,7 @@ fn subscribe_for_terminal_events(
                 MaybeNavigationTarget::Url(url) => cx.open_url(url),
 
                 MaybeNavigationTarget::PathLike(path_like_target) => {
-                    if !this.can_navigate_to_selected_word {
+                    if this.hover_target_tooltip.is_none() {
                         return;
                     }
                     let task_workspace = workspace.clone();
@@ -905,7 +911,8 @@ fn subscribe_for_terminal_events(
                                 )
                             })?
                             .await;
-                        if let Some((path_to_open, open_target)) = open_target {
+                        if let Some(open_target) = open_target {
+                            let path_to_open = open_target.path();
                             let opened_items = task_workspace
                                 .update_in(&mut cx, |workspace, window, cx| {
                                     workspace.open_paths(
@@ -980,22 +987,29 @@ fn subscribe_for_terminal_events(
 
 #[derive(Debug, Clone)]
 enum OpenTarget {
-    Worktree(Entry),
-    File(Metadata),
+    Worktree(PathWithPosition, Entry),
+    File(PathWithPosition, Metadata),
 }
 
 impl OpenTarget {
     fn is_file(&self) -> bool {
         match self {
-            OpenTarget::Worktree(entry) => entry.is_file(),
-            OpenTarget::File(metadata) => !metadata.is_dir,
+            OpenTarget::Worktree(_, entry) => entry.is_file(),
+            OpenTarget::File(_, metadata) => !metadata.is_dir,
         }
     }
 
     fn is_dir(&self) -> bool {
         match self {
-            OpenTarget::Worktree(entry) => entry.is_dir(),
-            OpenTarget::File(metadata) => metadata.is_dir,
+            OpenTarget::Worktree(_, entry) => entry.is_dir(),
+            OpenTarget::File(_, metadata) => metadata.is_dir,
+        }
+    }
+
+    fn path(&self) -> &PathWithPosition {
+        match self {
+            OpenTarget::Worktree(path, _) => path,
+            OpenTarget::File(path, _) => path,
         }
     }
 }
@@ -1005,7 +1019,7 @@ fn possible_open_target(
     cwd: &Option<PathBuf>,
     maybe_path: &str,
     cx: &mut Context<TerminalView>,
-) -> Task<Option<(PathWithPosition, OpenTarget)>> {
+) -> Task<Option<OpenTarget>> {
     let Some(workspace) = workspace.upgrade() else {
         return Task::ready(None);
     };
@@ -1014,7 +1028,22 @@ fn possible_open_target(
     let mut potential_paths = Vec::new();
     let original_path = PathWithPosition::from_path(PathBuf::from(maybe_path));
     let path_with_position = PathWithPosition::parse_str(maybe_path);
-    for prefix_str in GIT_DIFF_PATH_PREFIXES {
+    let worktree_candidates = workspace
+        .read(cx)
+        .worktrees(cx)
+        .sorted_by_key(|worktree| {
+            let worktree_root = worktree.read(cx).abs_path();
+            match cwd
+                .as_ref()
+                .and_then(|cwd| worktree_root.strip_prefix(cwd).ok())
+            {
+                Some(cwd_child) => cwd_child.components().count(),
+                None => usize::MAX,
+            }
+        })
+        .collect::<Vec<_>>();
+    // Since we do not check paths via FS and joining, we need to strip off potential `./`, `a/`, `b/` prefixes out of it.
+    for prefix_str in GIT_DIFF_PATH_PREFIXES.iter().chain(std::iter::once(&".")) {
         if let Some(stripped) = original_path.path.strip_prefix(prefix_str).ok() {
             potential_paths.push(PathWithPosition {
                 path: stripped.to_owned(),
@@ -1033,16 +1062,7 @@ fn possible_open_target(
     potential_paths.insert(0, original_path);
     potential_paths.insert(1, path_with_position);
 
-    for worktree in workspace.read(cx).worktrees(cx).sorted_by_key(|worktree| {
-        let worktree_root = worktree.read(cx).abs_path();
-        match cwd
-            .as_ref()
-            .and_then(|cwd| worktree_root.strip_prefix(cwd).ok())
-        {
-            Some(cwd_child) => cwd_child.components().count(),
-            None => usize::MAX,
-        }
-    }) {
+    for worktree in &worktree_candidates {
         let worktree_root = worktree.read(cx).abs_path();
         let paths_to_check = potential_paths
             .iter()
@@ -1065,13 +1085,13 @@ fn possible_open_target(
                 .iter()
                 .find(|path_to_check| entry.path.ends_with(&path_to_check.path))
             {
-                return Task::ready(Some((
+                return Task::ready(Some(OpenTarget::Worktree(
                     PathWithPosition {
                         path: worktree_root.join(&entry.path),
                         row: path_in_worktree.row,
                         column: path_in_worktree.column,
                     },
-                    OpenTarget::Worktree(entry.clone()),
+                    entry.clone(),
                 )));
             }
         }
@@ -1116,6 +1136,13 @@ fn possible_open_target(
                             column: path_to_check.column,
                         });
                     }
+                    for worktree in &worktree_candidates {
+                        paths_to_check.push(PathWithPosition {
+                            path: worktree.read(cx).abs_path().join(maybe_path),
+                            row: path_to_check.row,
+                            column: path_to_check.column,
+                        });
+                    }
                 }
             }
             paths_to_check
@@ -1125,7 +1152,7 @@ fn possible_open_target(
     cx.background_spawn(async move {
         for path_to_check in paths_to_check {
             if let Some(metadata) = fs.metadata(&path_to_check.path).await.ok().flatten() {
-                return Some((path_to_check, OpenTarget::File(metadata)));
+                return Some(OpenTarget::File(path_to_check, metadata));
             }
         }
         None
@@ -1247,7 +1274,6 @@ impl Render for TerminalView {
                         self.focus_handle.clone(),
                         focused,
                         self.should_show_cursor(focused, cx),
-                        self.can_navigate_to_selected_word,
                         self.block_below_cursor.clone(),
                     ))
                     .when_some(self.render_scrollbar(cx), |div, scrollbar| {