Merge pull request #2218 from zed-industries/file-finder-distance-sorting

Sort matches in file finder by distance to active item after score

crates/editor/src/test/editor_lsp_test_context.rs

@@ -39,7 +39,7 @@ impl<'a> EditorLspTestContext<'a> {
             pane::init(cx);
         });
 
-        let params = cx.update(AppState::test);
+        let app_state = cx.update(AppState::test);
 
         let file_name = format!(
             "file.{}",
@@ -56,10 +56,10 @@ impl<'a> EditorLspTestContext<'a> {
             }))
             .await;
 
-        let project = Project::test(params.fs.clone(), [], cx).await;
+        let project = Project::test(app_state.fs.clone(), [], cx).await;
         project.update(cx, |project, _| project.languages().add(Arc::new(language)));
 
-        params
+        app_state
             .fs
             .as_fake()
             .insert_tree("/root", json!({ "dir": { file_name.clone(): "" }}))

crates/file_finder/src/file_finder.rs

@@ -23,6 +23,7 @@ pub struct FileFinder {
     latest_search_id: usize,
     latest_search_did_cancel: bool,
     latest_search_query: String,
+    relative_to: Option<Arc<Path>>,
     matches: Vec<PathMatch>,
     selected: Option<(usize, Arc<Path>)>,
     cancel_flag: Arc<AtomicBool>,
@@ -90,7 +91,11 @@ impl FileFinder {
     fn toggle(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContext<Workspace>) {
         workspace.toggle_modal(cx, |workspace, cx| {
             let project = workspace.project().clone();
-            let finder = cx.add_view(|cx| Self::new(project, cx));
+            let relative_to = workspace
+                .active_item(cx)
+                .and_then(|item| item.project_path(cx))
+                .map(|project_path| project_path.path.clone());
+            let finder = cx.add_view(|cx| Self::new(project, relative_to, cx));
             cx.subscribe(&finder, Self::on_event).detach();
             finder
         });
@@ -115,7 +120,11 @@ impl FileFinder {
         }
     }
 
-    pub fn new(project: ModelHandle<Project>, cx: &mut ViewContext<Self>) -> Self {
+    pub fn new(
+        project: ModelHandle<Project>,
+        relative_to: Option<Arc<Path>>,
+        cx: &mut ViewContext<Self>,
+    ) -> Self {
         let handle = cx.weak_handle();
         cx.observe(&project, Self::project_updated).detach();
         Self {
@@ -125,6 +134,7 @@ impl FileFinder {
             latest_search_id: 0,
             latest_search_did_cancel: false,
             latest_search_query: String::new(),
+            relative_to,
             matches: Vec::new(),
             selected: None,
             cancel_flag: Arc::new(AtomicBool::new(false)),
@@ -137,6 +147,7 @@ impl FileFinder {
     }
 
     fn spawn_search(&mut self, query: String, cx: &mut ViewContext<Self>) -> Task<()> {
+        let relative_to = self.relative_to.clone();
         let worktrees = self
             .project
             .read(cx)
@@ -165,6 +176,7 @@ impl FileFinder {
             let matches = fuzzy::match_path_sets(
                 candidate_sets.as_slice(),
                 &query,
+                relative_to,
                 false,
                 100,
                 &cancel_flag,
@@ -377,7 +389,7 @@ mod tests {
             Workspace::new(Default::default(), 0, project, |_, _| unimplemented!(), cx)
         });
         let (_, finder) =
-            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), cx));
+            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), None, cx));
 
         let query = "hi".to_string();
         finder
@@ -453,7 +465,7 @@ mod tests {
             Workspace::new(Default::default(), 0, project, |_, _| unimplemented!(), cx)
         });
         let (_, finder) =
-            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), cx));
+            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), None, cx));
         finder
             .update(cx, |f, cx| f.spawn_search("hi".into(), cx))
             .await;
@@ -479,7 +491,7 @@ mod tests {
             Workspace::new(Default::default(), 0, project, |_, _| unimplemented!(), cx)
         });
         let (_, finder) =
-            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), cx));
+            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), None, cx));
 
         // Even though there is only one worktree, that worktree's filename
         // is included in the matching, because the worktree is a single file.
@@ -532,8 +544,9 @@ mod tests {
         let (_, workspace) = cx.add_window(|cx| {
             Workspace::new(Default::default(), 0, project, |_, _| unimplemented!(), cx)
         });
+
         let (_, finder) =
-            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), cx));
+            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), None, cx));
 
         // Run a search that matches two files with the same relative path.
         finder
@@ -551,6 +564,48 @@ mod tests {
         });
     }
 
+    #[gpui::test]
+    async fn test_path_distance_ordering(cx: &mut gpui::TestAppContext) {
+        cx.foreground().forbid_parking();
+
+        let app_state = cx.update(AppState::test);
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/root",
+                json!({
+                    "dir1": { "a.txt": "" },
+                    "dir2": {
+                        "a.txt": "",
+                        "b.txt": ""
+                    }
+                }),
+            )
+            .await;
+
+        let project = Project::test(app_state.fs.clone(), ["/root".as_ref()], cx).await;
+        let (_, workspace) = cx.add_window(|cx| {
+            Workspace::new(Default::default(), 0, project, |_, _| unimplemented!(), cx)
+        });
+
+        // When workspace has an active item, sort items which are closer to that item
+        // first when they have the same name. In this case, b.txt is closer to dir2's a.txt
+        // so that one should be sorted earlier
+        let b_path = Some(Arc::from(Path::new("/root/dir2/b.txt")));
+        let (_, finder) =
+            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), b_path, cx));
+
+        finder
+            .update(cx, |f, cx| f.spawn_search("a.txt".into(), cx))
+            .await;
+
+        finder.read_with(cx, |f, _| {
+            assert_eq!(f.matches[0].path.as_ref(), Path::new("dir2/a.txt"));
+            assert_eq!(f.matches[1].path.as_ref(), Path::new("dir1/a.txt"));
+        });
+    }
+
     #[gpui::test]
     async fn test_search_worktree_without_files(cx: &mut gpui::TestAppContext) {
         let app_state = cx.update(AppState::test);
@@ -573,7 +628,7 @@ mod tests {
             Workspace::new(Default::default(), 0, project, |_, _| unimplemented!(), cx)
         });
         let (_, finder) =
-            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), cx));
+            cx.add_window(|cx| FileFinder::new(workspace.read(cx).project().clone(), None, cx));
         finder
             .update(cx, |f, cx| f.spawn_search("dir".into(), cx))
             .await;

crates/fuzzy/src/matcher.rs

@@ -443,6 +443,7 @@ mod tests {
                 positions: Vec::new(),
                 path: candidate.path.clone(),
                 path_prefix: "".into(),
+                distance_to_relative_ancestor: usize::MAX,
             },
         );
 

crates/fuzzy/src/paths.rs

@@ -25,6 +25,9 @@ pub struct PathMatch {
     pub worktree_id: usize,
     pub path: Arc<Path>,
     pub path_prefix: Arc<str>,
+    /// Number of steps removed from a shared parent with the relative path
+    /// Used to order closer paths first in the search list
+    pub distance_to_relative_ancestor: usize,
 }
 
 pub trait PathMatchCandidateSet<'a>: Send + Sync {
@@ -78,6 +81,11 @@ impl Ord for PathMatch {
             .partial_cmp(&other.score)
             .unwrap_or(Ordering::Equal)
             .then_with(|| self.worktree_id.cmp(&other.worktree_id))
+            .then_with(|| {
+                other
+                    .distance_to_relative_ancestor
+                    .cmp(&self.distance_to_relative_ancestor)
+            })
             .then_with(|| self.path.cmp(&other.path))
     }
 }
@@ -85,6 +93,7 @@ impl Ord for PathMatch {
 pub async fn match_path_sets<'a, Set: PathMatchCandidateSet<'a>>(
     candidate_sets: &'a [Set],
     query: &str,
+    relative_to: Option<Arc<Path>>,
     smart_case: bool,
     max_results: usize,
     cancel_flag: &AtomicBool,
@@ -111,6 +120,7 @@ pub async fn match_path_sets<'a, Set: PathMatchCandidateSet<'a>>(
     background
         .scoped(|scope| {
             for (segment_idx, results) in segment_results.iter_mut().enumerate() {
+                let relative_to = relative_to.clone();
                 scope.spawn(async move {
                     let segment_start = segment_idx * segment_size;
                     let segment_end = segment_start + segment_size;
@@ -149,6 +159,15 @@ pub async fn match_path_sets<'a, Set: PathMatchCandidateSet<'a>>(
                                     positions: Vec::new(),
                                     path: candidate.path.clone(),
                                     path_prefix: candidate_set.prefix(),
+                                    distance_to_relative_ancestor: relative_to.as_ref().map_or(
+                                        usize::MAX,
+                                        |relative_to| {
+                                            distance_between_paths(
+                                                candidate.path.as_ref(),
+                                                relative_to.as_ref(),
+                                            )
+                                        },
+                                    ),
                                 },
                             );
                         }
@@ -172,3 +191,13 @@ pub async fn match_path_sets<'a, Set: PathMatchCandidateSet<'a>>(
     }
     results
 }
+
+/// Compute the distance from a given path to some other path
+/// If there is no shared path, returns usize::MAX
+fn distance_between_paths(path: &Path, relative_to: &Path) -> usize {
+    let mut path_components = path.components();
+    let mut relative_components = relative_to.components();
+
+    while path_components.next() == relative_components.next() {}
+    path_components.count() + relative_components.count() + 1
+}