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

WIP: Add machine readable console argument definition #394

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cdjackson
Copy link
Member

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 -:

        ZigBeeConsoleArgument option = ZigBeeConsoleArgumentBuilder.create(ZigBeeConsoleArgumentType.ENDPOINT)
                .withName("SOURCE-ENDPOINT").withDescription("The source endpoint to bind to").build();
        option.chain(ZigBeeConsoleArgumentBuilder.create(ZigBeeConsoleArgumentType.CLUSTER)
                .withDescription("The cluster to bind to").build());
        option.chain(ZigBeeConsoleArgumentBuilder.create(ZigBeeConsoleArgumentType.ENDPOINT)
                .withName("DESTINATION-ENDPOINT")
                .withDescription("The destination endpoint to bind to (defaults to local node)").isOptional().build());

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 chris@cd-jackson.com

@hsudbrock
Copy link
Contributor

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?

@cdjackson
Copy link
Member Author

Hi Henning,
Thanks - I'd not seen picocli, although I had done a search for such CLI processing frameworks so thanks for pointing it out. However, while I do like the idea of another library for this, I would also like to keep dependencies to a minimum, and avoid bloat.

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.

@hsudbrock
Copy link
Contributor

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.

@stale
Copy link

stale bot commented Oct 6, 2019

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.

@stale stale bot added the wontfix label Oct 6, 2019
@stale stale bot removed the wontfix label Oct 6, 2019
@zsmartsystems zsmartsystems deleted a comment Oct 6, 2019
Signed-off-by: Chris Jackson <chris@cd-jackson.com>
@stale
Copy link

stale bot commented Jan 13, 2020

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.

@stale stale bot added the wontfix label Jan 13, 2020
@cdjackson cdjackson added the pinned Will not be closed, even if stale label Jan 13, 2020
@stale stale bot removed the wontfix label Jan 13, 2020
@cdjackson cdjackson force-pushed the master branch 3 times, most recently from 3febbdb to 3da9824 Compare April 27, 2021 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pinned Will not be closed, even if stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants