[#742] Bugfix: Default values prevent correct parsing in argument groups

Closes #742
remkop · Jun 28, 2019 · b578d91 · b578d91
1 parent 5d2ac98
commit b578d91
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 4 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -31,6 +31,7 @@ Picocli follows [semantic versioning](http://semver.org/).
 - [#756] API: Make synopsis indent for multi-line synopsis configurable (related to #739).
 - [#739] Bugfix: infinite loop or exception when command name plus synopsis heading length equals or exceeds usage help message width. Thanks to [Arturo Alonso](https://github.com/thefang12) for raising this.
 - [#746] Bugfix: Apply default values to options and positional parameters in argument groups. Thanks to [Andreas Deininger](https://github.com/deining) for raising this.
+- [#742] Bugfix: Default values prevent correct parsing in argument groups. Thanks to [Andreas Deininger](https://github.com/deining) for raising this.
 - [#751] Build: Make build more portable.
 - [#753] Doc: Improve documentation for multi-value fields: mention the `split` attribute. Thanks to [feinstein](https://github.com/feinstein).
 - [#740] Doc: Update user manual to replace `parse` examples with `parseArgs`.

diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
@@ -10179,11 +10179,13 @@ void beforeMatchingGroupElement(ArgSpec argSpec) throws Exception {
                 ArgGroupSpec group = argSpec.group();
                 if (group == null || isInitializingDefaultValues) { return; }
                 GroupMatchContainer foundGroupMatchContainer = this.groupMatchContainer.findOrCreateMatchingGroup(argSpec, commandSpec.commandLine);
-                if (foundGroupMatchContainer.lastMatch().matchedMinElements() && argSpec.required()) {
+                if (foundGroupMatchContainer.lastMatch().matchedMinElements() &&
+                        (argSpec.required() || foundGroupMatchContainer.lastMatch().matchCount(argSpec) > 0)) {
                     // we need to create a new match; if maxMultiplicity has been reached, we need to add a new GroupMatchContainer.
+                    String previousMatch = argSpec.required() ? "is required" : "has already been matched";
                     String elementDescription = ArgSpec.describe(argSpec, "=");
                     Tracer tracer = commandSpec.commandLine.tracer;
-                    tracer.info("GroupMatch %s is complete: its mandatory elements are all matched. (User object: %s.) %s is required in the group, so it starts a new GroupMatch.%n", foundGroupMatchContainer.lastMatch(), foundGroupMatchContainer.group.userObject(), elementDescription);
+                    tracer.info("GroupMatch %s is complete: its mandatory elements are all matched. (User object: %s.) %s %s in the group, so it starts a new GroupMatch.%n", foundGroupMatchContainer.lastMatch(), foundGroupMatchContainer.group.userObject(), elementDescription, previousMatch);
                     foundGroupMatchContainer.addMatch(commandSpec.commandLine);
                     this.groupMatchContainer.findOrCreateMatchingGroup(argSpec, commandSpec.commandLine);
                 }

diff --git a/src/test/java/picocli/ArgGroupTest.java b/src/test/java/picocli/ArgGroupTest.java
@@ -2389,11 +2389,37 @@ static class DataSource {
         }
     }
 
-    @Ignore("NOTE: this test passes if `defaultValue = 'e'` is removed from the -g option definition")
     @Test
     // https://github.com/remkop/picocli/issues/742
     public void testIssue742FalseErrorMessage() {
+        //TestUtil.setTraceLevel("DEBUG");
         CommandLine cmd = new CommandLine(new Issue742());
-        cmd.parseArgs("-g=2", "-g=3");
+        ParseResult parseResult = cmd.parseArgs("-g=2", "-g=3");
+        List<ParseResult.GroupMatch> multiples = parseResult.getGroupMatches();
+        assertEquals(1, multiples.size());
+        GroupMatch groupMatch = multiples.get(0);
+
+        assertEquals(1, groupMatch.matchedSubgroups().size());
+        ArgGroupSpec dsGroup = cmd.getCommandSpec().argGroups().get(0);
+        //@SuppressWarnings("")
+        List<Issue742.DataSource> datasources = (List<Issue742.DataSource>) dsGroup.userObject();
+        assertEquals(2, datasources.size());
+
+        Issue742.DataSource ds1 = datasources.get(0);
+        assertEquals("2", ds1.aString);
+
+        Issue742.DataSource ds2 = datasources.get(1);
+        assertEquals("3", ds2.aString);
+
+        GroupMatchContainer modeGroupMatchContainer = groupMatch.matchedSubgroups().get(dsGroup);
+        assertEquals(2, modeGroupMatchContainer.matches().size());
+
+        GroupMatch dsGroupMatch1 = modeGroupMatchContainer.matches().get(0);
+        assertEquals(0, dsGroupMatch1.matchedSubgroups().size());
+        assertEquals(Arrays.asList("2"), dsGroupMatch1.matchedValues(dsGroup.args().iterator().next()));
+
+        GroupMatch dsGroupMatch2 = modeGroupMatchContainer.matches().get(1);
+        assertEquals(0, dsGroupMatch2.matchedSubgroups().size());
+        assertEquals(Arrays.asList("3"), dsGroupMatch2.matchedValues(dsGroup.args().iterator().next()));
     }
 }