-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Use split_once in FromStr docs #99335
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (or someone else) soon. Please see the contribution instructions for more information. |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
Removing unwrap is indeed overkill but I'd like to have |
I've updated the example to use @JohnTitor I am not sure what the proper process here is, can I change the label back to |
@rustbot label -S-waiting-on-author +S-waiting-on-review |
Yeah, you're right, you could also use the shorthand described in https://github.com/rust-lang/triagebot/wiki/Shortcuts. @bors r+ rollup |
Use split_once in FromStr docs Current implementation: ```rust fn from_str(s: &str) -> Result<Self, Self::Err> { let coords: Vec<&str> = s.trim_matches(|p| p == '(' || p == ')' ) .split(',') .collect(); let x_fromstr = coords[0].parse::<i32>()?; let y_fromstr = coords[1].parse::<i32>()?; Ok(Point { x: x_fromstr, y: y_fromstr }) } ``` Creating the vector is not necessary, `split_once` does the job better. Alternatively we could also remove `trim_matches` with `strip_prefix` and `strip_suffix`: ```rust let (x, y) = s .strip_prefix('(') .and_then(|s| s.strip_suffix(')')) .and_then(|s| s.split_once(',')) .unwrap(); ``` The question is how much 'correctness' is too much and distracts from the example. In a real implementation you would also not unwrap (or originally access the vector without bounds checks), but implementing a custom Error and adding a `From<ParseIntError>` and implementing the `Error` trait adds a lot of code to the example which is not relevant to the `FromStr` trait.
Rollup of 8 pull requests Successful merges: - rust-lang#97183 (wf-check generators) - rust-lang#98320 (Mention first and last macro in backtrace) - rust-lang#99335 (Use split_once in FromStr docs) - rust-lang#99347 (Use `LocalDefId` in `OpaqueTypeKey`) - rust-lang#99392 (Fix debuginfo tests.) - rust-lang#99404 (Use span_bug for unexpected field projection type) - rust-lang#99410 (Update invalid atomic ordering lint) - rust-lang#99434 (Fix `Skip::next` for non-fused inner iterators) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Current implementation:
Creating the vector is not necessary,
split_once
does the job better.Alternatively we could also remove
trim_matches
withstrip_prefix
andstrip_suffix
:The question is how much 'correctness' is too much and distracts from the example. In a real implementation you would also not unwrap (or originally access the vector without bounds checks), but implementing a custom Error and adding a
From<ParseIntError>
and implementing theError
trait adds a lot of code to the example which is not relevant to theFromStr
trait.