Preprocessor callbacks during parsing (was: Make string options behave same as boolean options)

[remkop]

From discussion on the picocli google group mailing list, topic raised by @maxandersen:

when I have:

@CommandLine.Option(names = {"--live" })
boolean live;

then jbang --live=true and jbang --live=false works, and jbang --live false is not interpreting the false argument as a possible value. and jbang --live=custom i can even intercept with custom value handler.

now...how can I get the same for

@CommandLine.Option(names = {"--live" })
String live;

with this I cannot force that only --live=myarg gets taken as input and that --live myarg should not be treated as a value to live.

any way for this to work ?

My reply:

The intention was that --option=value and --option value would be treated identically.
There is indeed some special logic for booleans, that takes the separator into account.
This is because boolean options have arity=0, meaning they are not expected to have a parameter.
If the user specified --option=value, then the presence of the separator "forces" us to accept "value" as a parameter, and that is what the logic does.

This is a special case. Generally I would think that treating --option=value and --option value differently would be confusing to users, but there may be other use case that I don't understand yet.

Currently the IParameterConsumer interface does not give any indication of whether the value was attached with a separator or not, that may have been a bad decision.

@maxandersen's reply:

I think it would been great to know if parameter is done via = or not as I really dont see another way of allowing to have options that can have a default beavhior but then be able to specify another option....well you can but that would require having one boolean nand then another flag for the option.

i.e. in my case its jbang where I want to do jbang edit --open myfile.java (which says please make temporary project for myfile.java for me to edit, oh and please open my default editor on that project)
but I also want to be able to write jbang edit --open=idea myfile.java which says the same but says, use the idea command/editor in this case.

There is no real way for me to know wether myfile.java is actually the editor he wants to use or not ....

I've used parameterconsumer in other cases pretty nicely for numeric values where if the value was a number i would acccept it but not if a string/filename that existed - but it just gets dirty.

And when I look at all my special cases I could have handled it much more uniformly if I had a "OnlyExplicitlyAssignedParameterConsumer.class" or something ;)

---

btw. see jbangdev/jbang#314 for latest issue i'm trying to solve and made me open this one. might give you some more context.

My reply:

Thank you for clarifying the use case: there is an option with an optional parameter, and this introduces ambiguity.

I like your proposed solution to try to disambiguate the input with a '=' separator;
it is intuitive that A is the option parameter in a command like jbang edit --open=A B, where jbang edit --open A B is more ambiguous.
The drawback is that users need to know this.

A similar solution (that already works with all versions of picocli) is to disambiguate with the -- end-of-options delimiter.
jbang edit --open -- A B # both A and B are positional parameters
jbang edit --open A -- B # A is a parameter for the open option and B is a positional parameter

This has the same drawback that users need to know this.
Arguably the -- end-of-options delimiter is less well-known, although it can be shown in the usage help with @Command(showEndOfOptionsDelimiterInUsageHelp=true).
Still, this is something to consider for commands that define both an option with an optional parameter and a positional parameter.

Ultimately, this is a limitation of the picocli parser: if a command has an option with an optional parameter (like --live[=editor]), then the picocli parser will currently greedily assign the value that follows the option to this option. This is regardless of whether the command also defines a positional parameter and the last argument could also have been interpreted as that positional parameter. This is especially painful if the positional parameter is mandatory (see #981).

I raised #980 to improve the parser, but this is not trivial:
it essentially requires the parser to detect ambiguity (all options or positional parameters whose arity is a range instead of a fixed number),
and build up multiple parse results: one for each possible interpretation of the input.
If we succeed in that, we need to decide what to do in cases where

• all interpretations failed (what error message to produce)
• multiple interpretations succeeded (which one to choose)

Your idea to disambiguate with '=' will not solve the full range of ambiguity problems, but it may be easier to implement than a backtracking parser.
Currently I can think of two mechanisms to do this:

1. introduce a new @Option(assignFallbackWhenSeparatorMissing = true|false) attribute.
   For options with an optional parameter, this attribute would tell the parser to use the '=' separator to disambiguate the input.
   The value following the option is only considered to be the option parameter if the value is attached to the option with the '=' separator.
2. introduce a new IParameterConsumer2 interface, that takes an additional LookBehind { SEPARATE, ATTACHED, ATTACHED_WITH_SEPARATOR} enum parameter to the consumeParameters method.
   We would also need an additional @Option(parameterConsumer2 = <class>) attribute.
   Applications can then use this to build custom disambiguation logic.
3. Somehow make this information (of whether a separator was specified or not) available to implementors of the existing IParameterConsumer interface.
   Perhaps by adding some method on ArgSpec, CommandSpec or any object reachable from these objects.
   Of by setting a static ThreadLocal. :-)

There may be other approaches, please let me know your thoughts.

@maxandersen's reply

Still, this is something to consider for commands that define both an
option with an optional parameter and a positional parameter.

issue is that it only work for one parameter - what if you have two ?

1. introduce a new @Option(assignFallbackWhenSeparatorMissing = true|false) attribute. For options with an optional parameter, this attribute would tell the parser to use the '=' separator to disambiguate the input. The value following the option is only considered to be the option parameter if the value is attached to the option with the '=' separator.

+1 for a boolean for its simplicity.

2. introduce a new IParameterConsumer2 interface, that takes an additional LookBehind { SEPARATE, ATTACHED, ATTACHED_WITH_SEPARATOR} enum parameter to the consumeParameters method. We would also need an additional @Option(parameterConsumer2 = <class>) attribute. Applications can then use this to build custom disambiguation logic.

doesn't feel right.

3. Somehow make this information (of whether a separator was specified or not) available to implementors of the existing IParameterConsumer interface. Perhaps by adding some method on ArgSpec, CommandSpec or any object reachable from these objects.

better than #2

Of by setting a static ThreadLocal. :-)

noooo! ::)

[remkop]

On a different ticket (#1214), @dwalluck offered an interesting suggestion that may help solve this issue: introducing a preprocessor that would allow implementors to modify option parameters without consuming them. Picocli would then process the modified option parameter. This could also work for positional parameters.

[remkop]

Something like this:

interface IParameterPreprocessor {
    /**
     * Called when either the specified command, or the specified option or positional parameter was
     * recognized by the picocli parser.
     * <p>Implementors are free to modify one or more of the specified command line arguments before they are
     * processed by the picocli parser (or the {@link IParameterConsumer parameter consumer}, if specified).
     * </p><p>
     * Implementors may optionally <em>consume</em> one or more of the specified command line arguments:
     * a return value of {@code true} signals that the preprocessor consumed the parameter:
     * picocli should skip further processing of this option or positional parameter, 
     * and the preprocessor implementation takes responsibility for assigning the
     * option or positional parameter a value. A return value of {@code false} means that picocli should
     * process the stack as usual for this option or positional parameter, and picocli is responsible for
     * assigning this option or positional parameter a value.
     * </p><p>
     * If the user input is invalid, implementations should throw a {@link ParameterException}
     * with a message to display to the user.
     * </p>
     * @param args the remaining command line arguments that follow the matched argument
     *           (the matched argument is not on the stack anymore)
     * @param commandSpec the command or subcommand that was matched, or 
     *           (if the specified {@code argSpec} is not {@code null}) the command that 
     *           the option or positional parameter belongs to
     * @param argSpec the option or positional parameter for which to pre-process command line arguments
     *             (may be {@code null} when this method is called for a subcommand that was matched)
     * @param info a map containing additional information on the current parser state, 
     *                including whether the option parameter was attached to the option name with
     *                a `=` separator, whether quotes have already been stripped off the option, etc.
     *                (TODO key-values to be documented here)
     * @returns true if the preprocessor consumed the parameter 
     *          and picocli should skip further processing of the stack for this option or positional parameter;
     *          false if picocli should continue processing the stack for this option or positional parameter
     * @throws ParameterException if the user input is invalid
     */
    boolean processParameters(Stack<String> args, CommandSpec commandSpec, ArgSpec argSpec, Map<String, Object> info);
}

Example usage:

@Option(names = {"--live" }, preprocessor = MyProcessor.class)
String live;

[maxandersen]

interesting. That does sound interesting.

+1

[remkop]

Note that #1256 discusses a related use case, where there is a need to get a callback when the parser recognized a subcommand (as opposed to when an option or positional parameter was recognized).

@MarkoMackic suggested that the new use case deserves a separate interface (something like ICommandPreprocessor, details TBD) since the use case does not involve options; when a subcommand is recognized there is no ArgSpec to pass to the preprocessor, and the meaning of the return value may also be different.

I think it makes sense to discuss these two use cases together to explore ideas on this.

[MarkoMackic]

@remkop So no preprocessors at model building time ?

[remkop]

@MarkoMackic, I see, good point: on #1256 you mentioned the use case of modifying the command dynamically at model build time for things like man page generation and perhaps completion script generation.

To be honest, I am not convinced about providing API for this: changing the model dynamically feels like one of those things that picocli should make possible, but not necessarily easy or convenient. From the next release (as per the new CommandSpec::removeSubcommand method added for #1256), it will be possible to achieve your use case with code like this:

static CommandLine createDynamicallyModifiedCommand() {
    CommandLine result = new CommandLine(new MyApp());
    if (someCondition()) {
        result.getCommandSpec().removeSubcommand("sub");
    }
    return result;
}

This method can then be used in main to launch the application, and can be passed to the ManPageGenerator.generateManPage method, for example. So at least these use cases are possible, and I may be convinced later that it should be made convenient.

Meanwhile, having an API that gives callbacks during parse time opens many possibilities that currently do not exist at all.

[MarkoMackic]

@remkop sorry but I don't agree that this is good API, or we didn't understand each other at all. As I currently understood let's say you want to remove subommand c which is subcommand of b which is subcommand of a which is subcommand of main command line.

If I understood correctly this is how I'd do it by current API proposal :

static CommandLine createDynamicallyModifiedCommand() {
    CommandLine result = new CommandLine(new MyApp());
    if (someCondition()) {
        result.getCommandSpec().subcommands().findSubcommand("a")
                 .getCommandSpec().subcommands().findSubcommand("b")
                 .removeSubcommand("c");
    }
    return result;
}

The code above is just an illustration. I don't know the actual method names.

[remkop]

@MarkoMackic Apologies, I thought you were okay with the parse-time preprocessor idea. Perhaps I closed #1256 too soon.

I would like to keep this ticket focused on the API for callbacks during parse time.
I created a separate ticket (#1259) to discuss your use case further.