From 0f2806f9f5fc4fdfcebd97a868d01c991fef4428 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 12:23:47 +0000 Subject: [PATCH] fix(cli): pick the argument-error format the way clap picks -o The raw scan took the first -o and missed clustered short flags, so `idx -o table config -o json get` and `idx -qojson config get` printed plain-text errors. Resolve -o with clap itself, ignoring the error being reported (Pullfrog review on #56). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/main.rs | 30 +++++++++++------------------- crates/idx-cli/tests/cli.rs | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index f94fb62..7214d17 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -112,27 +112,19 @@ fn argument_error(err: clap::Error) -> IdxError { } /// The output format the raw arguments ask for, when they did not parse: -/// `-o`/`--output` (any spelling), then `IDX_OUTPUT`, then auto. +/// `-o`/`--output` as clap reads it (clustered short flags, the last one +/// at any subcommand level), then `IDX_OUTPUT`, then auto. fn requested_output_from_args() -> OutputFormat { - // args_os: an argument that is not UTF-8 (itself the error being - // reported) must not panic here. - let args: Vec = std::env::args_os() - .skip(1) - .map(|arg| arg.to_string_lossy().into_owned()) - .collect(); - let flag = args.iter().enumerate().find_map(|(i, arg)| { - if arg == "-o" || arg == "--output" { - args.get(i + 1).cloned() - } else if let Some(value) = arg.strip_prefix("--output=") { - Some(value.to_string()) - } else if arg.starts_with("-o") && arg.len() > 2 { - Some(arg[2..].trim_start_matches('=').to_string()) - } else { - None - } - }); + use clap::CommandFactory; + // ignore_errors keeps every argument clap could read; args_os, because an + // argument that is not UTF-8 (itself the error being reported) must not + // panic here. + let flag = Cli::command() + .ignore_errors(true) + .try_get_matches_from(std::env::args_os()) + .ok() + .and_then(|matches| matches.get_one::("output").copied()); let setting = flag - .and_then(|value| OutputSetting::parse(&value)) .or_else(|| { std::env::var("IDX_OUTPUT") .ok() diff --git a/crates/idx-cli/tests/cli.rs b/crates/idx-cli/tests/cli.rs index 76cef35..c3c4ff5 100644 --- a/crates/idx-cli/tests/cli.rs +++ b/crates/idx-cli/tests/cli.rs @@ -2117,6 +2117,29 @@ fn offline_and_no_cache_flags_are_rejected() { ); } +#[test] +fn argument_errors_follow_the_output_format_clap_resolves() { + // The last -o wins at any subcommand level, and -o may sit in a cluster: + // argument errors pick the same format a successful command would. + for args in [ + &["-o", "table", "config", "-o", "json", "get"][..], + &["-qojson", "config", "get"][..], + ] { + run_error_json(test_bin("argument-error-json").args(args), "INVALIDINPUT"); + } + for args in [ + &["-o", "json", "config", "-o", "table", "get"][..], + &["-qotable", "config", "get"][..], + ] { + test_bin("argument-error-table") + .args(args) + .assert() + .failure() + .stdout("") + .stderr(predicate::str::starts_with("error: ")); + } +} + #[test] fn msn_screen_rejects_invalid_filter() { test_bin("msn-screen-invalid-filter")