Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing command line args multiple times throws `AlreadySelectedException` #7282

Merged
merged 1 commit into from Aug 19, 2014

Conversation

Projects
None yet
4 participants
@dadoonet
Copy link
Member

dadoonet commented Aug 14, 2014

This issue has been fixed in commons-cli:1.3 project which sadly has not been released yet.
See https://issues.apache.org/jira/browse/CLI-183

This patch builds another list of options with no selected groups by default.

When commons-cli:1.3 will be released, we need to remove this patch.

@dadoonet dadoonet self-assigned this Aug 14, 2014

@spinscale

This comment has been minimized.

Copy link
Member

spinscale commented Aug 18, 2014

should we add a test or did your normal implementation fail with this?

@dadoonet

This comment has been minimized.

Copy link
Member Author

dadoonet commented Aug 18, 2014

@spinscale I can do it although there will be a new PR in few days with a test which actually discovers that issue. Should I add another test for the current PR?

@uboness

This comment has been minimized.

Copy link
Contributor

uboness commented Aug 18, 2014

yeah... please add small dedicated test that checks it

@dadoonet

This comment has been minimized.

Copy link
Member Author

dadoonet commented Aug 18, 2014

@spinscale @uboness Added a test which fails without the patch and passes after patch being applied.

Option option = (Option) oOption;
copy.addOption(option);
}
OptionsSource.VERBOSITY.populate(copy);

This comment has been minimized.

Copy link
@spinscale

spinscale Aug 18, 2014

Member

why VERBOSITY but not HELP here?

This comment has been minimized.

Copy link
@uboness

uboness Aug 18, 2014

Contributor

The commons-cli bug only impacts option groups

@dadoonet

This comment has been minimized.

Copy link
Member Author

dadoonet commented Aug 18, 2014

@spinscale I just added a test for --help to make sure that it does not fail. Let me know.

@uboness

This comment has been minimized.

Copy link
Contributor

uboness commented Aug 18, 2014

@dadoonet while it's ok to add the test for help it doesn't really do much... cause if it fails on normal options (not just on groups), the current implement wouldn't have worked anyway, cause you're copying all the options as they are from the current options... the destiny of the help option is the same destiny of any other option you copy

@dadoonet

This comment has been minimized.

Copy link
Member Author

dadoonet commented Aug 19, 2014

@uboness agreed.

Any other comment? Does it look good to you guys?

@spinscale

This comment has been minimized.

Copy link
Member

spinscale commented Aug 19, 2014

LGTM, like the comment about the release of commons-cli 1.3, as the last release was five years ago :-)

Cli: parsing multiple times throws `AlreadySelectedException`
This issue has been fixed in commons-cli:1.3 project which sadly has not been released yet.
See https://issues.apache.org/jira/browse/CLI-183

This patch builds another list of options with no selected groups by default.

When commons-cli:1.3 will be released, we need to remove this patch.

Closes #7282.

@dadoonet dadoonet removed the review label Aug 19, 2014

dadoonet added a commit that referenced this pull request Aug 19, 2014

Cli: parsing multiple times throws `AlreadySelectedException`
This issue has been fixed in commons-cli:1.3 project which sadly has not been released yet.
See https://issues.apache.org/jira/browse/CLI-183

This patch builds another list of options with no selected groups by default.

When commons-cli:1.3 will be released, we need to remove this patch.

Closes #7282.

(cherry picked from commit 122c2b7)

@dadoonet dadoonet merged commit 122c2b7 into elastic:master Aug 19, 2014

@dadoonet dadoonet deleted the dadoonet:pr/cli-fix-clean-groups branch Aug 19, 2014

dadoonet added a commit that referenced this pull request Sep 8, 2014

Cli: parsing multiple times throws `AlreadySelectedException`
This issue has been fixed in commons-cli:1.3 project which sadly has not been released yet.
See https://issues.apache.org/jira/browse/CLI-183

This patch builds another list of options with no selected groups by default.

When commons-cli:1.3 will be released, we need to remove this patch.

Closes #7282.

@clintongormley clintongormley changed the title Cli: parsing multiple times throws `AlreadySelectedException` Internal: Parsing command line args multiple times throws `AlreadySelectedException` Sep 8, 2014

@clintongormley clintongormley changed the title Internal: Parsing command line args multiple times throws `AlreadySelectedException` Parsing command line args multiple times throws `AlreadySelectedException` Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.