-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add links to str primitive and StrExt in std::str module docs #22937
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
Conversation
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 CONTRIBUTING.md for more information. |
//! Rust's [`str`](../primitive.str.html) type is one of the core primitive types of the language. | ||
//! `&str` is the borrowed string type. This type of string can only be created from other strings, | ||
//! unless it is a static string (see below). As the word "borrowed" implies, this type of string | ||
//! is owned elsewhere, and this string cannot be moved out of. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest "and you cannot move out of it." here. I'm not the kind of person to say you shouldn't ever end a sentence with a preposition, but I think in this case the prose would benefit from not doing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request was merely correcting the issue, and I didn't touch any of the previous wording. I agree that the earlier sentence feels a bit wonky, but I don't know if the dangling preposition is the cause. IMO, this sentence flows much more naturally:
"It is not possible to move out of borrowed strings because they are owned elsewhere."
It's not as ornate as the original language, but it is precise and concise.
In the end though, it's not very relevant to the original issue/reason for the pull request. Not saying it's not worth discussion or changing, just that it's likely done lateral to this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My experience is that @steveklabnik doesn't mind when you touch up wording around an area that you're already modifying. Feel free to wait for his input, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, well then I don't mind. Just didn't want to be hijacking stuff :P which version of the sentence do you like better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My vote is for your version. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's totally fine to toss in small edits if you'd like.
I would actually replace 'static string' with &'static str
here, to be more specific.
And I like the suggested re-wording of that other sentence 👍
Can you squash these into one commit please! Thanks! r=me after |
Fixes #22902
r? @steveklabnik