Skip to content
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

Rustdoc: Add loopcounter (enumerate) section to the for-loop chapter #25925

Merged
merged 4 commits into from
Jun 5, 2015
Merged

Rustdoc: Add loopcounter (enumerate) section to the for-loop chapter #25925

merged 4 commits into from
Jun 5, 2015

Conversation

azerupi
Copy link
Contributor

@azerupi azerupi commented May 31, 2015

Hi

I added a little section in the for loops about the enumerate() function.
I think it's useful for beginners to know this function and how you can use it.

I used the title loopcounter, but it's probably not the best word to describe it. So let me know if there is a better word :)

Sometimes loop counters are useful and we should show new users how it is achieved in Rust.
@rust-highfive
Copy link
Collaborator

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

@Gankra
Copy link
Contributor

Gankra commented May 31, 2015

r? @steveklabnik

@@ -41,3 +41,38 @@ so our loop will print `0` through `9`, not `10`.
Rust does not have the “C-style” `for` loop on purpose. Manually controlling
each element of the loop is complicated and error prone, even for experienced C
developers.

# Loopcounter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer "Enumerate"

@steveklabnik
Copy link
Member

Thanks @azerupi ! I'm interested in adding this, but there's a number of small formatting things :)

@azerupi
Copy link
Contributor Author

azerupi commented May 31, 2015

Hi @steveklabnik
I corrected some of the issues you pointed out but I don't know what you mean with the spaces. Let me know what I still have to do and I will correct it right away :)

Outputs:

```rust
i = 0 and j = 5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look like Rust source code, you should probably just mark it as text. (Same for the output listing below.)

@killercup
Copy link
Member

@azerupi, I'm pretty sure @steveklabnik just meant adding empty lines above and below code blocks and headlines. I don't think he meant marking the output code blocks as rust, though, as they are clearly not valid Rust. I'd go with text for those (as I commented inline).

@azerupi
Copy link
Contributor Author

azerupi commented May 31, 2015

@killercup Sure, I also found it was a bit weird to format them as rust code but I thought that it was what @steveklabnik was asking, I probably misunderstood :)

Anyways, I changed it. Let me know if there are still things to change.

@steveklabnik
Copy link
Member

Looks great! Thanks so much.

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jun 1, 2015

📌 Commit 90057c2 has been approved by steveklabnik

@bors
Copy link
Contributor

bors commented Jun 1, 2015

⌛ Testing commit 90057c2 with merge 5b138f1...

@bors
Copy link
Contributor

bors commented Jun 1, 2015

💔 Test failed - auto-mac-32-opt

@Gankra
Copy link
Contributor

Gankra commented Jun 1, 2015

@bors retry

@alexcrichton
Copy link
Member

@bors: retry-

ah this doesn't look spurious:

---- Enumerate_1 stdout ----
    <anon>:2:31: 2:36 error: unresolved name `lines`
<anon>:2     for (linenumber, line) in lines.enumerate() {
                                       ^~~~~
note: in expansion of for loop expansion
<anon>:2:5: 4:6 note: expansion site
error: aborting due to previous error
thread 'Enumerate_1' panicked at 'Box<Any>', /Users/rustbuild/src/rust-buildbot/slave/auto-mac-32-opt/build/src/libsyntax/diagnostic.rs:211

@alexcrichton
Copy link
Member

@bors: r-

## On iterators:

```rust
for (linenumber, line) in lines.enumerate() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The integration tests correctly reported that lines is undefined. It would be okay if you added something like this:

# let lines = "hello\nworld".lines();

The # at the start will be removed before compiling this code but it will tell rustdoc to hide the line from the rendered output.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing :)

@azerupi
Copy link
Contributor Author

azerupi commented Jun 1, 2015

I updated the code, it should now pass the integration test (I think) :)

@azerupi
Copy link
Contributor Author

azerupi commented Jun 4, 2015

Could someone try again? :) @steveklabnik @gankro @alexcrichton

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jun 4, 2015

📌 Commit c4f42a1 has been approved by steveklabnik

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jun 5, 2015
Hi

I added a little section in the for loops about the `enumerate()` function.
I think it's useful for beginners to know this function and how you can use it. 

I used the title loopcounter, but it's probably not the best word to describe it. So let me know if there is a better word :)
bors added a commit that referenced this pull request Jun 5, 2015
@bors bors merged commit c4f42a1 into rust-lang:master Jun 5, 2015
@azerupi azerupi deleted the patch-4 branch June 30, 2015 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants