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 @Spec annotation to inject CommandSpec into command implementation #259

Closed
remkop opened this issue Jan 11, 2018 · 12 comments
Closed

Comments

@remkop
Copy link
Owner

remkop commented Jan 11, 2018

See for example the auto-help use case: a command help subcommand command should print help for the specified subcommand of the parent command. This means it needs to obtain a subcommand CommandLine or CommandSpec object from the parent command. The @ParentCommand annotation does not work because it injects the user-defined command object, not the picocli class.

Another use cases is custom commands that need to know exactly which options were actually specified on the command line. This could be accomplished by injecting the ParseResult (#257).

A generic @Inject annotation is generic and flexible enough to allow future extensions, while at the same time the meaning is intuitively clear to many users because of its similarity to JSR-330 (javax.inject.Inject).

@remkop remkop added this to the 3.0 milestone Jan 11, 2018
@remkop remkop changed the title Inject @CommandSpec or @ParentCommandSpec into command implementation Support @Inject annotation to inject CommandSpec or CommandLine into command implementation Feb 4, 2018
@remkop remkop closed this as completed in a781f80 Mar 2, 2018
remkop added a commit that referenced this issue Mar 2, 2018
@kakawait
Copy link
Contributor

kakawait commented Mar 3, 2018

@remkop Nice addition. First at all, I didn't try why I will speak about, is just theoretical analysis.

But currently on picocli-spring-boot-starter commands use to be a Spring component/bean. And Spring in addition to @Autowired annotation is also aware of @Inject annotation for DI.

But CommandSpec, CommandLine and ParseResult will not be registered on Spring context (no sense to be registered), so I'm afraid that Spring will complain about:

No bean of type ... found

What do you think about?

How we can solve that? I think with alternative and more old school way to inject that component for example by implementing a picocli interface which defined 3 setters (or whatever) that will be used by picocli to inject that components. A bit like what I did on picocli-spring-boot-starter

https://github.com/kakawait/picocli-spring-boot-starter/blob/master/picocli-spring-boot-autoconfigure/src/main/java/com/kakawait/spring/boot/picocli/autoconfigure/PicocliCommand.java#L53

https://github.com/kakawait/picocli-spring-boot-starter/blob/master/picocli-spring-boot-autoconfigure/src/main/java/com/kakawait/spring/boot/picocli/autoconfigure/PicocliCommandLineRunner.java#L59

@remkop
Copy link
Owner Author

remkop commented Mar 3, 2018

I think Spring is looking for the javax.inject.Inject annotation, while this is a custom picocli.CommandLine.Inject annotation. I don't think Spring will see the picocli annotation.

If a class uses both the picocli and the javax @Inject annotation, then at least one of them needs to be annotated with the fully qualified class name.

@kakawait
Copy link
Contributor

kakawait commented Mar 3, 2018

Lol my bad I read to quickly I though it was javax.inject.Inject. Sorry

@remkop
Copy link
Owner Author

remkop commented Mar 3, 2018

No worries. Thanks for checking!

@kakawait
Copy link
Contributor

kakawait commented Mar 3, 2018

Are you not afraid that people will have confusion between both annotations, and some of them will still import javax.inject.Inject instead of picocli.CommandLine.Inject

@remkop
Copy link
Owner Author

remkop commented Mar 3, 2018

Yes there is some risk there. The benefits are no external dependency and a name that is intuitively obvious in what it does. So there’s a trade-off. I’ll add a note in the documentation. Thanks for pointing this out.

@remkop remkop reopened this Mar 4, 2018
@remkop
Copy link
Owner Author

remkop commented Mar 4, 2018

It turns out that the custom HelpCommand use case also required that the Help.Ansi object and the output PrintStream are injected... It’s becoming a bit too “magical”...

Reconsidering defining a IHelpCommand interface instead.

@remkop
Copy link
Owner Author

remkop commented Mar 4, 2018

I introduced interface picocli.CommandLine.IHelpCommandInitializable to implement the custom help command use case.

I reverted commits 2cb8d66 and a781f80 to remove the @Inject functionality for now, until another use cases comes up.

@remkop remkop removed this from the 3.0 milestone Mar 4, 2018
remkop added a commit that referenced this issue Mar 4, 2018
…application field."

This reverts commit 2cb8d66
remkop added a commit that referenced this issue Mar 4, 2018
…ommandLine` into application field."

This reverts commit a781f80
@fcorneli
Copy link

I'm also looking for a way to "help subcommand" properly.

@remkop
Copy link
Owner Author

remkop commented Mar 12, 2018

@fcorneli Can you try with current master?

@remkop
Copy link
Owner Author

remkop commented Mar 12, 2018

Picocli 3.0 (in master) has a built-in help command that can give help on a specified subcommand or on the preceding command.

See https://github.com/remkop/picocli/wiki/Manual-(3.0.0-SNAPSHOT)#built-in-help-subcommand

@remkop remkop changed the title Support @Inject annotation to inject CommandSpec or CommandLine into command implementation Support @Inject annotation to inject CommandSpec into command implementation Jun 19, 2018
@remkop
Copy link
Owner Author

remkop commented Jun 19, 2018

FYI: I ran into a use case that requires access to the picocli command model from the application, so this will be available from picocli 3.2.

The use case has to do with JLine completion candidates (#393, also in v3.2).
Some completion candidates need to be looked up dynamically from some resource that is only available at runtime. Access to the CommandSpec is needed to update the option's ArgSpec.completionCandidates after the lookup.

remkop added a commit that referenced this issue Jun 20, 2018
remkop added a commit that referenced this issue Jun 30, 2018
…confusion with `javax.inject.Inject`
remkop added a commit that referenced this issue Jun 30, 2018
remkop added a commit that referenced this issue Jul 1, 2018
…o use `@PicoInject` for a limited form of Dependency Injection later
@remkop remkop changed the title Support @Inject annotation to inject CommandSpec into command implementation Support @Spec annotation to inject CommandSpec into command implementation Jul 1, 2018
remkop added a commit that referenced this issue Jul 1, 2018
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

3 participants