Remove `is_ignored` from `PathEntry` and lean more on the tree instead

Antonio Scandurra created

Change summary

zed/src/worktree.rs       | 59 ++++++++++++++++++++---------
zed/src/worktree/fuzzy.rs | 82 +++++++++++++++++++++++++++--------------
2 files changed, 95 insertions(+), 46 deletions(-)

Detailed changes

zed/src/worktree.rs 🔗

@@ -244,6 +244,10 @@ impl Snapshot {
         self.entries.summary().file_count
     }
 
+    pub fn visible_file_count(&self) -> usize {
+        self.entries.summary().visible_file_count
+    }
+
     pub fn root_entry(&self) -> Option<&Entry> {
         self.root_inode.and_then(|inode| self.entries.get(&inode))
     }
@@ -606,12 +610,7 @@ impl Entry {
     fn set_ignored(&mut self, ignored: bool) {
         match self {
             Entry::Dir { is_ignored, .. } => *is_ignored = Some(ignored),
-            Entry::File {
-                is_ignored, path, ..
-            } => {
-                *is_ignored = Some(ignored);
-                path.is_ignored = Some(ignored);
-            }
+            Entry::File { is_ignored, .. } => *is_ignored = Some(ignored),
         }
     }
 
@@ -638,14 +637,25 @@ impl sum_tree::Item for Entry {
     type Summary = EntrySummary;
 
     fn summary(&self) -> Self::Summary {
+        let file_count;
+        let visible_file_count;
+        if let Entry::File { is_ignored, .. } = self {
+            file_count = 1;
+            if is_ignored.unwrap_or(false) {
+                visible_file_count = 0;
+            } else {
+                visible_file_count = 1;
+            }
+        } else {
+            file_count = 0;
+            visible_file_count = 0;
+        }
+
         EntrySummary {
             max_ino: self.inode(),
-            file_count: if matches!(self, Self::File { .. }) {
-                1
-            } else {
-                0
-            },
-            recompute_is_ignored: self.is_ignored().is_none(),
+            file_count,
+            visible_file_count,
+            recompute_ignore_status: self.is_ignored().is_none(),
         }
     }
 }
@@ -662,14 +672,16 @@ impl sum_tree::KeyedItem for Entry {
 pub struct EntrySummary {
     max_ino: u64,
     file_count: usize,
-    recompute_is_ignored: bool,
+    visible_file_count: usize,
+    recompute_ignore_status: bool,
 }
 
 impl<'a> AddAssign<&'a EntrySummary> for EntrySummary {
     fn add_assign(&mut self, rhs: &'a EntrySummary) {
         self.max_ino = rhs.max_ino;
         self.file_count += rhs.file_count;
-        self.recompute_is_ignored |= rhs.recompute_is_ignored;
+        self.visible_file_count += rhs.visible_file_count;
+        self.recompute_ignore_status |= rhs.recompute_ignore_status;
     }
 }
 
@@ -688,6 +700,15 @@ impl<'a> sum_tree::Dimension<'a, EntrySummary> for FileCount {
     }
 }
 
+#[derive(Copy, Clone, Default, Debug, Eq, PartialEq, Ord, PartialOrd)]
+struct VisibleFileCount(usize);
+
+impl<'a> sum_tree::Dimension<'a, EntrySummary> for VisibleFileCount {
+    fn add_summary(&mut self, summary: &'a EntrySummary) {
+        self.0 += summary.visible_file_count;
+    }
+}
+
 struct BackgroundScanner {
     snapshot: Arc<Mutex<Snapshot>>,
     notify: Sender<ScanState>,
@@ -802,7 +823,7 @@ impl BackgroundScanner {
                 None,
                 Entry::File {
                     parent: None,
-                    path: PathEntry::new(inode, &relative_path, None),
+                    path: PathEntry::new(inode, &relative_path),
                     inode,
                     is_symlink,
                     is_ignored: None,
@@ -854,7 +875,7 @@ impl BackgroundScanner {
                     child_name,
                     Entry::File {
                         parent: Some(job.inode),
-                        path: PathEntry::new(child_inode, &child_relative_path, None),
+                        path: PathEntry::new(child_inode, &child_relative_path),
                         inode: child_inode,
                         is_symlink: child_is_symlink,
                         is_ignored: None,
@@ -1046,7 +1067,10 @@ impl BackgroundScanner {
 
             scope.execute(|| {
                 let entries_tx = entries_tx;
-                for entry in snapshot.entries.filter::<_, ()>(|e| e.recompute_is_ignored) {
+                for entry in snapshot
+                    .entries
+                    .filter::<_, ()>(|e| e.recompute_ignore_status)
+                {
                     entries_tx.send(entry.clone()).unwrap();
                 }
             });
@@ -1110,7 +1134,6 @@ impl BackgroundScanner {
                     root_path
                         .parent()
                         .map_or(path, |parent| path.strip_prefix(parent).unwrap()),
-                    None,
                 ),
                 is_ignored: None,
             }

zed/src/worktree/fuzzy.rs 🔗

@@ -2,7 +2,7 @@ use gpui::scoped_pool;
 
 use crate::sum_tree::SeekBias;
 
-use super::{char_bag::CharBag, Entry, FileCount, Snapshot};
+use super::{char_bag::CharBag, Entry, FileCount, Snapshot, VisibleFileCount};
 
 use std::{
     cmp::{max, min, Ordering, Reverse},
@@ -21,11 +21,10 @@ pub struct PathEntry {
     pub path_chars: CharBag,
     pub path: Arc<[char]>,
     pub lowercase_path: Arc<[char]>,
-    pub is_ignored: Option<bool>,
 }
 
 impl PathEntry {
-    pub fn new(ino: u64, path: &Path, is_ignored: Option<bool>) -> Self {
+    pub fn new(ino: u64, path: &Path) -> Self {
         let path = path.to_string_lossy();
         let lowercase_path = path.to_lowercase().chars().collect::<Vec<_>>().into();
         let path: Arc<[char]> = path.chars().collect::<Vec<_>>().into();
@@ -36,7 +35,6 @@ impl PathEntry {
             path_chars,
             path,
             lowercase_path,
-            is_ignored,
         }
     }
 }
@@ -90,7 +88,12 @@ where
     let query_chars = CharBag::from(&lowercase_query[..]);
 
     let cpus = num_cpus::get();
-    let path_count: usize = snapshots.clone().map(Snapshot::file_count).sum();
+    let path_count: usize = if include_ignored {
+        snapshots.clone().map(Snapshot::file_count).sum()
+    } else {
+        snapshots.clone().map(Snapshot::visible_file_count).sum()
+    };
+
     let segment_size = (path_count + cpus - 1) / cpus;
     let mut segment_results = (0..cpus).map(|_| BinaryHeap::new()).collect::<Vec<_>>();
 
@@ -111,22 +114,15 @@ where
 
                 let mut tree_start = 0;
                 for snapshot in trees {
-                    let tree_end = tree_start + snapshot.file_count();
+                    let tree_end = if include_ignored {
+                        tree_start + snapshot.file_count()
+                    } else {
+                        tree_start + snapshot.visible_file_count()
+                    };
                     if tree_start < segment_end && segment_start < tree_end {
                         let start = max(tree_start, segment_start) - tree_start;
                         let end = min(tree_end, segment_end) - tree_start;
-                        let mut cursor = snapshot.entries.cursor::<_, ()>();
-                        cursor.seek(&FileCount(start), SeekBias::Right);
-                        let path_entries = cursor
-                            .filter_map(|e| {
-                                if let Entry::File { path, .. } = e {
-                                    Some(path)
-                                } else {
-                                    None
-                                }
-                            })
-                            .take(end - start);
-
+                        let path_entries = path_entries_iter(snapshot, start, end, include_ignored);
                         let skipped_prefix_len = if include_root_name {
                             0
                         } else if let Some(Entry::Dir { .. }) = snapshot.root_entry() {
@@ -145,8 +141,7 @@ where
                             path_entries,
                             query,
                             lowercase_query,
-                            query_chars.clone(),
-                            include_ignored,
+                            query_chars,
                             smart_case,
                             results,
                             max_results,
@@ -176,6 +171,44 @@ where
     results
 }
 
+fn path_entries_iter<'a>(
+    snapshot: &'a Snapshot,
+    start: usize,
+    end: usize,
+    include_ignored: bool,
+) -> impl Iterator<Item = &'a PathEntry> {
+    let mut files_cursor = None;
+    let mut visible_files_cursor = None;
+    if include_ignored {
+        let mut cursor = snapshot.entries.cursor::<_, ()>();
+        cursor.seek(&FileCount(start), SeekBias::Right);
+        files_cursor = Some(cursor);
+    } else {
+        let mut cursor = snapshot.entries.cursor::<_, ()>();
+        cursor.seek(&VisibleFileCount(start), SeekBias::Right);
+        visible_files_cursor = Some(cursor);
+    }
+    files_cursor
+        .into_iter()
+        .flatten()
+        .chain(visible_files_cursor.into_iter().flatten())
+        .filter_map(move |e| {
+            if let Entry::File {
+                path, is_ignored, ..
+            } = e
+            {
+                if is_ignored.unwrap_or(false) && !include_ignored {
+                    None
+                } else {
+                    Some(path)
+                }
+            } else {
+                None
+            }
+        })
+        .take(end - start)
+}
+
 fn match_single_tree_paths<'a>(
     snapshot: &Snapshot,
     skipped_prefix_len: usize,
@@ -183,7 +216,6 @@ fn match_single_tree_paths<'a>(
     query: &[char],
     lowercase_query: &[char],
     query_chars: CharBag,
-    include_ignored: bool,
     smart_case: bool,
     results: &mut BinaryHeap<Reverse<PathMatch>>,
     max_results: usize,
@@ -194,11 +226,7 @@ fn match_single_tree_paths<'a>(
     best_position_matrix: &mut Vec<usize>,
 ) {
     for path_entry in path_entries {
-        if !path_entry.path_chars.is_superset(query_chars.clone()) {
-            continue;
-        }
-
-        if !include_ignored && path_entry.is_ignored.unwrap_or(false) {
+        if !path_entry.path_chars.is_superset(query_chars) {
             continue;
         }
 
@@ -502,7 +530,6 @@ mod tests {
                 path_chars,
                 path,
                 lowercase_path,
-                is_ignored: Some(false),
             });
         }
 
@@ -526,7 +553,6 @@ mod tests {
             &query[..],
             &lowercase_query[..],
             query_chars,
-            true,
             smart_case,
             &mut results,
             100,