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

Support @Command(scope=INHERIT) (was: mixinStandardHelpOptions versus CommandLine.ScopeType.INHERIT) #1164

Closed
rnc opened this issue Sep 6, 2020 · 6 comments

Comments

@rnc
Copy link
Contributor

rnc commented Sep 6, 2020

According to https://picocli.info/#_help_options I can add mixinStandardHelpOptions = true which replaces the individual

@Option(names = {"-h", "--help"}, usageHelp = true, description = "display this help message")
boolean usageHelpRequested;

However if for the latter I have configured CommandLine.ScopeType.INHERIT then the help command is added to every subcommand as I understand. But it seems that adding it via the mixinStandardHelpOptions = true it is not inherited. Would it be possible to consider enabling inheritance for

        mixinStandardHelpOptions = true

and possibly

        versionProvider = VersionProvider.class,

Thanks.

@remkop
Copy link
Owner

remkop commented Sep 7, 2020

Hi @rnc, I can see how this would be useful. I also use the same version provider, as well as the -h and -V options, in all subcommands when I have a CLI app with subcommands.

I don't want to change the semantics of mixinStandardHelpOptions = true and versionProvider = VersionProvider.class and make them INHERIT-scope so they are automatically applied it to all subcommands; that may break existing applications.

One alternative idea to solve this is to add programmatic API that applies these settings to the top-level command and all subcommands recursively, similar to CommandLine.setSeparator and CommandLine.setResourceBundle. What do you think?

That said, there are already two ways to deal with this with a limited amount of code repetition:

  • put mixinStandardHelpOptions = true and versionProvider = VersionProvider.class in a superclass that all my commands inherit from
  • put mixinStandardHelpOptions = true and versionProvider = VersionProvider.class in a class that is mixed in to all my commands

The last method (using a mixin) has the advantage that it allows other superclasses to inherit from, and it also works with @Command-annotated methods. I find myself using either of the above methods in my own CLI applications.

For example:

@Command(mixinStandardHelpOptions = true, versionProvider = CommonVersionProvider.class)
public class CommonStuffMixin {
}

@Command(name = "myapp")
public class TopLevelCommand {
    @Mixin CommonStuffMixin common;

    @Option(names = "-x")
    int x;

    @Command(subcommands = SubSubCommand.class)
    void subcommand(@Mixin CommonStuffMixin common2) {
    }

    public static void main(String[] args) {
        new CommandLine(new TopLevelCommand()).execute(args);
    }
}

@Command(name = "subsub")
public class SubSubCommand {
    @Mixin
    CommonStuffMixin common;

    @Option(names = "-y")
    int y;
}

public class CommonVersionProvider implements IVersionProvider {
    @Override
    public String[] getVersion() {
        return new String[] { "MyApp 1.2.3" };
    }
}

This installs the CommonVersionProvider and the standard help options (-h and -V) mixin in all commands, admittedly with some repetition.

Thoughts?

@rnc
Copy link
Contributor Author

rnc commented Sep 7, 2020

Hi!
Thanks for the reply!

I completely agree that changing the semantics of mixinStandardHelpOptions = true and versionProvider = VersionProvider.class would not be ideal.

I like the idea of e.g. CommandLine.setInherit (enum { mixinStandardHelpOptions | versionProvider | both } ) ( for instance).

I am aware of the Mixin (or superclass) ... but was trying to avoid that given the really nice INHERIT option you added. Currently I just did
https://github.com/project-ncl/bacon/compare/master...rnc:NCL5974?expand=1#diff-8cbde70865f66dfdb4b88703c5a1dd3eR114

which seems to do the equivalent without having to add the help command explicitly (via anotation/mixin/superclass) to all subclasses.

@remkop
Copy link
Owner

remkop commented Sep 8, 2020

Thinking about this some more, I can see how it would be useful to introduce a scope method in @Command, so you could do this:

@Command(name = "bacon", 
        scope = INHERIT, // copy all attributes defined here to subcommands and sub-subcommands
        description = "Bacon CLI",
        mixinStandardHelpOptions = true,
        versionProvider = VersionProvider.class,
        subcommands = { Da.class, Pig.class, Pnc.class })
public class App { ... }

This would copy all attributes (mixinStandardHelpOptions, versionProvider, but also name, description, etc) to all subcommands (and sub-subcommands). The only attribute not copied would be subcommands, to prevent infinite recursion.

Subcommands can override attributes by specifying their own values in the @Command annotation for their command definition. They likely already do this for attributes like name and description, for example:

@Command(name = "da", description = "Dependency Analysis related commands", subcommands = { Da.Lookup.class })
public class Da { ... }

The advantage of doing this declaratively in the annotations, instead of programmatically via setInherit or similar methods, is that it can be captured and modeled in the picocli-codegen annotation processor. This makes is easier to generate things like man documentation and GraalVM configurations from a command class.

This is a nice extension of #649.

@remkop remkop added this to the 4.6 milestone Sep 8, 2020
@rnc
Copy link
Contributor Author

rnc commented Sep 8, 2020

@remkop That sounds really neat! It would reduce boiler-plate code and make even nicer to work with :-)

@remkop remkop modified the milestones: 4.5.2, 4.6 Oct 14, 2020
@remkop remkop changed the title mixinStandardHelpOptions versus CommandLine.ScopeType.INHERIT Support @Command(scope=INHERIT) (was: mixinStandardHelpOptions versus CommandLine.ScopeType.INHERIT) Oct 28, 2020
@remkop
Copy link
Owner

remkop commented Nov 23, 2020

I pushed a commit to master that adds support for this, with some tests.
Still TODO: documentation.

@remkop
Copy link
Owner

remkop commented Nov 23, 2020

Added documentation.
This will be included in the next release (picocli 4.6).

CC @garretwilson you may like this extension of #649.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants