fix(yaml): Don't panic on multiple groups

Because we gradually build the `ArgGroup` as we parse the YAML, we don't
use `ArgGroup::new`.  Clap3 introduced an internal `id` in addition to
the public `name` and it appears that this custom initialization code
was not updated.

This shows the problem with publically exposing `impl Default`.
Choices:
- Remove `impl Default`
  - Always valid
  - Requires spreading invariants
  - Callers can't implement code the same way we do
- Add `ArgGroup::name`
  - Can be constructed in an invalid state
  - Centralizes invariants
  - A caller could implement code like the yaml logic

I decided to go with `ArgGroup::name`.

Fixes #2719
This commit is contained in:
Ed Page 2021-08-18 11:49:43 -05:00
parent 2fd26423e2
commit bd25b5f615
4 changed files with 49 additions and 8 deletions

View file

@ -105,12 +105,22 @@ impl<'help> ArgGroup<'help> {
/// # ;
/// ```
pub fn new<S: Into<&'help str>>(n: S) -> Self {
let name = n.into();
ArgGroup {
id: Id::from(&*name),
name,
..ArgGroup::default()
}
ArgGroup::default().name(n)
}
/// Sets the group name.
///
/// # Examples
///
/// ```rust
/// # use clap::{App, ArgGroup};
/// ArgGroup::default().name("config")
/// # ;
/// ```
pub fn name<S: Into<&'help str>>(mut self, n: S) -> Self {
self.name = n.into();
self.id = Id::from(&self.name);
self
}
/// Adds an [argument] to this group by name
@ -434,6 +444,7 @@ impl<'help> From<&'help Yaml> for ArgGroup<'help> {
.as_str()
.expect("failed to convert arg YAML name to str");
a.name = name_str;
a.id = Id::from(&a.name);
b.get(name_yaml)
.expect("failed to get name_str")
.as_hash()
@ -457,7 +468,7 @@ impl<'help> From<&'help Yaml> for ArgGroup<'help> {
"conflicts_with" => yaml_vec_or_str!(a, v, conflicts_with),
"name" => {
if let Some(ys) = v.as_str() {
a.name = ys;
a = a.name(ys);
}
a
}

View file

@ -40,8 +40,9 @@ fn examples_are_functional() {
let help_output = run_example(example_name, &["--help"]);
assert!(
help_output.status.success(),
"{} --help exited with nonzero",
"{} --help exited with nonzero: {}",
example_name,
String::from_utf8_lossy(&help_output.stderr),
);
assert!(
!help_output.stdout.is_empty(),

21
tests/fixtures/multiple_groups.yaml vendored Normal file
View file

@ -0,0 +1,21 @@
name: app
args:
- arg_1:
short: a
- arg_2:
short: b
- arg_3:
short: c
- arg_4:
short: d
groups:
- group_1:
args:
- arg_1
- arg_2
- group_2:
args:
- arg_3
- arg_4

View file

@ -256,3 +256,11 @@ fn arg_field_not_string() {
let yml = load_yaml!("fixtures/arg_field_not_string.yaml");
Arg::from(yml);
}
#[test]
fn multiple_groups() {
let yml = load_yaml!("fixtures/multiple_groups.yaml");
let matches = App::from(yml).try_get_matches_from(&["app", "-a", "-c"]);
eprintln!("{:?}", matches);
assert!(matches.is_ok());
}