Skip to content

Commit 42be792

Browse files
committed
[CLI-347] Options.addOptionGroup(OptionGroup) does not remove required
options from requiredOpts list
1 parent 50b149a commit 42be792

File tree

3 files changed

+19
-1
lines changed

3 files changed

+19
-1
lines changed

src/changes/changes.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
<action type="fix" issue="CLI-341" dev="ggregory" due-to="Ruiqi Dong, Gary Gregory">HelpFormatter infinite loop with 0 width input.</action>
3838
<action type="fix" issue="CLI-349" dev="ggregory" due-to="Leo Fernandes, Gary Gregory">Fail faster with a more precise NullPointerException: Option.processValue() throws NullPointerException when passed null value with value separator configured.</action>
3939
<action type="fix" issue="CLI-344" dev="ggregory" due-to="Ruiqi Dong, Gary Gregory">Fail faster with a more precise NullPointerException: DefaultParser.parse() throws NullPointerException when options parameter is null.</action>
40+
<action type="fix" issue="CLI-347" dev="ggregory" due-to="Ruiqi Dong, Gary Gregory">Options.addOptionGroup(OptionGroup) does not remove required options from requiredOpts list.</action>
4041
<!-- ADD -->
4142
<action type="add" issue="CLI-339" dev="ggregory" due-to="Claude Warren, Gary Gregory">Help formatter extension in the new package #314.</action>
4243
<action type="add" dev="ggregory" due-to="Gary Gregory">CommandLine.Builder implements Supplier&lt;CommandLine&gt;.</action>

src/main/java/org/apache/commons/cli/Options.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,10 @@ public Options addOptionGroup(final OptionGroup group) {
155155
// OptionGroup, either the group is required or
156156
// nothing is required
157157
option.setRequired(false);
158+
final String key = option.getKey();
159+
requiredOpts.remove(key);
158160
addOption(option);
159-
optionGroups.put(option.getKey(), group);
161+
optionGroups.put(key, group);
160162
}
161163
return this;
162164
}

src/test/java/org/apache/commons/cli/OptionsTest.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,21 @@ Licensed to the Apache Software Foundation (ASF) under one or more
3636
@SuppressWarnings("deprecation") // tests some deprecated classes
3737
class OptionsTest {
3838

39+
@Test
40+
void testRequiredOptionInGroupShouldNotBeInRequiredList() {
41+
final String key = "a";
42+
final Option option = new Option(key, "along", false, "Option A");
43+
option.setRequired(true);
44+
final Options options = new Options();
45+
options.addOption(option);
46+
assertTrue(options.getRequiredOptions().contains(key));
47+
final OptionGroup optionGroup = new OptionGroup();
48+
optionGroup.addOption(option);
49+
options.addOptionGroup(optionGroup);
50+
assertFalse(options.getOption(key).isRequired());
51+
assertFalse(options.getRequiredOptions().contains(key), "Option in group shouldn't be in required options list.");
52+
}
53+
3954
private void assertToStrings(final Option option) {
4055
// Should never throw.
4156
// Should return a String, not null.

0 commit comments

Comments
 (0)