fix(derive): Allow update_from when explicit requireds used

Fixes #4617
clap-rs · Jan 10, 2023 · af71ada · af71ada
1 parent b9ae787
commit af71ada
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 6 deletions.
diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs
@@ -187,7 +187,7 @@ pub fn gen_augment(
                     (Ty::Option, Some(sub_type)) => sub_type,
                     _ => &field.ty,
                 };
-                let implicit_methods = if **ty == Ty::Option || override_required {
+                let implicit_methods = if **ty == Ty::Option {
                     quote!()
                 } else {
                     quote_spanned! { kind.span()=>
@@ -196,10 +196,20 @@ pub fn gen_augment(
                     }
                 };
 
+                let override_methods = if override_required {
+                    quote_spanned! { kind.span()=>
+                        .subcommand_required(false)
+                        .arg_required_else_help(false)
+                    }
+                } else {
+                    quote!()
+                };
+
                 Some(quote! {
                     let #app_var = <#subcmd_type as clap::Subcommand>::augment_subcommands( #app_var );
                     let #app_var = #app_var
-                        #implicit_methods;
+                        #implicit_methods
+                        #override_methods;
                 })
             }
             Kind::Flatten(ty) => {
@@ -289,7 +299,7 @@ pub fn gen_augment(
                     }
 
                     Ty::Other => {
-                        let required = item.find_default_method().is_none() && !override_required;
+                        let required = item.find_default_method().is_none();
                         // `ArgAction::takes_values` is assuming `ArgAction::default_value` will be
                         // set though that won't always be true but this should be good enough,
                         // otherwise we'll report an "arg required" error when unwrapping.
@@ -310,6 +320,13 @@ pub fn gen_augment(
                 } else {
                     quote!()
                 };
+                let override_methods = if override_required {
+                    quote_spanned! { kind.span()=>
+                        .required(false)
+                    }
+                } else {
+                    quote!()
+                };
 
                 Some(quote_spanned! { field.span()=>
                     let #app_var = #app_var.arg({
@@ -321,6 +338,10 @@ pub fn gen_augment(
 
                         let arg = arg
                             #explicit_methods;
+
+                        let arg = arg
+                            #override_methods;
+
                         arg
                     });
                 })

diff --git a/clap_derive/src/derives/into_app.rs b/clap_derive/src/derives/into_app.rs
@@ -85,6 +85,8 @@ pub fn gen_for_enum(item: &Item, item_name: &Ident, generics: &Generics) -> Toke
             fn command_for_update<'b>() -> clap::Command {
                 let #app_var = clap::Command::new(#name);
                 <Self as clap::Subcommand>::augment_subcommands_for_update(#app_var)
+                    .subcommand_required(false)
+                    .arg_required_else_help(false)
             }
         }
     }

diff --git a/clap_derive/src/derives/subcommand.rs b/clap_derive/src/derives/subcommand.rs
@@ -244,6 +244,14 @@ fn gen_augment(
                     };
                     let initial_app_methods = item.initial_top_level_methods();
                     let final_from_attrs = item.final_top_level_methods();
+                    let override_methods = if override_required {
+                        quote_spanned! { kind.span()=>
+                            .subcommand_required(false)
+                            .arg_required_else_help(false)
+                        }
+                    } else {
+                        quote!()
+                    };
                     let subcommand = quote! {
                         let #app_var = #app_var.subcommand({
                             #deprecations;
@@ -253,7 +261,7 @@ fn gen_augment(
                             let #subcommand_var = #subcommand_var
                                 .subcommand_required(true)
                                 .arg_required_else_help(true);
-                            #subcommand_var #final_from_attrs
+                            #subcommand_var #final_from_attrs #override_methods
                         });
                     };
                     Some(subcommand)

diff --git a/tests/derive/basic.rs b/tests/derive/basic.rs
@@ -62,7 +62,15 @@ fn update_explicit_required() {
 
     let mut opt = Opt::try_parse_from(["test", "-f0", "-s1"]).unwrap();
 
-    assert!(opt.try_update_from(["test", "-f42"]).is_err());
+    opt.try_update_from(["test", "-f42"]).unwrap();
+
+    assert_eq!(
+        Opt {
+            first: 42,
+            second: 1
+        },
+        opt
+    );
 }
 
 #[test]

diff --git a/tests/derive/subcommands.rs b/tests/derive/subcommands.rs
@@ -397,7 +397,8 @@ fn update_subcommands_explicit_required() {
 
     // Full subcommand update
     let mut opt = Opt::Command1(Command1 { arg1: 12, arg2: 14 });
-    assert!(opt.try_update_from(["test"]).is_err(),);
+    opt.try_update_from(["test"]).unwrap();
+    assert_eq!(Opt::Command1(Command1 { arg1: 12, arg2: 14 }), opt);
 }
 
 #[test]