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.

Sign up for GitHub

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

Jump to bottom

libstd/libcore: fix various typos #74141

Merged
merged 1 commit into from
Jul 23, 2020
Merged

libstd/libcore: fix various typos #74141

merged 1 commit into from
Jul 23, 2020

Conversation

euclio
Copy link
Contributor

@euclio euclio commented Jul 8, 2020

No description provided.

@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 8, 2020
pickfire
pickfire reviewed Jul 8, 2020
View reviewed changes
src/libcore/macros/mod.rs Outdated
@@ -1044,7 +1044,7 @@ pub(crate) mod builtin {
};
}

/// Includes a utf8-encoded file as a string.
/// Includes a UTF-8-encoded file as a string.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Includes a UTF-8-encoded file as a string.
/// Includes a UTF-8 encoded file as a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will fix, but I'm curious: can you point me towards a grammar reference that indicates that this is the right choice?

Copy link
Contributor

@pickfire pickfire Jul 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It's not a grammar fix, just a readability fix. I never seen UTF-8 and encoded joined together in my life. I also find multiple - in a paragraph hard to read. Looking at wikipedia, I can find both versions there.

Copy link
Member

Choose a reason for hiding this comment

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

So like, the - wasn't even strictly necessary in the first place. For one example of a resource talking about this, see https://apastyle.apa.org/learn/faqs/when-use-hyphen

Basically all of these rules are about clarity. Does the hypen disambiguate, or not? I would argue that it's quite clear without the hyphen.

Copy link
Member

Choose a reason for hiding this comment

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

(I should note that I particularly over-use the hypen (oh my I just did it again in this sentence) so this is very much often my fault in Rust docs, though I didn't author this one...)

@pickfire
Copy link
Contributor

pickfire commented Jul 8, 2020

Did you go through this with some kind of type checkers like cargo-spellcheck?

steveklabnik
steveklabnik approved these changes Jul 8, 2020
View reviewed changes
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.

I agree with the one suggestion already made, but otherwise, looks great to me! r=me after that's fixed.

pickfire
pickfire approved these changes Jul 9, 2020
View reviewed changes
Copy link
Contributor

@pickfire pickfire left a comment

Choose a reason for hiding this comment

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

LGTM

@euclio
Copy link
Contributor Author

euclio commented Jul 9, 2020

Did you go through this with some kind of type checkers like cargo-spellcheck?

I made a super hacky custom rustdoc pass that ran a spellchecker over markdown compiled to plain text.

@jyn514 jyn514 added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Jul 23, 2020
@jyn514
Copy link
Member

jyn514 commented Jul 23, 2020

@bors r=@steveklabnik rollup=always

@bors
Copy link
Contributor

bors commented Jul 23, 2020

📌 Commit 133e91d has been approved by steveklabnik

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 23, 2020
@jyn514
Copy link
Member

jyn514 commented Jul 23, 2020

I made a super hacky custom rustdoc pass that ran a spellchecker over markdown compiled to plain text.

Do you mind sharing? That sounds useful :)

@Manishearth Manishearth mentioned this pull request Jul 23, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 23, 2020
@bors
Auto merge of rust-lang#74667 - Manishearth:rollup-s6k59sw, r=Manishe…
2bbfa02 
…arth

Rollup of 8 pull requests

Successful merges:

 - rust-lang#74141 (libstd/libcore: fix various typos)
 - rust-lang#74490 (add a Backtrace::disabled function)
 - rust-lang#74548 (one more Path::with_extension example, to demonstrate behavior)
 - rust-lang#74587 (Prefer constant over function)
 - rust-lang#74606 (Remove Linux workarounds for missing CLOEXEC support)
 - rust-lang#74637 (Make str point to primitive page)
 - rust-lang#74654 (require type defaults to be after const generic parameters)
 - rust-lang#74659 (Improve codegen for unchecked float casts on wasm)

Failed merges:

r? @ghost
@bors bors merged commit 9be1099 into rust-lang:master Jul 23, 2020
@euclio
Copy link
Contributor Author

euclio commented Jul 23, 2020

euclio@3b2046e

@pickfire
Copy link
Contributor

pickfire commented Jul 23, 2020 •
edited
Loading

How do you all find https://github.com/drahnr/cargo-spellcheck? Is it useful?

@euclio @jyn514 Do you think it would be good if we integrate that into tidy?

@jyn514
Copy link
Member

jyn514 commented Jul 23, 2020 •
edited
Loading

It has a dependency on autotools and libtool which is unfortunate ... could we get them to avoid that somehow?

@jyn514
Copy link
Member

jyn514 commented Jul 23, 2020

And it also doesn't work:

$ cargo spellcheck check
Error: Failed to parse manifest file /home/joshua/src/rust/src/bootstrap/Cargo.toml: No such file or directory (os error 2)
$ file /home/joshua/src/rust/src/bootstrap/Cargo.toml
/home/joshua/src/rust/src/bootstrap/Cargo.toml: ASCII text

@euclio
Copy link
Contributor Author

euclio commented Jul 23, 2020 •
edited
Loading

Yeah, I'm not surprisedcargo spellcheck doesn't work well with rustbuild, but maybe it will with some tweaks. I do think it would be worth integrating some sort of spell checking into libstd/libcore docs, though. Maybe we can open an issue for that to talk more?

@jyn514 The master branch shouldn't have a dependency on autotools anymore. It was getting that from hunspell-sys (which I maintain) and we recently moved the build over to cc.

@jyn514 jyn514 mentioned this pull request Jul 23, 2020
Open
@jyn514
Copy link
Member

jyn514 commented Jul 23, 2020

@euclio I opened #74697

@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steveklabnik steveklabnik steveklabnik approved these changes

@pickfire pickfire pickfire approved these changes

Assignees

@sfackler sfackler

Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.47.0
Development

Successfully merging this pull request may close these issues.
8 participants
@euclio @rust-highfive @pickfire @jyn514 @bors @steveklabnik @cuviper @sfackler