Skip to content

doc: some peek improvements #33265

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

Merged
merged 1 commit into from
Jul 7, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 9 additions & 11 deletions src/libcore/iter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1048,17 +1048,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
/// a value, it's wrapped in a `Some(T)`, but if the iterator is over, it
/// will return `None`.
/// Like [`next()`], if there is a value, it is wrapped in a `Some(T)`.
/// But if the iteration is over, `None` is returned.
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 [email protected], 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).

Copy link
Member Author

Choose a reason for hiding this comment

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

Sentence was rather long. Also, it doesn't sound ok to say "if the iterator is over".

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I agree "is over" is awkward wording. Hm.

///
/// [`next()`]: trait.Iterator.html#tymethod.next
///
/// Because `peek()` returns reference, and many iterators iterate over
/// references, this leads to a possibly confusing situation where the
/// Because `peek()` returns a reference, and many iterators iterate over
/// references, there can be a possibly confusing situation where the
/// return value is a double reference. You can see this effect in the
/// examples below, with `&&i32`.
/// examples below.
Copy link
Member Author

Choose a reason for hiding this comment

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

It should be clear where the double-references are

///
/// # Examples
///
Expand All @@ -1075,13 +1073,13 @@ impl<I: Iterator> Peekable<I> {
///
/// assert_eq!(iter.next(), Some(&2));
///
/// // we can peek() multiple times, the iterator won't advance
/// // The iterator does not advance even if we `peek` multiple times
/// 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()`
/// assert_eq!(iter.peek(), None);
/// assert_eq!(iter.next(), None);
/// ```
Expand Down Expand Up @@ -1113,10 +1111,10 @@ impl<I: Iterator> Peekable<I> {
///
/// let mut iter = xs.iter().peekable();
///
/// // there are still elements to iterate over
/// // There are still elements to iterate over
/// assert_eq!(iter.is_empty(), false);
///
/// // let's consume the iterator
/// // Let's consume the iterator
/// iter.next();
/// iter.next();
/// iter.next();
Expand Down