From 4d50cb180ed387cdc115c478983f87e005dc7411 Mon Sep 17 00:00:00 2001 From: "Kentrix (Edward)" Date: Wed, 15 Jun 2022 13:16:53 +0000 Subject: [PATCH] Tweaked output for default subcommands. Addressed various PR feedback. --- README.md | 2 +- argh/src/lib.rs | 95 ++++++++++++++++++++++++++++++++--------- argh/tests/lib.rs | 89 +++++++++++++++++++++++++++++++++++++- argh_derive/src/help.rs | 47 +++++++++++++------- argh_derive/src/lib.rs | 87 ++++++++++++++++--------------------- 5 files changed, 232 insertions(+), 88 deletions(-) diff --git a/README.md b/README.md index e50b3d1..668337e 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,7 @@ struct SubCommandTwo { } ``` -Is it possible to specify a optional subcommand, just like specifing +It is possible to specify a default subcommand, just like specifying a default for an optional argument. ```rust diff --git a/argh/src/lib.rs b/argh/src/lib.rs index 4bd3a3a..457b385 100644 --- a/argh/src/lib.rs +++ b/argh/src/lib.rs @@ -298,7 +298,18 @@ pub trait FromArgs: Sized { /// }, /// ); /// ``` - fn from_args(command_name: &[&str], args: &[&str]) -> Result; + fn from_args(command_name: &[&str], args: &[&str]) -> Result { + Self::from_args_show_command_usage(command_name, args, true) + } + + /// Just like `from_args`, but with an additional parameter to specify whether or not + /// to print the first line of command_usage. + /// Internal use only. + fn from_args_show_command_usage( + command_name: &[&str], + args: &[&str], + show_command_usage: bool, + ) -> Result; /// Get a String with just the argument names, e.g., options, flags, subcommands, etc, but /// without the values of the options and arguments. This can be useful as a means to capture @@ -432,6 +443,17 @@ pub trait FromArgs: Sized { /// ); /// ``` fn redact_arg_values(_command_name: &[&str], _args: &[&str]) -> Result, EarlyExit> { + Self::redact_arg_values_show_command_usage(_command_name, _args, true) + } + + /// Just like `redact_arg_values`, but with an additional parameter to specify whether or not + /// to print the first line of command_usage. + /// Internal use only. + fn redact_arg_values_show_command_usage( + _command_name: &[&str], + _args: &[&str], + _show_command_usage: bool, + ) -> Result, EarlyExit> { Ok(vec!["<>".into()]) } } @@ -667,27 +689,23 @@ impl_flag_for_integers![u8, u16, u32, u64, u128, i8, i16, i32, i64, i128,]; /// `parse_positionals`: Helper to parse positional arguments. /// `parse_subcommand`: Helper to parse a subcommand. /// `help_func`: Generate a help message. +/// `show_command_usage`: The bool value `help_func` is called with. #[doc(hidden)] -pub fn parse_struct_args( +pub fn parse_struct_args_show_command_usage( cmd_name: &[&str], args: &[&str], mut parse_options: ParseStructOptions<'_>, mut parse_positionals: ParseStructPositionals<'_>, mut parse_subcommand: Option>, - help_func: &dyn Fn() -> String, + help_func: &dyn Fn(bool) -> String, + show_command_usage: bool, ) -> Result<(), EarlyExit> { let mut help = false; let mut remaining_args = args; let mut positional_index = 0; let mut options_ended = false; let (has_default_subcommand_parse, subcmd_str) = match parse_subcommand { - Some(ref subcmd) => { - if let Some(subcmd_str) = subcmd.default { - (true, subcmd_str) - } else { - (false, "") - } - } + Some(ParseStructSubCommand { default: Some(subcmd_str), .. }) => (true, subcmd_str), _ => (false, ""), }; @@ -721,7 +739,7 @@ pub fn parse_struct_args( } if let Some(ref mut parse_subcommand) = parse_subcommand { - if parse_subcommand.parse(help, cmd_name, next_arg, remaining_args)? { + if parse_subcommand.parse(help, cmd_name, next_arg, remaining_args, true)? { // Unset `help`, since we handled it in the subcommand help = false; break 'parse_args; @@ -740,23 +758,30 @@ pub fn parse_struct_args( } if help { - // If there is also a subcommand, we'd like to print the help for that subcommand as well. + // If there is also a default subcommand, we'd like to print the help for that subcommand as well. if has_default_subcommand_parse { - let sub_parse = (&mut parse_subcommand.unwrap().parse_func)( + let mut unwrapped_parse_subcommand = parse_subcommand.unwrap(); + // Do not print usage again for the subcommand. + let sub_parse = (&mut unwrapped_parse_subcommand.parse_func)( &[cmd_name, &[subcmd_str]].concat(), args, + false, ) .expect_err("bad parse func"); - Err(EarlyExit { output: help_func() + &"\n" + &sub_parse.output, status: Ok(()) }) + let default_subcommand_help_msg = format!("When no subcommand is given, the command `{}` is used as the default. Allowing the additional:", unwrapped_parse_subcommand.default.unwrap()); + Err(EarlyExit { + output: format!("{}\n{}\n\n{}", help_func(show_command_usage), default_subcommand_help_msg, &sub_parse.output), + status: Ok(()), + }) } else { - Err(EarlyExit { output: help_func(), status: Ok(()) }) + Err(EarlyExit { output: help_func(show_command_usage), status: Ok(()) }) } } else if let Some(ref mut parse_subcommand) = parse_subcommand { - // If we have a do have a potential subcommand and it not parsed. + // If we have a potential subcommand and it is not parsed. if let Some(default_subcommand) = parse_subcommand.default { // Pass the args again to the default args. // TODO(qhua948, #130): make this better, pass args without re-parsing. - parse_subcommand.parse(help, cmd_name, default_subcommand, args).map(|_| ()) + parse_subcommand.parse(help, cmd_name, default_subcommand, args, true).map(|_| ()) } else { Ok(()) } @@ -765,6 +790,35 @@ pub fn parse_struct_args( } } +/// This function implements argument parsing for structs. +/// +/// `cmd_name`: The identifier for the current command. +/// `args`: The command line arguments. +/// `parse_options`: Helper to parse optional arguments. +/// `parse_positionals`: Helper to parse positional arguments. +/// `parse_subcommand`: Helper to parse a subcommand. +/// `help_func`: Generate a help message. +#[doc(hidden)] +pub fn parse_struct_args( + cmd_name: &[&str], + args: &[&str], + parse_options: ParseStructOptions<'_>, + parse_positionals: ParseStructPositionals<'_>, + parse_subcommand: Option>, + help_func: &dyn Fn(bool) -> String, + show_command_usage: bool, +) -> Result<(), EarlyExit> { + parse_struct_args_show_command_usage( + cmd_name, + args, + parse_options, + parse_positionals, + parse_subcommand, + help_func, + show_command_usage, + ) +} + #[doc(hidden)] pub struct ParseStructOptions<'a> { /// A mapping from option string literals to the entry @@ -887,9 +941,9 @@ pub struct ParseStructSubCommand<'a> { pub subcommands: &'static [&'static CommandInfo], // The function to parse the subcommand arguments. - pub parse_func: &'a mut dyn FnMut(&[&str], &[&str]) -> Result<(), EarlyExit>, + pub parse_func: &'a mut dyn FnMut(&[&str], &[&str], bool) -> Result<(), EarlyExit>, - // The default subcommand as a literal string, which should matches one of the subcommand's "name" attribute. + // The default subcommand as a literal string, which should match one of the subcommand's "name" attribute. pub default: Option<&'static str>, } @@ -900,6 +954,7 @@ impl<'a> ParseStructSubCommand<'a> { cmd_name: &[&str], arg: &str, remaining_args: &[&str], + show_command_usage: bool, ) -> Result { for subcommand in self.subcommands { if subcommand.name == arg { @@ -913,7 +968,7 @@ impl<'a> ParseStructSubCommand<'a> { remaining_args }; - (self.parse_func)(&command, remaining_args)?; + (self.parse_func)(&command, remaining_args, show_command_usage)?; return Ok(true); } diff --git a/argh/tests/lib.rs b/argh/tests/lib.rs index ba3e99e..3196dcf 100644 --- a/argh/tests/lib.rs +++ b/argh/tests/lib.rs @@ -232,7 +232,7 @@ Commands: one First subcommand. two Second subcommand. -Usage: cmdname one --x +When no subcommand is given, the command `one` is used as the default. Allowing the additional: First subcommand. @@ -419,6 +419,92 @@ fn option_subcommand_nested_multiple() { ); } +#[test] +fn option_subcommand_nested_multiple_show_help_both() { + #[derive(FromArgs, PartialEq, Debug)] + /// Top-level command. + struct TopLevel { + #[argh(subcommand, default = "my")] + nested: Option, + } + + #[derive(FromArgs, PartialEq, Debug)] + // Nested command. + #[argh(subcommand, name = "my", description = "My Subcmd")] + struct MySubCommandStruct { + #[argh(subcommand, default = "one")] + /// nested command. + nested_again: Option, + + #[argh(option, default = "69", description = "delta")] + /// the delta + delta: i32, + } + + #[derive(FromArgs, PartialEq, Debug)] + #[argh(subcommand)] + enum MySubCommandEnum { + One(SubCommandOne), + Two(SubCommandTwo), + } + + #[derive(FromArgs, PartialEq, Debug)] + /// First subcommand. + #[argh(subcommand, name = "one")] + struct SubCommandOne { + #[argh(option, default = "1")] + /// how many x + x: usize, + } + + #[derive(FromArgs, PartialEq, Debug)] + /// Second subcommand. + #[argh(subcommand, name = "two")] + struct SubCommandTwo { + #[argh(switch)] + /// whether to fooey + fooey: bool, + } + let help_str = r###"Usage: cmdname [] [] + +Top-level command. + +Options: + --help display usage information + +Commands: + my My Subcmd + +When no subcommand is given, the command `my` is used as the default. Allowing the additional: + +My Subcmd + +Options: + --delta delta + --help display usage information + +Commands: + one First subcommand. + two Second subcommand. + +When no subcommand is given, the command `one` is used as the default. Allowing the additional: + +First subcommand. + +Options: + --x how many x + --help display usage information +"###; + + match TopLevel::from_args(&["cmdname"], &["--help"]) { + Ok(_) => panic!("help was parsed as args"), + Err(e) => { + assert_eq!(help_str, e.output); + e.status.expect("help returned an error"); + } + } +} + #[test] fn option_subcommand_with_default_and_args() { #[derive(FromArgs, PartialEq, Debug)] @@ -1127,6 +1213,7 @@ Options: e.status.expect_err("should be an error"); } + // TODO(qhua948): undocumented feature of format! templating for {command_name} #[derive(FromArgs, PartialEq, Debug)] #[argh( description = "Destroy the contents of .", diff --git a/argh_derive/src/help.rs b/argh_derive/src/help.rs index c295825..05150d1 100644 --- a/argh_derive/src/help.rs +++ b/argh_derive/src/help.rs @@ -26,35 +26,36 @@ pub(crate) fn help( fields: &[StructField<'_>], subcommand: Option<&StructField<'_>>, ) -> TokenStream { - let mut format_lit = "Usage: {command_name}".to_string(); + let mut usage_section = "Usage: {command_name}".to_string(); let positional = fields.iter().filter(|f| f.kind == FieldKind::Positional); let mut has_positional = false; for arg in positional.clone() { has_positional = true; - format_lit.push(' '); - positional_usage(&mut format_lit, arg); + usage_section.push(' '); + positional_usage(&mut usage_section, arg); } let options = fields.iter().filter(|f| f.long_name.is_some()); for option in options.clone() { - format_lit.push(' '); - option_usage(&mut format_lit, option); + usage_section.push(' '); + option_usage(&mut usage_section, option); } if let Some(subcommand) = subcommand { - format_lit.push(' '); + usage_section.push(' '); if !subcommand.optionality.is_required() { - format_lit.push('['); + usage_section.push('['); } - format_lit.push_str(""); + usage_section.push_str(""); if !subcommand.optionality.is_required() { - format_lit.push(']'); + usage_section.push(']'); } - format_lit.push_str(" []"); + usage_section.push_str(" []"); } - format_lit.push_str(SECTION_SEPARATOR); + usage_section.push_str(SECTION_SEPARATOR); + let mut format_lit = "".to_string(); let description = require_description(errors, Span::call_site(), &ty_attrs.description, "type"); format_lit.push_str(&description); @@ -108,10 +109,26 @@ pub(crate) fn help( format_lit.push('\n'); - quote! { { - #subcommand_calculation - format!(#format_lit, command_name = #cmd_name_str_array_ident.join(" "), #subcommand_format_arg) - } } + let has_format_template_command_name = format_lit.contains("{command_name}"); + let command_name_format_arg = quote! { command_name = #cmd_name_str_array_ident.join(" ") }; + let together = format!("{}{}", usage_section, format_lit); + + // We also allow user to template a `{command_name}` in their Notes/Examples. + let second_section_format_tokens = if has_format_template_command_name { + quote! { format!(#format_lit, #command_name_format_arg, #subcommand_format_arg) } + } else { + quote! { format!(#format_lit, #subcommand_format_arg) } + }; + + quote! { & |show_command_usage: bool| { + #subcommand_calculation + if show_command_usage{ + format!(#together, #command_name_format_arg, #subcommand_format_arg) + } else { + #second_section_format_tokens + } + } + } } /// A section composed of exactly just the literals provided to the program. diff --git a/argh_derive/src/lib.rs b/argh_derive/src/lib.rs index 0a53d8b..b81df2d 100644 --- a/argh_derive/src/lib.rs +++ b/argh_derive/src/lib.rs @@ -304,28 +304,20 @@ fn impl_from_args_struct_from_args<'a>( let parse_subcommands = if let Some(subcommand) = subcommand { let name = subcommand.name; let ty = subcommand.ty_without_wrapper; - if let Some(ref default) = subcommand.attrs.default { - quote_spanned! { impl_span => - Some(argh::ParseStructSubCommand { - subcommands: <#ty as argh::SubCommands>::COMMANDS, - parse_func: &mut |__command, __remaining_args| { - #name = Some(<#ty as argh::FromArgs>::from_args(__command, __remaining_args)?); - Ok(()) - }, - default: Some( #default ), - }) - } + let default_opt = if let Some(ref default) = subcommand.attrs.default { + quote! { Some ( #default ) } } else { - quote_spanned! { impl_span => - Some(argh::ParseStructSubCommand { - subcommands: <#ty as argh::SubCommands>::COMMANDS, - parse_func: &mut |__command, __remaining_args| { - #name = Some(<#ty as argh::FromArgs>::from_args(__command, __remaining_args)?); - Ok(()) - }, - default: None, - }) - } + quote! { None } + }; + quote_spanned! { impl_span => + Some(argh::ParseStructSubCommand { + subcommands: <#ty as argh::SubCommands>::COMMANDS, + parse_func: &mut |__command, __remaining_args, __show_command_usage| { + #name = Some(<#ty as argh::FromArgs>::from_args_show_command_usage(__command, __remaining_args, __show_command_usage)?); + Ok(()) + }, + default: #default_opt, + }) } } else { quote_spanned! { impl_span => None } @@ -336,7 +328,7 @@ fn impl_from_args_struct_from_args<'a>( let help = help::help(errors, cmd_name_str_array_ident, type_attrs, fields, subcommand); let method_impl = quote_spanned! { impl_span => - fn from_args(__cmd_name: &[&str], __args: &[&str]) + fn from_args_show_command_usage(__cmd_name: &[&str], __args: &[&str], __show_command_usage: bool) -> std::result::Result { #![allow(clippy::unwrap_in_result)] @@ -362,7 +354,8 @@ fn impl_from_args_struct_from_args<'a>( last_is_repeating: #last_positional_is_repeating, }, #parse_subcommands, - &|| #help, + #help, + __show_command_usage, )?; let mut #missing_requirements_ident = argh::MissingRequirements::default(); @@ -426,29 +419,20 @@ fn impl_from_args_struct_redact_arg_values<'a>( let redact_subcommands = if let Some(subcommand) = subcommand { let name = subcommand.name; let ty = subcommand.ty_without_wrapper; - - if let Some(ref default) = subcommand.attrs.default { - quote_spanned! { impl_span => - Some(argh::ParseStructSubCommand { - subcommands: <#ty as argh::SubCommands>::COMMANDS, - parse_func: &mut |__command, __remaining_args| { - #name = Some(<#ty as argh::FromArgs>::redact_arg_values(__command, __remaining_args)?); - Ok(()) - }, - default: Some( #default ), - }) - } + let default_opt = if let Some(ref default) = subcommand.attrs.default { + quote! { Some ( #default ) } } else { - quote_spanned! { impl_span => - Some(argh::ParseStructSubCommand { - subcommands: <#ty as argh::SubCommands>::COMMANDS, - parse_func: &mut |__command, __remaining_args| { - #name = Some(<#ty as argh::FromArgs>::redact_arg_values(__command, __remaining_args)?); - Ok(()) - }, - default: None, - }) - } + quote! { None } + }; + quote_spanned! { impl_span => + Some(argh::ParseStructSubCommand { + subcommands: <#ty as argh::SubCommands>::COMMANDS, + parse_func: &mut |__command, __remaining_args, __show_command_usage| { + #name = Some(<#ty as argh::FromArgs>::redact_arg_values_show_command_usage(__command, __remaining_args, __show_command_usage)?); + Ok(()) + }, + default: #default_opt, + }) } } else { quote_spanned! { impl_span => None } @@ -465,7 +449,7 @@ fn impl_from_args_struct_redact_arg_values<'a>( let help = help::help(errors, cmd_name_str_array_ident, type_attrs, fields, subcommand); let method_impl = quote_spanned! { impl_span => - fn redact_arg_values(__cmd_name: &[&str], __args: &[&str]) -> std::result::Result, argh::EarlyExit> { + fn redact_arg_values_show_command_usage(__cmd_name: &[&str], __args: &[&str], __show_command_usage: bool) -> std::result::Result, argh::EarlyExit> { #( #init_fields )* argh::parse_struct_args( @@ -487,7 +471,8 @@ fn impl_from_args_struct_redact_arg_values<'a>( last_is_repeating: #last_positional_is_repeating, }, #redact_subcommands, - &|| #help, + #help, + __show_command_usage, )?; let mut #missing_requirements_ident = argh::MissingRequirements::default(); @@ -892,7 +877,7 @@ fn impl_from_args_enum( quote! { impl argh::FromArgs for #name { - fn from_args(command_name: &[&str], args: &[&str]) + fn from_args_show_command_usage(command_name: &[&str], args: &[&str], show_command_usage: bool) -> std::result::Result { let subcommand_name = if let Some(subcommand_name) = command_name.last() { @@ -904,7 +889,7 @@ fn impl_from_args_enum( #( if subcommand_name == <#variant_ty as argh::SubCommand>::COMMAND.name { return Ok(#name_repeating::#variant_names( - <#variant_ty as argh::FromArgs>::from_args(command_name, args)? + <#variant_ty as argh::FromArgs>::from_args_show_command_usage(command_name, args, show_command_usage)? )); } )* @@ -912,7 +897,7 @@ fn impl_from_args_enum( Err(argh::EarlyExit::from("no subcommand matched".to_owned())) } - fn redact_arg_values(command_name: &[&str], args: &[&str]) -> std::result::Result, argh::EarlyExit> { + fn redact_arg_values_show_command_usage(command_name: &[&str], args: &[&str], show_command_usage: bool) -> std::result::Result, argh::EarlyExit> { let subcommand_name = if let Some(subcommand_name) = command_name.last() { *subcommand_name } else { @@ -921,7 +906,7 @@ fn impl_from_args_enum( #( if subcommand_name == <#variant_ty as argh::SubCommand>::COMMAND.name { - return <#variant_ty as argh::FromArgs>::redact_arg_values(command_name, args); + return <#variant_ty as argh::FromArgs>::redact_arg_values_show_command_usage(command_name, args, show_command_usage); } )*