feat(derive): Allow skipping the implicit ArgGroup

This was prioritized to allow users to workaround problems when the implicit `ArgGroup` is getting in the way. Fixes clap-rs#4279
epage · Sep 30, 2022 · 2498147 · 2498147
1 parent 35eeba0
commit 2498147
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 3 deletions.
diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
@@ -315,11 +315,19 @@ pub fn gen_augment(
     let initial_app_methods = parent_item.initial_top_level_methods();
     let group_id = parent_item.ident().unraw().to_string();
     let final_app_methods = parent_item.final_top_level_methods();
+    let group_app_methods = if parent_item.skip_group() {
+        quote!()
+    } else {
+        quote!(
+            .group(clap::ArgGroup::new(#group_id).multiple(true))
+        )
+    };
     quote! {{
         #deprecations
         let #app_var = #app_var
             #initial_app_methods
-            .group(clap::ArgGroup::new(#group_id).multiple(true));
+            #group_app_methods
+            ;
         #( #args )*
         #app_var #final_app_methods
     }}

diff --git a/clap_derive/src/item.rs b/clap_derive/src/item.rs
@@ -50,6 +50,7 @@ pub struct Item {
     next_help_heading: Option<Method>,
     is_enum: bool,
     is_positional: bool,
+    skip_group: bool,
     kind: Sp<Kind>,
 }
 
@@ -272,6 +273,7 @@ impl Item {
             next_help_heading: None,
             is_enum: false,
             is_positional: true,
+            skip_group: false,
             kind,
         }
     }
@@ -334,6 +336,7 @@ impl Item {
                 continue;
             }
 
+            let actual_attr_kind = *attr.kind.get();
             let kind = match &attr.magic {
                 Some(MagicAttrName::FromGlobal) => {
                     if attr.value.is_some() {
@@ -377,7 +380,7 @@ impl Item {
                     let kind = Sp::new(Kind::Flatten, attr.name.clone().span());
                     Some(kind)
                 }
-                Some(MagicAttrName::Skip) => {
+                Some(MagicAttrName::Skip) if actual_attr_kind != AttrKind::Group => {
                     let expr = attr.value.clone();
                     let kind = Sp::new(
                         Kind::Skip(expr, self.kind.attr_kind()),
@@ -408,6 +411,8 @@ impl Item {
                     ));
                 }
 
+                (AttrKind::Group, AttrKind::Command) => {}
+
                 _ if attr.kind != expected_attr_kind => {
                     abort!(
                         attr.kind.span(),
@@ -803,6 +808,10 @@ impl Item {
                     self.env_casing = CasingStyle::from_lit(lit);
                 }
 
+                Some(MagicAttrName::Skip) if actual_attr_kind == AttrKind::Group => {
+                    self.skip_group = true;
+                }
+
                 None
                 // Magic only for the default, otherwise just forward to the builder
                 | Some(MagicAttrName::Short)
@@ -1029,6 +1038,10 @@ impl Item {
             .iter()
             .any(|m| m.name != "help" && m.name != "long_help")
     }
+
+    pub fn skip_group(&self) -> bool {
+        self.skip_group
+    }
 }
 
 #[derive(Clone)]

diff --git a/src/_derive/mod.rs b/src/_derive/mod.rs
@@ -4,6 +4,7 @@
 //! 2. [Attributes](#attributes)
 //!     1. [Terminology](#terminology)
 //!     2. [Command Attributes](#command-attributes)
+//!     2. [ArgGroup Attributes](#arggroup-attributes)
 //!     3. [Arg Attributes](#arg-attributes)
 //!     4. [ValueEnum Attributes](#valueenum-attributes)
 //!     5. [Possible Value Attributes](#possible-value-attributes)
@@ -28,6 +29,7 @@
 //! /// Doc comment
 //! #[derive(Parser)]
 //! #[command(CMD ATTRIBUTE)]
+//! #[group(GROUP ATTRIBUTE)]
 //! struct Cli {
 //!     /// Doc comment
 //!     #[arg(ARG ATTRIBUTE)]
@@ -46,6 +48,7 @@
 //! /// Doc comment
 //! #[derive(Args)]
 //! #[command(PARENT CMD ATTRIBUTE)]
+//! #[group(GROUP ATTRIBUTE)]
 //! struct Struct {
 //!     /// Doc comment
 //!     #[command(ARG ATTRIBUTE)]
@@ -173,6 +176,13 @@
 //! - `external_subcommand`: [`Command::allow_external_subcommand(true)`][crate::Command::allow_external_subcommands]
 //!   - Variant must be either `Variant(Vec<String>)` or `Variant(Vec<OsString>)`
 //!
+//! ### ArgGroup Attributes
+//!
+//! These correspond to the [`ArgGroup`][crate::ArgGroup] which is implicitly created for each
+//! `Args` derive.
+//!
+//! At the moment, only `#[group(skip)]` is supported
+//!
 //! ### Arg Attributes
 //!
 //! These correspond to a [`Arg`][crate::Arg].

diff --git a/tests/derive/groups.rs b/tests/derive/groups.rs
@@ -23,7 +23,6 @@ fn test_safely_nest_parser() {
 }
 
 #[test]
-#[should_panic = "'Compose' is already in use"]
 fn skip_group_avoids_duplicate_ids() {
     #[derive(Parser, Debug)]
     struct Opt {
@@ -34,6 +33,7 @@ fn skip_group_avoids_duplicate_ids() {
     }
 
     #[derive(clap::Args, Debug)]
+    #[group(skip)]
     pub struct Compose<L: clap::Args, R: clap::Args> {
         #[clap(flatten)]
         pub left: L,
@@ -42,6 +42,7 @@ fn skip_group_avoids_duplicate_ids() {
     }
 
     #[derive(clap::Args, Clone, Copy, Debug)]
+    #[group(skip)]
     pub struct Empty;
 
     use clap::CommandFactory;