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

Determine how to handle --quiet in commands with subcommands #12957

Closed
ehuss opened this issue Nov 11, 2023 · 2 comments · Fixed by #12959
Closed

Determine how to handle --quiet in commands with subcommands #12957

ehuss opened this issue Nov 11, 2023 · 2 comments · Fixed by #12959
Assignees
Labels
A-cargo-api Area: cargo-the-library API and internal code issues A-cli Area: Command-line interface, option parsing, etc.

Comments

@ehuss
Copy link
Contributor

ehuss commented Nov 11, 2023

Commands that have subcommands (like cargo report future-incompat, cargo config get, cargo owner add, cargo clean gc, etc.) currently don't support the --quiet flag. The problem is:

arg_quiet() is specified on every command individually, instead of using .global(true) like --verbose does. This is done in order to customize the help string on some commands (like cargo test).

AFAIK, we can't have both .global(true) and a per-command customized help text.

This means that commands like cargo owner add don't support the --quiet flag. It doesn't work to add .arg_quiet() on the subcommand because config_configure only looks at the arguments of the top-level command, and the global arguments. It isn't capable of seeing subcommands.

I don't really know how to fix this. Some ideas:

  • Change config_configure to somehow "see" subcommand arguments.
  • Have each subcommand manually set the quiet argument on the config. This might have some awkwardness, since config_configure handles things like clashing arguments (I suspect clap's conflicts_with won't help with that).
  • Drop support for per-command customized help text, and use .global(true).
@ehuss ehuss added the A-cargo-api Area: cargo-the-library API and internal code issues label Nov 11, 2023
@ehuss ehuss added the A-cli Area: Command-line interface, option parsing, etc. label Nov 11, 2023
@epage
Copy link
Contributor

epage commented Nov 11, 2023

Ill have to double check but I think you can have a global and locally override it and the value still gets propagated.

@ehuss
Copy link
Contributor Author

ehuss commented Nov 11, 2023

Hm, I was running into a panic, but I think just defining it as global fixes it. I'll post a PR with that.

@ehuss ehuss self-assigned this Nov 11, 2023
ehuss added a commit to ehuss/cargo that referenced this issue Nov 11, 2023
This fixes an issue where `--quiet` doesn't work with commands that have
subcommands. This is because `config_configure` only looks at the global
and top-level subcommand, and not deeper subcommands. The issue was that
`--quiet` was not defined as a global flag. This was changed in
rust-lang#6358 in order to give a better
help message for `cargo test --quiet`. I don't remember if clap just
didn't support overriding at the time, or if we just didn't know how it
worked. Anyways, it seems to work to override it now, so I think it
should be fine to mark it as global.

This should bring in `--quiet` more in-line with how `--verbose` works.
This means that `--quiet` is now accepted with `cargo report`,
`cargo help`, and `cargo config`.

This also fixes `--quiet` with `cargo clean gc`.

This should also help with supporting `--quiet` with the new `cargo
owner` subcommands being added in
rust-lang#11879.

Fixes rust-lang#12957
bors added a commit that referenced this issue Nov 12, 2023
Fix --quiet being used with nested subcommands.

This fixes an issue where `--quiet` doesn't work with commands that have subcommands. This is because `config_configure` only looks at the global and top-level subcommand, and not deeper subcommands. The issue was that `--quiet` was not defined as a global flag. This was changed in #6358 in order to give a better help message for `cargo test --quiet`. I don't remember if clap just didn't support overriding at the time, or if we just didn't know how it worked. Anyways, it seems to work to override it now, so I think it should be fine to mark it as global.

This should bring in `--quiet` more in-line with how `--verbose` works. This means that `--quiet` is now accepted with `cargo report`, `cargo help`, and `cargo config`.

This also fixes `--quiet` with `cargo clean gc`.

This should also help with supporting `--quiet` with the new `cargo owner` subcommands being added in
#11879.

As for this diff, the help text changes because the `--quiet` flag is changing position (except for the 3 commands mentioned above where it is now added).

Fixes #12957
@bors bors closed this as completed in 80ffb1d Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cargo-api Area: cargo-the-library API and internal code issues A-cli Area: Command-line interface, option parsing, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants