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

Make COMMAND subcommand parameter required? #625

Closed
sebthom opened this issue Feb 7, 2019 · 15 comments
Closed

Make COMMAND subcommand parameter required? #625

sebthom opened this issue Feb 7, 2019 · 15 comments

Comments

@sebthom
Copy link

sebthom commented Feb 7, 2019

Currently when I have a main command with subcommands, the help is rendered like this:

Usage: mycommand [-hqV] [-v]... [COMMAND]

Where the brackets around [COMMAND] signal that the command parameter is optional.
I need it to be required, i.e. the call()/run() method of the main command actually is not used.

Right now I throw an exception inside the call()/run() method and advice the user to specify a command. However I would prefer this to be a declarative setting, also I would like to see the usage help signal the COMMAND parameter as required.

@sebthom
Copy link
Author

sebthom commented Feb 7, 2019

I am using annotations btw.

@remkop
Copy link
Owner

remkop commented Feb 8, 2019

A similar request was just raised in #624. I don't object to doing this, but we need to flush out the semantics and annotation syntax.

Are we looking for something like this?

@Command(subcommands = {A.class, B.class}, subcommandRequired = true)
class App {}

Any ideas for a better name?

@AlcaYezz
Copy link
Contributor

AlcaYezz commented Feb 8, 2019

In my case, this is what I search for.

This name explain it's meaning by itself so it works.

@sebthom
Copy link
Author

sebthom commented Feb 8, 2019

That sounds good to me too.

@remkop
Copy link
Owner

remkop commented Apr 18, 2019

A more generic solution is related to #454 (Repeatable Subcommands): a command could have a multiplicity attribute similar to ArgGroups.

This multiplicity indicates how many times the subcommand ca be specified. The default would be "0..1", meaning that by default, a subcommand is optional but can be specified at most once.

If a subcommand can be specified multiple times, it should be defined something like the below:

@Command (multiplicity ="0..*")
class MyCommand {}

@AlcaYezz
Copy link
Contributor

AlcaYezz commented Apr 19, 2019

Command multiplicity won't fit our need I think. We need a way to tell picocli that a command require a subcommand and we can't make it with multiplicity.

@Command(name="foo")
class Foo {

    @Command(name="bar", multiplicity="0..1")
    Class Bar {}

    @Command(name="baz", multiplicity="0..1")
    Class Baz {}
}

Here foo can have a subcommand or not at all.

@Command(name="foo")
class Foo {

    @Command(name="bar", multiplicity="1")
    Class Bar {}

    @Command(name="baz", multiplicity="1")
    Class Baz {}
}

And here foo should have both subcommands to work. But there is no way to express that foo shouldn't terminate the command and that we should found either bar or baz but no both.

@remkop
Copy link
Owner

remkop commented Apr 20, 2019

Yes, you’re right. Multiplicity is not the answer here.

@remkop
Copy link
Owner

remkop commented Jun 11, 2019

I started to work on this. To clarify the requirements:

Automatic Error Handling

If a command defined as subcommandRequired = true is invoked without a subcommand, picocli should throw a ParameterException with the message "Missing required subcommand". The default exception handler will print this message and the usage help to the standard error stream.

TBD
Any feedback on this error message, or in general on the proposed approach?

TBD
Should the command logic itself be invoked before throwing the above "Missing required subcommand" ParameterException? I'm thinking yes: that would allow applications to throw a custom exception or take some other action before picocli throws its exception.

Usage Help Synopsis

Both the abbreviated synopsis and the automatic synopsis of a command with subcommands currently show the subcommands as [COMMAND]. The proposal is to replace this with COMMAND (without the [ and ] brackets signalling optionality) if subcommandRequired = true. This is one of the main drivers behind this ticket, since this part of the synopsis is not easily customizable.

TBD
The drawback is that COMMAND still a hard-coded string in the synopsis. It would be nice if apps were able to customize this, and specify for example [list | add | delete] for optional subcommands, and (list | add | delete) for required subcommands. This would likely require an additional annotation attribute:

@Command(name = "fs", subcommandRequired = true, synopsisSubcommandLabel = "(list | add | delete)")

and

@Command(name = "fs", subcommandRequired = false, synopsisSubcommandLabel = "[list | add | delete]")

TBD2
I imagine that it would be desirable to introduce a variable so that applications don't need to hard-code the command names:

@Command(name = "fs", subcommandRequired = false, synopsisSubcommandLabel = "[${SUBCOMMANDS}]")

There is still some hard-coding in the above, since there is no way to specify the separator (| in the above example). For each subcommand, the command name shown would be the command's name, not one of its aliases.


Feedback welcome!

@remkop
Copy link
Owner

remkop commented Jun 11, 2019

Analysis, revisited

The above analysis makes me think that the idea of adding a synopsisSubcommandLabel = "..." attribute (potentially with a ${SUBCOMMANDS} variable) is the most valuable part of this proposal.

I'm beginning to think we don't really need a subcommandRequired = true attribute...

The only value it adds is the error handling, which is fairly trivial to implement anyway:

@Command(name = "top", subcommands = Sub.class, synopsisSubcommandLabel = "COMMAND")
static class TopCommandWithRequiredSubcommand implements Runnable {
    @Spec CommandSpec spec;
    public void run() {
        throw new ParameterException(spec.commandLine(), "Missing required subcommand");
    }
}

Thoughts?

remkop added a commit that referenced this issue Jun 11, 2019
…ation of the subcommands part of the synopsis: by default this is `[COMMAND]`.
@remkop remkop closed this as completed in 7f97c3e Jun 12, 2019
@remkop
Copy link
Owner

remkop commented Jun 12, 2019

@sebthom @AlcaYezz I went ahead and implemented the synopsisSubcommandLabel idea.

Can you review and give feedback?
Relevant updates to the user manual are Synopsis Subcommand Label and Required Subcommands.

@remkop
Copy link
Owner

remkop commented Jun 13, 2019

I like this solution better than the original subcommandRequired = true idea:
It seems more in line with the idea to “make common things easy and make uncommon things possible”.

@remkop remkop modified the milestones: 4.0, 4.0-beta-2 Jun 18, 2019
@remkop
Copy link
Owner

remkop commented Jun 19, 2019

FYI, 4.0.0-beta-2 has been released with support for synopsisSubcommandLabel. It may take a few hours for Maven mirrors to update.
Thanks again for raising this!

@r2mzes
Copy link

r2mzes commented Sep 29, 2020

Hi, I have a question regarding this required subcommand. I read your comments but I am afraid I still do not fully understand how to use it :P

Let' say I have one main command defined like this:
@CommandLine.Command(name = "main", subcommands = { LoadCommand.class, ValidateCommand.class, RenderCommand.class }, subcommandsRepeatable = true) public class MainCommand

How can I make LoadCommand subcommand required and other subcommands optional?
I use the basic parsing approach with ParseResult.

I would appreciate your help :)

@remkop
Copy link
Owner

remkop commented Sep 29, 2020

@r2mzes Would you mind raising a separate ticket for this? Sometimes questions result in changes to picocli and then it’s good to have the full history in a single ticket.

@r2mzes
Copy link

r2mzes commented Sep 30, 2020

Actually this was exactly the reason why I posted here :D but sure, no problem :P

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

4 participants