Skip to content

Implement Error and Display traits for FromStrError #610

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

Merged
merged 1 commit into from
Dec 20, 2022

Conversation

raccog
Copy link
Contributor

@raccog raccog commented Dec 18, 2022

For issue #594.

I implemented the Error trait in the same way that it's implemented for uefi::Error.

I'm not sure where the Error trait is useful, so I haven't tested it beyond running the unit/qemu tests. :P I'm curious, though, what can this be used for?

Please let me know if this looks like a good solution for #594 and I'll implement these traits for other error types, as well.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@phip1611
Copy link
Member

I implemented the Error trait in the same way that it's implemented for uefi::Error.

Very good, this is a good contribution! thanks!

I'm not sure where the Error trait is usefu

If you want to integrate the errors with the anyhow crate, for example.

f,
"UCS-2 Conversion Error: {}",
match self {
Self::InvalidChar => "Invalid character",
Copy link
Member

Choose a reason for hiding this comment

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

@nicholasbishop I think we should find a solution to guarantee consistency. Either we use derive_more::Display for errors or the display impl should fall back to the Debug impl. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I think having Debug and Display be different is fine, with the general convention that Debug should be a bare-bones representation of the type, and Display should be human readable.

For example, in Rust's std, Utf8Error follows this pattern. It has derive(Debug) and an impl Derive that manually provides a more human-readable string.

Copy link
Member

Choose a reason for hiding this comment

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

ok

@raccog
Copy link
Contributor Author

raccog commented Dec 19, 2022

If you want to integrate the errors with the anyhow crate, for example.

Thanks for the tip, that crate looks interesting!

@phip1611 phip1611 merged commit f37f469 into rust-osdev:main Dec 20, 2022
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.

3 participants