farkhalit commented on code in PR #433:
URL: https://github.com/apache/commons-cli/pull/433#discussion_r3671820913
##########
src/main/java/org/apache/commons/cli/DefaultParser.java:
##########
@@ -518,16 +518,17 @@ private void handleProperties(final Properties
properties) throws ParseException
if (!cmd.hasOption(option) && !selected) {
// get the value from the properties
final String value = properties.getProperty(option);
-
- if (opt.hasArg()) {
- if (opt.isValuesEmpty()) {
-
opt.processValue(stripLeadingAndTrailingQuotesDefaultOff(value));
+ // the Options belong to the caller and outlive this parse, so
hold the value on a copy
+ final Option copy = (Option) opt.clone();
+ if (copy.hasArg()) {
+ if (copy.isValuesEmpty()) {
+
copy.processValue(stripLeadingAndTrailingQuotesDefaultOff(value));
Review Comment:
Good point, fixed. The no-arg skip check now runs against opt before the
clone, so a flag whose value isn't yes/true/1 never allocates a copy.
##########
src/main/java/org/apache/commons/cli/Parser.java:
##########
@@ -284,10 +284,12 @@ protected void processProperties(final Properties
properties) throws ParseExcept
if (!cmd.hasOption(option) && !selected) {
// get the value from the properties instance
final String value = properties.getProperty(option);
- if (opt.hasArg()) {
- if (opt.isValuesEmpty()) {
+ // the Options belong to the caller and outlive this parse, so
hold the value on a copy
+ final Option copy = (Option) opt.clone();
+ if (copy.hasArg()) {
+ if (copy.isValuesEmpty()) {
Review Comment:
Done here too, same restructure: the continue happens first so no clone is
made for a skipped flag.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
