-
Notifications
You must be signed in to change notification settings - Fork 88
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
WIP: Add machine readable console argument definition #394
base: master
Are you sure you want to change the base?
Conversation
6060fd8
to
8b7b9ad
Compare
I think that it would be really helpful for improving the console in this direction; in particular, command completion would be really helpful for many commands. I am mostly using the console via its integration in the openHAB ZigBee binding - I don't know yet if and how this could be integrated there as well; the console extension point of Eclipse SmartHome, AFAIK, does not support such nice features currently. When looking at how the arguments are modeled in the PR, it does not look unreasonable. However, I am wondering whether it is sensible to handcraft a model for command arguments in this library, or whether it would be more useful to use an existing library for CLI argument processing. The first that comes to my mind here is Picoli; I haven't used it myself so far, but heard praise at some places, and the description on the Github page sounds promising for what is needed here (modeling not only of named options but also of positional parameters as needed here; strongly typed parameters for many built-in types but with the option to define own types). Using such a library would avoid reinventing the wheel here. I haven't looked deeply at which library would be suited best here, and how this could be integrated, but wanted to bring this up here anyway :) Do you also think that it might be useful to investigate somewhat more in this direction? |
Hi Henning, Do you think picocli would fit in to the ESH framework? It's quite a different concept I think. Currently, this new system would continue to work within the ESH framework, just as it is now with little or no modification. Of course, you don't get autocomplete etc, but as you said, this probably isn't possible at the moment with the ESH console anyway. Picocli is also quite a large framework - about 10k lines of code in a single file, and I just wonder if it's not overkill given that my current implementation is a couple of classes, coming in at probably around 300 lines (this PR is 495 lines of changed code, and much of that is just the refactoring/renaming, not the implementation). Given that this goes into the "core" of any system needing the command line, I'm thinking it's excessive for what is needed. WDYT? Picocli also works a bit differently in the way the command lines are defined - ie it uses the more standard command line parameters rather than simple positional parameters. It looks like it is also able to use positions, but I don't think it can define a hierarchical tree as I've done here (although I might be wrong - I've not read all the docs). Of course, this isn't a major issue - we could choose to change the command structure as well if it was considered the best approach. So, I think between picocli and the custom implementation, my preference is to hand craft this at the moment - although I'm still happy to be convinced that it's better to go another route. |
Hi Chris, I also had a deeper look into picocli over the weekend - and I really like it and will probably use it from now on in many of the the small command line scripts I write in my day-to-day work :) That said, what is missing in picocli that would be needed here is (if I understood picocli correctly) the possibility to have code completion of one parameter be dependent on a previous parameter. (Example: A command with two parameters, the first being an endpoint, the second a cluster; then, e.g., completion should only provide clusters of the endpoint specified in the first parameter.) Maybe this is what you refer to with "hierarchical tree". Moreover, I agree that it might be quite hard to get this into ESH. I don't see a technical issue with integrating a console using picocli into the ESH console framework (like the integration of the console from this project into the binding); however, an integration of picocli into ESH giving benefits like strongly typed parameters and completion could be hard, also since picocli, from what I've read, cannot easily be used in a modular fashion. Regarding code size, I don't think that 10k lines of code is such a big deal, but this might be personal preference. I agree that many features are just not needed for the console here (like, e.g., Unix-shell-like command line parameters), and since it is not possible to include only the needed features, there will be a lot of unused code. My personal opinion is that this is not a big deal (but, as said above, personal preference...) So, with unlimited resources, it would be nice to include such a "hierarchical tree" feature into picocli and then use it for the console here, but I see your point in hand-crafting here at the moment. |
8b7b9ad
to
58beada
Compare
58beada
to
7630080
Compare
7630080
to
5238cdb
Compare
5238cdb
to
dfb2e0d
Compare
dfb2e0d
to
0302417
Compare
0302417
to
3ab2f8f
Compare
3ab2f8f
to
156e6f4
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
156e6f4
to
849c143
Compare
Signed-off-by: Chris Jackson <[email protected]>
849c143
to
b936006
Compare
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
3febbdb
to
3da9824
Compare
This PR creates a system to define console command arguments in a machine readable format. This allows the user to implement features such as command completion and context sensitive help.
As an example, a command may be defined as follows -:
A user could then provide a list of endpoints by knowing that the first argument requires an endpoint...
@hsudbrock I'd welcome your comment on this as I think you are currently the only other user of the console commands.
Signed-off-by: Chris Jackson [email protected]