improve terminology and documentation

Author: pascalkuthe

gix-worktree/src/index/status.rs

@@ -1,4 +1,8 @@
-//!
+//! Compute changes that need to be applied to the ! index to obtain a different
+//state (either a worktree or a second index). ! Also computes other related
+//information like conflicts/added files and provides ! everything necessary
+//for `gix diff`/`git status` (when not both arguments are revisions) ! and
+//dirtiness checks that are run before commands like `git merge`/`git rebase`
 
 use bstr::BStr;
 
@@ -8,53 +12,65 @@ pub mod worktree;
 ///
 pub mod index;
 
-///
-pub mod recorder;
+mod recorder;
+pub use recorder::Recorder;
 
 ///
-pub mod diff;
+pub mod content;
 
 /// The status of an index entry in a worktree
-#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Default)]
-pub enum Status<T = ()> {
-    #[default]
-    /// The file in the worktree is identical to the index entry
-    Unchanged,
-    /// An index entry has no corresponding file in the worktree.
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)]
+pub enum Change<T = ()> {
+    /// This index entry has been removed
     Removed,
-    /// The type of file changed (symlink <=> file) performing a
-    /// diff is usually not necessary/desired
+    /// The type of file changed (for example symlink <=> file) performing
+    /// a diff
     TypeChange,
-    /// The worktree file that belongs to this index has a changed stat
-    /// therefore could have been modified.
-    ///
-    /// Note that this doesn't necessarily mean that the *content* of the file changed.
-    /// A modified even is emitted whenever a change could have occurred. It is up
-    /// to API consumers to check the file for content changes
+    /// This index entry has been modified in some form (permission change
+    /// or content change or both)
     Modified {
         /// Indicates that one of the stat changes was an executable bit change
         /// which is a significant change itself (for git status)
         /// these files don't need to be rechanged
         executable_bit_changed: bool,
-        /// The output of the diff run on this entry.
+        /// The output of the [`ContentComparison`] run on this entry.
         /// if the there is no content change and only the executable bit
         /// changed than this is `None`
-        diff: Option<T>,
+        content_change: Option<T>,
     },
     /// An index entry that correspond to an untracked worktree file marked with `git add`
     Added,
 }
 
-///
+/// Collects the changes produced by comparing an index entry to
+/// the worktree (or another index)
 pub trait Collector<'index>: Send {
     /// Data generated by comparing two files/entries
-    type Diff;
+    type ContentChange;
     ///
     fn visit_entry(
         &mut self,
         entry: &'index gix_index::Entry,
         path: &'index BStr,
-        status: Status<Self::Diff>,
+        status: Option<Change<Self::ContentChange>>,
         conflict: bool,
     );
 }
+
+/// Compares the content of two blobs while performing a diff
+pub trait ContentComparison: Send + Sync {
+    /// Output data produced by this
+    type Output;
+    /// Compares an index entry to a different blob. The size of the blob
+    /// and a lazy handle to read its data (from disk or ODB) is provided. If
+    /// this function returns `None` the entry and the blob are assumed to be
+    /// identical if this assumption is broken by a faulty implementation then
+    /// the index state can be messed up
+    fn compare_blob<'a, E>(
+        &self,
+        entry: &'a gix_index::Entry,
+        blob_size: usize,
+        blob: impl content::LazyBlob<'a, E>,
+        resolve_oid: impl FnMut(gix_hash::ObjectId) -> Result<&'a [u8], E>,
+    ) -> Result<Option<Self::Output>, E>;
+}

gix-worktree/src/index/status/diff.rs renamed to gix-worktree/src/index/status/content.rs

@@ -4,34 +4,24 @@ use gix_index as index;
 use gix_object::encode::loose_header;
 use index::Entry;
 
-///
+use crate::index::status::ContentComparison;
+
+/// Lazy borrwoed access to blob data from disk or ODB
 pub trait LazyBlob<'a, E> {
-    ///
+    /// Returns the contents of this blob, potentially performs IO
+    /// and other expensive operations and should only be called
+    /// when necessary.
     fn read(self) -> Result<&'a [u8], E>;
 }
 
-///
-pub trait Diff: Send + Sync {
-    ///
-    type Output;
-    ///
-    fn content_changed<'a, E>(
-        &self,
-        entry: &'a Entry,
-        blob_size: usize,
-        blob: impl LazyBlob<'a, E>,
-        resolve_oid: impl FnMut(gix_hash::ObjectId) -> Result<&'a [u8], E>,
-    ) -> Result<Option<Self::Output>, E>;
-}
-
-/// compares to blobs by comparing their size and oid, only looks at the file if
+/// Compares to blobs by comparing their size and oid, only looks at the file if
 /// the size matches, therefore very fast
-pub struct Fast;
+pub struct FastEq;
 
-impl Diff for Fast {
+impl ContentComparison for FastEq {
     type Output = ();
 
-    fn content_changed<'a, E>(
+    fn compare_blob<'a, E>(
         &self,
         entry: &'a Entry,
         blob_size: usize,
@@ -58,12 +48,12 @@ impl Diff for Fast {
 /// Compares files to blobs by comparing their oids. Same as [`Fast`] but does
 /// not contain a fast path for files with mismatched files and therefore always
 /// returns an OID that can be reused later
-pub struct Hash;
+pub struct HashEq;
 
-impl Diff for Hash {
+impl ContentComparison for HashEq {
     type Output = ObjectId;
 
-    fn content_changed<'a, E>(
+    fn compare_blob<'a, E>(
         &self,
         entry: &'a Entry,
         _blob_size: usize,

gix-worktree/src/index/status/recorder.rs

@@ -2,26 +2,27 @@ use bstr::BStr;
 
 use gix_index as index;
 
-use crate::index::status::{Collector, Status};
+use crate::index::status::{Change, Collector};
 
-///
+/// Convenience [`Collecotr`] implementation that collects all non-trivial
+/// changes into a `Vec`
 #[derive(Debug, Default)]
 pub struct Recorder<'index, T = ()> {
-    /// collected records, unchanged fields are excluded
-    pub records: Vec<(&'index BStr, Status<T>, bool)>,
+    /// collected changes, index entries without conflicts or changes are excluded
+    pub records: Vec<(&'index BStr, Option<Change<T>>, bool)>,
 }
 
 impl<'index, T: Send> Collector<'index> for Recorder<'index, T> {
-    type Diff = T;
+    type ContentChange = T;
 
     fn visit_entry(
         &mut self,
         _entry: &'index index::Entry,
         path: &'index BStr,
-        status: Status<Self::Diff>,
+        status: Option<Change<Self::ContentChange>>,
         conflict: bool,
     ) {
-        if !matches!(status, Status::Unchanged) {
+        if conflict || status.is_some() {
             self.records.push((path, status, conflict))
         }
     }

gix-worktree/src/index/status/worktree.rs

@@ -2,8 +2,7 @@ use std::io;
 use std::marker::PhantomData;
 use std::path::Path;
 
-use crate::index::status::diff::{self, Diff};
-use crate::index::status::{Collector, Status};
+use crate::index::status::{content, Change, Collector, ContentComparison};
 use crate::{fs, read};
 use bstr::BStr;
 use filetime::FileTime;
@@ -62,12 +61,18 @@ impl Default for Options {
     }
 }
 
-/// Calculates the status of worktree
-pub fn status<'index, T: Send>(
+/// Calculates the changes that need to be applied to an index to obtain a
+/// worktree. Note that this isn't technically quite what this function does
+/// as this also provides some additional information (whether a file has
+/// conflicts) and files that were added with `git add` are shown as a special
+/// changes despite not technically requiring a change to the index (since `gid
+/// add` already added the file to the index) but the naming matches the intuition
+/// of a git user (and matches `git status`/git diff`)
+pub fn changes_to_obtain<'index, T: Send>(
     index: &'index mut index::State,
     worktree: &Path,
-    collector: &mut impl Collector<'index, Diff = T>,
-    diff: &impl Diff<Output = T>,
+    collector: &mut impl Collector<'index, ContentChange = T>,
+    diff: &impl ContentComparison<Output = T>,
     options: Options,
 ) -> Result<(), Error> {
     // the order is absoluty critical here
@@ -116,13 +121,13 @@ struct State<'a, 'b> {
     options: &'a Options,
 }
 
-type StatusResult<'index, T> = Result<(&'index index::Entry, &'index BStr, Status<T>, bool), Error>;
+type StatusResult<'index, T> = Result<(&'index index::Entry, &'index BStr, Option<Change<T>>, bool), Error>;
 
 impl<'index> State<'_, 'index> {
     fn process<T>(
         &mut self,
         entry: &'index mut index::Entry,
-        diff: &impl Diff<Output = T>,
+        diff: &impl ContentComparison<Output = T>,
     ) -> Option<StatusResult<'index, T>> {
         let conflict = match entry.stage() {
             0 => false,
@@ -146,8 +151,8 @@ impl<'index> State<'_, 'index> {
         &mut self,
         entry: &mut index::Entry,
         git_path: &BStr,
-        diff: &impl Diff<Output = T>,
-    ) -> Result<Status<T>, Error> {
+        diff: &impl ContentComparison<Output = T>,
+    ) -> Result<Option<Change<T>>, Error> {
         // TODO fs caache
         let worktree_path = path::try_from_bstr(git_path).map_err(|_| Error::IllformedUtf8)?;
         let worktree_path = self.worktree.join(worktree_path);
@@ -163,24 +168,24 @@ impl<'index> State<'_, 'index> {
                 // TODO: submodules:
                 //   if entry.mode.contains(Mode::COMMIT) &&
                 //     resolve_gitlink_ref(ce->name, "HEAD", &sub))
-                return Ok(Status::Removed);
+                return Ok(Some(Change::Removed));
             }
             Ok(metadata) => metadata,
-            Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(Status::Removed),
+            Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(Some(Change::Removed)),
             Err(err) => {
                 return Err(err.into());
             }
         };
         if self.options.check_added && entry.flags.contains(index::entry::Flags::INTENT_TO_ADD) {
-            return Ok(Status::Added);
+            return Ok(Some(Change::Added));
         }
         let new_stat = index::entry::Stat::from_fs(&metadata)?;
         let executable_bit_changed =
             match entry
                 .mode
                 .change_to_match_fs(&metadata, self.options.fs.symlink, self.options.fs.executable_bit)
             {
-                Some(index::entry::mode::Change::Type { .. }) => return Ok(Status::TypeChange),
+                Some(index::entry::mode::Change::Type { .. }) => return Ok(Some(Change::TypeChange)),
                 Some(index::entry::mode::Change::ExecutableBit) => true,
                 None => false,
             };
@@ -202,7 +207,7 @@ impl<'index> State<'_, 'index> {
         {
             racy_clean = new_stat.is_racy(self.timestamp, self.options.stat);
             if !racy_clean {
-                return Ok(Status::Unchanged);
+                return Ok(None);
             }
         }
 
@@ -212,22 +217,22 @@ impl<'index> State<'_, 'index> {
             entry,
             options: self.options,
         };
-        let diff = diff.content_changed::<Error>(entry, metadata.len() as usize, file, |_| Ok(&[]))?;
+        let content_change = diff.compare_blob::<Error>(entry, metadata.len() as usize, file, |_| Ok(&[]))?;
         // this file is racy clean! Set the size to 0 so we keep detecting this
         // as the file is updated
-        if diff.is_some() && racy_clean {
+        if content_change.is_some() && racy_clean {
             entry.stat.size = 0;
         }
-        if diff.is_some() || executable_bit_changed {
-            Ok(Status::Modified {
+        if content_change.is_some() || executable_bit_changed {
+            Ok(Some(Change::Modified {
                 executable_bit_changed,
-                diff,
-            })
+                content_change,
+            }))
         } else {
             // don't diff against this file next time since
             // we know the file is unchanged
             entry.stat = new_stat;
-            Ok(Status::Unchanged)
+            Ok(None)
         }
     }
 }
@@ -237,7 +242,7 @@ struct Reducer<'a, 'index, T: Collector<'index>> {
     phantom: PhantomData<fn(&'index ())>,
 }
 
-impl<'index, T, C: Collector<'index, Diff = T>> Reduce for Reducer<'_, 'index, C> {
+impl<'index, T, C: Collector<'index, ContentChange = T>> Reduce for Reducer<'_, 'index, C> {
     type Input = Vec<StatusResult<'index, T>>;
 
     type FeedProduce = ();
@@ -248,8 +253,8 @@ impl<'index, T, C: Collector<'index, Diff = T>> Reduce for Reducer<'_, 'index, C
 
     fn feed(&mut self, items: Self::Input) -> Result<Self::FeedProduce, Self::Error> {
         for item in items {
-            let (entry, path, status, conflict) = item?;
-            self.collector.visit_entry(entry, path, status, conflict);
+            let (entry, path, change, conflict) = item?;
+            self.collector.visit_entry(entry, path, change, conflict);
         }
         Ok(())
     }
@@ -265,7 +270,7 @@ struct WorktreeFile<'a> {
     options: &'a Options,
 }
 
-impl<'a> diff::LazyBlob<'a, Error> for WorktreeFile<'a> {
+impl<'a> content::LazyBlob<'a, Error> for WorktreeFile<'a> {
     fn read(self) -> Result<&'a [u8], Error> {
         let res = read::data_to_buf_with_meta(
             self.path,