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.

Sign up for GitHub

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

Jump to bottom

Proposal to add Peekable::peek_mut #77491

Merged
merged 1 commit into from
Nov 25, 2020
Merged

Proposal to add Peekable::peek_mut #77491

merged 1 commit into from
Nov 25, 2020

Conversation

lukaslueg
Copy link
Contributor

A "peekable" iterator has a peek()-method which provides an immutable reference to the next item. We currently do not have a method to modify that item, which we could easily add via a peek_mut(). See the test for a use-case (alike to my original use case), where a "pristine" iterator is passed on after modifying its state via peek_mut().

If there is interest in this, I can expand on the tests and docs.

@rust-highfive
Copy link
Collaborator

r? @kennytm

(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 Oct 3, 2020
@jyn514 jyn514 added A-iterators Area: Iterators T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 3, 2020
@jyn514
Copy link
Member

jyn514 commented Oct 3, 2020

What's the use case for adding this?

@lukaslueg
Copy link
Contributor Author

lukaslueg commented Oct 3, 2020 •
edited by jyn514
Loading

I've a iterator of BufRead where I peek into the buffer via fill_buf, which requires mutable access to fill the buffer.

@timvermeulen
Copy link
Contributor

+1, I've been wanting to add this 🙂

@lukaslueg lukaslueg marked this pull request as draft October 4, 2020 13:17
@Dylan-DPC-zz
Copy link

@lukaslueg can you open an issue regarding this? once that's done update issue=none to the issue number and we can see what's next after that. Thanks

@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2020
@lukaslueg lukaslueg mentioned this pull request Oct 23, 2020
Closed
2 tasks
@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 17, 2020
@Dylan-DPC-zz
Copy link

@rust-lang/libs would like your thoughts on this proposal so we can move forward with this

@dtolnay
Copy link
Member

dtolnay commented Nov 21, 2020

+1, I am on board with this.

@dtolnay dtolnay assigned dtolnay and unassigned kennytm Nov 21, 2020
@lukaslueg
Copy link
Contributor Author

Expanded docs, similar to peek(), rebased

@jyn514
Copy link
Member

jyn514 commented Nov 22, 2020 

tidy error: /checkout/library/core/src/iter/adapters/mod.rs: too many lines (3004) (add `// ignore-tidy-filelength` to the file to suppress this error)

I'd add ignore-tidy-filelength for now and it can be split up later at some point.

@lukaslueg lukaslueg marked this pull request as ready for review November 22, 2020 19:05
@bors
Copy link
Contributor

bors commented Nov 23, 2020

☔ The latest upstream changes (presumably #79319) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@lukaslueg
Copy link
Contributor Author

lukaslueg commented Nov 23, 2020 •
edited
Loading

Rebased due to #77697. Note that I went light on adding new tests for peek_mut since it's implementation is identical to peek().

@m-ou-se
Copy link
Member

m-ou-se commented Nov 24, 2020

Looks like nobody is against adding this (as unstable). :)

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 24, 2020

📌 Commit 3b01562 has been approved by m-ou-se

@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-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Nov 24, 2020
@bors
Copy link
Contributor

bors commented Nov 25, 2020

⌛ Testing commit 3b01562 with merge b387f62...

@bors
Copy link
Contributor

bors commented Nov 25, 2020

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing b387f62 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 25, 2020
@bors bors merged commit b387f62 into rust-lang:master Nov 25, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 25, 2020
@lukaslueg lukaslueg deleted the peek_mut branch November 25, 2020 08:43
@m-ou-se m-ou-se mentioned this pull request Dec 14, 2020
Merged
@lukaslueg lukaslueg mentioned this pull request Feb 9, 2021
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 17, 2021
@Dylan-DPC
Rollup merge of rust-lang#81938 - lukaslueg:stab_peek_mut, r=KodrAus,…
03abd47 
…JohnTitor

Stabilize `peekable_peek_mut`

Resolves rust-lang#78302. Also adds some documentation on `std::iter::Iterator::peekable()` regarding the new method.

The feature was added in rust-lang#77491 in Nov' 20, which is recently, but the feature seems reasonably small. Never did a stabilization-pr, excuse my ignorance if there is a protocol I'm not aware of.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 8, 2021
@Dylan-DPC
Rollup merge of rust-lang#81938 - lukaslueg:stab_peek_mut, r=Amanieu
5f6fb2c 
Stabilize `peekable_peek_mut`

Resolves rust-lang#78302. Also adds some documentation on `std::iter::Iterator::peekable()` regarding the new method.

The feature was added in rust-lang#77491 in Nov' 20, which is recently, but the feature seems reasonably small. Never did a stabilization-pr, excuse my ignorance if there is a protocol I'm not aware of.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 8, 2021
@Dylan-DPC
Rollup merge of rust-lang#81938 - lukaslueg:stab_peek_mut, r=Amanieu
461297e 
Stabilize `peekable_peek_mut`

Resolves rust-lang#78302. Also adds some documentation on `std::iter::Iterator::peekable()` regarding the new method.

The feature was added in rust-lang#77491 in Nov' 20, which is recently, but the feature seems reasonably small. Never did a stabilization-pr, excuse my ignorance if there is a protocol I'm not aware of.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@dtolnay dtolnay

Labels
A-iterators Area: Iterators merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.50.0
Development

Successfully merging this pull request may close these issues.
10 participants
@lukaslueg @rust-highfive @jyn514 @timvermeulen @Dylan-DPC-zz @dtolnay @bors @m-ou-se @kennytm @rustbot