Skip to content

feat(cli): add --quiet to rustup toolchain list and friends - #3862

Merged
rami3l merged 2 commits into
masterfrom
feat/list-quiet
Jun 7, 2024
Merged

feat(cli): add --quiet to rustup toolchain list and friends#3862
rami3l merged 2 commits into
masterfrom
feat/list-quiet

Conversation

@rami3l

@rami3lrami3l commented Jun 5, 2024

Copy link
Copy Markdown
Member

Closes#3519.

@rami3l
rami3l marked this pull request as ready for review June 7, 2024 01:59
@rami3l
rami3l requested a review from djcJune 7, 2024 01:59
djc
djc approved these changes Jun 7, 2024

@djcdjc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

It feels like we should more tightly model verbosity as an enum internally (along the lines of enum Verbosity { Default, Quiet, Verbose }) so we can be more intentional about how to apply it.

@rami3l

Copy link
Copy Markdown
MemberAuthor

It feels like we should more tightly model verbosity as an enum internally (along the lines of enum Verbosity { Default, Quiet, Verbose }) so we can be more intentional about how to apply it.

@djc I agree. However for the moment being we only have less than 5 instances of --quiets and --verboses in the codebase each, and they don't even form a pair in some places, so I'm a bit hesitant. I'd like to change that when the pattern becomes more visible, but for now I'm inclined to merge this as-is.

@rami3l
rami3l added this pull request to the merge queue Jun 7, 2024
Merged via the queue into master with commit a47877eJun 7, 2024
@rami3l
rami3l deleted the feat/list-quiet branch June 7, 2024 16:06
@djc

djc commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

I ran into https://github.com/rust-lang/rustup/blob/master/src/cli/common.rs#L171 where the interaction of verbose and quiet in the NotifyOnConsole/DownloadTracker (which themselves suffer from crappy design) doesn't seem very well-defined, that's why I was thinking about it.

Sign up for freeto 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.

Add a "single column" mode for rustup toolchain list and friends

2 participants

@rami3l@djc