file_finder: Fix path matching on starting slash (#39480)

Dino , Piotr Osiewicz , and Piotr Osiewicz created 2 months ago

These changes update the way the file finder decides wether to only look
for an absolute path or for a relative path too.

When the provided query started with a slash (`/`) the file finder would
assume this to be an absolute path so would always try to find an
absolute path and return no matches if none was found. This is meant to
support situtations where, for example, a CLI tool might output the
absolute path of a file and the user can copy and paste that in the file
finder.

However, it's should be possible to use slash (`/`) at the start of the
query to specify that only relative files inside a folder should be
matched, which would not work in this scenario.

With these changes, the file finder will first check if the path is
absolute and, if it is and no absolute matches were found, it'll still
try to find relative matches, otherwise it'll simply look for relative
matches.

Closes #39350

Release Notes:

- Fixed project files matches when using slash (`/`) at the start in
order to consider relative paths

---------

Co-authored-by: Piotr Osiewicz <[e-mail]>
Co-authored-by: Piotr Osiewicz <[e-mail]>

Change summary

crates/file_finder/src/file_finder.rs       | 49 +++++++++++++++++-----
crates/file_finder/src/file_finder_tests.rs | 46 +++++++++++++++++++++
2 files changed, 84 insertions(+), 11 deletions(-)

Detailed changes

crates/file_finder/src/file_finder.rs

@@ -1172,18 +1172,25 @@ impl FileFinderDelegate {
         )
     }
 
+    /// Attempts to resolve an absolute file path and update the search matches if found.
+    ///
+    /// If the query path resolves to an absolute file that exists in the project,
+    /// this method will find the corresponding worktree and relative path, create a
+    /// match for it, and update the picker's search results.
+    ///
+    /// Returns `true` if the absolute path exists, otherwise returns `false`.
     fn lookup_absolute_path(
         &self,
         query: FileSearchQuery,
         window: &mut Window,
         cx: &mut Context<Picker<Self>>,
-    ) -> Task<()> {
+    ) -> Task<bool> {
         cx.spawn_in(window, async move |picker, cx| {
             let Some(project) = picker
                 .read_with(cx, |picker, _| picker.delegate.project.clone())
                 .log_err()
             else {
-                return;
+                return false;
             };
 
             let query_path = Path::new(query.path_query());
@@ -1216,7 +1223,7 @@ impl FileFinderDelegate {
                     })
                     .log_err();
                 if update_result.is_none() {
-                    return;
+                    return abs_file_exists;
                 }
             }
 
@@ -1229,6 +1236,7 @@ impl FileFinderDelegate {
                     anyhow::Ok(())
                 })
                 .log_err();
+            abs_file_exists
         })
     }
 
@@ -1377,13 +1385,14 @@ impl PickerDelegate for FileFinderDelegate {
         } else {
             let path_position = PathWithPosition::parse_str(raw_query);
             let raw_query = raw_query.trim().trim_end_matches(':').to_owned();
-            let path = path_position.path.to_str();
-            let path_trimmed = path.unwrap_or(&raw_query).trim_end_matches(':');
+            let path = path_position.path.clone();
+            let path_str = path_position.path.to_str();
+            let path_trimmed = path_str.unwrap_or(&raw_query).trim_end_matches(':');
             let file_query_end = if path_trimmed == raw_query {
                 None
             } else {
                 // Safe to unwrap as we won't get here when the unwrap in if fails
-                Some(path.unwrap().len())
+                Some(path_str.unwrap().len())
             };
 
             let query = FileSearchQuery {
@@ -1392,11 +1401,29 @@ impl PickerDelegate for FileFinderDelegate {
                 path_position,
             };
 
-            if Path::new(query.path_query()).is_absolute() {
-                self.lookup_absolute_path(query, window, cx)
-            } else {
-                self.spawn_search(query, window, cx)
-            }
+            cx.spawn_in(window, async move |this, cx| {
+                let _ = maybe!(async move {
+                    let is_absolute_path = path.is_absolute();
+                    let did_resolve_abs_path = is_absolute_path
+                        && this
+                            .update_in(cx, |this, window, cx| {
+                                this.delegate
+                                    .lookup_absolute_path(query.clone(), window, cx)
+                            })?
+                            .await;
+
+                    // Only check for relative paths if no absolute paths were
+                    // found.
+                    if !did_resolve_abs_path {
+                        this.update_in(cx, |this, window, cx| {
+                            this.delegate.spawn_search(query, window, cx)
+                        })?
+                        .await;
+                    }
+                    anyhow::Ok(())
+                })
+                .await;
+            })
         }
     }
 

crates/file_finder/src/file_finder_tests.rs

@@ -3069,3 +3069,49 @@ async fn test_filename_precedence(cx: &mut TestAppContext) {
         );
     });
 }
+
+#[gpui::test]
+async fn test_paths_with_starting_slash(cx: &mut TestAppContext) {
+    let app_state = init_test(cx);
+    app_state
+        .fs
+        .as_fake()
+        .insert_tree(
+            path!("/root"),
+            json!({
+                "a": {
+                    "file1.txt": "",
+                    "b": {
+                        "file2.txt": "",
+                    },
+                }
+            }),
+        )
+        .await;
+
+    let project = Project::test(app_state.fs.clone(), [path!("/root").as_ref()], cx).await;
+
+    let (picker, workspace, cx) = build_find_picker(project, cx);
+
+    let matching_abs_path = "/file1.txt".to_string();
+    picker
+        .update_in(cx, |picker, window, cx| {
+            picker
+                .delegate
+                .update_matches(matching_abs_path, window, cx)
+        })
+        .await;
+    picker.update(cx, |picker, _| {
+        assert_eq!(
+            collect_search_matches(picker).search_paths_only(),
+            vec![rel_path("a/file1.txt").into()],
+            "Relative path starting with slash should match"
+        )
+    });
+    cx.dispatch_action(SelectNext);
+    cx.dispatch_action(Confirm);
+    cx.read(|cx| {
+        let active_editor = workspace.read(cx).active_item_as::<Editor>(cx).unwrap();
+        assert_eq!(active_editor.read(cx).title(cx), "file1.txt");
+    });
+}