Conversation
|
It touches guide-tasks also because I didn't think it was worth pulling into a different branch. I don't know if you will care. |
src/doc/tutorial.md
Outdated
There was a problem hiding this comment.
nit: do not like the stacked spaces here – this is not a proper way of justifying text (would be done in the frontend if needed).
|
Fixed nits and added |
|
Travis CI stalled. Can you please restart it? [EDIT] Nix that. Sorry about double build. Just saw Travis CI has a queue. |
|
Could you please review this again? |
src/doc/tutorial.md
Outdated
There was a problem hiding this comment.
any reason you capitalized "Can" here, while using lowercase elsewhere?
src/doc/tutorial.md
Outdated
There was a problem hiding this comment.
The semicolon at the end of this fn item is unnecessary, and including it probably violates some style guideline somewhere. (I'm a little surprised this actually parses.)
(As opposed to semi-colon at the end of the let closure = ...;, which is necessary.)
|
I don't have any serious problem with the spirit of this change. (at points during its development I wondered if I would prefer to just revise the documentation here more completely, but I don't want to take the time to do a proper job of that.) So if you could incorporate the feedback I have provided, then I'll put it through another round of review (and hopefully eventually mark it for incorporation onto master). |
src/doc/tutorial.md
Outdated
There was a problem hiding this comment.
I think this would be better phrased as "closure has already been assigned the type |int| -> () via inference." (I'm planning on providing @mdinger with a pull request which will show him another phrasing.)
Most important: distinguish function decl sugar for omitting `-> ()` from type inference on closures. I also tried to add a couple more examples to further emphasize this distinction. Note that this sugar (of omitting `-> ()`) is actually already briefly mentioned in an earlier section, so it is a little tricky deciding whether to put more material here, or to move it up to the previous section. Other drive-by fixes: * Fix the line length of the code blocks to fit in the width provided in the rendered HTML * Some minor revisions to wording (e.g. try to clarify in some cases where a type mismatch is arising).
|
@mdinger I attempted to open a PR against your repository, but for some reason github's interface is not letting me select your repository. (I would have expected you to show up between Anyway, if you want to see my changes (or even just pull them into your own repository), you can see the commit here: pnkfelix@636f7d2 |
Suggested revisions to PR 13676.
I didn't think the concept of returning unit from the functions section was difficult but I did think it important to contrast the function and closure syntax directly. Then it makes more sense to me to discuss it in closures where it is directly relevant. In the functions section, the distinction may not matter yet. |
|
Regarding pnkfelix/rust@636f7d2: |
There was a problem hiding this comment.
I don't think the term assign is great here since it's a normal parameter being passed.
There was a problem hiding this comment.
The language there is poor in a number of respects, sorry I missed that earlier.
Improve tutorial discussion of closures, e.g. with respect to type inference and variable capture. Fix #13621 ---- original description follows I'd like this pulled to master if possible but if not I'd appreciate comments on what I need to change. I found the closures difficult to understand as they were so I tried to explain it so I would've had an easier time understanding it. I think it's better at least, somewhat. I don't know that everyone liked the `-> ()` I included but I thought explicit is best to aid understanding. I thought it was much harder to understand than it should have been. [EDIT] - Clicked too early. This doesn't `make check` without errors on my Xubuntu on Virtualbox machine. Not sure why. I don't think I changed anything problematic. I'll try `make check` on master tomorrow. Opened #13621 regarding this.
Improve tutorial discussion of closures, e.g. with respect to type inference and variable capture.
Fix #13621
---- original description follows
I'd like this pulled to master if possible but if not I'd appreciate comments on what I need to change. I found the closures difficult to understand as they were so I tried to explain it so I would've had an easier time understanding it. I think it's better at least, somewhat.
I don't know that everyone liked the
-> ()I included but I thought explicit is best to aid understanding. I thought it was much harder to understand than it should have been.[EDIT] - Clicked too early.
This doesn't
make checkwithout errors on my Xubuntu on Virtualbox machine. Not sure why. I don't think I changed anything problematic. I'll trymake checkon master tomorrow.Opened #13621 regarding this.