From c8e79b8f7b83205bb34d7f7bca80bb9ac4933cf0 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 13:06:16 +0000 Subject: [PATCH] fix(cli): keep a recovered -o when a retry reaches --help or --version Dropping an unknown argument can expose a later --help, and clap then returns no matches, losing an -o already read. Keep the last pass that read one (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 | 21 ++++++++++++++------- crates/idx-cli/tests/cli.rs | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index 6c1f349..df02338 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -128,11 +128,21 @@ fn requested_output_from_args() -> OutputFormat { } /// clap stops at an unknown argument, so an `-o` after it would be missed; -/// drop each unknown argument and parse again. What is left is read with -/// ignore_errors, which keeps the matches made before any other error. +/// drop each unknown argument and parse again. Each pass is read with +/// ignore_errors, which keeps the matches made before any other error; the +/// last pass that read an `-o` wins (a later pass can reach `--help`, which +/// returns no matches at all). fn output_flag(mut args: Vec) -> Option { use clap::CommandFactory; use clap::error::{ContextKind, ContextValue, ErrorKind}; + let read = |args: &[std::ffi::OsString]| { + Cli::command() + .ignore_errors(true) + .try_get_matches_from(args) + .ok() + .and_then(|matches| matches.get_one::("output").copied()) + }; + let mut found = read(&args); for _ in 0..args.len() { let Err(err) = Cli::command().try_get_matches_from(&args) else { break; @@ -150,12 +160,9 @@ fn output_flag(mut args: Vec) -> Option { break; }; args.remove(index + 1); + found = read(&args).or(found); } - Cli::command() - .ignore_errors(true) - .try_get_matches_from(args) - .ok() - .and_then(|matches| matches.get_one::("output").copied()) + found } fn bootstrap_output_format(cli: &Cli) -> OutputFormat { diff --git a/crates/idx-cli/tests/cli.rs b/crates/idx-cli/tests/cli.rs index 9ab558e..5cdd8ee 100644 --- a/crates/idx-cli/tests/cli.rs +++ b/crates/idx-cli/tests/cli.rs @@ -2134,6 +2134,25 @@ fn argument_errors_follow_the_output_format_clap_resolves() { .args(["config", "get", "--bogus", "-o", "json"]), "INVALIDINPUT", ); + // ...and so does one before it when --help or --version follows. + for args in [ + &["-o", "json", "config", "get", "--bogus", "--help"][..], + &["-o", "json", "--bogus", "--version"][..], + ] { + run_error_json( + test_bin("argument-error-json") + .env("IDX_OUTPUT", "table") + .args(args), + "INVALIDINPUT", + ); + } + test_bin("argument-error-table") + .env("IDX_OUTPUT", "json") + .args(["-o", "table", "config", "get", "--bogus", "--help"]) + .assert() + .failure() + .stdout("") + .stderr(predicate::str::starts_with("error: ")); test_bin("argument-error-table") .args(["config", "get", "--bogus", "-o", "table"]) .assert()