diff --git a/Cargo.lock b/Cargo.lock index b7ca2f8744b..dcbe9847936 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2272,6 +2272,7 @@ name = "gix-refspec" version = "0.35.0" dependencies = [ "bstr", + "gix-error", "gix-glob", "gix-hash", "gix-revision", @@ -2299,9 +2300,9 @@ dependencies = [ "gix-revwalk", "gix-testtools", "gix-trace", + "insta", "permutohedron", "serde", - "thiserror 2.0.17", ] [[package]] diff --git a/gitoxide-core/src/repository/revision/explain.rs b/gitoxide-core/src/repository/revision/explain.rs index 45e67427b48..4a4565ca412 100644 --- a/gitoxide-core/src/repository/revision/explain.rs +++ b/gitoxide-core/src/repository/revision/explain.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>) -> Option<()> { + fn disambiguate_prefix( + &mut self, + prefix: gix::hash::Prefix, + hint: Option>, + ) -> 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(()) } } diff --git a/gix-error/src/error.rs b/gix-error/src/error.rs new file mode 100644 index 00000000000..a3f23b5097c --- /dev/null +++ b/gix-error/src/error.rs @@ -0,0 +1,55 @@ +use crate::{exn, Error, Exn}; +use std::fmt::Formatter; + +pub(super) enum Inner { + Boxed(Box), + Exn(Box), +} + +impl Error { + /// Create a new instance representing the given `error`. + pub fn from_error(error: impl std::error::Error + Send + Sync + 'static) -> Self { + Error { + inner: Inner::Boxed(Box::new(error)), + } + } +} + +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match &self.inner { + Inner::Boxed(err) => std::fmt::Display::fmt(&*err, f), + Inner::Exn(frame) => std::fmt::Display::fmt(frame, f), + } + } +} + +impl std::fmt::Debug for Error { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match &self.inner { + Inner::Boxed(err) => std::fmt::Debug::fmt(&*err, f), + Inner::Exn(frame) => std::fmt::Debug::fmt(frame, f), + } + } +} + +impl std::error::Error for Error { + /// Return the first source of an [Exn](crate::Exn) error, or the source of a boxed error. + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match &self.inner { + Inner::Boxed(err) => err.source(), + Inner::Exn(frame) => frame.children().first().map(|f| f.as_error()), + } + } +} + +impl From> for Error +where + E: std::error::Error + Send + Sync + 'static, +{ + fn from(err: Exn) -> Self { + Error { + inner: Inner::Exn(err.into()), + } + } +} diff --git a/gix-error/src/exn/debug.rs b/gix-error/src/exn/debug.rs index 785fd5efe8e..b038b29d7d2 100644 --- a/gix-error/src/exn/debug.rs +++ b/gix-error/src/exn/debug.rs @@ -32,14 +32,16 @@ impl fmt::Debug for Frame { fn write_exn(f: &mut Formatter<'_>, frame: &Frame, level: usize, prefix: &str) -> fmt::Result { write!(f, "{}", frame.as_error())?; - write_location(f, frame)?; + if !f.alternate() { + write_location(f, frame)?; + } let children = frame.children(); let children_len = children.len(); for (i, child) in children.iter().enumerate() { write!(f, "\n{prefix}|")?; - write!(f, "\n{prefix}|-> ")?; + write!(f, "\n{prefix}└─ ")?; let child_child_len = child.children().len(); if level == 0 && children_len == 1 && child_child_len == 1 { diff --git a/gix-error/src/exn/display.rs b/gix-error/src/exn/display.rs index 4abb5bbfc10..1af7674f467 100644 --- a/gix-error/src/exn/display.rs +++ b/gix-error/src/exn/display.rs @@ -14,10 +14,16 @@ use std::fmt; -use crate::exn::Exn; +use crate::exn::{Exn, Frame}; impl fmt::Display for Exn { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self.as_error()) } } + +impl fmt::Display for Frame { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.as_error()) + } +} diff --git a/gix-error/src/exn/ext.rs b/gix-error/src/exn/ext.rs index 8dabb30deb0..d71e0b72021 100644 --- a/gix-error/src/exn/ext.rs +++ b/gix-error/src/exn/ext.rs @@ -13,6 +13,7 @@ // limitations under the License. use crate::exn::Exn; +use std::error::Error; /// A trait bound of the supported error type of [`Exn`]. pub trait ErrorExt: std::error::Error + Send + Sync + 'static { @@ -24,6 +25,15 @@ pub trait ErrorExt: std::error::Error + Send + Sync + 'static { { Exn::new(self) } + + /// Raise this error as a new exception, with type erasure. + #[track_caller] + fn raise_something(self) -> Exn + where + Self: Sized, + { + Exn::new(self).something() + } } impl ErrorExt for T where T: std::error::Error + Send + Sync + 'static {} @@ -38,6 +48,12 @@ pub trait OptionExt { where A: std::error::Error + Send + Sync + 'static, F: FnOnce() -> A; + + /// Construct a new [`Exn`] on the `None` variant, with type erasure. + fn ok_or_raise_something(self, err: F) -> Result + where + A: std::error::Error + Send + Sync + 'static, + F: FnOnce() -> A; } impl OptionExt for Option { @@ -54,6 +70,15 @@ impl OptionExt for Option { None => Err(Exn::new(err())), } } + + #[track_caller] + fn ok_or_raise_something(self, err: F) -> Result + where + A: std::error::Error + Send + Sync + 'static, + F: FnOnce() -> A, + { + self.ok_or_raise(err).map_err(Exn::something) + } } /// An extension trait for [`Result`] to provide context information on [`Exn`]s. @@ -71,6 +96,19 @@ pub trait ResultExt { where A: std::error::Error + Send + Sync + 'static, F: FnOnce() -> A; + + /// Raise something inside the [`Result`], providing no additional error context. + /// + /// Apply [`Exn::something`] on the `Err` variant, refer to it for more information. + fn or_something(self) -> Result; + + /// Raise a new exception on the [`Exn`] inside the [`Result`], and type-erase the result. + /// + /// Apply [`Exn::raise`] and [`Exn::something`] on the `Err` variant, refer to it for more information. + fn or_raise_something(self, err: F) -> Result + where + A: std::error::Error + Send + Sync + 'static, + F: FnOnce() -> A; } impl ResultExt for std::result::Result @@ -91,6 +129,23 @@ where Err(e) => Err(Exn::new(e).raise(err())), } } + + #[track_caller] + fn or_something(self) -> Result { + match self { + Ok(v) => Ok(v), + Err(e) => Err(Exn::new(e).something()), + } + } + + #[track_caller] + fn or_raise_something(self, err: F) -> Result + where + A: Error + Send + Sync + 'static, + F: FnOnce() -> A, + { + self.or_raise(err).map_err(Exn::something) + } } impl ResultExt for std::result::Result> @@ -111,4 +166,21 @@ where Err(e) => Err(e.raise(err())), } } + + #[track_caller] + fn or_something(self) -> Result { + match self { + Ok(v) => Ok(v), + Err(e) => Err(e.something()), + } + } + + #[track_caller] + fn or_raise_something(self, err: F) -> Result + where + A: Error + Send + Sync + 'static, + F: FnOnce() -> A, + { + self.or_raise(err).map_err(Exn::something) + } } diff --git a/gix-error/src/exn/impls.rs b/gix-error/src/exn/impls.rs index 82d3da37cb1..11e2a7bc1ef 100644 --- a/gix-error/src/exn/impls.rs +++ b/gix-error/src/exn/impls.rs @@ -13,15 +13,36 @@ // limitations under the License. use std::fmt; +use std::fmt::Formatter; use std::marker::PhantomData; +use std::ops::Deref; use std::panic::Location; -/// An exception type that can hold an error tree and additional context. +/// An error that merely says that something is wrong. +/// It's the default type for [Exn], indicating that this is about the error chain, +/// not about this specific error. +#[derive(Debug)] +pub struct Something; + +impl fmt::Display for Something { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + f.write_str("Something went wrong") + } +} + +impl std::error::Error for Something {} + +/// An exception type that can hold an [error tree](Exn::from_iter) and the call site. /// /// While an error chain, a list, is automatically created when [raise](Exn::raise) /// and friends are invoked, one can also use [`Exn::from_iter`] to create an error /// that has multiple causes. -pub struct Exn { +/// +/// # `Exn` == `Exn` +/// +/// `Exn` act's like `Box`, but with the capability +/// to store a tree of errors along with their *call sites*. +pub struct Exn { // trade one more indirection for less stack size frame: Box, phantom: PhantomData, @@ -42,31 +63,43 @@ impl Exn { /// /// See also [`ErrorExt::raise`](crate::ErrorExt) for a fluent way to convert an error into an `Exn` instance. /// + /// Note that **sources of `error` are degenerated to their string representation** and all type information is erased. + /// /// [source chain of the error]: std::error::Error::source #[track_caller] pub fn new(error: E) -> Self { - struct SourceError(String); + /// A way to keep all information of errors returned by `source()` chains. + struct SourceError { + display: String, + debug: String, + alt_debug: String, + } impl fmt::Debug for SourceError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Debug::fmt(&self.0, f) + let dbg = if f.alternate() { &self.alt_debug } else { &self.debug }; + f.write_str(dbg) } } impl fmt::Display for SourceError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Display::fmt(&self.0, f) + f.write_str(&self.display) } } impl std::error::Error for SourceError {} - fn walk(error: &dyn std::error::Error, location: &'static Location<'static>) -> Vec { + fn walk_sources(error: &dyn std::error::Error, location: &'static Location<'static>) -> Vec { if let Some(source) = error.source() { let children = vec![Frame { - error: Box::new(SourceError(source.to_string())), + error: Box::new(SourceError { + display: source.to_string(), + debug: format!("{:?}", source), + alt_debug: format!("{:#?}", source), + }), location, - children: walk(source, location), + children: walk_sources(source, location), }]; children } else { @@ -75,7 +108,7 @@ impl Exn { } let location = Location::caller(); - let children = walk(&error, location); + let children = walk_sources(&error, location); let frame = Frame { error: Box::new(error), location, @@ -89,6 +122,8 @@ impl Exn { } /// Create a new exception with the given error and children. + /// + /// It's no error if `children` is empty. #[track_caller] pub fn from_iter(children: I, err: E) -> Self where @@ -112,6 +147,19 @@ impl Exn { new_exn } + /// Use the current exception the head of a chain, adding `err` to its children. + #[track_caller] + pub fn chain(mut self, err: impl Into>) -> Exn { + let err = err.into(); + self.frame.children.push(*err.frame); + self + } + + /// Raise a new [`Something`] exception; this will make the current exception a child [`Something`]. + pub fn something(self) -> Exn { + self.raise(Something) + } + /// Return the current exception. pub fn as_error(&self) -> &E { self.frame @@ -130,12 +178,28 @@ impl Exn { } } + /// Turn ourselves into a type that implements [`std::error::Error`]. + pub fn into_error(self) -> crate::Error { + self.into() + } + /// Return the underlying exception frame. pub fn as_frame(&self) -> &Frame { &self.frame } } +impl Deref for Exn +where + E: std::error::Error + Send + Sync + 'static, +{ + type Target = E; + + fn deref(&self) -> &Self::Target { + self.as_error() + } +} + /// A frame in the exception tree. pub struct Frame { /// The error that occurred at this frame. @@ -148,7 +212,7 @@ pub struct Frame { impl Frame { /// Return the error as a reference to [`std::error::Error`]. - pub fn as_error(&self) -> &dyn std::error::Error { + pub fn as_error(&self) -> &(dyn std::error::Error + 'static) { &*self.error } @@ -162,3 +226,21 @@ impl Frame { &self.children } } + +impl From> for Box +where + E: std::error::Error + Send + Sync + 'static, +{ + fn from(err: Exn) -> Self { + err.frame + } +} + +impl From> for Frame +where + E: std::error::Error + Send + Sync + 'static, +{ + fn from(err: Exn) -> Self { + *err.frame + } +} diff --git a/gix-error/src/exn/macros.rs b/gix-error/src/exn/macros.rs index f21b7ddc0e7..ca61cd538c9 100644 --- a/gix-error/src/exn/macros.rs +++ b/gix-error/src/exn/macros.rs @@ -88,3 +88,15 @@ macro_rules! ensure { } }}; } + +/// Construct a [`Message`](crate::Message) from a string literal or format string. +#[macro_export] +macro_rules! message { + ($msg:literal $(,)?) => { + // Avoid allocations for string literals. + $crate::Message::new($msg) + }; + ($fmt:expr, $($arg:tt)*) => { + $crate::Message::new(format!($fmt, $($arg)*)) + }; +} diff --git a/gix-error/src/exn/mod.rs b/gix-error/src/exn/mod.rs index 8bf706e3ce8..b8c50b3a53b 100644 --- a/gix-error/src/exn/mod.rs +++ b/gix-error/src/exn/mod.rs @@ -23,5 +23,4 @@ mod impls; mod macros; pub use self::ext::{ErrorExt, OptionExt, ResultExt}; -pub use self::impls::Exn; -pub use self::impls::Frame; +pub use self::impls::{Exn, Frame, Something}; diff --git a/gix-error/src/lib.rs b/gix-error/src/lib.rs index afc7a3e2bde..f7ed07be7f7 100644 --- a/gix-error/src/lib.rs +++ b/gix-error/src/lib.rs @@ -12,19 +12,47 @@ /// A result type to hide the [Exn] error wrapper. mod exn; -pub use exn::{ErrorExt, Exn, OptionExt, ResultExt}; +pub use exn::{ErrorExt, Exn, Frame, OptionExt, ResultExt, Something}; /// An error type that wraps an inner type-erased boxed `std::error::Error` or an `Exn` frame. +/// +/// In that, it's similar to `anyhow`, but with support for tracking the call site and trees of errors. pub struct Error { - #[expect(dead_code)] - inner: Inner, + inner: error::Inner, } -#[expect(dead_code)] -enum Inner { - Boxed(Box), - Exn(Box), +mod error; + +mod message { + use std::borrow::Cow; + use std::fmt::{Debug, Display, Formatter}; + + /// An error that is further described in a message. + #[derive(Debug)] + pub struct Message { + /// The error message. + pub message: Cow<'static, str>, + } + + /// Lifecycle + impl Message { + /// Create a new instance that displays the given `message`. + pub fn new(message: impl Into>) -> Self { + Message { + message: message.into(), + } + } + } + + impl Display for Message { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.write_str(self.message.as_ref()) + } + } + + impl std::error::Error for Message {} } +pub use message::Message; mod parse { use bstr::BString; diff --git a/gix-error/tests/error/error.rs b/gix-error/tests/error/error.rs new file mode 100644 index 00000000000..d840f2e55fd --- /dev/null +++ b/gix-error/tests/error/error.rs @@ -0,0 +1,110 @@ +use crate::{debug_string, new_tree_error, Error as Simple, ErrorWithSource}; +use gix_error::{Error, ErrorExt}; +use std::error::Error as _; + +#[test] +fn from_exn_error() { + let err = Error::from(Simple("one").raise()); + assert_eq!(err.to_string(), "one"); + insta::assert_snapshot!(debug_string(&err), @"one, at gix-error/tests/error/error.rs:7:41"); + insta::assert_debug_snapshot!(err, @"one"); + assert_eq!(err.source().map(debug_string), None); +} + +#[test] +fn from_exn_error_tree() { + let err = Error::from(new_tree_error().raise(Simple("topmost"))); + assert_eq!(err.to_string(), "topmost"); + insta::assert_snapshot!(debug_string(&err), @r" + topmost, at gix-error/tests/error/error.rs:16:44 + | + └─ E6, at gix-error/tests/error/main.rs:25:9 + | + └─ E5, at gix-error/tests/error/main.rs:17:18 + | | + | └─ E3, at gix-error/tests/error/main.rs:9:21 + | | | + | | └─ E1, at gix-error/tests/error/main.rs:8:30 + | | + | └─ E10, at gix-error/tests/error/main.rs:12:22 + | | | + | | └─ E9, at gix-error/tests/error/main.rs:11:30 + | | + | └─ E12, at gix-error/tests/error/main.rs:15:23 + | | + | └─ E11, at gix-error/tests/error/main.rs:14:32 + | + └─ E4, at gix-error/tests/error/main.rs:20:21 + | | + | └─ E2, at gix-error/tests/error/main.rs:19:30 + | + └─ E8, at gix-error/tests/error/main.rs:23:21 + | + └─ E7, at gix-error/tests/error/main.rs:22:30 + "); + insta::assert_debug_snapshot!(err, @r" + topmost + | + └─ E6 + | + └─ E5 + | | + | └─ E3 + | | | + | | └─ E1 + | | + | └─ E10 + | | | + | | └─ E9 + | | + | └─ E12 + | | + | └─ E11 + | + └─ E4 + | | + | └─ E2 + | + └─ E8 + | + └─ E7 + "); + assert_eq!( + err.source().map(debug_string).as_deref(), + Some(r#"Error("E6")"#), + "The source is the first child" + ); +} + +#[test] +fn from_any_error() { + let err = Error::from_error(Simple("one")); + assert_eq!(err.to_string(), "one"); + assert_eq!(debug_string(&err), r#"Error("one")"#); + insta::assert_debug_snapshot!(err, @r#" + Error( + "one", + ) + "#); + assert_eq!(err.source().map(debug_string), None); +} + +#[test] +fn from_any_error_with_source() { + let err = Error::from_error(ErrorWithSource("main", Simple("one"))); + assert_eq!(err.to_string(), "main", "display is the error itself"); + assert_eq!(debug_string(&err), r#"ErrorWithSource("main", Error("one"))"#); + insta::assert_debug_snapshot!(err, @r#" + ErrorWithSource( + "main", + Error( + "one", + ), + ) + "#); + assert_eq!( + err.source().map(debug_string).as_deref(), + Some(r#"Error("one")"#), + "The source is provided by the wrapped error" + ); +} diff --git a/gix-error/tests/error/exn.rs b/gix-error/tests/error/exn.rs index 501d8693724..85364b667f3 100644 --- a/gix-error/tests/error/exn.rs +++ b/gix-error/tests/error/exn.rs @@ -12,22 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. +use crate::{debug_string, new_tree_error, Error}; use gix_error::ErrorExt; use gix_error::Exn; use gix_error::OptionExt; use gix_error::ResultExt; -#[derive(Debug)] -struct Error(&'static str); - -impl std::fmt::Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} - -impl std::error::Error for Error {} - #[test] fn error_call_chain() { let e1 = Error("E1").raise(); @@ -35,63 +25,82 @@ fn error_call_chain() { let e3 = e2.raise(Error("E3")); let e4 = e3.raise(Error("E4")); let e5 = e4.raise(Error("E5")); + insta::assert_debug_snapshot!(e5, @r" + E5 + | + └─ E4 + | + └─ E3 + | + └─ E2 + | + └─ E1 + "); insta::assert_snapshot!(debug_string(e5), @r" - E5, at gix-error/tests/error/exn.rs:37:17 + E5, at gix-error/tests/error/exn.rs:27:17 | - |-> E4, at gix-error/tests/error/exn.rs:36:17 + └─ E4, at gix-error/tests/error/exn.rs:26:17 | - |-> E3, at gix-error/tests/error/exn.rs:35:17 + └─ E3, at gix-error/tests/error/exn.rs:25:17 | - |-> E2, at gix-error/tests/error/exn.rs:34:17 + └─ E2, at gix-error/tests/error/exn.rs:24:17 | - |-> E1, at gix-error/tests/error/exn.rs:33:26 + └─ E1, at gix-error/tests/error/exn.rs:23:26 "); } #[test] fn error_tree() { - let e1 = Error("E1").raise(); - let e3 = e1.raise(Error("E3")); - - let e9 = Error("E9").raise(); - let e10 = e9.raise(Error("E10")); - - let e11 = Error("E11").raise(); - let e12 = e11.raise(Error("E12")); - - let e5 = Exn::from_iter([e3, e10, e12], Error("E5")); - - let e2 = Error("E2").raise(); - let e4 = e2.raise(Error("E4")); - - let e7 = Error("E7").raise(); - let e8 = e7.raise(Error("E8")); - - let e6 = Exn::from_iter([e5, e4, e8], Error("E6")); - insta::assert_snapshot!(debug_string(e6), @r" - E6, at gix-error/tests/error/exn.rs:70:14 + let err = new_tree_error(); + insta::assert_debug_snapshot!(err, @r" + E6 + | + └─ E5 + | | + | └─ E3 + | | | + | | └─ E1 + | | + | └─ E10 + | | | + | | └─ E9 + | | + | └─ E12 + | | + | └─ E11 | - |-> E5, at gix-error/tests/error/exn.rs:62:14 + └─ E4 | | - | |-> E3, at gix-error/tests/error/exn.rs:54:17 + | └─ E2 + | + └─ E8 + | + └─ E7 + "); + insta::assert_snapshot!(debug_string(err), @r" + E6, at gix-error/tests/error/main.rs:25:9 + | + └─ E5, at gix-error/tests/error/main.rs:17:18 + | | + | └─ E3, at gix-error/tests/error/main.rs:9:21 | | | - | | |-> E1, at gix-error/tests/error/exn.rs:53:26 + | | └─ E1, at gix-error/tests/error/main.rs:8:30 | | - | |-> E10, at gix-error/tests/error/exn.rs:57:18 + | └─ E10, at gix-error/tests/error/main.rs:12:22 | | | - | | |-> E9, at gix-error/tests/error/exn.rs:56:26 + | | └─ E9, at gix-error/tests/error/main.rs:11:30 | | - | |-> E12, at gix-error/tests/error/exn.rs:60:19 + | └─ E12, at gix-error/tests/error/main.rs:15:23 | | - | |-> E11, at gix-error/tests/error/exn.rs:59:28 + | └─ E11, at gix-error/tests/error/main.rs:14:32 | - |-> E4, at gix-error/tests/error/exn.rs:65:17 + └─ E4, at gix-error/tests/error/main.rs:20:21 | | - | |-> E2, at gix-error/tests/error/exn.rs:64:26 + | └─ E2, at gix-error/tests/error/main.rs:19:30 | - |-> E8, at gix-error/tests/error/exn.rs:68:17 + └─ E8, at gix-error/tests/error/main.rs:23:21 | - |-> E7, at gix-error/tests/error/exn.rs:67:26 + └─ E7, at gix-error/tests/error/main.rs:22:30 "); } @@ -100,9 +109,9 @@ fn result_ext() { let result: Result<(), Error> = Err(Error("An error")); let result = result.or_raise(|| Error("Another error")); insta::assert_snapshot!(debug_string(result.unwrap_err()), @r" - Another error, at gix-error/tests/error/exn.rs:101:25 + Another error, at gix-error/tests/error/exn.rs:110:25 | - |-> An error, at gix-error/tests/error/exn.rs:101:25 + └─ An error, at gix-error/tests/error/exn.rs:110:25 "); } @@ -110,7 +119,7 @@ fn result_ext() { fn option_ext() { let result: Option<()> = None; let result = result.ok_or_raise(|| Error("An error")); - insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:112:25"); + insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:121:25"); } #[test] @@ -121,7 +130,7 @@ fn from_error() { } let result = foo(); - insta::assert_snapshot!(debug_string(result.unwrap_err()),@"An error, at gix-error/tests/error/exn.rs:119:9"); + insta::assert_snapshot!(debug_string(result.unwrap_err()),@"An error, at gix-error/tests/error/exn.rs:128:9"); } #[test] @@ -131,7 +140,7 @@ fn bail() { } let result = foo(); - insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:130:9"); + insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:139:9"); } #[test] @@ -152,19 +161,10 @@ fn ensure_fail() { } let result = foo(); - insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:150:9"); + insta::assert_snapshot!(debug_string(result.unwrap_err()), @"An error, at gix-error/tests/error/exn.rs:159:9"); } #[test] fn result_ok() -> Result<(), Exn> { Ok(()) } - -fn debug_string(input: impl std::fmt::Debug) -> String { - if cfg!(windows) { - let out = format!("{input:?}"); - out.replace('\\', "/") - } else { - format!("{input:?}") - } -} diff --git a/gix-error/tests/error/main.rs b/gix-error/tests/error/main.rs index 972e73a3a3b..c98cddbf189 100644 --- a/gix-error/tests/error/main.rs +++ b/gix-error/tests/error/main.rs @@ -1 +1,63 @@ +mod error; mod exn; + +mod utils { + use gix_error::{ErrorExt, Exn}; + + pub fn new_tree_error() -> Exn { + let e1 = Error("E1").raise(); + let e3 = e1.raise(Error("E3")); + + let e9 = Error("E9").raise(); + let e10 = e9.raise(Error("E10")); + + let e11 = Error("E11").raise(); + let e12 = e11.raise(Error("E12")); + + let e5 = Exn::from_iter([e3, e10, e12], Error("E5")); + + let e2 = Error("E2").raise(); + let e4 = e2.raise(Error("E4")); + + let e7 = Error("E7").raise(); + let e8 = e7.raise(Error("E8")); + + Exn::from_iter([e5, e4, e8], Error("E6")) + } + + #[derive(Debug)] + pub struct Error(pub &'static str); + + impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } + } + + impl std::error::Error for Error {} + + pub fn debug_string(input: impl std::fmt::Debug) -> String { + if cfg!(windows) { + let out = format!("{input:?}"); + out.replace('\\', "/") + } else { + format!("{input:?}") + } + } + + #[derive(Debug)] + pub struct ErrorWithSource(pub &'static str, pub Error); + + impl std::fmt::Display for ErrorWithSource { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } + } + + impl std::error::Error for ErrorWithSource { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.1) + } + } +} +pub use utils::*; diff --git a/gix-refspec/Cargo.toml b/gix-refspec/Cargo.toml index 1481bd1f518..d6905409345 100644 --- a/gix-refspec/Cargo.toml +++ b/gix-refspec/Cargo.toml @@ -15,6 +15,7 @@ rust-version = "1.82" doctest = false [dependencies] +gix-error = { version = "0.0.0", path = "../gix-error" } gix-revision = { version = "^0.39.0", path = "../gix-revision", default-features = false } gix-validate = { version = "^0.10.1", path = "../gix-validate" } gix-hash = { version = "^0.21.2", path = "../gix-hash" } diff --git a/gix-refspec/src/parse.rs b/gix-refspec/src/parse.rs index 6844d2f7ecb..df5022b3725 100644 --- a/gix-refspec/src/parse.rs +++ b/gix-refspec/src/parse.rs @@ -25,7 +25,7 @@ pub enum Error { #[error(transparent)] ReferenceName(#[from] gix_validate::reference::name::Error), #[error(transparent)] - RevSpec(#[from] gix_revision::spec::parse::Error), + RevSpec(#[from] Box), } /// Define how the parsed refspec should be used. @@ -38,13 +38,13 @@ pub enum Operation { } pub(crate) mod function { - use bstr::{BStr, ByteSlice}; - use crate::{ parse::{Error, Operation}, types::Mode, RefSpecRef, }; + use bstr::{BStr, ByteSlice}; + use gix_error::Exn; /// Parse `spec` for use in `operation` and return it if it is valid. pub fn parse(mut spec: &BStr, operation: Operation) -> Result, Error> { @@ -181,7 +181,7 @@ pub(crate) mod function { .map_err(Error::from) .or_else(|err| { if allow_revspecs { - gix_revision::spec::parse(spec, &mut super::revparse::Noop)?; + gix_revision::spec::parse(spec, &mut super::revparse::Noop).map_err(Exn::into_box)?; Ok(spec) } else { Err(err) @@ -197,6 +197,7 @@ pub(crate) mod function { mod revparse { use bstr::BStr; + use gix_error::Exn; use gix_revision::spec::parse::delegate::{ Kind, Navigate, PeelTo, PrefixHint, ReflogLookup, Revision, SiblingBranch, Traversal, }; @@ -204,52 +205,54 @@ mod revparse { pub(crate) struct Noop; impl Revision for Noop { - fn find_ref(&mut self, _name: &BStr) -> Option<()> { - Some(()) + fn find_ref(&mut self, _name: &BStr) -> Result<(), Exn> { + Ok(()) } - fn disambiguate_prefix(&mut self, _prefix: gix_hash::Prefix, _hint: Option>) -> Option<()> { - Some(()) + fn disambiguate_prefix(&mut self, _prefix: gix_hash::Prefix, _hint: Option>) -> Result<(), Exn> { + Ok(()) } - fn reflog(&mut self, _query: ReflogLookup) -> Option<()> { - Some(()) + fn reflog(&mut self, _query: ReflogLookup) -> Result<(), Exn> { + Ok(()) } - fn nth_checked_out_branch(&mut self, _branch_no: usize) -> Option<()> { - Some(()) + fn nth_checked_out_branch(&mut self, _branch_no: usize) -> Result<(), Exn> { + Ok(()) } - fn sibling_branch(&mut self, _kind: SiblingBranch) -> Option<()> { - Some(()) + fn sibling_branch(&mut self, _kind: SiblingBranch) -> Result<(), Exn> { + Ok(()) } } impl Navigate for Noop { - fn traverse(&mut self, _kind: Traversal) -> Option<()> { - Some(()) + fn traverse(&mut self, _kind: Traversal) -> Result<(), Exn> { + Ok(()) } - fn peel_until(&mut self, _kind: PeelTo<'_>) -> Option<()> { - Some(()) + fn peel_until(&mut self, _kind: PeelTo<'_>) -> Result<(), Exn> { + Ok(()) } - fn find(&mut self, _regex: &BStr, _negated: bool) -> Option<()> { - Some(()) + fn find(&mut self, _regex: &BStr, _negated: bool) -> Result<(), Exn> { + Ok(()) } - fn index_lookup(&mut self, _path: &BStr, _stage: u8) -> Option<()> { - Some(()) + fn index_lookup(&mut self, _path: &BStr, _stage: u8) -> Result<(), Exn> { + Ok(()) } } impl Kind for Noop { - fn kind(&mut self, _kind: gix_revision::spec::Kind) -> Option<()> { - Some(()) + fn kind(&mut self, _kind: gix_revision::spec::Kind) -> Result<(), Exn> { + Ok(()) } } impl gix_revision::spec::parse::Delegate for Noop { - fn done(&mut self) {} + fn done(&mut self) -> Result<(), Exn> { + Ok(()) + } } } diff --git a/gix-revision/Cargo.toml b/gix-revision/Cargo.toml index 3495acdc63a..55ae624a912 100644 --- a/gix-revision/Cargo.toml +++ b/gix-revision/Cargo.toml @@ -38,11 +38,11 @@ gix-trace = { version = "^0.1.17", path = "../gix-trace", optional = true } bstr = { version = "1.12.0", default-features = false, features = ["std"] } bitflags = { version = "2", optional = true } -thiserror = "2.0.17" serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } document-features = { version = "0.2.1", optional = true } [dev-dependencies] +insta = "1.46.0" gix-odb = { path = "../gix-odb" } gix-testtools = { path = "../tests/tools" } permutohedron = "0.2.4" diff --git a/gix-revision/src/describe.rs b/gix-revision/src/describe.rs index cb92dec099f..ded95b7d254 100644 --- a/gix-revision/src/describe.rs +++ b/gix-revision/src/describe.rs @@ -127,24 +127,14 @@ impl Default for Options<'_> { } } -/// The error returned by the [`describe()`][function::describe()] function. -#[derive(Debug, thiserror::Error)] -#[allow(missing_docs)] -pub enum Error { - #[error("The parents of commit {} could not be added to graph during traversal", oid.to_hex())] - InsertParentsToGraph { - #[source] - err: crate::graph::insert_parents::Error, - oid: gix_hash::ObjectId, - }, - #[error("A commit could not be decoded during traversal")] - Decode(#[from] gix_object::decode::Error), -} +/// The error returned by the [`describe()`](function::describe()) function. +pub type Error = gix_error::Message; pub(crate) mod function { use std::{borrow::Cow, cmp::Ordering}; use bstr::BStr; + use gix_error::{message, Exn, ResultExt}; use gix_hash::oid; use super::{Error, Outcome}; @@ -154,7 +144,7 @@ pub(crate) mod function { }; /// Given a `commit` id, traverse the commit `graph` and collect candidate names from the `name_by_oid` mapping to produce - /// an `Outcome`, which converted [`into_format()`][Outcome::into_format()] will produce a typical `git describe` string. + /// an `Outcome`, which converted [`into_format()`](Outcome::into_format()) will produce a typical `git describe` string. /// /// Note that the `name_by_oid` map is returned in the [`Outcome`], which can be forcefully returned even if there was no matching /// candidate by setting `fallback_to_oid` to true. @@ -167,7 +157,7 @@ pub(crate) mod function { fallback_to_oid, first_parent, }: Options<'name>, - ) -> Result>, Error> { + ) -> Result>, Exn> { let _span = gix_trace::coarse!( "gix_revision::describe()", commit = %commit, @@ -309,7 +299,7 @@ pub(crate) mod function { commit: gix_hash::ObjectId, commit_flags: Flags, first_parent: bool, - ) -> Result<(), Error> { + ) -> Result<(), Exn> { graph .insert_parents( &commit, @@ -320,7 +310,7 @@ pub(crate) mod function { &mut |_parent_id, flags| *flags |= commit_flags, first_parent, ) - .map_err(|err| Error::InsertParentsToGraph { err, oid: commit })?; + .or_raise(|| message!("could not insert parents of commit {} into graph", commit.to_hex()))?; Ok(()) } @@ -329,7 +319,7 @@ pub(crate) mod function { graph: &mut Graph<'_, '_, Flags>, best_candidate: &mut Candidate<'_>, first_parent: bool, - ) -> Result { + ) -> Result> { let mut commits_seen = 0; while let Some(commit) = queue.pop_value() { commits_seen += 1; diff --git a/gix-revision/src/merge_base/function.rs b/gix-revision/src/merge_base/function.rs index b84805b869c..224ea815c1a 100644 --- a/gix-revision/src/merge_base/function.rs +++ b/gix-revision/src/merge_base/function.rs @@ -3,7 +3,7 @@ use std::cmp::Ordering; use gix_hash::ObjectId; use gix_revwalk::graph; -use super::Error; +use super::{Error, Simple}; use crate::{merge_base::Flags, Graph, PriorityQueue}; /// Given a commit at `first` id, traverse the commit `graph` and return all possible merge-base between it and `others`, @@ -63,13 +63,15 @@ fn remove_redundant( let commit = graph.get_mut(id).expect("previously added"); commit.data |= Flags::RESULT; for parent_id in commit.parents.clone() { - graph.get_or_insert_full_commit(parent_id, |parent| { - // prevent double-addition - if !parent.data.contains(Flags::STALE) { - parent.data |= Flags::STALE; - walk_start.push((parent_id, GenThenTime::from(&*parent))); - } - })?; + graph + .get_or_insert_full_commit(parent_id, |parent| { + // prevent double-addition + if !parent.data.contains(Flags::STALE) { + parent.data |= Flags::STALE; + walk_start.push((parent_id, GenThenTime::from(&*parent))); + } + }) + .map_err(|_| Simple("could not insert parent commit into graph"))?; } } walk_start.sort_by(|a, b| a.0.cmp(&b.0)); @@ -121,7 +123,8 @@ fn remove_redundant( parent.data |= Flags::STALE; stack.push((*parent_id, GenThenTime::from(&*parent))); } - })? + }) + .map_err(|_| Simple("could not insert parent commit into graph"))? .is_some() { break; @@ -151,16 +154,20 @@ fn paint_down_to_common( graph: &mut Graph<'_, '_, graph::Commit>, ) -> Result, Error> { let mut queue = PriorityQueue::::new(); - graph.get_or_insert_full_commit(first, |commit| { - commit.data |= Flags::COMMIT1; - queue.insert(GenThenTime::from(&*commit), first); - })?; + graph + .get_or_insert_full_commit(first, |commit| { + commit.data |= Flags::COMMIT1; + queue.insert(GenThenTime::from(&*commit), first); + }) + .map_err(|_| Simple("could not insert commit into graph"))?; for other in others { - graph.get_or_insert_full_commit(*other, |commit| { - commit.data |= Flags::COMMIT2; - queue.insert(GenThenTime::from(&*commit), *other); - })?; + graph + .get_or_insert_full_commit(*other, |commit| { + commit.data |= Flags::COMMIT2; + queue.insert(GenThenTime::from(&*commit), *other); + }) + .map_err(|_| Simple("could not insert commit into graph"))?; } let mut out = Vec::new(); @@ -180,12 +187,14 @@ fn paint_down_to_common( } for parent_id in commit.parents.clone() { - graph.get_or_insert_full_commit(parent_id, |parent| { - if (parent.data & flags_without_result) != flags_without_result { - parent.data |= flags_without_result; - queue.insert(GenThenTime::from(&*parent), parent_id); - } - })?; + graph + .get_or_insert_full_commit(parent_id, |parent| { + if (parent.data & flags_without_result) != flags_without_result { + parent.data |= flags_without_result; + queue.insert(GenThenTime::from(&*parent), parent_id); + } + }) + .map_err(|_| Simple("could not insert parent commit into graph"))?; } } diff --git a/gix-revision/src/merge_base/mod.rs b/gix-revision/src/merge_base/mod.rs index b07e39691d5..aeb899ed82c 100644 --- a/gix-revision/src/merge_base/mod.rs +++ b/gix-revision/src/merge_base/mod.rs @@ -15,13 +15,20 @@ bitflags::bitflags! { } /// The error returned by the [`merge_base()`][function::merge_base()] function. -#[derive(Debug, thiserror::Error)] -#[allow(missing_docs)] -pub enum Error { - #[error("A commit could not be inserted into the graph")] - InsertCommit(#[from] gix_revwalk::graph::get_or_insert_default::Error), +pub type Error = Simple; + +/// A simple error type for merge base operations. +#[derive(Debug)] +pub struct Simple(pub &'static str); + +impl std::fmt::Display for Simple { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.0) + } } +impl std::error::Error for Simple {} + pub(crate) mod function; mod octopus { diff --git a/gix-revision/src/spec/parse/delegate.rs b/gix-revision/src/spec/parse/delegate.rs index 3ff9039e991..870a3dc675f 100644 --- a/gix-revision/src/spec/parse/delegate.rs +++ b/gix-revision/src/spec/parse/delegate.rs @@ -1,4 +1,5 @@ use bstr::BStr; +use gix_error::Exn; /// Usually the first methods to call when parsing a rev-spec to set an anchoring revision (which is typically a `Commit` object). /// Methods can be called multiple time to either try input or to parse another rev-spec that is part of a range. @@ -11,22 +12,22 @@ pub trait Revision { /// Resolve `name` as reference which might not be a valid reference name. The name may be partial like `main` or full like /// `refs/heads/main` solely depending on the users input. /// Symbolic referenced should be followed till their object, but objects **must not yet** be peeled. - fn find_ref(&mut self, name: &BStr) -> Option<()>; + fn find_ref(&mut self, name: &BStr) -> Result<(), Exn>; /// An object prefix to disambiguate, returning `None` if it is ambiguous or wasn't found at all. /// /// If `hint` is set, it should be used to disambiguate multiple objects with the same prefix. - fn disambiguate_prefix(&mut self, prefix: gix_hash::Prefix, hint: Option>) -> Option<()>; + fn disambiguate_prefix(&mut self, prefix: gix_hash::Prefix, hint: Option>) -> Result<(), Exn>; /// Lookup the reflog of the previously set reference, or dereference `HEAD` to its reference /// to obtain the ref name (as opposed to `HEAD` itself). /// If there is no such reflog entry, return `None`. - fn reflog(&mut self, query: ReflogLookup) -> Option<()>; + fn reflog(&mut self, query: ReflogLookup) -> Result<(), Exn>; /// When looking at `HEAD`, `branch_no` is the non-null checkout in the path, e.g. `1` means the last branch checked out, /// `2` is the one before that. /// Return `None` if there is no branch as the checkout history (via the reflog) isn't long enough. - fn nth_checked_out_branch(&mut self, branch_no: usize) -> Option<()>; + fn nth_checked_out_branch(&mut self, branch_no: usize) -> Result<(), Exn>; /// Lookup the previously set branch or dereference `HEAD` to its reference to use its name to lookup the sibling branch of `kind` /// in the configuration (typically in `refs/remotes/…`). The sibling branches are always local tracking branches. @@ -34,7 +35,7 @@ pub trait Revision { /// of `refs/heads/`. /// Note that the caller isn't aware if the previously set reference is a branch or not and might call this method even though no reference /// is known. - fn sibling_branch(&mut self, kind: SiblingBranch) -> Option<()>; + fn sibling_branch(&mut self, kind: SiblingBranch) -> Result<(), Exn>; } /// Combine one or more specs into a range of multiple. @@ -48,16 +49,16 @@ pub trait Kind { /// and no second specification is provided. /// /// Note that the method can be called even if other invariants are not fulfilled, treat these as errors. - fn kind(&mut self, kind: crate::spec::Kind) -> Option<()>; + fn kind(&mut self, kind: crate::spec::Kind) -> Result<(), Exn>; } /// Once an anchor is set one can adjust it using traversal methods. pub trait Navigate { /// Adjust the current revision to traverse the graph according to `kind`. - fn traverse(&mut self, kind: Traversal) -> Option<()>; + fn traverse(&mut self, kind: Traversal) -> Result<(), Exn>; /// Peel the current object until it reached `kind` or `None` if the chain does not contain such object. - fn peel_until(&mut self, kind: PeelTo<'_>) -> Option<()>; + fn peel_until(&mut self, kind: PeelTo<'_>) -> Result<(), Exn>; /// Find the first revision/commit whose message matches the given `regex` (which is never empty). /// to see how it should be matched. @@ -65,7 +66,7 @@ pub trait Navigate { /// /// If no revision is known yet, find the _youngest_ matching commit from _any_ reference, including `HEAD`. /// Otherwise, only find commits reachable from the currently set revision. - fn find(&mut self, regex: &BStr, negated: bool) -> Option<()>; + fn find(&mut self, regex: &BStr, negated: bool) -> Result<(), Exn>; /// Look up the given `path` at the given `stage` in the index returning its blob id, /// or return `None` if it doesn't exist at this `stage`. @@ -74,7 +75,7 @@ pub trait Navigate { /// * `stage` ranges from 0 to 2, with 0 being the base, 1 being ours, 2 being theirs. /// * `path` without prefix is relative to the root of the repository, while prefixes like `./` and `../` make it /// relative to the current working directory. - fn index_lookup(&mut self, path: &BStr, stage: u8) -> Option<()>; + fn index_lookup(&mut self, path: &BStr, stage: u8) -> Result<(), Exn>; } /// A hint to make disambiguation when looking up prefixes possible. diff --git a/gix-revision/src/spec/parse/function.rs b/gix-revision/src/spec/parse/function.rs index 0e9b37657b6..5834881ca59 100644 --- a/gix-revision/src/spec/parse/function.rs +++ b/gix-revision/src/spec/parse/function.rs @@ -1,11 +1,11 @@ use std::{str::FromStr, time::SystemTime}; -use bstr::{BStr, BString, ByteSlice, ByteVec}; - use crate::{ spec, spec::parse::{delegate, delegate::SiblingBranch, Delegate, Error}, }; +use bstr::{BStr, BString, ByteSlice, ByteVec}; +use gix_error::{ErrorExt, Exn, ResultExt}; /// Parse a git [`revspec`](https://git-scm.com/docs/git-rev-parse#_specifying_revisions) and call `delegate` for each token /// successfully parsed. @@ -13,14 +13,16 @@ use crate::{ /// Note that the `delegate` is expected to maintain enough state to lookup revisions properly. /// Returns `Ok(())` if all of `input` was consumed, or the error if either the `revspec` syntax was incorrect or /// the `delegate` failed to perform the request. -pub fn parse(mut input: &BStr, delegate: &mut impl Delegate) -> Result<(), Error> { +pub fn parse(mut input: &BStr, delegate: &mut impl Delegate) -> Result<(), Exn> { use delegate::{Kind, Revision}; let mut delegate = InterceptRev::new(delegate); let mut prev_kind = None; if let Some(b'^') = input.first() { input = next(input).1; let kind = spec::Kind::ExcludeReachable; - delegate.kind(kind).ok_or(Error::Delegate)?; + delegate + .kind(kind) + .or_raise(|| Error::new(format!("delegate.kind({kind:?}) failed")))?; prev_kind = kind.into(); } @@ -33,38 +35,48 @@ pub fn parse(mut input: &BStr, delegate: &mut impl Delegate) -> Result<(), Error return if input.is_empty() { Ok(()) } else { - Err(Error::UnconsumedInput { input: input.into() }) + Err(Error::new_with_input("unconsumed input", input).raise()) }; } if let Some((rest, kind)) = try_range(input) { if let Some(prev_kind) = prev_kind { - return Err(Error::KindSetTwice { prev_kind, kind }); + return Err(Error::new(format!( + "cannot set spec kind more than once (was {prev_kind:?}, now {kind:?})" + )) + .raise()); } if !found_revision { - delegate.find_ref("HEAD".into()).ok_or(Error::Delegate)?; + delegate + .find_ref("HEAD".into()) + .or_raise(|| Error::new("delegate did not find the HEAD reference"))?; } - delegate.kind(kind).ok_or(Error::Delegate)?; + delegate + .kind(kind) + .or_raise(|| Error::new(format!("delegate.kind({kind:?}) failed")))?; (input, found_revision) = { let remainder = revision(rest.as_bstr(), &mut delegate)?; (remainder, remainder != rest) }; if !found_revision { - delegate.find_ref("HEAD".into()).ok_or(Error::Delegate)?; + delegate + .find_ref("HEAD".into()) + .or_raise(|| Error::new("delegate did not find the HEAD reference"))?; } } if input.is_empty() { - delegate.done(); - Ok(()) + delegate + .done() + .or_raise(|| Error::new("delegate.done() failed after all input was consumed")) } else { - Err(Error::UnconsumedInput { input: input.into() }) + Err(Error::new_with_input("unconsumed input", input).raise()) } } mod intercept { - use bstr::{BStr, BString}; - use crate::spec::parse::{delegate, Delegate}; + use bstr::{BStr, BString}; + use gix_error::Exn; #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] pub(crate) enum PrefixHintOwned { @@ -121,9 +133,9 @@ mod intercept { where T: Delegate, { - fn done(&mut self) { + fn done(&mut self) -> Result<(), Exn> { self.done = true; - self.inner.done(); + self.inner.done() } } @@ -131,7 +143,7 @@ mod intercept { where T: Delegate, { - fn find_ref(&mut self, name: &BStr) -> Option<()> { + fn find_ref(&mut self, name: &BStr) -> Result<(), Exn> { self.last_ref = name.to_owned().into(); self.inner.find_ref(name) } @@ -140,20 +152,20 @@ mod intercept { &mut self, prefix: gix_hash::Prefix, hint: Option>, - ) -> Option<()> { + ) -> Result<(), Exn> { self.last_prefix = Some((prefix, hint.map(Into::into))); self.inner.disambiguate_prefix(prefix, hint) } - fn reflog(&mut self, query: delegate::ReflogLookup) -> Option<()> { + fn reflog(&mut self, query: delegate::ReflogLookup) -> Result<(), Exn> { self.inner.reflog(query) } - fn nth_checked_out_branch(&mut self, branch_no: usize) -> Option<()> { + fn nth_checked_out_branch(&mut self, branch_no: usize) -> Result<(), Exn> { self.inner.nth_checked_out_branch(branch_no) } - fn sibling_branch(&mut self, kind: delegate::SiblingBranch) -> Option<()> { + fn sibling_branch(&mut self, kind: delegate::SiblingBranch) -> Result<(), Exn> { self.inner.sibling_branch(kind) } } @@ -162,19 +174,19 @@ mod intercept { where T: Delegate, { - fn traverse(&mut self, kind: delegate::Traversal) -> Option<()> { + fn traverse(&mut self, kind: delegate::Traversal) -> Result<(), Exn> { self.inner.traverse(kind) } - fn peel_until(&mut self, kind: delegate::PeelTo<'_>) -> Option<()> { + fn peel_until(&mut self, kind: delegate::PeelTo<'_>) -> Result<(), Exn> { self.inner.peel_until(kind) } - fn find(&mut self, regex: &BStr, negated: bool) -> Option<()> { + fn find(&mut self, regex: &BStr, negated: bool) -> Result<(), Exn> { self.inner.find(regex, negated) } - fn index_lookup(&mut self, path: &BStr, stage: u8) -> Option<()> { + fn index_lookup(&mut self, path: &BStr, stage: u8) -> Result<(), Exn> { self.inner.index_lookup(path, stage) } } @@ -183,7 +195,7 @@ mod intercept { where T: Delegate, { - fn kind(&mut self, kind: crate::spec::Kind) -> Option<()> { + fn kind(&mut self, kind: crate::spec::Kind) -> Result<(), Exn> { self.inner.kind(kind) } } @@ -193,7 +205,7 @@ use intercept::InterceptRev; fn try_set_prefix(delegate: &mut impl Delegate, hex_name: &BStr, hint: Option>) -> Option<()> { gix_hash::Prefix::from_hex(hex_name.to_str().expect("hexadecimal only")) .ok() - .and_then(|prefix| delegate.disambiguate_prefix(prefix, hint)) + .and_then(|prefix| delegate.disambiguate_prefix(prefix, hint).ok()) } fn long_describe_prefix(name: &BStr) -> Option<(&BStr, delegate::PrefixHint<'_>)> { @@ -296,7 +308,7 @@ fn parens(input: &[u8]) -> Result>, Error> { return Ok(Some((inner, input[idx + 1..].as_bstr(), idx + 1))); } } - Err(Error::UnclosedBracePair { input: input.into() }) + Err(Error::new_with_input("unclosed brace pair", input)) } fn try_parse(input: &BStr) -> Result, Error> { @@ -306,7 +318,10 @@ fn try_parse(input: &BStr) -> Result .and_then(|n| { n.parse().ok().map(|n| { if n == T::default() && input[0] == b'-' { - return Err(Error::NegativeZero { input: input.into() }); + return Err(Error::new_with_input( + "negative zero is invalid - remove the minus sign", + input, + )); } Ok(n) }) @@ -314,28 +329,50 @@ fn try_parse(input: &BStr) -> Result .transpose() } -fn revision<'a, T>(mut input: &'a BStr, delegate: &mut InterceptRev<'_, T>) -> Result<&'a BStr, Error> +fn revision<'a, T>(mut input: &'a BStr, delegate: &mut InterceptRev<'_, T>) -> Result<&'a BStr, Exn> where T: Delegate, { use delegate::{Navigate, Revision}; - fn consume_all(res: Option<()>) -> Result<&'static BStr, Error> { - res.ok_or(Error::Delegate).map(|_| "".into()) + fn consume_all(res: Result<(), Exn>, err: impl FnOnce() -> String) -> Result<&'static BStr, Exn> { + res.map(|_| "".into()).or_raise(|| Error::new(err())) } match input.as_bytes() { - [b':'] => return Err(Error::MissingColonSuffix), - [b':', b'/'] => return Err(Error::EmptyTopLevelRegex), + [b':'] => { + return Err( + Error::new("':' must be followed by either slash and regex or path to lookup in HEAD tree").raise(), + ) + } + [b':', b'/'] => return Err(Error::new("':/' must be followed by a regular expression").raise()), [b':', b'/', regex @ ..] => { let (regex, negated) = parse_regex_prefix(regex.as_bstr())?; if regex.is_empty() { - return Err(Error::UnconsumedInput { input: input.into() }); + return Err(Error::new_with_input("unconsumed input", input).raise()); } - return consume_all(delegate.find(regex, negated)); + return consume_all(delegate.find(regex, negated), || { + format!("Delegate couldn't find {regex} (negated: {negated})") + }); + } + [b':', b'0', b':', path @ ..] => { + return consume_all(delegate.index_lookup(path.as_bstr(), 0), || { + format!("Couldn't find index '{path}' stage 0", path = path.as_bstr()) + }) + } + [b':', b'1', b':', path @ ..] => { + return consume_all(delegate.index_lookup(path.as_bstr(), 1), || { + format!("Couldn't find index '{path}' stage 1", path = path.as_bstr()) + }) + } + [b':', b'2', b':', path @ ..] => { + return consume_all(delegate.index_lookup(path.as_bstr(), 2), || { + format!("Couldn't find index '{path}' stage 2", path = path.as_bstr()) + }) + } + [b':', path @ ..] => { + return consume_all(delegate.index_lookup(path.as_bstr(), 0), || { + format!("Couldn't find index '{path}' stage 0 (implicit)", path = path.as_bstr()) + }) } - [b':', b'0', b':', path @ ..] => return consume_all(delegate.index_lookup(path.as_bstr(), 0)), - [b':', b'1', b':', path @ ..] => return consume_all(delegate.index_lookup(path.as_bstr(), 1)), - [b':', b'2', b':', path @ ..] => return consume_all(delegate.index_lookup(path.as_bstr(), 2)), - [b':', path @ ..] => return consume_all(delegate.index_lookup(path.as_bstr(), 0)), _ => {} } @@ -382,7 +419,9 @@ where let mut sep = sep_pos.map(|pos| input[pos]); let mut has_ref_or_implied_name = name.is_empty(); if name.is_empty() && sep == Some(b'@') && sep_pos.and_then(|pos| input.get(pos + 1)) != Some(&b'{') { - delegate.find_ref("HEAD".into()).ok_or(Error::Delegate)?; + delegate + .find_ref("HEAD".into()) + .or_raise(|| Error::new("delegate did not find the HEAD reference"))?; sep_pos = sep_pos.map(|pos| pos + 1); sep = match sep_pos.and_then(|pos| input.get(pos).copied()) { None => return Ok("".into()), @@ -402,92 +441,100 @@ where name.is_empty().then_some(()).or_else(|| { #[allow(clippy::let_unit_value)] { - let res = delegate.find_ref(name)?; + let res = delegate.find_ref(name).ok()?; has_ref_or_implied_name = true; res.into() } }) }) - .ok_or(Error::Delegate)?; + .ok_or_else(|| Error::new_with_input("couldn't parse revision", input))?; } input = { if let Some(b'@') = sep { let past_sep = input[sep_pos.map_or(input.len(), |pos| pos + 1)..].as_bstr(); - let (nav, rest, _consumed) = parens(past_sep)?.ok_or_else(|| Error::AtNeedsCurlyBrackets { - input: input[sep_pos.unwrap_or(input.len())..].into(), + let (nav, rest, _consumed) = parens(past_sep)?.ok_or_else(|| { + Error::new_with_input( + "@ character must be standalone or followed by {}", + &input[sep_pos.unwrap_or(input.len())..], + ) })?; let nav = nav.as_ref(); if let Some(n) = try_parse::(nav)? { if n < 0 { if name.is_empty() { - delegate - .nth_checked_out_branch(n.unsigned_abs()) - .ok_or(Error::Delegate)?; + delegate.nth_checked_out_branch(n.unsigned_abs()).or_raise(|| { + Error::new_with_input( + format!("delegate.nth_checked_out_branch({n:?}) didn't find a branch"), + nav, + ) + })?; } else { - return Err(Error::RefnameNeedsPositiveReflogEntries { nav: nav.into() }); + return Err(Error::new_with_input( + "reference name must be followed by positive numbers in @{n}", + nav, + ) + .raise()); } } else if has_ref_or_implied_name { + let lookup = if n >= 100000000 { + let time = nav + .to_str() + .or_raise(|| Error::new_with_input("could not parse time for reflog lookup", nav)) + .and_then(|date| { + gix_date::parse(date, None) + .or_raise(|| Error::new_with_input("could not parse time for reflog lookup", nav)) + })?; + delegate::ReflogLookup::Date(time) + } else { + delegate::ReflogLookup::Entry(n.try_into().expect("non-negative isize fits usize")) + }; delegate - .reflog(if n >= 100000000 { - let time = nav - .to_str() - .map_err(|_| Error::Time { - input: nav.into(), - source: None, - }) - .and_then(|date| { - gix_date::parse(date, None).map_err(|err| Error::Time { - input: nav.into(), - source: Some(err.into_box()), - }) - })?; - delegate::ReflogLookup::Date(time) - } else { - delegate::ReflogLookup::Entry(n.try_into().expect("non-negative isize fits usize")) - }) - .ok_or(Error::Delegate)?; + .reflog(lookup) + .or_raise(|| Error::new_with_input(format!("delegate.reflog({lookup:?}) failed"), nav))?; } else { - return Err(Error::ReflogLookupNeedsRefName { name: (*name).into() }); + return Err(Error::new_with_input("reflog entries require a ref name", *name).raise()); } } else if let Some(kind) = SiblingBranch::parse(nav) { if has_ref_or_implied_name { - delegate.sibling_branch(kind).ok_or(Error::Delegate) + delegate + .sibling_branch(kind) + .or_raise(|| Error::new_with_input(format!("delegate.sibling_branch({kind:?}) failed"), nav)) } else { - Err(Error::SiblingBranchNeedsBranchName { name: (*name).into() }) + Err(Error::new_with_input( + "sibling branches like 'upstream' or 'push' require a branch name with remote configuration", + *name, + ) + .raise()) }?; } else if has_ref_or_implied_name { let time = nav .to_str() - .map_err(|_| Error::Time { - input: nav.into(), - source: None, - }) + .map_err(|_| Error::new_with_input("could not parse time for reflog lookup", nav)) .and_then(|date| { - gix_date::parse(date, Some(SystemTime::now())).map_err(|err| Error::Time { - input: nav.into(), - source: Some(err.into_box()), - }) + gix_date::parse(date, Some(SystemTime::now())) + .map_err(|_| Error::new_with_input("could not parse time for reflog lookup", nav)) })?; + let lookup = delegate::ReflogLookup::Date(time); delegate - .reflog(delegate::ReflogLookup::Date(time)) - .ok_or(Error::Delegate)?; + .reflog(lookup) + .or_raise(|| Error::new_with_input(format!("delegate.reflog({lookup:?}) failed"), nav))?; } else { - return Err(Error::ReflogLookupNeedsRefName { name: (*name).into() }); + return Err(Error::new_with_input("reflog entries require a ref name", *name).raise()); } rest } else { if sep_pos == Some(0) && sep == Some(b'~') { - return Err(Error::MissingTildeAnchor); + return Err(Error::new("tilde needs to follow an anchor, like @~").raise()); } input[sep_pos.unwrap_or(input.len())..].as_bstr() } }; - navigate(input, delegate) + Ok(navigate(input, delegate)?) } -fn navigate<'a, T>(input: &'a BStr, delegate: &mut InterceptRev<'_, T>) -> Result<&'a BStr, Error> +fn navigate<'a, T>(input: &'a BStr, delegate: &mut InterceptRev<'_, T>) -> Result<&'a BStr, Exn> where T: Delegate, { @@ -503,9 +550,10 @@ where .transpose()? .unwrap_or((1, 0)); if number != 0 { - delegate - .traverse(delegate::Traversal::NthAncestor(number)) - .ok_or(Error::Delegate)?; + let traversal = delegate::Traversal::NthAncestor(number); + delegate.traverse(traversal).or_raise(|| { + Error::new_with_input(format!("delegate.traverse({traversal:?}) failed"), input) + })?; } cursor += consumed; } @@ -516,34 +564,55 @@ where .transpose()? { if negative { - delegate - .traverse(delegate::Traversal::NthParent( - number - .checked_mul(-1) - .ok_or_else(|| Error::InvalidNumber { - input: past_sep.expect("present").into(), - })? - .try_into() - .expect("non-negative"), - )) - .ok_or(Error::Delegate)?; - delegate.kind(spec::Kind::RangeBetween).ok_or(Error::Delegate)?; + let traversal = delegate::Traversal::NthParent( + number + .checked_mul(-1) + .ok_or_else(|| { + Error::new_with_input("could not parse number", past_sep.expect("present")) + })? + .try_into() + .expect("non-negative"), + ); + delegate.traverse(traversal).or_raise(|| { + Error::new_with_input( + "delegate.traverse({traversal:?}) failed", + past_sep.unwrap_or_default(), + ) + })?; + let kind = spec::Kind::RangeBetween; + delegate.kind(kind).or_raise(|| { + Error::new_with_input( + format!("delegate.kind({kind:?}) failed"), + past_sep.unwrap_or_default(), + ) + })?; if let Some((prefix, hint)) = delegate.last_prefix.take() { - match hint { + match &hint { Some(hint) => delegate.disambiguate_prefix(prefix, hint.to_ref().into()), None => delegate.disambiguate_prefix(prefix, None), } - .ok_or(Error::Delegate)?; + .or_raise(|| { + Error::new_with_input( + format!("delegate.disambiguate_prefix({hint:?}) failed"), + past_sep.unwrap_or_default(), + ) + })?; } else if let Some(name) = delegate.last_ref.take() { - delegate.find_ref(name.as_bstr()).ok_or(Error::Delegate)?; + delegate.find_ref(name.as_bstr()).or_raise(|| { + Error::new_with_input( + format!("delegate.find_ref({name}) failed"), + past_sep.unwrap_or_default(), + ) + })?; } else { - return Err(Error::UnconsumedInput { - input: input[cursor..].into(), - }); + return Err(Error::new_with_input("unconsumed input", &input[cursor..]).raise()); } - delegate.done(); cursor += consumed; - return Ok(input[cursor..].as_bstr()); + let rest = input[cursor..].as_bstr(); + delegate + .done() + .or_raise(|| Error::new_with_input("navigation succeeded, delegate.done() failed", rest))?; + return Ok(rest); } else if number == 0 { delegate.peel_until(delegate::PeelTo::ObjectKind(gix_object::Kind::Commit)) } else { @@ -551,7 +620,7 @@ where number.try_into().expect("positive number"), )) } - .ok_or(Error::Delegate)?; + .or_raise(|| Error::new_with_input("unknown navigation", past_sep.unwrap_or_default()))?; cursor += consumed; } else if let Some((kind, _rest, consumed)) = past_sep.and_then(|past_sep| parens(past_sep).transpose()).transpose()? @@ -567,35 +636,55 @@ where regex if regex.starts_with(b"/") => { let (regex, negated) = parse_regex_prefix(regex[1..].as_bstr())?; if !regex.is_empty() { - delegate.find(regex, negated).ok_or(Error::Delegate)?; + delegate.find(regex, negated).or_raise(|| { + Error::new(format!("Delegate couldn't find {regex} (negated: {negated})")) + })?; } continue; } - invalid => return Err(Error::InvalidObject { input: invalid.into() }), + invalid => return Err(Error::new_with_input("cannot peel to unknown target", invalid).raise()), }; - delegate.peel_until(target).ok_or(Error::Delegate)?; + delegate.peel_until(target).or_raise(|| { + Error::new_with_input( + format!("delegate.peel_until({target:?}) failed"), + past_sep.unwrap_or_default(), + ) + })?; } else if past_sep.and_then(<[_]>::first) == Some(&b'!') { + let rest = input[cursor + 1..].as_bstr(); + let kind = spec::Kind::ExcludeReachableFromParents; + delegate + .kind(kind) + .or_raise(|| Error::new_with_input(format!("delegate.kind({kind:?}) failed"), rest))?; delegate - .kind(spec::Kind::ExcludeReachableFromParents) - .ok_or(Error::Delegate)?; - delegate.done(); - return Ok(input[cursor + 1..].as_bstr()); + .done() + .or_raise(|| Error::new_with_input("navigation succeeded, delegate.done() failed", rest))?; + return Ok(rest); } else if past_sep.and_then(<[_]>::first) == Some(&b'@') { + let rest = input[cursor + 1..].as_bstr(); + let kind = spec::Kind::IncludeReachableFromParents; delegate - .kind(spec::Kind::IncludeReachableFromParents) - .ok_or(Error::Delegate)?; - delegate.done(); - return Ok(input[cursor + 1..].as_bstr()); - } else { + .kind(kind) + .or_raise(|| Error::new_with_input(format!("delegate.kind({kind:?}) failed"), rest))?; delegate - .traverse(delegate::Traversal::NthParent(1)) - .ok_or(Error::Delegate)?; + .done() + .or_raise(|| Error::new_with_input("navigation succeeded, delegate.done() failed", rest))?; + return Ok(rest); + } else { + let parent = delegate::Traversal::NthParent(1); + delegate.traverse(parent).or_raise(|| { + Error::new_with_input( + format!("delegate.parent({parent:?}) failed",), + past_sep.unwrap_or_default(), + ) + })?; } } b':' => { + let to = delegate::PeelTo::Path(input[cursor..].as_bstr()); delegate - .peel_until(delegate::PeelTo::Path(input[cursor..].as_bstr())) - .ok_or(Error::Delegate)?; + .peel_until(to) + .or_raise(|| Error::new(format!("delegate.peel_until({to:?}) failed")))?; return Ok("".into()); } _ => return Ok(input[cursor - 1..].as_bstr()), @@ -608,7 +697,7 @@ fn parse_regex_prefix(regex: &BStr) -> Result<(&BStr, bool), Error> { Ok(match regex.strip_prefix(b"!") { Some(regex) if regex.first() == Some(&b'!') => (regex.as_bstr(), false), Some(regex) if regex.first() == Some(&b'-') => (regex[1..].as_bstr(), true), - Some(_regex) => return Err(Error::UnspecifiedRegexModifier { regex: regex.into() }), + Some(_regex) => return Err(Error::new_with_input("need one character after /!, typically -", regex)), None => (regex, false), }) } @@ -616,21 +705,27 @@ fn parse_regex_prefix(regex: &BStr) -> Result<(&BStr, bool), Error> { fn try_parse_usize(input: &BStr) -> Result, Error> { let mut bytes = input.iter().peekable(); if bytes.peek().filter(|&&&b| b == b'-' || b == b'+').is_some() { - return Err(Error::SignedNumber { input: input.into() }); + return Err(Error::new_with_input( + "negative or explicitly positive numbers are invalid here", + input, + )); } let num_digits = bytes.take_while(|b| b.is_ascii_digit()).count(); if num_digits == 0 { return Ok(None); } let input = &input[..num_digits]; - let number = try_parse(input)?.ok_or_else(|| Error::InvalidNumber { input: input.into() })?; + let number = try_parse(input)?.ok_or_else(|| Error::new_with_input("could not parse number", input))?; Ok(Some((number, num_digits))) } fn try_parse_isize(input: &BStr) -> Result, Error> { let mut bytes = input.iter().peekable(); if bytes.peek().filter(|&&&b| b == b'+').is_some() { - return Err(Error::SignedNumber { input: input.into() }); + return Err(Error::new_with_input( + "explicitly positive numbers are invalid here", + input, + )); } let negative = bytes.peek() == Some(&&b'-'); let num_digits = bytes.take_while(|b| b.is_ascii_digit() || *b == &b'-').count(); @@ -640,7 +735,7 @@ fn try_parse_isize(input: &BStr) -> Result, Error> return Ok(Some((-1, negative, num_digits))); } let input = &input[..num_digits]; - let number = try_parse(input)?.ok_or_else(|| Error::InvalidNumber { input: input.into() })?; + let number = try_parse(input)?.ok_or_else(|| Error::new_with_input("could not parse number", input))?; Ok(Some((number, negative, num_digits))) } diff --git a/gix-revision/src/spec/parse/mod.rs b/gix-revision/src/spec/parse/mod.rs index 41ddb8b2028..41a5ad0be2c 100644 --- a/gix-revision/src/spec/parse/mod.rs +++ b/gix-revision/src/spec/parse/mod.rs @@ -1,48 +1,6 @@ -use crate::spec; -use bstr::BString; - -/// The error returned by [`spec::parse()`][crate::spec::parse()]. -#[derive(Debug, thiserror::Error)] -#[allow(missing_docs)] -pub enum Error { - #[error("'~' needs to follow an anchor, like '@~'.")] - MissingTildeAnchor, - #[error("':' needs to be followed by either '/' and regex or the path to lookup in the HEAD tree.")] - MissingColonSuffix, - #[error("':/' must be followed by a regular expression.")] - EmptyTopLevelRegex, - #[error("Need one character after '/!', typically '-', but got {:?}", .regex)] - UnspecifiedRegexModifier { regex: BString }, - #[error("Cannot peel to {:?} - unknown target.", .input)] - InvalidObject { input: BString }, - #[error("Could not parse time {:?} for revlog lookup.", .input)] - Time { - input: BString, - source: Option>, - }, - #[error("Sibling branches like 'upstream' or 'push' require a branch name with remote configuration, got {:?}", .name)] - SiblingBranchNeedsBranchName { name: BString }, - #[error("Reflog entries require a ref name, got {:?}", .name)] - ReflogLookupNeedsRefName { name: BString }, - #[error("A reference name must be followed by positive numbers in '@{{n}}', got {:?}", .nav)] - RefnameNeedsPositiveReflogEntries { nav: BString }, - #[error("Negative or explicitly positive numbers are invalid here: {:?}", .input)] - SignedNumber { input: BString }, - #[error("Could not parse number from {input:?}")] - InvalidNumber { input: BString }, - #[error("Negative zeroes are invalid: {:?} - remove the '-'", .input)] - NegativeZero { input: BString }, - #[error("The opening brace in {:?} was not matched", .input)] - UnclosedBracePair { input: BString }, - #[error("Cannot set spec kind more than once. Previous value was {:?}, now it is {:?}", .prev_kind, .kind)] - KindSetTwice { prev_kind: spec::Kind, kind: spec::Kind }, - #[error("The @ character is either standing alone or followed by `{{}}`, got {:?}", .input)] - AtNeedsCurlyBrackets { input: BString }, - #[error("A portion of the input could not be parsed: {:?}", .input)] - UnconsumedInput { input: BString }, - #[error("The delegate didn't indicate success - check delegate for more information")] - Delegate, -} +use gix_error::Exn; +/// The error returned by [`spec::parse()`](crate::spec::parse()). +pub use gix_error::ParseError as Error; /// pub mod delegate; @@ -57,7 +15,7 @@ pub trait Delegate: delegate::Revision + delegate::Navigate + delegate::Kind { /// It can be used as a marker to finalize internal data structures. /// /// Note that it will not be called if there is unconsumed input. - fn done(&mut self); + fn done(&mut self) -> Result<(), Exn>; } pub(crate) mod function; diff --git a/gix-revision/tests/revision/describe/mod.rs b/gix-revision/tests/revision/describe/mod.rs index e541c2fde4e..86f1895b59c 100644 --- a/gix-revision/tests/revision/describe/mod.rs +++ b/gix-revision/tests/revision/describe/mod.rs @@ -1,10 +1,10 @@ -use std::{borrow::Cow, path::PathBuf}; - +use gix_error::Exn; use gix_object::bstr::ByteSlice; use gix_revision::{ describe, describe::{Error, Outcome}, }; +use std::{borrow::Cow, path::PathBuf}; use crate::hex_to_id; @@ -13,8 +13,11 @@ mod format; fn run_test( transform_odb: impl FnOnce(gix_odb::Handle) -> gix_odb::Handle, options: impl Fn(gix_hash::ObjectId) -> gix_revision::describe::Options<'static>, - run_assertions: impl Fn(Result>, Error>, gix_hash::ObjectId) -> crate::Result, -) -> crate::Result { + run_assertions: impl Fn( + Result>, Exn>, + gix_hash::ObjectId, + ) -> Result<(), gix_error::Error>, +) -> Result<(), gix_error::Error> { let store = odb_at("."); let store = transform_odb(store); let commit_id = hex_to_id("01ec18a3ebf2855708ad3c9d244306bc1fae3e9b"); @@ -32,7 +35,7 @@ fn run_test( } #[test] -fn option_none_if_no_tag_found() -> crate::Result { +fn option_none_if_no_tag_found() -> Result<(), gix_error::Error> { run_test( std::convert::identity, |_| Default::default(), @@ -44,7 +47,7 @@ fn option_none_if_no_tag_found() -> crate::Result { } #[test] -fn fallback_if_configured_in_options_but_no_candidate_or_names() -> crate::Result { +fn fallback_if_configured_in_options_but_no_candidate_or_names() -> Result<(), gix_error::Error> { run_test( std::convert::identity, |_| describe::Options { @@ -66,7 +69,7 @@ fn fallback_if_configured_in_options_but_no_candidate_or_names() -> crate::Resul } #[test] -fn fallback_if_configured_in_options_and_max_candidates_zero() -> crate::Result { +fn fallback_if_configured_in_options_and_max_candidates_zero() -> Result<(), gix_error::Error> { run_test( std::convert::identity, |_| describe::Options { @@ -86,7 +89,7 @@ fn fallback_if_configured_in_options_and_max_candidates_zero() -> crate::Result } #[test] -fn not_enough_candidates() -> crate::Result { +fn not_enough_candidates() -> Result<(), gix_error::Error> { let name = Cow::Borrowed(b"at-c5".as_bstr()); run_test( std::convert::identity, @@ -118,7 +121,7 @@ fn not_enough_candidates() -> crate::Result { } #[test] -fn typical_usecases() -> crate::Result { +fn typical_usecases() -> Result<(), gix_error::Error> { let name = Cow::Borrowed(b"main".as_bstr()); run_test( std::convert::identity, @@ -198,7 +201,7 @@ fn typical_usecases() -> crate::Result { } #[test] -fn shallow_yields_no_result_if_provided_refs_are_in_truncated_part_of_history() -> crate::Result { +fn shallow_yields_no_result_if_provided_refs_are_in_truncated_part_of_history() -> Result<(), gix_error::Error> { run_test( |_| odb_at("shallow-1-clone"), |_| describe::Options { @@ -223,7 +226,7 @@ fn shallow_yields_no_result_if_provided_refs_are_in_truncated_part_of_history() } #[test] -fn shallow_yields_result_if_refs_are_available() -> crate::Result { +fn shallow_yields_result_if_refs_are_available() -> Result<(), gix_error::Error> { let name = Cow::Borrowed(b"at-c5".as_bstr()); run_test( |_| odb_at("shallow-2-clone"), diff --git a/gix-revision/tests/revision/spec/parse/anchor/at_symbol.rs b/gix-revision/tests/revision/spec/parse/anchor/at_symbol.rs index dae8a7a100c..f801ea6a7ec 100644 --- a/gix-revision/tests/revision/spec/parse/anchor/at_symbol.rs +++ b/gix-revision/tests/revision/spec/parse/anchor/at_symbol.rs @@ -1,12 +1,13 @@ -use gix_revision::spec; - use crate::spec::parse::{parse, try_parse}; #[test] fn braces_must_be_closed() { for unclosed_spec in ["@{something", "@{", "@{..@"] { let err = try_parse(unclosed_spec).unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnclosedBracePair {input} if input == unclosed_spec[1..])); + assert_eq!( + err.input.as_ref().map(|i| i.as_ref()), + Some(unclosed_spec[1..].as_bytes()) + ); } } @@ -75,7 +76,7 @@ fn reflog_by_unix_timestamp_for_current_branch() { #[test] fn reflog_by_date_with_date_parse_failure() { let err = try_parse("@{foo}").unwrap_err(); - assert!(matches!(err, spec::parse::Error::Time {input, source} if input == "foo" && source.is_some())); + insta::assert_snapshot!(err, @"could not parse time for reflog lookup: foo"); } #[test] @@ -86,7 +87,8 @@ fn reflog_by_date_for_hash_is_invalid() { ("v1.2.3-0-g1234@{42 +0030}", "v1.2.3-0-g1234"), ] { let err = try_parse(spec).unwrap_err(); - assert!(matches!(err, spec::parse::Error::ReflogLookupNeedsRefName {name} if name == full_name)); + assert_eq!(err.input.as_ref().map(|i| i.as_ref()), Some(full_name.as_bytes())); + assert!(err.message.contains("reflog entries require a ref name")); } } @@ -132,7 +134,8 @@ fn reflog_by_entry_for_hash_is_invalid() { ("v1.2.3-0-g1234@{2}", "v1.2.3-0-g1234"), ] { let err = try_parse(spec).unwrap_err(); - assert!(matches!(err, spec::parse::Error::ReflogLookupNeedsRefName {name} if name == full_name)); + assert_eq!(err.input.as_ref().map(|i| i.as_ref()), Some(full_name.as_bytes())); + assert!(err.message.contains("reflog entries require a ref name")); } } @@ -178,17 +181,16 @@ fn sibling_branch_for_hash_is_invalid() { ("v1.2.3-0-g1234@{upstream}", "v1.2.3-0-g1234"), ] { let err = try_parse(spec).unwrap_err(); - assert!(matches!(err, spec::parse::Error::SiblingBranchNeedsBranchName {name} if name == full_name)); + assert_eq!(err.input.as_ref().map(|i| i.as_ref()), Some(full_name.as_bytes())); + assert!(err.message.contains("sibling branches")); } } #[test] fn nth_checked_out_branch_for_refname_is_invalid() { let err = try_parse("r1@{-1}").unwrap_err(); - assert!( - matches!(err, spec::parse::Error::RefnameNeedsPositiveReflogEntries {nav} if nav == "-1"), - "its undefined how to handle negative numbers and specified ref names" - ); + // its undefined how to handle negative numbers and specified ref names + insta::assert_snapshot!(err, @"reference name must be followed by positive numbers in @{n}: -1"); } #[test] @@ -210,10 +212,8 @@ fn nth_checked_out_branch() { #[test] fn numbers_within_braces_cannot_be_negative_zero() { let err = try_parse("@{-0}").unwrap_err(); - assert!( - matches!(err, spec::parse::Error::NegativeZero {input} if input == "-0"), - "negative zero is not accepted, even though it could easily be defaulted to 0 which is a valid value" - ); + // negative zero is not accepted, even though it could easily be defaulted to 0 which is a valid value + insta::assert_snapshot!(err, @"negative zero is invalid - remove the minus sign: -0"); } #[test] diff --git a/gix-revision/tests/revision/spec/parse/anchor/colon_symbol.rs b/gix-revision/tests/revision/spec/parse/anchor/colon_symbol.rs index a4fd29ef673..2a9cc1014b0 100644 --- a/gix-revision/tests/revision/spec/parse/anchor/colon_symbol.rs +++ b/gix-revision/tests/revision/spec/parse/anchor/colon_symbol.rs @@ -1,5 +1,3 @@ -use gix_revision::spec; - use crate::spec::parse::{parse, try_parse}; #[test] @@ -104,25 +102,22 @@ fn various_valid_index_lookups_by_path_and_stage() { #[test] fn empty_top_level_regex_are_invalid() { let err = try_parse(":/").unwrap_err(); - assert!( - matches!(err, spec::parse::Error::EmptyTopLevelRegex), - "git also can't do it, finds nothing instead. It could be the youngest commit in theory, but isn't" - ); + // git also can't do it, finds nothing instead. It could be the youngest commit in theory, but isn't + insta::assert_snapshot!(err, @"':/' must be followed by a regular expression"); } #[test] fn regex_with_empty_exclamation_mark_prefix_is_invalid() { let err = try_parse(r#":/!hello"#).unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnspecifiedRegexModifier {regex} if regex == "!hello")); + assert_eq!(err.input.as_ref().map(|i| i.as_ref()), Some(b"!hello".as_ref())); + insta::assert_snapshot!(err, @"need one character after /!, typically -: !hello"); } #[test] fn needs_suffix() { let err = try_parse(":").unwrap_err(); - assert!( - matches!(err, spec::parse::Error::MissingColonSuffix), - "git also can't do it, finds nothing instead. It could be the youngest commit in theory, but isn't" - ); + // git also can't do it, finds nothing instead. It could be the youngest commit in theory, but isn't + insta::assert_snapshot!(err, @"':' must be followed by either slash and regex or path to lookup in HEAD tree"); } #[test] diff --git a/gix-revision/tests/revision/spec/parse/anchor/refnames.rs b/gix-revision/tests/revision/spec/parse/anchor/refnames.rs index 981a3bdb82b..4fd52c0e9ce 100644 --- a/gix-revision/tests/revision/spec/parse/anchor/refnames.rs +++ b/gix-revision/tests/revision/spec/parse/anchor/refnames.rs @@ -35,10 +35,7 @@ fn at_in_ranges_is_allowed() { #[test] fn strange_revspecs_do_not_panic() { let err = try_parse(".@.").unwrap_err(); - assert!(matches!( - err, - gix_revision::spec::parse::Error::AtNeedsCurlyBrackets { input } if input == "@." - )); + insta::assert_snapshot!(err, @"@ character must be standalone or followed by {}: @."); } #[test] diff --git a/gix-revision/tests/revision/spec/parse/kind.rs b/gix-revision/tests/revision/spec/parse/kind.rs index 78315c594a9..c50668d9cc2 100644 --- a/gix-revision/tests/revision/spec/parse/kind.rs +++ b/gix-revision/tests/revision/spec/parse/kind.rs @@ -1,12 +1,10 @@ -use gix_revision::spec; - use crate::spec::parse::{try_parse, try_parse_opts, Options}; #[test] fn cannot_declare_ranges_multiple_times() { for invalid_spec in ["^HEAD..", "^HEAD..."] { - let err = try_parse(invalid_spec).unwrap_err(); - assert!(matches!(err, spec::parse::Error::KindSetTwice { .. })); + let err = try_parse(invalid_spec).unwrap_err().into_box(); + assert!(err.message.contains("cannot set spec kind more than once")); } } @@ -19,11 +17,10 @@ fn delegate_can_refuse_spec_kinds() { ..Default::default() }, ) - .unwrap_err(); - assert!( - matches!(err, spec::parse::Error::Delegate), - "Delegates can refuse spec kind changes to abort parsing early in case they want single-specs only" - ); + .unwrap_err() + .into_box(); + // Delegates can refuse spec kind changes to abort parsing early in case they want single-specs only + insta::assert_snapshot!(err, @"delegate.kind(ExcludeReachable) failed"); } mod include_parents { @@ -56,8 +53,8 @@ mod include_parents { #[test] fn trailing_caret_exclamation_mark_must_end_the_input() { - let err = try_parse("r1^@~1").unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnconsumedInput { .. })); + let err = try_parse("r1^@~1").unwrap_err().into_box(); + assert!(err.message.contains("unconsumed input")); } } @@ -101,8 +98,8 @@ mod exclude_parents { #[test] fn trailing_caret_exclamation_mark_must_end_the_input() { - let err = try_parse("r1^!~1").unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnconsumedInput { .. })); + let err = try_parse("r1^!~1").unwrap_err().into_box(); + assert!(err.message.contains("unconsumed input")); } } @@ -230,26 +227,26 @@ mod range { #[test] fn minus_with_n_omitted_has_to_end_there() { - let err = try_parse("r1^-^").unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnconsumedInput { .. })); + let err = try_parse("r1^-^").unwrap_err().into_box(); + assert!(err.message.contains("unconsumed input")); } #[test] fn minus_with_n_has_to_end_there() { - let err = try_parse("r1^-42^").unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnconsumedInput { .. })); + let err = try_parse("r1^-42^").unwrap_err().into_box(); + assert!(err.message.contains("unconsumed input")); } #[test] fn minus_with_n_has_to_end_there_and_handle_range_suffix() { - let err = try_parse("r1^-42..").unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnconsumedInput { .. })); + let err = try_parse("r1^-42..").unwrap_err().into_box(); + assert!(err.message.contains("unconsumed input")); } #[test] fn minus_with_n_omitted_has_to_end_there_and_handle_range_suffix() { - let err = try_parse("r1^-..").unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnconsumedInput { .. })); + let err = try_parse("r1^-..").unwrap_err().into_box(); + assert!(err.message.contains("unconsumed input")); } #[test] diff --git a/gix-revision/tests/revision/spec/parse/mod.rs b/gix-revision/tests/revision/spec/parse/mod.rs index db0c352c345..56784bb7b8c 100644 --- a/gix-revision/tests/revision/spec/parse/mod.rs +++ b/gix-revision/tests/revision/spec/parse/mod.rs @@ -1,3 +1,4 @@ +use gix_error::{bail, Exn, Something}; use gix_object::bstr::{BStr, BString}; use gix_revision::{ spec, @@ -82,26 +83,30 @@ impl Recorder { } } -fn set_val(fn_name: &str, store: &mut [Option; 2], val: T) -> Option<()> { +fn set_val(fn_name: &str, store: &mut [Option; 2], val: T) -> Result<(), Exn> { for entry in store.iter_mut() { if entry.is_none() { *entry = Some(val); - return Some(()); + return Ok(()); } } panic!("called {fn_name}() more than twice with '{val:?}'"); } impl delegate::Revision for Recorder { - fn find_ref(&mut self, input: &BStr) -> Option<()> { + fn find_ref(&mut self, input: &BStr) -> Result<(), Exn> { self.called(Call::FindRef); set_val("find_ref", &mut self.find_ref, input.into()) } - fn disambiguate_prefix(&mut self, input: gix_hash::Prefix, hint: Option>) -> Option<()> { + fn disambiguate_prefix( + &mut self, + input: gix_hash::Prefix, + hint: Option>, + ) -> Result<(), Exn> { self.called(Call::DisambiguatePrefix); if self.opts.reject_prefix { - return None; + bail!(Something); } set_val("disambiguate_prefix", &mut self.prefix, input)?; if let Some(hint) = hint { @@ -117,10 +122,10 @@ impl delegate::Revision for Recorder { }, )?; } - Some(()) + Ok(()) } - fn reflog(&mut self, entry: delegate::ReflogLookup) -> Option<()> { + fn reflog(&mut self, entry: delegate::ReflogLookup) -> Result<(), Exn> { self.called(Call::Reflog); set_val( "current_branch_reflog", @@ -136,26 +141,26 @@ impl delegate::Revision for Recorder { ) } - fn nth_checked_out_branch(&mut self, branch: usize) -> Option<()> { + fn nth_checked_out_branch(&mut self, branch: usize) -> Result<(), Exn> { assert_ne!(branch, 0); self.called(Call::NthCheckedOutBranch); set_val("nth_checked_out_branch", &mut self.nth_checked_out_branch, branch) } - fn sibling_branch(&mut self, kind: delegate::SiblingBranch) -> Option<()> { + fn sibling_branch(&mut self, kind: delegate::SiblingBranch) -> Result<(), Exn> { self.called(Call::SiblingBranch); set_val("sibling_branch", &mut self.sibling_branch, format!("{kind:?}")) } } impl delegate::Navigate for Recorder { - fn traverse(&mut self, kind: delegate::Traversal) -> Option<()> { + fn traverse(&mut self, kind: delegate::Traversal) -> Result<(), Exn> { self.called(Call::Traverse); self.traversal.push(kind); - Some(()) + Ok(()) } - fn peel_until(&mut self, kind: delegate::PeelTo) -> Option<()> { + fn peel_until(&mut self, kind: delegate::PeelTo) -> Result<(), Exn> { self.called(Call::PeelUntil); self.peel_to.push(match kind { delegate::PeelTo::ObjectKind(kind) => PeelToOwned::ObjectKind(kind), @@ -163,40 +168,41 @@ impl delegate::Navigate for Recorder { delegate::PeelTo::Path(path) => PeelToOwned::Path(path.into()), delegate::PeelTo::RecursiveTagObject => PeelToOwned::RecursiveTagObject, }); - Some(()) + Ok(()) } - fn find(&mut self, regex: &BStr, negated: bool) -> Option<()> { + fn find(&mut self, regex: &BStr, negated: bool) -> Result<(), Exn> { self.called(Call::Find); self.patterns.push((regex.into(), negated)); - Some(()) + Ok(()) } - fn index_lookup(&mut self, path: &BStr, stage: u8) -> Option<()> { + fn index_lookup(&mut self, path: &BStr, stage: u8) -> Result<(), Exn> { self.called(Call::IndexLookup); self.index_lookups.push((path.into(), stage)); - Some(()) + Ok(()) } } impl delegate::Kind for Recorder { - fn kind(&mut self, kind: spec::Kind) -> Option<()> { + fn kind(&mut self, kind: spec::Kind) -> Result<(), Exn> { self.called(Call::Kind); if self.opts.reject_kind { - return None; + bail!(Something); } if self.kind.is_none() { self.kind = Some(kind); } else if !self.opts.no_internal_assertions { panic!("called kind more than once with '{kind:?}'"); } - Some(()) + Ok(()) } } impl Delegate for Recorder { - fn done(&mut self) { + fn done(&mut self) -> Result<(), Exn> { self.done = true; + Ok(()) } } @@ -204,11 +210,11 @@ fn parse(spec: &str) -> Recorder { try_parse_opts(spec, Options::default()).unwrap() } -fn try_parse(spec: &str) -> Result { +fn try_parse(spec: &str) -> Result> { try_parse_opts(spec, Default::default()) } -fn try_parse_opts(spec: &str, options: Options) -> Result { +fn try_parse_opts(spec: &str, options: Options) -> Result> { let mut rec = Recorder::with(options); spec::parse(spec.into(), &mut rec)?; Ok(rec) diff --git a/gix-revision/tests/revision/spec/parse/navigate/caret_symbol.rs b/gix-revision/tests/revision/spec/parse/navigate/caret_symbol.rs index 245f9411604..bfed6eb3ad3 100644 --- a/gix-revision/tests/revision/spec/parse/navigate/caret_symbol.rs +++ b/gix-revision/tests/revision/spec/parse/navigate/caret_symbol.rs @@ -1,4 +1,4 @@ -use gix_revision::{spec, spec::parse::delegate::Traversal}; +use gix_revision::spec::parse::delegate::Traversal; use crate::spec::parse::{parse, try_parse, PeelToOwned as PeelTo}; @@ -57,8 +57,9 @@ fn followed_by_zero_is_peeling_to_commit() { #[test] fn explicitly_positive_numbers_are_invalid() { - let err = try_parse("@^+1").unwrap_err(); - assert!(matches!(err, spec::parse::Error::SignedNumber {input} if input == "+1")); + let err = try_parse("@^+1").unwrap_err().into_box(); + assert_eq!(err.input.as_ref().map(|i| i.as_ref()), Some(b"+1".as_ref())); + assert!(err.message.contains("positive numbers are invalid")); } #[test] @@ -180,12 +181,13 @@ fn empty_braces_deref_a_tag() { #[test] fn invalid_object_type() { - let err = try_parse("@^{invalid}").unwrap_err(); - assert!(matches!(err, spec::parse::Error::InvalidObject {input} if input == "invalid")); + let err = try_parse("@^{invalid}").unwrap_err().into_box(); + assert_eq!(err.input.as_ref().map(|i| i.as_ref()), Some(b"invalid".as_ref())); + assert!(err.message.contains("cannot peel")); - let err = try_parse("@^{Commit}").unwrap_err(); + let err = try_parse("@^{Commit}").unwrap_err().into_box(); assert!( - matches!(err, spec::parse::Error::InvalidObject {input} if input == "Commit"), + err.input.as_ref().map(|i| i.as_ref()) == Some(b"Commit".as_ref()) && err.message.contains("cannot peel"), "these types are case sensitive" ); } @@ -202,33 +204,41 @@ fn invalid_caret_without_previous_refname() { ); for revspec in ["^^^HEAD", "^^HEAD"] { - let err = try_parse(revspec).unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnconsumedInput {input} if input == "HEAD")); + let err = try_parse(revspec).unwrap_err().into_box(); + assert_eq!(err.input.as_ref().map(|i| i.as_ref()), Some(b"HEAD".as_ref())); + assert!(err.message.contains("unconsumed input")); } } #[test] fn incomplete_escaped_braces_in_regex_are_invalid() { - let err = try_parse(r"@^{/a\{1}}").unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnconsumedInput {input} if input == "}")); + let err = try_parse(r"@^{/a\{1}}").unwrap_err().into_box(); + assert_eq!(err.input.as_ref().map(|i| i.as_ref()), Some(b"}".as_ref())); + assert!(err.message.contains("unconsumed input")); - let err = try_parse(r"@^{/a{1\}}").unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnclosedBracePair {input} if input == r"{/a{1\}}")); + let err = try_parse(r"@^{/a{1\}}").unwrap_err().into_box(); + assert!( + err.input.as_ref().map(|i| i.as_ref()) == Some(br"{/a{1\}}".as_ref()) && err.message.contains("unclosed brace") + ); } #[test] fn regex_with_empty_exclamation_mark_prefix_is_invalid() { - let err = try_parse(r#"@^{/!hello}"#).unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnspecifiedRegexModifier {regex} if regex == "!hello")); + let err = try_parse(r#"@^{/!hello}"#).unwrap_err().into_box(); + assert_eq!(err.input.as_ref().map(|i| i.as_ref()), Some(b"!hello".as_ref())); + assert!(err.message.contains("need one character after")); } #[test] fn bad_escapes_can_cause_brace_mismatch() { - let err = try_parse(r"@^{\}").unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnclosedBracePair {input} if input == r"{\}")); + let err = try_parse(r"@^{\}").unwrap_err().into_box(); + assert!(err.input.as_ref().map(|i| i.as_ref()) == Some(br"{\}".as_ref()) && err.message.contains("unclosed brace")); - let err = try_parse(r"@^{{\}}").unwrap_err(); - assert!(matches!(err, spec::parse::Error::UnclosedBracePair {input} if input == r"{{\}}")); + let err = try_parse(r"@^{{\}}").unwrap_err().into_box(); + // The raw string r"{{\}}" contains actual backslashes, so the input would be r"{{\}}" + assert!( + err.input.as_ref().map(|i| i.as_ref()) == Some(br"{{\}}".as_ref()) && err.message.contains("unclosed brace") + ); } #[test] diff --git a/gix-revision/tests/revision/spec/parse/navigate/tilde_symbol.rs b/gix-revision/tests/revision/spec/parse/navigate/tilde_symbol.rs index ae1ba826cdc..012a64f5397 100644 --- a/gix-revision/tests/revision/spec/parse/navigate/tilde_symbol.rs +++ b/gix-revision/tests/revision/spec/parse/navigate/tilde_symbol.rs @@ -1,11 +1,11 @@ -use gix_revision::{spec, spec::parse::delegate::Traversal}; +use gix_revision::spec::parse::delegate::Traversal; use crate::spec::parse::{parse, try_parse}; #[test] fn without_anchor_is_invalid() { - let err = try_parse("~").unwrap_err(); - assert!(matches!(err, spec::parse::Error::MissingTildeAnchor)); + let err = try_parse("~").unwrap_err().into_box(); + assert!(err.message.contains("tilde needs to follow an anchor")); } #[test] diff --git a/gix/src/commit.rs b/gix/src/commit.rs index f12c5f88631..8ffaef8ef2b 100644 --- a/gix/src/commit.rs +++ b/gix/src/commit.rs @@ -33,10 +33,10 @@ impl From for Error { /// #[cfg(feature = "revision")] pub mod describe { - use std::borrow::Cow; - + use gix_error::Exn; use gix_hash::ObjectId; use gix_hashtable::HashMap; + use std::borrow::Cow; use crate::{bstr::BStr, ext::ObjectIdExt, Repository}; @@ -84,7 +84,7 @@ pub mod describe { #[error(transparent)] OpenCache(#[from] crate::repository::commit_graph_if_enabled::Error), #[error(transparent)] - Describe(#[from] gix_revision::describe::Error), + Describe(#[from] Box), #[error("Could not produce an unambiguous shortened id for formatting.")] ShortId(#[from] crate::id::shorten::Error), #[error(transparent)] @@ -244,7 +244,8 @@ pub mod describe { first_parent: self.first_parent, max_candidates: self.max_candidates, }, - )?; + ) + .map_err(Exn::into_box)?; Ok(outcome.map(|outcome| Resolution { outcome, diff --git a/gix/src/lib.rs b/gix/src/lib.rs index 204faf492ac..01a61914873 100644 --- a/gix/src/lib.rs +++ b/gix/src/lib.rs @@ -145,7 +145,7 @@ pub use gix_utils as utils; pub use gix_validate as validate; pub use hash::{oid, ObjectId}; -pub use gix_error::Exn; +pub use gix_error::{Error, Exn}; pub mod interrupt; diff --git a/gix/src/revision/spec/parse/delegate/mod.rs b/gix/src/revision/spec/parse/delegate/mod.rs index d0c2611a8c7..206f5023bc4 100644 --- a/gix/src/revision/spec/parse/delegate/mod.rs +++ b/gix/src/revision/spec/parse/delegate/mod.rs @@ -1,14 +1,14 @@ use std::collections::HashSet; -use gix_hash::ObjectId; -use gix_revision::spec::{parse, parse::delegate}; -use smallvec::SmallVec; - -use super::{Delegate, Error, ObjectKindHint}; +use super::{error, Delegate, Error, ObjectKindHint}; use crate::{ ext::{ObjectIdExt, ReferenceExt}, Repository, }; +use gix_error::{bail, message, ErrorExt, Exn, Message, ResultExt, Something}; +use gix_hash::ObjectId; +use gix_revision::spec::{parse, parse::delegate}; +use smallvec::SmallVec; type Replacements = SmallVec<[(ObjectId, ObjectId); 1]>; @@ -21,7 +21,7 @@ impl<'repo> Delegate<'repo> { ambiguous_objects: Default::default(), idx: 0, kind: None, - err: Vec::new(), + delayed_errors: Vec::new(), prefix: Default::default(), last_call_was_disambiguate_prefix: Default::default(), opts, @@ -29,28 +29,29 @@ impl<'repo> Delegate<'repo> { } } - pub fn into_err(mut self) -> Error { + pub fn into_delayed_errors(mut self) -> Option { let repo = self.repo; + let mut errors = self.delayed_errors; for err in self .ambiguous_objects .iter_mut() .zip(self.prefix) .filter_map(|(a, b)| a.take().filter(|candidates| candidates.len() > 1).zip(b)) - .map(|(candidates, prefix)| Error::ambiguous(candidates, prefix, repo)) + .map(|(candidates, prefix)| error::ambiguous(candidates, prefix, repo)) .rev() { - self.err.insert(0, err); + errors.insert(0, err.into()); } - Error::from_errors(self.err) + (!errors.is_empty()).then(|| Error::from_errors(errors)) } pub fn into_rev_spec(mut self) -> Result, Error> { fn zero_or_one_objects_or_ambiguity_err( mut candidates: [Option>; 2], prefix: [Option; 2], - mut errors: Vec, repo: &Repository, ) -> Result<[Option; 2], Error> { + let mut errors = Vec::new(); let mut out = [None, None]; for ((candidates, prefix), out) in candidates.iter_mut().zip(prefix).zip(out.iter_mut()) { let candidates = candidates.take(); @@ -66,7 +67,8 @@ impl<'repo> Delegate<'repo> { _ => { errors.insert( 0, - Error::ambiguous(candidates, prefix.expect("set when obtaining candidates"), repo), + error::ambiguous(candidates, prefix.expect("set when obtaining candidates"), repo) + .into(), ); return Err(Error::from_errors(errors)); } @@ -80,24 +82,27 @@ impl<'repo> Delegate<'repo> { kind: Option, [first, second]: [Option; 2], ) -> Result { + pub fn malformed() -> Message { + message!("The rev-spec is malformed and misses a ref name") + } use gix_revision::spec::Kind::*; Ok(match kind.unwrap_or_default() { - IncludeReachable => gix_revision::Spec::Include(first.ok_or(Error::Malformed)?), - ExcludeReachable => gix_revision::Spec::Exclude(first.ok_or(Error::Malformed)?), + IncludeReachable => gix_revision::Spec::Include(first.ok_or_else(malformed)?), + ExcludeReachable => gix_revision::Spec::Exclude(first.ok_or_else(malformed)?), RangeBetween => gix_revision::Spec::Range { - from: first.ok_or(Error::Malformed)?, - to: second.ok_or(Error::Malformed)?, + from: first.ok_or_else(malformed)?, + to: second.ok_or_else(malformed)?, }, ReachableToMergeBase => gix_revision::Spec::Merge { - theirs: first.ok_or(Error::Malformed)?, - ours: second.ok_or(Error::Malformed)?, + theirs: first.ok_or_else(malformed)?, + ours: second.ok_or_else(malformed)?, }, - IncludeReachableFromParents => gix_revision::Spec::IncludeOnlyParents(first.ok_or(Error::Malformed)?), - ExcludeReachableFromParents => gix_revision::Spec::ExcludeParents(first.ok_or(Error::Malformed)?), + IncludeReachableFromParents => gix_revision::Spec::IncludeOnlyParents(first.ok_or_else(malformed)?), + ExcludeReachableFromParents => gix_revision::Spec::ExcludeParents(first.ok_or_else(malformed)?), }) } - let range = zero_or_one_objects_or_ambiguity_err(self.objs, self.prefix, self.err, self.repo)?; + let range = zero_or_one_objects_or_ambiguity_err(self.objs, self.prefix, self.repo)?; Ok(crate::revision::Spec { path: self.paths[0].take().or(self.paths[1].take()), first_ref: self.refs[0].take(), @@ -109,47 +114,62 @@ impl<'repo> Delegate<'repo> { } impl parse::Delegate for Delegate<'_> { - fn done(&mut self) { - self.follow_refs_to_objects_if_needed(); - self.disambiguate_objects_by_fallback_hint( + fn done(&mut self) -> Result<(), Exn> { + self.follow_refs_to_objects_if_needed_delay_errors()?; + self.disambiguate_objects_by_fallback_hint_delay_errors( self.kind_implies_committish() .then_some(ObjectKindHint::Committish) .or(self.opts.object_kind_hint), ); + if self.delayed_errors.is_empty() { + Ok(()) + } else { + let err = Exn::from_iter( + self.delayed_errors.drain(..), + message!("Encountered unprocessed errors when finishing"), + ); + Err(err.something()) + } } } impl delegate::Kind for Delegate<'_> { - fn kind(&mut self, kind: gix_revision::spec::Kind) -> Option<()> { + fn kind(&mut self, kind: gix_revision::spec::Kind) -> Result<(), Exn> { use gix_revision::spec::Kind::*; self.kind = Some(kind); if self.kind_implies_committish() { - self.disambiguate_objects_by_fallback_hint(ObjectKindHint::Committish.into()); + self.disambiguate_objects_by_fallback_hint_delay_errors(ObjectKindHint::Committish.into()); } if matches!(kind, RangeBetween | ReachableToMergeBase) { self.idx += 1; } - Some(()) + Ok(()) } } impl Delegate<'_> { + fn has_err(&self) -> bool { + !self.delayed_errors.is_empty() + } fn kind_implies_committish(&self) -> bool { self.kind.unwrap_or(gix_revision::spec::Kind::IncludeReachable) != gix_revision::spec::Kind::IncludeReachable } - fn disambiguate_objects_by_fallback_hint(&mut self, hint: Option) { - fn require_object_kind(repo: &Repository, obj: &gix_hash::oid, kind: gix_object::Kind) -> Result<(), Error> { - let obj = repo.find_object(obj)?; + + fn disambiguate_objects_by_fallback_hint_delay_errors(&mut self, hint: Option) { + fn require_object_kind(repo: &Repository, obj: &gix_hash::oid, kind: gix_object::Kind) -> Result<(), Exn> { + let obj = repo.find_object(obj).or_something()?; if obj.kind == kind { Ok(()) } else { - Err(Error::ObjectKind { - actual: obj.kind, - expected: kind, - oid: obj.id.attach(repo).shorten_or_id(), - }) + Err(message!( + "Object {oid} was a {actual}, but needed it to be a {expected}", + actual = obj.kind, + expected = kind, + oid = obj.id.attach(repo).shorten_or_id(), + ) + .raise_something()) } } @@ -186,27 +206,34 @@ impl Delegate<'_> { }; if errors.len() == objs.len() { - self.err.extend(errors.into_iter().map(|(_, err)| err)); + self.delayed_errors + .extend(errors.into_iter().map(|(_, err)| gix_error::Error::from(err).into())); } else { for (obj, err) in errors { objs.remove(&obj); - self.err.push(err); + self.delayed_errors.push(gix_error::Error::from(err).into()); } } } } } - fn follow_refs_to_objects_if_needed(&mut self) -> Option<()> { + + fn follow_refs_to_objects_if_needed_delay_errors(&mut self) -> Result<(), Exn> { let repo = self.repo; for (r, obj) in self.refs.iter().zip(self.objs.iter_mut()) { if let (Some(ref_), obj_opt @ None) = (r, obj) { if let Some(id) = ref_.target.try_id().map(ToOwned::to_owned).or_else(|| { match ref_.clone().attach(repo).peel_to_id() { Err(err) => { - self.err.push(Error::PeelToId { - name: ref_.name.clone(), - source: err, - }); + self.delayed_errors.push( + err.raise() + .raise(message!( + "Could not peel '{}' to obtain its target", + ref_.name.as_bstr() + )) + .into_error() + .into(), + ); None } Ok(id) => Some(id.detach()), @@ -216,7 +243,7 @@ impl Delegate<'_> { } } } - Some(()) + Ok(()) } fn unset_disambiguate_call(&mut self) { @@ -224,9 +251,9 @@ impl Delegate<'_> { } } -fn peel(repo: &Repository, obj: &gix_hash::oid, kind: gix_object::Kind) -> Result { - let mut obj = repo.find_object(obj)?; - obj = obj.peel_to_kind(kind)?; +fn peel(repo: &Repository, obj: &gix_hash::oid, kind: gix_object::Kind) -> Result { + let mut obj = repo.find_object(obj).or_something()?; + obj = obj.peel_to_kind(kind).or_something()?; debug_assert_eq!(obj.kind, kind, "bug in Object::peel_to_kind() which didn't deliver"); Ok(obj.id) } @@ -234,22 +261,22 @@ fn peel(repo: &Repository, obj: &gix_hash::oid, kind: gix_object::Kind) -> Resul fn handle_errors_and_replacements( destination: &mut Vec, objs: &mut HashSet, - errors: Vec<(ObjectId, Error)>, + errors: Vec<(ObjectId, Exn)>, replacements: &mut Replacements, -) -> Option<()> { +) -> Result<(), Exn> { if errors.len() == objs.len() { - destination.extend(errors.into_iter().map(|(_, err)| err)); - None + destination.extend(errors.into_iter().map(|(_, err)| gix_error::Error::from(err).into())); + bail!(Something) } else { for (obj, err) in errors { objs.remove(&obj); - destination.push(err); + destination.push(gix_error::Error::from(err).into()); } for (find, replace) in replacements { objs.remove(find); objs.insert(*replace); } - Some(()) + Ok(()) } } diff --git a/gix/src/revision/spec/parse/delegate/navigate.rs b/gix/src/revision/spec/parse/delegate/navigate.rs index ca32061acd7..11824c869bf 100644 --- a/gix/src/revision/spec/parse/delegate/navigate.rs +++ b/gix/src/revision/spec/parse/delegate/navigate.rs @@ -1,11 +1,11 @@ -use std::collections::HashSet; - +use gix_error::{bail, message, ErrorExt, Exn, OptionExt, ResultExt}; use gix_hash::ObjectId; use gix_index::entry::Stage; use gix_revision::spec::parse::{ delegate, delegate::{PeelTo, Traversal}, }; +use std::collections::HashSet; use crate::{ bstr::{BStr, ByteSlice}, @@ -13,23 +13,24 @@ use crate::{ object, revision::spec::parse::{ delegate::{handle_errors_and_replacements, peel, Replacements}, - Delegate, Error, + Delegate, }, Object, }; impl delegate::Navigate for Delegate<'_> { - fn traverse(&mut self, kind: Traversal) -> Option<()> { + fn traverse(&mut self, kind: Traversal) -> Result<(), Exn> { self.unset_disambiguate_call(); - self.follow_refs_to_objects_if_needed()?; + self.follow_refs_to_objects_if_needed_delay_errors()?; let mut replacements = Replacements::default(); - let mut errors = Vec::new(); + let mut errors = Vec::<(ObjectId, Exn)>::new(); let objs = match self.objs[self.idx].as_mut() { Some(objs) => objs, None => { - self.err.push(Error::TraversalWithoutStartObject); - return None; + bail!( + message!("Tried to navigate the commit-graph without providing an anchor first").raise_something() + ) } }; let repo = self.repo; @@ -37,25 +38,27 @@ impl delegate::Navigate for Delegate<'_> { for obj in objs.iter() { match kind { Traversal::NthParent(num) => { - match self.repo.find_object(*obj).map_err(Error::from).and_then(|obj| { + match self.repo.find_object(*obj).or_something().and_then(|obj| { obj.try_into_commit().map_err(|err| { let object::try_into::Error { actual, expected, id } = err; - Error::ObjectKind { - oid: id.attach(repo).shorten_or_id(), - actual, - expected, - } + message!( + "Object {oid} was a {actual}, but needed it to be a {expected}", + oid = id.attach(repo).shorten_or_id(), + ) + .raise_something() }) }) { Ok(commit) => match commit.parent_ids().nth(num.saturating_sub(1)) { Some(id) => replacements.push((commit.id, id.detach())), None => errors.push(( commit.id, - Error::ParentOutOfRange { - oid: commit.id().shorten_or_id(), - desired: num, - available: commit.parent_ids().count(), - }, + message!( + "Commit {oid} has {available} parents and parent number {desired} is out of range", + oid = commit.id().shorten_or_id(), + desired = num, + available = commit.parent_ids().count(), + ) + .raise_something(), )), }, Err(err) => errors.push((*obj, err)), @@ -74,44 +77,42 @@ impl delegate::Navigate for Delegate<'_> { Some(commit) => replacements.push((*obj, commit.id)), None => errors.push(( *obj, - Error::AncestorOutOfRange { - oid: id.shorten_or_id(), - desired: num, - available: id - .ancestors() - .first_parent_only() - .all() - .expect("cannot fail without sorting") - .skip(1) - .count(), - }, + message!("Commit {oid} has {available} ancestors along the first parent and ancestor number {num} is out of range", + oid = id.shorten_or_id(), + available = id + .ancestors() + .first_parent_only() + .all() + .expect("cannot fail without sorting") + .skip(1) + .count() + ).raise_something() )), } } } } - handle_errors_and_replacements(&mut self.err, objs, errors, &mut replacements) + handle_errors_and_replacements(&mut self.delayed_errors, objs, errors, &mut replacements) } - fn peel_until(&mut self, kind: PeelTo<'_>) -> Option<()> { + fn peel_until(&mut self, kind: PeelTo<'_>) -> Result<(), Exn> { self.unset_disambiguate_call(); - self.follow_refs_to_objects_if_needed()?; + self.follow_refs_to_objects_if_needed_delay_errors()?; let mut replacements = Replacements::default(); - let mut errors = Vec::new(); - let objs = self.objs[self.idx].as_mut()?; + let mut errors = Vec::<(ObjectId, Exn)>::new(); + let objs = self.objs[self.idx] + .as_mut() + .ok_or_raise_something(|| message!("Couldn't get object at internal index {idx}", idx = self.idx))?; let repo = self.repo; match kind { PeelTo::ValidObject => { for obj in objs.iter() { - match repo.find_object(*obj) { - Ok(_) => {} - Err(err) => { - errors.push((*obj, err.into())); - } - }; + if let Err(err) = repo.find_object(*obj) { + errors.push((*obj, err.raise_something())); + } } } PeelTo::ObjectKind(kind) => { @@ -127,16 +128,20 @@ impl delegate::Navigate for Delegate<'_> { let lookup_path = |obj: &ObjectId| { let tree_id = peel(repo, obj, gix_object::Kind::Tree)?; if path.is_empty() { - return Ok((tree_id, gix_object::tree::EntryKind::Tree.into())); + return Ok::<_, Exn>((tree_id, gix_object::tree::EntryKind::Tree.into())); } - let mut tree = repo.find_object(tree_id)?.into_tree(); - let entry = - tree.peel_to_entry_by_path(gix_path::from_bstr(path))? - .ok_or_else(|| Error::PathNotFound { - path: path.into(), - object: obj.attach(repo).shorten_or_id(), - tree: tree_id.attach(repo).shorten_or_id(), - })?; + let mut tree = repo.find_object(tree_id).or_something()?.into_tree(); + let entry = tree + .peel_to_entry_by_path(gix_path::from_bstr(path)) + .or_something()? + .ok_or_raise_something(|| { + message!( + "Could not find path {path:?} in tree {tree} of parent object {object}", + path = path, + object = obj.attach(repo).shorten_or_id(), + tree = tree_id.attach(repo).shorten_or_id(), + ) + })?; Ok((entry.object_id(), entry.mode())) }; for obj in objs.iter() { @@ -156,18 +161,18 @@ impl delegate::Navigate for Delegate<'_> { for oid in objs.iter() { match oid.attach(repo).object().and_then(Object::peel_tags_to_end) { Ok(obj) => replacements.push((*oid, obj.id)), - Err(err) => errors.push((*oid, err.into())), + Err(err) => errors.push((*oid, err.raise_something())), } } } } - handle_errors_and_replacements(&mut self.err, objs, errors, &mut replacements) + handle_errors_and_replacements(&mut self.delayed_errors, objs, errors, &mut replacements) } - fn find(&mut self, regex: &BStr, negated: bool) -> Option<()> { + fn find(&mut self, regex: &BStr, negated: bool) -> Result<(), Exn> { self.unset_disambiguate_call(); - self.follow_refs_to_objects_if_needed()?; + self.follow_refs_to_objects_if_needed_delay_errors()?; #[cfg(not(feature = "revparse-regex"))] let matches = |message: &BStr| -> bool { message.contains_str(regex) ^ negated }; @@ -184,15 +189,14 @@ impl delegate::Navigate for Delegate<'_> { } } Err(err) => { - self.err.push(err.into()); - return None; + bail!(err.raise_something()); } }; match self.objs[self.idx].as_mut() { Some(objs) => { let repo = self.repo; - let mut errors = Vec::new(); + let mut errors = Vec::<(ObjectId, Exn)>::new(); let mut replacements = Replacements::default(); for oid in objs.iter() { match oid @@ -205,8 +209,12 @@ impl delegate::Navigate for Delegate<'_> { let mut matched = false; let mut count = 0; let commits = iter.map(|res| { - res.map_err(Error::from).and_then(|commit| { - commit.id().object().map_err(Error::from).map(Object::into_commit) + res.map_err(|err| err.raise_something()).and_then(|commit| { + commit + .id() + .object() + .map_err(|err| err.raise_something()) + .map(Object::into_commit) }) }); for commit in commits { @@ -225,88 +233,88 @@ impl delegate::Navigate for Delegate<'_> { if !matched { errors.push(( *oid, - Error::NoRegexMatch { - regex: regex.into(), - commits_searched: count, - oid: oid.attach(repo).shorten_or_id(), - }, + message!( + "None of {commits_searched} commits from {oid} matched {kind} {regex:?}", + regex = regex, + commits_searched = count, + oid = oid.attach(repo).shorten_or_id(), + kind = if cfg!(feature = "revparse-regex") { + "regex" + } else { + "text" + } + ) + .raise_something(), )); } } - Err(err) => errors.push((*oid, err.into())), + Err(err) => errors.push((*oid, err.raise_something())), } } - handle_errors_and_replacements(&mut self.err, objs, errors, &mut replacements) + handle_errors_and_replacements(&mut self.delayed_errors, objs, errors, &mut replacements) } - None => match self.repo.references() { - Ok(references) => match references.all() { - Ok(references) => { - match self - .repo - .rev_walk( - references - .peeled() - .ok()? - .filter_map(Result::ok) - .filter(|r| r.id().header().ok().is_some_and(|obj| obj.kind().is_commit())) - .filter_map(|r| r.detach().peeled), - ) - .sorting(crate::revision::walk::Sorting::ByCommitTime(Default::default())) - .all() - { - Ok(iter) => { - let mut matched = false; - let mut count = 0; - let commits = iter.map(|res| { - res.map_err(Error::from).and_then(|commit| { - commit.id().object().map_err(Error::from).map(Object::into_commit) - }) - }); - for commit in commits { - count += 1; - match commit { - Ok(commit) => { - if matches(commit.message_raw_sloppy()) { - self.objs[self.idx] - .get_or_insert_with(HashSet::default) - .insert(commit.id); - matched = true; - break; - } - } - Err(err) => self.err.push(err), - } - } - if matched { - Some(()) - } else { - self.err.push(Error::NoRegexMatchAllRefs { - regex: regex.into(), - commits_searched: count, - }); - None - } - } - Err(err) => { - self.err.push(err.into()); - None + None => { + let references = self.repo.references().or_something()?; + let references = references.all().or_something()?; + let iter = self + .repo + .rev_walk( + references + .peeled() + .or_raise_something(|| message!("Couldn't configure iterator for peeling"))? + .filter_map(Result::ok) + .filter(|r| r.id().header().ok().is_some_and(|obj| obj.kind().is_commit())) + .filter_map(|r| r.detach().peeled), + ) + .sorting(crate::revision::walk::Sorting::ByCommitTime(Default::default())) + .all() + .or_something()?; + let mut matched = false; + let mut count = 0; + let commits = iter.map(|res| { + res.map_err(|err| err.raise_something()).and_then(|commit| { + commit + .id() + .object() + .map_err(|err| err.raise_something()) + .map(Object::into_commit) + }) + }); + for commit in commits { + count += 1; + match commit { + Ok(commit) => { + if matches(commit.message_raw_sloppy()) { + self.objs[self.idx] + .get_or_insert_with(HashSet::default) + .insert(commit.id); + matched = true; + break; } } + Err(err) => self.delayed_errors.push(gix_error::Error::from(err).into()), } - Err(err) => { - self.err.push(err.into()); - None - } - }, - Err(err) => { - self.err.push(err.into()); - None } - }, + if matched { + Ok(()) + } else { + Err(message!( + "None of {commits_searched} commits reached from all references matched {kind} {regex:?}", + regex = regex, + commits_searched = count, + kind = if cfg!(feature = "revparse-regex") { + "regex" + } else { + "text" + } + ) + .raise_something()) + } + } } } - fn index_lookup(&mut self, path: &BStr, stage: u8) -> Option<()> { + fn index_lookup(&mut self, path: &BStr, stage: u8) -> Result<(), Exn> { let stage = match stage { 0 => Stage::Unconflicted, 1 => Stage::Base, @@ -317,43 +325,42 @@ impl delegate::Navigate for Delegate<'_> { ), }; self.unset_disambiguate_call(); - match self.repo.index() { - Ok(index) => match index.entry_by_path_and_stage(path, stage) { - Some(entry) => { - self.objs[self.idx] - .get_or_insert_with(HashSet::default) - .insert(entry.id); + let index = self.repo.index().or_something()?; + match index.entry_by_path_and_stage(path, stage) { + Some(entry) => { + self.objs[self.idx] + .get_or_insert_with(HashSet::default) + .insert(entry.id); - self.paths[self.idx] = Some(( - path.to_owned(), - entry - .mode - .to_tree_entry_mode() - .unwrap_or(gix_object::tree::EntryKind::Blob.into()), - )); - Some(()) - } - None => { - let stage_hint = [Stage::Unconflicted, Stage::Base, Stage::Ours] - .iter() - .filter(|our_stage| **our_stage != stage) - .find_map(|stage| index.entry_index_by_path_and_stage(path, *stage).map(|_| *stage)); - let exists = self - .repo - .workdir() - .is_some_and(|root| root.join(gix_path::from_bstr(path)).exists()); - self.err.push(Error::IndexLookup { - desired_path: path.into(), - desired_stage: stage, - exists, - stage_hint, - }); - None - } - }, - Err(err) => { - self.err.push(err.into()); - None + self.paths[self.idx] = Some(( + path.to_owned(), + entry + .mode + .to_tree_entry_mode() + .unwrap_or(gix_object::tree::EntryKind::Blob.into()), + )); + Ok(()) + } + None => { + let stage_hint = [Stage::Unconflicted, Stage::Base, Stage::Ours] + .iter() + .filter(|our_stage| **our_stage != stage) + .find_map(|stage| index.entry_index_by_path_and_stage(path, *stage).map(|_| *stage)); + let exists = self + .repo + .workdir() + .is_some_and(|root| root.join(gix_path::from_bstr(path)).exists()); + Err(message!( + "Path {path:?} did not exist in index at stage {desired_stage}{stage_hint}{exists}", + exists = exists + .then(|| ". It exists on disk") + .unwrap_or(". It does not exist on disk"), + stage_hint = stage_hint + .map(|actual| format!(". It does exist at stage {}", actual as u8)) + .unwrap_or_default(), + desired_stage = stage as u8, + ) + .raise_something()) } } } diff --git a/gix/src/revision/spec/parse/delegate/revision.rs b/gix/src/revision/spec/parse/delegate/revision.rs index 8cecf217fa8..1aa4b783afb 100644 --- a/gix/src/revision/spec/parse/delegate/revision.rs +++ b/gix/src/revision/spec/parse/delegate/revision.rs @@ -1,11 +1,12 @@ -use std::collections::HashSet; - +use gix_error::{bail, message, ErrorExt, Exn, ResultExt, Something}; use gix_hash::ObjectId; use gix_revision::spec::parse::{ delegate, delegate::{ReflogLookup, SiblingBranch}, }; +use std::collections::HashSet; +use crate::revision::spec::parse::error; use crate::{ bstr::{BStr, BString, ByteSlice}, ext::ReferenceExt, @@ -14,20 +15,19 @@ use crate::{ }; impl delegate::Revision for Delegate<'_> { - fn find_ref(&mut self, name: &BStr) -> Option<()> { + fn find_ref(&mut self, name: &BStr) -> Result<(), Exn> { self.unset_disambiguate_call(); - if !self.err.is_empty() && self.refs[self.idx].is_some() { - return None; + if self.has_err() && self.refs[self.idx].is_some() { + bail!(Something) } match self.repo.refs.find(name) { Ok(r) => { assert!(self.refs[self.idx].is_none(), "BUG: cannot set the same ref twice"); self.refs[self.idx] = Some(r); - Some(()) + Ok(()) } Err(err) => { - self.err.push(err.into()); - None + bail!(err.raise_something()) } } } @@ -36,29 +36,23 @@ impl delegate::Revision for Delegate<'_> { &mut self, prefix: gix_hash::Prefix, _must_be_commit: Option>, - ) -> Option<()> { + ) -> Result<(), Exn> { self.last_call_was_disambiguate_prefix[self.idx] = true; let mut candidates = Some(HashSet::default()); self.prefix[self.idx] = Some(prefix); let empty_tree_id = gix_hash::ObjectId::empty_tree(prefix.as_oid().kind()); - let res = if prefix.as_oid() == empty_tree_id { + let ok = if prefix.as_oid() == empty_tree_id { candidates.as_mut().expect("set").insert(empty_tree_id); Ok(Some(Err(()))) } else { self.repo.objects.lookup_prefix(prefix, candidates.as_mut()) - }; + } + .or_something()?; - match res { - Err(err) => { - self.err.push(err.into()); - None - } - Ok(None) => { - self.err.push(Error::PrefixNotFound { prefix }); - None - } - Ok(Some(Ok(_) | Err(()))) => { + match ok { + None => Err(message!("An object prefixed {} could not be found", prefix).raise_something()), + Some(Ok(_) | Err(())) => { assert!(self.objs[self.idx].is_none(), "BUG: cannot set the same prefix twice"); let candidates = candidates.expect("set above"); match self.opts.refs_hint { @@ -67,12 +61,12 @@ impl delegate::Revision for Delegate<'_> { { self.ambiguous_objects[self.idx] = Some(candidates.clone()); self.objs[self.idx] = Some(candidates); - Some(()) + Ok(()) } RefsHint::PreferObject => { self.ambiguous_objects[self.idx] = Some(candidates.clone()); self.objs[self.idx] = Some(candidates); - Some(()) + Ok(()) } RefsHint::PreferRef | RefsHint::PreferObjectOnFullLengthHexShaUseRefOtherwise | RefsHint::Fail => { match self.repo.refs.find(&prefix.to_string()) { @@ -80,21 +74,23 @@ impl delegate::Revision for Delegate<'_> { assert!(self.refs[self.idx].is_none(), "BUG: cannot set the same ref twice"); if self.opts.refs_hint == RefsHint::Fail { self.refs[self.idx] = Some(ref_.clone()); - self.err.push(Error::AmbiguousRefAndObject { - prefix, - reference: ref_, - }); - self.err.push(Error::ambiguous(candidates, prefix, self.repo)); - None + self.delayed_errors.push( + message!( + "The short hash {prefix} matched both the reference {name} and at least one object", + name = ref_.name + ) + .into(), + ); + Err(error::ambiguous(candidates, prefix, self.repo).raise_something()) } else { self.refs[self.idx] = Some(ref_); - Some(()) + Ok(()) } } Err(_) => { self.ambiguous_objects[self.idx] = Some(candidates.clone()); self.objs[self.idx] = Some(candidates); - Some(()) + Ok(()) } } } @@ -103,7 +99,7 @@ impl delegate::Revision for Delegate<'_> { } } - fn reflog(&mut self, query: ReflogLookup) -> Option<()> { + fn reflog(&mut self, query: ReflogLookup) -> Result<(), Exn> { self.unset_disambiguate_call(); let r = match &mut self.refs[self.idx] { Some(r) => r.clone().attach(self.repo), @@ -112,14 +108,8 @@ impl delegate::Revision for Delegate<'_> { *val = Some(r.clone().detach()); r } - Ok(None) => { - self.err.push(Error::UnbornHeadsHaveNoRefLog); - return None; - } - Err(err) => { - self.err.push(err.into()); - return None; - } + Ok(None) => return Err(message!("Unborn heads do not have a reflog yet").raise_something()), + Err(err) => return Err(err.raise_something()), }, }; @@ -141,50 +131,42 @@ impl delegate::Revision for Delegate<'_> { { Some(closest_line) => closest_line.new_oid, None => match last { - None => { - self.err.push(Error::EmptyReflog); - return None; - } + None => return Err(message!("Reflog does not contain any entries").raise_something()), Some(id) => id, }, }; self.objs[self.idx] .get_or_insert_with(HashSet::default) .insert(id_to_insert); - Some(()) + Ok(()) } ReflogLookup::Entry(no) => match it.nth(no).and_then(Result::ok) { Some(line) => { self.objs[self.idx] .get_or_insert_with(HashSet::default) .insert(line.new_oid); - Some(()) - } - None => { - let available = platform.rev().ok().flatten().map_or(0, Iterator::count); - self.err.push(Error::RefLogEntryOutOfRange { - reference: r.detach(), - desired: no, - available, - }); - None + Ok(()) } + None => Err(message!( + "Reference {r:?} has {available} ref-log entries and entry number {no} is out of range", + available = platform.rev().ok().flatten().map_or(0, Iterator::count) + ) + .raise_something()), }, }, - None => { - self.err.push(Error::MissingRefLog { - reference: r.name().as_bstr().into(), - action: match query { - ReflogLookup::Entry(_) => "lookup reflog entry by index", - ReflogLookup::Date(_) => "lookup reflog entry by date", - }, - }); - None - } + None => Err(message!( + "Reference {reference:?} does not have a reference log, cannot {action}", + action = match query { + ReflogLookup::Entry(_) => "lookup reflog entry by index", + ReflogLookup::Date(_) => "lookup reflog entry by date", + }, + reference = r.name().as_bstr() + ) + .raise_something()), } } - fn nth_checked_out_branch(&mut self, branch_no: usize) -> Option<()> { + fn nth_checked_out_branch(&mut self, branch_no: usize) -> Result<(), Exn> { self.unset_disambiguate_call(); fn prior_checkouts_iter<'a>( platform: &'a mut gix_ref::file::log::iter::Platform<'static, '_>, @@ -196,22 +178,24 @@ impl delegate::Revision for Delegate<'_> { .and_then(|from_to| from_to.find(" to ").map(|pos| &from_to[..pos])) .map(|from_branch| (from_branch.into(), line.previous_oid)) })), - None => Err(Error::MissingRefLog { - reference: "HEAD".into(), - action: "search prior checked out branch", - }), + None => Err(message!( + "Reference HEAD does not have a reference log, cannot search prior checked out branch", + ) + .raise() + .into_error() + .into()), } } let head = match self.repo.head() { Ok(head) => head, - Err(err) => { - self.err.push(err.into()); - return None; - } + Err(err) => return Err(err.raise_something()), }; - match prior_checkouts_iter(&mut head.log_iter()).map(|mut it| it.nth(branch_no.saturating_sub(1))) { - Ok(Some((ref_name, id))) => { + let ok = prior_checkouts_iter(&mut head.log_iter()) + .map(|mut it| it.nth(branch_no.saturating_sub(1))) + .or_something()?; + match ok { + Some((ref_name, id)) => { let id = match self.repo.find_reference(ref_name.as_bstr()) { Ok(mut r) => { let id = r.peel_to_id().map(crate::Id::detach).unwrap_or(id); @@ -221,25 +205,19 @@ impl delegate::Revision for Delegate<'_> { Err(_) => id, }; self.objs[self.idx].get_or_insert_with(HashSet::default).insert(id); - Some(()) - } - Ok(None) => { - self.err.push(Error::PriorCheckoutOutOfRange { - desired: branch_no, - available: prior_checkouts_iter(&mut head.log_iter()) - .map(Iterator::count) - .unwrap_or(0), - }); - None - } - Err(err) => { - self.err.push(err); - None + Ok(()) } + None => Err(message!( + "HEAD has {available} prior checkouts and checkout number {branch_no} is out of range", + available = prior_checkouts_iter(&mut head.log_iter()) + .map(Iterator::count) + .unwrap_or(0) + ) + .raise_something()), } } - fn sibling_branch(&mut self, kind: SiblingBranch) -> Option<()> { + fn sibling_branch(&mut self, kind: SiblingBranch) -> Result<(), Exn> { self.unset_disambiguate_call(); let reference = match &mut self.refs[self.idx] { val @ None => match self.repo.head().map(crate::Head::try_into_referent) { @@ -248,12 +226,12 @@ impl delegate::Revision for Delegate<'_> { r } Ok(None) => { - self.err.push(Error::UnbornHeadForSibling); - return None; + return Err( + message!("Unborn heads cannot have push or upstream tracking branches").raise_something() + ) } Err(err) => { - self.err.push(err.into()); - return None; + return Err(err.raise_something()); } }, Some(r) => r.clone().attach(self.repo), @@ -262,28 +240,35 @@ impl delegate::Revision for Delegate<'_> { SiblingBranch::Upstream => remote::Direction::Fetch, SiblingBranch::Push => remote::Direction::Push, }; + let make_message = || { + message!( + "Error when obtaining {direction} tracking branch for {name}", + name = reference.name().as_bstr(), + direction = direction.as_str() + ) + }; match reference.remote_tracking_ref_name(direction) { - None => self.err.push(Error::NoTrackingBranch { - name: reference.inner.name, - direction, - }), - Some(Err(err)) => self.err.push(Error::GetTrackingBranch { - name: reference.inner.name, - direction, - source: Box::new(err), - }), + None => self.delayed_errors.push( + message!( + "Branch named {name} does not have a {direction} tracking branch configured", + name = reference.name().as_bstr(), + direction = direction.as_str() + ) + .into(), + ), + Some(Err(err)) => self + .delayed_errors + .push(err.raise().raise(make_message()).into_error().into()), Some(Ok(name)) => match self.repo.find_reference(name.as_ref()) { - Err(err) => self.err.push(Error::GetTrackingBranch { - name: reference.inner.name, - direction, - source: Box::new(err), - }), + Err(err) => self + .delayed_errors + .push(err.raise().raise(make_message()).into_error().into()), Ok(r) => { self.refs[self.idx] = r.inner.into(); - return Some(()); + return Ok(()); } }, } - None + Err(message!("Couldn't find sibling of {kind:?}", kind = kind).raise_something()) } } diff --git a/gix/src/revision/spec/parse/error.rs b/gix/src/revision/spec/parse/error.rs index 3cbcab3353d..0872c6b02e2 100644 --- a/gix/src/revision/spec/parse/error.rs +++ b/gix/src/revision/spec/parse/error.rs @@ -1,6 +1,6 @@ -use std::collections::HashSet; - +use gix_error::{message, Exn}; use gix_hash::ObjectId; +use std::collections::HashSet; use super::Error; use crate::{bstr, bstr::BString, ext::ObjectIdExt, Repository}; @@ -51,95 +51,94 @@ impl std::fmt::Display for CandidateInfo { } } -impl Error { - pub(crate) fn ambiguous(candidates: HashSet, prefix: gix_hash::Prefix, repo: &Repository) -> Self { - #[derive(PartialOrd, Ord, Eq, PartialEq, Copy, Clone)] - enum Order { - Tag, - Commit, - Tree, - Blob, - Invalid, - } - let candidates = { - let mut c: Vec<_> = candidates - .into_iter() - .map(|oid| { - let obj = repo.find_object(oid); - let order = match &obj { - Err(_) => Order::Invalid, - Ok(obj) => match obj.kind { - gix_object::Kind::Tag => Order::Tag, - gix_object::Kind::Commit => Order::Commit, - gix_object::Kind::Tree => Order::Tree, - gix_object::Kind::Blob => Order::Blob, - }, - }; - (oid, obj, order) - }) - .collect(); - c.sort_by(|lhs, rhs| lhs.2.cmp(&rhs.2).then_with(|| lhs.0.cmp(&rhs.0))); - c - }; - Error::AmbiguousPrefix { - prefix, - info: candidates - .into_iter() - .map(|(oid, find_result, _)| { - let info = match find_result { - Ok(obj) => match obj.kind { - gix_object::Kind::Tree | gix_object::Kind::Blob => CandidateInfo::Object { kind: obj.kind }, - gix_object::Kind::Tag => { - let tag = obj.to_tag_ref(); - CandidateInfo::Tag { name: tag.name.into() } - } - gix_object::Kind::Commit => { - use bstr::ByteSlice; - let commit = obj.to_commit_ref(); - let date = match commit.committer() { - Ok(signature) => signature.time.trim().to_owned(), - Err(_) => { - let committer = commit.committer; - let manually_parsed_best_effort = committer - .rfind_byte(b'>') - .map(|pos| committer[pos + 1..].trim().as_bstr().to_string()); - manually_parsed_best_effort.unwrap_or_default() - } - }; - CandidateInfo::Commit { - date, - title: commit.message().title.trim().into(), - } - } - }, - Err(err) => CandidateInfo::FindError { source: err }, - }; - (oid.attach(repo).shorten().unwrap_or_else(|_| oid.into()), info) - }) - .collect(), - } +pub(crate) fn ambiguous( + candidates: HashSet, + prefix: gix_hash::Prefix, + repo: &Repository, +) -> gix_error::Message { + #[derive(PartialOrd, Ord, Eq, PartialEq, Copy, Clone)] + enum Order { + Tag, + Commit, + Tree, + Blob, + Invalid, } + let candidates = { + let mut c: Vec<_> = candidates + .into_iter() + .map(|oid| { + let obj = repo.find_object(oid); + let order = match &obj { + Err(_) => Order::Invalid, + Ok(obj) => match obj.kind { + gix_object::Kind::Tag => Order::Tag, + gix_object::Kind::Commit => Order::Commit, + gix_object::Kind::Tree => Order::Tree, + gix_object::Kind::Blob => Order::Blob, + }, + }; + (oid, obj, order) + }) + .collect(); + c.sort_by(|lhs, rhs| lhs.2.cmp(&rhs.2).then_with(|| lhs.0.cmp(&rhs.0))); + c + }; + let info: Vec<_> = candidates + .into_iter() + .map(|(oid, find_result, _)| { + let info = match find_result { + Ok(obj) => match obj.kind { + gix_object::Kind::Tree | gix_object::Kind::Blob => CandidateInfo::Object { kind: obj.kind }, + gix_object::Kind::Tag => { + let tag = obj.to_tag_ref(); + CandidateInfo::Tag { name: tag.name.into() } + } + gix_object::Kind::Commit => { + use bstr::ByteSlice; + let commit = obj.to_commit_ref(); + let date = match commit.committer() { + Ok(signature) => signature.time.trim().to_owned(), + Err(_) => { + let committer = commit.committer; + let manually_parsed_best_effort = committer + .rfind_byte(b'>') + .map(|pos| committer[pos + 1..].trim().as_bstr().to_string()); + manually_parsed_best_effort.unwrap_or_default() + } + }; + CandidateInfo::Commit { + date, + title: commit.message().title.trim().into(), + } + } + }, + Err(err) => CandidateInfo::FindError { source: err }, + }; + (oid.attach(repo).shorten().unwrap_or_else(|_| oid.into()), info) + }) + .collect(); + message!( + "Short id {prefix} is ambiguous. Candidates are:\n{info}", + prefix = prefix, + info = info + .iter() + .map(|(oid, info)| format!("\t{oid} {info}")) + .collect::>() + .join("\n") + ) +} +impl Error { pub(crate) fn from_errors(errors: Vec) -> Self { match errors.len() { 0 => unreachable!( "BUG: cannot create something from nothing, must have recorded some errors to call from_errors()" ), 1 => errors.into_iter().next().expect("one"), - _ => { - let mut it = errors.into_iter().rev(); - let mut recent = Error::Multi { - current: Box::new(it.next().expect("at least one error")), - next: None, - }; - for err in it { - recent = Error::Multi { - current: Box::new(err), - next: Some(Box::new(recent)), - } - } - recent - } + num_errors => Exn::from_iter(errors.into_iter(), message!("{} errors", num_errors)) + .into_error() + .into(), } } } diff --git a/gix/src/revision/spec/parse/mod.rs b/gix/src/revision/spec/parse/mod.rs index 68cdf5e7ea3..9cfcd88d947 100644 --- a/gix/src/revision/spec/parse/mod.rs +++ b/gix/src/revision/spec/parse/mod.rs @@ -1,7 +1,5 @@ -use std::collections::HashSet; - use gix_hash::ObjectId; -use gix_revision::spec::parse; +use std::collections::HashSet; use crate::{bstr::BStr, revision::Spec, Repository}; @@ -35,8 +33,13 @@ impl<'repo> Spec<'repo> { pub fn from_bstr<'a>(spec: impl Into<&'a BStr>, repo: &'repo Repository, opts: Options) -> Result { let mut delegate = Delegate::new(repo, opts); match gix_revision::spec::parse(spec.into(), &mut delegate) { - Err(parse::Error::Delegate) => Err(delegate.into_err()), - Err(err) => Err(err.into()), + Err(err) => { + if let Some(delegate_err) = delegate.into_delayed_errors() { + Err(err.chain(delegate_err).into_error().into()) + } else { + Err(err.into_error().into()) + } + } Ok(()) => delegate.into_rev_spec(), } } @@ -54,7 +57,8 @@ struct Delegate<'repo> { kind: Option, opts: Options, - err: Vec, + /// Keeps track of errors that are supposed to be returned later. + delayed_errors: Vec, /// The ambiguous prefix obtained during a call to `disambiguate_prefix()`. prefix: [Option; 2], /// If true, we didn't try to do any other transformation which might have helped with disambiguation. diff --git a/gix/src/revision/spec/parse/types.rs b/gix/src/revision/spec/parse/types.rs index 1045364eb13..d9f70f3ea9d 100644 --- a/gix/src/revision/spec/parse/types.rs +++ b/gix/src/revision/spec/parse/types.rs @@ -1,5 +1,3 @@ -use crate::{bstr::BString, object, reference, remote}; - /// A hint to know what to do if refs and object names are equal. #[derive(Default, Debug, Copy, Clone, PartialEq, Eq)] pub enum RefsHint { @@ -55,145 +53,18 @@ pub struct Options { #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { - #[error("Could not peel '{}' to obtain its target", name)] - PeelToId { - name: gix_ref::FullName, - source: reference::peel::Error, - }, - #[error("The rev-spec is malformed and misses a ref name")] - Malformed, - #[error("Unborn heads do not have a reflog yet")] - UnbornHeadsHaveNoRefLog, - #[error("Unborn heads cannot have push or upstream tracking branches")] - UnbornHeadForSibling, - #[error("Branch named {name} does not have a {} tracking branch configured", direction.as_str())] - NoTrackingBranch { - name: gix_ref::FullName, - direction: remote::Direction, - }, - #[error("Error when obtaining {} tracking branch for {name}", direction.as_str())] - GetTrackingBranch { - name: gix_ref::FullName, - direction: remote::Direction, - source: Box, - }, - #[error("Reference {reference:?} does not have a reference log, cannot {action}")] - MissingRefLog { reference: BString, action: &'static str }, - #[error("HEAD has {available} prior checkouts and checkout number {desired} is out of range")] - PriorCheckoutOutOfRange { desired: usize, available: usize }, - #[error("Reference {:?} has {available} ref-log entries and entry number {desired} is out of range", reference.name.as_bstr())] - RefLogEntryOutOfRange { - reference: gix_ref::Reference, - desired: usize, - available: usize, - }, - #[error( - "Commit {oid} has {available} ancestors along the first parent and ancestor number {desired} is out of range" - )] - AncestorOutOfRange { - oid: gix_hash::Prefix, - desired: usize, - available: usize, - }, - #[error("Commit {oid} has {available} parents and parent number {desired} is out of range")] - ParentOutOfRange { - oid: gix_hash::Prefix, - desired: usize, - available: usize, - }, - #[error("Path {desired_path:?} did not exist in index at stage {}{}{}", *desired_stage as u8, stage_hint.map(|actual|format!(". It does exist at stage {}", actual as u8)).unwrap_or_default(), exists.then(|| ". It exists on disk").unwrap_or(". It does not exist on disk"))] - IndexLookup { - desired_path: BString, - desired_stage: gix_index::entry::Stage, - stage_hint: Option, - exists: bool, - }, - #[error(transparent)] - FindHead(#[from] reference::find::existing::Error), - #[error(transparent)] - Index(#[from] crate::worktree::open_index::Error), - #[error(transparent)] - RevWalkIterInit(#[from] crate::reference::iter::init::Error), - #[error(transparent)] - RevWalkAllReferences(#[from] gix_ref::packed::buffer::open::Error), - #[cfg(feature = "revparse-regex")] - #[error(transparent)] - InvalidRegex(#[from] regex::Error), - #[cfg_attr( - feature = "revparse-regex", - error("None of {commits_searched} commits from {oid} matched regex {regex:?}") - )] - #[cfg_attr( - not(feature = "revparse-regex"), - error("None of {commits_searched} commits from {oid} matched text {regex:?}") - )] - NoRegexMatch { - regex: BString, - oid: gix_hash::Prefix, - commits_searched: usize, + // TODO: just an intermediate step to change everything to messages + #[error("{}", source)] + Error { + #[from] + source: gix_error::Error, }, - #[cfg_attr( - feature = "revparse-regex", - error("None of {commits_searched} commits reached from all references matched regex {regex:?}") - )] - #[cfg_attr( - not(feature = "revparse-regex"), - error("None of {commits_searched} commits reached from all references matched text {regex:?}") - )] - NoRegexMatchAllRefs { regex: BString, commits_searched: usize }, - #[error( - "The short hash {prefix} matched both the reference {} and at least one object", reference.name)] - AmbiguousRefAndObject { - /// The prefix to look for. - prefix: gix_hash::Prefix, - /// The reference matching the prefix. - reference: gix_ref::Reference, + // TODO: just an intermediate step to change everything to messages + #[error("{}", source.message)] + Message { + #[from] + source: gix_error::Message, }, #[error(transparent)] - IdFromHex(#[from] gix_hash::decode::Error), - #[error(transparent)] FindReference(#[from] gix_ref::file::find::existing::Error), - #[error(transparent)] - FindObject(#[from] object::find::existing::Error), - #[error(transparent)] - LookupPrefix(#[from] gix_odb::store::prefix::lookup::Error), - #[error(transparent)] - PeelToKind(#[from] object::peel::to_kind::Error), - #[error("Object {oid} was a {actual}, but needed it to be a {expected}")] - ObjectKind { - oid: gix_hash::Prefix, - actual: gix_object::Kind, - expected: gix_object::Kind, - }, - #[error(transparent)] - Parse(#[from] gix_revision::spec::parse::Error), - #[error("An object prefixed {prefix} could not be found")] - PrefixNotFound { prefix: gix_hash::Prefix }, - #[error("Short id {prefix} is ambiguous. Candidates are:\n{}", info.iter().map(|(oid, info)| format!("\t{oid} {info}")).collect::>().join("\n"))] - AmbiguousPrefix { - prefix: gix_hash::Prefix, - info: Vec<(gix_hash::Prefix, super::error::CandidateInfo)>, - }, - #[error("Could not find path {path:?} in tree {tree} of parent object {object}")] - PathNotFound { - object: gix_hash::Prefix, - tree: gix_hash::Prefix, - path: BString, - }, - #[error("{current}")] - Multi { - current: Box, - #[source] - next: Option>, - }, - #[error(transparent)] - Traverse(#[from] crate::revision::walk::iter::Error), - #[error("Tried to navigate the commit-graph without providing an anchor first")] - TraversalWithoutStartObject, - #[error(transparent)] - Walk(#[from] crate::revision::walk::Error), - #[error("Spec does not contain a single object id")] - SingleNotFound, - #[error("Reflog does not contain any entries")] - EmptyReflog, } diff --git a/gix/tests/gix/revision/spec/from_bytes/mod.rs b/gix/tests/gix/revision/spec/from_bytes/mod.rs index c728932d8d1..dc860da4fa2 100644 --- a/gix/tests/gix/revision/spec/from_bytes/mod.rs +++ b/gix/tests/gix/revision/spec/from_bytes/mod.rs @@ -146,8 +146,23 @@ fn access_blob_through_tree() { "we capture tree-paths" ); + let err = parse_spec("0000000000cdc:missing", &repo).unwrap_err(); + // TODO: the error should resolve to the debug tree. + insta::assert_debug_snapshot!(err, @r#" + Error { + source: delegate.peel_until(Path("missing")) failed, at /Users/byron/dev/github.com/GitoxideLabs/gitoxide/gix-revision/src/spec/parse/function.rs:687:22 + | + |-> Something went wrong, at gix/src/revision/spec/parse/delegate/mod.rs:269:9 + | + |-> Something went wrong, at /Users/byron/dev/github.com/GitoxideLabs/gitoxide/gix/src/revision/spec/parse/mod.rs:38:29 + | + |-> Something went wrong, at /Users/byron/dev/github.com/GitoxideLabs/gitoxide/gix/src/revision/spec/parse/mod.rs:38:29 + | + |-> Could not find path "missing" in tree 0000000000c of parent object 0000000000c, at /Users/byron/dev/github.com/GitoxideLabs/gitoxide/gix/src/revision/spec/parse/mod.rs:38:29, + } + "#); assert_eq!( - parse_spec("0000000000cdc:missing", &repo).unwrap_err().to_string(), + err.to_string(), "Could not find path \"missing\" in tree 0000000000c of parent object 0000000000c" ); }