Merge pull request #2352 from GitoxideLabs/gix-error

`gix-error` punch-through

Author: Byron

Cargo.lock

@@ -236,6 +236,7 @@ members = [
     "gix-trace",
     "gix-commitgraph",
     "gix-chunk",
+    "gix-error",
     "gix-quote",
     "gix-object",
     "gix-glob",

Cargo.toml

@@ -141,6 +141,7 @@ is usable to some extent.
   * [gix-dir](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-dir)
   * [gix-merge](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-merge)
   * [gix-shallow](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-shallow)
+  * [gix-error](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-error)
   * `gitoxide-core`
 * **very early**  _(possibly without any documentation and many rough edges)_
   * [gix-blame](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-blame)

README.md

@@ -572,8 +572,12 @@ A mechanism to associate metadata with any object, and keep revisions of it usin
      - note that it's less critical to support it as `gitoxide` allows access but prevents untrusted configuration to become effective.
 
 ### gix-date
-* [ ] parse git dates
-* [ ] serialize `Time`
+* [x] parse git dates
+* [x] serialize `Time`
+
+### gix-error
+
+A basic crate for comon error types and utilities, changed as needed to replace `thiserror`.
 
 ### gix-credentials
 * [x] launch git credentials helpers with a given action

crate-status.md

@@ -125,15 +125,21 @@ fn resolve_revspec(
     let result = repo.rev_parse(revspec.as_bstr());
 
     match result {
-        Err(gix::revision::spec::parse::Error::FindReference(gix::refs::file::find::existing::Error::NotFound {
-            name,
-        })) => {
-            let root = repo.workdir().map(ToOwned::to_owned);
-            let name = gix::path::os_string_into_bstring(name.into())?;
-
-            Ok((ObjectId::null(gix::hash::Kind::Sha1), root, name))
+        Err(err) => {
+            // When the revspec is just a name, the delegate tries to resolve a reference which fails.
+            // We extract the error from the tree to learn the name, and treat it as file.
+            let not_found = err
+                .iter_frames()
+                .find_map(|f| f.downcast::<gix::refs::file::find::existing::Error>());
+            if let Some(gix::refs::file::find::existing::Error::NotFound { name }) = not_found {
+                let root = repo.workdir().map(ToOwned::to_owned);
+                let name = gix::path::os_string_into_bstring(name.into())?;
+
+                Ok((ObjectId::null(gix::hash::Kind::Sha1), root, name))
+            } else {
+                Err(err.into())
+            }
         }
-        Err(err) => Err(err.into()),
         Ok(resolved_revspec) => {
             let blob_id = resolved_revspec
                 .single()

gitoxide-core/src/repository/diff.rs

@@ -9,12 +9,13 @@ use gix::{
             Delegate,
         },
     },
+    Exn,
 };
 
 pub fn explain(spec: std::ffi::OsString, mut out: impl std::io::Write) -> anyhow::Result<()> {
     let mut explain = Explain::new(&mut out);
     let spec = gix::path::os_str_into_bstr(&spec)?;
-    gix::revision::plumbing::spec::parse(spec, &mut explain)?;
+    gix::revision::plumbing::spec::parse(spec, &mut explain).map_err(gix::Error::from)?;
     if let Some(err) = explain.err {
         bail!(err);
     }
@@ -41,9 +42,10 @@ impl<'a> Explain<'a> {
             err: None,
         }
     }
-    fn prefix(&mut self) -> Option<()> {
+    fn prefix(&mut self) -> Result<(), Exn> {
         self.call += 1;
-        write!(self.out, "{:02}. ", self.call).ok()
+        write!(self.out, "{:02}. ", self.call).ok();
+        Ok(())
     }
     fn revision_name(&self) -> BString {
         self.ref_name.clone().unwrap_or_else(|| {
@@ -56,13 +58,18 @@ impl<'a> Explain<'a> {
 }
 
 impl delegate::Revision for Explain<'_> {
-    fn find_ref(&mut self, name: &BStr) -> Option<()> {
+    fn find_ref(&mut self, name: &BStr) -> Result<(), Exn> {
         self.prefix()?;
         self.ref_name = Some(name.into());
-        writeln!(self.out, "Lookup the '{name}' reference").ok()
+        writeln!(self.out, "Lookup the '{name}' reference").ok();
+        Ok(())
     }
 
-    fn disambiguate_prefix(&mut self, prefix: gix::hash::Prefix, hint: Option<delegate::PrefixHint<'_>>) -> Option<()> {
+    fn disambiguate_prefix(
+        &mut self,
+        prefix: gix::hash::Prefix,
+        hint: Option<delegate::PrefixHint<'_>>,
+    ) -> Result<(), Exn> {
         self.prefix()?;
         self.oid_prefix = Some(prefix);
         writeln!(
@@ -76,10 +83,11 @@ impl delegate::Revision for Explain<'_> {
                     format!("commit {generation} generations in future of reference {ref_name:?}"),
             }
         )
-        .ok()
+        .ok();
+        Ok(())
     }
 
-    fn reflog(&mut self, query: ReflogLookup) -> Option<()> {
+    fn reflog(&mut self, query: ReflogLookup) -> Result<(), Exn> {
         self.prefix()?;
         self.has_implicit_anchor = true;
         let ref_name: &BStr = self.ref_name.as_ref().map_or_else(|| "HEAD".into(), AsRef::as_ref);
@@ -92,16 +100,18 @@ impl delegate::Revision for Explain<'_> {
                 ref_name
             )
             .ok(),
-        }
+        };
+        Ok(())
     }
 
-    fn nth_checked_out_branch(&mut self, branch_no: usize) -> Option<()> {
+    fn nth_checked_out_branch(&mut self, branch_no: usize) -> Result<(), Exn> {
         self.prefix()?;
         self.has_implicit_anchor = true;
-        writeln!(self.out, "Find the {branch_no}th checked-out branch of 'HEAD'").ok()
+        writeln!(self.out, "Find the {branch_no}th checked-out branch of 'HEAD'").ok();
+        Ok(())
     }
 
-    fn sibling_branch(&mut self, kind: SiblingBranch) -> Option<()> {
+    fn sibling_branch(&mut self, kind: SiblingBranch) -> Result<(), Exn> {
         self.prefix()?;
         self.has_implicit_anchor = true;
         let ref_info = match self.ref_name.as_ref() {
@@ -117,12 +127,13 @@ impl delegate::Revision for Explain<'_> {
             },
             ref_info
         )
-        .ok()
+        .ok();
+        Ok(())
     }
 }
 
 impl delegate::Navigate for Explain<'_> {
-    fn traverse(&mut self, kind: Traversal) -> Option<()> {
+    fn traverse(&mut self, kind: Traversal) -> Result<(), Exn> {
         self.prefix()?;
         let name = self.revision_name();
         writeln!(
@@ -133,10 +144,11 @@ impl delegate::Navigate for Explain<'_> {
                 Traversal::NthParent(no) => format!("Select the {no}. parent of revision named '{name}'"),
             }
         )
-        .ok()
+        .ok();
+        Ok(())
     }
 
-    fn peel_until(&mut self, kind: PeelTo<'_>) -> Option<()> {
+    fn peel_until(&mut self, kind: PeelTo<'_>) -> Result<(), Exn> {
         self.prefix()?;
         writeln!(
             self.out,
@@ -148,10 +160,11 @@ impl delegate::Navigate for Explain<'_> {
                 PeelTo::Path(path) => format!("Lookup the object at '{path}' from the current tree-ish"),
             }
         )
-        .ok()
+        .ok();
+        Ok(())
     }
 
-    fn find(&mut self, regex: &BStr, negated: bool) -> Option<()> {
+    fn find(&mut self, regex: &BStr, negated: bool) -> Result<(), Exn> {
         self.prefix()?;
         self.has_implicit_anchor = true;
         let negate_text = if negated { "does not match" } else { "matches" };
@@ -172,10 +185,11 @@ impl delegate::Navigate for Explain<'_> {
                 ),
             }
         )
-        .ok()
+        .ok();
+        Ok(())
     }
 
-    fn index_lookup(&mut self, path: &BStr, stage: u8) -> Option<()> {
+    fn index_lookup(&mut self, path: &BStr, stage: u8) -> Result<(), Exn> {
         self.prefix()?;
         self.has_implicit_anchor = true;
         writeln!(
@@ -190,12 +204,13 @@ impl delegate::Navigate for Explain<'_> {
                 _ => unreachable!("BUG: parser assures of that"),
             }
         )
-        .ok()
+        .ok();
+        Ok(())
     }
 }
 
 impl delegate::Kind for Explain<'_> {
-    fn kind(&mut self, kind: spec::Kind) -> Option<()> {
+    fn kind(&mut self, kind: spec::Kind) -> Result<(), Exn> {
         self.prefix()?;
         self.call = 0;
         writeln!(
@@ -211,14 +226,16 @@ impl delegate::Kind for Explain<'_> {
                     unreachable!("BUG: 'single' mode is implied but cannot be set explicitly"),
             }
         )
-        .ok()
+        .ok();
+        Ok(())
     }
 }
 
 impl Delegate for Explain<'_> {
-    fn done(&mut self) {
+    fn done(&mut self) -> Result<(), Exn> {
         if !self.has_implicit_anchor && self.ref_name.is_none() && self.oid_prefix.is_none() {
             self.err = Some("Incomplete specification lacks its anchor, like a reference or object name".into());
         }
+        Ok(())
     }
 }

gitoxide-core/src/repository/revision/explain.rs

@@ -15,7 +15,7 @@ mod _ref {
         }
 
         /// Try to parse the timestamp and create an owned instance from this shared one.
-        pub fn to_owned(&self) -> Result<Signature, gix_date::parse::Error> {
+        pub fn to_owned(&self) -> Result<Signature, gix_date::Error> {
             Ok(Signature {
                 name: self.name.to_owned(),
                 email: self.email.to_owned(),
@@ -58,7 +58,7 @@ mod _ref {
 
         /// Parse the `time` field for access to the passed time since unix epoch, and the time offset.
         /// The format is expected to be [raw](gix_date::parse_header()).
-        pub fn time(&self) -> Result<gix_date::Time, gix_date::parse::Error> {
+        pub fn time(&self) -> Result<gix_date::Time, gix_date::Error> {
             self.time.parse()
         }
     }

gix-actor/src/signature/mod.rs

@@ -413,7 +413,9 @@ fn since() -> gix_testtools::Result {
         gix_blame::Options {
             diff_algorithm: gix_diff::blob::Algorithm::Histogram,
             ranges: BlameRanges::default(),
-            since: Some(gix_date::parse("2025-01-31", None)?),
+            since: Some(
+                gix_date::parse("2025-01-31", None).expect("TODO: should be able to to retrieve inner from Exn"),
+            ),
             rewrites: Some(gix_diff::Rewrites::default()),
             debug_track_path: false,
         },

gix-blame/tests/blame.rs

@@ -19,11 +19,11 @@ doctest = false
 serde = ["dep:serde", "bstr/serde"]
 
 [dependencies]
+gix-error = { version = "0.0.0", path = "../gix-error" }
 bstr = { version = "1.12.0", default-features = false, features = ["std"] }
 serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] }
 itoa = "1.0.17"
 jiff = "0.2.17"
-thiserror = "2.0.17"
 # TODO: used for quick and easy `TimeBacking: std::io::Write` implementation, but could make that `Copy`
 #       and remove this dep with custom impl
 smallvec = { version = "1.15.1", features = ["write"] }

gix-date/Cargo.toml

@@ -15,6 +15,8 @@ pub mod time;
 pub mod parse;
 pub use parse::function::{parse, parse_header};
 
+pub use gix_error::ParseError as Error;
+
 /// A timestamp with timezone.
 #[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]