Repository navigation
fix(cli): accept --all on environments, organizations, and profiles list - #120
Conversation
tee8z
left a comment
There was a problem hiding this comment.
Looks good. One all_at_once_arg() serves the registry lists and the local lists. The tree walk in all_is_accepted_by_every_list_command keeps a future list from missing --all, and the second test covers the rejection path. The changelog and the Paging docs are updated.
…ltage-cli-accept-all-on-environments-list-and-stop # Conflicts: # CHANGELOG.md
…li-accept-all-on-environments-list-and-stop
…-the-cli' into brandon/dev-2953-voltage-cli-add-environments-create Resolve environments create against the layers below it: - EnvironmentCommand flattens DiscoveryCommand (list, get) from #126 and adds create, so `environments list` keeps the --all that #120 added and `environments get` keeps the picker. - Resolver::discover keeps #126's DiscoveryCommand argument and uses this branch's requires_login and login_organization helpers. - pick asks for an environment only for `environments get`, and #120's parse test matches EnvironmentCommand::Discovery(DiscoveryCommand::List). - CHANGELOG, docs/commands.md, the login guide, and check-coverage.py keep both the get and create entries.
tee8z
left a comment
There was a problem hiding this comment.
Approved at 118b143. The code is the same as at my earlier approval (c6318f4); the diff since then comes from the master merges. One all_at_once_arg() serves the registry and local lists, the tree walk keeps a future list from missing --all, and the rejection path is tested.
CRAP on this head: 849 of 1060 functions measured, 0 violations. Highest: 18.10 wait_for_payment (unchanged), then 18.00 endpoint (unchanged). CI is green.
Problem
voltage environments list --org ORG --allfailed with exit code 2:unexpected argument '--all' found. #94 made API lists without paging, such aswallets list, accept--alland ignore it. The local lists are clap-derived subcommands, not registry operations, so #94 did not reach them.organizations listandprofiles listhad the same failure.Change
environments list --allorganizations list --allprofiles list --allprofiles get NAME --all,context --allsrc/cli.rs: the--allargument from feat(cli): accept --all on lists that return every item at once #94 moves intoall_at_once_arg(). The registry lists and the two localListvariants (ListCommand::List,ProfileCommand::List) use it. The help text is now "No effect: this list shows every item at once", which is also true for the localprofiles list.all_is_accepted_by_every_list_commandwalks the whole command tree and fails if anylistsubcommand has no--all, so a new list cannot miss it.all_is_ignored_by_local_lists_and_rejected_by_other_local_commandstests the three local lists and the rejection path for local commands that are not lists. Both tests fail without the fix.CHANGELOG.md(Unreleased) anddocs/guides/requests.md(Paging) are updated.About
--wallet <UUID>in the usage lineThat usage line is not a real requirement. When clap suggests a similar flag (
--all→--wallet),Parser::did_you_mean_errorin clap_builder 4.6.7 adds the suggested argument to the matches "to build a proper usage string". So the usage line names the suggestion as if it were given.voltage environments list --walleshows the same line. Now that--allis accepted, this command no longer prints that suggestion. The clap behavior for other typos is out of scope here.Closes #119
Linear: DEV-2954
Try it on staging
Setup: Try a pull request on staging
Expect: the same organization table as without
--all, exit 0. Before this change:error: unexpected argument '--all' found, exit 2.Use an organization ID from the previous command. Expect: the environment table, exit 0.
Expect: the saved profiles (or an empty list), exit 0; the help shows
--allwithNo effect: this list shows every item at once.Expect:
unexpected argument '--all' foundandexit 2for both, because neither is a list.Checks
Run on the head after merging
master(a3bf7cd):cargo fmt --check,cargo clippy --locked --all-targets -- -D warnings,cargo machete --with-metadata,cargo test --locked(201 unit + 149 integration tests pass),python3 scripts/check-coverage.py: all pass.scripts/crap-report.sh, cargo-llvm-cov 0.8.5, knots 1.16.0): 849 of 1060 functions measured, 0 violations. The highest score is 18.10 (src/api.rswait_for_payment, unchanged).endpointinsrc/cli.rsis at 18.00 and this PR does not change it.voltage-pr-reviewself-review led to one fix: the help text no longer says "response", becauseprofiles listis local.organizations list --allandenvironments list --org ORG_ID --allexit 0 with their tables, andprofiles list --allexits 0.profiles get x --allandcontext --allare rejected withunexpected argument '--all'. The help shows the new text.Merge order
#123 (
environments create) now sits above this PR in stack #135, and the overlap is resolved: itsEnvironmentCommandflattensDiscoveryCommand(from #126), whoselistcarriesall_at_once_arg(), and the merges into #126 and #123 update theenvironments list --allassertion inall_is_ignored_by_local_lists_and_rejected_by_other_local_commands.all_is_accepted_by_every_list_commandpasses on every layer. Nothing more is needed here.Stack
Stack #135, layer 2 of 9 (bottom targets
master):--allonorganizations,environments, andprofiles listVOLTAGE_*variablevoltage context --format envvoltage feedbackand panicsvoltage environments createflake.nixwith a KaifileThe layers are chained with merge commits, because history is not rewritten. Use squash and merge, bottom first.