diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a0f9c6..d4a86c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * Update minimal rust version to 1.46 because of bitflags 1.3 * Fixed [a bug that occurs when the type of `map` becomes ambiguous](https://github.com/TeXitoi/structopt/issues/490). +* Add support for [skip for enum variant subcommands](https://github.com/TeXitoi/structopt/issues/493) # v0.3.22 (2021-07-04) diff --git a/structopt-derive/src/attrs.rs b/structopt-derive/src/attrs.rs index aa16145..35acd5a 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 b80ae17..b2835b4 100644 --- a/structopt-derive/src/lib.rs +++ b/structopt-derive/src/lib.rs @@ -416,6 +416,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(); @@ -478,7 +479,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, @@ -486,26 +487,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, @@ -542,13 +546,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 }); - } + }) }, } }); @@ -595,58 +599,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 1fc8e76..4ee738b 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 + ); +}