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

doc: some peek improvements #33265

Merged
merged 1 commit into from Jul 7, 2016
Merged

doc: some peek improvements #33265

merged 1 commit into from Jul 7, 2016

Conversation

tshepang
Copy link
Member

No description provided.

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@@ -1031,17 +1031,15 @@ impl<I: ExactSizeIterator> ExactSizeIterator for Peekable<I> {}
impl<I: Iterator> Peekable<I> {
/// Returns a reference to the next() value without advancing the iterator.
///
/// The `peek()` method will return the value that a call to [`next()`] would
/// return, but does not advance the iterator. Like [`next()`], if there is
Copy link
Member Author

Choose a reason for hiding this comment

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

Line removed for it was just repetition of the intro

Copy link
Member

Choose a reason for hiding this comment

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

It's repeated for a reason; you may be looking at just the method's definition, and not the introduction.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand... how does one look at doc body and not see the intro?

Copy link
Member

Choose a reason for hiding this comment

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

Output isn't just for HTML, eventually we will have things like "look up this method on the command line", which many languages have and many people ask for.

Copy link
Member Author

Choose a reason for hiding this comment

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

I expect that the intro is always going to appear... note that this is an intro for the method, not the module.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it is the intro for the method. The method's docs should fully describe it, even if it's also discussed as part of the module itself, or the trait.

Copy link
Member Author

@tshepang tshepang Jul 6, 2016

Choose a reason for hiding this comment

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

Not sure we understand each other... in what cases will the doc for a method appear without its intro? By intro I mean the first line that appears in the doc for the method itself, in this case:

   Returns a reference to the next() value without advancing the iterator. 

Copy link
Member

Choose a reason for hiding this comment

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

Ah ha, we are completely misunderstanding each other. So sorry! I agree with you here. I was confusing which part we were talking about; I thought you were concerned with repetition from the module-level or trait-level docs, not in the same doc bloc. And with these comments and the diff, I was confused.

Thank you for pushing back on this, I agree with you.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wish I could express meself better... I too often fail to find proper words to describe stuff.

Copy link
Member

Choose a reason for hiding this comment

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

It's all good. This was entirely me, not you. (Well, and partially github)

On Jul 6, 2016, 17:56 -0400, Tshepang Lekhonkhobenotifications@github.com, wrote:

Insrc/libcore/iter/mod.rs(#33265 (comment)):

@@ -1031,17 +1031,15 @@ impl<I: ExactSizeIterator>ExactSizeIterator for Peekable{}>impl<I: Iterator>Peekable{>/// Returns a reference to the next() value without advancing the iterator.>///>- /// The peek() method will return the value that a call to [next()] would>- /// return, but does not advance the iterator. Like [next()], if there is

I wish I could express meself better... I too often fail to find proper words to describe stuff.


You are receiving this because you were mentioned.
Reply to this email directly,view it on GitHub(https://github.com/rust-lang/rust/pull/33265/files/e01e4cc4dbf657e411ecc16498e5f5c33d55f4d0#r69819098), ormute the thread(https://github.com/notifications/unsubscribe/AABsitkkqlKwXboovivMQ3Rvssi1aCKcks5qTCSZgaJpZM4ISXbk).

@GuillaumeGomez GuillaumeGomez assigned steveklabnik and unassigned brson Apr 28, 2016
@GuillaumeGomez
Copy link
Member

cc @steveklabnik

/// assert_eq!(iter.peek(), Some(&&3));
/// assert_eq!(iter.peek(), Some(&&3));
///
/// assert_eq!(iter.next(), Some(&3));
///
/// // after the iterator is finished, so is peek()
/// // After the iterator is finished, so is `peek`
Copy link
Member

Choose a reason for hiding this comment

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

please keep the ()s at the end of the function name

@steveklabnik
Copy link
Member

Sorry for missing this somehow! Thanks for this. Lots of good changes, but I disagree with removing the big block at the start.

@steveklabnik
Copy link
Member

@tshepang ping! Can you please un-remove the first bit here so we can get this merged?

@tshepang
Copy link
Member Author

tshepang commented Jul 5, 2016

You did not yet address my comment there

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jul 6, 2016

📌 Commit db26b3f has been approved by steveklabnik

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 6, 2016
bors added a commit that referenced this pull request Jul 7, 2016
@bors bors merged commit db26b3f into rust-lang:master Jul 7, 2016
@tshepang tshepang deleted the peek branch July 7, 2016 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants