Improve cmd-click in terminal to find more paths (#26174)

Closes https://github.com/zed-industries/zed/issues/25701

Reworks the way cmd-click is handled:

* first, all worktree entries are checked for existence

This allows more fine-grained lookup of entries that are in the
worktree, but their path in the terminal is not "full": in case neither
`cwd` no worktree's root + that temrinal paths form a valid path
(https://github.com/zed-industries/zed/issues/25701)

The worktrees are sorted by "the most close to cwd first" so such files
are attempted to resolved in the most specific worktree.

This also fixes no cmd-click working in the remote ssh.

* second, only if the client is local, do the FS checks to find
non-indexed files

Release Notes:

- Improved cmd-click in terminal to find more paths

Cargo.lock

@@ -13610,6 +13610,7 @@ dependencies = [
  "gpui",
  "itertools 0.14.0",
  "language",
+ "log",
  "project",
  "rand 0.8.5",
  "schemars",

crates/terminal_view/Cargo.toml

@@ -27,6 +27,7 @@ futures.workspace = true
 gpui.workspace = true
 itertools.workspace = true
 language.workspace = true
+log.workspace = true
 project.workspace = true
 task.workspace = true
 schemars.workspace = true
@@ -50,3 +51,6 @@ gpui = { workspace = true, features = ["test-support"] }
 project = { workspace = true, features = ["test-support"] }
 rand.workspace = true
 workspace = { workspace = true, features = ["test-support"] }
+
+[package.metadata.cargo-machete]
+ignored = ["log"]

crates/terminal_view/src/terminal_view.rs

@@ -4,16 +4,15 @@ pub mod terminal_panel;
 pub mod terminal_scrollbar;
 pub mod terminal_tab_tooltip;
 
-use collections::HashSet;
 use editor::{actions::SelectAll, scroll::ScrollbarAutoHide, Editor, EditorSettings};
-use futures::{stream::FuturesUnordered, StreamExt};
 use gpui::{
     anchored, deferred, div, impl_actions, AnyElement, App, DismissEvent, Entity, EventEmitter,
     FocusHandle, Focusable, KeyContext, KeyDownEvent, Keystroke, MouseButton, MouseDownEvent,
     Pixels, Render, ScrollWheelEvent, Stateful, Styled, Subscription, Task, WeakEntity,
 };
+use itertools::Itertools;
 use persistence::TERMINAL_DB;
-use project::{search::SearchQuery, terminals::TerminalKind, Fs, Metadata, Project};
+use project::{search::SearchQuery, terminals::TerminalKind, Entry, Metadata, Project};
 use schemars::JsonSchema;
 use terminal::{
     alacritty_terminal::{
@@ -32,10 +31,7 @@ use terminal_tab_tooltip::TerminalTooltip;
 use ui::{
     h_flex, prelude::*, ContextMenu, Icon, IconName, Label, Scrollbar, ScrollbarState, Tooltip,
 };
-use util::{
+use util::{debug_panic, paths::PathWithPosition, ResultExt};
-    paths::{PathWithPosition, SanitizedPath},
-    ResultExt,
-};
 use workspace::{
     item::{
         BreadcrumbText, Item, ItemEvent, SerializableItem, TabContentParams, TabTooltipContent,
@@ -67,7 +63,7 @@ const REGEX_SPECIAL_CHARS: &[char] = &[
 
 const CURSOR_BLINK_INTERVAL: Duration = Duration::from_millis(500);
 
-const GIT_DIFF_PATH_PREFIXES: &[char] = &['a', 'b'];
+const GIT_DIFF_PATH_PREFIXES: &[&str] = &["a", "b"];
 
 /// Event to transmit the scroll from the element to the view
 #[derive(Clone, Debug, PartialEq)]
@@ -876,20 +872,13 @@ fn subscribe_for_terminal_events(
                 this.can_navigate_to_selected_word = match maybe_navigation_target {
                     Some(MaybeNavigationTarget::Url(_)) => true,
                     Some(MaybeNavigationTarget::PathLike(path_like_target)) => {
-                        if let Ok(fs) = workspace.update(cx, |workspace, cx| {
+                        let valid_files_to_open_task = possible_open_target(
-                            workspace.project().read(cx).fs().clone()
+                            &workspace,
-                        }) {
+                            &path_like_target.terminal_dir,
-                            let valid_files_to_open_task = possible_open_targets(
+                            &path_like_target.maybe_path,
-                                fs,
+                            cx,
-                                &workspace,
+                        );
-                                &path_like_target.terminal_dir,
+                        smol::block_on(valid_files_to_open_task).is_some()
-                                &path_like_target.maybe_path,
-                                cx,
-                            );
-                            !smol::block_on(valid_files_to_open_task).is_empty()
-                        } else {
-                            false
-                        }
                     }
                     None => false,
                 };
@@ -904,21 +893,11 @@ fn subscribe_for_terminal_events(
                         return;
                     }
                     let task_workspace = workspace.clone();
-                    let Some(fs) = workspace
-                        .update(cx, |workspace, cx| {
-                            workspace.project().read(cx).fs().clone()
-                        })
-                        .ok()
-                    else {
-                        return;
-                    };
-
                     let path_like_target = path_like_target.clone();
                     cx.spawn_in(window, |terminal_view, mut cx| async move {
-                        let valid_files_to_open = terminal_view
+                        let open_target = terminal_view
                             .update(&mut cx, |_, cx| {
-                                possible_open_targets(
+                                possible_open_target(
-                                    fs,
                                     &task_workspace,
                                     &path_like_target.terminal_dir,
                                     &path_like_target.maybe_path,
@@ -926,60 +905,60 @@ fn subscribe_for_terminal_events(
                                 )
                             })?
                             .await;
-                        let paths_to_open = valid_files_to_open
+                        if let Some((path_to_open, open_target)) = open_target {
-                            .iter()
+                            let opened_items = task_workspace
-                            .map(|(p, _)| p.path.clone())
+                                .update_in(&mut cx, |workspace, window, cx| {
-                            .collect();
+                                    workspace.open_paths(
-                        let opened_items = task_workspace
+                                        vec![path_to_open.path.clone()],
-                            .update_in(&mut cx, |workspace, window, cx| {
+                                        OpenVisible::OnlyDirectories,
-                                workspace.open_paths(
+                                        None,
-                                    paths_to_open,
+                                        window,
-                                    OpenVisible::OnlyDirectories,
+                                        cx,
-                                    None,
+                                    )
-                                    window,
+                                })
-                                    cx,
+                                .context("workspace update")?
-                                )
+                                .await;
-                            })
+                            if opened_items.len() != 1 {
-                            .context("workspace update")?
+                                debug_panic!(
-                            .await;
+                                    "Received {} items for one path {path_to_open:?}",
+                                    opened_items.len(),
+                                );
+                            }
 
-                        let mut has_dirs = false;
+                            if let Some(opened_item) = opened_items.first() {
-                        for ((path, metadata), opened_item) in valid_files_to_open
+                                if open_target.is_file() {
-                            .into_iter()
+                                    if let Some(Ok(opened_item)) = opened_item {
-                            .zip(opened_items.into_iter())
+                                        if let Some(row) = path_to_open.row {
-                        {
+                                            let col = path_to_open.column.unwrap_or(0);
-                            if metadata.is_dir {
+                                            if let Some(active_editor) =
-                                has_dirs = true;
+                                                opened_item.downcast::<Editor>()
-                            } else if let Some(Ok(opened_item)) = opened_item {
+                                            {
-                                if let Some(row) = path.row {
+                                                active_editor
-                                    let col = path.column.unwrap_or(0);
+                                                    .downgrade()
-                                    if let Some(active_editor) = opened_item.downcast::<Editor>() {
+                                                    .update_in(&mut cx, |editor, window, cx| {
-                                        active_editor
+                                                        editor.go_to_singleton_buffer_point(
-                                            .downgrade()
+                                                            language::Point::new(
-                                            .update_in(&mut cx, |editor, window, cx| {
+                                                                row.saturating_sub(1),
-                                                editor.go_to_singleton_buffer_point(
+                                                                col.saturating_sub(1),
-                                                    language::Point::new(
+                                                            ),
-                                                        row.saturating_sub(1),
+                                                            window,
-                                                        col.saturating_sub(1),
+                                                            cx,
-                                                    ),
+                                                        )
-                                                    window,
+                                                    })
-                                                    cx,
+                                                    .log_err();
-                                                )
+                                            }
-                                            })
+                                        }
-                                            .log_err();
                                     }
+                                } else if open_target.is_dir() {
+                                    task_workspace.update(&mut cx, |workspace, cx| {
+                                        workspace.project().update(cx, |_, cx| {
+                                            cx.emit(project::Event::ActivateProjectPanel);
+                                        })
+                                    })?;
                                 }
                             }
                         }
 
-                        if has_dirs {
-                            task_workspace.update(&mut cx, |workspace, cx| {
-                                workspace.project().update(cx, |_, cx| {
-                                    cx.emit(project::Event::ActivateProjectPanel);
-                                })
-                            })?;
-                        }
-
                         anyhow::Ok(())
                     })
                     .detach_and_log_err(cx)
@@ -996,105 +975,158 @@ fn subscribe_for_terminal_events(
     vec![terminal_subscription, terminal_events_subscription]
 }
 
-fn possible_open_paths_metadata(
+#[derive(Debug, Clone)]
-    fs: Arc<dyn Fs>,
+enum OpenTarget {
-    row: Option<u32>,
+    Worktree(Entry),
-    column: Option<u32>,
+    File(Metadata),
-    potential_paths: HashSet<PathBuf>,
-    cx: &mut Context<TerminalView>,
-) -> Task<Vec<(PathWithPosition, Metadata)>> {
-    cx.background_spawn(async move {
-        let mut canonical_paths = HashSet::default();
-        for path in potential_paths {
-            if let Ok(canonical) = fs.canonicalize(&path).await {
-                let sanitized = SanitizedPath::from(canonical);
-                canonical_paths.insert(sanitized.as_path().to_path_buf());
-            } else {
-                canonical_paths.insert(path);
-            }
-        }
-
-        let mut paths_with_metadata = Vec::with_capacity(canonical_paths.len());
-
-        let mut fetch_metadata_tasks = canonical_paths
-            .into_iter()
-            .map(|potential_path| async {
-                let metadata = fs.metadata(&potential_path).await.ok().flatten();
-                (
-                    PathWithPosition {
-                        path: potential_path,
-                        row,
-                        column,
-                    },
-                    metadata,
-                )
-            })
-            .collect::<FuturesUnordered<_>>();
-
-        while let Some((path, metadata)) = fetch_metadata_tasks.next().await {
-            if let Some(metadata) = metadata {
-                paths_with_metadata.push((path, metadata));
-            }
-        }
-
-        paths_with_metadata
-    })
 }
 
-fn possible_open_targets(
+impl OpenTarget {
-    fs: Arc<dyn Fs>,
+    fn is_file(&self) -> bool {
+        match self {
+            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,
+        }
+    }
+}
+
+fn possible_open_target(
     workspace: &WeakEntity<Workspace>,
     cwd: &Option<PathBuf>,
-    maybe_path: &String,
+    maybe_path: &str,
-
     cx: &mut Context<TerminalView>,
-) -> Task<Vec<(PathWithPosition, Metadata)>> {
+) -> Task<Option<(PathWithPosition, OpenTarget)>> {
-    let path_position = PathWithPosition::parse_str(maybe_path.as_str());
+    let Some(workspace) = workspace.upgrade() else {
-    let row = path_position.row;
+        return Task::ready(None);
-    let column = path_position.column;
+    };
-    let maybe_path = path_position.path;
+    // We have to check for both paths, as on Unix, certain paths with positions are valid file paths too.
-
+    // We can be on FS remote part, without real FS, so cannot canonicalize or check for existence the path right away.
-    let potential_paths = if maybe_path.is_absolute() {
+    let mut potential_paths = Vec::new();
-        HashSet::from_iter([maybe_path])
+    let original_path = PathWithPosition::from_path(PathBuf::from(maybe_path));
-    } else if maybe_path.starts_with("~") {
+    let path_with_position = PathWithPosition::parse_str(maybe_path);
-        maybe_path
+    for prefix_str in GIT_DIFF_PATH_PREFIXES {
-            .strip_prefix("~")
+        if let Some(stripped) = original_path.path.strip_prefix(prefix_str).ok() {
-            .ok()
+            potential_paths.push(PathWithPosition {
-            .and_then(|maybe_path| Some(dirs::home_dir()?.join(maybe_path)))
+                path: stripped.to_owned(),
-            .map_or_else(HashSet::default, |p| HashSet::from_iter([p]))
+                row: original_path.row,
-    } else {
+                column: original_path.column,
-        let mut potential_cwd_and_workspace_paths = HashSet::default();
-        if let Some(cwd) = cwd {
-            let abs_path = Path::join(cwd, &maybe_path);
-            potential_cwd_and_workspace_paths.insert(abs_path);
-        }
-        if let Some(workspace) = workspace.upgrade() {
-            workspace.update(cx, |workspace, cx| {
-                for potential_worktree_path in workspace
-                    .worktrees(cx)
-                    .map(|worktree| worktree.read(cx).abs_path().join(&maybe_path))
-                {
-                    potential_cwd_and_workspace_paths.insert(potential_worktree_path);
-                }
-
-                for prefix in GIT_DIFF_PATH_PREFIXES {
-                    let prefix_str = &prefix.to_string();
-                    if maybe_path.starts_with(prefix_str) {
-                        let stripped = maybe_path.strip_prefix(prefix_str).unwrap_or(&maybe_path);
-                        for potential_worktree_path in workspace
-                            .worktrees(cx)
-                            .map(|worktree| worktree.read(cx).abs_path().join(&stripped))
-                        {
-                            potential_cwd_and_workspace_paths.insert(potential_worktree_path);
-                        }
-                    }
-                }
             });
         }
-        potential_cwd_and_workspace_paths
+        if let Some(stripped) = path_with_position.path.strip_prefix(prefix_str).ok() {
-    };
+            potential_paths.push(PathWithPosition {
+                path: stripped.to_owned(),
+                row: path_with_position.row,
+                column: path_with_position.column,
+            });
+        }
+    }
+    potential_paths.insert(0, original_path);
+    potential_paths.insert(1, path_with_position);
 
-    possible_open_paths_metadata(fs, row, column, potential_paths, cx)
+    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,
+        }
+    }) {
+        let worktree_root = worktree.read(cx).abs_path();
+        let paths_to_check = potential_paths
+            .iter()
+            .map(|path_with_position| PathWithPosition {
+                path: path_with_position
+                    .path
+                    .strip_prefix(&worktree_root)
+                    .unwrap_or(&path_with_position.path)
+                    .to_owned(),
+                row: path_with_position.row,
+                column: path_with_position.column,
+            })
+            .collect::<Vec<_>>();
+
+        let mut traversal = worktree
+            .read(cx)
+            .traverse_from_path(true, true, false, "".as_ref());
+        while let Some(entry) = traversal.next() {
+            if let Some(path_in_worktree) = paths_to_check
+                .iter()
+                .find(|path_to_check| entry.path.ends_with(&path_to_check.path))
+            {
+                return Task::ready(Some((
+                    PathWithPosition {
+                        path: worktree_root.join(&entry.path),
+                        row: path_in_worktree.row,
+                        column: path_in_worktree.column,
+                    },
+                    OpenTarget::Worktree(entry.clone()),
+                )));
+            }
+        }
+    }
+
+    if !workspace.read(cx).project().read(cx).is_local() {
+        return Task::ready(None);
+    }
+    let fs = workspace.read(cx).project().read(cx).fs().clone();
+
+    let paths_to_check = potential_paths
+        .into_iter()
+        .flat_map(|path_to_check| {
+            let mut paths_to_check = Vec::new();
+            let maybe_path = &path_to_check.path;
+            if maybe_path.starts_with("~") {
+                if let Some(home_path) =
+                    maybe_path
+                        .strip_prefix("~")
+                        .ok()
+                        .and_then(|stripped_maybe_path| {
+                            Some(dirs::home_dir()?.join(stripped_maybe_path))
+                        })
+                {
+                    paths_to_check.push(PathWithPosition {
+                        path: home_path,
+                        row: path_to_check.row,
+                        column: path_to_check.column,
+                    });
+                }
+            } else {
+                paths_to_check.push(PathWithPosition {
+                    path: maybe_path.clone(),
+                    row: path_to_check.row,
+                    column: path_to_check.column,
+                });
+                if maybe_path.is_relative() {
+                    if let Some(cwd) = &cwd {
+                        paths_to_check.push(PathWithPosition {
+                            path: cwd.join(maybe_path),
+                            row: path_to_check.row,
+                            column: path_to_check.column,
+                        });
+                    }
+                }
+            }
+            paths_to_check
+        })
+        .collect::<Vec<_>>();
+
+    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)));
+            }
+        }
+        None
+    })
 }
 
 fn regex_to_literal(regex: &str) -> String {