Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External subcommand #1883

Merged
merged 4 commits into from
May 1, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions clap_derive/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub enum Kind {
Subcommand(Sp<Ty>),
Flatten,
Skip(Option<Expr>),
ExternalSubcommand,
}

#[derive(Clone)]
Expand Down Expand Up @@ -299,6 +300,11 @@ impl Attrs {
self.set_kind(kind);
}

ExternalSubcommand(ident) => {
let kind = Sp::new(Kind::ExternalSubcommand, ident.span());
self.set_kind(kind);
}

Flatten(ident) => {
let kind = Sp::new(Kind::Flatten, ident.span());
self.set_kind(kind);
Expand Down Expand Up @@ -425,7 +431,7 @@ 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::Flatten => res,
Kind::Arg(_) | Kind::Flatten | Kind::ExternalSubcommand => res,
}
}

Expand Down Expand Up @@ -460,6 +466,13 @@ impl Attrs {
);
}
}

Kind::ExternalSubcommand => {
abort! { res.kind.span(),
"`external_subcommand` can be used only on enum variants"
}
}

Kind::Subcommand(_) => {
if res.has_custom_parser {
abort!(
Expand Down Expand Up @@ -571,7 +584,7 @@ impl Attrs {
} else {
abort!(
kind.span(),
"subcommand, flatten and skip cannot be used together"
"`subcommand`, `flatten`, `external_subcommand` and `skip` cannot be used together"
);
}
}
Expand Down
8 changes: 7 additions & 1 deletion clap_derive/src/derives/from_argmatches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// commit#ea76fa1b1b273e65e3b0b1046643715b49bec51f which is licensed under the
// MIT/Apache 2.0 license.
use proc_macro2::TokenStream;
use proc_macro_error::abort;
use quote::{quote, quote_spanned};
use syn::{punctuated::Punctuated, spanned::Spanned, Field, Ident, Token, Type};

Expand Down Expand Up @@ -92,7 +93,12 @@ pub fn gen_constructor(
);
let field_name = field.ident.as_ref().unwrap();
let kind = attrs.kind();
match &*attrs.kind() {
match &*kind {
Kind::ExternalSubcommand => {
abort! { kind.span(),
"`external_subcommand` can be used only on enum variants"
}
}
Kind::Subcommand(ty) => {
let subcmd_type = match (**ty, sub_type(&field.ty)) {
(Ty::Option, Some(sub_type)) => sub_type,
Expand Down
2 changes: 1 addition & 1 deletion clap_derive/src/derives/into_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ pub fn gen_app_augmentation(
);
let kind = attrs.kind();
match &*kind {
Kind::Subcommand(_) | Kind::Skip(_) => None,
Kind::Subcommand(_) | Kind::Skip(_) | Kind::ExternalSubcommand => None,
Kind::Flatten => {
let ty = &field.ty;
Some(quote_spanned! { kind.span()=>
Expand Down
197 changes: 144 additions & 53 deletions clap_derive/src/derives/subcommand.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
attrs::{Attrs, Kind, Name, DEFAULT_CASING, DEFAULT_ENV_CASING},
derives::{from_argmatches, into_app},
utils::Sp,
utils::{is_simple_ty, subty_if_name, Sp},
};

use proc_macro2::{Ident, Span, TokenStream};
Expand Down Expand Up @@ -49,62 +49,72 @@ fn gen_augment_subcommands(
) -> TokenStream {
use syn::Fields::*;

let subcommands = variants.iter().map(|variant| {
let attrs = Attrs::from_struct(
variant.span(),
&variant.attrs,
Name::Derived(variant.ident.clone()),
parent_attribute.casing(),
parent_attribute.env_casing(),
);
let kind = attrs.kind();
match &*kind {
Kind::Flatten => match variant.fields {
Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => {
let ty = &unnamed[0];
quote! {
let app = <#ty as ::clap::Subcommand>::augment_subcommands(app);
let subcommands: Vec<_> = variants
.iter()
.map(|variant| {
let attrs = Attrs::from_struct(
variant.span(),
&variant.attrs,
Name::Derived(variant.ident.clone()),
parent_attribute.casing(),
parent_attribute.env_casing(),
);
let kind = attrs.kind();

match &*kind {
Kind::ExternalSubcommand => {
quote_spanned! { kind.span()=>
let app = app.setting(::clap::AppSettings::AllowExternalSubcommands);
}
}
_ => abort!(
variant,
"`flatten` is usable only with single-typed tuple variants"
),
},

_ => {
let app_var = Ident::new("subcommand", Span::call_site());
let arg_block = match variant.fields {
Named(ref fields) => {
into_app::gen_app_augmentation(&fields.named, &app_var, &attrs)
}
Unit => quote!( #app_var ),

Kind::Flatten => match variant.fields {
Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => {
let ty = &unnamed[0];
quote_spanned! { ty.span()=>
{
<#ty as ::clap::IntoApp>::augment_clap(#app_var)
}
quote! {
let app = <#ty as ::clap::Subcommand>::augment_subcommands(app);
}
}
Unnamed(..) => {
abort!(variant, "non single-typed tuple enums are not supported")
}
};
_ => abort!(
variant,
"`flatten` is usable only with single-typed tuple variants"
),
},

let name = attrs.cased_name();
let from_attrs = attrs.top_level_methods();
let version = attrs.version();
quote! {
let app = app.subcommand({
let #app_var = ::clap::App::new(#name);
let #app_var = #arg_block;
#app_var#from_attrs#version
});
_ => {
let app_var = Ident::new("subcommand", Span::call_site());
let arg_block = match variant.fields {
Named(ref fields) => {
into_app::gen_app_augmentation(&fields.named, &app_var, &attrs)
}
Unit => quote!( #app_var ),
Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => {
let ty = &unnamed[0];
quote_spanned! { ty.span()=>
{
<#ty as ::clap::IntoApp>::augment_clap(#app_var)
}
}
}
Unnamed(..) => {
abort!(variant, "non single-typed tuple enums are not supported")
}
};

let name = attrs.cased_name();
let from_attrs = attrs.top_level_methods();
let version = attrs.version();
quote! {
let app = app.subcommand({
let #app_var = ::clap::App::new(#name);
let #app_var = #arg_block;
#app_var#from_attrs#version
});
}
}
}
}
});
})
.collect();
Copy link
Member

@pksunkara pksunkara Apr 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need the collect here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need collect here so this mapping is executed right now, not lazily. I need it because it sets ext_subcmd and the code below relies on it.


let app_methods = parent_attribute.top_level_methods();
let version = parent_attribute.version();
Expand All @@ -123,17 +133,69 @@ fn gen_from_subcommand(
parent_attribute: &Attrs,
) -> TokenStream {
use syn::Fields::*;

let mut ext_subcmd = None;

let (flatten_variants, variants): (Vec<_>, Vec<_>) = variants
.iter()
.map(|variant| {
.filter_map(|variant| {
let attrs = Attrs::from_struct(
variant.span(),
&variant.attrs,
Name::Derived(variant.ident.clone()),
parent_attribute.casing(),
parent_attribute.env_casing(),
);
(variant, attrs)

if let Kind::ExternalSubcommand = &*attrs.kind() {
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,

_ => abort!(
variant.span(),
"The enum variant marked with `external_attribute` must be \
a single-typed tuple, and the type must be either `Vec<String>` \
or `Vec<OsString>`."
),
};

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.span(),
"The type must be either `Vec<String>` or `Vec<OsString>` \
to be used with `external_subcommand`."
),
};

ext_subcmd = Some((span, &variant.ident, str_ty, values_of));
None
} else {
Some((variant, attrs))
}
})
.partition(|(_, attrs)| {
let kind = attrs.kind();
Expand Down Expand Up @@ -180,16 +242,45 @@ fn gen_from_subcommand(
}
});

let wildcard = match ext_subcmd {
Some((span, var_name, str_ty, values_of)) => quote_spanned! { span=>
("", ::std::option::Option::None) => ::std::option::Option::None,

(external, Some(matches)) => {
::std::option::Option::Some(#name::#var_name(
::std::iter::once(#str_ty::from(external))
.chain(
matches.#values_of("").into_iter().flatten().map(#str_ty::from)
)
.collect::<::std::vec::Vec<_>>()
))
}

(external, None) => {
::std::option::Option::Some(#name::#var_name({
let mut v = ::std::vec::Vec::with_capacity(1);
v.push(#str_ty::from(external));
v
}))
}
},

None => quote!(_ => None),
};

quote! {
fn from_subcommand<'b>(
name: &'b str,
sub: Option<&'b ::clap::ArgMatches>) -> Option<Self>
{
match (name, sub) {
#( #match_arms ),*,
#( #match_arms, )*
other => {
#( #child_subcommands )*;
None
#( #child_subcommands )else*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the else here.

Copy link
Contributor Author

@CreepySkeleton CreepySkeleton Apr 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This else serves as a separator for if statements.

Before:

if let Some(...) = ... {
   //...
}
if let Some(...) = ... {
   //...
}
// ...

After:

if let Some(...) = ... {
   //...
} else if let Some(...) = ... {
   //...
}
// ...

The new variant looks better in cargo expand, this is the whole motivation


match other {
#wildcard
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion clap_derive/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub enum ClapAttr {
ArgEnum(Ident),
Subcommand(Ident),
VerbatimDocComment(Ident),
ExternalSubcommand(Ident),

// ident [= "string literal"]
About(Ident, Option<LitStr>),
Expand Down Expand Up @@ -170,7 +171,7 @@ impl Parse for ClapAttr {
"flatten" => Ok(Flatten(name)),
"arg_enum" => Ok(ArgEnum(name)),
"subcommand" => Ok(Subcommand(name)),

"external_subcommand" => Ok(ExternalSubcommand(name)),
"verbatim_doc_comment" => Ok(VerbatimDocComment(name)),

"default_value" => Ok(DefaultValue(name, None)),
Expand Down
2 changes: 1 addition & 1 deletion clap_derive/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ mod ty;
pub use self::{
doc_comments::process_doc_comment,
spanned::Sp,
ty::{sub_type, subty_if_name, Ty},
ty::{is_simple_ty, sub_type, subty_if_name, Ty},
};
2 changes: 1 addition & 1 deletion clap_derive/src/utils/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub fn subty_if_name<'a>(ty: &'a syn::Type, name: &str) -> Option<&'a syn::Type>
subty_if(ty, |seg| seg.ident == name)
}

fn is_simple_ty(ty: &syn::Type, name: &str) -> bool {
pub fn is_simple_ty(ty: &syn::Type, name: &str) -> bool {
only_last_segment(ty)
.map(|segment| {
if let PathArguments::None = segment.arguments {
Expand Down
Loading