Improve range docs #77874
Improve range docs #77874
Conversation
camelid
added
C-cleanup
|
r? @dtolnay (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
| /// assert_eq!(arr[ .. 3], [0,1,2 ]); | ||
| /// assert_eq!(arr[ ..=3], [0,1,2,3 ]); | ||
| /// assert_eq!(arr[1.. ], [ 1,2,3,4]); | ||
| /// assert_eq!(arr[1.. 3], [ 1,2 ]); // Range |
There was a problem hiding this comment.
I'm not sure why the comment // Range was here; it didn't make sense to me, so I removed it.
There was a problem hiding this comment.
It looks like these must have been copy-pasted to all the Range* types? So the comment is probably to emphasize that that particular line is the one for this type.
Seems like this example should be maybe somewhere where we reference it from all the types, and call-out Range, RangeTo, etc on every line. I guess there's not a core::range module to put it on...
Maybe this example could go on RangeBounds, and all the Range* types could link over there as a way to jump between them?
There was a problem hiding this comment.
So the comment is probably to emphasize that that particular line is the one for this type.
Ah, I see; thanks for clearing that up :)
There was a problem hiding this comment.
For now I just updated all the examples to make them consistent.
camelid
force-pushed
the
range-docs-readability
branch
2 times, most recently
from
59543ff to
8778fdc
Compare
|
Ready for a final review! |
library/core/src/ops/range.rs
Outdated
| @@ -6,6 +6,9 @@ use crate::hash::Hash; | |||
| /// `RangeFull` is primarily used as a [slicing index], its shorthand is `..`. | |||
| /// It cannot serve as an [`Iterator`] because it doesn't have a starting point. | |||
| /// | |||
| /// `RangeFull` is a [*zero-sized type*][ZST], which means it is a singleton – | |||
| /// there is only one instance of the `RangeFull` type. | |||
There was a problem hiding this comment.
suggestion: I would avoid the "singleton" and "instance" language here, since one can have (.., 4, ..) and such which could easily be argued to be two instances. (Just because something is a ZST doesn't mean that their addresses are always ptr::eq, which is demonstrated most clearly by the slice iterators.)
Maybe just leave the mention off? It's defined as pub struct RangeFull;, so I don't think we need to say this on every such type -- people can look it up in the book if they're curious. (For example, fmt::Error doesn't say it's a ZST.) Especially since I don't think there's a case where it being a ZST actually semantically affects safe code.
There was a problem hiding this comment.
Makes sense. I think I added this because I say "This is the RangeFull" later on, and I thought this might make it clearer to people why I say "the": that there's only one possible value. But I can remove it :)
* Mention that `RangeFull` is a ZST and thus a singleton * Improve code formatting and legibility * Various other readability improvements
camelid
force-pushed
the
range-docs-readability
branch
from
8778fdc to
a885c50
Compare
|
📌 Commit a885c50 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions, checks-azure |
Uh oh!
There was an error while loading. Please reload this page.