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

Update docs for std::str #40682

Merged
merged 4 commits into from
Mar 29, 2017
Merged

Update docs for std::str #40682

merged 4 commits into from
Mar 29, 2017

Conversation

tigleym
Copy link
Contributor

@tigleym tigleym commented Mar 20, 2017

fixes #29375

I noticed there are docs for the str primitive type, which contained extensive examples, so my additions give a more general explanation of &str. But please let me know if something can be explained more or changed.

r? @steveklabnik

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

@steveklabnik
Copy link
Member

Hi @tigleym ! Thank you!

There's an issue here though; these docs seem to be for impl<S: Borrow<str>> SliceConcatExt<str> for [S] {, not for &str generally. That is, these docs should go here:

//! Unicode string slices.
//!
//! *[See also the `str` primitive type](../../std/primitive.str.html).*
note also the //!.

Mind moving them up there and then we can give it a deeper review? Thanks 😄

Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

Just some minor nits, thank so you much!

@@ -10,9 +10,28 @@

//! Unicode string slices.
//!
//! The `&str` type is one of the two main string types, the other being `String`.
//! Unlike its `String` counterpart, its contents are borrowed and therefore
//! cannot be moved someplace else.
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 pretty solid, but I would probably end up just ending the sentence after "borrowed", as you can move &str values, just not what they point to, which is very tricky and not really relevant here, IMHO.

//! cannot be moved someplace else.
//!
//! # Basic Usage
//! A basic string declaration of `&str` type:
Copy link
Member

Choose a reason for hiding this comment

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

could you insert a //! above this line, to give it some space?

Also, I might say "Defining a variable with the type &str" here, sounds a teeny bit more natural.

//! We can explicitly specify `hello_world`'s lifetime as well:
//!
//! ```
//! let hello_world:&'static str = "Hello, world!";
Copy link
Member

Choose a reason for hiding this comment

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

space after :, please!

@steveklabnik
Copy link
Member

@bors: r+ rollup

thanks a ton!

@bors
Copy link
Contributor

bors commented Mar 27, 2017

📌 Commit da74e86 has been approved by steveklabnik

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 29, 2017
Update docs for std::str

fixes rust-lang#29375

I noticed there are docs for the `str` primitive type, which contained extensive examples, so my additions give a more general explanation of `&str`. But please let me know if something can be explained more or changed.

r? @steveklabnik
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 29, 2017
Update docs for std::str

fixes rust-lang#29375

I noticed there are docs for the `str` primitive type, which contained extensive examples, so my additions give a more general explanation of `&str`. But please let me know if something can be explained more or changed.

r? @steveklabnik
bors added a commit that referenced this pull request Mar 29, 2017
Rollup of 5 pull requests

- Successful merges: #40682, #40731, #40783, #40838, #40864
- Failed merges:
@bors bors merged commit da74e86 into rust-lang:master Mar 29, 2017
@donniebishop donniebishop mentioned this pull request Apr 1, 2017
11 tasks
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.

API Docs: std::str
4 participants