[jira] [Comment Edited] (GROOVY-8520) Replace commons-cli with picocli in CliBuilder
[
https://issues.apache.org/jira/browse/GROOVY-8520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16478756#comment-16478756
]
Remko Popma edited comment on GROOVY-8520 at 5/17/18 8:48 AM:
--
Reopening: please review https://github.com/apache/groovy/pull/701.
This PR addresses some issues raised in feedback on the 2.5-rc2 release.
was (Author: rem...@yahoo.com):
Reopening: please review [PR #710|https://github.com/apache/groovy/pull/701].
This PR addresses some issues raised in feedback on the 2.5-rc2 release.
> Replace commons-cli with picocli in CliBuilder
> --
>
> Key: GROOVY-8520
> URL: https://issues.apache.org/jira/browse/GROOVY-8520
> Project: Groovy
> Issue Type: Improvement
> Components: command line processing
>Reporter: Remko Popma
>Assignee: Paul King
>Priority: Major
> Fix For: 2.5.0-rc-2
>
>
> This ticket proposes to replace commons-cli with picocli in
> {{groovy.util.CliBuilder}}.
> See [discussion on the mailing
> list|https://lists.apache.org/thread.html/d60b6d5d4411e9ba0d7dc209cde8a9bb4abb00f0b9c0322f068c322e@%3Cdev.groovy.apache.org%3E]
> for the original proposal and comparison with other CLI libraries.
> Goals for the initial implementation:
> * preserve the current CliBuilder behaviour as much as possible
> * deliver an implementation, tests and documentation in time to be included
> in the 2.5 GA release
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (GROOVY-8520) Replace commons-cli with picocli in CliBuilder
[
https://issues.apache.org/jira/browse/GROOVY-8520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16446924#comment-16446924
]
Remko Popma edited comment on GROOVY-8520 at 4/21/18 6:17 PM:
--
I replaced pull request https://github.com/apache/groovy/pull/687 with a
cleaner PR https://github.com/apache/groovy/pull/688 which is rebased on the
GROOVY_2_5_X branch and has all commits squashed.
It adds a new module {{groovy-cli-picocli}} with a
{{groovy.cli.picocli.CliBuilder}} class, tests and documentation. This
implementation of {{CliBuilder}} is fully compatible with the commons-cli
implementation as far as I can see.
Based on [~paulk]'s feedback I guess the next step would be to replace all
places where Groovy uses commons-cli internally with the picocli equivalent. I
can work on this in the next week or so. Not sure what is involved in removing
commons-cli from the classpath for all tests.
was (Author: rem...@yahoo.com):
I replaced pull request https://github.com/apache/groovy/pull/687 with a
cleaner PR https://github.com/apache/groovy/pull/688 which is rebased on the
GROOVY_2_5_X branch and has all commits squashed.
It adds a new module {{groovy-cli-picocli}} with a
{{groovy.cli.picocli.CliBuilder}} class, tests and documentation.
Based on [~paulk]'s feedback I guess the next step would be to replace all
places where Groovy uses commons-cli internally with the picocli equivalent. I
can work on this in the next week or so. Not sure what is involved in removing
commons-cli from the classpath for all tests.
> Replace commons-cli with picocli in CliBuilder
> --
>
> Key: GROOVY-8520
> URL: https://issues.apache.org/jira/browse/GROOVY-8520
> Project: Groovy
> Issue Type: Improvement
> Components: command line processing
>Reporter: Remko Popma
>Priority: Major
>
> This ticket proposes to replace commons-cli with picocli in
> {{groovy.util.CliBuilder}}.
> See [discussion on the mailing
> list|https://lists.apache.org/thread.html/d60b6d5d4411e9ba0d7dc209cde8a9bb4abb00f0b9c0322f068c322e@%3Cdev.groovy.apache.org%3E]
> for the original proposal and comparison with other CLI libraries.
> Goals for the initial implementation:
> * preserve the current CliBuilder behaviour as much as possible
> * deliver an implementation, tests and documentation in time to be included
> in the 2.5 GA release
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (GROOVY-8520) Replace commons-cli with picocli in CliBuilder
[
https://issues.apache.org/jira/browse/GROOVY-8520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16435635#comment-16435635
]
Remko Popma edited comment on GROOVY-8520 at 4/12/18 3:07 PM:
--
I have a clean solution for the caching issue and CliBuilder instances can now
safely be reused to parse different input multiple times. I’ll push this to my
fork soon.
Update: this is now pushed.
I published picocli v3.0.0-alpha-6. If jcenter cannot find the jar yet you can
get the [jar from
GitHub|https://github.com/remkop/picocli/releases/download/v3.0.0-alpha-6/picocli-3.0.0-alpha-6.jar]
and build with a local file dependency (see build.gradle line 182 in my fork).
was (Author: rem...@yahoo.com):
I have a clean solution for the caching issue and CliBuilder instances can now
safely be reused to parse different input multiple times. I’ll push this to my
fork soon.
> Replace commons-cli with picocli in CliBuilder
> --
>
> Key: GROOVY-8520
> URL: https://issues.apache.org/jira/browse/GROOVY-8520
> Project: Groovy
> Issue Type: Improvement
> Components: command line processing
>Reporter: Remko Popma
>Priority: Major
>
> This ticket proposes to replace commons-cli with picocli in
> {{groovy.util.CliBuilder}}.
> See [discussion on the mailing
> list|https://lists.apache.org/thread.html/d60b6d5d4411e9ba0d7dc209cde8a9bb4abb00f0b9c0322f068c322e@%3Cdev.groovy.apache.org%3E]
> for the original proposal and comparison with other CLI libraries.
> Goals for the initial implementation:
> * preserve the current CliBuilder behaviour as much as possible
> * deliver an implementation, tests and documentation in time to be included
> in the 2.5 GA release
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (GROOVY-8520) Replace commons-cli with picocli in CliBuilder
[
https://issues.apache.org/jira/browse/GROOVY-8520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16425493#comment-16425493
]
Remko Popma edited comment on GROOVY-8520 at 4/4/18 1:30 PM:
-
h3. Breakdown of failing tests in {{CliBuilderTest}}
h4. Behavioural Changes (please give feedback)
* testTypedUnparsedFromInstance/testTypedUnparsedFromSpec: fails because the
test tries to define an {{@Unparsed Integer[] nums}} field/method. The new
CliBuilder impl. only allows field/methods of type {{String[]}} array or
{{List}} to be annotated with {{@Unparsed}}. Picocli distinguishes between
positional parameters (which can be typed) and "unmatched" input (list of
strings). I propose to disallow strongly typed {{@Unparsed}} fields and add
functionality for strongly typed positional parameters later (e.g. different
types at different positions would then be possible). This would be different
from previous 2.5 releases though. Thoughts?
* testFailedParsePrintsUsage: the new CliBuilder introduces an {{errorWriter}}
(default stderr) for error messages on invalid user input. Requested
({{--help}}) usage help goes to the existing {{printWriter}} (default stdout).
This follows command line application conventions: diagnostic output should go
to stderr to prevent it from being parsed when its output is used as input for
another process. Requested help should always go to stdout so that users can
pipe it to {{less}} or {{grep}}. This would be an incompatible change though.
Thoughts?
* testParseFromInstance: previous CliBuilder initialized annotated {{Boolean}}
fields/setters to {{false}} even when they were not specified on the command
line. Picocli does not modify options that are not specified. The latter seems
more correct/natural to me but would be different behaviour from previous 2.5
releases. Thoughts?
h4. Bugs that I may need help with
* testParseFromSpec: for some reason the map to interface coercion is not
working...
* testParseFromInstanceFlagEdgeCases (same as above)
h4. Other bugs
* testMultipleOccurrencesSeparateSeparate: to be backwards compatible,
{{cli.a}} should return the first element of the values matched for the {{-a}}
option, not the full list
* testMultipleOccurrencesTogetherJuxtaposed (same as above)
* testMultipleOccurrencesSeparateJuxtaposed (same as above)
* testMultipleOccurrencesTogetherSeparate (same as above)
* testMixedBurstingAndLongOptions: to be backwards compatible, {{longOpt}}
names should be registered with both a single hyphen and with two hyphens in
picocli.
* testMultipleArgs: to be backwards compatible, {{args: 2}} (currently mapped
to {{arity=2}} in picocli, but see below) needs to count option parameters
_after_ the parameters have been split into parts.
* testValSepClass: to be backwards compatible, {{args: 2}} should be mapped to
{{arity="1..2"}} in picocli
h4. Invalid (test uses commons-cli API)
* testLongAndShortOpts_allOptionsValid
* testSampleLong
* testSampleShort
* testLongOptsOnly_nonOptionShouldStopArgProcessing
h4. Fixed
* testConvertClass: fix by adding {{as CommandLine.ITypeConverter}} when
installing the closure instance as type converter on the OptionSpec.Builder
(CliBuilder line 497)
* testParseFromInstance: fix by changing {{m.invoke(t, [it])}} to
{{m.invoke(t, [it].toArray())}} at CliBuilder line 442, 550, 553 and 558
h3. TODO
* Add tests for unparsed variations:
** {{@Unparsed String[] unmatched}} field
** {{@Unparsed List unmatched}} field
** {{@Unparsed List unmatched()}} method on spec interface and concrete
instance
** {{@Unparsed unmatched(List)}} setter method on concrete instance
** {{@Unparsed unmatched(String[])}} setter method on concrete instance
** {{@Unparsed unmatched(String)}} setter (actually accumulator) method on
concrete instance
* other tests...
was (Author: rem...@yahoo.com):
h3. Breakdown of failing tests in {{CliBuilderTest}}
h4. Behavioural Changes (please give feedback)
* testTypedUnparsedFromInstance/testTypedUnparsedFromSpec: fails because the
test tries to define an {{@Unparsed Integer[] nums}} field/method. The new
CliBuilder impl. only allows field/methods of type {{String[]}} array or
{{List}} to be annotated with {{@Unparsed}}. Picocli distinguishes between
positional parameters (which can be typed) and "unmatched" input (list of
strings). I propose to disallow strongly typed {{@Unparsed}} fields and add
functionality for strongly typed positional parameters later (e.g. different
types at different positions would then be possible). This would be different
from previous 2.5 releases though. Thoughts?
* testFailedParsePrintsUsage: the new CliBuilder introduces an {{errorWriter}}
(default stderr) for error messages on invalid user input. Requested usage help
goes to the existing {{printWriter}} (default stdout). This follows command
line appl
[jira] [Comment Edited] (GROOVY-8520) Replace commons-cli with picocli in CliBuilder
[
https://issues.apache.org/jira/browse/GROOVY-8520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16425493#comment-16425493
]
Remko Popma edited comment on GROOVY-8520 at 4/4/18 1:23 PM:
-
h3. Breakdown of failing tests in {{CliBuilderTest}}
h4. Behavioural Changes (please give feedback)
* testTypedUnparsedFromInstance/testTypedUnparsedFromSpec: fails because the
test tries to define an {{@Unparsed Integer[] nums}} field/method. The new
CliBuilder impl. only allows field/methods of type {{String[]}} array or
{{List}} to be annotated with {{@Unparsed}}. Picocli distinguishes between
positional parameters (which can be typed) and "unmatched" input (list of
strings). I propose to disallow strongly typed {{@Unparsed}} fields and add
functionality for strongly typed positional parameters later (e.g. different
types at different positions would then be possible). This would be different
from previous 2.5 releases though. Thoughts?
* testFailedParsePrintsUsage: the new CliBuilder introduces an {{errorWriter}}
(default stderr) for error messages on invalid user input. Requested usage help
goes to the existing {{printWriter}} (default stdout). This follows command
line application conventions: diagnostic output should go to stderr to prevent
it from being parsed when its output is used as input for another process.
Requested help should always go to stdout so that users can pipe it to {{less}}
or {{grep}}. This would be an incompatible change though. Thoughts?
* testParseFromInstance: previous CliBuilder initialized annotated {{Boolean}}
fields/setters to {{false}} even when they were not specified on the command
line. Picocli does not modify options that are not specified. The latter seems
more correct/natural to me but would be different behaviour from previous 2.5
releases. Thoughts?
h4. Bugs that I may need help with
* testParseFromSpec: for some reason the map to interface coercion is not
working...
* testParseFromInstanceFlagEdgeCases (same as above)
h4. Other bugs
* testMultipleOccurrencesSeparateSeparate: to be backwards compatible,
{{cli.a}} should return the first element of the values matched for the {{-a}}
option, not the full list
* testMultipleOccurrencesTogetherJuxtaposed (same as above)
* testMultipleOccurrencesSeparateJuxtaposed (same as above)
* testMultipleOccurrencesTogetherSeparate (same as above)
* testMixedBurstingAndLongOptions: to be backwards compatible, {{longOpt}}
names should be registered with both a single hyphen and with two hyphens in
picocli.
* testMultipleArgs: to be backwards compatible, {{args: 2}} (mapped to
{{arity=2}} in picocli) needs to count option parameters _after_ the parameters
have been split into parts.
* testValSepClass: to be backwards compatible, {{args: 2}} should be mapped to
{{arity="1..2"}} in picocli
h4. Invalid (test uses commons-cli API)
* testLongAndShortOpts_allOptionsValid
* testSampleLong
* testSampleShort
* testLongOptsOnly_nonOptionShouldStopArgProcessing
h4. Fixed
* testConvertClass: fix by adding {{as CommandLine.ITypeConverter}} when
installing the closure instance as type converter on the OptionSpec.Builder
(CliBuilder line 497)
* testParseFromInstance: fix by changing {{m.invoke(t, [it])}} to
{{m.invoke(t, [it].toArray())}} at CliBuilder line 442, 550, 553 and 558
h3. TODO
* Add tests for unparsed variations:
** {{@Unparsed String[] unmatched}} field
** {{@Unparsed List unmatched}} field
** {{@Unparsed List unmatched()}} method on spec interface and concrete
instance
** {{@Unparsed unmatched(List)}} setter method on concrete instance
** {{@Unparsed unmatched(String[])}} setter method on concrete instance
** {{@Unparsed unmatched(String)}} setter (actually accumulator) method on
concrete instance
* other tests...
was (Author: rem...@yahoo.com):
h3. Breakdown of failing tests in {{CliBuilderTest}}
h4. Behavioural Changes (please give feedback)
* testTypedUnparsedFromInstance/testTypedUnparsedFromSpec: fails because the
test tries to define an {{@Unparsed Integer[] nums}} field/method. The new
CliBuilder impl. only allows field/methods of type {{String[]}} array or
{{List}} to be annotated with {{@Unparsed}}. Picocli distinguishes between
positional parameters (which can be typed) and "unmatched" input (list of
strings). I propose to disallow strongly typed {{@Unparsed}} fields and add
functionality for strongly typed positional parameters later (e.g. different
types at different positions would then be possible). This would be different
from previous 2.5 releases though. Thoughts?
* testFailedParsePrintsUsage: the new CliBuilder introduces an {{errorWriter}}
(default stderr) for error messages on invalid user input. Requested usage help
goes to the existing {{printWriter}} (default stdout). This follows command
line application conventions: diagnostic output
