-
Notifications
You must be signed in to change notification settings - Fork 254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
POC: use snafu
instead of derive_more
for errors
#1604
Conversation
It's because that snafu uses each variant as a struct instead of tuple? // snafu
#[derive(Debug, snafu::Snafu)]
pub enum Error {
#[snafu(display("Metadata Error: {source}"), context(false))]
Metadata {
/// Error source
source: MetadataError,
}
}
// dervive_more
#[derive(Debug, dervive_more::Display)]
enum Error {
#[display(fmt = "Metadata Error: {_0}")]
Metadata(MetadataError),
} In downstream code then one has to do: // snafu
if let Err(Error::Metadata { e }) = my_err {}
// derive_more
if let Err(Error::Metadata(e)) = my_err {} Looks like a good solution to me :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! This approach also looks good, nice 👍
use core::fmt::{self, Debug, Display}; | ||
|
||
#[derive(Clone, PartialEq, Eq)] | ||
pub struct DisplayError<T>(pub T); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Offhand I can't quite see what the reason for this is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
snafu uses its own custom snafu::Error type in no-std contexts, therefore we needed to define a wrapper type that implements snafu::AsErrorSource for external error types and this led to a bit of boilerplate.snafu uses its own custom snafu::Error type in no-std contexts, therefore we needed to define a wrapper type that implements snafu::AsErrorSource for external error types and this led to a bit of boilerplate.
Most likely ☝️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, I should have paid more attention reading :)
closing in favor of #1600 |
Description
derive_more
#1600 (see Replacederive_more
(and removesyn
1.0 in favour of 2.0)? #1503)derive_more
usage withsnafu
Changing the library was somewhat easy, however:
snafu
uses its own customsnafu::Error
type in no-std contexts, therefore we needed to define a wrapper type that implementssnafu::AsErrorSource
for external error types and this led to a bit of boilerplate.snafu
doesn't support tuple variants yet, so its impossible to define anError
enum like this:it will be defined as:
snafu
or use some other library or defineDisplay
/From
by hand