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

change ~[] -> Vec for collect() #14992

Merged
merged 1 commit into from Jun 18, 2014

Conversation

Projects
None yet
3 participants
@nathantypanski
Contributor

nathantypanski commented Jun 18, 2014

This updates the documentation for result::collect() and
option::collect() to use the new-style syntax for owned pointers and
vectors.

closes #14991

@nathantypanski nathantypanski changed the title from change ~[] -> Box<Vec> for collect() to change ~[] -> Vec for collect() Jun 18, 2014

/// else { return Some(x + 1); }
/// }
/// let v = vec!(1u, 2u);
/// let res: Option<Vec<uint>> = collect(v.iter().map(inc_conditionally));

This comment has been minimized.

@huonw

huonw Jun 18, 2014

Member

I personally prefer the closure form, rather than inc_conditionally taking a reference, since I think it's clearer what is happening.

/// let res = collect(v.iter().map(|&x| inc_conditionally(x)));
/// assert!(res == Some(~[2u, 3, 4]));
/// fn inc_conditionally(x: &uint) -> Option<uint> {
/// if x == &uint::MAX { return None }

This comment has been minimized.

@huonw

huonw Jun 18, 2014

Member

This would be more idiomatic as

if *x == uint::MAX { None }
else { Some(*x + 1) }

(i.e. explicit dereferences & dropping the returns that were there before.)

@huonw

This comment has been minimized.

Member

huonw commented Jun 18, 2014

Also, neither of these examples are being tested automatically; I guess it might be because rustdoc needs them in fenced form (@alexcrichton, did this change recently? Was it meant to change?), could you change them so they don't go out of date again:

```rust
fn inc_conditionally(...)
...
```
@nathantypanski

This comment has been minimized.

Contributor

nathantypanski commented Jun 18, 2014

Sure, I'll update as mentioned.

@nathantypanski

This comment has been minimized.

Contributor

nathantypanski commented Jun 18, 2014

Done; the tests for these docs run now and they pass.

/// let res = collect(v.iter().map(|&x| inc_conditionally(x)));
/// assert!(res == Some(~[2u, 3, 4]));
/// ```rust
/// use std::option::collect;

This comment has been minimized.

@huonw

huonw Jun 18, 2014

Member

The convention is for functions to be used module-qualified, i.e. import std::option and call option::collect.

This comment has been minimized.

@nathantypanski

nathantypanski Jun 18, 2014

Contributor

Updated as noted. Thanks for the style tip.

@nathantypanski

This comment has been minimized.

Contributor

nathantypanski commented Jun 18, 2014

Oh, I missed the bit about closure form. I might as well go all-out and update it, rather than doing it halfway a hundred times.

change ~[] -> Vec for collect()
This updates the documentation for result::collect() and
option::collect() to use the new-style syntax for vectors, instead of
the old ~[].

Also updates the code blocks for these docs so they will be tested
automatically.

closes #14991
@huonw

This comment has been minimized.

huonw commented on feceb12 Jun 18, 2014

r+ thanks!

@bors

This comment has been minimized.

Contributor

bors commented on feceb12 Jun 18, 2014

saw approval from huonw
at nathantypanski@feceb12

This comment has been minimized.

Contributor

bors replied Jun 18, 2014

merging nathantypanski/rust/collect-docs = feceb12 into auto

This comment has been minimized.

Contributor

bors replied Jun 18, 2014

nathantypanski/rust/collect-docs = feceb12 merged ok, testing candidate = 410d70b

This comment has been minimized.

Contributor

bors replied Jun 18, 2014

fast-forwarding master to auto = 410d70b

bors added a commit that referenced this pull request Jun 18, 2014

auto merge of #14992 : nathantypanski/rust/collect-docs, r=huonw
This updates the documentation for result::collect() and
option::collect() to use the new-style syntax for owned pointers and
vectors.

closes #14991

@bors bors closed this Jun 18, 2014

@bors bors merged commit feceb12 into rust-lang:master Jun 18, 2014

2 checks passed

continuous-integration/travis-ci The Travis CI build passed
Details
default all tests passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment