Skip to content

added feedback to doc flag and doc in terminal when opening browser#3674

Merged
rami3l merged 1 commit intorust-lang:masterfrom
brettearle:doc-open-prompt-3663
Mar 8, 2024
Merged

added feedback to doc flag and doc in terminal when opening browser#3674
rami3l merged 1 commit intorust-lang:masterfrom
brettearle:doc-open-prompt-3663

Conversation

@brettearle
Copy link
Copy Markdown
Contributor

@brettearle brettearle commented Feb 17, 2024

Fix #3663

Copy link
Copy Markdown

@MrTheSaw MrTheSaw left a comment

Choose a reason for hiding this comment

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

That's the sort of thing I was thinking. Looks good.

@rami3l
Copy link
Copy Markdown
Member

rami3l commented Feb 25, 2024

@brettearle I think with Rustup you can forget about merging the master occasionally: GitHub Merge Queue will do this for you eventually, and doing so now is almost certainly a no-op.

@brettearle
Copy link
Copy Markdown
Contributor Author

Too easy, I will not merge in future unless git yells on the push 🫸

@brettearle
Copy link
Copy Markdown
Contributor Author

Removed the prefix

@brettearle brettearle requested a review from djc March 4, 2024 11:44
Copy link
Copy Markdown
Contributor

@djc djc left a comment

Choose a reason for hiding this comment

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

Now that I think about it, this should probably go to stderr instead of stdout. Otherwise this looks okay to me.

@brettearle
Copy link
Copy Markdown
Contributor Author

@djc why output it as an error? happy to change just would like to know reasoning as I must be missing some knowledge

@djc
Copy link
Copy Markdown
Contributor

djc commented Mar 5, 2024

stderr is generally what is used for status updates/log-like output to the user; stdout should be used for showing the requested output.

@djc
Copy link
Copy Markdown
Contributor

djc commented Mar 5, 2024

(Please also squash the commits here into a single one.)

@brettearle brettearle force-pushed the doc-open-prompt-3663 branch 2 times, most recently from e22d90d to 449300a Compare March 6, 2024 10:23
Copy link
Copy Markdown
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.

#3674 (comment) still needs to be addressed. The current code still prints to stdout.

@brettearle
Copy link
Copy Markdown
Contributor Author

Yeah not sure what happened there. Will sort.

@brettearle brettearle force-pushed the doc-open-prompt-3663 branch from be93095 to defd4ca Compare March 6, 2024 20:21
@rami3l rami3l enabled auto-merge March 7, 2024 14:50
@rami3l rami3l added this pull request to the merge queue Mar 8, 2024
Merged via the queue into rust-lang:master with commit b4b9a2e Mar 8, 2024
@rami3l rami3l modified the milestones: 1.28.0, 1.27.0 Mar 8, 2024
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.

Say what ought to happen when running rustup doc.

4 participants