From f43b7c65941c53adc0616b8646a21dc255862eb2 Mon Sep 17 00:00:00 2001 From: Kevin K Date: Sat, 27 Aug 2016 19:02:38 -0400 Subject: [PATCH] imp(YAML Errors): vastly improves error messages when using YAML When errors are made while developing, the panic error messages have been improved instead of relying on the default panic message which is extremely unhelpful. Closes #574 --- examples/17_yaml.yml | 4 +- src/app/mod.rs | 118 ++++++++++++++++++++++++------------------- src/args/arg.rs | 77 +++++++++++----------------- src/args/group.rs | 23 ++------- src/args/macros.rs | 47 +++++++++++++++++ src/args/mod.rs | 2 + tests/app.yml | 4 +- 7 files changed, 153 insertions(+), 122 deletions(-) create mode 100644 src/args/macros.rs diff --git a/examples/17_yaml.yml b/examples/17_yaml.yml index 358077a8e05..4e5589151f9 100644 --- a/examples/17_yaml.yml +++ b/examples/17_yaml.yml @@ -1,5 +1,5 @@ name: yml_app -version: 1.0 +version: "1.0" about: an example using a .yml file to build a CLI author: Kevin K. @@ -69,7 +69,7 @@ subcommands: # Rust code later - subcmd: about: demos subcommands from yaml - version: 0.1 + version: "0.1" author: Kevin K. # Subcommand args are exactly like App args args: diff --git a/src/app/mod.rs b/src/app/mod.rs index ce7f325f7f4..c2131679b86 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -1345,54 +1345,57 @@ impl<'a> From<&'a Yaml> for App<'a, 'a> { } else { yaml }; - if let Some(v) = yaml["version"].as_str() { - a = a.version(v); - } - if let Some(v) = yaml["author"].as_str() { - a = a.author(v); - } - if let Some(v) = yaml["bin_name"].as_str() { - a = a.bin_name(v); - } - if let Some(v) = yaml["about"].as_str() { - a = a.about(v); - } - if let Some(v) = yaml["before_help"].as_str() { - a = a.before_help(v); - } - if let Some(v) = yaml["template"].as_str() { - a = a.template(v); - } - if let Some(v) = yaml["after_help"].as_str() { - a = a.after_help(v); + + macro_rules! yaml_str { + ($a:ident, $y:ident, $i:ident) => { + if let Some(v) = $y[stringify!($i)].as_str() { + $a = $a.$i(v); + } else if $y[stringify!($i)] != Yaml::BadValue { + panic!("Failed to convert YAML value {:?} to a string", $y[stringify!($i)]); + } + }; } + + yaml_str!(a, yaml, version); + yaml_str!(a, yaml, bin_name); + yaml_str!(a, yaml, about); + yaml_str!(a, yaml, before_help); + yaml_str!(a, yaml, after_help); + yaml_str!(a, yaml, template); + yaml_str!(a, yaml, usage); + yaml_str!(a, yaml, help); + yaml_str!(a, yaml, help_short); + yaml_str!(a, yaml, version_short); + yaml_str!(a, yaml, alias); + yaml_str!(a, yaml, visible_alias); + if let Some(v) = yaml["display_order"].as_i64() { a = a.display_order(v as usize); - } - if let Some(v) = yaml["usage"].as_str() { - a = a.usage(v); - } - if let Some(v) = yaml["help"].as_str() { - a = a.help(v); - } - if let Some(v) = yaml["help_short"].as_str() { - a = a.help_short(v); - } - if let Some(v) = yaml["version_short"].as_str() { - a = a.version_short(v); + } else if yaml["display_order"] != Yaml::BadValue { + panic!("Failed to convert YAML value {:?} to a u64", yaml["display_order"]); } if let Some(v) = yaml["setting"].as_str() { - a = a.setting(v.parse().ok().expect("unknown AppSetting found in YAML file")); + a = a.setting(v.parse().expect("unknown AppSetting found in YAML file")); + } else if yaml["setting"] != Yaml::BadValue { + panic!("Failed to convert YAML value {:?} to an AppSetting", yaml["setting"]); } if let Some(v) = yaml["settings"].as_vec() { for ys in v { if let Some(s) = ys.as_str() { - a = a.setting(s.parse().ok().expect("unknown AppSetting found in YAML file")); + a = a.setting(s.parse().expect("unknown AppSetting found in YAML file")); } } + } else { + if let Some(v) = yaml["settings"].as_str() { + a = a.setting(v.parse().expect("unknown AppSetting found in YAML file")); + } else if yaml["settings"] != Yaml::BadValue { + panic!("Failed to convert YAML value {:?} to a string", yaml["settings"]); + } } if let Some(v) = yaml["global_setting"].as_str() { a = a.setting(v.parse().ok().expect("unknown AppSetting found in YAML file")); + } else if yaml["global_setting"] != Yaml::BadValue { + panic!("Failed to convert YAML value {:?} to an AppSetting", yaml["setting"]); } if let Some(v) = yaml["global_settings"].as_vec() { for ys in v { @@ -1400,27 +1403,40 @@ impl<'a> From<&'a Yaml> for App<'a, 'a> { a = a.global_setting(s.parse().ok().expect("unknown AppSetting found in YAML file")); } } - } - if let Some(v) = yaml["alias"].as_str() { - a = a.alias(v); - } - if let Some(v) = yaml["aliases"].as_vec() { - for ys in v { - if let Some(s) = ys.as_str() { - a = a.alias(s); - } + } else { + if let Some(v) = yaml["global_settings"].as_str() { + a = a.global_setting(v.parse().expect("unknown AppSetting found in YAML file")); + } else if yaml["global_settings"] != Yaml::BadValue { + panic!("Failed to convert YAML value {:?} to a string", yaml["global_settings"]); } } - if let Some(v) = yaml["visible_alias"].as_str() { - a = a.visible_alias(v); - } - if let Some(v) = yaml["visible_aliases"].as_vec() { - for ys in v { - if let Some(s) = ys.as_str() { - a = a.visible_alias(s); + + macro_rules! vec_or_str { + ($a:ident, $y:ident, $as_vec:ident, $as_single:ident) => {{ + let maybe_vec = $y[stringify!($as_vec)].as_vec(); + if let Some(vec) = maybe_vec { + for ys in vec { + if let Some(s) = ys.as_str() { + $a = $a.$as_single(s); + } else { + panic!("Failed to convert YAML value {:?} to a string", ys); + } + } + } else { + if let Some(s) = $y[stringify!($as_vec)].as_str() { + $a = $a.$as_single(s); + } else if $y[stringify!($as_vec)] != Yaml::BadValue { + panic!("Failed to convert YAML value {:?} to either a vec or string", $y[stringify!($as_vec)]); + } + } + $a } - } + }; } + + a = vec_or_str!(a, yaml, aliases, alias); + a = vec_or_str!(a, yaml, visible_aliases, visible_alias); + if let Some(v) = yaml["args"].as_vec() { for arg_yaml in v { a = a.arg(Arg::from_yaml(&arg_yaml.as_hash().unwrap())); diff --git a/src/args/arg.rs b/src/args/arg.rs index 86a7a3b0e6e..f99a89aae7c 100644 --- a/src/args/arg.rs +++ b/src/args/arg.rs @@ -145,71 +145,52 @@ impl<'a, 'b> Arg<'a, 'b> { let mut a = Arg::with_name(name_str); let arg_settings = y.get(name_yml).unwrap().as_hash().unwrap(); - macro_rules! vec_or_str { - ($v:ident, $a:ident, $c:ident) => {{ - let maybe_vec = $v.as_vec(); - if let Some(vec) = maybe_vec { - for ys in vec { - if let Some(s) = ys.as_str() { - $a = $a.$c(s); - } - } - } else { - if let Some(s) = $v.as_str() { - $a = $a.$c(s); - } - } - $a - } - }; - } - for (k, v) in arg_settings.iter() { a = match k.as_str().unwrap() { - "short" => a.short(v.as_str().unwrap()), - "long" => a.long(v.as_str().unwrap()), - "help" => a.help(v.as_str().unwrap()), - "required" => a.required(v.as_bool().unwrap()), - "takes_value" => a.takes_value(v.as_bool().unwrap()), - "index" => a.index(v.as_i64().unwrap() as u64), - "global" => a.global(v.as_bool().unwrap()), - "multiple" => a.multiple(v.as_bool().unwrap()), - "hidden" => a.hidden(v.as_bool().unwrap()), - "next_line_help" => a.next_line_help(v.as_bool().unwrap()), - "empty_values" => a.empty_values(v.as_bool().unwrap()), - "group" => a.group(v.as_str().unwrap()), - "number_of_values" => a.number_of_values(v.as_i64().unwrap() as u64), - "max_values" => a.max_values(v.as_i64().unwrap() as u64), - "min_values" => a.min_values(v.as_i64().unwrap() as u64), - "value_name" => a.value_name(v.as_str().unwrap()), - "use_delimiter" => a.use_delimiter(v.as_bool().unwrap()), - "value_delimiter" => a.value_delimiter(v.as_str().unwrap()), - "required_unless" => a.required_unless(v.as_str().unwrap()), - "display_order" => a.display_order(v.as_i64().unwrap() as usize), - "default_value" => a.default_value(v.as_str().unwrap()), + "short" => yaml_to_str!(a, v, short), + "long" => yaml_to_str!(a, v, long), + "help" => yaml_to_str!(a, v, help), + "required" => yaml_to_bool!(a, v, required), + "takes_value" => yaml_to_bool!(a, v, takes_value), + "index" => yaml_to_u64!(a, v, index), + "global" => yaml_to_bool!(a, v, global), + "multiple" => yaml_to_bool!(a, v, multiple), + "hidden" => yaml_to_bool!(a, v, hidden), + "next_line_help" => yaml_to_bool!(a, v, next_line_help), + "empty_values" => yaml_to_bool!(a, v, empty_values), + "group" => yaml_to_str!(a, v, group), + "number_of_values" => yaml_to_u64!(a, v, number_of_values), + "max_values" => yaml_to_u64!(a, v, max_values), + "min_values" => yaml_to_u64!(a, v, min_values), + "value_name" => yaml_to_str!(a, v, value_name), + "use_delimiter" => yaml_to_bool!(a, v, use_delimiter), + "value_delimiter" => yaml_to_str!(a, v, value_delimiter), + "required_unless" => yaml_to_str!(a, v, required_unless), + "display_order" => yaml_to_usize!(a, v, display_order), + "default_value" => yaml_to_str!(a, v, default_value), "value_names" => { - vec_or_str!(v, a, value_name) + yaml_vec_or_str!(v, a, value_name) } "groups" => { - vec_or_str!(v, a, group) + yaml_vec_or_str!(v, a, group) } "requires" => { - vec_or_str!(v, a, requires) + yaml_vec_or_str!(v, a, requires) } "conflicts_with" => { - vec_or_str!(v, a, conflicts_with) + yaml_vec_or_str!(v, a, conflicts_with) } "overrides_with" => { - vec_or_str!(v, a, overrides_with) + yaml_vec_or_str!(v, a, overrides_with) } "possible_values" => { - vec_or_str!(v, a, possible_value) + yaml_vec_or_str!(v, a, possible_value) } "required_unless_one" => { - vec_or_str!(v, a, required_unless) + yaml_vec_or_str!(v, a, required_unless) } "required_unless_all" => { - a = vec_or_str!(v, a, required_unless); + a = yaml_vec_or_str!(v, a, required_unless); a.setb(ArgSettings::RequiredUnlessAll); a } diff --git a/src/args/group.rs b/src/args/group.rs index 05b678e87bc..4114b282cf0 100644 --- a/src/args/group.rs +++ b/src/args/group.rs @@ -458,7 +458,7 @@ impl<'a> From<&'a BTreeMap> for ArgGroup<'a> { let mut a = ArgGroup::default(); let group_settings = if b.len() == 1 { let name_yml = b.keys().nth(0).expect("failed to get name"); - let name_str = name_yml.as_str().expect("failed to convert name to str"); + let name_str = name_yml.as_str().expect("failed to convert arg YAML name to str"); a.name = name_str; b.get(name_yml) .expect("failed to get name_str") @@ -472,12 +472,7 @@ impl<'a> From<&'a BTreeMap> for ArgGroup<'a> { a = match k.as_str().unwrap() { "required" => a.required(v.as_bool().unwrap()), "args" => { - for ys in v.as_vec().unwrap() { - if let Some(s) = ys.as_str() { - a = a.arg(s); - } - } - a + yaml_vec_or_str!(v, a, arg) } "arg" => { if let Some(ys) = v.as_str() { @@ -486,20 +481,10 @@ impl<'a> From<&'a BTreeMap> for ArgGroup<'a> { a } "requires" => { - for ys in v.as_vec().unwrap() { - if let Some(s) = ys.as_str() { - a = a.requires(s); - } - } - a + yaml_vec_or_str!(v, a, requires) } "conflicts_with" => { - for ys in v.as_vec().unwrap() { - if let Some(s) = ys.as_str() { - a = a.conflicts_with(s); - } - } - a + yaml_vec_or_str!(v, a, conflicts_with) } "name" => { if let Some(ys) = v.as_str() { diff --git a/src/args/macros.rs b/src/args/macros.rs new file mode 100644 index 00000000000..1a0ad6bbee7 --- /dev/null +++ b/src/args/macros.rs @@ -0,0 +1,47 @@ + +macro_rules! yaml_vec_or_str { + ($v:ident, $a:ident, $c:ident) => {{ + let maybe_vec = $v.as_vec(); + if let Some(vec) = maybe_vec { + for ys in vec { + if let Some(s) = ys.as_str() { + $a = $a.$c(s); + } else { + panic!("Failed to convert YAML value {:?} to a string", ys); + } + } + } else { + if let Some(s) = $v.as_str() { + $a = $a.$c(s); + } else { + panic!("Failed to convert YAML value {:?} to either a vec or string", $v); + } + } + $a + } + }; +} + +macro_rules! yaml_to_str { + ($a:ident, $v:ident, $c:ident) => {{ + $a.$c($v.as_str().unwrap_or_else(|| panic!("failed to convert YAML {:?} value to a string", $v))) + }}; +} + +macro_rules! yaml_to_bool { + ($a:ident, $v:ident, $c:ident) => {{ + $a.$c($v.as_bool().unwrap_or_else(|| panic!("failed to convert YAML {:?} value to a string", $v))) + }}; +} + +macro_rules! yaml_to_u64 { + ($a:ident, $v:ident, $c:ident) => {{ + $a.$c($v.as_i64().unwrap_or_else(|| panic!("failed to convert YAML {:?} value to a string", $v)) as u64) + }}; +} + +macro_rules! yaml_to_usize { + ($a:ident, $v:ident, $c:ident) => {{ + $a.$c($v.as_i64().unwrap_or_else(|| panic!("failed to convert YAML {:?} value to a string", $v)) as usize) + }}; +} diff --git a/src/args/mod.rs b/src/args/mod.rs index b3daa9fa637..3bb1b3b24f8 100644 --- a/src/args/mod.rs +++ b/src/args/mod.rs @@ -8,6 +8,8 @@ pub use self::group::ArgGroup; pub use self::any_arg::{AnyArg, DispOrder}; pub use self::settings::ArgSettings; +#[macro_use] +mod macros; mod arg; pub mod any_arg; mod arg_matches; diff --git a/tests/app.yml b/tests/app.yml index b2fb0b256a5..20aae5168cf 100644 --- a/tests/app.yml +++ b/tests/app.yml @@ -1,5 +1,5 @@ name: claptests -version: 1.0 +version: "1.0" about: tests clap library author: Kevin K. settings: @@ -81,7 +81,7 @@ arg_groups: subcommands: - subcmd: about: tests subcommands - version: 0.1 + version: "0.1" author: Kevin K. args: - scoption: