diff --git a/CHANGELOG.md b/CHANGELOG.md index ba9b74f..0cbaece 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes ## Unreleased +- A list that returns every item in one response, such as `wallets list`, accepts `--all` and ignores it, because there are no more pages. Before, it rejected `--all`. `--all` still pages through lists that take `--limit`. - A command that acts on one wallet, such as `wallets get`, also takes the wallet as `--wallet ID`. If the argument and `--wallet` name different wallets, the command stops. Without either, the error names both ways to pass it. A profile's or `VOLTAGE_WALLET_ID`'s wallet is never used, so a delete always names its wallet. - A request that gets no response says why: a timeout (for the auth service, its fixed 30-second limit, which `--timeout` does not change), a connection not made within the 10-second connect limit (which `--timeout` does not change), a failed TLS connection, a connection that could not be made, or one that closed early. A hint names the next step. A write that timed out or closed early may have been applied, so its hint says to look up the resource ID before running the command again, never to retry. This covers API, auth, price, and checkout stream requests, which before said only that the request failed. - `--output-file` on a path that already exists says so and suggests a new path, instead of showing an OS error; any other refusal keeps its cause and gets the same hint. Both say that no change was sent. diff --git a/src/cli.rs b/src/cli.rs index e75cb8a..9a4be4c 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -927,6 +927,24 @@ fn add_children(mut parent: ClapCommand, prefix: &[String]) -> ClapCommand { } /// The command for one operation: its positional target, documented filters, and options. +/// `--all` pages through a list that takes `--limit`. A list without paging accepts `--all` +/// too and ignores it, because its one response already holds every item. +fn list_args(cmd: ClapCommand, operation: &Operation) -> ClapCommand { + if operation.has_parameter("limit") { + PaginationFlags::augment_args(cmd) + } else if operation.lists_everything_at_once() { + cmd.arg( + Arg::new("all_at_once") + .long("all") + .action(ArgAction::SetTrue) + .help("No effect: this list returns every item in one response") + .help_heading("Query"), + ) + } else { + cmd + } +} + /// The resource ID argument and `--id`, one of which is required. A wallet command can /// name its wallet with `--wallet` instead, so `target_id` checks that one was given. fn target_args(cmd: ClapCommand, target: ResourceTarget, wallet: bool) -> ClapCommand { @@ -989,9 +1007,7 @@ fn endpoint(mut cmd: ClapCommand, operation: &Operation, about: String) -> ClapC if operation.method != Method::Get { cmd = ExecuteFlags::augment_args(cmd); } - if operation.has_parameter("limit") { - cmd = PaginationFlags::augment_args(cmd); - } + cmd = list_args(cmd, operation); if operation.id.lists_all_organizations() { cmd = AllOrgsFlags::augment_args(cmd); } @@ -1421,6 +1437,22 @@ mod tests { ); } + #[test] + fn all_is_accepted_by_every_list_and_pages_only_a_paginated_one() { + // `wallets list` returns every wallet at once, so `--all` changes nothing. + assert!(!api(&["wallets", "list", "--all"]).all); + assert!(!api(&["webhooks", "list", "--all"]).all); + assert!(api(&["payments", "list", "--all"]).all); + // A command that is not a list still rejects it. + let get = + Invocation::try_parse_from(["voltage", "wallets", "get", WALLET, "--all"]).unwrap_err(); + assert!( + get.message.contains("unexpected argument '--all'"), + "{}", + get.message + ); + } + #[test] fn wallet_flag_names_the_wallet_a_wallet_command_acts_on() { const OTHER: &str = "44444444-4444-4444-8444-444444444444"; diff --git a/src/registry.rs b/src/registry.rs index f998287..8332f94 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -466,6 +466,14 @@ impl Operation { }) } + /// A `list` command whose one response holds every item: it has no `--limit` to page. + pub fn lists_everything_at_once(&self) -> bool { + self.method == Method::Get + && self.target.is_none() + && self.command.last().is_some_and(|word| word == "list") + && !self.has_parameter("limit") + } + pub fn has_parameter(&self, name: &str) -> bool { self.parameters .iter() @@ -512,6 +520,24 @@ mod tests { use super::*; use std::collections::BTreeSet; + #[test] + fn only_lists_without_paging_return_everything_at_once() { + for id in [ + OperationId::GetAllOrganizationsWalletsAsUser, + OperationId::GetWebhooksAsUser, + ] { + assert!(operation(id).lists_everything_at_once(), "{id:?}"); + } + // A paginated list, a single read, and a change are not. + for id in [ + OperationId::GetPayments, + OperationId::GetWallet, + OperationId::CreateWallet, + ] { + assert!(!operation(id).lists_everything_at_once(), "{id:?}"); + } + } + #[test] fn registry_matches_the_contract() { let ids: BTreeSet<_> = OPERATIONS.iter().map(|operation| operation.id).collect();