Skip to content

Commit

Permalink
[#810][#833] Non-validating ArgGroups are now automatically set to be…
Browse files Browse the repository at this point in the history
… non-exclusive. Non-validating `@ArgGroup` should not throw validation exceptions.

Closes #810
Closes #833
  • Loading branch information
remkop committed Oct 10, 2019
1 parent 02f44a6 commit f5e67af
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 8 deletions.
2 changes: 2 additions & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ Picocli follows [semantic versioning](http://semver.org/).
* [#811] (Bugfix) `CommandLine.setResourceBundle` did not propagate resource bundle to subcommands recursively. Thanks to [thope](https://github.com/frontfact) for the pull request with the bug fix.
* [#826] (Enhancement) Suppress compiler warning "Supported source version 'RELEASE_6' from annotation processor... less than -source..." in picocli-codegen.
* [#815] (Enhancement) `@ArgGroup` should match multiple occurrences of a multi-value `@Option` in the same group instance, not create new group for each occurrence. Thanks to [kacchi](https://github.com/kacchi) for raising this.
* [#810] (Bugfix) `@ArgGroup` should not validate when marked as `validate = false`. Thanks to [Andreas Deininger](https://github.com/deining) for raising this.
* [#833] (Enhancement) Non-validating ArgGroups are now automatically set to be non-exclusive. Thanks to [Andreas Deininger](https://github.com/deining) for raising this.
* [#813] (DOC) Clarify usage of negatable boolean `@Option` with default value "true". Thanks to [Yann ROBERT](https://github.com/YannRobert) for raising this.
* [#814] (DOC) Document how a CLI application can be packaged for distribution.
* [#820] (DOC) Update user manual section on ANSI supported platforms: mention Windows Subsystem for Linux under Windows 10.
Expand Down
19 changes: 14 additions & 5 deletions src/main/java/picocli/CommandLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -7300,13 +7300,14 @@ private Object calcDefaultValue(boolean interpolate) {
}

private String defaultValueFromProvider() {
if (commandSpec == null) { return null; } // still initializing...
String fromProvider = null;
IDefaultValueProvider defaultValueProvider = null;
try {
defaultValueProvider = commandSpec.defaultValueProvider();
fromProvider = defaultValueProvider == null ? null : defaultValueProvider.defaultValue(this);
} catch (Exception ex) {
new Tracer().info("Error getting default value for %s from %s: %s", this, defaultValueProvider, ex);
new Tracer().info("Error getting default value for %s from %s: %s%n", this, defaultValueProvider, ex);
}
return fromProvider;
}
Expand Down Expand Up @@ -8327,7 +8328,7 @@ public static class ArgGroupSpec implements IOrdered {
ArgGroupSpec(ArgGroupSpec.Builder builder) {
heading = NO_HEADING .equals(builder.heading) ? null : builder.heading;
headingKey = NO_HEADING_KEY.equals(builder.headingKey) ? null : builder.headingKey;
exclusive = builder.exclusive;
exclusive = builder.exclusive && builder.validate; // non-validating groups cannot be exclusive: https://github.com/remkop/picocli/issues/810
multiplicity = builder.multiplicity;
validate = builder.validate;
order = builder.order;
Expand All @@ -8343,6 +8344,10 @@ public static class ArgGroupSpec implements IOrdered {
int i = 1;
for (ArgGroupSpec sub : subgroups) { sub.parentGroup = this; sub.id = id + "." + i++; }
for (ArgSpec arg : args) { arg.group = this; }

if (!validate && builder.exclusive) {
new Tracer().info("Setting exclusive=%s because %s is a non-validating group.%n", exclusive, synopsisUnit());
}
}

/** Returns a new {@link Builder}.
Expand Down Expand Up @@ -10601,11 +10606,15 @@ private void failGroupMultiplicityExceeded(List<ParseResult.GroupMatch> groupMat
msg += match.toString();
Map<ArgGroupSpec, GroupMatchContainer> subgroups = match.matchedSubgroups();
for (ArgGroupSpec group : subgroups.keySet()) {
addValueToListInMap(matchesPerGroup, group, subgroups.get(group).matches());
if (group.validate()) { // don't raise errors for non-validating groups: https://github.com/remkop/picocli/issues/810
addValueToListInMap(matchesPerGroup, group, subgroups.get(group).matches());
}
}
}
if (!simplifyErrorMessageForSingleGroup(matchesPerGroup, commandLine)) {
commandLine.interpreter.maybeThrow(new MaxValuesExceededException(commandLine, "Error: expected only one match but got " + msg));
if (!matchesPerGroup.isEmpty()) {
if (!simplifyErrorMessageForSingleGroup(matchesPerGroup, commandLine)) {
commandLine.interpreter.maybeThrow(new MaxValuesExceededException(commandLine, "Error: expected only one match but got " + msg));
}
}
}

Expand Down
57 changes: 54 additions & 3 deletions src/test/java/picocli/ArgGroupTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2537,19 +2537,18 @@ static class ValidatingOptions {
boolean y = false;
}
}

@Test
public void testIssue807NestedValidation() {
// should not throw MutuallyExclusiveArgsException
new CommandLine(new Issue807NestedCommand()).parseArgs("-s", "-v", "-x", "-y");
}

static class Issue829Group {
int x;
int y;
@Option(names = "-x") void x(int x) {this.x = x;}
@Option(names = "-y") void y(int y) {this.y = y;}
}

@Command(subcommands = Issue829Subcommand.class)
static class Issue829TopCommand {
@ArgGroup Issue829Group group;
Expand Down Expand Up @@ -2591,12 +2590,64 @@ static class Issue815 {
}
@Test
public void testIssue815() {
TestUtil.setTraceLevel("DEBUG");
//TestUtil.setTraceLevel("DEBUG");
Issue815 userObject = new Issue815();
new CommandLine(userObject).parseArgs("--id=123", "--id=456");
assertNotNull(userObject.group);
assertNull(userObject.group.age);
assertNotNull(userObject.group.id);
assertEquals(Arrays.asList("123", "456"), userObject.group.id);
}

static class Issue810Command {
@ArgGroup(validate = false, heading = "%nGrouped options:%n")
MyGroup myGroup = new MyGroup();

static class MyGroup {
@Option(names = "-s", description = "ssss", required = true, defaultValue = "false")
boolean s = false;

@Option(names = "-v", description = "vvvv", required = true, defaultValue = "false")
boolean v = false;
}
}
@Test
public void testIssue810Validation() {
// should not throw MutuallyExclusiveArgsException
Issue810Command app = new Issue810Command();
new CommandLine(app).parseArgs("-s", "-v");
assertTrue(app.myGroup.s);
assertTrue(app.myGroup.v);

// app = new Issue810Command();
new CommandLine(app).parseArgs("-s");
assertTrue(app.myGroup.s);
assertFalse(app.myGroup.v);

new CommandLine(app).parseArgs("-v");
assertFalse(app.myGroup.s);
assertTrue(app.myGroup.v);
}


static class Issue810WithExplicitExclusiveGroup {
@ArgGroup(exclusive = true, validate = false, heading = "%nGrouped options:%n")
MyGroup myGroup = new MyGroup();

static class MyGroup {
@Option(names = "-s", required = true)
boolean s = false;

@Option(names = "-v", required = true)
boolean v = false;
}
}
@Test
public void testNonValidatingOptionsAreNotExclusive() {
CommandSpec spec = CommandSpec.forAnnotatedObject(new Issue810Command());
assertFalse(spec.argGroups().get(0).exclusive());

CommandSpec spec2 = CommandSpec.forAnnotatedObject(new Issue810WithExplicitExclusiveGroup());
assertFalse(spec2.argGroups().get(0).exclusive());
}
}

0 comments on commit f5e67af

Please sign in to comment.