-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
refactor range examples #35759
refactor range examples #35759
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
//! assert_eq!(arr[1..3], [ 1,2 ]); // Range | ||
//! | ||
//! assert_eq!(arr[ ...3], [0,1,2,3 ]); // RangeToIncusive | ||
//! assert_eq!(arr[1...3], [ 1,2,3 ]); // RangeInclusive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you copied these from the existing examples, but I wonder if we should eliminate the extra spacing, since it's not idiomatic Rust. Hmm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could just add a note to that effect.
On Aug 17, 2016 2:59 PM, "Steve Klabnik" notifications@github.com wrote:
In src/libcore/ops.rs
#35759 (comment):@@ -64,6 +64,20 @@
//!
//! See the documentation for each trait for a minimum implementation that
//! prints something to the screen.
+//!
+//! This example shows the behavior of the variousRange*
structs.
+//!
+//! ```rust
+//! let arr = [0, 1, 2, 3, 4];
+//!
+//! assert_eq!(arr[ .. ], [0,1,2,3,4]); // RangeFull
+//! assert_eq!(arr[ ..3], [0,1,2 ]); // RangeTo
+//! assert_eq!(arr[1.. ], [ 1,2,3,4]); // RangeFrom
+//! assert_eq!(arr[1..3], [ 1,2 ]); // Range
+//!
+//! assert_eq!(arr[ ...3], [0,1,2,3 ]); // RangeToIncusive
+//! assert_eq!(arr[1...3], [ 1,2,3 ]); // RangeInclusiveI know you copied these from the existing examples, but I wonder if we
should eliminate the extra spacing, since it's not idiomatic Rust. Hmm—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang/rust/pull/35759/files/7564da22d890784af83bcdebac404c2d6d674bf7#r75185100,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGfGaWOEno8eN_VLR0fTdvOGkxKdZBxTks5qg1oVgaJpZM4JmxaP
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This documents unstable syntax, so if we want to do that it needs #![feature(inclusive_range_syntax)]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added, thank you.
https://github.com/rust-lang/rust/pull/35759/files#diff-0283b5f16e1f7cf368f3f56668838f4bR1519 should be below https://github.com/rust-lang/rust/pull/35759/files#diff-0283b5f16e1f7cf368f3f56668838f4bR1523 and similar with all the other examples |
^ done. Thanks! |
@bors: r+ rollup |
📌 Commit 9542d11 has been approved by |
☔ The latest upstream changes (presumably #35857) made this pull request unmergeable. Please resolve the merge conflicts. |
25886a0
to
31b7ff4
Compare
@bors: r+ rollup |
📌 Commit 711333f has been approved by |
…les, r=steveklabnik refactor range examples This pull request adds a module-level example of how all the range operators work. It also slims down struct-level examples in lieu of a link to module examples.
…les, r=steveklabnik refactor range examples This pull request adds a module-level example of how all the range operators work. It also slims down struct-level examples in lieu of a link to module examples.
…les, r=steveklabnik refactor range examples This pull request adds a module-level example of how all the range operators work. It also slims down struct-level examples in lieu of a link to module examples.
This PR fails linkchecker (see rollup #36126):
To quote a comment from linkchecker:
|
@bors: r- |
☔ The latest upstream changes (presumably #36126) made this pull request unmergeable. Please resolve the merge conflicts. |
This pull request adds a module-level example of how all the range operators work. It also slims down struct-level examples in lieu of a link to module examples. add feature for inclusive_range_syntax fix incorrectly closed code fences
711333f
to
34be21d
Compare
Travis looks spurious, let's give it a try with bors. @bors: r+ rollup |
📌 Commit 34be21d has been approved by |
…les, r=steveklabnik refactor range examples This pull request adds a module-level example of how all the range operators work. It also slims down struct-level examples in lieu of a link to module examples.
Fails tests:
|
@bors r- Pulling it out for #35759 (comment) (and so I can re-roll the rollup). Once fixed we can re-approve and put it back into the queue. |
@@ -2192,11 +2219,13 @@ impl<Idx: PartialOrd<Idx>> RangeInclusive<Idx> { | |||
/// array elements up to and including the index indicated by `end`. | |||
/// | |||
/// ``` | |||
/// #![feature(inclusive_range_syntax)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line shouldn't have been removed.
I'm pretty sure that this will fail tidy since directory links are used (ie. |
Alas, local |
Sorry, it's actually linkchecker (and not tidy) that complains (which only runs when using rustbuild). You can however see the current failures on travis (at the very end) (I would paste them here, put apparently iOS/Safari is not capable of copying from large plaintext files). |
|
|
@matthew-piziak ping! any interest in keeping up with this PR? |
☔ The latest upstream changes (presumably #36864) made this pull request unmergeable. Please resolve the merge conflicts. |
@steveklabnik Sorry, I've been traveling for the past three weeks. I should be back in the game around 7 October. Right now this PR is soft-blocking on #36417. Feel free to close; I'll keep the branch alive on my fork. |
@matthew-piziak no problem, since it's blocked on something else, and you're willing to keep the branch going, let's close it for now so that I don't have to remember that every time I look through the queue. Safe travels! |
This pull request adds a module-level example of how all the range operators work. It also slims down struct-level examples in lieu of a link to module examples.