Bugfix/cli 221 add Option.Builder.listValueSeparator() #382
Conversation
…roperly use options with single argument-commaseparated values. To remain backward compatibility to the java-property-style parsing wth default valueSeparator '=', this mutually exclusive new method needed to be added
|
@garydgregory : would be nice if you can have a look at it when it suits you. I updated the branch after latest release, I bumped the doc version of the new api to 1.11.0 (as new API is in it). I don't need a release soon, I just seek "closure" one way or the other on this issue. |
|
@ppkarwasz @Claudenw do either of you have an opinion here? |
| /** The character that is the value separator. */ | ||
| private char valueSeparator; | ||
|
|
||
| /** multiple values are within a single argument separated by valueSeparator char */ |
There was a problem hiding this comment.
A sentence should start with a capital letter.
| } | ||
|
|
||
| /** | ||
| * defines the separator used to split a list of values passed in a single argument. |
There was a problem hiding this comment.
A sentence should start with a capital letter.
| * | ||
| * </pre> | ||
| * | ||
| * @since 1.11.0 |
There was a problem hiding this comment.
The since tag should be after the param tags.
| private char valueSeparator; | ||
|
|
||
| /** multiple values are within a single argument separated by valueSeparator char */ | ||
| private boolean valueSeparatorUsedForSingleArgument; |
There was a problem hiding this comment.
A sentence should start with a capital letter.
|
@garydgregory fixed the "low level comments", not sure what your "pr github resolve conversation policy" for tivial fixes is, for now I leave it for you to resolve. |
Hello @JoergBudi There is no official policy, but what I usually do is close the conversations I started. Note that my comments, "A sentence should start with a capital letter," have not been addressed. Ty! |
|
Hi Gary, |
|
I posted a longish overview into the CLI-221. I would like to see discussion to identify and resolve the end of option argument frame detection issues. |
Strictly speaking, the described behaviour in CLI-221 is not a bug
as it is needed to fulfill the documented behaviour described in
https://issues.apache.org/jira/browse/CLI-325
around interpreting java property like arguments.
However, when you want to parse list values instead of java properties,
the behaviour is bad as the user need to terminate a list argument with
double dash --, which is counterintuitive.
Also changing the option's order changes the behaviour.
The suggested solution in PR adds the new
Option.Builder.listValueSeparator() methods, which must be used instead
of Option.Builder.valueSeparator() to achieve the behaviour desired
in this issues' description, see javadoc for usage.
It is only added in the DefaultParser() and not fixed for the
other deprecated parser implementations.
The user can then use the option in any order, e.g.
!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
Thanks for your contribution to Apache Commons! Your help is appreciated!
Before you push a pull request, review this list:
mvn; that'smvnon the command line by itself.