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

Add ability to disable short option clustering #136

Open
cmnbroad opened this issue Apr 27, 2018 · 6 comments
Open

Add ability to disable short option clustering #136

cmnbroad opened this issue Apr 27, 2018 · 6 comments

Comments

@cmnbroad
Copy link

cmnbroad commented Apr 27, 2018

We'd like to have the ability to disable short arg clustering, since it results in some confusion for our users. #137 is a PR with proposed changes.

@pholser
Copy link
Collaborator

pholser commented Sep 21, 2018

@cmnbroad Thanks for your interest in JOpt Simple!

Can you elaborate a bit on the confusion that short option clustering is causing? There are a couple of potential remedies that don't involve adding new API:

  • Add long-option synonyms for the short options whose clustering is causing confusion
  • Note that though short options can be clustered, they need not be. For example, it's common to invoke a Unix tar extraction as tar -xvf [file], but tar -x -v -f [file].

I'm reluctant to add API unless its value is clear and convincing.

@cmnbroad
Copy link
Author

cmnbroad commented Sep 21, 2018

Hi @pholser We generally use long and short names for all of our options (though we use single letter short names only sparingly due to the shear number we have, so most of our users aren't even aware that clustering is a thing). The confusion happens when the user types a bogus long option name, i.e., -selectType. The error message the user sees is that -s is unrecognized. I think its somewhat compounded by the fact that the parser DOES accept a legitimate long name even when its specified with -. See broadinstitute/gatk#4705.

@pholser
Copy link
Collaborator

pholser commented Sep 24, 2018

@cmnbroad Thanks for the extra info.

A couple of questions:

  1. Do you train your option parsers to disallow abbreviations of long options? If not, would this help?

  2. If indeed you rarely train your parsers with short options, would removing the short options help?

  3. If neither 1) nor 2) help in your cases, I'd certainly consider your PR. I'm a bit confused by the outcome of the test that was added:

    accepts("a"); accepts("b"); accepts("c"); no-clustering;

    options = parse("-abcd");

    !has("a"); !has("b"); !has("c"); !has("abcd");

I guess I'd expect this to raise UnrecognizedOptionException. Does options have a single non-option arg? If not, do you want such an arg to be silently discarded? I think maybe more tests/work are needed to see how disallowing clustering would play with other configurations. What do you think?

CC: @twoqubed

@cmnbroad
Copy link
Author

@pholser How do you do accomplish # 1 (disallow abbreviations of long options) ? I'd certainly give it a try. And yes, its been a while since I wrote this test, but it does seem like it should raise an exception. I'll take a closer look.

@pholser
Copy link
Collaborator

pholser commented Sep 27, 2018

@cmnbroad To disallow (unambiguous) abbreviations of options, construct an OptionParser with a false parameter for allowAbbreviations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants