Improve the way pruned items are handled

That way, consumers can start observing more items, which helps with UI.

Author: Byron

gix-dir/src/entry.rs

@@ -27,7 +27,7 @@ pub enum Kind {
 pub enum Status {
     /// The filename of an entry was `.git`, which is generally pruned.
     DotGit,
-    /// The provided pathspec prevented further processing as the path didn't match, or it is a `.git` directory.
+    /// The provided pathspec prevented further processing as the path didn't match.
     /// If this happens, no further checks are done so we wouldn't know if the path is also ignored for example (by mention in `.gitignore`).
     Pruned,
     /// Always in conjunction with a directory on disk that is also known as cone-mode sparse-checkout exclude marker - i.e. a directory

gix-dir/src/lib.rs

@@ -30,13 +30,14 @@ pub struct EntryRef<'a> {
     /// pruned very early on.
     pub status: entry::Status,
     /// Further specify the what the entry is on disk, similar to a file mode.
-    pub disk_kind: entry::Kind,
+    /// This is `None` if the entry was pruned by a pathspec that could not match, as we then won't invest the time to obtain
+    /// the kind of the entry on disk.
+    pub disk_kind: Option<entry::Kind>,
     /// The kind of entry according to the index, if tracked. *Usually* the same as `disk_kind`.
     pub index_kind: Option<entry::Kind>,
-    /// Determines how the pathspec matched, which is information that is definitely available for untracked files,
-    /// but will be `None` for directories and repositories as they don't necessarily have to match to allow recursion,
-    /// which is why they are not tested.
-    /// Can also be `None` if no pathspec matched, but will be `Some(PathspecMatch::Excluded)` if a negative pathspec matched.
+    /// Determines how the pathspec matched.
+    /// Can also be `None` if no pathspec matched, or if the status check stopped prior to checking for pathspec matches which is the case for [`entry::Status::DotGit`].
+    /// Note that it can also be `Some(PathspecMatch::Excluded)` if a negative pathspec matched.
     pub pathspec_match: Option<entry::PathspecMatch>,
 }
 
@@ -48,7 +49,7 @@ pub struct Entry {
     /// The status of entry, most closely related to what we know from `git status`, but not the same.
     pub status: entry::Status,
     /// Further specify the what the entry is on disk, similar to a file mode.
-    pub disk_kind: entry::Kind,
+    pub disk_kind: Option<entry::Kind>,
     /// The kind of entry according to the index, if tracked. *Usually* the same as `disk_kind`.
     pub index_kind: Option<entry::Kind>,
     /// Indicate how the pathspec matches the entry. See more in [`EntryRef::pathspec_match`].

gix-dir/src/walk/classify.rs

@@ -89,7 +89,7 @@ impl From<&Entry> for Outcome {
     fn from(e: &Entry) -> Self {
         Outcome {
             status: e.status,
-            disk_kind: e.disk_kind.into(),
+            disk_kind: e.disk_kind,
             index_kind: e.index_kind,
             pathspec_match: e.pathspec_match,
         }
@@ -147,26 +147,27 @@ pub fn path(
 
     maybe_status = maybe_status
         .or_else(|| (index_kind.map(|k| k.is_dir()) == kind.map(|k| k.is_dir())).then_some(entry::Status::Tracked));
+
+    // We always check the pathspec to have the value filled in reliably.
+    out.pathspec_match = ctx
+        .pathspec
+        .pattern_matching_relative_path(
+            rela_path.as_bstr(),
+            disk_kind.map(|ft| ft.is_dir()),
+            ctx.pathspec_attributes,
+        )
+        .map(|m| {
+            if m.is_excluded() {
+                PathspecMatch::Excluded
+            } else {
+                m.kind.into()
+            }
+        });
     if let Some(status) = maybe_status {
         return Ok(out.with_status(status).with_kind(kind, index_kind));
     }
     debug_assert!(maybe_status.is_none(), "It only communicates a single stae right now");
 
-    let mut do_pathspec_match = || {
-        ctx.pathspec
-            .pattern_matching_relative_path(
-                rela_path.as_bstr(),
-                disk_kind.map(|ft| ft.is_dir()),
-                ctx.pathspec_attributes,
-            )
-            .map(|m| {
-                if m.is_excluded() {
-                    PathspecMatch::Excluded
-                } else {
-                    m.kind.into()
-                }
-            })
-    };
     let mut maybe_upgrade_to_repository = |current_kind| {
         if recurse_repositories {
             return current_kind;
@@ -201,7 +202,6 @@ pub fn path(
         .map_err(Error::ExcludesAccess)?
     {
         if emit_ignored.is_some() {
-            out.pathspec_match = do_pathspec_match();
             if kind.map_or(false, |d| d.is_dir()) && out.pathspec_match.is_none() {
                 // we have patterns that didn't match at all. Try harder.
                 out.pathspec_match = ctx
@@ -223,11 +223,8 @@ pub fn path(
 
     if kind.map_or(false, |ft| ft.is_dir()) {
         kind = maybe_upgrade_to_repository(kind);
-    } else {
-        out.pathspec_match = do_pathspec_match();
-        if out.pathspec_match.is_none() {
-            status = entry::Status::Pruned;
-        }
+    } else if out.pathspec_match.is_none() {
+        status = entry::Status::Pruned;
     }
     Ok(out.with_status(status).with_kind(kind, index_kind))
 }

gix-dir/src/walk/function.rs

@@ -126,20 +126,13 @@ pub(super) fn can_recurse(
     for_deletion: Option<ForDeletionMode>,
     delegate: &mut dyn Delegate,
 ) -> bool {
-    let disk_kind = match info.disk_kind {
-        None => {
-            // NOTE: assume something is wrong with the entry on disk, so just skip it.
-            return false;
-        }
-        Some(kind) => kind,
-    };
-    if !disk_kind.is_dir() {
+    if info.disk_kind.map_or(true, |k| !k.is_dir()) {
         return false;
     }
     let entry = EntryRef {
         rela_path: Cow::Borrowed(rela_path),
         status: info.status,
-        disk_kind,
+        disk_kind: info.disk_kind,
         index_kind: info.index_kind,
         pathspec_match: info.pathspec_match,
     };
@@ -157,29 +150,21 @@ pub(super) fn emit_entry(
         emit_tracked,
         emit_ignored,
         emit_empty_directories,
-        for_deletion,
         ..
     }: Options,
     out: &mut Outcome,
     delegate: &mut dyn Delegate,
 ) -> Action {
     out.seen_entries += 1;
-    let kind = match info.disk_kind {
-        Some(kind) => kind,
-        None => {
-            // NOTE: assume something is wrong with the entry on disk, so just skip it.
-            return Action::Continue;
-        }
-    };
-
-    if for_deletion.is_some() && info.pathspec_match.is_none() {
-        return Action::Continue;
-    }
 
-    if (!emit_empty_directories && kind == entry::Kind::EmptyDirectory
+    if (!emit_empty_directories && info.disk_kind == Some(entry::Kind::EmptyDirectory)
         || !emit_tracked && info.status == entry::Status::Tracked)
         || emit_ignored.is_none() && matches!(info.status, entry::Status::Ignored(_))
-        || !emit_pruned && info.status.is_pruned()
+        || !emit_pruned
+            && (info.status.is_pruned()
+                || info
+                    .pathspec_match
+                    .map_or(true, |m| m == entry::PathspecMatch::Excluded))
     {
         return Action::Continue;
     }
@@ -189,7 +174,7 @@ pub(super) fn emit_entry(
         EntryRef {
             rela_path,
             status: info.status,
-            disk_kind: kind,
+            disk_kind: info.disk_kind,
             index_kind: info.index_kind,
             pathspec_match: info.pathspec_match,
         },

gix-dir/src/walk/mod.rs

@@ -73,7 +73,7 @@ pub trait Delegate {
     fn can_recurse(&mut self, entry: EntryRef<'_>, for_deletion: Option<ForDeletionMode>) -> bool {
         entry
             .status
-            .can_recurse(Some(entry.disk_kind), entry.pathspec_match, for_deletion)
+            .can_recurse(entry.disk_kind, entry.pathspec_match, for_deletion)
     }
 }
 

gix-dir/src/walk/readdir.rs

@@ -103,19 +103,11 @@ pub(super) struct State {
 impl State {
     /// Hold the entry with the given `status` if it's a candidate for collapsing the containing directory.
     fn held_for_directory_collapse(&mut self, rela_path: &BStr, info: classify::Outcome, opts: &Options) -> bool {
-        let kind = match info.disk_kind {
-            Some(kind) => kind,
-            None => {
-                // NOTE: this can be a `.git` directory or file, and we don't get the file type for it, or it's pruned files.
-                return false;
-            }
-        };
-
         if opts.should_hold(info.status) {
             self.on_hold.push(Entry {
                 rela_path: rela_path.to_owned(),
                 status: info.status,
-                disk_kind: kind,
+                disk_kind: info.disk_kind,
                 index_kind: info.index_kind,
                 pathspec_match: info.pathspec_match,
             });
@@ -155,7 +147,7 @@ impl Mark {
         delegate: &mut dyn walk::Delegate,
     ) -> walk::Action {
         if num_entries == 0 {
-            let mut empty_info = classify::Outcome {
+            let empty_info = classify::Outcome {
                 disk_kind: if num_entries == 0 {
                     assert_ne!(
                         dir_info.disk_kind,
@@ -168,19 +160,11 @@ impl Mark {
                 },
                 ..dir_info
             };
-            debug_assert!(
-                dir_info.pathspec_match.is_none(),
-                "we don't execute pathspecs on directories as they not emitted, unless they are empty"
-            );
-            empty_info.pathspec_match = ctx
-                .pathspec
-                .directory_matches_prefix(dir_rela_path.as_bstr(), true)
-                .then_some(PathspecMatch::Always);
             if opts.should_hold(empty_info.status) {
                 state.on_hold.push(Entry {
                     rela_path: dir_rela_path.to_owned(),
                     status: empty_info.status,
-                    disk_kind: empty_info.disk_kind.expect("known by now"),
+                    disk_kind: empty_info.disk_kind,
                     index_kind: empty_info.index_kind,
                     pathspec_match: empty_info.pathspec_match,
                 });
@@ -244,7 +228,7 @@ impl Mark {
             .map(|e| (e.disk_kind, e.status, e.pathspec_match))
         {
             entries += 1;
-            if kind == entry::Kind::Repository {
+            if kind == Some(entry::Kind::Repository) {
                 *prevent_collapse = true;
                 return None;
             }
@@ -325,7 +309,7 @@ impl Mark {
         state.on_hold.push(Entry {
             rela_path: dir_rela_path.to_owned(),
             status: dir_status,
-            disk_kind: dir_info.disk_kind.expect("set for directories"),
+            disk_kind: dir_info.disk_kind,
             index_kind: dir_info.index_kind,
             pathspec_match: dir_pathspec_match,
         });

gix-dir/tests/dir_walk_cwd.rs

@@ -1,4 +1,4 @@
-use crate::walk_utils::{collect, entrya, fixture, options};
+use crate::walk_utils::{collect, entry, fixture, options};
 use gix_dir::entry::Kind::File;
 use gix_dir::entry::Status::Untracked;
 use gix_dir::walk;
@@ -24,9 +24,9 @@ fn prefixes_work_as_expected() -> gix_testtools::Result {
     assert_eq!(
         &entries,
         &[
-            entrya("d/a", Untracked, File),
-            entrya("d/b", Untracked, File),
-            entrya("d/d/a", Untracked, File),
+            entry("d/a", Untracked, File),
+            entry("d/b", Untracked, File),
+            entry("d/d/a", Untracked, File),
         ]
     );
     Ok(())