From 020e23ea13f33d5191e2d1a73d99652cde4933dc Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Mon, 3 Nov 2025 10:22:41 -0800 Subject: [PATCH] Disable help triggers on subcommands --- native/src/base/argh.rs | 12 ++----- native/src/base/derive/argh/mod.rs | 37 ++++++++++++---------- native/src/base/derive/argh/parse_attrs.rs | 33 +++++++++++-------- 3 files changed, 43 insertions(+), 39 deletions(-) diff --git a/native/src/base/argh.rs b/native/src/base/argh.rs index 016d7dc72..020a75a43 100644 --- a/native/src/base/argh.rs +++ b/native/src/base/argh.rs @@ -697,7 +697,7 @@ pub fn parse_struct_args( 'parse_args: while let Some(&next_arg) = remaining_args.first() { remaining_args = &remaining_args[1..]; - if (parse_options.help_triggers().contains(&next_arg)) && !options_ended { + if (parse_options.help_triggers.contains(&next_arg)) && !options_ended { help = true; continue; } @@ -765,7 +765,7 @@ impl<'a> ParseStructOptions<'a> { .arg_to_slot .iter() .find_map(|&(name, pos)| if name == arg { Some(pos) } else { None }) - .ok_or_else(|| unrecognized_argument(arg, self.arg_to_slot, self.help_triggers()))?; + .ok_or_else(|| unrecognized_argument(arg, self.arg_to_slot, self.help_triggers))?; match self.slots[pos] { ParseStructOption::Flag(ref mut b) => b.set_flag(arg), @@ -791,14 +791,6 @@ impl<'a> ParseStructOptions<'a> { Ok(()) } - - fn help_triggers(&self) -> &'a [&'a str] { - if self.help_triggers.is_empty() { - &["-h", "--help"] - } else { - self.help_triggers - } - } } fn unrecognized_argument( diff --git a/native/src/base/derive/argh/mod.rs b/native/src/base/derive/argh/mod.rs index 68a9640fe..837345170 100644 --- a/native/src/base/derive/argh/mod.rs +++ b/native/src/base/derive/argh/mod.rs @@ -423,22 +423,27 @@ fn impl_from_args_struct_from_args<'a>( /// /// Defaults to vec!["-h", "--help"] if type_attrs.help_triggers is None fn get_help_triggers(type_attrs: &TypeAttrs) -> Vec { - type_attrs - .help_triggers - .as_ref() - .map_or_else(Vec::new, |s| { - s.iter() - .filter_map(|s| { - let trigger = s.value(); - let trigger_trimmed = trigger.trim().to_owned(); - if trigger_trimmed.is_empty() { - None - } else { - Some(trigger_trimmed) - } - }) - .collect::>() - }) + if type_attrs.is_subcommand.is_some() { + // Subcommands should never have any help triggers + Vec::new() + } else { + type_attrs.help_triggers.as_ref().map_or_else( + || vec!["-h".to_string(), "--help".to_string()], + |s| { + s.iter() + .filter_map(|s| { + let trigger = s.value(); + let trigger_trimmed = trigger.trim().to_owned(); + if trigger_trimmed.is_empty() { + None + } else { + Some(trigger_trimmed) + } + }) + .collect::>() + }, + ) + } } /// Ensures that only trailing positional args are non-required. diff --git a/native/src/base/derive/argh/parse_attrs.rs b/native/src/base/derive/argh/parse_attrs.rs index a7526e4a3..0d4b0de23 100644 --- a/native/src/base/derive/argh/parse_attrs.rs +++ b/native/src/base/derive/argh/parse_attrs.rs @@ -306,54 +306,61 @@ impl TypeAttrs { continue; } - let ml = if let Some(ml) = argh_attr_to_meta_list(errors, attr) { - ml + let ml: Vec = if let Some(ml) = argh_attr_to_meta_list(errors, attr) { + ml.into_iter().collect() } else { continue; }; - for meta in ml { + for meta in ml.iter() { let name = meta.path(); if name.is_ident("description") { - if let Some(m) = errors.expect_meta_name_value(&meta) { + if let Some(m) = errors.expect_meta_name_value(meta) { parse_attr_description(errors, m, &mut this.description); } } else if name.is_ident("error_code") { - if let Some(m) = errors.expect_meta_list(&meta) { + if let Some(m) = errors.expect_meta_list(meta) { this.parse_attr_error_code(errors, m); } } else if name.is_ident("example") { - if let Some(m) = errors.expect_meta_name_value(&meta) { + if let Some(m) = errors.expect_meta_name_value(meta) { this.parse_attr_example(errors, m); } } else if name.is_ident("name") { - if let Some(m) = errors.expect_meta_name_value(&meta) { + if let Some(m) = errors.expect_meta_name_value(meta) { this.parse_attr_name(errors, m); } } else if name.is_ident("note") { - if let Some(m) = errors.expect_meta_name_value(&meta) { + if let Some(m) = errors.expect_meta_name_value(meta) { this.parse_attr_note(errors, m); } } else if name.is_ident("subcommand") { - if let Some(ident) = errors.expect_meta_word(&meta).and_then(|p| p.get_ident()) - { + if let Some(ident) = errors.expect_meta_word(meta).and_then(|p| p.get_ident()) { this.parse_attr_subcommand(errors, ident); } } else if name.is_ident("help_triggers") { - if let Some(m) = errors.expect_meta_list(&meta) { + if let Some(m) = errors.expect_meta_list(meta) { Self::parse_help_triggers(m, errors, &mut this); } } else { errors.err( - &meta, + meta, concat!( "Invalid type-level `argh` attribute\n", "Expected one of: `description`, `error_code`, `example`, `name`, ", - "`note`, `subcommand`", + "`note`, `subcommand`, `help_triggers`", ), ); } } + + if this.is_subcommand.is_some() && this.help_triggers.is_some() { + let help_meta = ml + .iter() + .find(|meta| meta.path().is_ident("help_triggers")) + .unwrap(); + errors.err(help_meta, "Cannot use `help_triggers` on a subcommand"); + } } this.check_error_codes(errors);