thiserror icon indicating copy to clipboard operation
thiserror copied to clipboard

Consider exposing formatter in display attribute

Open dtolnay opened this issue 3 years ago • 2 comments

For this sort of code today [https://github.com/dtolnay/thiserror/pull/175#issuecomment-1264050944]:

#[error("{code}{}", match .message {
    Some(msg) => format!(" - {}", &msg),
    None => "".to_owned(),
})]
SungrowError { code: u16, message: Option<String> },

one might instead write something like this, without any String allocation:

#[error(formatter, {
    write!(formatter, "{code}")?;
    if let Some(msg) = message {
        write!(formatter, " - {msg}")?;
    }
})]
SungrowError { code: u16, message: Option<String> },

or possibly this:

#[error("{}", |formatter| {
    write!(formatter, "{code}")?;
    if let Some(msg) = message {
        write!(formatter, " - {msg}")?;
    }
})]
SungrowError { code: u16, message: Option<String> },

Unclear whether either of these is that much better than:

#[error("{}", {
    struct Msg<'a>(&'a u16, &'a Option<String>);
    impl Display for Msg<'_> {
        fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
            write!(formatter, "{}", self.0)?;
            if let Some(msg) = self.1 {
                write!(formatter, " - {msg}")?;
            }
            Ok(())
        }
    }
    Msg(.code, .message)
})]
SungrowError { code: u16, message: Option<String> },

or, using an adapter around Fn(&mut fmt::Formatter) -> fmt::Result to trim the boilerplate:

#[error("{}", DisplayFn(|formatter: &mut fmt::Formatter| {
    write!(formatter, "{}", .code)?;
    if let Some(msg) = .message {
        write!(formatter, " - {msg}")?;
    }
    Ok(())
}))]
SungrowError { code: u16, message: Option<String> },
struct DisplayFn<T>(T);

impl<T: Fn(&mut fmt::Formatter) -> fmt::Result> Display for DisplayFn<T> {
    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
        (self.0)(formatter)
    }
}

dtolnay avatar Oct 20 '22 17:10 dtolnay

For a concrete motivation for this request, I have the following definition:

pub mod api {
    #[derive(Error, Debug)]
    pub enum Error {
        #[error("specified server {0} not found")]
        ServerNotFound(String),
...
        #[error("expected {0} elements, got {1}")]
        UnexpectedData(usize, usize, Box<dyn DebugT>),
        #[error("{0}")]
        InsufficientData(String, Option<Box<dyn DebugT>>),
}

later, I use it in cases where I want to see the whole thing that lacks the inner thing:

...
            let v = self.get_foo().await?;

            v.bar.ok_or_else(|| {
                app_err::Error::InsufficientData(
                    "no bar in response".to_string(),
                    Some(Box::new(v)),
                )
            })
...

and also in places where there simply isn't a thing at all to see:

...
            let v: Option<Vec<Foo>> = outer.pop().unwrap().foo;
            let mut v: Vec<Foo> = v.ok_or_else(|| {
                app_err::Error::InsufficientData("no Foo vec found".to_string(), None)
            })?;
...

thenewwazoo avatar Nov 03 '22 22:11 thenewwazoo

Personally, I'd prefer something like #[error(with = fn)], where fn is Fn(..., &mut fmt::Formatter) -> fmt::Result, with ... being references to the fields of the enum variant.

#[derive(Error, Debug)]
pub enum DummyError {
    #[error(with = display_what_happened)]
    WhatHappened(io::Error, PathBuf),
}

fn display_what_happened(err: &io::Error, path: &PathBuf, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
    todo!()
}

Personally I prefer this version, as while all the others are nice. They basically instantly break auto-formatting.

vallentin avatar Mar 10 '24 04:03 vallentin