-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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 example for iterator_flatten #105034
Add example for iterator_flatten #105034
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
@rustbot label +A-docs |
@@ -1495,6 +1495,14 @@ pub trait Iterator { | |||
/// assert_eq!(merged, "alphabetagamma"); | |||
/// ``` | |||
/// | |||
/// Flattening also works on other types like Option and Result: |
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.
It's not really specific to Option
or Result
though. It's that Flatten
doesn't require Item: Iterator
, it only needs Item: IntoIterator
and Option/Result are types that implement that.
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.
I think i do not really understand what you mean, should we not add this example should i change it?
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.
I'm pointing out that "types like Option and Result" is very vague. It doesn't tell the reader which those types have in common so they can be used with Flatten. I don't have a particular improvement in mind, I was just pointing out the vagueness and how it actually works.
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.
It seems like we already document that "iterator of iterators or an iterator of things that can be turned into iterators" above -- I'm not sure another example is all that useful. I think we have enough examples personally for flatten.
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.
I think this is okay, although its placement is interrupting the "you can also rewrite this" example below, so we should move this down. For wording, perhaps:
/// Flattening also works on other types like Option and Result: | |
/// Flattening works on any `IntoIterator` type, including `Option` and `Result`: |
Although we're not showing Result
, so maybe do so too, or remove that part.
r? @cuviper since you seemed positive about the original bug report, maybe you have thoughts on how to phrase this well. |
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.
Your new example is still interrupting the two "alphabetagamma" examples -- please move yours down, and then I think this looks good!
@rustbot label -S-waiting-on-review +S-waiting-on-author |
@rustbot label +S-waiting-on-review -S-waiting-on-author |
@bors r+ rollup |
…flatten_doc, r=cuviper Add example for iterator_flatten Adds an Example to iterator_flatten Fixes rust-lang#82687
Rollup of 9 pull requests Successful merges: - rust-lang#105034 (Add example for iterator_flatten) - rust-lang#105708 (Enable atomic cas for bpf targets) - rust-lang#106175 (Fix bad import suggestion with nested `use` tree) - rust-lang#106204 (No need to take opaques in `check_type_bounds`) - rust-lang#106387 (Revert "bootstrap: Get rid of `tail_args` in `stream_cargo`") - rust-lang#106636 (Accept old spelling of Fuchsia target triples) - rust-lang#106639 (update Miri) - rust-lang#106640 (update test for inductive canonical cycles) - rust-lang#106647 (rustdoc: merge common CSS for `a`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Adds an Example to iterator_flatten
Fixes #82687