file_finder: Fix history items not using worktree path (#39304)

Marco Mihai Condrache created 2 months ago

Closes #39283

Release Notes:

- Fixed: In multi-repo workspaces, files with the same name are no
longer hidden in the file picker after one is opened

---------

Signed-off-by: Marco Mihai Condrache <[e-mail]>

Change summary

crates/file_finder/src/file_finder.rs       |  13 +
crates/file_finder/src/file_finder_tests.rs | 108 ++++++++++++++++++++++
2 files changed, 117 insertions(+), 4 deletions(-)

Detailed changes

crates/file_finder/src/file_finder.rs

@@ -559,7 +559,12 @@ impl Matches {
 
         let new_history_matches = matching_history_items(history_items, currently_opened, query);
         let new_search_matches: Vec<Match> = new_search_matches
-            .filter(|path_match| !new_history_matches.contains_key(&path_match.0.path))
+            .filter(|path_match| {
+                !new_history_matches.contains_key(&ProjectPath {
+                    path: path_match.0.path.clone(),
+                    worktree_id: WorktreeId::from_usize(path_match.0.worktree_id),
+                })
+            })
             .map(Match::Search)
             .collect();
 
@@ -690,7 +695,7 @@ fn matching_history_items<'a>(
     history_items: impl IntoIterator<Item = &'a FoundPath>,
     currently_opened: Option<&'a FoundPath>,
     query: &FileSearchQuery,
-) -> HashMap<Arc<RelPath>, Match> {
+) -> HashMap<ProjectPath, Match> {
     let mut candidates_paths = HashMap::default();
 
     let history_items_by_worktrees = history_items
@@ -744,9 +749,9 @@ fn matching_history_items<'a>(
                         worktree_id: WorktreeId::from_usize(path_match.worktree_id),
                         path: Arc::clone(&path_match.path),
                     })
-                    .map(|(_, found_path)| {
+                    .map(|(project_path, found_path)| {
                         (
-                            Arc::clone(&path_match.path),
+                            project_path.clone(),
                             Match::History {
                                 path: found_path.clone(),
                                 panel_match: Some(ProjectPanelOrdMatch(path_match)),

crates/file_finder/src/file_finder_tests.rs

@@ -929,6 +929,114 @@ async fn test_single_file_worktrees(cx: &mut TestAppContext) {
     picker.update(cx, |f, _| assert_eq!(f.delegate.matches.len(), 0));
 }
 
+#[gpui::test]
+async fn test_history_items_uniqueness_for_multiple_worktree(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,
+    });
+
+    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(),
+            1,
+            "Should have exactly 1 history match"
+        );
+        assert_eq!(
+            search_matches.search.len(),
+            1,
+            "Should have exactly 1 search match (the other package.json)"
+        );
+
+        if let Match::History { path, .. } = &matches[0] {
+            assert_eq!(path.project.worktree_id, worktree_id1);
+            assert_eq!(path.project.path.as_ref(), rel_path("package.json"));
+        }
+
+        if let Match::Search(path_match) = &matches[1] {
+            assert_eq!(
+                WorktreeId::from_usize(path_match.0.worktree_id),
+                worktree_id2
+            );
+            assert_eq!(path_match.0.path.as_ref(), rel_path("package.json"));
+        }
+    });
+}
+
 #[gpui::test]
 async fn test_create_file_for_multiple_worktrees(cx: &mut TestAppContext) {
     let app_state = init_test(cx);