-
Notifications
You must be signed in to change notification settings - Fork 418
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
add support for @Command annotation on methods (in addition to classes) #416
Conversation
Quite interesting idea... Can it be used like |
Codecov Report
@@ Coverage Diff @@
## master #416 +/- ##
============================================
- Coverage 89.23% 88.35% -0.88%
- Complexity 264 278 +14
============================================
Files 4 4
Lines 3566 3702 +136
Branches 851 882 +31
============================================
+ Hits 3182 3271 +89
- Misses 188 220 +32
- Partials 196 211 +15
Continue to review full report at Codecov.
|
@kravemir : if it is possible using the |
@kravemir : with a bit of tweaking, this example works even without having to use @Command(name="app", subcommands={App.Interface.class})
class App {
@Command(name="interface")
static class Interface {
@Parameters(arity="0..1", description="network interface")
String iface;
@Command void list() { /* ... */ }
@Command void up() { /* use this.iface */ }
@Command void down() { /* use this.iface */ }
// ... Results in:
Is this the use case you were thinking about? |
@illes It's almost what I though about. But, I would like something more like:
Together with:
|
@kravemir : short answer: no, picocli does not offer that parsing logic, and my proposed changes do not affect parsing picocli goes something like:
as a workaround, you could e.g.:
|
I've started to take a look at this. I like the idea, and I'd like to evolve this so it can be merged. I noticed that users need to obtain a
The Still looking at the code and thinking about this... |
@remkop : Thanks for reviewing. I implemented the name change from One way to evolve would be to add a |
Thanks for updating! I’ll take another look. By the way, I found this library https://github.com/tomitribe/crest which has a similar style of defining commands with methods. Not intending to imitate, but it might be useful to see how they approached things or solved certain issues. Not sure if they support subcommands for example. Anyway, their docs may be interesting. |
Sorry for the wait! This has finally bubbled to the top of my todo list now. Good point about adding some way to make command methods subcommands of the command class. Like you suggested, one way is to have a A potential alternative way would be to simply always add command methods as subcommands of the command class that contains the method if the class is annotated with If the class is not annotated with What do you think? Can I ask a favour: would it be possible to rebase on master? (If that's too much hassle I can do a merge, but I like the cleaner history of a rebase.) |
…-command-methods-v2 # Conflicts: # src/test/java/picocli/CommandLineTest.java
I like the idea: |
Sorry for the late response: traveling with family and have intermittent access to internet and PC. Thanks for the changes! |
@illes I went ahead and merged the PR. Many thanks! There are a few changes I would like your opinion on:
I have some work in progress to fix the NPE for methods with |
Also, #22 will dovetail nicely into this PR: that will allow users to specify description text in a separate properties file (or other resource bundle) to keep the method parameter annotations brief and readable. |
…es from #416-command-methods-v2
…ir starting index from their method parameter position, but their max index from the type
# Conflicts: # src/main/java/picocli/CommandLine.java # src/test/java/picocli/CommandLineTest.java
allowing
@Command
annotation on methods cuts the boilerplate in situations where a class would have to be created just to allow integration withpicocli
.Consider a simple tail example:
versus
The attached commit is a minimal implementation and test cases for this feature.