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

fix(doc): don't show the opening message when --path is used #3748

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

0x61nas
Copy link
Contributor

@0x61nas 0x61nas commented Mar 29, 2024

This might seem like a travel or unnecessary thing, especially since the message is written to stderr so it won't affect piping or similar use cases.
But since stdout and stderr are ambiguous in most terminals (i.e., they're printed in the output) this might cause some confusion and/or annoyance to some users.

src/cli/rustup_mode.rs Outdated Show resolved Hide resolved
src/cli/rustup_mode.rs Outdated Show resolved Hide resolved
Copy link
Member

@rami3l rami3l left a comment

Choose a reason for hiding this comment

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

LGTM!

Besides, your topic_name does remind me of an edge case that I forgot to handle in #3674: rustup doc std::fs.
Reusing this variable gives Opening docs named `std::fs` in your browser, I guess that looks just right?

@0x61nas
Copy link
Contributor Author

0x61nas commented Mar 29, 2024

LGTM!

Besides, your topic_name does remind me of an edge case that I forgot to handle in #3674: rustup doc std::fs. Reusing this variable gives Opening docs named `std::fs` in your browser, I guess that looks just right?

this edge case are still exists, when i try rustup doc std::fs it rising this message Opening docs in your browser, as if i didn't specify a topic, but still opens the correct page

sorry, i forget to run the init command, yup every thing seams to work fine now

src/cli/rustup_mode.rs Outdated Show resolved Hide resolved
src/cli/rustup_mode.rs Outdated Show resolved Hide resolved
This might seem like a travel or unnecessary thing, especially since the message
is written to `stderr`  so it won't affect piping or similar use cases.
But since `stdout` and `stderr` are ambiguous in most terminals (i.e., they're printed in the output)
this might cause some confusion and/or annoyance to some users.
@rami3l rami3l enabled auto-merge March 29, 2024 11:06
@rami3l rami3l added this pull request to the merge queue Mar 29, 2024
Merged via the queue into rust-lang:master with commit 7e181ae Mar 29, 2024
21 checks passed
@rami3l
Copy link
Member

rami3l commented Mar 29, 2024

@0x61nas Thanks again!

@0x61nas 0x61nas deleted the fix/docs-path branch March 29, 2024 17:24
@rami3l rami3l mentioned this pull request Apr 14, 2024
3 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.

2 participants