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

CommandLine.usage on subcommand does not properly display command and subcommand names #563

Closed
Blatwurst opened this issue Dec 1, 2018 · 18 comments
Milestone

Comments

@Blatwurst
Copy link

Blatwurst commented Dec 1, 2018

The Picocli documentation says this:

The usage help message for each subcommand is produced by calling CommandLine.usage(new SubCommand(), out).

But given that I have a CmdRotate subcommand object, when I make this call:

CommandLine.usage(CmdRotate(), System.out)

or this, which makes good sense since I'm doing this from inside the CmdRotate object:

CommandLine.usage(this, System.out)

I get this:

Usage: <main class> [-jsv] spec...
Perform a key rotation operation
spec... one or more account id specs: #, #-#, #:#, 'all' or 'partners'
-j, --json Output JSON.
-s, --show Show effects but don't change anything
-v, --verbose Be verbose.

Note that instead of "dbkeyrotator rotate", I get "<main class>" for the command and subcommand names. If I force an invalid invocation of my subcommand so that this help is automatically printed, or I use the "help" subcommand, it prints out correctly:

Missing required parameter: spec
Usage: dbkeyrotator rotate [-jsv] spec...
Perform a key rotation operation
spec... one or more account id specs: #, #-#, #:#, 'all' or 'partners'
-j, --json Output JSON.
-s, --show Show effects but don't change anything
-v, --verbose Be verbose.

I'll point out that I'm making this call in Kotlin rather than Java, but I don't see how that should make any difference. Is this a bug? Have I missed something? If not, how can I explicitly cause the help for a subcommand to print correctly?

I'm thinking of digging into the code myself (wish the source was available via Maven), but thought I should report this anyway.

@Blatwurst
Copy link
Author

Blatwurst commented Dec 1, 2018

btw, I'm using version 3.8.0, and I'm adding my subcommands programmatically since Kotlin doesn't seem to play nice with putting Class objects in an array in an annotation parameter:

val commandLine = CommandLine(DbKeyRotator())
.addSubcommand("rotate", CmdRotate())
.addSubcommand("keys", CmdKeys())
.addSubcommand("help", HelpCommand())
commandLine.parseWithHandler(RunLast(), args)

@remkop
Copy link
Owner

remkop commented Dec 1, 2018

I suspect that the subcommands do not have a @Command(name = "MYNAME") annotation.

When the subcommands are added programmatically, and the subcommand does not have a name explicitly set via the annotation, then picocli sets the subcommand name, but it sounds like there is a bug in that logic.

I will try to reproduce the issue now.

By the way, what is the problem with registering subcommands by specifying Class objects in an array in the annotations?

@remkop remkop added this to the 3.8.1 milestone Dec 1, 2018
@remkop
Copy link
Owner

remkop commented Dec 1, 2018

I am unable to reproduce the problem.

Here is the test I created, but it passes:

@Rule
public final SystemOutRule systemOutRule = new SystemOutRule().enableLog().muteForSuccessfulTests();

@Command(mixinStandardHelpOptions = true) class Top563 {}
@Command(mixinStandardHelpOptions = true) class Sub563 {}

// test for https://github.com/remkop/picocli/issues/563
@Test
public void testSubcommandWithoutAnnotationName() {
    CommandLine top = new CommandLine(new Top563());
    top.addSubcommand("subname", new Sub563());

    CommandLine sub = top.getSubcommands().get("subname");
    assertEquals("subname", sub.getCommandName());
    assertEquals("subname", sub.getCommandSpec().name());
    assertEquals("<main class> subname", sub.getCommandSpec().qualifiedName());

    String expected = String.format("" +
            "Usage: <main class> subname [-hV]%n" +
            "  -h, --help      Show this help message and exit.%n" +
            "  -V, --version   Print version information and exit.%n");
    sub.usage(System.out);
    assertEquals(expected, systemOutRule.getLog());
}

I also pushed this test to master, it is the last test in SubcommandTests.

@Blatwurst
Copy link
Author

First of all, Thanks Remko for an AMAZING bit of code!!! Picocli rocks!

I got why this is broken, and looking at the code, I found a fix! Calling a static function means that the constructed CommandLine object isn't available, so the necessary context is not available. So it seemed that a solution must involve my CommandLine object. I now stash a reference to that in the parent command. I was already injecting the parent into the subcommands, so my subcommands now have access to the CommandLine through the parent. I have a common super class for all of my subcommands that contains the injected parent. So I just added this method to my subcommand super class:

fun usage(out: PrintStream) {
        for (subcommand in parent.commandLine.getSubcommands()) {
            if (subcommand.value.commandSpec.userObject() == this) {
                subcommand.value.usage(out)
                break
            }
        }
    }

This works!

A complete solution would add overloads for the other forms of usage(), but I only need this one so far. I'm not sure if I need to worry about this ever not matching a subcommand...don't think so.

@remkop
Copy link
Owner

remkop commented Dec 2, 2018

Re-reading the problem description, I may know what the problem is.

When you call CommandLine.usage(this, System.out), picocli constructs a new CommandSpec model object for the CmdRotate object. This new model is not part of the command hierarchy that was used to parse the input. It will not have the name set (because it is not associated with a parent command).

You should be able to resolve this by giving each subcommand a @Command(name = "MYNAME") annotation.

A better alternative is to use the @Spec annotation to get a reference to the model object inside your business logic. This will allow your subcommand to show the full command hierarchy in its usage help message. That looks something like this in Java:

class CmdRotate implements Runnable {
   @Spec CommandSpec spec;

    public void run() {
        new CommandLine(spec).usage(System.out);
    }
}

@Blatwurst
Copy link
Author

Hi Remko,

Your test is not using the static CommandLine.usage() method. That method is what's broken. If you have the subcommand's CommandLine, printing the help works great. The problem is that what your test code does is a different animal completely than calling the static CommandLine.usage() method. I wanted exactly that functionality call. I don't want my code to have to understand about the CommandLine and such. That's what CommandLine.usage() is supposed to provide, right? My code basically provides what I think CommandLine.usage() is meant to do.

The problem with the Class objects in the annotations is the fact that I'm using the Kotlin language rather than Java. I couldn't figure out how to make it work...the Kotlin compiler kept giving me errors about non-const crapola. I just punted. There may very well be a way to do it in Kotlin.

@Blatwurst
Copy link
Author

Blatwurst commented Dec 2, 2018

Aha! Ok, I'll consider your solution. I still think it would be great if CommandLine.usage() did the right thing. If not, maybe you should remove it. I couldn't make it do the right thing even for the parent command. In that case, it doesn't print any information about the subcommands.

I haven't tried it, but I'm guessing that using @command(name = "MYNAME") isn't going to provide a full solution, because it still won't know the name of the parent command, or even of its existence.

Thanks for all of your comments above!

@remkop
Copy link
Owner

remkop commented Dec 2, 2018

The best solution for you is to inject the @Spec into the subcommands. This has the full hierarchy. (See my previous comment.)

You are right that the static method only has information about a single command without access to the subcommands if the parent cannot have the @Command(subcommands = ...) annotation. I should add some javadoc to that effect.

I need to take another look at Kotlin regarding defining subcommands in the annotations.

@Blatwurst
Copy link
Author

Confirmed. This definition of my usage() method works great (again, just once in my superclass):

@Spec
var spec: CommandSpec? = null

fun usage(out: PrintStream) {
    CommandLine(spec).usage(out)
}

With all the work you've already done, I'll try to contribute by taking a deeper look at the Kotlin issue. It's the least I can do.

@remkop
Copy link
Owner

remkop commented Dec 2, 2018

That would be great! I’m by no means a Kotlin expert.

@remkop
Copy link
Owner

remkop commented Dec 2, 2018

By the way, in the start of the problem description you mention the picocli documentation. Can you point me to the section you’re referring to?
I’m wondering if there’s something that needs to be improved in the documentation.

@remkop
Copy link
Owner

remkop commented Dec 2, 2018

I checked but sources.jar should be available on Maven Central: https://search.maven.org/classic/#search%7Cga%7C1%7Ca%3A%22picocli%22

remkop added a commit that referenced this issue Dec 2, 2018
@Blatwurst
Copy link
Author

The line I quote is near the bottom of section 13.7.

My IDE can usually pull the sources for a package. I don't know why it's not finding the sources, as they're clearly there. Sorry for assuming that my IDE was right and they weren't.

@remkop
Copy link
Owner

remkop commented Dec 2, 2018

I fixed the documentation in the just-released version 3.8.1. Thanks for raising this!

remkop added a commit that referenced this issue Dec 2, 2018
@remkop
Copy link
Owner

remkop commented Dec 2, 2018

I found out it is possible to specify classes in Kotlin annotations with the following notation:

@Command(subcommands = [SubCmd::class])

I added a Kotlin example to the picocli-examples module: https://github.com/remkop/picocli/tree/master/picocli-examples/src/main/kotlin/picocli/examples/kotlin/subcommands

If you feel like providing a PR for the user manual that demonstrates subcommands that would be great!

@remkop
Copy link
Owner

remkop commented Dec 4, 2018

@Blatwurst, did you get a chance to try the notation for subclasses above?

@remkop
Copy link
Owner

remkop commented Dec 4, 2018

Since we improved the documentation in 3.8.1 I’m closing this ticket. Feel free to reopen or raise another one if we need to follow up on anything.

@remkop remkop closed this as completed Dec 4, 2018
@remkop
Copy link
Owner

remkop commented Dec 4, 2018

Updated the Kotlin section of the user manual for 3.8.2 to mention the subcommand notation. Releasing 3.8.2 now.

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

No branches or pull requests

2 participants