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

Guideline for which types need a FromStr impl #63

Open
dtolnay opened this Issue May 18, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@dtolnay
Copy link
Member

dtolnay commented May 18, 2017

Some types should have Display but not FromStr, for example most error types. What determines whether a type should support FromStr? Examples to generalize:

  • numbers
  • IpAddr / SocketAddr
  • Url
  • LogLevel

Also the FromStr impl should match the Display impl. Are there any exceptions to this?

@Nemo157

This comment has been minimized.

Copy link
Member

Nemo157 commented May 18, 2017

I would include any type that's likely to be passed via a command line flag/environment variable/non-structured config item. Having a generic way to parse values in all those places really makes getting the data you need into an application easy.

@Kixunil

This comment has been minimized.

Copy link

Kixunil commented May 31, 2017

Are there any exceptions to this?

URL encoding? %41 should be decoded as A but it can be encoded as A.

@Nemo157

This comment has been minimized.

Copy link
Member

Nemo157 commented Jun 1, 2017

So:

  • If both a FromStr impl and Display impl are provided, FromStr should parse the output of Display and give back an equivalent object (x.to_string().parse() = Ok(x) ∀ x: FromStr + Display). This does not imply the inverse, FromStr will commonly be a lossy conversion, e.g. "0001".parse::<u32>() == Ok(1).

Although, there are still at least two stdlib types that might be an exception to this: f32 and f64. Looking at their documentation I can't find any info on whether they guarantee round-tripping through their default Display output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.