Skip to content

Commit

Permalink
Merge #2823 #2824 #2827
Browse files Browse the repository at this point in the history
2823: fix(derive): Subcommands not working within macro_rules r=epage a=epage



2824: docs(derive): Fix explanation on optional string list argument r=epage a=epage



2827: feat: Add backtraces to errors r=epage a=epage



Co-authored-by: Ed Page <eopage@gmail.com>
  • Loading branch information
bors[bot] and epage committed Oct 7, 2021
4 parents 6c2daef + e42487f + 7761cf0 + 7b5a4c9 commit 3a1b02c
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 63 deletions.
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ termcolor = { version = "1.1", optional = true }
terminal_size = { version = "0.1.12", optional = true }
lazy_static = { version = "1", optional = true }
regex = { version = "1.0", optional = true }
backtrace = { version = "0.3", optional = true }

[dev-dependencies]
regex = "1.0"
Expand All @@ -89,7 +90,7 @@ default = [
"suggestions",
"unicode_help",
]
debug = ["clap_derive/debug"] # Enables debug messages
debug = ["clap_derive/debug", "backtrace"] # Enables debug messages

# Used in default
std = ["indexmap/std"] # support for no_std in a backwards-compatible way
Expand Down
2 changes: 1 addition & 1 deletion clap_derive/examples/example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ struct Opt {

// An optional list of values, will be `None` if not present on
// the command line, will be `Some(vec![])` if no argument is
// provided (i.e. `--optv`) and will be `Some(Some(String))` if
// provided (i.e. `--optv`) and will be `Some(Vec<String>)` if
// argument list is provided (e.g. `--optv a b c`).
#[clap(long)]
optv: Option<Vec<String>>,
Expand Down
10 changes: 5 additions & 5 deletions clap_derive/src/derives/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{

use proc_macro2::{Ident, Span, TokenStream};
use proc_macro_error::{abort, abort_call_site};
use quote::{quote, quote_spanned};
use quote::{format_ident, quote, quote_spanned};
use syn::{
punctuated::Punctuated, spanned::Spanned, token::Comma, Attribute, Data, DataStruct,
DeriveInput, Field, Fields, Type,
Expand Down Expand Up @@ -372,7 +372,7 @@ pub fn gen_constructor(fields: &Punctuated<Field, Comma>, parent_attribute: &Att
);
let field_name = field.ident.as_ref().unwrap();
let kind = attrs.kind();
let arg_matches = quote! { arg_matches };
let arg_matches = format_ident!("arg_matches");
match &*kind {
Kind::ExternalSubcommand => {
abort! { kind.span(),
Expand Down Expand Up @@ -438,7 +438,7 @@ pub fn gen_updater(
} else {
quote!()
};
let arg_matches = quote! { arg_matches };
let arg_matches = format_ident!("arg_matches");

match &*kind {
Kind::ExternalSubcommand => {
Expand Down Expand Up @@ -547,10 +547,10 @@ fn gen_parsers(
}
_ => &field.ty,
};
// Use `quote!` to give this identifier the same hygiene
// Give this identifier the same hygiene
// as the `arg_matches` parameter definition. This
// allows us to refer to `arg_matches` within a `quote_spanned` block
let arg_matches = quote! { arg_matches };
let arg_matches = format_ident!("arg_matches");

let field_value = match **ty {
Ty::Bool => {
Expand Down
9 changes: 5 additions & 4 deletions clap_derive/src/derives/into_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ pub fn gen_for_enum(enum_name: &Ident, attrs: &[Attribute]) -> TokenStream {
Sp::call_site(DEFAULT_ENV_CASING),
);
let name = attrs.cased_name();
let app_var = Ident::new("app", Span::call_site());

quote! {
#[allow(dead_code, unreachable_code, unused_variables)]
Expand All @@ -112,14 +113,14 @@ pub fn gen_for_enum(enum_name: &Ident, attrs: &[Attribute]) -> TokenStream {
#[deny(clippy::correctness)]
impl clap::IntoApp for #enum_name {
fn into_app<'b>() -> clap::App<'b> {
let app = clap::App::new(#name)
let #app_var = clap::App::new(#name)
.setting(clap::AppSettings::SubcommandRequiredElseHelp);
<#enum_name as clap::Subcommand>::augment_subcommands(app)
<#enum_name as clap::Subcommand>::augment_subcommands(#app_var)
}

fn into_app_for_update<'b>() -> clap::App<'b> {
let app = clap::App::new(#name);
<#enum_name as clap::Subcommand>::augment_subcommands_for_update(app)
let #app_var = clap::App::new(#name);
<#enum_name as clap::Subcommand>::augment_subcommands_for_update(#app_var)
}
}
}
Expand Down
62 changes: 33 additions & 29 deletions clap_derive/src/derives/subcommand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{

use proc_macro2::{Ident, Span, TokenStream};
use proc_macro_error::{abort, abort_call_site};
use quote::{quote, quote_spanned};
use quote::{format_ident, quote, quote_spanned};
use syn::{
punctuated::Punctuated, spanned::Spanned, Attribute, Data, DataEnum, DeriveInput,
FieldsUnnamed, Token, Variant,
Expand Down Expand Up @@ -119,6 +119,8 @@ fn gen_augment(
) -> TokenStream {
use syn::Fields::*;

let app_var = Ident::new("app", Span::call_site());

let subcommands: Vec<_> = variants
.iter()
.filter_map(|variant| {
Expand Down Expand Up @@ -147,11 +149,11 @@ fn gen_augment(
Some(subty) => {
if is_simple_ty(subty, "OsString") {
quote_spanned! { kind.span()=>
let app = app.setting(clap::AppSettings::AllowExternalSubcommands).setting(clap::AppSettings::AllowInvalidUtf8ForExternalSubcommands);
let #app_var = #app_var.setting(clap::AppSettings::AllowExternalSubcommands).setting(clap::AppSettings::AllowInvalidUtf8ForExternalSubcommands);
}
} else {
quote_spanned! { kind.span()=>
let app = app.setting(clap::AppSettings::AllowExternalSubcommands);
let #app_var = #app_var.setting(clap::AppSettings::AllowExternalSubcommands);
}
}
}
Expand All @@ -170,11 +172,11 @@ fn gen_augment(
let ty = &unnamed[0];
let subcommand = if override_required {
quote! {
let app = <#ty as clap::Subcommand>::augment_subcommands_for_update(app);
let #app_var = <#ty as clap::Subcommand>::augment_subcommands_for_update(#app_var);
}
} else {
quote! {
let app = <#ty as clap::Subcommand>::augment_subcommands(app);
let #app_var = <#ty as clap::Subcommand>::augment_subcommands(#app_var);
}
};
Some(subcommand)
Expand All @@ -186,24 +188,24 @@ fn gen_augment(
},

Kind::Subcommand(_) => {
let app_var = Ident::new("subcommand", Span::call_site());
let subcommand_var = Ident::new("subcommand", Span::call_site());
let arg_block = match variant.fields {
Named(_) => {
abort!(variant, "non single-typed tuple enums are not supported")
}
Unit => quote!( #app_var ),
Unit => quote!( #subcommand_var ),
Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => {
let ty = &unnamed[0];
if override_required {
quote_spanned! { ty.span()=>
{
<#ty as clap::Subcommand>::augment_subcommands_for_update(#app_var)
<#ty as clap::Subcommand>::augment_subcommands_for_update(#subcommand_var)
}
}
} else {
quote_spanned! { ty.span()=>
{
<#ty as clap::Subcommand>::augment_subcommands(#app_var)
<#ty as clap::Subcommand>::augment_subcommands(#subcommand_var)
}
}
}
Expand All @@ -216,35 +218,35 @@ fn gen_augment(
let name = attrs.cased_name();
let from_attrs = attrs.top_level_methods();
let subcommand = quote! {
let app = app.subcommand({
let #app_var = clap::App::new(#name);
let #app_var = #arg_block;
let #app_var = #app_var.setting(::clap::AppSettings::SubcommandRequiredElseHelp);
#app_var#from_attrs
let #app_var = #app_var.subcommand({
let #subcommand_var = clap::App::new(#name);
let #subcommand_var = #arg_block;
let #subcommand_var = #subcommand_var.setting(::clap::AppSettings::SubcommandRequiredElseHelp);
#subcommand_var#from_attrs
});
};
Some(subcommand)
}

_ => {
let app_var = Ident::new("subcommand", Span::call_site());
let subcommand_var = Ident::new("subcommand", Span::call_site());
let arg_block = match variant.fields {
Named(ref fields) => {
args::gen_augment(&fields.named, &app_var, &attrs, override_required)
args::gen_augment(&fields.named, &subcommand_var, &attrs, override_required)
}
Unit => quote!( #app_var ),
Unit => quote!( #subcommand_var ),
Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => {
let ty = &unnamed[0];
if override_required {
quote_spanned! { ty.span()=>
{
<#ty as clap::Args>::augment_args_for_update(#app_var)
<#ty as clap::Args>::augment_args_for_update(#subcommand_var)
}
}
} else {
quote_spanned! { ty.span()=>
{
<#ty as clap::Args>::augment_args(#app_var)
<#ty as clap::Args>::augment_args(#subcommand_var)
}
}
}
Expand All @@ -257,10 +259,10 @@ fn gen_augment(
let name = attrs.cased_name();
let from_attrs = attrs.top_level_methods();
let subcommand = quote! {
let app = app.subcommand({
let #app_var = clap::App::new(#name);
let #app_var = #arg_block;
#app_var#from_attrs
let #app_var = #app_var.subcommand({
let #subcommand_var = clap::App::new(#name);
let #subcommand_var = #arg_block;
#subcommand_var#from_attrs
});
};
Some(subcommand)
Expand All @@ -272,7 +274,7 @@ fn gen_augment(
let app_methods = parent_attribute.top_level_methods();
quote! {
#( #subcommands )*;
app #app_methods
#app_var #app_methods
}
}

Expand Down Expand Up @@ -352,6 +354,8 @@ fn gen_from_arg_matches(

let mut ext_subcmd = None;

let subcommand_name_var = format_ident!("name");
let sub_arg_matches_var = format_ident!("sub_arg_matches");
let (flatten_variants, variants): (Vec<_>, Vec<_>) = variants
.iter()
.filter_map(|variant| {
Expand Down Expand Up @@ -436,7 +440,7 @@ fn gen_from_arg_matches(
};

quote! {
if #sub_name == name {
if #sub_name == #subcommand_name_var {
return Some(#name :: #variant_name #constructor_block)
}
}
Expand All @@ -462,9 +466,9 @@ fn gen_from_arg_matches(
let wildcard = match ext_subcmd {
Some((span, var_name, str_ty, values_of)) => quote_spanned! { span=>
::std::option::Option::Some(#name::#var_name(
::std::iter::once(#str_ty::from(name))
::std::iter::once(#str_ty::from(#subcommand_name_var))
.chain(
sub_arg_matches.#values_of("").into_iter().flatten().map(#str_ty::from)
#sub_arg_matches_var.#values_of("").into_iter().flatten().map(#str_ty::from)
)
.collect::<::std::vec::Vec<_>>()
))
Expand All @@ -475,9 +479,9 @@ fn gen_from_arg_matches(

quote! {
fn from_arg_matches(arg_matches: &clap::ArgMatches) -> Option<Self> {
if let Some((name, sub_arg_matches)) = arg_matches.subcommand() {
if let Some((#subcommand_name_var, #sub_arg_matches_var)) = arg_matches.subcommand() {
{
let arg_matches = sub_arg_matches;
let arg_matches = #sub_arg_matches_var;
#( #subcommands )*
}

Expand Down
20 changes: 20 additions & 0 deletions clap_derive/tests/nested.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,23 @@ fn use_option() {

expand_ty!(my_field: Option<String>);
}

#[test]
fn issue_447() {
macro_rules! Command {
( $name:ident, [
#[$meta:meta] $var:ident($inner:ty)
] ) => {
#[derive(Debug, PartialEq, clap::Clap)]
enum $name {
#[$meta]
$var($inner),
}
};
}

Command! {GitCmd, [
#[clap(external_subcommand)]
Ext(Vec<String>)
]}
}

0 comments on commit 3a1b02c

Please sign in to comment.