Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Feature Request: Support for [serde(try_into = "type", try_from = "type")] #1524

Closed
fanzeyi opened this issue May 12, 2019 · 2 comments
Closed

Comments

@fanzeyi
Copy link
Contributor

fanzeyi commented May 12, 2019

A little background, I'm trying to make toml work with one of the enum I have (toml does not work with enums). I discovered the serde(into = "type", from = "type") attributes, and it works perfectly with my case. However, I do want to have the ability to handle unexpected input when serializing/de-serializing the enum.

I think it would be nice if serde could support a similar attribute try_into and try_from. An example could be:

use std::convert::TryFrom;

#[derive(Debug, Clone, Deserialize)]
#[serde(try_from = "String")]
enum Color {
    Rgb(u8, u8, u8)
}

impl TryFrom<String> for Color {
    type Error = String;

    fn try_from(value: String) -> Result<Self, Self::Error> {
        // ... manually parsing string
        Err("Some error message".to_string())
    }
}

One complication of supporting try_into and try_from is error handling. I'm not familiar with implementing serializer and deserializer with serde but I saw there is Error::Message in the example of implementing a data format. I wonder if it possible to have the user to return a String error message and passed that to the actual seralizer/deserializer etc.

Please let me know what do you think about this idea. Thanks!

@fanzeyi fanzeyi changed the title Support for TryFrom / TryInto Feature Request: Support for [serde(try_into = "type", try_from = "type")] May 12, 2019
@dtolnay
Copy link
Member

dtolnay commented May 12, 2019

I would accept a PR for a try_from attribute. I would prefer not to do try_into yet until someone justifies a use case for it. The existing into attribute is already sort of confusing with the way it needs to clone self.

@davidbarsky
Copy link

Just wanted to check—I think this issue could be closed because #1526 is merged?

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Development

No branches or pull requests

3 participants