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: improve Drain examples #30272

Merged
merged 1 commit into from
Dec 18, 2015
Merged

doc: improve Drain examples #30272

merged 1 commit into from
Dec 18, 2015

Conversation

tshepang
Copy link
Member

@tshepang tshepang commented Dec 8, 2015

Second sentence actually repeats info from first sentence. "from start to end" also feels like it adds nothing.

I also extended Vec::drain example.

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@tshepang tshepang changed the title doc: trim redundant info on drain, and improve one example doc: trim redundant info on drain, and improve related examples Dec 8, 2015
@@ -1015,8 +1015,7 @@ impl String {
}

/// Create a draining iterator that removes the specified range in the string
/// and yields the removed chars from start to end. The element range is
/// removed even if the iterator is not consumed until the end.
Copy link
Member

Choose a reason for hiding this comment

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

The information about removal is good I think. You might otherwise think that only elements extracted from the iterator are removed from the String or Vec, or VecDeque.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bluss it's only the extracted elements that are removed

Copy link
Member

Choose a reason for hiding this comment

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

No it's not, I implemented this for Vec and String.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I understand you. Following does not panic:

let mut v = vec![1, 2, 3];
assert_eq!(vec![3], v.drain(2..).collect::<Vec<_>>());
assert_eq!(vec![1, 2], v);
v.drain(..);
assert!(v.is_empty());

Copy link
Member

Choose a reason for hiding this comment

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

ok, I mean that:

let mut v = vec![1, 2, 3];
v.drain(..).take(1).collect::<Vec<_>>();

All elements in the range are removed even if only 1 element is taken ("extracted") from the iterator.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now I understand. But that is somewhat orthogonal isn't it... not worth mentioning in the summary. Also, the examples I added also demonstrate the fact you mention.

Copy link
Member

Choose a reason for hiding this comment

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

It should be mentioned somewhere in the docs for the method IMO. It's something users have already wondered about, always best to say it explicitly.

Copy link
Member Author

Choose a reason for hiding this comment

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

alright

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 re-instated the info @bluss

@tshepang tshepang changed the title doc: trim redundant info on drain, and improve related examples doc: improve Drain examples Dec 17, 2015
@bluss
Copy link
Member

bluss commented Dec 17, 2015

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Dec 17, 2015

📌 Commit 46e2296 has been approved by bluss

bors added a commit that referenced this pull request Dec 18, 2015
Second sentence actually repeats info from first sentence. "from start to end" also feels like it adds nothing.

I also extended Vec::drain example.
@bors
Copy link
Contributor

bors commented Dec 18, 2015

⌛ Testing commit 46e2296 with merge f963eb2...

@bors
Copy link
Contributor

bors commented Dec 18, 2015

⛄ The build was interrupted to prioritize another pull request.

@bors bors merged commit 46e2296 into rust-lang:master Dec 18, 2015
@tshepang tshepang deleted the doc-drain branch December 18, 2015 09:59
bors added a commit that referenced this pull request Dec 18, 2015
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.

5 participants