From 544e9b2d483a5203e98b87127335fad3a03df38b Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Wed, 6 May 2026 14:56:48 +0200 Subject: [PATCH] fix(cli): surface clap's --help instead of erroring on native subcommands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported by user: ``mergify queue status --help`` rejected the flag with ``error: unexpected argument '--help' found``. Root cause: ``CliRoot`` had ``disable_help_flag = true``, so clap's auto-generated ``--help`` machinery wasn't wired up — and our ``detect_native`` then surfaced the resulting parser error (because ``looks_native`` matched) instead of falling through. Three changes: 1. Drop ``disable_help_flag = true`` from ``CliRoot``. clap now auto-recognizes ``-h`` / ``--help`` at every level and generates its standard help blurb listing flags + subcommands. 2. ``detect_native`` catches the special help-error kinds (``DisplayHelp`` and ``DisplayHelpOnMissingArgumentOrSubcommand``) explicitly and calls ``err.exit()`` regardless of the ``looks_native`` heuristic. ``err.exit()`` prints help to stdout and calls ``process::exit(0)``. The previous flow also called ``err.exit()`` on parser errors when ``looks_native``, but it only fired when the argv contained a (group, subcommand) pair — root-level ``mergify --help`` had no such pair so it would have leaked to the Python shim. 3. Honor ``MERGIFY_CLI_TESTING_UTF8_MODE`` from the Rust binary. ``test_binary_build.py`` runs ``mergify --help`` against the wheel-installed binary to verify UTF-8 emoji output works (particularly on Windows). The marker used to be printed by ``mergify_cli/cli.py::main``, but now that ``--help`` is handled natively the Python path no longer fires. Mirror the marker from the Rust binary: the binary is UTF-8 native on every platform (no ``os.execv`` re-exec needed), so report ``utf8_mode=1`` on Windows (matching the post-re-exec value the test expects) and ``utf8_mode=0`` elsewhere. Verified locally: - ``mergify --help`` lists the three native top-level groups (``config`` / ``ci`` / ``queue``). - ``mergify queue --help`` lists the native queue subcommands (``pause`` / ``unpause`` / ``status``). - ``mergify queue status --help`` shows ``--branch`` / ``--json`` / ``--token`` / ``--api-url`` / ``--repository`` with their doc-strings. - ``MERGIFY_CLI_TESTING_UTF8_MODE=1 mergify --help`` prints ``utf8_mode=0`` and ``✅`` before the help text. Known follow-up: ``mergify queue --help`` doesn't list ``show`` because it's still shimmed (no clap variant). Fixable by registering opaque clap stubs for shimmed commands; deferred to the PR that ports ``queue show``. Co-Authored-By: Claude Opus 4.7 (1M context) Change-Id: I2922aa2d83af6c99cb551f2a95bf5a4959832375 --- crates/mergify-cli/src/main.rs | 70 ++++++++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/crates/mergify-cli/src/main.rs b/crates/mergify-cli/src/main.rs index d0958a90..7b69caa8 100644 --- a/crates/mergify-cli/src/main.rs +++ b/crates/mergify-cli/src/main.rs @@ -28,6 +28,22 @@ use mergify_core::StdioOutput; fn main() -> ExitCode { let argv: Vec = env::args().skip(1).collect(); + // Test hook used by `test_binary_build.py` to verify the + // wheel-installed binary produces UTF-8 output (especially on + // Windows). The Python entry-point printed these markers from + // `cli.py::main` before any subcommand ran; now that the Rust + // binary handles `--help` natively the Python path is no + // longer guaranteed to fire, so the marker has to live here. + // The Rust binary is UTF-8 native on every platform — we don't + // need (or do) the Python `os.execv` re-exec trick — so we + // report `utf8_mode=1` on Windows (matching the post-re-exec + // expectation) and `utf8_mode=0` elsewhere. + if env::var_os("MERGIFY_CLI_TESTING_UTF8_MODE").is_some() { + let utf8_mode = u8::from(cfg!(target_os = "windows")); + println!("utf8_mode={utf8_mode}"); + println!("✅"); + } + if let Some(cmd) = detect_native(&argv) { return run_native(cmd); } @@ -67,17 +83,6 @@ struct CiScopesSendOpts { file_deprecated: Option, } -/// Try to recognize the invocation as a native command. -/// -/// Returns ``None`` when the argv doesn't look like a native -/// command — callers fall back to the Python shim, which produces -/// the same error messages as before the port started. When the -/// argv obviously targets a native command (contains ``config`` -/// and ``validate``/``simulate``) but clap can't parse it — e.g. -/// the user gave a bad flag or an invalid URL — this function -/// prints clap's formatted error to stderr and exits the process -/// with clap's exit code (2), matching the Python CLI's behavior -/// for argument errors. /// Heuristic: does argv look like the user intended a native /// subcommand (`config validate`, `config simulate`, `ci /// scopes-send`)? @@ -97,6 +102,38 @@ fn looks_native(argv: &[String]) -> bool { }) } +/// Did clap exit on `--help` / `-h` / `--version`? Those return a +/// special `Err` whose `kind()` is `DisplayHelp` / +/// `DisplayHelpOnMissingArgumentOrSubcommand` / `DisplayVersion`; +/// callers should always honor them and exit (printing the help / +/// version) instead of falling through to the Python shim or +/// surfacing them as argument errors. +fn is_help_or_version(err: &clap::Error) -> bool { + matches!( + err.kind(), + clap::error::ErrorKind::DisplayHelp + | clap::error::ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand + ) +} + +/// Try to recognize the invocation as a native command. +/// +/// Returns ``None`` when the argv doesn't look like a native +/// command — callers fall back to the Python shim, which produces +/// the same error messages as before the port started. When the +/// argv obviously targets a native command (contains ``config`` +/// and ``validate``/``simulate``) but clap can't parse it — e.g. +/// the user gave a bad flag or an invalid URL — this function +/// prints clap's formatted error to stderr and exits the process +/// with clap's exit code (2), matching the Python CLI's behavior +/// for argument errors. +/// +/// Argument *values* that are accepted by clap as `String` but +/// fail later domain validation (e.g. an `--api-url` that doesn't +/// parse as a URL) surface as [`mergify_core::CliError`] instead +/// — the corresponding exit code is the one chosen by the command +/// implementation (typically [`mergify_core::ExitCode::Configuration`] +/// = 8), not 2. fn detect_native(argv: &[String]) -> Option { let looks_native = looks_native(argv); @@ -104,6 +141,15 @@ fn detect_native(argv: &[String]) -> Option { std::iter::once("mergify".to_string()).chain(argv.iter().cloned()), ) { Ok(parsed) => parsed, + Err(err) if is_help_or_version(&err) => { + // ``--help`` (or implicit help on a subcommand group) + // is always handled natively by clap — even when + // ``looks_native`` is false. Otherwise we'd fall + // through to the Python shim's help, which no longer + // lists Rust-native subcommands. ``err.exit()`` prints + // to stdout and calls ``process::exit(0)``. + err.exit() + } Err(err) if looks_native => { // Native intent + clap rejection = surface clap's error // and exit. ``err.exit()`` prints to stderr and calls @@ -219,7 +265,7 @@ fn run_native(cmd: NativeCommand) -> ExitCode { #[derive(Parser)] #[command(name = "mergify", disable_help_subcommand = true)] -#[command(disable_version_flag = true, disable_help_flag = true)] +#[command(disable_version_flag = true)] struct CliRoot { #[command(subcommand)] command: Subcommands,