collections: update docs of slice get() and friends #38216

birkenfeld · 2016-12-07T09:19:47Z

for the new SliceIndex trait. Also made the docs of the unchecked
versions a bit clearer; they return a reference, not an "unsafe
pointer".

rust-highfive · 2016-12-07T09:19:51Z

r? @aturon

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

frewsxcv · 2016-12-07T20:52:22Z

Looks like Travis found an error in a doc example.

aturon · 2016-12-08T01:55:48Z

src/libcollections/slice.rs

@@ -343,15 +343,17 @@ impl<T> [T] {
        core_slice::SliceExt::last_mut(self)
    }

-    /// Returns the element of a slice at the given index, or `None` if the
-    /// index is out of bounds.
+    /// Returns the element or subslice of a slice at the given position or


It might be more clear to spell out the overloading in more detail here -- that is, to say something like:

If given a position, returns the element at that position of None if out of bounds.

If given a range, returns the subslice corresponding to that range, or None if out of bounds.

And similarly elsewhere.

Otherwise r=me once travis is happy.

Tried to do that now without too much repetition. Let me know if you want the details in all the method docs.

GuillaumeGomez · 2016-12-09T07:25:02Z

src/libcollections/slice.rs

-    /// Returns a pointer to the element at the given index, without doing
-    /// bounds checking. So use it very carefully!
+    /// Returns a reference to an element or subslice, without doing bounds
+    /// checking.  So use it very carefully!


Extra whitespace in here.

GuillaumeGomez · 2016-12-09T07:25:31Z

src/libcollections/slice.rs

+    /// index.
+    ///
+    /// - If given a position, returns a reference to the element at that
+    ///   position or `None` if out of bounds.


Missing url for None.

Huh? There are more instances of None without a link in the file. Does it have to be linked every time?

Not to mention that every reference to None seems to be different. There's (all in non-std crates that get re-exported to std)

[`None`]: ../../std/option/enum.Option.html#variant.None [`None`]: option/enum.Option.html#variant.None [option]: ../../std/option/enum.Option.html

Which is the preferred one?

The first one. And yes, every method/struct/enum/... should be linked. I'm currently fixing all this but when I see a PR, I try to enforce it there as well.

GuillaumeGomez · 2016-12-09T07:25:36Z

src/libcollections/slice.rs

+    /// - If given a position, returns a reference to the element at that
+    ///   position or `None` if out of bounds.
+    /// - If given a range, returns the subslice corresponding to that range,
+    ///   or `None` if out of bounds.


GuillaumeGomez

Just one last issue! :)

GuillaumeGomez · 2016-12-10T13:14:10Z

src/libcollections/slice.rs

@@ -361,7 +370,10 @@ impl<T> [T] {
        core_slice::SliceExt::get(self, index)
    }

-    /// Returns a mutable reference to the element at the given index.
+    /// Returns a mutable reference to an element or subslice depending on the
+    /// type of index (see `get`) or [`None`] if the index is out of bounds.


Missing get url. Also, should be get().

So that's just #method.get I assume?

Normally yes.

birkenfeld · 2016-12-15T16:25:03Z

ping?

GuillaumeGomez · 2016-12-15T16:29:28Z

Don't forget to ping people (just like you did) once you updated. :)

Thanks for your work!

@bors: r+ rollup

bors · 2016-12-15T16:29:29Z

📌 Commit 6a80241 has been approved by GuillaumeGomez

frewsxcv · 2016-12-15T19:48:56Z

It looks like the Option links are broken in this PR:

Linkcheck stage2 (x86_64-unknown-linux-gnu)
std/primitive.slice.html:185: broken link - /checkout/obj/build/x86_64-unknown-linux-gnu/std/option/enum.Option.html
std/primitive.slice.html:187: broken link - /checkout/obj/build/x86_64-unknown-linux-gnu/std/option/enum.Option.html
std/primitive.slice.html:199: broken link - /checkout/obj/build/x86_64-unknown-linux-gnu/std/option/enum.Option.html

frewsxcv · 2016-12-15T19:51:27Z

@bors r-

frewsxcv · 2016-12-15T20:02:47Z

No need to hold these changes up for the links, so feel free to remove them. The doc links are very fragile right now and need improvement.

GuillaumeGomez · 2016-12-15T21:48:32Z

src/libcollections/slice.rs

+    /// - If given a range, returns the subslice corresponding to that range,
+    ///   or [`None`] if out of bounds.
+    ///
+    /// [`None`]: ../../std/option/enum.Option.html#variant.None


There is an extra "..". Should be: ../std/option/enum.Option.html#variant.None.

GuillaumeGomez · 2016-12-15T21:48:41Z

src/libcollections/slice.rs

+    /// type of index (see [`get()`]) or [`None`] if the index is out of bounds.
+    ///
+    /// [`get()`]: #method.get
+    /// [`None`]: ../../std/option/enum.Option.html#variant.None


Same as above.

GuillaumeGomez · 2016-12-15T21:49:22Z

Please fix the url. If this change doesn't work, then just remove them but I'd like to give it a try first.

for the new SliceIndex trait. Also made the docs of the unchecked versions a bit clearer; they return a reference, not an "unsafe pointer".

birkenfeld · 2016-12-16T14:35:52Z

Ok, updated. If it is still not ok, you got access to the branch. I think project owners can do these small fixups much quicker on their own.

GuillaumeGomez · 2016-12-16T15:45:43Z

Still broken urls. Well, let's just forget about them this time. I'll remove them from your PR in the next days (if you don't update it before). :)

birkenfeld · 2016-12-16T15:51:52Z

Aren't they now exactly how you said they should be?

GuillaumeGomez · 2016-12-16T18:36:37Z

Yes, but now they need a supplementary ".." it said it wasn't needed previously (take a look at the CI for the details). Since I can't explain this behaviour, let's just forget about the urls (even if it pains me a lot...).

alexcrichton · 2017-01-17T23:35:20Z

Closing due to inactivity, but feel free to resubmit with tests fixed!

@GuillaumeGomez

collections: update docs of slice get() and friends Resubmit of rust-lang#38216. r? @GuillaumeGomez BTW, instead of closing a PR just because it is old and the team member who offered to fix it up did not have the time to do so, why not ping them instead? (cc @alexcrichton)

rust-highfive assigned aturon Dec 7, 2016

aturon reviewed Dec 8, 2016

View reviewed changes

birkenfeld force-pushed the slice-doc branch from aecbbee to 27ec894 Compare December 9, 2016 07:20

GuillaumeGomez requested changes Dec 9, 2016

View reviewed changes

birkenfeld force-pushed the slice-doc branch from 27ec894 to 5dcd548 Compare December 9, 2016 17:53

GuillaumeGomez requested changes Dec 10, 2016

View reviewed changes

birkenfeld force-pushed the slice-doc branch from 5dcd548 to 6a80241 Compare December 10, 2016 14:29

GuillaumeGomez mentioned this pull request Dec 15, 2016

Rollup of 6 pull requests #38387

Closed

GuillaumeGomez requested changes Dec 15, 2016

View reviewed changes

collections: update docs of slice get() and friends

2c7bb2a

for the new SliceIndex trait. Also made the docs of the unchecked versions a bit clearer; they return a reference, not an "unsafe pointer".

birkenfeld force-pushed the slice-doc branch from 6a80241 to 2c7bb2a Compare December 16, 2016 14:35

GuillaumeGomez approved these changes Dec 16, 2016

View reviewed changes

alexcrichton closed this Jan 17, 2017

birkenfeld mentioned this pull request Jan 18, 2017

collections: update docs of slice get() and friends #39150

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

collections: update docs of slice get() and friends #38216

collections: update docs of slice get() and friends #38216

birkenfeld commented Dec 7, 2016

rust-highfive commented Dec 7, 2016

frewsxcv commented Dec 7, 2016

aturon Dec 8, 2016

birkenfeld Dec 9, 2016

GuillaumeGomez Dec 9, 2016

birkenfeld Dec 9, 2016

GuillaumeGomez Dec 9, 2016

birkenfeld Dec 9, 2016

birkenfeld Dec 9, 2016

GuillaumeGomez Dec 9, 2016

birkenfeld Dec 9, 2016

GuillaumeGomez Dec 9, 2016

GuillaumeGomez left a comment

GuillaumeGomez Dec 10, 2016

birkenfeld Dec 10, 2016

GuillaumeGomez Dec 10, 2016

birkenfeld commented Dec 15, 2016

GuillaumeGomez commented Dec 15, 2016

bors commented Dec 15, 2016

frewsxcv commented Dec 15, 2016

frewsxcv commented Dec 15, 2016

frewsxcv commented Dec 15, 2016

GuillaumeGomez Dec 15, 2016

GuillaumeGomez Dec 15, 2016

GuillaumeGomez commented Dec 15, 2016

birkenfeld commented Dec 16, 2016 •

edited

Loading

GuillaumeGomez commented Dec 16, 2016

birkenfeld commented Dec 16, 2016

GuillaumeGomez commented Dec 16, 2016

alexcrichton commented Jan 17, 2017

collections: update docs of slice get() and friends #38216

collections: update docs of slice get() and friends #38216

Conversation

birkenfeld commented Dec 7, 2016

rust-highfive commented Dec 7, 2016

frewsxcv commented Dec 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

birkenfeld commented Dec 15, 2016

GuillaumeGomez commented Dec 15, 2016

bors commented Dec 15, 2016

frewsxcv commented Dec 15, 2016

frewsxcv commented Dec 15, 2016

frewsxcv commented Dec 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Dec 15, 2016

birkenfeld commented Dec 16, 2016 • edited Loading

GuillaumeGomez commented Dec 16, 2016

birkenfeld commented Dec 16, 2016

GuillaumeGomez commented Dec 16, 2016

alexcrichton commented Jan 17, 2017

birkenfeld commented Dec 16, 2016 •

edited

Loading