[CLI-340] Add CommandLine.getParsedOptionValues() by Claudenw · Pull Request #334 · apache/commons-cli

Claudenw · 2024-11-12T17:41:36Z

Fix for CLI-340 (add getParsedOptionValues())
fix a minor formatting issue in Help javadoc.

garydgregory

See exception management comment (rebase on git master svp)
For all Javadoc of new methods: Can the method return null? If so, when? What happens if the option does not exist?
Are all these APIs really needed? Couldn't we just add a minimal set following the YAGNI principle. Not my use case, so you tell me ;-)

garydgregory · 2024-11-12T19:01:52Z


+    /**
+     * Gets a version of this {@code Option} converted to an array of a particular type.
+     *


Can the method return null? If so, when? What happens if the option does not exist?

As with all the value getter methods if the option is not in the command line null is returned unless the default value is defined.

OK, then please update the Javadoc instead of writing it up here ;-)

garydgregory · 2024-11-12T19:02:37Z

+     * @see PatternOptionBuilder
+     * @since 1.10.0
+     */
+    public <T> T[] getParsedOptionValues(final char opt) throws ParseException {


No need to abbreviate IMO: opt -> option. Also, using opt in Javadoc makes for awkward reading.

I am following the pattern used in the class. Methods from earlier versions used opt, I elected to do the same.

Well, it's slightly awkward (to me) to read "@param opt the name of the option." If 'opt' is the name, then why not call it 'name' or 'optionName'? Might as well do the best we can for new code. We can fix the rest separately if you want or do it all in this PR since.

Claudenw · 2024-11-12T23:10:27Z

For all Javadoc of new methods: Can the method return null? If so, when? What happens if the option does not exist?

I updated the javadoc to reflect the null or default value return as appropriate. I also updated the javadoc for the getParsedOptionValue() methods.

Are all these APIs really needed? Couldn't we just add a minimal set following the YAGNI principle. Not my use case, so you tell me ;-)

There are all here because they follow the same pattern as getParsedOptionValue() methods. I think they need to be here because V1 supports all those access methods for getOptionValue, getParsedOptionValue, and getOptionValues.

I think that in V2 I would limit each set to 4 implementations getOptionValue(Option), getOptionValue(OptionGroup), getOptionValue(Option, default), and getOptionValue(OptionGroup, default). The char and String versions are just fluff. I think that since the application has to create the Options it can keep them around and use them directly without referring to the char or String versions keys.

garydgregory

Hi @Claudenw

I've added 3 comments.

garydgregory · 2024-11-12T23:51:28Z


+    /**
+     * Gets a version of this {@code Option} converted to an array of a particular type.
+     *


OK, then please update the Javadoc instead of writing it up here ;-)

garydgregory · 2024-11-12T23:53:55Z

+     * @see PatternOptionBuilder
+     * @since 1.10.0
+     */
+    public <T> T[] getParsedOptionValues(final char opt) throws ParseException {


Well, it's slightly awkward (to me) to read "@param opt the name of the option." If 'opt' is the name, then why not call it 'name' or 'optionName'? Might as well do the best we can for new code. We can fix the rest separately if you want or do it all in this PR since.

Claudenw · 2024-11-15T16:26:35Z

If you want me to change "opt" to "option" throughout the CommandLine class I'll do it. But having some method params be "opt" and others be "option" seems sloppy.

garydgregory · 2024-11-15T16:35:42Z

If you want me to change "opt" to "option" throughout the CommandLine class I'll do it. But having some method params be "opt" and others be "option" seems sloppy.

Hi @Claudenw
I don't want to give you more work so I'll leave it up to you and it could be done later.

Claudenw · 2024-11-26T11:25:45Z

@garydgregory Let's fix the naming in V2.

garydgregory

LGTM. I'll make the trivial changes mentioned later.

Specifically we'll need this feature from the upcoming v1.10 of commons-cli: apache/commons-cli#334 The assumption here is that the release should be available *reasonably soon*. No release date has been announced, but going by the pace of previous releases and the activity/minor version diff it seems . It was either this or patch the class implementation temporarily, doing this seemed like the better option. Collaborators/testers need to obtain a prebuilt package or build from source in the meantime.

### What changes were proposed in this pull request? This PR aims to upgrade `commons-cli` to 1.10.0 which is the first release tested with **Java 25-ea**. ### Why are the changes needed? To bring the latest improvements and bug fixes. - https://commons.apache.org/proper/commons-cli/changes.html#a1.10.0 (2025-07-30) - [Run Java 24 and 25-ea](apache/commons-cli@e6a611f) - [[CLI-333] org.apache.commons.cli.Option.Builder implements](apache/commons-cli@c9698e6) - apache/commons-cli#314 - apache/commons-cli#334 - [[CLI-341] HelpFormatter infinite loop with 0 width input](apache/commons-cli@bce0f6a) - [[CLI-343] OptionFormatter.getBothOpt() lacks validation for Options](apache/commons-cli@859d5e5) - [[CLI-344] Option.processValue() throws NullPointerException when passed](apache/commons-cli@4e0cdd0) - [[CLI-347] Options.addOptionGroup(OptionGroup) does not remove required](apache/commons-cli@42be792) - [[CLI-349] DefaultParser.parse() throws NullPointerException when options](apache/commons-cli@ff678b0) ### Does this PR introduce _any_ user-facing change? No behavior change. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #51816 from dongjoon-hyun/SPARK-53102. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

### What changes were proposed in this pull request? This PR aims to upgrade `commons-cli` to 1.10.0 which is the first release tested with Java 25-ea. ### Why are the changes needed? To bring the latest improvements and bug fixes. - https://commons.apache.org/proper/commons-cli/changes.html#a1.10.0 (2025-07-30) - [Run Java 24 and 25-ea](apache/commons-cli@e6a611f) - [[CLI-333] org.apache.commons.cli.Option.Builder implements](apache/commons-cli@c9698e6) - apache/commons-cli#314 - apache/commons-cli#334 - [[CLI-341] HelpFormatter infinite loop with 0 width input](apache/commons-cli@bce0f6a) - [[CLI-343] OptionFormatter.getBothOpt() lacks validation for Options](apache/commons-cli@859d5e5) - [[CLI-344] Option.processValue() throws NullPointerException when passed](apache/commons-cli@4e0cdd0) - [[CLI-347] Options.addOptionGroup(OptionGroup) does not remove required](apache/commons-cli@42be792) - [[CLI-349] DefaultParser.parse() throws NullPointerException when options](apache/commons-cli@ff678b0) ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #2349 from dongjoon-hyun/ORC-1968. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

### What changes were proposed in this pull request? This PR aims to upgrade `commons-cli` to 1.10.0 which is the first release tested with Java 25-ea. ### Why are the changes needed? To bring the latest improvements and bug fixes. - https://commons.apache.org/proper/commons-cli/changes.html#a1.10.0 (2025-07-30) - [Run Java 24 and 25-ea](apache/commons-cli@e6a611f) - [[CLI-333] org.apache.commons.cli.Option.Builder implements](apache/commons-cli@c9698e6) - apache/commons-cli#314 - apache/commons-cli#334 - [[CLI-341] HelpFormatter infinite loop with 0 width input](apache/commons-cli@bce0f6a) - [[CLI-343] OptionFormatter.getBothOpt() lacks validation for Options](apache/commons-cli@859d5e5) - [[CLI-344] Option.processValue() throws NullPointerException when passed](apache/commons-cli@4e0cdd0) - [[CLI-347] Options.addOptionGroup(OptionGroup) does not remove required](apache/commons-cli@42be792) - [[CLI-349] DefaultParser.parse() throws NullPointerException when options](apache/commons-cli@ff678b0) ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #2349 from dongjoon-hyun/ORC-1968. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 7a1a34f) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

Claudenw self-assigned this Nov 12, 2024

Claudenw marked this pull request as ready for review November 12, 2024 17:42

Claudenw requested a review from garydgregory November 12, 2024 17:45

garydgregory requested changes Nov 12, 2024

View reviewed changes

Claudenw added 4 commits November 13, 2024 00:00

Fix for CLI-340

44a4fa2

removed unused import

9a5ff69

fixed javadocs and catch(Throwable) issues

d05f194

rebased and fixed catch issue

a55b1ff

Claudenw force-pushed the CLI-340_add_getParsedOptionValues branch from d1df605 to a55b1ff Compare November 13, 2024 00:04

Claudenw requested a review from garydgregory November 15, 2024 16:25

Remove extra whitespace

a7f3e69

garydgregory approved these changes Nov 29, 2024

View reviewed changes

garydgregory merged commit 764c148 into apache:master Nov 29, 2024

garydgregory changed the title ~~Fix for CLI-340~~ [CLI-340] CommandLine.getParsedOptionValues() Nov 29, 2024

garydgregory changed the title ~~[CLI-340] CommandLine.getParsedOptionValues()~~ [CLI-340] Add 'CommandLine.getParsedOptionValues() Nov 29, 2024

garydgregory changed the title ~~[CLI-340] Add 'CommandLine.getParsedOptionValues()~~ [CLI-340] Add CommandLine.getParsedOptionValues() Nov 29, 2024

asfgit pushed a commit that referenced this pull request Nov 29, 2024

[CLI-340] Add CommandLine.getParsedOptionValues() #334

aad30ef

ann-hav mentioned this pull request Jun 30, 2025

SYM-684: Sensitivity matrix import, basic support havochvatten/symphony-import#2

Merged

This was referenced Aug 4, 2025

[SPARK-53102][BUILD] Upgrade commons-cli to 1.10.0 apache/spark#51816

Closed

ORC-1968: Upgrade commons-cli to 1.10.0 apache/orc#2349

Closed

Uh oh!

Conversation

Claudenw commented Nov 12, 2024 • edited by garydgregory Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garydgregory left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Claudenw commented Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

garydgregory left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Claudenw commented Nov 15, 2024

Uh oh!

garydgregory commented Nov 15, 2024

Uh oh!

Claudenw commented Nov 26, 2024

Uh oh!

garydgregory left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Claudenw commented Nov 12, 2024 •

edited by garydgregory

Loading

garydgregory left a comment •

edited

Loading

Claudenw commented Nov 12, 2024 •

edited

Loading