Fix ghost files appearing in the project panel when clicking relative paths in the terminal (#22688)

tims created 1 year ago

Closes #15705

When opening a file from the terminal, if the file path is relative, we
attempt to guess all possible paths where the file could be. This
involves generating paths for each worktree, the current terminal
directory, etc. For example, if we have two worktrees, `dotfiles` and
`example`, and `foo.txt` in `example/a`, the generated paths might look
like this:

- `/home/tims/dotfiles/../example/a/foo.txt` from the `dotfiles`
worktree
- `/home/tims/example/../example/a/foo.txt` from the `example` worktree
- `/home/tims/example/a/foo.txt` from the current terminal directory
(This is already canonicalized)

Note that there should only be a single path, but multiple paths are
created due to missing canonicalization.

Later, when opening these paths, the worktree prefix is stripped, and
the remaining path is used to open the file in its respective worktree.

As a result, the above three paths would resolve like this:

- `../example/a/foo.txt` as the filename in the `dotfiles` worktree
(Ghost file)
- `../example/a/foo.txt` as the filename in the `example` worktree
(Ghost file)
- `foo.txt` as the filename in the `a` directory of the `example`
worktree (This opens the file)

This PR fixes the issue by canonicalizing these paths before adding them
to the HashSet.

Before:

![before](https://github.com/user-attachments/assets/7cb98b86-1adf-462f-bcc6-9bff6a8425cd)

After:

![after](https://github.com/user-attachments/assets/44568167-2a5a-4022-ba98-b359d2c6e56b)


Release Notes:

- Fixed ghost files appearing in the project panel when clicking
relative paths in the terminal.

Change summary

crates/terminal_view/src/terminal_view.rs | 46 +++++++++++-------------
1 file changed, 22 insertions(+), 24 deletions(-)

Detailed changes

crates/terminal_view/src/terminal_view.rs

@@ -27,7 +27,10 @@ use terminal::{
 use terminal_element::{is_blank, TerminalElement};
 use terminal_panel::TerminalPanel;
 use ui::{h_flex, prelude::*, ContextMenu, Icon, IconName, Label, Tooltip};
-use util::{paths::PathWithPosition, ResultExt};
+use util::{
+    paths::{PathWithPosition, SanitizedPath},
+    ResultExt,
+};
 use workspace::{
     item::{BreadcrumbText, Item, ItemEvent, SerializableItem, TabContentParams},
     register_serializable_item,
@@ -780,9 +783,19 @@ fn possible_open_paths_metadata(
     cx: &mut ViewContext<TerminalView>,
 ) -> Task<Vec<(PathWithPosition, Metadata)>> {
     cx.background_executor().spawn(async move {
-        let mut paths_with_metadata = Vec::with_capacity(potential_paths.len());
+        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 = potential_paths
+        let mut fetch_metadata_tasks = canonical_paths
             .into_iter()
             .map(|potential_path| async {
                 let metadata = fs.metadata(&potential_path).await.ok().flatten();
@@ -819,19 +832,19 @@ fn possible_open_targets(
     let column = path_position.column;
     let maybe_path = path_position.path;
 
-    let abs_path = if maybe_path.is_absolute() {
-        Some(maybe_path)
+    let potential_paths = if maybe_path.is_absolute() {
+        HashSet::from_iter([maybe_path])
     } else if maybe_path.starts_with("~") {
         maybe_path
             .strip_prefix("~")
             .ok()
             .and_then(|maybe_path| Some(dirs::home_dir()?.join(maybe_path)))
+            .map_or_else(HashSet::default, |p| HashSet::from_iter([p]))
     } else {
         let mut potential_cwd_and_workspace_paths = HashSet::default();
         if let Some(cwd) = cwd {
             let abs_path = Path::join(cwd, &maybe_path);
-            let canonicalized_path = abs_path.canonicalize().unwrap_or(abs_path);
-            potential_cwd_and_workspace_paths.insert(canonicalized_path);
+            potential_cwd_and_workspace_paths.insert(abs_path);
         }
         if let Some(workspace) = workspace.upgrade() {
             workspace.update(cx, |workspace, cx| {
@@ -856,25 +869,10 @@ fn possible_open_targets(
                 }
             });
         }
-
-        return possible_open_paths_metadata(
-            fs,
-            row,
-            column,
-            potential_cwd_and_workspace_paths,
-            cx,
-        );
-    };
-
-    let canonicalized_paths = match abs_path {
-        Some(abs_path) => match abs_path.canonicalize() {
-            Ok(path) => HashSet::from_iter([path]),
-            Err(_) => HashSet::default(),
-        },
-        None => HashSet::default(),
+        potential_cwd_and_workspace_paths
     };
 
-    possible_open_paths_metadata(fs, row, column, canonicalized_paths, cx)
+    possible_open_paths_metadata(fs, row, column, potential_paths, cx)
 }
 
 fn regex_to_literal(regex: &str) -> String {