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

Allow repeatable subcommands? #454

Closed
idanarye opened this issue Aug 26, 2018 · 30 comments
Closed

Allow repeatable subcommands? #454

idanarye opened this issue Aug 26, 2018 · 30 comments
Labels
theme: parser An issue or change related to the parser type: API 🔌 type: enhancement ✨
Milestone

Comments

@idanarye
Copy link

idanarye commented Aug 26, 2018

I came here looking for a solution for a problem similar to #358 - the need for a syntax for lists of multi-field entries - but had a different solution in mind. These solutions are not mutually exclusive, so I elected to open a new ticket for it.

My idea is to have repeatable subcommands. So - if we take the example from #358 - print will be the main command and have, say, file as a subcommand. We will write it like so:

print --paper A4 \
    file A.pdf \
    file B.pdf --count 3 \
    file C.pdf --count 3 --rotate left \
    file D.pdf \
    file E.pdf --rotate right

This will create 6 CommandLine objects:

  • PrintCommand(paper = "A4")
  • FileCommand(name = "A.pdf", count = , rotate = )
  • FileCommand(name = "B.pdf", count = 3, rotate = )
  • FileCommand(name = "C.pdf", count = 3, rotate = "left")
  • FileCommand(name = "D.pdf", count = , rotate = )
  • FileCommand(name = "E.pdf", count = , rotate = "right")

A bit more verbose than @kravemir's original syntax (since you need to specify the options for each file) but much more readable IMHO.

I tried to do it by making file a subcommand of itself:

CommandLine printCommand = new CommandLine(new PrintCommand());
CommandLine fileCommand = new CommandLine(new FileCommand());
printCommand.addSubcommand(fileCommand);
fileCommand.addSubcommand(fileCommand);

And it did parse that command line - but it was always using the same FileCommand object so I only got E.pdf's parameters. Maybe if a CommandLine could receive a factory for objects, and construct a new one for each subcommand it parses?

@kravemir
Copy link

kravemir commented Aug 26, 2018

It's very similar to, what was suggested later on, in comment: #358 (comment) :-)

The idea is @CompositeParameters, which is a bit similar to @Mixin. The @CompositeParameters can contain parameters and options, and can be used as a repeatable part of command.

Anyway, I'm up for implementation of such solution. The print was just an example, the real use-case is image manipulation/transformation tool.

EDIT: not exactly same thing, but similar.

@kravemir
Copy link

Also, an issue already present: #434

@remkop
Copy link
Owner

remkop commented Aug 26, 2018

@idanarye I can see there is definitely a need for improvement in this area.

I’m not sure yet about repeatable subcommands; I feel the hierarchy is important in a number of ways (incl. usage help and autocompletion), and worry that would be lost if subcommands could work at any level.

I will focus on #358 first.

About your example, have you tried passing in FileCommand.class instead of new FileCommand()? I believe (away from PC) that picocli will instantiate the class and use the instance as the user object for the CommandLine.

@idanarye
Copy link
Author

About your example, have you tried passing in FileCommand.class instead of new FileCommand()? I believe (away from PC) that picocli will instantiate the class and use the instance as the user object for the CommandLine.

Didn't work. Picocli did initiate a class instance from the Class object - but it only did so once.

@remkop
Copy link
Owner

remkop commented Aug 27, 2018

@idanarye There is a lot of merit to your suggestion. If composite options (and composite positional parameters) are implemented with commands under the hood, all the parsing logic can be reused pretty much as is.

I'm still thinking about:

  • what should the annotations API look like (currently thinking a @Composite annotation on the bean class that defines the @Parameters elements)
  • what should the programmatic API look like (currently thinking ArgSpec has a CommandSpec attribute for composite options and composite positional parameters, which is null for non-composite options - it does not feel right to model this as an is a relationship)
  • usage help: what should the usage help for a composite option/positional parameter look like (difference from a normal option)? What changes in the CommandLine.Help API are needed?

@remkop
Copy link
Owner

remkop commented Aug 27, 2018

I realized that composite ArgSpecs having a CommandSpec.Builder is a better model, especially if the builder takes just the class of the composite bean in the builder constructor. Then builder.build() creates a new CommandSpec and a new composite instance each time the composite option (or composite positional parameter) is specified on the command line. The builder would act like the factory you mentioned earlier.

remkop added a commit that referenced this issue Mar 18, 2019
* mutually exclusive options (#199)
* option that must co-occur (#295)
* option grouping in the usage help message (#450)
* repeating composite arguments (#358 and #635) (this should also cover the use cases presented in #454 and #434 requests for repeatable subcommands)
@remkop remkop added this to the 4.0 milestone Mar 19, 2019
@remkop remkop modified the milestones: 4.0, 4.0-alpha-1 Mar 30, 2019
@remkop
Copy link
Owner

remkop commented Mar 30, 2019

Picocli 4.0.0-alpha-1 has been released which includes support for repeating composite groups.
See https://picocli.info/#_argument_groups for details.

I believe this should should meet the requirements of the original use case.

Please try this and provide feedback. We can still make changes.

What do you think of the annotations API? What about the programmatic API? Does it work as expected? Are the input validation error messages correct and clear? Is the documentation clear and complete? Anything you want to change or improve? Any other feedback?

@remkop remkop closed this as completed Mar 30, 2019
@remkop remkop modified the milestones: 4.0-alpha-1, 4.0 Mar 30, 2019
@remkop remkop reopened this Mar 30, 2019
@remkop
Copy link
Owner

remkop commented Mar 31, 2019

Reopened as repeatable subcommands may have use cases not covered by repeatable argument groups. #635 may be an example.

@hanslovsky
Copy link

I just tried and I can add sub-commands with double-dash syntax, so that would be a feasible solution for me, as well. I could even have a sub-command with repeatable argument groups and that would be a working solution already, e.g. instead of

command sub-command --opt1=a --opt2=a sub-command --opt1=b --opt2=b

I would have something like this (I made sub-commands plural to stress the fact that it can add multiples):

command sub-commands --opt1=a --opt2=a --opt1=b --opt2=b

where opt1 and opt2 are part of an ArgGroup. This is a little less intuitive than multiple repetitions of sub-command but it should be a viable workaround.

@remkop
Copy link
Owner

remkop commented Apr 18, 2019

To support repeating subcommands it may make sense to give commands a multiplicity attribute.

This multiplicity indicates how many times the subcommand ca be specified. The default would be "0..1", meaning that by default, a subcommand is optional but can be specified at most once.

If a subcommand can be specified multiple times, it should be defined something like the below:

@Command (multiplicity ="0..*")
class MyCommand {}

@remkop
Copy link
Owner

remkop commented May 16, 2019

This is proving to be a popular feature: see this stackoverflow question.

remkop added a commit that referenced this issue Jun 13, 2019
@remkop
Copy link
Owner

remkop commented Jun 13, 2019

TBD: after doing some prototyping:

  • API: how should applications enable this? @Command(repeatable = true)? @Command(siblings = true)?
  • What are the exact requirements? Do we want to allow end users to specify either subcommands or the current command or do we also want to allow sibling commands?

What is clear is that to support this, the ParseResult class needs another method subcommands() that returns the list of subcommands that were matched for the current command.

@remkop
Copy link
Owner

remkop commented Jun 13, 2019

I am thinking to shelve this feature until the requirements are more clear.

The example in #635 and the description of this ticket are the only use cases so far., and they can be addressed with composite repeating groups (although the verb "add" in #635 suggests that a subcommand would be a better fit than than a repeating option group).

All, if you have ideas, suggestions, requirements: please comment here!

@lakemove
Copy link

To add 1 possible usecase :

eodcli \
  import --db=jdbc:oracle:local --user=sa --sql="select * from data" \
  report --out /tmp/report.csv \
  email  --recipients Jay@gmail.com 

chain several subcommands together : read db , generate report and send email notification.
jvm is too expensive to use pipe.

@remkop
Copy link
Owner

remkop commented Jan 29, 2020

Status update

I believe the implementation is now complete. I added tests and updated the documentation.

Remaining work is just doc tweaking; I will reorder the sections under Subcommands a little.
When this is done I will close this ticket.

How you can help

If you are interested in repeating subcommands, you may have a use case that you want to try this on. You can test by checking out the latest master and building with:

gradlew clean publishToMavenLocal

That should publish picocli-4.2.0-SNAPSHOT to your local .m2 Maven cache. You can then try this in a project that uses the info.picocli:picocli:4.2.0-SNAPSHOT dependency.

Feedback welcome!

@hanslovsky
Copy link

This looks cool! Haven't really gotten to try this yet but I prepared a kscript from your example above which may be helpful for quick prototyping:

#!/usr/bin/env kscript

@file:DependsOn("info.picocli:picocli:4.2.0-SNAPSHOT")

import picocli.CommandLine
import picocli.CommandLine.Command

@Command(name="A",
        subcommandsRepeatable = true,
        subcommands = [B::class, C::class, D::class])
class A

@Command(name="B",
        subcommandsRepeatable = true,
        subcommands = [E::class, F::class, G::class])
class B

@Command(name="C") class C {}
@Command(name="D") class D {}
@Command(name="E") class E {}
@Command(name="F") class F {}
@Command(name="G") class G {}

val a = A()
val cl = CommandLine(a)
val parseRsult = cl.parseArgs(*args)

@hanslovsky
Copy link

I tried a repeatable subcommand with a String CommandLine.Option. If the option has arity="1", then everything works as expected but if the arity="*" then the repeated specification of the subcommand will be interpreted as an option. This is an ill-defined problem by itself and I understand that the ambiguity has to be resolved arbitrarily, i.e. parse everything as a value for the option with arity="*". Is this the intented behavior? The alternative would be for the sub-command know any other sub-command names but I can see that this can get messy quickly. As an alternative solution, is there an argument to indicate that subcommand is completet, e.g. --.

This is my example:

#!/usr/bin/env kscript

@file:DependsOn("info.picocli:picocli:4.2.0-SNAPSHOT")

import java.util.concurrent.Callable

import picocli.CommandLine
import picocli.CommandLine.Command
import picocli.CommandLine.Option
import picocli.CommandLine.Parameters

@Command(name="MainCmd",
        subcommandsRepeatable = true,
        subcommands = [Container::class])
class MainCmd : Callable<Int> {
    @Option(names=["--help", "-h"], usageHelp=true)
    var helpRequested: Boolean = false

    override fun call(): Int {
        return 0
    }
}

@Command(name="--with-container")
class Container : Callable<Int> {
    @Parameters(arity="1")
    lateinit var path: String

    @Option(names=["--dataset", "-d"], arity="*")
    var datasets: Array<String>? = null
    
    @Option(names=["--help", "-h"], usageHelp=true)
    var helpRequested: Boolean = false

    override fun call(): Int {
        println(this)
        return 0
    }

    override fun toString() = ContainerData(path, datasets?.toList()).toString()
}

data class ContainerData(val path: String, val datasets: List<String>?)

val mainCmd = MainCmd()
val cl = CommandLine(mainCmd)
val exitCode = cl.execute(*args)

This results in:

$ ./test-repeatable-subcommands.kts --with-container abc --dataset a --with-container xyz --dataset x
ContainerData(path=abc, datasets=[a, --with-container, xyz, x])

If I change the arity of Container.datasets to"1", this is the output (which is what I would like to see):

$ ./test-repeatable-subcommands.kts --with-container abc --dataset a --with-container xyz --dataset x
ContainerData(path=abc, datasets=[a])
ContainerData(path=xyz, datasets=[x])

@idanarye
Copy link
Author

I need this for the plotting feature in a data collection app I should be am writing. Each plot has:

  • One or more axes.
  • Zero or more filters - which are sliders/combos that can be played with in the GUI.
  • One or more formulas - which are the values that I show in the plot.

Even though my app is still WIP I don't want to make it depend on a snapshot (I can procrastinate wait until you officially release 4.2.0) so I copied the data classes to a new repository and wrote a CLI just for creating and printing them: https://github.com/idanarye/test-picocli-repeatable-subcommands.

What I did is create a parent CommandLine that contains the PlotEntry object, and repeatable subcommands that axes, filters and formulas to it. This allows me to write single commands that create complete plot entries:

java -jar build/libs/shadow.jar my-plot \
    axis Year sample.year \
    axis Height sample.year -u centimeters \
    filter Age 'sample.year - person.birthYear' -u years -t NUMERIC_RANGE \
    filter Gender person.gender -t TEXTUAL_SINGLE \
    formula Salary sample.salary --symbol $ -u USD -S LOGARITHMIC \
    formula Happyness 'sample.pizzaEaten + sample.beerConsumed' --symbol ':-)'

The only drawback of this approach is that I need to some more processing after CommandLine.execute() to handle the result command - could be nice if I could make picocli execute some method on the parent command after all its subcommands have finished.

remkop added a commit that referenced this issue Jan 30, 2020
@remkop
Copy link
Owner

remkop commented Jan 30, 2020

@hanslovsky Thanks for raising the arity issue! That was a bug.
I pushed a fix to master.

I also added repeatable-subcmds-example.kts to picocli-examples/src/main/kotlin/picocli/examples/kotlin based on your example script.

I tested the fix in Java but I was unable to run your script from IntelliJ (error: invalid argument: --with-container). Can you try again with the latest master?

@remkop
Copy link
Owner

remkop commented Jan 30, 2020

@idanarye, the simplest way to invoke a method on the top-level command object that I can think of would be to call it from the main method in your program.

Something like this:

@Command(name = "topcmd", subcommandsRepeatable = true)
class TopCmd implements Runnable {
    public void run() {
        System.out.println("topcmd called");
    }

    @Command
    void axis(@Parameters(index = "0") String str, @Parameters(index="1") File f) {
        System.out.println("axis command called");
    }

    @Command
    void filter(@Option(names = "-u") String u, @Option(names="-t") String t) {
        System.out.println("filter command called");
    }

    /* ... */
    public void postProcessing() {
        System.out.println("...and we're done!");
    }

    public static void main(String... args) {
        args = "axis Year a.year axis Height b.year filter -u=x -t=y filter -t=tt".split(" ");

        TopCmd top = new TopCmd();
        int exitCode = new CommandLine(top).execute(args);
        top.postProcessing(); // execute some method on the parent command after all its subcommands have finished.
    }
}

remkop added a commit that referenced this issue Jan 30, 2020
@hanslovsky
Copy link

@hanslovsky Thanks for raising the arity issue! That was a bug.
I pushed a fix to master.

I also added repeatable-subcmds-example.kts to picocli-examples/src/main/kotlin/picocli/examples/kotlin based on your example script.

I tested the fix in Java but I was unable to run your script from IntelliJ (error: invalid argument: --with-container). Can you try again with the latest master?

@remkop I pulled the most recent master and tested my script and now it works as expected. Thank you for the quick fix.

@idanarye
Copy link
Author

@idanarye, the simplest way to invoke a method on the top-level command object that I can think of would be to call it from the main method in your program.

It gets a bit trickier when you want to have multiple "real" subcommands and the ones you want to do postprocessing on are not the root ones, but it's still doable.

@remkop
Copy link
Owner

remkop commented Jan 31, 2020

@idanarye I see what you mean now.

Picocli can help find the parent command of the executed subcommands. All that is required is that we make the command methods (or the Callable for a @Command-annotated class) return something, like an int, for example. (This is a natural thing to do anyway if your application needs to control the command's exit code.)

That allows you to use the ParseResult::asCommandLineList and CommandLine::getExecutionResult methods to find the parent: the parent will have a null execution result, but the executed subcommands will have a non-null execution result. For example:

// all commands now return an int
@Command(name = "topcmd", subcommandsRepeatable = true)
static class TopCmd implements Callable<Integer> {
    public Integer call() {
        System.out.println("topcmd called");
        return 0;
    }

    @Command
    int axis(@Parameters(index = "0") String str, @Parameters(index="1") File f) {
        System.out.println("axis command called");
        return 0;
    }

    @Command
    int filter(@Option(names = "-u") String u, @Option(names="-t") String t) {
        System.out.println("filter command called");
        return 0;
    }

    /* ... */
    public void postProcessing() {
        System.out.println("...and we're done!");
    }

    public static void main(String... args) {
        args = "axis Year a.year axis Height b.year filter -u=x -t=y filter -t=tt".split(" ");

        CommandLine cmd = new CommandLine(new TopCmd());
        int exitCode = cmd.execute(args);

        // Given that all commands return something,
        // we can use the CommandLine::getExecutionResult method 
        // to find the parent command of the subcommands that were executed.
        List<CommandLine> matched = cmd.getParseResult().asCommandLineList();
        CommandLine parent = null;
        for (CommandLine commandLine : matched) {
            if (commandLine.getExecutionResult() == null) { // this command was not executed
                parent = commandLine;
            } else {
                break; // we found the first subcommand that was executed
            }
        }
        System.out.println("The parent of the executed subcommands is: " + parent.getCommandName());

        // execute some method on the parent command
        if (parent.getCommand() instanceof TopCmd) {
            TopCmd top = parent.getCommand();
            top.postProcessing();
        }
    }
}

In Java 8, the loop to find the parent can be a bit cleaner:

// Given that all commands return something,
// we can use the CommandLine::getExecutionResult method 
// to find the parent command of the subcommands that were executed.
Optional<CommandLine> parent = cmd.getParseResult().asCommandLineList().stream()
        .takeWhile(c -> c.getExecutionResult() == null)
        .reduce((cmd1, cmd2) -> cmd2);

@idanarye
Copy link
Author

Why return a meaningless integer, when I return a value for the top command to process instead?

I ended up with something like this:

	CommandLine cli = new CommandLine(new App());
	cli
	    .addSubcommand("add", new PlotCommandAdd())
	    .addSubcommand("show", new PlotCommandShow())
	    .execute(args);
	SubcommandPostprocessing postprocessingTarget = null;
	LinkedList<Object> postprocessingArgs = null;
	for (CommandLine cmd : cli.getParseResult().asCommandLineList()) {
	    if (cmd.getCommand() instanceof SubcommandPostprocessing) {
		if (postprocessingTarget != null) {
		    postprocessingTarget.postProcess(postprocessingArgs);
		}
		postprocessingTarget = cmd.getCommand();
		postprocessingArgs = new LinkedList<>();
	    } else if (postprocessingArgs != null) {
		Object executionResult = cmd.getExecutionResult();
		if (executionResult != null) {
		    postprocessingArgs.add(executionResult);
		}
	    }
	}
	if (postprocessingTarget != null) {
	    postprocessingTarget.postProcess(postprocessingArgs);
	}

I added the execute-and-switch part in the middle of the loop hoping I can run multiple top level commands:

java -jar build/libs/shadow.jar \
    add my-plot \
        axis Year sample.year \
        axis Height sample.year -u centimeters \
        filter Age 'sample.year - person.birthYear' -u years -t NUMERIC_RANGE \
        filter Gender person.gender -t TEXTUAL_SINGLE \
        formula Salary sample.salary --symbol $ -u USD -S LOGARITHMIC \
        formula Happyness 'sample.pizzaEaten + sample.beerConsumed' --symbol ':-)' \
    show \
        axis Year sample.year \
        axis Height sample.year -u centimeters \
        filter Age 'sample.year - person.birthYear' -u years -t NUMERIC_RANGE \
        filter Gender person.gender -t TEXTUAL_SINGLE \
        formula Salary sample.salary --symbol $ -u USD -S LOGARITHMIC \
        formula Happyness 'sample.pizzaEaten + sample.beerConsumed' --symbol ':-)'

But this doesn't work. Should picocli recognize that show is not a subcommand of add and pop the command stack to check if it's another top level command?

@remkop
Copy link
Owner

remkop commented Feb 1, 2020

At the moment, it is not possible to go up the hierarchy with repeatable subcommands.
I've added a note to that effect in the documentation (build/docs/html5/index.html#_repeatable_subcommands).
I think repeatable subcommands can be confusing, so for now I'd like to keep it simple.
Thoughts?

@idanarye
Copy link
Author

idanarye commented Feb 1, 2020

I'm already abusing them enough as is, so I'm not going to request any farther complications.

@remkop
Copy link
Owner

remkop commented Feb 12, 2020

@idanarye, @kravemir, @hanslovsky, @lakemove, All,

picocli 4.2.0 has been released, including this feature. Enjoy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: parser An issue or change related to the parser type: API 🔌 type: enhancement ✨
Projects
None yet
Development

No branches or pull requests

5 participants