From 0b48e238f2e9ccc27a3e34fd9fbdff344c8b4c88 Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Fri, 24 Feb 2023 18:13:26 -0800 Subject: [PATCH 1/3] Sort file finder matches by distance to the active item after match score --- crates/file_finder/src/file_finder.rs | 26 ++++++++++++++++------ crates/fuzzy/src/matcher.rs | 1 + crates/fuzzy/src/paths.rs | 32 +++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 7561a68222c4d30133659cf4ae0901a1f0ea1ffa..08718870f1f2903d5de43d195c323e2415408152 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/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>, matches: Vec, selected: Option<(usize, Arc)>, cancel_flag: Arc, @@ -90,7 +91,11 @@ impl FileFinder { fn toggle(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContext) { 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, cx: &mut ViewContext) -> Self { + pub fn new( + project: ModelHandle, + relative_to: Option>, + cx: &mut ViewContext, + ) -> 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) -> 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. @@ -533,7 +545,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)); // Run a search that matches two files with the same relative path. finder @@ -573,7 +585,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; diff --git a/crates/fuzzy/src/matcher.rs b/crates/fuzzy/src/matcher.rs index 51ae75bac23a981804dd4626fc9daf0cc4f50af6..dafafe40a0630ebe364f5d29963ef2475aa3c2b8 100644 --- a/crates/fuzzy/src/matcher.rs +++ b/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, }, ); diff --git a/crates/fuzzy/src/paths.rs b/crates/fuzzy/src/paths.rs index 8d9ec97d9b26eb6c2d6b4ce7db8268eb05aac0d7..da9789a23f297113adf15c49323901cebe0ddbc2 100644 --- a/crates/fuzzy/src/paths.rs +++ b/crates/fuzzy/src/paths.rs @@ -25,6 +25,9 @@ pub struct PathMatch { pub worktree_id: usize, pub path: Arc, pub path_prefix: Arc, + /// 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>, 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,10 @@ 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: distance_to_relative_ancestor( + candidate.path.as_ref(), + &relative_to, + ), }, ); } @@ -172,3 +186,21 @@ 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_to_relative_ancestor(path: &Path, relative_to: &Option>) -> usize { + let Some(relative_to) = relative_to else { + return usize::MAX; + }; + + for (path_ancestor_count, path_ancestor) in path.ancestors().enumerate() { + for (relative_ancestor_count, relative_ancestor) in relative_to.ancestors().enumerate() { + if path_ancestor == relative_ancestor { + return path_ancestor_count + relative_ancestor_count; + } + } + } + + usize::MAX +} From 36f3d3d738a5a780130cf41bad36091b709dc672 Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Sat, 25 Feb 2023 14:06:54 -0800 Subject: [PATCH 2/3] Add test for new sorting behavior --- .../src/test/editor_lsp_test_context.rs | 6 +-- crates/file_finder/src/file_finder.rs | 43 +++++++++++++++++++ crates/fuzzy/src/paths.rs | 12 ++---- 3 files changed, 50 insertions(+), 11 deletions(-) diff --git a/crates/editor/src/test/editor_lsp_test_context.rs b/crates/editor/src/test/editor_lsp_test_context.rs index 345709abf33f89862579cedd8482318063d0da6a..1b6d846e710f4ebd6f84c8053ab88d4c36294ef5 100644 --- a/crates/editor/src/test/editor_lsp_test_context.rs +++ b/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(): "" }})) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 08718870f1f2903d5de43d195c323e2415408152..273440dce253f54e8ccf5cb520bff321d403b1be 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -544,6 +544,7 @@ 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(), None, cx)); @@ -563,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); diff --git a/crates/fuzzy/src/paths.rs b/crates/fuzzy/src/paths.rs index da9789a23f297113adf15c49323901cebe0ddbc2..1293e9fa7be45bdee41c83bc0422a8efa340cbe9 100644 --- a/crates/fuzzy/src/paths.rs +++ b/crates/fuzzy/src/paths.rs @@ -194,13 +194,9 @@ fn distance_to_relative_ancestor(path: &Path, relative_to: &Option>) - return usize::MAX; }; - for (path_ancestor_count, path_ancestor) in path.ancestors().enumerate() { - for (relative_ancestor_count, relative_ancestor) in relative_to.ancestors().enumerate() { - if path_ancestor == relative_ancestor { - return path_ancestor_count + relative_ancestor_count; - } - } - } + let mut path_components = path.components(); + let mut relative_components = relative_to.components(); - usize::MAX + while path_components.next() == relative_components.next() {} + path_components.count() + relative_components.count() + 1 } From cdc6566d871dc5dbe30a7c24faba985e6b80321b Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Sat, 25 Feb 2023 14:12:25 -0800 Subject: [PATCH 3/3] fixup poor utility naming --- crates/fuzzy/src/paths.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/fuzzy/src/paths.rs b/crates/fuzzy/src/paths.rs index 1293e9fa7be45bdee41c83bc0422a8efa340cbe9..2c5ce81b1ccd09def80c7938aa1fe8591323500b 100644 --- a/crates/fuzzy/src/paths.rs +++ b/crates/fuzzy/src/paths.rs @@ -159,9 +159,14 @@ 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: distance_to_relative_ancestor( - candidate.path.as_ref(), - &relative_to, + 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(), + ) + }, ), }, ); @@ -189,11 +194,7 @@ pub async fn match_path_sets<'a, Set: PathMatchCandidateSet<'a>>( /// Compute the distance from a given path to some other path /// If there is no shared path, returns usize::MAX -fn distance_to_relative_ancestor(path: &Path, relative_to: &Option>) -> usize { - let Some(relative_to) = relative_to else { - return 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();