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

Clarify non-exact length in the Iterator::take documentation #83272

Merged
merged 1 commit into from
Mar 22, 2021

Conversation

kornelski
Copy link
Contributor

@kornelski kornelski commented Mar 18, 2021

There's an example which demonstrates incomplete length case, but it'd be best to explain it right from the start.

@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 18, 2021
@steffahn
Copy link
Member

steffahn commented Mar 18, 2021

I don’t feel like this is as accurate as it could be. It only says that “fewer” than n elements are returned “if the underlying iterator ends sooner”; both the condition and the result could be more clear.

Also the first line summary no longer indicates that a prefix is returned.

Maybe something like

Creates an iterator that yields the first (up to) n elements.

take(n) yields elements until n elements are yielded or the end of the iterator is reached (whichever happens first). The returned iterator is a prefix of length n if the original iterator contains at least n elements, otherwise it contains all of the (fewer than n) elements of the original iterator.


We could also update skip along the way, currently saying

Creates an iterator that skips the first n elements.

After they have been consumed, the rest of the elements are yielded. Rather than overriding this method directly, instead override the nth method.

to something like

Creates an iterator that skips the first (up to) n elements.

skip(n) skips elements until n elements are skipped or the end of the iterator is reached (whichever happens first). After that, all the remaining elements are yielded. In particular, if the original iterator is too short, then the returned iterator is empty.

Rather than overriding this method directly, instead override the nth method.

If you haven’t noticed by now, I enjoy being precise. I’m not sure if my versions aren’t potentially confusing or too lengthy, so some feedback would be appreciated.


Edit: This part

In particular, if the original iterator is too short, then the returned iterator is empty.

might be better as an example,

also I do acknowledge that working on skip may very well be beyond the scope of what you want to do in this PR.

@kornelski
Copy link
Contributor Author

kornelski commented Mar 18, 2021

I've added the extended explanation. Injected "(up to)" looks jarring to me, so I've avoided it.

Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dtolnay
Copy link
Member

dtolnay commented Mar 21, 2021

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 21, 2021

📌 Commit 6cfdc38 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 21, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 22, 2021
Rollup of 7 pull requests

Successful merges:

 - rust-lang#82374 (Add license metadata for std dependencies)
 - rust-lang#82683 (Document panicking cases for integer division and remainder)
 - rust-lang#83272 (Clarify non-exact length in the Iterator::take documentation)
 - rust-lang#83338 (Fix test for rust-lang#82270)
 - rust-lang#83351 (post-drop-elab check-const: explain why we still check qualifs)
 - rust-lang#83367 (Improve error message for unassigned query provider)
 - rust-lang#83372 (SplitInclusive is public API)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f441c2a into rust-lang:master Mar 22, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 22, 2021
@kornelski kornelski deleted the takedocs branch March 23, 2021 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants