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

Confusing docs for core::ops::ControlFlow #133963

Closed
snprajwal opened this issue Dec 6, 2024 · 1 comment · Fixed by #134032
Closed

Confusing docs for core::ops::ControlFlow #133963

snprajwal opened this issue Dec 6, 2024 · 1 comment · Fixed by #134032
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@snprajwal
Copy link
Contributor

snprajwal commented Dec 6, 2024

Location

core::ops::ControlFlow and all its methods. Rust docs link

Summary

The docs for core::ops::ControlFlow::is_break() look like:

assert!(ControlFlow::<i32, String>::Break(3).is_break());
assert!(!ControlFlow::<String, i32>::Continue(3).is_break());

Firstly, the types switch across the two examples, which is unintuitive. The reader needs to spare the brain cells to identify which type represents Continue and Break in each case (even though the variant being used is different), which could also distract them from noticing the ! in the second statement. This would be much clearer if it looks like:

assert!(ControlFlow::<String, i32>::Break("stop right there!").is_break());
assert!(!ControlFlow::<String, i32>::Continue(42).is_break());

The main change being that the types are consistent across the examples.

The same change would look good for the other methods on this enum.

I'm happy to submit a PR if someone greenlights this :)

@snprajwal snprajwal added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Dec 6, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 6, 2024
@jieyouxu jieyouxu added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Dec 6, 2024
@Noratrieb
Copy link
Member

sure, just make a PR! next time you don't need to ask for things like this, you can just do them

@bors bors closed this as completed in e0f3db0 Dec 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 10, 2024
Rollup merge of rust-lang#134032 - snprajwal:fix-docs, r=joboet

docs: better examples for `std::ops::ControlFlow`

Fixes rust-lang#133963. Lesson learnt, never force-push from a bare clone of a repo 💀
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
4 participants