Skip to content

Expand docs on Peekable::peek_mut#79478

Merged
bors merged 1 commit intorust-lang:masterfrom
lukaslueg:peek_mut_docs
Nov 28, 2020
Merged

Expand docs on Peekable::peek_mut#79478
bors merged 1 commit intorust-lang:masterfrom
lukaslueg:peek_mut_docs

Conversation

@lukaslueg
Copy link
Copy Markdown
Contributor

@lukaslueg lukaslueg commented Nov 27, 2020

Slightly expand docs on std::iter::Peekable::peek_mut, tracked in #78302

r? @m-ou-se

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 27, 2020
@m-ou-se
Copy link
Copy Markdown
Member

m-ou-se commented Nov 27, 2020

Thanks!

I think the if let Some(p @ &mut &2) syntax might be a bit too advanced/unknown for a basic example. Maybe a regular if let Some(p) followed by something like an assert_eq!(*p, &2) would be better. What do you think?

@lukaslueg
Copy link
Copy Markdown
Contributor Author

lukaslueg commented Nov 27, 2020

I thought about that too and decided on the nested if in the first PR. Now I changed my mind because afaics we are encouraging people to use features which prevent nesting (e.g. match ... Some(n) => { if n > 0 {). The comment, therefore, says "and match on the next value.".

No strong opinion, poke at me and I'll change to an assert, which equally reflects what's going on.

@m-ou-se
Copy link
Copy Markdown
Member

m-ou-se commented Nov 27, 2020

I feel like the original one (the two ifs) was also fine. if and and assert_eq!() would be fine. Or even just .unwrap() would be fine. But I think it's better if we avoid the p @ &mut &2 syntax, as it distracts a bit from what the example is trying to show, since most people will be unfamiliar with it.

@lukaslueg
Copy link
Copy Markdown
Contributor Author

lukaslueg commented Nov 27, 2020

agreed

@m-ou-se
Copy link
Copy Markdown
Member

m-ou-se commented Nov 27, 2020

@bors r+ rollup

@bors
Copy link
Copy Markdown
Collaborator

bors commented Nov 27, 2020

📌 Commit 08ec201 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 27, 2020
@jonas-schievink jonas-schievink mentioned this pull request Nov 28, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 28, 2020
@bors
Auto merge of rust-lang#79507 - jonas-schievink:rollup-e5yeayh, r=jon…
e37f25a 
…as-schievink

Rollup of 10 pull requests

Successful merges:

 - rust-lang#78086 (Improve doc for 'as _')
 - rust-lang#78853 (rustc_parse: fix ConstBlock expr span)
 - rust-lang#79234 (Resolve typedefs in HashMap gdb/lldb pretty-printers)
 - rust-lang#79344 (Convert UNC path to local path to satisfy install script on Windows)
 - rust-lang#79383 (Fix bold code formatting in keyword docs)
 - rust-lang#79460 (Remove intermediate vectors from `add_bounds`)
 - rust-lang#79474 (Change comments on types to doc-comments)
 - rust-lang#79476 (Sync rustc_codegen_cranelift)
 - rust-lang#79478 (Expand docs on Peekable::peek_mut)
 - rust-lang#79486 (Slightly improve code samples in E0591)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 248e5ad into rust-lang:master Nov 28, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 28, 2020
@lukaslueg lukaslueg deleted the peek_mut_docs branch November 28, 2020 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@m-ou-se m-ou-se

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.50.0

Development

Successfully merging this pull request may close these issues.

5 participants

@lukaslueg @m-ou-se @bors @rust-highfive @rustbot