Replace range function with range notation #20909

jatinn · 2015-01-11T01:26:10Z

Going over some examples noticed that the range function would show a warning.
So have gone ahead and replaced the example with the range notation.

Also, I have removed the type of uint u instead of changing it to us, as I believe it would be more beneficial to introduce and explain the different types in a separate section.

Going over some examples noticed that the range function would show a warning. So have gone ahead and replaced the example with the range notation. Also, I have removed the type of uint `u` instead of changing it to `us`, as I believe it would be more beneficial to introduce and explain the different types in a separate section.

rust-highfive · 2015-01-11T01:26:19Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @huonw (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. 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 CONTRIBUTING.md for more information.

huonw · 2015-01-11T01:33:08Z

src/doc/trpl/looping.md

@@ -54,8 +54,8 @@ The other kind of looping construct in Rust is the `while` loop. It looks like
 this:

 ```{rust}
-let mut x = 5u;       // mut x: uint


Maybe these type comments could be preserved? // mut x: i32 for this one.

Preserve type comments.

Preserve type comments

ftxqxd · 2015-01-11T02:54:11Z

src/doc/trpl/looping.md

@@ -18,7 +18,7 @@ for (x = 0; x < 10; x++) {
 Instead, it looks like this:

 ```{rust}
-for x in range(0, 10) {
+for x in (0..10) {


The parentheses around 0..10 here are not needed. (Same for line 126 below.)

Nice I did not know that.
However not sure about removing the parens mainly as it allows for easier edits.
The second example

for x in (0..10) { if x % 2 == 0 { continue; } println!("{}", x); }

could be refactored to

for x in (0..10).filter(|&x| (x % 2 != 0)) { println!("{}", x); }

where parens become mandatory right?
Keeping the parens saves you from jumping to the begining to add them later.

However if there is a style guide/convention I am breaking let me know and I will make the edit

AFAIK there is no style guide for range literals yet, as they are quite a recent addition. Still, I think that in cases like this, parentheses are just unnecessary noise. The same argument that parentheses make adding methods calls easier could apply to basically any binary operator…

let x = (a + b); // The parentheses make it easier to change it to this let x = (a + b).abs();

…but I think that everyone will agree that it’s bad style to put parentheses around every addition operation, and I think this applies to all other operations as well (and thus ..). Also, I don't normally call iterator methods on range iterators because they’re so simple, so I think the case of writing something like (a..b).filter(...) is rare enough anyway to justify a style decision to require parentheses unconditionally.

I also think that because the people who are reading The Rust Programming Language will probably be learning the range notation for the first time, it’s best to show that parentheses are not needed, rather than making them think that they are by using them everywhere.

(Also, if you do end up removing the parens, I just realised you’ll need to update line 41 as well as 21 and 126.)

yeah you make a valid point

range notation does not require parens

huonw · 2015-01-19T02:11:47Z

Hey there @jatinn, it looks like I've let this one slip through the cracks a bit. Github is telling me it needs a rebase, could you do that and then we'll see where we stand?

(If it seems like I'm being slow to respond feel free to ask me to re-review explicitly. :) )

Original [issue](#19278) that inspired this patch. The [reference.md] has evolved past simple grammatical constructs, and it serves a different purpose. The intent for the proposed _grammar.md_ is to hold **only** the official reference for the language grammar. This document would keep track of grammatical changes to the language over time, facilitate discussions over proposed changes to the existing grammar, and serve as basis for building parsers by third-parties (IDE's, GitHub linguist, CodeMirror, etc.). The current state of the PR contains all the grammars that were available in [reference.md] and nothing else. There are still a lot of missing pieces that weren't available. The following are just a few of the definitions missing: - [Functions](https://github.com/icorderi/rust/blob/docs/grammar/src/doc/grammar.md#functions) - [Structures](https://github.com/icorderi/rust/blob/docs/grammar/src/doc/grammar.md#structures) - [Traits](https://github.com/icorderi/rust/blob/docs/grammar/src/doc/grammar.md#traits) - [Implementations](https://github.com/icorderi/rust/blob/docs/grammar/src/doc/grammar.md#implementations) - [Operators](https://github.com/icorderi/rust/blob/docs/grammar/src/doc/grammar.md#unary-operator-expressions) - [Statements](https://github.com/icorderi/rust/blob/docs/grammar/src/doc/grammar.md#statements) - [Expressions](https://github.com/icorderi/rust/blob/docs/grammar/src/doc/grammar.md#expressions) [reference.md]: https://github.com/rust-lang/rust/blob/master/src/doc/reference.md We need help from people familiar with those grammatical constructs to fill in the missing pieces.

alexcrichton · 2015-01-20T16:42:38Z

Looks like a number of other commits have gotten mixed into this PR, perhaps a reset --hard + cherry-pick is in order now?

Only made 2 changes: 1) Update the year to 2015 in LICENSE-MIT 2) Update the year in COPYRIGHT No other changes were made.

Continuation of #21428

Going over some examples noticed that the range function would show a warning. So have gone ahead and replaced the example with the range notation. Also, I have removed the type of uint `u` instead of changing it to `us`, as I believe it would be more beneficial to introduce and explain the different types in a separate section.

Preserve type comments.

range notation does not require parens

jatinn · 2015-01-21T03:06:53Z

src/doc/trpl/looping.md

@@ -54,7 +54,7 @@ The other kind of looping construct in Rust is the `while` loop. It looks like
 this:

 ```{rust}
-let mut x = 5; // mut x: u32
+let mut x = 5;        // mut x: i32


realized another pr was merged that made the range(1,10) to 1..10 changes
Only thing is the incorrect type in the comment, if the pr is going to be a complicated merge you can just close it and I can create a new one for it.

I think that might be easiest, yeah. Thanks for your effort! (Feel free to "@huonw" me on the new PR 😄)

icorderi and others added 3 commits November 26, 2014 16:40

Added src/doc/grammar.md to hold Rust grammar

ffc5f1c

Copied all the grammar productions from reference.md to grammar.md

ab24ffe

rust-highfive assigned huonw Jan 11, 2015

huonw reviewed Jan 11, 2015
View reviewed changes

jatinn added 2 commits January 10, 2015 20:39

Update looping.md

55ebf6e

Preserve type comments.

Update looping.md

2303122

Preserve type comments

ftxqxd reviewed Jan 11, 2015
View reviewed changes

fay-jai and others added 2 commits January 10, 2015 23:35

update mit-license and copyright

602a7d1

Update looping.md

1347825

range notation does not require parens

ftxqxd mentioned this pull request Jan 12, 2015

Switching out range(0,10) example to 0..10. Tests fine #20930

Merged

Jatin Nagpal and others added 2 commits January 19, 2015 21:01

rebase

59d2fda

bors and others added 6 commits January 20, 2015 19:56

Auto merge of #20919 - fay-jai:update, r=brson

583c5c5

Only made 2 changes: 1) Update the year to 2015 in LICENSE-MIT 2) Update the year in COPYRIGHT No other changes were made.

Auto merge of #21439 - alexcrichton:rollup, r=alexcrichton

29bd9a0

Continuation of #21428

Update looping.md

7be4fbd

Preserve type comments.

Update looping.md

87bed33

range notation does not require parens

Merge branch 'master' of https://github.com/jatinn/rust

54824ce

jatinn reviewed Jan 21, 2015
View reviewed changes

jatinn closed this Jan 21, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace range function with range notation #20909

Replace range function with range notation #20909

Uh oh!

jatinn commented Jan 11, 2015

Uh oh!

rust-highfive commented Jan 11, 2015

Uh oh!

huonw Jan 11, 2015

Uh oh!

jatinn Jan 11, 2015

Uh oh!

ftxqxd Jan 11, 2015

Uh oh!

jatinn Jan 11, 2015

Uh oh!

ftxqxd Jan 11, 2015

Uh oh!

jatinn Jan 11, 2015

Uh oh!

huonw commented Jan 19, 2015

Uh oh!

alexcrichton commented Jan 20, 2015

Uh oh!

jatinn Jan 21, 2015

Uh oh!

huonw Jan 21, 2015

Uh oh!

Uh oh!

Replace range function with range notation #20909

Replace range function with range notation #20909

Uh oh!

Conversation

jatinn commented Jan 11, 2015

Uh oh!

rust-highfive commented Jan 11, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huonw commented Jan 19, 2015

Uh oh!

alexcrichton commented Jan 20, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!