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.88k stars 420 forks source link

Options in ArgGroup with ScopeType.INHERIT do not retain their group in subcommands #1212

Open LorenKeagle opened 3 years ago

LorenKeagle commented 3 years ago

I defined a set of options in an ArgGroup in my main application class, and defined the options as CommandLine.ScopeType.INHERIT so that they will be available globally in subcommands (All descriptions removed for readability):

    @ArgGroup(heading = " File Output Options%n")
    OutputOptions outputOptions;

    static class OutputOptions {

        @Option(names = {"-e", "--edit-in-place"}, required = true, scope = CommandLine.ScopeType.INHERIT)
        boolean editInPlace;

        @Option(names = {"-o", "--output-config"}, required = true, paramLabel = "FILE", scope = CommandLine.ScopeType.INHERIT)
        File outputFile;
    }

If I run a simple 'help' command (using the mixin) on the main app, I see these options grouped as expected:

File Output Options
  -e, --edit-in-place
  -o, --output-config=FILE

However, subcommand help appears to lose the grouping (not sure about the exclusivity):

  -c, --config=FILE
  -e, --edit-in-place 
      --generate-settings=FILE
  -i, --interactive
  -o, --output-config=FILE 
  -s, --settings=FILE 
  -t, --tail-log
  -v, --verbose

Option groups defined in the subcommand are displayed correctly with their group heading, but inherited options are all grouped together with subcommand non-grouped options.

remkop commented 3 years ago

Hi @LorenKeagle, thank you for raising this! This seems related to https://github.com/remkop/picocli/issues/1182

Status

To summarize the discussion in #1182: as of picocli 4.5.1, inherited options do not combine well with argument groups. Only the options themselves are inherited to subcommands, any other argument group attributes (like headers, or validation on exclusivity with other options) is not inherited.

Because of this, as of picocli 4.5.x, I recommend against using inherited options in argument groups. I will update the documentation to make it clear that this is a restriction of inherited options. (DONE)

Workaround

For the purpose of grouping options under a header in the usage help, one idea is to use @Mixin in all commands that share these options. If the @Mixin contains the ArgGroup, then each Command using this Mixin will have its own copy of the group.

Can you check if that works for you?

Long-term resolution

Analysis is needed to determine whether it is feasible to support inherited options in argument groups.

The current syntax allows one option in a group to be inherited, while other options in that same group are not inherited. This will obviously cause problems.

One idea to solve this would be to allow scope = CommandLine.ScopeType.INHERIT on the argument group. Something like this:

@Command
class MyCommand {
    @ArgGroup(heading = " File Output Options%n", scope = CommandLine.ScopeType.INHERIT)
    OutputOptions outputOptions;
}

The above would then cause the whole group (with all its options, heading, validation settings, etc.) to be inherited by all subcommands of MyCommand.

Thoughts?

LorenKeagle commented 3 years ago

Thanks for the quick reply.

I needed to get this tool out the door, so I went with the simplest solution to me which was inheritance. That appears to work just fine.

Now that I've delivered, I'll convert my base class into a Mixin and report back if there's any issues I see with that. I think the only added complexity is how to initialize and validate the mixin parameters/options only for subcommands that use it? There's an example for verbose logging using a mixin, but that's intended for global use. The Mixin approach makes sense because I can apply it to subcommands as needed, but I think I will still have to call an init() or validate() method in each subcommand class.

remkop commented 3 years ago

@LorenKeagle Glad to hear you got something that is working. 👍 Ultimately that is what matters! 😄

LorenKeagle commented 3 years ago

I was able to update my application to use separate Mixins for logging as well as some common configuration options. Seems to be working well so far.

tomdcc commented 2 years ago

Just thought I'd add a note from another user who would like to see this, with an explanation of our use-case.

Most commands in our app connect to a service, and there are various "context" type options that configure how to do that - URL, authentication, etc. Many users will need to always set 2 or 3 of these.

Using mixins or inheritance on the individual commands that need these doesn't work for our case, where due to these being kinda unwieldy, we would like customers to be able to e.g. alias foo="java -jar foo.jar --global-opt-1=ldkfjsdlkfj --global-opt-2=dsfds" and then run foo command subcommand --subcom-opt. But we would also like to allow customers to run java -jar foo.jar command --global-opt --other-opt if that's more convenient for them.

That leaves us adding these options to our top-level command with inherited scope (as a mixin, but I don't think it needs to be). And it all works great, except that, due to these being somewhat different to per-command options, we'd like to document them in help text separately. An @ArgGroup works great, but running subcommand --help loses that info, they're back in the main pool. So supporting at least help generation for this case would be great.

FWIW we don't need any of the other functionality of arg groups currently.

Thanks

remkop commented 2 years ago

@tomdcc Thank you for raising this!

I think you can achieve your requirements with a mixin (see example below), so I did not understand this part of your comment:

Using mixins or inheritance on the individual commands that need these doesn't work for our case (...)

I tried the below, and it seems to do what you have in mind:

Am I missing something?

import picocli.CommandLine;
import picocli.CommandLine.ArgGroup;
import picocli.CommandLine.Command;
import picocli.CommandLine.Mixin;
import picocli.CommandLine.Option;

public class MixinDemo {

    @Command(mixinStandardHelpOptions = true) // add --help and --version to all commands that have this mixin
    static class MyMixin {
        @ArgGroup(heading = "%nShared options that work on every command:%n", validate = false)
        MyGroup group; // = new MyGroup(); //https://picocli.info/#_assigning_default_values_in_argument_groups
    }
    static class MyGroup {
        @Option(names = "--global-opt-1", description = "I'm nice.") String global1;
        @Option(names = "--global-opt-2", description = "I'm nicer.") String global2;
    }

    @Command(name = "command", subcommands = {Sub1.class, Sub2.class})
    static class MyCommand implements Runnable {
        @Mixin MyMixin myMixin;

        public void run() { }
    }

    @Command(name = "sub1")
    static class Sub1 implements Runnable {
        @Mixin MyMixin myMixin;

        @Option(names = "--sub1-opt-1", description = "I'm ok.") String opt1;
        @Option(names = "--sub1-opt-2", description = "I'm oker.") String opt2;
        public void run() { }

        @Command
        public void sub1sub1(
            @Mixin MyMixin mixin,
            @Option(names = "--sub1-sub1-opt-1", description = "I'm sub ok.") String subopt1,
            @Option(names = "--sub1-sub1-opt-2", description = "I'm sub oker.") String subopt2
        ) {}
    }

    @Command(name = "sub2")
    static class Sub2 implements Runnable {
        @Mixin MyMixin myMixin;

        @Option(names = "--sub2-opt-1", description = "I'm ok.") String opt1;
        @Option(names = "--sub2-opt-2", description = "I'm oker.") String opt2;
        public void run() { }
    }

    public static void main(String[] args) {
        System.out.println("---------------");
        new CommandLine(new MyCommand()).execute("--help");

        System.out.println("---------------");
        new CommandLine(new MyCommand()).execute("sub1", "--help");

        System.out.println("---------------");
        new CommandLine(new MyCommand()).execute("sub1", "sub1sub1", "--help");
    }
}

Running this prints the following (which seems to be what you are looking for, unless I am missing something):

---------------
Usage: command [-hV] [--global-opt-1=<global1>] [--global-opt-2=<global2>]
               [COMMAND]
  -h, --help      Show this help message and exit.
  -V, --version   Print version information and exit.

Shared options that work on every command:
      --global-opt-1=<global1>
                  I'm nice.
      --global-opt-2=<global2>
                  I'm nicer.
Commands:
  sub1
  sub2
---------------
Usage: command sub1 [-hV] [--global-opt-1=<global1>] [--global-opt-2=<global2>]
                    [--sub1-opt-1=<opt1>] [--sub1-opt-2=<opt2>] [COMMAND]
  -h, --help                Show this help message and exit.
      --sub1-opt-1=<opt1>   I'm ok.
      --sub1-opt-2=<opt2>   I'm oker.
  -V, --version             Print version information and exit.

Shared options that work on every command:
      --global-opt-1=<global1>
                            I'm nice.
      --global-opt-2=<global2>
                            I'm nicer.
Commands:
  sub1sub1
---------------
Usage: command sub1 sub1sub1 [-hV] [--global-opt-1=<global1>]
                             [--global-opt-2=<global2>]
                             [--sub1-sub1-opt-1=<arg1>]
                             [--sub1-sub1-opt-2=<arg2>]
  -h, --help      Show this help message and exit.
      --sub1-sub1-opt-1=<arg1>
                  I'm sub ok.
      --sub1-sub1-opt-2=<arg2>
                  I'm sub oker.
  -V, --version   Print version information and exit.

Shared options that work on every command:
      --global-opt-1=<global1>
                  I'm nice.
      --global-opt-2=<global2>
                  I'm nicer.
pio-kol commented 2 years ago

Hi @remkop , thanks for the detailed answer - that's not exactly what @tomdcc meant but that was very helpful anyway:

Please find a slightly updated example - we want to use scope = CommandLine.ScopeType.INHERIT instead of adding a mixin field into all subcommands. The reason for that is that in the case of the inheritance we can read the value of the param from the root command no matter where it was set by user (with something like this.spec.root().findOption(name)).

If mixin is used in each subcommand then the value will be available either in subcommand or root command depending on where it was set by the user, which makes it inconvenient to get this value.

What we wanted to achieve is propagating the value no matter where in the command it was set. The mixin is irrelevant here (we could skip it)

import picocli.CommandLine;
import picocli.CommandLine.ArgGroup;
import picocli.CommandLine.Command;
import picocli.CommandLine.Mixin;
import picocli.CommandLine.Option;

public class MixinDemo {

    @Command(mixinStandardHelpOptions = true) // add --help and --version to all commands that have this mixin
    static class MyMixin {

        @ArgGroup(heading = "%nShared options that work on every command:%n", validate = false)
        MyGroup group; // = new MyGroup(); //https://picocli.info/#_assigning_default_values_in_argument_groups
    }

    static class MyGroup {

        @Option(names = "--global-opt-1", description = "I'm nice.", scope = CommandLine.ScopeType.INHERIT)
        String global1;
        @Option(names = "--global-opt-2", description = "I'm nicer.", scope = CommandLine.ScopeType.INHERIT)
        String global2;
    }

    @Command(name = "command", subcommands = {Sub1.class, Sub2.class})
    static class MyCommand implements Runnable {

        @Mixin
        MyMixin myMixin;

        public void run() {
        }
    }

    @Command(name = "sub1")
    static class Sub1 implements Runnable {

        @Option(names = "--sub1-opt-1", description = "I'm ok.")
        String opt1;
        @Option(names = "--sub1-opt-2", description = "I'm oker.")
        String opt2;

        public void run() {
        }

        @Command
        public void sub1sub1(
            @Option(names = "--sub1-sub1-opt-1", description = "I'm sub ok.") String subopt1,
            @Option(names = "--sub1-sub1-opt-2", description = "I'm sub oker.") String subopt2
        ) {
        }
    }

    @Command(name = "sub2")
    static class Sub2 implements Runnable {

        @Option(names = "--sub2-opt-1", description = "I'm ok.")
        String opt1;
        @Option(names = "--sub2-opt-2", description = "I'm oker.")
        String opt2;

        public void run() {
        }
    }

    public static void main(String[] args) {
        System.out.println("---------------");
        new CommandLine(new MyCommand()).execute("--help");

        System.out.println("---------------");
        new CommandLine(new MyCommand()).execute("sub1", "--help");

        System.out.println("---------------");
        new CommandLine(new MyCommand()).execute("sub1", "sub1sub1", "--help");
    }
}

It outputs the correct grouped options from main command:

image

But in the case of the subcommands, it will mix all the options:

image

The reason for this behavior seems that ArgGroup is simply not inheritable what is possible with options so it's ignored in subcommand.

remkop commented 2 years ago

@pio-kol @tomdcc Thanks for the clarification.

It took a bit of creative puzzling, but I have been able to come up with a solution that meets your requirements and works with the current version of picocli. Please see https://github.com/remkop/picocli/blob/main/picocli-examples/src/main/java/picocli/examples/arggroup/ArgGroupMixinDemo2.java

This solution still uses mixins, but takes some effort to ensure that all global option values are stored only in the mixin of the top-level command. The mixins of the subcommands don't store option values themselves, but delegate to the mixin of the top-level command to store and retrieve option values. This is a variation of the global option use case in the user manual.

It works, but it is not a trivial solution.

I will look at introducing an INHERIT scope in the @ArgGroup annotation. I am not clear yet whether the validation aspect of ArgGroups can be inherited or not, or what trade-offs would be involved there. I imagine this will take some analysis and it may be a long time before I will be able to get to it.

pio-kol commented 2 years ago

@remkop Thanks for your engagement!

Another workaround is to simply provide the description in the footer and do not rely on it being generated by the tool. It will not display it in the args list too this way which may be good not to garbage the core options.