Skip to content

Commit c21db13

Browse files
jpage-godaddyclaudejgowdy-godaddy
authored
fix: render help for <group> help subcommand form (#15)
## Summary `<group> help` (e.g. `gddy auth help`, `gddy application help`) was reported as broken: it returned `unknown command "help" for "gddy auth"` even though the group's help listing advertises a `help` command. ### Root cause The engine ships a *curated* root `help` command, so it calls `.disable_help_subcommand(true)` on the root. In clap v4 that setting **propagates to every subcommand and cannot be re-enabled per child**, so no group has a working `help` subcommand at parse time — yet the group help *listing* still shows one (it's rendered via `render_long_help()`, which builds the group in isolation and re-adds `help`). On top of that, a pre-flight check (`unknown_group_command_message`) reported the `help` token as an unknown command before clap ran. ### Fix Detect the `<group> help [sub...]` form in `Cli::run` and render the target's help directly, reusing the curated help-rendering path via a new shared `render_help_for_parts` helper. This matches clap's documented equivalence between `cmd group help sub` and `cmd help group sub`. Scoped to **groups** (pure subcommand dispatch, so a `help` token there is unambiguously a help request). Leaf commands are left to clap so a literal `help` positional argument still parses, and `<leaf> --help` is unaffected. ## Testing - Added `cli_runtime_group_help_subcommand_renders_group_help` (TDD: confirmed it fails before the fix, passes after). Covers `<group> help` and `<group> help <sub>`. - `cargo fmt --check`, `clippy -D warnings`, `cargo doc -D warnings`, full test suite + doctests all pass. - Verified end-to-end against the real `gddy` CLI (patched to this engine): `gddy auth help`, `gddy application help`, and `gddy auth help status` all render help with exit 0; root `help`, `help auth`, `auth login --help`, and the `auth bogus` unknown-command error are unaffected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Jay Gowdy <jgowdy@godaddy.com>
1 parent 7aab74a commit c21db13

2 files changed

Lines changed: 366 additions & 13 deletions

File tree

src/cli.rs

Lines changed: 167 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -794,7 +794,7 @@ impl Cli {
794794
.iter()
795795
.map(|arg| arg.to_string_lossy().into_owned())
796796
.collect::<Vec<_>>();
797-
let clap_args = normalize_optional_global_flags_before_command(&self.root, &text_args);
797+
let mut clap_args = normalize_optional_global_flags_before_command(&self.root, &text_args);
798798
if has_root_version_flag(&text_args, &self.root, &self.config.name) {
799799
return self.finish_run(CliRunOutput {
800800
exit_code: 0,
@@ -811,9 +811,43 @@ impl Cli {
811811
if let Some(output) = self.try_run_search_bypass(&text_args) {
812812
return output;
813813
}
814-
if let Some(message) =
815-
unknown_group_command_message(&self.root, &text_args, &self.config.name)
814+
// Resolve the positional command path once and share it between the
815+
// group-help rewrite and the unknown-command check below.
816+
let bool_flags = derive_bool_flags(&self.root);
817+
let value_flags = derive_value_flags(&self.root);
818+
let positionals =
819+
positional_command_tokens(&text_args, &self.config.name, &bool_flags, &value_flags);
820+
// Positional tokens after a `--` separator are literal operands, not
821+
// command keywords, so the group-help shim must not treat a `help`
822+
// among them as a help request. Count the positionals that precede any
823+
// `--` to mark where genuine command keywords end.
824+
let command_keyword_count = match text_args.iter().position(|arg| arg == "--") {
825+
Some(end) => positional_command_tokens(
826+
&text_args[..end],
827+
&self.config.name,
828+
&bool_flags,
829+
&value_flags,
830+
)
831+
.len(),
832+
None => positionals.len(),
833+
};
834+
if let Some(parts) =
835+
group_help_target_parts(&self.root, &positionals, command_keyword_count)
816836
{
837+
// Rewrite `<group> help [sub...]` into the canonical
838+
// `help <group> [sub...]` so it flows through the curated root
839+
// `help` command, which also runs global-flag parsing and the
840+
// `pre_run` hook (matching `help <group>` and bare-group help).
841+
// Only the positional command tokens are reordered; every flag and
842+
// its value is preserved in place so e.g. `--output json` survives.
843+
clap_args = rewrite_group_help_args(
844+
&clap_args,
845+
&self.config.name,
846+
&bool_flags,
847+
&value_flags,
848+
&parts,
849+
);
850+
} else if let Some(message) = unknown_group_command_message(&self.root, &positionals) {
817851
return self.finish_run(CliRunOutput {
818852
exit_code: 1,
819853
rendered: message,
@@ -1209,13 +1243,23 @@ impl Cli {
12091243
.get_many::<String>("command")
12101244
.map(|values| values.map(String::as_str).collect::<Vec<_>>())
12111245
.unwrap_or_default();
1246+
self.render_help_for_parts(&parts)
1247+
}
1248+
1249+
/// Renders the curated help text for a resolved command path.
1250+
///
1251+
/// Empty `parts` render the root help. A path that resolves to a group or
1252+
/// command renders that command's long help; an unresolved path returns the
1253+
/// standard "unknown command" guidance with a non-zero exit code. Shared by
1254+
/// the root `help <path>` command and the `<group> help` subcommand form.
1255+
fn render_help_for_parts(&self, parts: &[&str]) -> CliRunOutput {
12121256
if parts.is_empty() {
12131257
return CliRunOutput {
12141258
exit_code: 0,
12151259
rendered: self.root.clone().render_long_help().to_string(),
12161260
};
12171261
}
1218-
let Some(command) = find_help_target(&self.root, &parts) else {
1262+
let Some(command) = find_help_target(&self.root, parts) else {
12191263
return CliRunOutput {
12201264
exit_code: 1,
12211265
rendered: format!(
@@ -1767,22 +1811,15 @@ fn direct_subcommand<'command>(
17671811
})
17681812
}
17691813

1770-
fn unknown_group_command_message(
1771-
root: &Command,
1772-
args: &[String],
1773-
root_name: &str,
1774-
) -> Option<String> {
1775-
let bool_flags = derive_bool_flags(root);
1776-
let value_flags = derive_value_flags(root);
1777-
let positionals = positional_command_tokens(args, root_name, &bool_flags, &value_flags);
1814+
fn unknown_group_command_message(root: &Command, positionals: &[String]) -> Option<String> {
17781815
if positionals.is_empty() {
17791816
return None;
17801817
}
17811818

17821819
let mut current = root;
17831820
let mut path = vec![root.get_name().to_owned()];
17841821
for token in positionals {
1785-
if let Some(next) = current.find_subcommand(&token) {
1822+
if let Some(next) = current.find_subcommand(token) {
17861823
current = next;
17871824
path.push(next.get_name().to_owned());
17881825
continue;
@@ -1798,6 +1835,123 @@ fn unknown_group_command_message(
17981835
None
17991836
}
18001837

1838+
/// Detects the `<group> help [sub...]` form and returns the command path whose
1839+
/// help should be rendered.
1840+
///
1841+
/// The engine ships a curated root `help` command, so it disables clap's
1842+
/// auto-generated help subcommand on the root. That setting propagates to every
1843+
/// subcommand and cannot be re-enabled per child, so `<group> help` would
1844+
/// otherwise hit clap's "unrecognized subcommand" error even though the group's
1845+
/// help listing advertises a `help` entry. We recognize the form here so the
1846+
/// caller can route it through the curated help renderer, matching clap's
1847+
/// documented equivalence between `cmd group help sub` and `cmd help group sub`.
1848+
///
1849+
/// Only groups (commands that have subcommands) are matched: a group is pure
1850+
/// subcommand dispatch, so a `help` token in that position is unambiguously a
1851+
/// help request. Leaf commands may accept a literal `help` positional argument,
1852+
/// so they are left for clap to parse (`<leaf> --help` still works). A group
1853+
/// that registers its own real `help` subcommand is likewise deferred to clap,
1854+
/// which dispatches the user-defined command (only auto-generated help is
1855+
/// suppressed).
1856+
///
1857+
/// `command_keyword_count` is the number of leading positionals that are
1858+
/// genuine command keywords (those before any `--`). A `help` at or beyond that
1859+
/// index is a literal operand after `--`, not a help request, so it is ignored.
1860+
fn group_help_target_parts(
1861+
root: &Command,
1862+
positionals: &[String],
1863+
command_keyword_count: usize,
1864+
) -> Option<Vec<String>> {
1865+
let help_index = positionals.iter().position(|token| token == "help")?;
1866+
// A leading `help` is the curated root help command; let it flow through.
1867+
if help_index == 0 {
1868+
return None;
1869+
}
1870+
// A `help` after a `--` separator is a literal operand; leave it for clap.
1871+
if help_index >= command_keyword_count {
1872+
return None;
1873+
}
1874+
let prefix = &positionals[..help_index];
1875+
let mut current = root;
1876+
for token in prefix {
1877+
current = current.find_subcommand(token)?;
1878+
}
1879+
// The token before `help` must resolve to a group; leaves are left to clap.
1880+
current.get_subcommands().next()?;
1881+
// Defer to clap when the group defines a real `help` subcommand of its own.
1882+
if current.find_subcommand("help").is_some() {
1883+
return None;
1884+
}
1885+
// `<group> help <sub...>` shows help for `<group> <sub...>`.
1886+
let suffix = &positionals[help_index + 1..];
1887+
Some(prefix.iter().chain(suffix).cloned().collect())
1888+
}
1889+
1890+
/// Rewrites a `<group> help [sub...]` invocation into the canonical
1891+
/// `help <group> [sub...]` argument vector.
1892+
///
1893+
/// Only the positional command tokens are reordered (from `[group..., help,
1894+
/// sub...]` to `[help, group..., sub...]`); every flag — including `key=value`
1895+
/// forms, value-consuming flags, unknown flags that consume a value, and
1896+
/// anything after `--` — is preserved in its original place. Reordering keeps
1897+
/// the positional count unchanged, so the rewritten stream is filled slot for
1898+
/// slot. `parts` is the resolved command path (group + subcommand) from
1899+
/// [`group_help_target_parts`].
1900+
fn rewrite_group_help_args(
1901+
clap_args: &[String],
1902+
root_name: &str,
1903+
bool_flags: &BTreeSet<String>,
1904+
value_flags: &BTreeSet<String>,
1905+
parts: &[String],
1906+
) -> Vec<String> {
1907+
// New positional order: the curated `help` command, then the command path.
1908+
let mut next_positional = std::iter::once("help".to_owned())
1909+
.chain(parts.iter().cloned())
1910+
.peekable();
1911+
let mut out = Vec::with_capacity(clap_args.len());
1912+
let mut iter = clap_args.iter().peekable();
1913+
if iter
1914+
.peek()
1915+
.is_some_and(|arg| arg_matches_root_name(arg, root_name))
1916+
&& let Some(program) = iter.next()
1917+
{
1918+
out.push(program.clone());
1919+
}
1920+
1921+
let mut take_positional =
1922+
|fallback: &String| next_positional.next().unwrap_or(fallback.clone());
1923+
1924+
while let Some(arg) = iter.next() {
1925+
if arg == "--" {
1926+
out.push(arg.clone());
1927+
// Everything after `--` is positional.
1928+
for rest in iter.by_ref() {
1929+
out.push(take_positional(rest));
1930+
}
1931+
break;
1932+
}
1933+
if arg.contains('=') || bool_flags.contains(arg) {
1934+
out.push(arg.clone());
1935+
continue;
1936+
}
1937+
if value_flags.contains(arg) || unknown_flag_consumes_value(arg, iter.peek()) {
1938+
out.push(arg.clone());
1939+
if let Some(value) = iter.next() {
1940+
out.push(value.clone());
1941+
}
1942+
continue;
1943+
}
1944+
if arg.starts_with('-') {
1945+
out.push(arg.clone());
1946+
continue;
1947+
}
1948+
out.push(take_positional(arg));
1949+
}
1950+
// Defensive: emit any positionals not yet placed (counts normally match).
1951+
out.extend(next_positional);
1952+
out
1953+
}
1954+
18011955
fn positional_command_tokens(
18021956
args: &[String],
18031957
root_name: &str,

0 commit comments

Comments
 (0)