-
Notifications
You must be signed in to change notification settings - Fork 424
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
Documentation for programmatic API uses deprecated AbstractParseResultHandler #885
Comments
That is a good point, thank you!
That said, the docs need updating. Are you interested in providing a pull request to update the programmatic API guide for picocli 4.0? |
@rbresalier There have been some updates. Can you check the latest version and provide feedback? Thanks! |
Thanks @remkop ! It is a bit hard to review it because I first went to https://github.com/remkop/picocli/wiki/Programmatic-API, but that is not updated yet. So I clicked on the 3 commits and tried to read that, it does look like you updated it. Is there a page with the final result similar to https://github.com/remkop/picocli/wiki/Programmatic-API that I could review? Some words about why and how I use the Programmatic API. In my applications, I have a common library that adds and handles a few options that are common to all of my applications that use the library. Then each application adds its own custom options. So part of the options are specified in a common library, and others are specified in the application that uses the library. I could not see how to accomplish this with the annotations API, it seems you can't have some options defined in one common source library and the rest of the options defined in custom applications. This is why I chose the programmatic API. I'm also coming from Python using the argparse library, where I could handle common options quite easily, similar to CommandSpec and parseResult=parseArgs(). So the way I do this is:
So using the whole "execute()" strategy wasn't really what I was after. I ended up preferring the parseArgs(). This is very consistent with how I do this in my python utilities, where I create an argparse object (similar to CommandSpec), then parse it (pr= argparse.parse_args()), and then deal with the pr in both my application and the common library. I don't like having to create an "execute" function that will get called to handle my logic, it makes it harder to access the data from my calling class and the calling library class. I much prefer the parseArgs way of doing things. So I would much rather the parseArgs method is shown in the first chapter of the documentation and not the execute(). The old handle() thing threw me off a little when I first started, and after I read the entire document and found parseArgs I saw that it was what I wanted. So I think that parseArgs in the main way folks who use the Programmatic API would want to do it, so should be mentioned first in the doc, and maybe later in the document discuss execute() as another way - I think it would be less popular. I was originally exploring the old handler system when I found it out was deprecated and figured I could be a good citizen and report this. But I ended up not even using it. A few other comments, and this is maybe going beyond the main topic of this issue, maybe I should create other issues?
|
I updated the wiki. It wasn’t maintained. It now points to the maintained document. I need some more time to look at your additional comments. |
@rbresalier Thank you for explaining your use case! The documentation for the programmatic API does not cover all of picocli’s features. Let me start with your questions:
About reusing common options: picocli offers mixins for this. Mixins make it possible to reuse options and positional parameters that are defined via the annotations API as well as the programmatic API. I understand your preference for the The Your argument that people using the programmatic API may be more interested in using picocli as a library (and use |
Thanks for this great information @remkop ! The reason I was going to the wiki for the programmatic API is that it is the first link when you google search "picocli programmatic API". Its very good that you now have just a link there to take one to the official documentation. I was obviously unaware of the mixin capability for the reusable options. It could be useful to make this more "discoverable". To make it more discoverable, I think it could be useful to add a one liner about creating reusable common options into the "Introduction" chapter of the main documentation and also the example in the quick guide so that a reader can become aware of it right away. The one liner could have a link to the more detailed documentation. I'm going to look into changing my application and my library to using the annotations with mixins as I think it is probably more readable. Thanks for pointing out about showing the enum choices and default values - that is very helpful information! My comments of the review for the updated programmatic API documentation:
I found it a little confusing. I see that ProgrammaticCommand::run() is called, not execute. Took me about 30 seconds to see that you were talking about the call to commandLine.execute(args) on the next line. I would change the comment to: ` // ProgrammaticCommand::run() will be called from inside commandLine.execute() When I saw the chapter on "Bindings" originally, I wasn't sure what it really meant. I think putting some examples of its usage could be helpful. A general comment about "subcommands", when I first encountered this in the main documentation and also the programmatic API, I didn't know what you meant by subcommands, so it was hard to understand. Took me a small amount of time to figure it out. Maybe show an example first what it means, like invoking
before going into how to use subcommands.
|
@rbresalier I updated the Programmatic API manual based on your feedback. It now mentions the more meaningful "Reusing Options with Mixins" instead of just "Mixins"; the subcommands section has some more explanation and gives the well-known Please take a look. |
We made significant improvements to the documentation for the programmatic API, so I am going to declare victory and close this ticket. Please feel free to create a new ticket if something comes up. |
The documentation for the programmatic API, https://picocli.info/picocli-3.0-programmatic-api.html, shows coding using AbstractParseResultHandler. However the apidocs state that AbstractParseResultHandler is obsolete: https://picocli.info/apidocs/picocli/CommandLine.AbstractParseResultHandler.html
So should the programmatic API docs be updated for the non-deprecated way (or remove AbstractParseResultHandler)? Or is AbstractParseResultHandler really deprecated and that should be removed from the apidocs?
The text was updated successfully, but these errors were encountered: