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

Add configuration option for writing enum variants as strings #214

Merged
merged 3 commits into from
Sep 1, 2019

Conversation

daboross
Copy link
Collaborator

@daboross daboross commented Sep 1, 2019

Submitting this after #207 was merged.

Adds a config option for encoding enums variant names as strings. Does not change any defaults.

Notably, this does not change rmp_serde::to_vec_named. That remains encoding struct fields names as strings, but enum variants as integers.

Fixes #172. Supersedes and closes #154.

I'm not 100% sure why this code was here, which is why I'm glad we have
pull requests for reviewing. It limited enum variants to u32, though,
so this allows it to now decode encoded string variants successfully.
@kornelski kornelski merged commit 48263dd into 3Hren:master Sep 1, 2019
@daboross daboross deleted the v2-variant-string-fix branch September 2, 2019 00:33
@untitaker
Copy link

untitaker commented Dec 3, 2019

For c-style enums this serializes quite poorly.

enum Foo {
   A,
   B,
   C
}

Resulting msgpack is {"A": None} where I expected "A". Can this be fixed before release or does this need another option, were I to open a PR?

It generally seems like rmp's output cannot really be consumed from other languages.

@kornelski
Copy link
Collaborator

Yeah, it'd be good to optimize it for fieldless enums.

@daboross
Copy link
Collaborator Author

daboross commented Dec 5, 2019

serde_json does this just by special casing serialize_unit_variant! https://github.com/serde-rs/json/blob/1977e693491745dc0ab3a153d53366eaafbc7aba/src/ser.rs#L883-L890

I think it should be fairly easy for us to adopt the same approach if we also modify the deserializer to accept this form of enum.

Edit: opened #225.

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.

rmp_serde: Open to optionally tagging enums with names?
3 participants