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

EnumIs: Allow custom variant function names #336

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Mange
Copy link

@Mange Mange commented Feb 28, 2024

I have an enum that basically looks like this:

#[derive(EnumIs)]
enum Value {
  U8(u8),
  String(String),
  Boolean(bool),
}

When deriving EnumIs for this enum I get is_u_8, which I want to replace with is_u8 instead. This PR implements the following workaround:

#[derive(EnumIs)]
enum Value {
  #[strum(name = "u8")]
  U8(u8),
  String(String),
  Boolean(bool),
}

This could still be useful for other cases, perhaps one want to rename Boolean to is_bool, for example.

I've also fixed warnings that were emitted while running tests, and some Clippy warnings in the enum_is tests file.

@Mange
Copy link
Author

Mange commented Mar 27, 2024

Is there anything else I can do here to help out? :-)

@Peternator7
Copy link
Owner

Hi @Mange, sorry for the delay here. It's been a busy few weeks. PR looks generally good, and makes sense to me. Two quick things:

  1. Could you add the same ability to try_as_*? It seems like a natural extension for symmetry
  2. I like the brevity of name, but I think it's going to be a little too confusing that it doesn't change the Display (nor do I think it should). I'm having a difficult time thinking of something brief and descriptive, so maybe something like generated_functions_name or preferred_name_in_fns. Very open to ideas here 😅

@Mange
Copy link
Author

Mange commented Apr 15, 2024

Serde had a similar thing with rename. You could use #[serde(rename = "other_name")] and it would affect both serialization and deserialization, oruse #[serde(rename = (deserialize = "original_name", serialize = "new_name"))] to have different names in different derives. [Doc]

Perhaps we could do something similar to this here, either:

#[strum(name = "other_name")] // uses the name for all derives.
#[strum(name = (enum_is = "other_name")] // uses default name for other derives.

// …or, to make configuration appear more scoped, only allow `name` to be for an
// explicit derive kind.
#[strum(enum_is = (name = "other_name"))]

@Peternator7
Copy link
Owner

Sorry for the delay getting back to you. I've been thinking a good bit about this. It's a big change, but I think adopting the same convention as serde is reasonable, and I'd be willing to start the migration. If we do, let's just use the same language that serde uses (aka rename and alias) rather than name.

This is going to be a bigger change, but will likely be good for the long term consistency of strum and probably any apps that use both strum and serde

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