Nested execution flow for subcommands is slightly awkward.

[michaelpj]

Here's a fairly natural flow for a command-subcommand process:

• Arguments for parent command are parsed
• Parent command performs some operations to initialize some state etc.
• Subcommand starts
  • Subcommand arguments are parsed
  • Subcommand performs some operations, using its own args, the parent command args, and the parent command state.
  • Subcommand finishes
• Parent command performs some cleanup operations.

This is kind of hard to do at the moment. CommandLine::parseWithHandler lets you run all the commands in sequence, and a solution to #247 would solve the problem of accessing the parent command's args and state, but the bit where the parent command also clears up its own state at the end is hard to set up.

Ideally, I imagine something like:

interface Subcommand<T> {
    public void run(T parent);
}

which subcommands implement. Then there would be a natural flow where each command calls its subcommands, passing in itself.

[remkop]

The parseWithHandler method already gives you the flexibility to accomplish such cleanup.

You could implement a custom IParseResultHandler that runs all commands in sequence and finally calls any cleanup method on the parent command (or on all commands for that matter).

For example:

// This example subclasses picocli.CommandLine.RunAll - you may want to 
// create a Decorator that delegates to RunAll instead

class RunAllWithCleanup extends RunAll {
    public List<Object> handleParseResult(List<CommandLine> parsedCommands, PrintStream out, Help.Ansi ansi) {
        List<Object> result = super.handleParseResult(parsedCommands, out, ansi);
        cleanup(parsedCommands.get(0).getCommand());
        return result;
    }

    private void cleanup(Object topLevelCommand) {
        if (topLevelCommand instanceof Closeable) {
            ((Closeable) topLevelCommand).close();
        }
    }
}

Usage:

new CommandLine(new TopLevelCommand())
        .parseWithHandler(new RunAllWithCleanup(), System.err, args);

[remkop]

This is a good use case though. I will be sure to mention it in the article I'm planning to write on using picocli to create command line apps with subcommands.

[michaelpj]

Yes, you can do it, but it's a bit awkward and not very typesafe. I don't have particularly strong feelings about how you handle it, but hopefully it's a useful case to bear in mind!

[remkop]

I tried to experiment with your suggestion.

You can create a custom IParseResultHandler that assumes commands and subcommands all implement the Consumer interface so we can pass them their parent command.

I arrive at something like this.

public class TypeSafeRunAll<T extends Consumer<T>> implements IParseResultHandler {
    public List<Object> handleParseResult(List<CommandLine> parsedCommands, PrintStream out, Help.Ansi ansi) {
        if (CommandLine.printHelpIfRequested(parsedCommands, out, ansi)) {
            return Collections.emptyList();
        }
        T parent = null;
        for (CommandLine parsed : parsedCommands) {
            execute(parsed, parent);
            parent = (T) parsed.getCommand();
        }
        return Collections.emptyList();
    }

    private void execute(CommandLine parsed, T parent) {
        Object command = parsed.getCommand();
        if (!(command instanceof Consumer)) {
            throw new CommandLine.ExecutionException(parsed, "All commands must implement the Consumer interface");
        }
        ((T) command).accept(parent);
    }

    public static void main(String[] args) {
        args = new String[] {"-v", "sub"};
        new CommandLine(new Top()).parseWithHandler(new TypeSafeRunAll(), System.out, args);
    }

    @CommandLine.Command(name = "top", subcommands = Sub.class)
    static class Top<K> implements Consumer<K> {
        @Option(names = "-v") boolean verbose;
        @Override
        public void accept(K parent) {
            System.out.println("Top says hi " + parent);
        }
    }

    @CommandLine.Command(name = "sub")
    static class Sub<K> implements Consumer<K> {
        @Override
        public void accept(K parent) {
            System.out.println("Sub says hi " + parent);
            System.out.println("Parent verbose = " + ((Top) parent).verbose);
        }
    }
}

I think #247 is a good idea but I believe that picocli already provides the API to accomplish what you are asking for in this ticket, unless I'm missing something.

[remkop]

@michaelpj Have you had a chance to think about this further?

I heard your feedback that the provided mechanism is awkward and not typesafe, but I have trouble seeing how it can be improved upon. I experimented with the Subcommand<T> interface you propose but it does not offer any additional type safety that I can see. The problem is that the T is the class of the subcommand and most subcommands don't really share a type...

Please let me know if you want to propose further improvements (I'm open to ideas) or if you want to close this ticket.

[michaelpj]

I'm on holiday at the moment and not currently thinking about this. I'll get back to you!

[remkop]

I have thought about this some more. The parseWithHandlers method was designed to accomodate this and similar use cases where an application needs to execute commands in a different order or call additional operations on some commands.

Calling some cleanup method on the parent command after executing all commands is a perfect example of such custom processing, and it seems to me that the design caters for it well.

As you point out, there is a trade-off: any custom processing needs to have some knowledge of what additional operations the commands may support, in order to invoke these operations. The alternative proposed in the original post is to push this knowledge into the commands themselves. Since #247 added support for the @ParentCommand annotation this can be done without introducing an additional Subcommand<T> interface.

So, I believe all the tools are there to support the mentioned use cases. Feel free to reopen with feedback.