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

Replace DisplayAsDisplay and PathAsDisplay with AsDisplay trait #251

Merged
merged 1 commit into from
Sep 2, 2023

Conversation

mina86
Copy link
Contributor

@mina86 mina86 commented Sep 1, 2023

Rather than having separate traits implementing as_display method,
replace DisplayAsDisplay and PathAsDisplay traits with a AsDisplay
trait. The difference between those two traits is in the result
returned by the as_display method. With AsDisplay trait this is
captured by an associated type.

The main motivation for the change is making it simpler to support
no_std builds in the future. Previously, PathAsDisplay would have to
be handled specially in such builds on the side of macro
expansion. Now, thiserror-impl doesn’t need to be aware of any
complications around AsDisplay since they are all contained in
thiserror crate.

Rather than having separate traits implementing as_display method,
replace DisplayAsDisplay and PathAsDisplay traits with a AsDisplay
trait.  The difference between those two traits is in the result
returned by the as_display method.  With AsDisplay trait this is
captured by an associated type.

The main motivation for the change is making it simpler to support
no_std builds in the future.  Previously, PathAsDisplay would have to
be handled specially in such builds on the side of macro expansion.
Now, thiserror-impl doesn’t need to be aware of any complications
around AsDisplay since they are all contained in thiserror crate.
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dtolnay dtolnay merged commit f0c79cb into dtolnay:master Sep 2, 2023
9 checks passed
@mina86 mina86 deleted the b branch September 4, 2023 13:56
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

Successfully merging this pull request may close these issues.

2 participants