Implement FromStr for Val #16926
Implement FromStr for Val #16926
Conversation
| impl core::str::FromStr for Val { | ||
| type Err = ValParseError; | ||
|
|
||
| fn from_str(s: &str) -> Result<Self, Self::Err> { |
There was a problem hiding this comment.
Would love to see this pulled out as a const function if it's compatible. Not a blocker if it's not a simple change though.
There was a problem hiding this comment.
I checked and every single function that's being called is non-const 😆
There was a problem hiding this comment.
Specifically: str::trim, str::bytes(), iterator::position, str::split_at, ?, f32::parse are not const
There was a problem hiding this comment.
Damn, thanks for clarifying!
bushrat011899
added
C-Feature
| return Ok(Val::Auto); | ||
| } | ||
|
|
||
| let Some(end_of_number) = s |
There was a problem hiding this comment.
Only small nit with this PR is that this doesn't accommodate all of rust's valid f32 parsing inputs; I don't think people will often want to make values of 1e5 px or whatever, but it's possible that values like that may show up in automatically-formatted float output, for example.
There was a problem hiding this comment.
This doesn't support: NaN, inf, infinity, +inf, 1E4 and 1e5. I think most developers won't need this, and it can be added later
There was a problem hiding this comment.
Indeed. That's why I approved it :)
mweatherley
added
S-Ready-For-Final-Review
Objective
This PR implements
FromStrforVal, so developers can parse values like10pxand50%Testing
Added tests for this. I think they cover pretty much everything, and it's a fairly simple unit test.
Limitations
Currently the following float values are not parsed:
inf,-inf,+infinity,NaN2.5E10,2.5e10,2.5E-10For my use case this is perfectly fine but other developers might want to support these values