-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add grammar in docs for {f32,f64}::from_str, mention known bug. #56217
Conversation
frewsxcv
commented
Nov 25, 2018
•
edited
Loading
edited
- Original bug about documenting grammar
- formalize FromStr syntax #32243
- Known bug with parsing
- Float parsing can fail on valid float literals #31407
r? @rust-lang/docs @rust-lang/libs |
/// [0-9]+)?) | ||
/// ``` | ||
/// | ||
/// # Known bugs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Historically, we haven't mentioned known bugs in the docs, but considering this known bug has existed for years and people keep hitting it, I think it's worth calling it out here.
b8b884c
to
d1c1978
Compare
- Original bug about documenting grammar - rust-lang#32243 - Known bug with parsing - rust-lang#31407
d1c1978
to
1798d51
Compare
FWIW this isn't the first instance of documenting a known issue, and I think while we don't really have a policy around this the defacto policy is that "if someone cares enough to document the issue while not being willing to fix it, then seems fine to document". |
I'm not a fan of including a grammar or regular expression like that, especially in the std docs. We've gradually been moving any kind of grammar into the Reference, so i would consider that a much better place for that. As far as documenting known issues, i'm okay with it, but i think it could be better phrased: # Known issues
In some situations, some strings that should create a valid float instead return an
error. See [issue #31407] for details. |
The point of this grammar is to document valid input formats for this particular standard library API. So I can get a better understanding of what you're envisioning, do you have any examples of sections in the Reference that document specific standard library APIs?
Moving the Rust language syntax grammar to the Reference makes sense to me since it's documenting the behaviors of the language itself, as opposed to a specific standard library API
Thanks for the suggestion! I'll update it to reflect your wording |
There's a "Special types and traits" page that seems to document some of the more-common items from the standard library. But i think i get your point. Since this is library code, it makes sense to document it here. (Though i'm not sure if we ever got together a better documentation of all type conversion functionality than the There is a grammar for floating-point literals in the reference; does this match the regular expression you've entered there? I guess it wouldn't include the type suffix, but i think everything else matches? |
A few other differences come to mind:
|
Also looks like float literals can have a capital |
cc @rust-lang/wg-grammar - I think some people in there aren't in the docs group on github. |
@QuietMisdreavus I'm not sure what the ping (wg-grammar) was for; can you elaborate? |
@Centril We talked about it on Discord, but my question was effectively "does the grammar WG care about the grammar of library functions", which turns out to be "not officially". @frewsxcv Coming back to this, it's fine to have it here, but i find the regex a little hard to read. Can you convert it to BNF? I think that's a little easier to decode at a glance if you're not familiar with the syntax. |
Here's my attempt at writing out the grammar using W3C's EBNF syntax:
It can be visualized here. Thoughts? Is this more clear than the regex? |
Co-Authored-By: frewsxcv <[email protected]>
Picking someone from T-libs randomly... r? @Kimundi |
Anyone on the libs team have any concerns about this? |
ping from triage. @Kimundi have you had time to review? |
I think this looks good, thanks so much! Barring any objections from @rust-lang/libs, i think this is ready to go. I'll go ahead and push this forward, but feel free to back it out if there's a problem. @bors r+ rollup |
📌 Commit 8af02fa has been approved by |
…ietMisdreavus Add grammar in docs for {f32,f64}::from_str, mention known bug. - Original bug about documenting grammar - rust-lang#32243 - Known bug with parsing - rust-lang#31407
Rollup of 5 pull requests Successful merges: - #56217 (Add grammar in docs for {f32,f64}::from_str, mention known bug.) - #57294 (When using value after move, point at span of local) - #57652 (Update/remove some old readmes) - #57802 (Print visible name for types as well as modules.) - #57865 (Don't ICE when logging unusual types) Failed merges: r? @ghost