remkop / picocli

Picocli is a modern framework for building powerful, user-friendly, GraalVM-enabled command line apps with ease. It supports colors, autocompletion, subcommands, and more. In 1 source file so apps can include as source & avoid adding a dependency. Written in Java, usable from Groovy, Kotlin, Scala, etc.
https://picocli.info
Apache License 2.0
4.93k stars 424 forks source link

Bottom-up sub-commands declaration #1442

Closed rsenden closed 2 years ago

rsenden commented 3 years ago

Registering subcommands declaratively is currently being done using a top-down approach, with a parent command listing its sub-commands. Have you ever considered allowing sub-command hierarchies to be defined using a bottom-up approach?

So, in addition to the current @Command(subcommands={...}), you would have something like @Command(parent=...), with picocli automatically binding each subcommand to the declared parent.

I think this makes sense in a larger/modular/extensible CLI application, for example:

remkop commented 3 years ago

HI @rsenden, thank you for raising this. Yes I did consider this, for the reasons you mention. The main issue is discoverability. Top-down, starting from the root command, it is easy to discover all commands in the hierarchy. Bottom-up would be difficult without scanning all classes in the classpath for picocli annotations.

rsenden commented 3 years ago

Hi @remkop, thanks for your reply.

I was hoping that maybe with the annotation processor you could do this discovery during compile-time rather than at runtime (but that would make the annotation processor more or less required unless you also implement runtime classpath scanning).

Alternatively, such advanced CLI applications are likely to use some DI framework, so maybe picocli could provide some optional functionality that allows for injecting the full list of commands. Picocli could then automatically organize this collection of commands into a subcommand tree based on annotations, for example using a parent element on @Command, or using a separate @SubcommandOf(Parent.class) annotation. Picocli could have a constructor that allows for manually providing the collection of commands, or picocli could automatically get the collection of commands from the provided factory.

Of course the main disadvantage of this approach would be that this optional functionality requires a DI framework (or some IFactory implementation that performs classpath scanning or annotation processor based discovery).

Alternatively, picocli could provide an easier API for programmatically adding commands and subcommands, for example something like the following:

DI framework users can simply get all command instances injected, and then do something like the following (where getParent() uses some custom mechanism for determining the parent command class): cmds.forEach(cmd->commandLine.addCommand(getParent(cmd), cmd))

This way picocli doesn't need to do any classpath scanning and doesn't introduce any annotations/elements that require a DI framework to function.

Main issue with this approach though would be ordering; either the caller needs to somehow ensure that parent commands are added before their subcommands, or picocli would need to gracefully handle scenarios where subcommands are being added before their parent command has been added.

For comparison, following is some (very raw, non-optimized) code that I'm currently using for adding subcommand beans managed by Micronaut to picocli; having the addCommand(parent, child) method described above would greatly simplify this (and even more so if I figure out how to programmatically define Micronaut annotation-based Qualifiers).

    private static void addSubcommands(ApplicationContext context, MicronautFactory factory, CommandLine commandLine,
            RootCommand rootCommand) {
        Map<Class<?>, List<Object>> parentToSubcommandsMap = getParentToSubcommandsMap(context);
        addSubcommands(parentToSubcommandsMap, factory, commandLine, rootCommand);
    }

    private static final void addSubcommands(Map<Class<?>, List<Object>> parentToSubcommandsMap,
            MicronautFactory factory, CommandLine commandLine, Object command) {
        List<Object> subcommands = parentToSubcommandsMap.get(command.getClass());
        if (subcommands != null) {
            for (Object subcommand : subcommands) {
                CommandLine subCommandLine = new CommandLine(subcommand, factory);
                addSubcommands(parentToSubcommandsMap, factory, subCommandLine, subcommand);
                commandLine.addSubcommand(subCommandLine);
            }
        }
    }

    private static final Map<Class<?>, List<Object>> getParentToSubcommandsMap(ApplicationContext context) {
        // TODO: Use proper Qualifier to get only SubcommandOf-annotated beans instead
        // of filtering manually
        Collection<BeanDefinition<?>> beanDefinitions = context.getBeanDefinitions(Qualifiers.any());
        return beanDefinitions.stream().filter(bd -> bd.hasAnnotation(SubcommandOf.class))
                .collect(Collectors.groupingBy(bd -> bd.getAnnotation(SubcommandOf.class).classValue().get(),
                        Collectors.mapping(context::getBean, Collectors.toList())));
    }
remkop commented 3 years ago

Hi @rsenden, as I am sure you have sensed while you were writing your previous comment, every solution brings its own set of complications/complexity. I am not convinced that the benefits outweigh the drawbacks, so I won't be pursuing this.