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

Docs: guide copy edits, subjective improvements, and other suggestions #22954

Merged
merged 2 commits into from
Mar 24, 2015

Conversation

ches
Copy link
Contributor

@ches ches commented Mar 2, 2015

Greetings Rustaceans!

I've just been getting acquainted with Rust through the guide. First let me say that it's already in great shape, chapters are kept a good length to be digestible and paced to move the reader along fluidly, so my compliments to contributors!

Along the way I noticed a few minor copy errors, and also a few areas that I thought more subjectively could stand to be improved. My commits here are divided so that minor edits unlikely to be very contentious could be cherry-picked, and then topically on parts that might generate more discussion.

I also have some comments and questions that aren't directly associated with any changes on this branch yet. I'm not sure how you like to triage this sort of thing but I'll present them below and if it's appropriate they could be moved to separate issues or I might be able to help work some of them out within the scope of this PR. Sorry that these are a lot to take in, pretty much everything below here can be digested independently of the current changes in this PR so you could read the rest later 😄

Questions and Comments

I'll give stable links to doc revisions as of this writing.

  1. The example using PartialEq in the Traits chapter is poor—we have no idea how PartialEq works at this point in the text (or at any point, AFAICT), so it isn't clear why it won't work as a trait bound in this situation and Float almost magically does, with the aid of existing tailor-made identity functions that seem unlikely to be so conveniently available when we encounter a scenario like this in our real-world code.

    This section just seems glossed over, or perhaps content has moved around over time or there's an assumption that implementing equality with PartialEq should be covered in the guide eventually so this example will be less foreign. As it stands the text is hard to follow and not very meaningful.

  2. I found treatment of the relationship of trait objects to pointers in the Static and Dynamic Dispatch chapter unclear. The "Why Pointers?" section opens with this line:

    The use of language like "fat pointer" implies that a trait object is always a pointer of some form, but why?

    But the phrase "fat pointer" hasn't been used anywhere before. This is some of the more complex material in the guide, but this section nevertheless feels displaced, not clearly connecting preceding subject matter. Earlier we've covered the internal representation of trait objects and significance of pointers they contain, but it hasn't been spelled out (other than what &Foo syntax suggests) that trait objects are references (and why). That's what the "Why Pointers?" section is aiming to do I gather, but it seems out of place, I think it'd make more sense to cover this before the gory details of their internals.

  3. Suggestion: move the Error Handling chapter much earlier in the Intermediate section of the guide, or even into the Basics section. I know the Intermediate section isn't intended to be read in order per se, but plenty of people like me are just going to read it straight through anyway 😁 These are pretty fundamental concepts to understand and Option, Result, and idioms like unwrap() and .ok().expect() are referenced numerous times throughout the rest of the guide. They feature pretty prominently as early as Standard Input and Guessing Game chapters in Basics, in fact. I happen to have a good understanding of these already through encountering their analogs in typed functional languages, but if I didn't I believe I really would have appreciated reading Error Handling much earlier.

  4. In the rustdoc chapter, a comment at the beginning of the first source example refers to a "link" crate attribute being needed. There seems to be no such attribute present in the source. I believe this refers to crate_type according to the reference, but it'd be nice if this example were updated/clarified (I think crate_id is deprecated/obsolete too).

    This brings me to a related comment also: after encountering crate attributes in the reference and also docs on Cargo configuration like crate-type = ["dylib"], I'm uncertain about the relationship/redundancy between these. I'm sure this is the kind of thing where docs are simply struggling to keep pace with rapid changes in Rust and Cargo, just wanted to flag that this distinction ought to be clearly covered in the docs for one or the other at some point, it's presently hard to track down.

  5. Minor: link to sample editor configurations in the introductory chapter is broken, probably the generator automatically translates .md links to .html. Perhaps it shouldn't do that for absolute URLs.

  6. Following from my changes to the enums coverage in Compound Data Types in this PR: sum types are an important topic and I tried to make some improvements, but I think the motivating example of Character with Digit(i32) and Other variants is a pretty weak one, and a better example could greatly improve cohesion with the Ordering coverage later in the section and how that ties into pattern matching in the subsequent chapter. I just haven't thought of a better example to suggest yet.

    In particular, the text states:

    This may seem rather limiting, but it's a limitation which we can overcome.

    This is referring to Character, and actually to more than one limitation: the preceding admonition that its variants aren't comparable/don't have ordering, and don't support binary operators like * and +. Overcoming these limitations actually never gets explained—we next cover how Ordering works as an enum itself for plain i32s, but never get around to showing how this might be applied to our Digit variant type.

    Since the coverage of enums already segues into pattern matching and this could be even tighter with a stronger example, it might be nice if our example enum were somehow connected to the final example program for the Basics section too, where Ordering reappears. I don't see how it would fit with the current guessing game example, but food for thought.

  7. #[derive] seems conspicuously missing from the guide. It would probably make sense to introduce after showing simple examples of implementing equality and/or ordering traits by hand, which have been mentioned as possibilities above. Perhaps it's too much to breach this as early as the Basic section though without traits being introduced. #[derive] itself and the derivable traits can certainly be saved for Intermediate and referenced as covered later, in any case.

r? @steveklabnik for docs.

@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 @steveklabnik (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.

@steveklabnik
Copy link
Member

Hey @ches!

First of all, thank you for putting this level of care into a PR. I haven't reviewed the diff yet, but I love getting huge descriptions.

That being said, it's also very hard to manage big ones with unrelated changes like this ,so feel free to open individual ones in the future, as they tend to be much easier to handle, since we can handle them peacemeal. The cherry-picking thing is true, but since @bors can't do that automatically, in our case, it's a significantly more work.

On to the diff!

@steveklabnik
Copy link
Member

Broad overview comment: much of what you're talking about is basic fallout from moving from "The guide + guides" to a single, unified book, and it hasn't gotten a full editing pass yet, becuase I'm still writing some chapters. So good eyes, and I agree.

@steveklabnik
Copy link
Member

The Rustdoc chapter is being entirely re-written in #22549, so that will be fixed by that.

@steveklabnik
Copy link
Member

#[derive] seems conspicuously missing from the guide.

There's still a lot of things missing, but you're right. It would belong in the trait chapter, for sure.

@@ -49,7 +49,7 @@ languages.

A *vector* is a dynamic or "growable" array, implemented as the standard
library type [`Vec<T>`](../std/vec/) (we'll talk about what the `<T>` means
later). Vectors always allocate their data on the heap. Vectors are to slices
later). Vectors always allocate their data on the heap. Vectors are to arrays
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong, the original is correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pondered this twice on two separate passes and still decided my change was correct, so maybe that says something about ambiguity.

&str is stack-allocated, String is heap-allocated; arrays are stack-allocated, vectors are heap-allocated. The preceding chapter refers to Strings as "growable", this paragraph refers to vectors as growable. That's how I'm reading the analogy. If this isn't right, I'd argue there's something nonintuitive here. IIRC there is is absolutely no other use of the term "slice" in the guides up to this point except for "string slices", so what else can "Vectors are to slices what String is to &str" possibly be referring to?

Copy link
Member

Choose a reason for hiding this comment

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

Ahh interesting. The intention with this section is that a vector is a owning data structure. A slice is a borrowing data structure. A String is an owning data structure. A string slice is a borrowing data structure. Internally, String is a wrapper over a Vec, and &'str is basically the same thing as a slice, a pointer and a length.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should probably move addressing this part into another patch then—by your clarifying explanation this has several fundamental problems as it stands:

  • Ownership hasn't been introduced at all by this stage, it comes later in the intermediate section of the book, so that analogy is completely lost.

  • As I noted before, there is is no other use of the term "slice" in the guides up to this point except for "string slice".

    The unqualified term "slice" is introduced later in this chapter, after the analogy line we're discussing. The only logical assumption the reader can make about the analogy at that point is that it refers to string slices, and that makes no sense.

Possibly the part of this chapter that defines slices could be moved ahead of the part on vectors and that would help, although the line that says

You can also take a slice of a vector, String, or &str...

should be reworked to not mention vectors before they're defined, but state that slices of vectors are possible later in the vectors part.

This doesn't help with the ownership analogy being lost on the reader, though.

@steveklabnik
Copy link
Member

As far as the diff goes, I'm generally happy with all of these changes, thanks. I left some nits inline :)

@ches
Copy link
Contributor Author

ches commented Mar 4, 2015

That being said, it's also very hard to manage big ones with unrelated changes like this ,so feel free to open individual ones in the future, as they tend to be much easier to handle, since we can handle them peacemeal. The cherry-picking thing is true, but since @bors can't do that automatically, in our case, it's a significantly more work.

Noted. With source it's usually pretty obvious how to limit the scope of a PR, with docs I'd assume it's easier to review at once, but I won't make that assumption if I have doc changes again. Not used to the workflow of the bot, because otherwise I don't know how I would've separated the commits any differently really.

Anyhoo.

@steveklabnik
Copy link
Member

Not used to the workflow of the bot,

Yeah, absolutely no worries. Like I said, very happy for the patch in general :)

@bors
Copy link
Contributor

bors commented Mar 6, 2015

☔ The latest upstream changes (presumably #23031) made this pull request unmergeable. Please resolve the merge conflicts.

@steveklabnik
Copy link
Member

Hey @ches, are you interested in keeping up with this PR?

@ches
Copy link
Contributor Author

ches commented Mar 19, 2015

Yep! I'm just settling back in after some travel, should be able to update this tomorrow.

What's the preferred protocol on rebasing? I recall reading in contrib guidelines that the team likes follow-up commits to make response to feedback clear. Should I merge master into my topic branch to bring up-to-date?

@steveklabnik
Copy link
Member

You can feel free to just rebase, it's fine by me. If you go the 'merge master into your topic branch' route, you just end up having to rebase those out at the end anyway.

@ches
Copy link
Contributor Author

ches commented Mar 20, 2015

Rebased on master and updated according to feedback:

  • Changed links that I touched to reference-style instead of inline
  • Conceding defeat on the term "arm" for pattern matching clauses, for now 😝
  • We lost the guessing game chapter entirely in rand is gone, fix the book #22518

I suggest that the vector/slice analogy issue in the very first hidden diff above should be moved to a separate issue. Anything in my list of points in the initial PR message that warrant that too?

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Mar 22, 2015

📌 Commit 92294e7 has been approved by steveklabnik

@steveklabnik
Copy link
Member

Thank you!

@mdinger
Copy link
Contributor

mdinger commented Mar 22, 2015

You write really well. This section just sounds so much better now. You should contribute more often.

@ches
Copy link
Contributor Author

ches commented Mar 23, 2015

@mdinger Thank you! I'll certainly try. I'm just getting started with Rust in spare time but if there are particular areas of docs that people feel are in need of attention, let me know and I'll look them over with a newcomer's fresh eyes.

@mdinger
Copy link
Contributor

mdinger commented Mar 23, 2015

Well, there are pain points all over. Follow reddit/rust and they're discussed frequently. Of course there are doc bugs.

Lifetimes questions constantly come up (awesome lifetime docs would be really great!). Partially because there is a lack of clear documentation but also because it's unclear what awesome lifetime docs will look like. See here for examples and links to perplexed readers.

Closures were a problem (mainly outdated because closures just changed a lot) but it looks like @steveklabnik is about to make them a bit better with #23568 .

Making one cohesive document which handles people of all backgrounds proves problematic. Systems programmers don't like the simple things explained. Non-systems guys don't them glossed. Deep type language tends to be used by developers which can cause problems until simpler docs are written: see #22914

Docs are still a moving target, but it's getting better. Compiler breaking old Rust is decreasing and this makes things a lot easier. Also check out rbe if you haven't seen it. It's not prominently advertised on the main site.

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Mar 23, 2015
Greetings Rustaceans!

I've just been getting acquainted with Rust through the guide. First let me say that it's already in great shape, chapters are kept a good length to be digestible and paced to move the reader along fluidly, so my compliments to contributors!

Along the way I noticed a few minor copy errors, and also a few areas that I thought more subjectively could stand to be improved. My commits here are divided so that minor edits unlikely to be very contentious could be cherry-picked, and then topically on parts that might generate more discussion.

I also have some comments and questions that aren't directly associated with any changes on this branch yet. I'm not sure how you like to triage this sort of thing but I'll present them below and if it's appropriate they could be moved to separate issues or I might be able to help work some of them out within the scope of this PR. Sorry that these are a lot to take in, pretty much everything below here can be digested independently of the current changes in this PR so you could read the rest later 😄

### Questions and Comments

I'll give stable links to doc revisions as of this writing.

1. The [example using `PartialEq` in the Traits chapter][1] is poor—we have no idea how `PartialEq` works at this point in the text (or at any point, AFAICT), so it isn't clear why it won't work as a trait bound in this situation and `Float` almost magically does, with the aid of existing tailor-made identity functions that seem unlikely to be so conveniently available when we encounter a scenario like this in our real-world code.

   This section just seems glossed over, or perhaps content has moved around over time or there's an assumption that implementing equality with `PartialEq` should be covered in the guide eventually so this example will be less foreign. As it stands the text is hard to follow and not very meaningful.
2. I found treatment of the relationship of trait objects to pointers in the *Static and Dynamic Dispatch* chapter unclear. [The "Why Pointers?" section][2] opens with this line:

   > The use of language like "fat pointer" implies that a trait object is always a pointer of some form, but why?

   But the phrase "fat pointer" hasn't been used anywhere before. This is some of the more complex material in the guide, but this section nevertheless feels displaced, not clearly connecting preceding subject matter. Earlier we've covered the internal representation of trait objects and significance of pointers they contain, but it hasn't been spelled out (other than what `&Foo` syntax suggests) that trait objects are references (and why). That's what the "Why Pointers?" section is aiming to do I gather, but it seems out of place, I think it'd make more sense to cover this before the gory details of their internals.
3. Suggestion: move the *Error Handling* chapter much earlier in the Intermediate section of the guide, or even into the Basics section. I know the Intermediate section isn't intended to be read in order per se, but plenty of people like me are just going to read it straight through anyway 😁 These are pretty fundamental concepts to understand and `Option`, `Result`, and idioms like `unwrap()` and `.ok().expect()` are referenced numerous times throughout the rest of the guide. They feature pretty prominently as early as *Standard Input* and *Guessing Game* chapters in Basics, in fact. I happen to have a good understanding of these already through encountering their analogs in typed functional languages, but if I didn't I believe I really would have appreciated reading *Error Handling* much earlier.
4. In the `rustdoc` chapter, a [comment at the beginning of the first source example][3] refers to a "link" crate attribute being needed. There seems to be no such attribute present in the source. I believe this refers to `crate_type` [according to the reference][4], but it'd be nice if this example were updated/clarified (I think `crate_id` is deprecated/obsolete too).

   This brings me to a related comment also: after encountering crate attributes in the reference and also docs on Cargo configuration like `crate-type = ["dylib"]`, I'm uncertain about the relationship/redundancy between these. I'm sure this is the kind of thing where docs are simply struggling to keep pace with rapid changes in Rust and Cargo, just wanted to flag that this distinction ought to be clearly covered in the docs for one or the other at some point, it's presently hard to track down.
5. Minor: link to sample editor configurations in [the introductory chapter][5] is broken, probably the generator automatically translates `.md` links to `.html`. Perhaps it shouldn't do that for absolute URLs.
6. Following from my changes to the enums coverage in [*Compound Data Types*][6] in this PR: sum types are an important topic and I tried to make some improvements, but I think the motivating example of `Character` with `Digit(i32)` and `Other` variants is a pretty weak one, and a better example could greatly improve cohesion with the `Ordering` coverage later in the section and how that ties into pattern matching in the subsequent chapter. I just haven't thought of a better example to suggest yet.

   In particular, the text states:

   > This may seem rather limiting, but it's a limitation which we can overcome.

   This is referring to `Character`, and actually to more than one limitation: the preceding admonition that its variants aren't comparable/don't have ordering, and don't support binary operators like `*` and `+`. Overcoming these limitations actually never gets explained—we next cover how `Ordering` works as an enum itself for plain `i32`s, but never get around to showing how this might be applied to our `Digit` variant type.

   Since the coverage of enums already segues into pattern matching and this could be even tighter with a stronger example, it might be nice if our example enum were somehow connected to the final example program for the Basics section too, where `Ordering` reappears. I don't see how it would fit with the current guessing game example, but food for thought.
7. `#[derive]` seems conspicuously missing from the guide. It would probably make sense to introduce after showing simple examples of implementing equality and/or ordering traits by hand, which have been mentioned as possibilities above. Perhaps it's too much to breach this as early as the Basic section though without traits being introduced. `#[derive]` itself and the derivable traits can certainly be saved for Intermediate and referenced as covered later, in any case.

r? @steveklabnik for docs.

[1]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/traits.md#our-inverse-example
[2]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/static-and-dynamic-dispatch.md#why-pointers
[3]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/documentation.md#creating-documentation
[4]: http://doc.rust-lang.org/reference.html#linkage
[5]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/hello-world.md
[6]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/compound-data-types.md#enums
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 23, 2015
Greetings Rustaceans!

I've just been getting acquainted with Rust through the guide. First let me say that it's already in great shape, chapters are kept a good length to be digestible and paced to move the reader along fluidly, so my compliments to contributors!

Along the way I noticed a few minor copy errors, and also a few areas that I thought more subjectively could stand to be improved. My commits here are divided so that minor edits unlikely to be very contentious could be cherry-picked, and then topically on parts that might generate more discussion.

I also have some comments and questions that aren't directly associated with any changes on this branch yet. I'm not sure how you like to triage this sort of thing but I'll present them below and if it's appropriate they could be moved to separate issues or I might be able to help work some of them out within the scope of this PR. Sorry that these are a lot to take in, pretty much everything below here can be digested independently of the current changes in this PR so you could read the rest later 😄

### Questions and Comments

I'll give stable links to doc revisions as of this writing.

1. The [example using `PartialEq` in the Traits chapter][1] is poor—we have no idea how `PartialEq` works at this point in the text (or at any point, AFAICT), so it isn't clear why it won't work as a trait bound in this situation and `Float` almost magically does, with the aid of existing tailor-made identity functions that seem unlikely to be so conveniently available when we encounter a scenario like this in our real-world code.

   This section just seems glossed over, or perhaps content has moved around over time or there's an assumption that implementing equality with `PartialEq` should be covered in the guide eventually so this example will be less foreign. As it stands the text is hard to follow and not very meaningful.
2. I found treatment of the relationship of trait objects to pointers in the *Static and Dynamic Dispatch* chapter unclear. [The "Why Pointers?" section][2] opens with this line:

   > The use of language like "fat pointer" implies that a trait object is always a pointer of some form, but why?

   But the phrase "fat pointer" hasn't been used anywhere before. This is some of the more complex material in the guide, but this section nevertheless feels displaced, not clearly connecting preceding subject matter. Earlier we've covered the internal representation of trait objects and significance of pointers they contain, but it hasn't been spelled out (other than what `&Foo` syntax suggests) that trait objects are references (and why). That's what the "Why Pointers?" section is aiming to do I gather, but it seems out of place, I think it'd make more sense to cover this before the gory details of their internals.
3. Suggestion: move the *Error Handling* chapter much earlier in the Intermediate section of the guide, or even into the Basics section. I know the Intermediate section isn't intended to be read in order per se, but plenty of people like me are just going to read it straight through anyway 😁 These are pretty fundamental concepts to understand and `Option`, `Result`, and idioms like `unwrap()` and `.ok().expect()` are referenced numerous times throughout the rest of the guide. They feature pretty prominently as early as *Standard Input* and *Guessing Game* chapters in Basics, in fact. I happen to have a good understanding of these already through encountering their analogs in typed functional languages, but if I didn't I believe I really would have appreciated reading *Error Handling* much earlier.
4. In the `rustdoc` chapter, a [comment at the beginning of the first source example][3] refers to a "link" crate attribute being needed. There seems to be no such attribute present in the source. I believe this refers to `crate_type` [according to the reference][4], but it'd be nice if this example were updated/clarified (I think `crate_id` is deprecated/obsolete too).

   This brings me to a related comment also: after encountering crate attributes in the reference and also docs on Cargo configuration like `crate-type = ["dylib"]`, I'm uncertain about the relationship/redundancy between these. I'm sure this is the kind of thing where docs are simply struggling to keep pace with rapid changes in Rust and Cargo, just wanted to flag that this distinction ought to be clearly covered in the docs for one or the other at some point, it's presently hard to track down.
5. Minor: link to sample editor configurations in [the introductory chapter][5] is broken, probably the generator automatically translates `.md` links to `.html`. Perhaps it shouldn't do that for absolute URLs.
6. Following from my changes to the enums coverage in [*Compound Data Types*][6] in this PR: sum types are an important topic and I tried to make some improvements, but I think the motivating example of `Character` with `Digit(i32)` and `Other` variants is a pretty weak one, and a better example could greatly improve cohesion with the `Ordering` coverage later in the section and how that ties into pattern matching in the subsequent chapter. I just haven't thought of a better example to suggest yet.

   In particular, the text states:

   > This may seem rather limiting, but it's a limitation which we can overcome.

   This is referring to `Character`, and actually to more than one limitation: the preceding admonition that its variants aren't comparable/don't have ordering, and don't support binary operators like `*` and `+`. Overcoming these limitations actually never gets explained—we next cover how `Ordering` works as an enum itself for plain `i32`s, but never get around to showing how this might be applied to our `Digit` variant type.

   Since the coverage of enums already segues into pattern matching and this could be even tighter with a stronger example, it might be nice if our example enum were somehow connected to the final example program for the Basics section too, where `Ordering` reappears. I don't see how it would fit with the current guessing game example, but food for thought.
7. `#[derive]` seems conspicuously missing from the guide. It would probably make sense to introduce after showing simple examples of implementing equality and/or ordering traits by hand, which have been mentioned as possibilities above. Perhaps it's too much to breach this as early as the Basic section though without traits being introduced. `#[derive]` itself and the derivable traits can certainly be saved for Intermediate and referenced as covered later, in any case.

r? @steveklabnik for docs.

[1]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/traits.md#our-inverse-example
[2]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/static-and-dynamic-dispatch.md#why-pointers
[3]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/documentation.md#creating-documentation
[4]: http://doc.rust-lang.org/reference.html#linkage
[5]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/hello-world.md
[6]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/compound-data-types.md#enums
Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 24, 2015
Greetings Rustaceans!

I've just been getting acquainted with Rust through the guide. First let me say that it's already in great shape, chapters are kept a good length to be digestible and paced to move the reader along fluidly, so my compliments to contributors!

Along the way I noticed a few minor copy errors, and also a few areas that I thought more subjectively could stand to be improved. My commits here are divided so that minor edits unlikely to be very contentious could be cherry-picked, and then topically on parts that might generate more discussion.

I also have some comments and questions that aren't directly associated with any changes on this branch yet. I'm not sure how you like to triage this sort of thing but I'll present them below and if it's appropriate they could be moved to separate issues or I might be able to help work some of them out within the scope of this PR. Sorry that these are a lot to take in, pretty much everything below here can be digested independently of the current changes in this PR so you could read the rest later 😄

### Questions and Comments

I'll give stable links to doc revisions as of this writing.

1. The [example using `PartialEq` in the Traits chapter][1] is poor—we have no idea how `PartialEq` works at this point in the text (or at any point, AFAICT), so it isn't clear why it won't work as a trait bound in this situation and `Float` almost magically does, with the aid of existing tailor-made identity functions that seem unlikely to be so conveniently available when we encounter a scenario like this in our real-world code.

   This section just seems glossed over, or perhaps content has moved around over time or there's an assumption that implementing equality with `PartialEq` should be covered in the guide eventually so this example will be less foreign. As it stands the text is hard to follow and not very meaningful.
2. I found treatment of the relationship of trait objects to pointers in the *Static and Dynamic Dispatch* chapter unclear. [The "Why Pointers?" section][2] opens with this line:

   > The use of language like "fat pointer" implies that a trait object is always a pointer of some form, but why?

   But the phrase "fat pointer" hasn't been used anywhere before. This is some of the more complex material in the guide, but this section nevertheless feels displaced, not clearly connecting preceding subject matter. Earlier we've covered the internal representation of trait objects and significance of pointers they contain, but it hasn't been spelled out (other than what `&Foo` syntax suggests) that trait objects are references (and why). That's what the "Why Pointers?" section is aiming to do I gather, but it seems out of place, I think it'd make more sense to cover this before the gory details of their internals.
3. Suggestion: move the *Error Handling* chapter much earlier in the Intermediate section of the guide, or even into the Basics section. I know the Intermediate section isn't intended to be read in order per se, but plenty of people like me are just going to read it straight through anyway 😁 These are pretty fundamental concepts to understand and `Option`, `Result`, and idioms like `unwrap()` and `.ok().expect()` are referenced numerous times throughout the rest of the guide. They feature pretty prominently as early as *Standard Input* and *Guessing Game* chapters in Basics, in fact. I happen to have a good understanding of these already through encountering their analogs in typed functional languages, but if I didn't I believe I really would have appreciated reading *Error Handling* much earlier.
4. In the `rustdoc` chapter, a [comment at the beginning of the first source example][3] refers to a "link" crate attribute being needed. There seems to be no such attribute present in the source. I believe this refers to `crate_type` [according to the reference][4], but it'd be nice if this example were updated/clarified (I think `crate_id` is deprecated/obsolete too).

   This brings me to a related comment also: after encountering crate attributes in the reference and also docs on Cargo configuration like `crate-type = ["dylib"]`, I'm uncertain about the relationship/redundancy between these. I'm sure this is the kind of thing where docs are simply struggling to keep pace with rapid changes in Rust and Cargo, just wanted to flag that this distinction ought to be clearly covered in the docs for one or the other at some point, it's presently hard to track down.
5. Minor: link to sample editor configurations in [the introductory chapter][5] is broken, probably the generator automatically translates `.md` links to `.html`. Perhaps it shouldn't do that for absolute URLs.
6. Following from my changes to the enums coverage in [*Compound Data Types*][6] in this PR: sum types are an important topic and I tried to make some improvements, but I think the motivating example of `Character` with `Digit(i32)` and `Other` variants is a pretty weak one, and a better example could greatly improve cohesion with the `Ordering` coverage later in the section and how that ties into pattern matching in the subsequent chapter. I just haven't thought of a better example to suggest yet.

   In particular, the text states:

   > This may seem rather limiting, but it's a limitation which we can overcome.

   This is referring to `Character`, and actually to more than one limitation: the preceding admonition that its variants aren't comparable/don't have ordering, and don't support binary operators like `*` and `+`. Overcoming these limitations actually never gets explained—we next cover how `Ordering` works as an enum itself for plain `i32`s, but never get around to showing how this might be applied to our `Digit` variant type.

   Since the coverage of enums already segues into pattern matching and this could be even tighter with a stronger example, it might be nice if our example enum were somehow connected to the final example program for the Basics section too, where `Ordering` reappears. I don't see how it would fit with the current guessing game example, but food for thought.
7. `#[derive]` seems conspicuously missing from the guide. It would probably make sense to introduce after showing simple examples of implementing equality and/or ordering traits by hand, which have been mentioned as possibilities above. Perhaps it's too much to breach this as early as the Basic section though without traits being introduced. `#[derive]` itself and the derivable traits can certainly be saved for Intermediate and referenced as covered later, in any case.

r? @steveklabnik for docs.

[1]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/traits.md#our-inverse-example
[2]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/static-and-dynamic-dispatch.md#why-pointers
[3]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/documentation.md#creating-documentation
[4]: http://doc.rust-lang.org/reference.html#linkage
[5]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/hello-world.md
[6]: https://github.com/rust-lang/rust/blob/157614249594f187f421cd97f928e64c5ab5c1fa/src/doc/trpl/compound-data-types.md#enums
@bors bors merged commit 92294e7 into rust-lang:master Mar 24, 2015
@ches ches deleted the docs branch July 20, 2023 11:29
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.

5 participants