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

impl std::str::FromStr #3

Open
yoshuawuyts opened this issue Oct 25, 2018 · 2 comments
Open

impl std::str::FromStr #3

yoshuawuyts opened this issue Oct 25, 2018 · 2 comments

Comments

@yoshuawuyts
Copy link
Contributor

yoshuawuyts commented Oct 25, 2018

Feature Request

Summary

Implement the std::str::fromStr trait for Manifest.

Motivation

It'd be great if the following could work:

let path = "./assets/manifest.webmanifest";
let manifest: Manifest = fs::read_to_string(path)?.parse()?;

This not only looks nice, but it removes the need for consumers to import serde and serde_json as peer dependencies to perform parsing.

Guide-level explanation

By implementing str::FromStr conversions can be done using .parse()?. We should add an example for this too to the README under ## Examples using a heading like: ### Read Manifest from Disk.

Reference-level explanation

The from_str method should wrap around serde_json::from_str()? .

Drawbacks

None.

Rationale and alternatives

Alternatively we could implement TryInto to replace .parse()? with .try_into(). But that's currently not available on stable yet, so we can add it once it is.

Unresolved Questions

None.

@yoshuawuyts
Copy link
Contributor Author

Alternative usage:

let path = "./assets/manifest.webmanifest";
let manifest = fs::read_to_string(path)?.parse::<Manifest>()?;

But potentially slightly more confusing.

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

No branches or pull requests

1 participant