From ca6d8769f9bfa74c5a63b9776a928e5138fa72a5 Mon Sep 17 00:00:00 2001 From: Chip Senkbeil Date: Sun, 29 Aug 2021 15:54:22 -0500 Subject: [PATCH 1/3] Add skip attribute support for enum variants --- structopt-derive/src/attrs.rs | 7 ++- structopt-derive/src/lib.rs | 97 +++++++++++++++++++---------------- tests/subcommands.rs | 46 +++++++++++++++++ 3 files changed, 103 insertions(+), 47 deletions(-) diff --git a/structopt-derive/src/attrs.rs b/structopt-derive/src/attrs.rs index aa161454..35acd5a4 100644 --- a/structopt-derive/src/attrs.rs +++ b/structopt-derive/src/attrs.rs @@ -405,6 +405,7 @@ impl Attrs { parent_attrs: Option<&Attrs>, argument_casing: Sp, env_casing: Sp, + allow_skip: bool, ) -> Self { let mut res = Self::new(span, name, parent_attrs, None, argument_casing, env_casing); res.push_attrs(attrs); @@ -418,8 +419,10 @@ impl Attrs { } match &*res.kind { Kind::Subcommand(_) => abort!(res.kind.span(), "subcommand is only allowed on fields"), - Kind::Skip(_) => abort!(res.kind.span(), "skip is only allowed on fields"), - Kind::Arg(_) | Kind::ExternalSubcommand | Kind::Flatten => res, + Kind::Skip(_) if !allow_skip => { + abort!(res.kind.span(), "skip is only allowed on fields") + } + Kind::Arg(_) | Kind::ExternalSubcommand | Kind::Flatten | Kind::Skip(_) => res, } } diff --git a/structopt-derive/src/lib.rs b/structopt-derive/src/lib.rs index b8e86de4..2afb1897 100644 --- a/structopt-derive/src/lib.rs +++ b/structopt-derive/src/lib.rs @@ -409,6 +409,7 @@ fn gen_clap(attrs: &[Attribute]) -> GenOutput { None, Sp::call_site(DEFAULT_CASING), Sp::call_site(DEFAULT_ENV_CASING), + false, ); let tokens = { let name = attrs.cased_name(); @@ -471,7 +472,7 @@ fn gen_augment_clap_enum( ) -> TokenStream { use syn::Fields::*; - let subcommands = variants.iter().map(|variant| { + let subcommands = variants.iter().filter_map(|variant| { let attrs = Attrs::from_struct( variant.span(), &variant.attrs, @@ -479,26 +480,29 @@ fn gen_augment_clap_enum( Some(parent_attribute), parent_attribute.casing(), parent_attribute.env_casing(), + true, ); let kind = attrs.kind(); match &*kind { + Kind::Skip(_) => None, + Kind::ExternalSubcommand => { let app_var = Ident::new("app", Span::call_site()); - quote_spanned! { attrs.kind().span()=> + Some(quote_spanned! { attrs.kind().span()=> let #app_var = #app_var.setting( ::structopt::clap::AppSettings::AllowExternalSubcommands ); - } + }) }, Kind::Flatten => { match variant.fields { Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => { let ty = &unnamed[0]; - quote! { + Some(quote! { let app = <#ty as ::structopt::StructOptInternal>::augment_clap(app); - } + }) }, _ => abort!( variant, @@ -535,13 +539,13 @@ fn gen_augment_clap_enum( let name = attrs.cased_name(); let from_attrs = attrs.top_level_methods(); let version = attrs.version(); - quote! { + Some(quote! { let app = app.subcommand({ let #app_var = ::structopt::clap::SubCommand::with_name(#name); let #app_var = #arg_block; #app_var#from_attrs#version }); - } + }) }, } }); @@ -588,58 +592,61 @@ fn gen_from_subcommand( Some(parent_attribute), parent_attribute.casing(), parent_attribute.env_casing(), + true, ); let variant_name = &variant.ident; - if let Kind::ExternalSubcommand = *attrs.kind() { - if ext_subcmd.is_some() { - abort!( - attrs.kind().span(), - "Only one variant can be marked with `external_subcommand`, \ + match *attrs.kind() { + Kind::ExternalSubcommand => { + if ext_subcmd.is_some() { + abort!( + attrs.kind().span(), + "Only one variant can be marked with `external_subcommand`, \ this is the second" - ); - } + ); + } - let ty = match variant.fields { - Unnamed(ref fields) if fields.unnamed.len() == 1 => &fields.unnamed[0].ty, + let ty = match variant.fields { + Unnamed(ref fields) if fields.unnamed.len() == 1 => &fields.unnamed[0].ty, - _ => abort!( - variant, - "The enum variant marked with `external_attribute` must be \ + _ => abort!( + variant, + "The enum variant marked with `external_attribute` must be \ a single-typed tuple, and the type must be either `Vec` \ or `Vec`." - ), - }; + ), + }; - let (span, str_ty, values_of) = match subty_if_name(ty, "Vec") { - Some(subty) => { - if is_simple_ty(subty, "String") { - ( - subty.span(), - quote!(::std::string::String), - quote!(values_of), - ) - } else { - ( - subty.span(), - quote!(::std::ffi::OsString), - quote!(values_of_os), - ) + let (span, str_ty, values_of) = match subty_if_name(ty, "Vec") { + Some(subty) => { + if is_simple_ty(subty, "String") { + ( + subty.span(), + quote!(::std::string::String), + quote!(values_of), + ) + } else { + ( + subty.span(), + quote!(::std::ffi::OsString), + quote!(values_of_os), + ) + } } - } - None => abort!( - ty, - "The type must be either `Vec` or `Vec` \ + None => abort!( + ty, + "The type must be either `Vec` or `Vec` \ to be used with `external_subcommand`." - ), - }; + ), + }; - ext_subcmd = Some((span, variant_name, str_ty, values_of)); - None - } else { - Some((variant, attrs)) + ext_subcmd = Some((span, variant_name, str_ty, values_of)); + None + } + Kind::Skip(_) => None, + _ => Some((variant, attrs)), } }) .partition(|(_, attrs)| match &*attrs.kind() { diff --git a/tests/subcommands.rs b/tests/subcommands.rs index 1fc8e76a..ccf2b22e 100644 --- a/tests/subcommands.rs +++ b/tests/subcommands.rs @@ -301,3 +301,49 @@ fn external_subcommand_optional() { assert_eq!(Opt::from_iter(&["test"]), Opt { sub: None }); } + +#[test] +fn skip_subcommand() { + #[derive(Debug, PartialEq, StructOpt)] + struct Opt { + #[structopt(subcommand)] + sub: Subcommands, + } + + #[derive(Debug, PartialEq, StructOpt)] + enum Subcommands { + Add, + Remove, + + #[allow(dead_code)] + #[structopt(skip)] + Skip, + } + + assert_eq!( + Opt::from_iter(&["test", "add"]), + Opt { + sub: Subcommands::Add + } + ); + + assert_eq!( + Opt::from_iter(&["test", "remove"]), + Opt { + sub: Subcommands::Remove + } + ); + + let res = Opt::from_iter_safe(&["test", "skip"]); + assert!( + matches!( + res, + Err(clap::Error { + kind: clap::ErrorKind::UnknownArgument, + .. + }), + ), + "Unexpected result: {:?}", + res + ); +} From b0055a7305058aaa5794232770997cc96ffd0808 Mon Sep 17 00:00:00 2001 From: Chip Senkbeil Date: Sun, 29 Aug 2021 15:57:26 -0500 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c35dd24..12ccca9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # v0.3.23 (unreleased) * Update minimal rust version to 1.46 because of bitflags 1.3 +* Add support for [skip for enum variant subcommands](https://github.com/TeXitoi/structopt/issues/493) # v0.3.22 (2021-07-04) From 6c3e232c306a075c560dd24fcd5cbef3138715ff Mon Sep 17 00:00:00 2001 From: Chip Senkbeil Date: Sun, 29 Aug 2021 16:04:07 -0500 Subject: [PATCH 3/3] Remove trailing comma in matches macro to support rust 1.46.0 --- tests/subcommands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/subcommands.rs b/tests/subcommands.rs index ccf2b22e..4ee738b1 100644 --- a/tests/subcommands.rs +++ b/tests/subcommands.rs @@ -341,7 +341,7 @@ fn skip_subcommand() { Err(clap::Error { kind: clap::ErrorKind::UnknownArgument, .. - }), + }) ), "Unexpected result: {:?}", res