Skip to content
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

Extract Display derive into separate reusable crate #250

Closed
drmason13 opened this issue Aug 22, 2023 · 2 comments
Closed

Extract Display derive into separate reusable crate #250

drmason13 opened this issue Aug 22, 2023 · 2 comments

Comments

@drmason13
Copy link

I stumbled upon #65 while searching for a way to derive Display using this crate without deriving Error at all. The Display impl and #[error(...)] syntax is just so good and convenient, I want to use it all over the place. I've seen dispaydoc but I'm not keen on using doc strings for this.

To borrow your ascii diagram from #65...

----------------------------------------------------------------------------------->
derive(Error)    derive(Error)  OR  derive(Display)    derive(Error, Display)
#[error(...)]    #[error(...)]      #[display(...)]    #[error(...)] + #[display(...)]

     ^ what most people use        ^ what I am wishing for:                  ^
                                     * use the Display impl from thiserror   |
                                     * no impl of Error                      |
                                                                            /
                                                     where we actually are: 
                                                      * thiserror Error impl
                                                      * 3rd Party Display impl

I think it would go something like this:

  • break the Display derive out into its own crate
  • make thiserror depend on that new crate.
  • add #[display(...)] to power the Display derive instead of #[error(...)] if #[derive(Error)] is not present
  • reexport the Derive macro from the new crate in thiserror. (this I expect is the most iffy?)

Then everyone can pick and choose whether to derive Error or just derive Display and those implementations are always compatible. Plus there's a nice new but familiar Display derive crate anyone can use.

I'll try and make a branch that simply cuts out the Error derive as a little experiment. See how I get on.

@mzabaluev
Copy link

I don't think there necessarily needs to be another crate for this, there are macro crates for general-purpose Display derives on crates.io already. But exposing the Display derive specifalized for "errory" values would be helpful.

I see two use cases for this:

  1. Until error_in_core is stabilized, this could be convenient for conditional no_std support that does not require fiddling with optional dependencies in Cargo.toml:
#[derive(Debug)]
#[cfg_attr(feature = "std", derive(thiserror::Error))]
#[cfg_attr(not(feature = "std"), derive(thiserror::Display))]
enum Error {
    #[error("failed because X")] // works for the Display derive as well
    CauseX,
    // ...
}

This Display derive would also honor #[from], so you get the same conversion impls generated.

  1. In the common design pattern where the error value has a "kind" enum member, the enum itself should not be an Error, but it may be useful to derive Display on it with the convenience of error attributes.

@dtolnay
Copy link
Owner

dtolnay commented Apr 16, 2024

I appreciate the suggestions, but this is not something I would want to support in this crate (or as its own crate for Display). I'd like to keep this crate as focused around std::error::Error as possible.

@dtolnay dtolnay closed this as completed Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants