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

Adds support for the default attr to tuple variants in enums. #1442

Merged
merged 1 commit into from Dec 11, 2018

Conversation

tcr
Copy link
Contributor

@tcr tcr commented Dec 11, 2018

This allows the following to work:

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub enum DOM {
    Text(
        String,
        #[serde(default, skip_serializing_if = "HashMap::is_empty")] HashMap<...>), // optional data
    ),
    Element(...),
    Comment(...),
}

DOM::Text("...", HashMap::new()) becomes encoded as an adjacently tagged enum tuple variant, but because of the skip_serializing_if parameter we may omit the second field of the variant when serializing, if the condition is true. When deserializing, at the moment, serde complains that it was expecting a tuple of two types, but found 1. The new behavior after this PR is if the default attr was used, Default::default() (or a program-supplied method) is called to populate that value.

This survives the round trip for me with both serde_json and ron.

@tcr tcr changed the title Adds support for the default attr to newtype variants in enums. Adds support for the default attr to tuple variants in enums. Dec 11, 2018
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

@dtolnay dtolnay merged commit 65705e2 into serde-rs:master Dec 11, 2018
@dtolnay
Copy link
Member

dtolnay commented Dec 11, 2018

Added some tests in 8b4074e and released in 1.0.82.

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

Successfully merging this pull request may close these issues.

None yet

2 participants