file_finder: Include worktree root name in multi-worktrees workspace (#40415)

Coenen Benjamin created

Closes #39865

Release Notes:

- Fixed file finder display when searching for files in history if you
had several worktrees opened in a workspace. It now displays the
worktree root name to avoid confusion if you have several files with
same name in different worktrees.

---------

Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com>

Change summary

crates/file_finder/src/file_finder.rs       |  45 ++++++
crates/file_finder/src/file_finder_tests.rs | 141 +++++++++++++++++++++++
crates/fuzzy/src/paths.rs                   |  31 ++++
3 files changed, 209 insertions(+), 8 deletions(-)

Detailed changes

crates/file_finder/src/file_finder.rs 🔗

@@ -21,7 +21,9 @@ use gpui::{
 };
 use open_path_prompt::OpenPathPrompt;
 use picker::{Picker, PickerDelegate};
-use project::{PathMatchCandidateSet, Project, ProjectPath, WorktreeId};
+use project::{
+    PathMatchCandidateSet, Project, ProjectPath, WorktreeId, worktree_store::WorktreeStore,
+};
 use search::ToggleIncludeIgnored;
 use settings::Settings;
 use std::{
@@ -538,11 +540,14 @@ impl Matches {
 
     fn push_new_matches<'a>(
         &'a mut self,
+        worktree_store: Entity<WorktreeStore>,
+        cx: &'a App,
         history_items: impl IntoIterator<Item = &'a FoundPath> + Clone,
         currently_opened: Option<&'a FoundPath>,
         query: Option<&FileSearchQuery>,
         new_search_matches: impl Iterator<Item = ProjectPanelOrdMatch>,
         extend_old_matches: bool,
+        path_style: PathStyle,
     ) {
         let Some(query) = query else {
             // assuming that if there's no query, then there's no search matches.
@@ -556,8 +561,25 @@ impl Matches {
                 .extend(history_items.into_iter().map(path_to_entry));
             return;
         };
-
-        let new_history_matches = matching_history_items(history_items, currently_opened, query);
+        // If several worktress are open we have to set the worktree root names in path prefix
+        let several_worktrees = worktree_store.read(cx).worktrees().count() > 1;
+        let worktree_name_by_id = several_worktrees.then(|| {
+            worktree_store
+                .read(cx)
+                .worktrees()
+                .map(|worktree| {
+                    let snapshot = worktree.read(cx).snapshot();
+                    (snapshot.id(), snapshot.root_name().into())
+                })
+                .collect()
+        });
+        let new_history_matches = matching_history_items(
+            history_items,
+            currently_opened,
+            worktree_name_by_id,
+            query,
+            path_style,
+        );
         let new_search_matches: Vec<Match> = new_search_matches
             .filter(|path_match| {
                 !new_history_matches.contains_key(&ProjectPath {
@@ -694,7 +716,9 @@ impl Matches {
 fn matching_history_items<'a>(
     history_items: impl IntoIterator<Item = &'a FoundPath>,
     currently_opened: Option<&'a FoundPath>,
+    worktree_name_by_id: Option<HashMap<WorktreeId, Arc<RelPath>>>,
     query: &FileSearchQuery,
+    path_style: PathStyle,
 ) -> HashMap<ProjectPath, Match> {
     let mut candidates_paths = HashMap::default();
 
@@ -734,13 +758,18 @@ fn matching_history_items<'a>(
     let mut matching_history_paths = HashMap::default();
     for (worktree, candidates) in history_items_by_worktrees {
         let max_results = candidates.len() + 1;
+        let worktree_root_name = worktree_name_by_id
+            .as_ref()
+            .and_then(|w| w.get(&worktree).cloned());
         matching_history_paths.extend(
             fuzzy::match_fixed_path_set(
                 candidates,
                 worktree.to_usize(),
+                worktree_root_name,
                 query.path_query(),
                 false,
                 max_results,
+                path_style,
             )
             .into_iter()
             .filter_map(|path_match| {
@@ -937,15 +966,18 @@ impl FileFinderDelegate {
                 self.matches.get(self.selected_index).cloned()
             };
 
+            let path_style = self.project.read(cx).path_style(cx);
             self.matches.push_new_matches(
+                self.project.read(cx).worktree_store(),
+                cx,
                 &self.history_items,
                 self.currently_opened_path.as_ref(),
                 Some(&query),
                 matches.into_iter(),
                 extend_old_matches,
+                path_style,
             );
 
-            let path_style = self.project.read(cx).path_style(cx);
             let query_path = query.raw_query.as_str();
             if let Ok(mut query_path) = RelPath::new(Path::new(query_path), path_style) {
                 let available_worktree = self
@@ -1365,7 +1397,11 @@ impl PickerDelegate for FileFinderDelegate {
                     separate_history: self.separate_history,
                     ..Matches::default()
                 };
+                let path_style = self.project.read(cx).path_style(cx);
+
                 self.matches.push_new_matches(
+                    project.worktree_store(),
+                    cx,
                     self.history_items.iter().filter(|history_item| {
                         project
                             .worktree_for_id(history_item.project.worktree_id, cx)
@@ -1377,6 +1413,7 @@ impl PickerDelegate for FileFinderDelegate {
                     None,
                     None.into_iter(),
                     false,
+                    path_style,
                 );
 
                 self.first_update = false;

crates/file_finder/src/file_finder_tests.rs 🔗

@@ -2503,6 +2503,147 @@ async fn test_search_results_refreshed_on_adding_and_removing_worktrees(
     });
 }
 
+#[gpui::test]
+async fn test_history_items_uniqueness_for_multiple_worktree_open_all_files(
+    cx: &mut TestAppContext,
+) {
+    let app_state = init_test(cx);
+    app_state
+        .fs
+        .as_fake()
+        .insert_tree(
+            path!("/repo1"),
+            json!({
+                "package.json": r#"{"name": "repo1"}"#,
+                "src": {
+                    "index.js": "// Repo 1 index",
+                }
+            }),
+        )
+        .await;
+
+    app_state
+        .fs
+        .as_fake()
+        .insert_tree(
+            path!("/repo2"),
+            json!({
+                "package.json": r#"{"name": "repo2"}"#,
+                "src": {
+                    "index.js": "// Repo 2 index",
+                }
+            }),
+        )
+        .await;
+
+    let project = Project::test(
+        app_state.fs.clone(),
+        [path!("/repo1").as_ref(), path!("/repo2").as_ref()],
+        cx,
+    )
+    .await;
+
+    let (workspace, cx) = cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx));
+    let (worktree_id1, worktree_id2) = cx.read(|cx| {
+        let worktrees = workspace.read(cx).worktrees(cx).collect::<Vec<_>>();
+        (worktrees[0].read(cx).id(), worktrees[1].read(cx).id())
+    });
+
+    workspace
+        .update_in(cx, |workspace, window, cx| {
+            workspace.open_path(
+                ProjectPath {
+                    worktree_id: worktree_id1,
+                    path: rel_path("package.json").into(),
+                },
+                None,
+                true,
+                window,
+                cx,
+            )
+        })
+        .await
+        .unwrap();
+
+    cx.dispatch_action(workspace::CloseActiveItem {
+        save_intent: None,
+        close_pinned: false,
+    });
+    workspace
+        .update_in(cx, |workspace, window, cx| {
+            workspace.open_path(
+                ProjectPath {
+                    worktree_id: worktree_id2,
+                    path: rel_path("package.json").into(),
+                },
+                None,
+                true,
+                window,
+                cx,
+            )
+        })
+        .await
+        .unwrap();
+
+    cx.dispatch_action(workspace::CloseActiveItem {
+        save_intent: None,
+        close_pinned: false,
+    });
+
+    let picker = open_file_picker(&workspace, cx);
+    cx.simulate_input("package.json");
+
+    picker.update(cx, |finder, _| {
+        let matches = &finder.delegate.matches.matches;
+
+        assert_eq!(
+            matches.len(),
+            2,
+            "Expected 1 history match + 1 search matches, but got {} matches: {:?}",
+            matches.len(),
+            matches
+        );
+
+        assert_matches!(matches[0], Match::History { .. });
+
+        let search_matches = collect_search_matches(finder);
+        assert_eq!(
+            search_matches.history.len(),
+            2,
+            "Should have exactly 2 history match"
+        );
+        assert_eq!(
+            search_matches.search.len(),
+            0,
+            "Should have exactly 0 search match (because we already opened the 2 package.json)"
+        );
+
+        if let Match::History { path, panel_match } = &matches[0] {
+            assert_eq!(path.project.worktree_id, worktree_id2);
+            assert_eq!(path.project.path.as_ref(), rel_path("package.json"));
+            let panel_match = panel_match.as_ref().unwrap();
+            assert_eq!(panel_match.0.path_prefix, rel_path("repo2").into());
+            assert_eq!(panel_match.0.path, rel_path("package.json").into());
+            assert_eq!(
+                panel_match.0.positions,
+                vec![6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17]
+            );
+        }
+
+        if let Match::History { path, panel_match } = &matches[1] {
+            assert_eq!(path.project.worktree_id, worktree_id1);
+            assert_eq!(path.project.path.as_ref(), rel_path("package.json"));
+            let panel_match = panel_match.as_ref().unwrap();
+            assert_eq!(panel_match.0.path_prefix, rel_path("repo1").into());
+            assert_eq!(panel_match.0.path, rel_path("package.json").into());
+            assert_eq!(
+                panel_match.0.positions,
+                vec![6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17]
+            );
+        }
+    });
+}
+
 #[gpui::test]
 async fn test_selected_match_stays_selected_after_matches_refreshed(cx: &mut gpui::TestAppContext) {
     let app_state = init_test(cx);

crates/fuzzy/src/paths.rs 🔗

@@ -88,9 +88,11 @@ impl Ord for PathMatch {
 pub fn match_fixed_path_set(
     candidates: Vec<PathMatchCandidate>,
     worktree_id: usize,
+    worktree_root_name: Option<Arc<RelPath>>,
     query: &str,
     smart_case: bool,
     max_results: usize,
+    path_style: PathStyle,
 ) -> Vec<PathMatch> {
     let lowercase_query = query.to_lowercase().chars().collect::<Vec<_>>();
     let query = query.chars().collect::<Vec<_>>();
@@ -98,10 +100,31 @@ pub fn match_fixed_path_set(
 
     let mut matcher = Matcher::new(&query, &lowercase_query, query_char_bag, smart_case, true);
 
-    let mut results = Vec::new();
+    let mut results = Vec::with_capacity(candidates.len());
+    let (path_prefix, path_prefix_chars, lowercase_prefix) = match worktree_root_name {
+        Some(worktree_root_name) => {
+            let mut path_prefix_chars = worktree_root_name
+                .display(path_style)
+                .chars()
+                .collect::<Vec<_>>();
+            path_prefix_chars.extend(path_style.separator().chars());
+            let lowercase_pfx = path_prefix_chars
+                .iter()
+                .map(|c| c.to_ascii_lowercase())
+                .collect::<Vec<_>>();
+
+            (worktree_root_name, path_prefix_chars, lowercase_pfx)
+        }
+        None => (
+            RelPath::empty().into(),
+            Default::default(),
+            Default::default(),
+        ),
+    };
+
     matcher.match_candidates(
-        &[],
-        &[],
+        &path_prefix_chars,
+        &lowercase_prefix,
         candidates.into_iter(),
         &mut results,
         &AtomicBool::new(false),
@@ -111,7 +134,7 @@ pub fn match_fixed_path_set(
             positions: positions.clone(),
             is_dir: candidate.is_dir,
             path: candidate.path.into(),
-            path_prefix: RelPath::empty().into(),
+            path_prefix: path_prefix.clone(),
             distance_to_relative_ancestor: usize::MAX,
         },
     );