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

Use strum to replace manual implementation of as_str and FromStr #487

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

crazytonyli
Copy link
Contributor

Here are the two core changes:

  1. Use strum_macros::EnumString derive macro to replace our own FromStr implementation.
  2. Use strum_macros::Display derive macro to replace our own as_str implementation. The macro adds std::fmt::Display implementation to the attached enums.

Breaking change

The enums now no longer have the as_str function. Instead, they have to_string.

Do not use strum_macros::AsRefStr

strum also has a derive macro AsRefStr to add fn as_ref() -> AsRef<str> to the attached enums. However, the function does not match the to_string implementation generated by strum_macros::Display macro. The Custom(String) variant returns "Custom", instead of the String value in the tuple.

wp_api/src/lib.rs Outdated Show resolved Hide resolved
Base automatically changed from search-results to trunk January 16, 2025 06:17
Copy link
Contributor

@oguzkocer oguzkocer left a comment

Choose a reason for hiding this comment

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

@crazytonyli I left one important comment about #[strum(serialize = "{value}")], but otherwise this looks good to me.

wp_api/src/search_results.rs Show resolved Hide resolved
wp_api/src/lib.rs Outdated Show resolved Hide resolved
wp_api/src/lib.rs Outdated Show resolved Hide resolved
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