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

Flag to treat first positional parameter as end-of-options #284

Closed
pditommaso opened this issue Feb 10, 2018 · 13 comments
Closed

Flag to treat first positional parameter as end-of-options #284

pditommaso opened this issue Feb 10, 2018 · 13 comments

Comments

@pditommaso
Copy link
Contributor

pditommaso commented Feb 10, 2018

Take in consideration a command as shown below:

class Cmd {
    @Option(names = "--alpha") String alpha;
    @Parameters(index = "0", arity = "1") String foo;
    @Parameters(index = "1..*", arity = "*") List<String> params;
}

It parses correctly a command line like:

<cmd> foo xx --alpha --beta 

resulting:

cmd.foo == 'foo'
cmd.params == ['xx', '--alpha', '--beta']

Instead the command line:

<cmd> foo --xx --alpha --beta 

throws the exception:

UnmatchedArgumentException: Unmatched argument [--xx]

Is this a feature or a bug?

I'm more inclined to the latter, also because I've noticed that when a command does not specify any double - option it is able to parse the above command line without reporting the unmatched argument exception.

I know that I could bypass that exception using a -- separator, however in any case it looks a bit incoherent behaviour.

@remkop
Copy link
Owner

remkop commented Feb 10, 2018

Yes, that looks strange. I’ll take a look.

@remkop
Copy link
Owner

remkop commented Feb 10, 2018

To me, the first result is unexpected (and wrong). A command like this:

<cmd> foo xx --alpha --beta 

Should result in

alpha  = --beta
foo    = foo
params = [xx]

It appears that the arity = "*" causes the parser to consume remaining arguments greedily without checking whether the argument is an option. This is incorrect. Only after a -- argument should the parser no longer try to interpret arguments as options.

The second result looks correct. Command line arguments that look like an option should result in UnmatchedParameterException (unless unmatchedArgumentsAllowed is set to true).

@pditommaso
Copy link
Contributor Author

It depends. I was looking exactly for this behaviour and the problem is that last example.

Let me give some other details about by use case: I need to implement a command line which behave somehow similar to the one of docker ie.

<tool> [--tool option].. <third-party-command> [command parameters]

Everything after the third-party-command should be delegated to the that command ie. picocli should not try to map option parameters. Basically it should work as when using the double-dash separator but the user should be be required to type the -- characters.

@remkop
Copy link
Owner

remkop commented Feb 10, 2018

That sounds a bit like the use case described in #215. To implement that, I propose to add a CommandLine.setStopOnUnmatched(boolean) method. If stopOnUnmatched is set to true, and an unknown option is encountered on the command line, the parser will stop interpreting the arguments and move that argument and all remaining arguments into the unmatchedArguments list. This allows the application to do the parsing in phases.

Would this meet your requirements?

About the first result, I do intend to fix that since the current behaviour does not follow the specification as described in the Mixing Options and Positional Parameters section of the user manual.

@pditommaso
Copy link
Contributor Author

Nearly. But the proposed stopOnUnmatched could result on a wrong parsing just for a user typo. Maybe there should be a way to mark a parameter as greedy ie. that will consume everything that comes after.

@pditommaso
Copy link
Contributor Author

pditommaso commented Feb 10, 2018

In alternative it should be possible to disable the Mixing options and position parameters feature to allow the implementation of a positional parameter command line.

@remkop
Copy link
Owner

remkop commented Feb 10, 2018

Ok, that may be a good idea.

By the way, I want to understand your use case better. Can you give a few examples of the application you are implementing with support for 3rd party commands? In addition, an example of how a user typo could give wrong parsing result would also be useful.

@pditommaso
Copy link
Contributor Author

Think about Docker, for example:

docker run --rm -ti <image-name> [command line passed to the image]

The run command need to capture the options --rm and -ti (and eventually others) and the image-name parameter, after that it would be required a parameter capturing all values whether they are.

In my understanding the stopOnUnmatched stop the parsing as soon there's an unmatched option, let's say --rn instead of --rm. But I need to report an error in that case.

@remkop
Copy link
Owner

remkop commented Feb 10, 2018

(FYI messages crossed. I posted this before seeing your last message.)

In your first example above, I assume that your intention was to capture the 3rd party command name and its arguments in the params list. I want to clarify that with stopOnUnmatched, you shouldn't do this.

Instead, ensure that your command only has options and positional parameters that apply to your command. Avoid any potentially ambiguous options or positional parameters with variable arity (arity = "*").

Then the 3rd party command name and its arguments will be captured in the unmatchedArguments list instead of in the @Parameters field in your command:

class Cmd {
    @Option(names = "--alpha") String alpha;
    @Parameters(index = "0", arity = "1") String foo;

    public static void main(String... args) {
        CommandLine commandLine = new CommandLine(new Cmd())
            .setStopOnUnmatched(true);
        commandLine.parse(args);

        List<String> unmatched = commandLine.getUnmatchedArguments();
        invokeThirdPartyCommand(unmatched);
    }
}

@remkop
Copy link
Owner

remkop commented Feb 10, 2018

Thanks for the docker example. That clarifies the spelling mistake concern you mentioned.

I see how you are better served with an option that disables the Mixing options and position parameters behaviour. Something like stopAtPositional?

stopAtPositional

When this option is enabled, the parser interprets the first positional parameter as "end of options" so the remaining arguments are all treated as positional parameters. The command must have a multi-value (array, Collection or Map) field to store the positional parameters.

If the command does not have a multi-value field, or the field's arity is too small to store all command line arguments, an exception is thrown. (I am debating whether to throw MaxValuesforFieldExceededException here, or to keep the current behaviour, which is to throw an UnmatchedArgumentException, unless unmatchedArgumentsAllowed is set to true.)

stopAtUnmatched

stopAtPositional is different from stopAtUnmatched. Picocli considers a command line argument "unmatched" when the argument does not match an option name and either of the following is true:

  • it "resembles" an option (has the same prefix as the other options, or starts with "-" if the command has no options)
  • there are no remaining slots for positional parameters (the command has no positional parameters or it has a limited number and they have all been given a value)

Unmatched command line arguments are stored in the CommandLine's unmatchedArguments list, not in a command field. (There is a plan to introduce an @Unmatched annotation to give commands access to these arguments.)

@pditommaso
Copy link
Contributor Author

The stopAtPositional would work !

@remkop remkop added this to the 2.3 milestone Feb 11, 2018
@remkop remkop changed the title Open parameter does not capture values starting with a -- prefix Flag to treat first positional parameter as end-of-options Feb 11, 2018
remkop added a commit that referenced this issue Feb 11, 2018
remkop added a commit that referenced this issue Feb 11, 2018
remkop added a commit that referenced this issue Feb 12, 2018
@remkop
Copy link
Owner

remkop commented Feb 13, 2018

Fixed in master and 2.x branch. Please verify.

@remkop remkop closed this as completed Feb 13, 2018
@remkop
Copy link
Owner

remkop commented Feb 13, 2018

Released in 2.3.0

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

No branches or pull requests

2 participants