For file finder queries, match in all gitignored worktree entries (#3748)

Kirill Bulatov created 2 years ago

Deals with https://github.com/zed-industries/community/issues/2347
Part of https://github.com/zed-industries/community/issues/1538

Now file finder will match all gitignored worktree entries.
Zed does not traverse gitignored dirs by default, which means that not
all gitignored files will be matches, but all that were toggled in
project panel and all root non-directory gitignored entries will be now
used, hopefully causing less questions.

Release Notes:

- Improved file finder to match all gitignored files that were added
into worktrees (e.g. due to opening gitignored directories in project
panel)

Change summary

crates/file_finder/src/file_finder.rs  | 131 +++++++++++++++++++++++++--
crates/file_finder2/src/file_finder.rs | 120 +++++++++++++++++++++++--
2 files changed, 229 insertions(+), 22 deletions(-)

Detailed changes

crates/file_finder/src/file_finder.rs

@@ -324,15 +324,10 @@ impl FileFinderDelegate {
         let include_root_name = worktrees.len() > 1;
         let candidate_sets = worktrees
             .into_iter()
-            .map(|worktree| {
-                let worktree = worktree.read(cx);
-                PathMatchCandidateSet {
-                    snapshot: worktree.snapshot(),
-                    include_ignored: worktree
-                        .root_entry()
-                        .map_or(false, |entry| entry.is_ignored),
-                    include_root_name,
-                }
+            .map(|worktree| PathMatchCandidateSet {
+                snapshot: worktree.read(cx).snapshot(),
+                include_ignored: true,
+                include_root_name,
             })
             .collect::<Vec<_>>();
 
@@ -1058,7 +1053,7 @@ mod tests {
     }
 
     #[gpui::test]
-    async fn test_ignored_files(cx: &mut TestAppContext) {
+    async fn test_ignored_root(cx: &mut TestAppContext) {
         let app_state = init_test(cx);
         app_state
             .fs
@@ -1115,7 +1110,105 @@ mod tests {
                 f.delegate_mut().spawn_search(test_path_like("hi"), cx)
             })
             .await;
-        finder.read_with(cx, |f, _| assert_eq!(f.delegate().matches.len(), 7));
+        finder.update(cx, |f, _| {
+            assert_eq!(
+                collect_search_results(f),
+                vec![
+                    PathBuf::from("ignored-root/happiness"),
+                    PathBuf::from("ignored-root/height"),
+                    PathBuf::from("ignored-root/hi"),
+                    PathBuf::from("ignored-root/hiccup"),
+                    PathBuf::from("tracked-root/happiness"),
+                    PathBuf::from("tracked-root/height"),
+                    PathBuf::from("tracked-root/hi"),
+                    PathBuf::from("tracked-root/hiccup"),
+                ],
+                "All files in all roots (including gitignored) should be searched"
+            )
+        });
+    }
+
+    #[gpui::test]
+    async fn test_ignored_files(cx: &mut TestAppContext) {
+        let app_state = init_test(cx);
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/root",
+                json!({
+                    ".git": {},
+                    ".gitignore": "ignored_a\n.env\n",
+                    "a": {
+                        "banana_env": "11",
+                        "bandana_env": "12",
+                    },
+                    "ignored_a": {
+                        "ignored_banana_env": "21",
+                        "ignored_bandana_env": "22",
+                        "ignored_nested": {
+                            "ignored_nested_banana_env": "31",
+                            "ignored_nested_bandana_env": "32",
+                        },
+                    },
+                    ".env": "something",
+                }),
+            )
+            .await;
+
+        let project = Project::test(app_state.fs.clone(), ["/root".as_ref()], cx).await;
+        let window = cx.add_window(|cx| Workspace::test_new(project, cx));
+        let workspace = window.root(cx);
+        cx.dispatch_action(window.into(), Toggle);
+
+        let finder = cx.read(|cx| workspace.read(cx).modal::<FileFinder>().unwrap());
+
+        finder
+            .update(cx, |finder, cx| {
+                finder.delegate_mut().update_matches("env".to_string(), cx)
+            })
+            .await;
+        finder.update(cx, |f, _| {
+            assert_eq!(
+                collect_search_results(f),
+                vec![
+                    PathBuf::from(".env"),
+                    PathBuf::from("a/banana_env"),
+                    PathBuf::from("a/bandana_env"),
+                ],
+                "Root gitignored files and all non-gitignored files should be searched"
+            )
+        });
+
+        let _ = workspace
+            .update(cx, |workspace, cx| {
+                workspace.open_abs_path(
+                    PathBuf::from("/root/ignored_a/ignored_banana_env"),
+                    true,
+                    cx,
+                )
+            })
+            .await
+            .unwrap();
+        cx.foreground().run_until_parked();
+        finder
+            .update(cx, |finder, cx| {
+                finder.delegate_mut().update_matches("env".to_string(), cx)
+            })
+            .await;
+        finder.update(cx, |f, _| {
+            assert_eq!(
+                collect_search_results(f),
+                vec![
+                    PathBuf::from(".env"),
+                    PathBuf::from("a/banana_env"),
+                    PathBuf::from("a/bandana_env"),
+                    PathBuf::from("ignored_a/ignored_banana_env"),
+                    PathBuf::from("ignored_a/ignored_bandana_env"),
+                ],
+                "Root gitignored dir got listed and its entries got into worktree, but all gitignored dirs below it were not listed. Old entries + new listed gitignored entries should be searched"
+            )
+        });
     }
 
     #[gpui::test]
@@ -2192,4 +2285,20 @@ mod tests {
             absolute: None,
         }
     }
+
+    fn collect_search_results(picker: &Picker<FileFinderDelegate>) -> Vec<PathBuf> {
+        let matches = &picker.delegate().matches;
+        assert!(
+            matches.history.is_empty(),
+            "Should have no history matches, but got: {:?}",
+            matches.history
+        );
+        let mut results = matches
+            .search
+            .iter()
+            .map(|path_match| Path::new(path_match.path_prefix.as_ref()).join(&path_match.path))
+            .collect::<Vec<_>>();
+        results.sort();
+        results
+    }
 }

crates/file_finder2/src/file_finder.rs

@@ -354,15 +354,10 @@ impl FileFinderDelegate {
         let include_root_name = worktrees.len() > 1;
         let candidate_sets = worktrees
             .into_iter()
-            .map(|worktree| {
-                let worktree = worktree.read(cx);
-                PathMatchCandidateSet {
-                    snapshot: worktree.snapshot(),
-                    include_ignored: worktree
-                        .root_entry()
-                        .map_or(false, |entry| entry.is_ignored),
-                    include_root_name,
-                }
+            .map(|worktree| PathMatchCandidateSet {
+                snapshot: worktree.read(cx).snapshot(),
+                include_ignored: true,
+                include_root_name,
             })
             .collect::<Vec<_>>();
 
@@ -1038,7 +1033,7 @@ mod tests {
     }
 
     #[gpui::test]
-    async fn test_ignored_files(cx: &mut TestAppContext) {
+    async fn test_ignored_root(cx: &mut TestAppContext) {
         let app_state = init_test(cx);
         app_state
             .fs
@@ -1081,7 +1076,94 @@ mod tests {
                 picker.delegate.spawn_search(test_path_like("hi"), cx)
             })
             .await;
-        picker.update(cx, |picker, _| assert_eq!(picker.delegate.matches.len(), 7));
+        picker.update(cx, |picker, _| {
+            assert_eq!(
+                collect_search_results(picker),
+                vec![
+                    PathBuf::from("ignored-root/happiness"),
+                    PathBuf::from("ignored-root/height"),
+                    PathBuf::from("ignored-root/hi"),
+                    PathBuf::from("ignored-root/hiccup"),
+                    PathBuf::from("tracked-root/happiness"),
+                    PathBuf::from("tracked-root/height"),
+                    PathBuf::from("tracked-root/hi"),
+                    PathBuf::from("tracked-root/hiccup"),
+                ],
+                "All files in all roots (including gitignored) should be searched"
+            )
+        });
+    }
+
+    #[gpui::test]
+    async fn test_ignored_files(cx: &mut TestAppContext) {
+        let app_state = init_test(cx);
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/root",
+                json!({
+                    ".git": {},
+                    ".gitignore": "ignored_a\n.env\n",
+                    "a": {
+                        "banana_env": "11",
+                        "bandana_env": "12",
+                    },
+                    "ignored_a": {
+                        "ignored_banana_env": "21",
+                        "ignored_bandana_env": "22",
+                        "ignored_nested": {
+                            "ignored_nested_banana_env": "31",
+                            "ignored_nested_bandana_env": "32",
+                        },
+                    },
+                    ".env": "something",
+                }),
+            )
+            .await;
+
+        let project = Project::test(app_state.fs.clone(), ["/root".as_ref()], cx).await;
+
+        let (picker, workspace, cx) = build_find_picker(project, cx);
+
+        cx.simulate_input("env");
+        picker.update(cx, |picker, _| {
+            assert_eq!(
+                collect_search_results(picker),
+                vec![
+                    PathBuf::from(".env"),
+                    PathBuf::from("a/banana_env"),
+                    PathBuf::from("a/bandana_env"),
+                ],
+                "Root gitignored files and all non-gitignored files should be searched"
+            )
+        });
+
+        let _ = workspace
+            .update(cx, |workspace, cx| {
+                workspace.open_abs_path(
+                    PathBuf::from("/root/ignored_a/ignored_banana_env"),
+                    true,
+                    cx,
+                )
+            })
+            .await
+            .unwrap();
+        cx.run_until_parked();
+        cx.simulate_input("env");
+        picker.update(cx, |picker, _| {
+            assert_eq!(
+                collect_search_results(picker),
+                vec![
+                    PathBuf::from(".env"),
+                    PathBuf::from("a/banana_env"),
+                    PathBuf::from("a/bandana_env"),
+                    PathBuf::from("ignored_a/ignored_banana_env"),
+                    PathBuf::from("ignored_a/ignored_bandana_env"),
+                ],
+                "Root gitignored dir got listed and its entries got into worktree, but all gitignored dirs below it were not listed. Old entries + new listed gitignored entries should be searched"
+            )
+        });
     }
 
     #[gpui::test]
@@ -1846,4 +1928,20 @@ mod tests {
                 .clone()
         })
     }
+
+    fn collect_search_results(picker: &Picker<FileFinderDelegate>) -> Vec<PathBuf> {
+        let matches = &picker.delegate.matches;
+        assert!(
+            matches.history.is_empty(),
+            "Should have no history matches, but got: {:?}",
+            matches.history
+        );
+        let mut results = matches
+            .search
+            .iter()
+            .map(|path_match| Path::new(path_match.path_prefix.as_ref()).join(&path_match.path))
+            .collect::<Vec<_>>();
+        results.sort();
+        results
+    }
 }