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

ToString Keep/Rename/Remove #132

Closed
Peternator7 opened this issue Oct 7, 2020 · 3 comments · Fixed by #180
Closed

ToString Keep/Rename/Remove #132

Peternator7 opened this issue Oct 7, 2020 · 3 comments · Fixed by #180

Comments

@Peternator7
Copy link
Owner

Evaluating the naming scheme and value of all the macros used in strum. Some are more or less redundant such as ToString now that Display exists.

This is a tracking issue to solicit feedback about the usefulness of ToString and whether it should be kept and if so, what it's name should be.

@Peternator7
Copy link
Owner Author

Generally, Display should be preferred over ToString, but ToString has been kept around for compatibility. Given it's probably the wrong thing to do, I would propose removing this macro all together.

@linclelinkpart5
Copy link
Contributor

I'm in favor of this change, it would mirror what Rust's std is aiming for as well.

@b-zee
Copy link

b-zee commented Jan 21, 2022

@Peternator7 Not sure if this is worth a separate issue, but there might be a reason to keep ToString around.

I assume it is not uncommon for people to use strum_macros in combination with thiserror. Using both leads to a conflict as both derives end up implementing Display (?):

#[derive(thiserror::Error, strum_macros::Display)]
enum Error {
    #[error("My custom error")]
    CustomError,
}
error[E0119]: conflicting implementations of trait `std::fmt::Display` for type `Error`

I assume strum_macros::ToString will be circumventing this problem, but it is deprecated. ToString leads to a similar error, because ToString is implemented for any Display.

Edit: I guess I didn't quite think this through. Basically thiserror is all about Display too, so it doesn't make much sense to mix both libraries, I guess.
A little background of my 'problem': I want to relay Error diagnostics into an identifiable string (like 'CustomError') and have a description. But there can only be a single conversion method to String; so it's either the short name or the longer description.
With both a conversion to String and &'static str there is the possibility to get two different representations though, so that might be an option. Otherwise it might be required to go with some kind of custom type/trait that will print based on a match with arms per variant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants