This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-cli.git
The following commit(s) were added to refs/heads/master by this push:
new df669bd4 Test how '!' marks a required option in PatternOptionBuilder
(#449)
df669bd4 is described below
commit df669bd47f1be804af1843b24c1152a6cd1b0a8e
Author: Andrea Cosentino <[email protected]>
AuthorDate: Fri Oct 30 11:09:46 2026 +0100
Test how '!' marks a required option in PatternOptionBuilder (#449)
* Fix PatternOptionBuilder '!' marking the wrong option as required
The Javadoc says '!' goes in front of a mandatory option, and its own
example, "vp:!f/", is meant to make -f required. The flag was only
applied once the next option character was read, so it ended up on the
option before the '!': the example made -p required and left -f
optional. It only worked when '!' was the first character of the
pattern, which is the one case the tests covered.
Now '!' applies to the option that follows it. A '!' with no option
after it, as in "a:b!", used to land on the previous option; it now
throws IllegalArgumentException, so a pattern written against the old
behavior fails right away instead of quietly losing its required
option.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
* Keep the '!' behavior in PatternOptionBuilder and test it
The earlier commit on this branch made '!' mark the option after it,
following the 2008 Javadoc sentence "The exclamation mark precede a
mandatory option". CLI-55 has the intent the other way round: "hc!<"
makes -c mandatory, so '!' marks the option before it, which is what
the code has always done and what the Javadoc now says.
Back out that change so existing patterns keep working, and add tests
for the current behavior instead: the Javadoc example, the CLI-55
example, a trailing '!' and several '!' in one pattern. Also say that a
'!' at the start of the pattern marks the first option.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Andrea Cosentino <[email protected]>
---------
Signed-off-by: Andrea Cosentino <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
Co-authored-by: Gary Gregory <[email protected]>
---
.../apache/commons/cli/PatternOptionBuilder.java | 2 +-
.../commons/cli/PatternOptionBuilderTest.java | 37 ++++++++++++++++++++++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java
b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java
index f31bf21a..ac53dd97 100644
--- a/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java
+++ b/src/main/java/org/apache/commons/cli/PatternOptionBuilder.java
@@ -70,7 +70,7 @@ import java.util.Map;
*
* <p>
* For example, the following accepts command-line arguments such as {@code -v
-p string-value -f https://example.com}. The exclamation mark makes the
- * preceding option mandatory.
+ * preceding option mandatory, here {@code -p}. An exclamation mark at the
start of the pattern makes the first option mandatory.
* </p>
*
* <pre>
diff --git a/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java
b/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java
index dd1013ea..874c5d68 100644
--- a/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java
+++ b/src/test/java/org/apache/commons/cli/PatternOptionBuilderTest.java
@@ -99,6 +99,35 @@ class PatternOptionBuilderTest {
assertNull(line.getOptionObject("n"), "n value");
}
+ @Test
+ void testRequiredMarkAfterOption() throws Exception {
+ // The class Javadoc example: '!' makes the preceding option, -p,
mandatory.
+ final Options options = PatternOptionBuilder.parsePattern("vp:!f/");
+ assertFalse(options.getOption("v").isRequired(), "v");
+ assertTrue(options.getOption("p").isRequired(), "p");
+ assertFalse(options.getOption("f").isRequired(), "f");
+ final MissingOptionException e =
assertThrows(MissingOptionException.class,
+ () -> new DefaultParser().parse(options, new String[] { "-v",
"-f", "https://example.com"; }));
+ assertEquals(1, e.getMissingOptions().size());
+ assertTrue(e.getMissingOptions().contains("p"));
+ }
+
+ @Test
+ void testRequiredMarkAtEnd() {
+ final Options options = PatternOptionBuilder.parsePattern("a:b!");
+ assertFalse(options.getOption("a").isRequired(), "a");
+ assertTrue(options.getOption("b").isRequired(), "b");
+ }
+
+ @Test
+ void testRequiredMarkBeforeValueCode() {
+ // The example from CLI-55: "hc!<" makes -c mandatory.
+ final Options options = PatternOptionBuilder.parsePattern("hc!<");
+ assertFalse(options.getOption("h").isRequired(), "h");
+ assertTrue(options.getOption("c").isRequired(), "c");
+ assertEquals(PatternOptionBuilder.EXISTING_FILE_VALUE,
options.getOption("c").getType());
+ }
+
@Test
void testRequiredOption() throws Exception {
final Options options = PatternOptionBuilder.parsePattern("!n%m%");
@@ -108,6 +137,14 @@ class PatternOptionBuilderTest {
assertTrue(e.getMissingOptions().contains("n"));
}
+ @Test
+ void testRequiredOptions() {
+ final Options options = PatternOptionBuilder.parsePattern("a!b:!c");
+ assertTrue(options.getOption("a").isRequired(), "a");
+ assertTrue(options.getOption("b").isRequired(), "b");
+ assertFalse(options.getOption("c").isRequired(), "c");
+ }
+
@Test
void testSimplePattern() throws Exception {
/*
