From b3df4d02ad01308e887e5e3963f85d440cbd9d49 Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:34:02 -0500 Subject: [PATCH] feat(cli): accept --wallet as the wallet a wallet command acts on Closes #80. --- CHANGELOG.md | 1 + docs/guides/login-and-profiles.md | 2 +- src/cli.rs | 101 ++++++++++++++++++++++++++++-- 3 files changed, 98 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa533d2..ba9b74f 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 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. - `-o` is short for `--output`, as in `voltage wallets list -o json`. diff --git a/docs/guides/login-and-profiles.md b/docs/guides/login-and-profiles.md index 0809e42..9f38ccb 100644 --- a/docs/guides/login-and-profiles.md +++ b/docs/guides/login-and-profiles.md @@ -35,7 +35,7 @@ With several saved credentials, select one with `--account` or through a profile ## Scope -A resource ID is positional, or passed as `--id ID`, as in `voltage wallets get --id WALLET_ID`. The enclosing scope comes from `--org`, `--env`, `--wallet`, and `--webhook`: +A resource ID is positional, or passed as `--id ID`, as in `voltage wallets get --id WALLET_ID`. A command that acts on one wallet also takes it as `--wallet WALLET_ID`; a profile's or `VOLTAGE_WALLET_ID`'s wallet never selects it. The enclosing scope comes from `--org`, `--env`, `--wallet`, and `--webhook`: ```sh voltage wallets list --org ORG_ID --env ENV_ID diff --git a/src/cli.rs b/src/cli.rs index a6e5280..e75cb8a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -12,7 +12,7 @@ use crate::{ output::OutputFormat, payment::{AmountUnit, Currency, Network, PaymentDirection, ReceiveKind, WaitTarget}, price::ConversionRequest, - registry::{Method, OPERATIONS, Operation, OperationId, Parameter, operation}, + registry::{Method, OPERATIONS, Operation, OperationId, Parameter, ResourceTarget, operation}, terminal::Terminal, }; use clap::{ @@ -724,6 +724,35 @@ fn payment_alias_parent(create_payment: &Operation) -> &[String] { .unwrap_or_default() } +/// The operation's resource ID: its argument or `--id`. A wallet operation also takes an +/// explicit `--wallet`, which names the wallet everywhere else; a profile's or +/// `VOLTAGE_WALLET_ID`'s wallet never selects it, so a delete always names its wallet. +fn target_id( + operation: &Operation, + target: Option, + m: &ArgMatches, +) -> Result> { + let id = target.and_then(|target| target.id()); + if !operation.targets_wallet() { + return Ok(id); + } + match (id, m.get_one::("wallet").copied()) { + (Some(id), Some(wallet)) if id != wallet => Err(Error::usage(format!( + "WALLET_ID {id} and --wallet {wallet} name different wallets" + )) + .with_hint("Pass the wallet once, as WALLET_ID or --wallet.")), + (Some(id), _) | (None, Some(id)) => Ok(Some(id)), + (None, None) => { + let command = operation.command.join(" "); + Err( + Error::usage(format!("{command} needs a wallet ID")).with_hint(format!( + "Pass it as an argument or with --wallet, as in `voltage {command} WALLET_ID`. `voltage wallets list` shows the IDs." + )), + ) + } + } +} + fn api_invocation( operation: &'static Operation, alias: Option, @@ -758,7 +787,7 @@ fn api_invocation( Ok(ApiInvocation { operation, alias, - resource_id: target.and_then(|target| target.id()), + resource_id: target_id(operation, target, m)?, filters, query: query.query, all: pagination.is_some_and(|flags| flags.all), @@ -898,6 +927,18 @@ fn add_children(mut parent: ClapCommand, prefix: &[String]) -> ClapCommand { } /// The command for one operation: its positional target, documented filters, and options. +/// 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 { + let cmd = TargetFlags::augment_args(cmd) + .mut_arg("resource_id", |arg| arg.value_name(target.value_name())); + if wallet { + cmd.mut_group("TargetFlags", |group| group.required(false)) + } else { + cmd + } +} + fn endpoint(mut cmd: ClapCommand, operation: &Operation, about: String) -> ClapCommand { let command_name = cmd.get_name().to_owned(); let mut notes = Vec::new(); @@ -915,7 +956,7 @@ fn endpoint(mut cmd: ClapCommand, operation: &Operation, about: String) -> ClapC } if !operation.is_environment_scoped() { notes.push(if operation.targets_wallet() { - "Scope:\n Wallets have organization scope. --env validates the wallet on reads and mutations; it does not change this endpoint's scope.".to_owned() + "Scope:\n Wallets have organization scope. --env validates the wallet on reads and mutations; it does not change this endpoint's scope. --wallet ID names the wallet in place of WALLET_ID.".to_owned() } else { "Scope:\n This endpoint has no environment filter. An environment in a profile does not narrow this operation.".to_owned() }); @@ -936,8 +977,7 @@ fn endpoint(mut cmd: ClapCommand, operation: &Operation, about: String) -> ClapC // `-h` stays short; `--help` adds the notes. cmd = cmd.after_long_help(notes.join("\n\n")); if let Some(target) = operation.target { - cmd = TargetFlags::augment_args(cmd) - .mut_arg("resource_id", |arg| arg.value_name(target.value_name())); + cmd = target_args(cmd, target, operation.targets_wallet()); } for parameter in operation .query_filters() @@ -1381,6 +1421,57 @@ mod tests { ); } + #[test] + fn wallet_flag_names_the_wallet_a_wallet_command_acts_on() { + const OTHER: &str = "44444444-4444-4444-8444-444444444444"; + let wallet = Some(WALLET.parse().unwrap()); + assert_eq!( + api(&["wallets", "get", "--wallet", WALLET]).resource_id, + wallet + ); + assert_eq!( + api(&["wallets", "get", WALLET, "--wallet", WALLET]).resource_id, + wallet + ); + assert_eq!( + api(&["wallets", "delete", "--wallet", WALLET]).resource_id, + wallet + ); + + let differ = + Invocation::try_parse_from(["voltage", "wallets", "get", WALLET, "--wallet", OTHER]) + .unwrap_err(); + assert_eq!(differ.kind, Kind::Usage); + assert_eq!( + differ.message, + format!("WALLET_ID {WALLET} and --wallet {OTHER} name different wallets") + ); + assert_eq!( + differ.hint.as_deref(), + Some("Pass the wallet once, as WALLET_ID or --wallet.") + ); + + let missing = Invocation::try_parse_from(["voltage", "wallets", "get"]).unwrap_err(); + assert_eq!(missing.kind, Kind::Usage); + assert_eq!(missing.message, "wallets get needs a wallet ID"); + assert!( + missing + .hint + .unwrap() + .contains("voltage wallets get WALLET_ID") + ); + + // Commands that act on something else still require their own ID. + let payment = + Invocation::try_parse_from(["voltage", "payments", "get", "--wallet", WALLET]) + .unwrap_err(); + assert!( + payment.message.contains(">"), + "{}", + payment.message + ); + } + #[test] fn short_o_selects_the_output_format_before_or_after_the_subcommand() { for args in [