CLI-254: "test" gets parsed as test, quotes die :-( #58
Conversation
stoty
force-pushed
the
CLI-254
branch
2 times, most recently
from
c8d337b
to
14e7d40
Compare
|
-1: Changing the default behavior is a no go. |
|
@garydgregory But this doesn't change the default behaviour. It is adding a new parameter to a new constructor, that must be explicitly set to false to enable the sane behavior. |
|
Rebased and added the Builder to DefaultParser. |
There was a problem hiding this comment.
@stoty
Thank you for the updated PR. Please see my comments scattered throughout.
| } | ||
|
|
||
| /** | ||
| * Create a new DefaultParser instance with the specified partial matching and quote |
| * @since 1.5 | ||
| */ | ||
| public static final class Builder | ||
| { |
There was a problem hiding this comment.
Place { at the end of the previous line.
| /** | ||
| * Sets if balanced leading and trailing double quotes should be stripped from option arguments. | ||
| * | ||
| * with "stripping of balanced leading and trailing double quotes from option arguments" turned |
There was a problem hiding this comment.
Odd start for a sentence. Sentences should start with a capitalized word.
| * | ||
| * with "stripping of balanced leading and trailing double quotes from option arguments" turned | ||
| * on, the outermost balanced double quotes of option arguments values will be removed. | ||
| * ie. |
There was a problem hiding this comment.
Odd formatting where "ie." is a line by itself. Better to spell it out anyway.
| } | ||
|
|
||
| /** | ||
| * Constructs an DefaultParser with the values declared by this {@link Builder}. |
There was a problem hiding this comment.
In general, a build methods "Builds", a contructor "Constructs", so "Constructs" -> "Builds".
| @Test | ||
| public void testLongOptionWithEqualsQuoteHandling() throws Exception | ||
| { | ||
| assumeTrue(parser instanceof DefaultParser); |
There was a problem hiding this comment.
Why do all these methods say assumeTrue(parser instanceof DefaultParser);?
If these tests are only for DefaultParser, then put the new tests in the proper subclass.
|
While working on this, I discovered that quote handlings needs to be a tri-valued parameter. |
add a new constructor parameter to DefaultParser that disables quote
processing on option arguments