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

Added a cli infrastructure #7094

Merged
merged 1 commit into from Aug 2, 2014
Merged

Added a cli infrastructure #7094

merged 1 commit into from Aug 2, 2014

Conversation

uboness
Copy link
Contributor

@uboness uboness commented Jul 30, 2014

CliTool is a base class for command-line interface tools (such as the plugin manager and potentially others). It supports the following:

  • single or multi command tool
  • help printing infrastructure (based on help files)
  • consistent mechanism of parsing arguments (based on commons-cli lib)
  • separation of argument parsing and command execution (for easier unit testing)
  • terminal abstraction (will use System.console() when available)

String cmdName = args[0];
cmd = config.cmd(cmdName);
if (cmd == null) {
terminal.printError("unknown command [%s]. Use [-h] option to list available commands", cmdName);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible use of config.printUsage here as well or directly list available commands to be more user friendly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought about it... decided to only leave it with a comment about -h... could very well be that the user just accidentally mistyped and didn't want to make things too verbose

@spinscale
Copy link
Contributor

Left a few comments, mostly about the tests.

I like the idea a in general, a bit worried about the performance (didnt test anything, maybe I am plain wrong here), but I'd like to see the PluginManager adapted as part of this PR to make sure everything works as we expect and all features are covered instead of finding out later, that we cannot do everything needed with this.

*
* Two modes are supported:
*
* - Singe command mode. The tool exposes a single command that can potentially accept arguments (eg. CLI options).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Singe/Single

@uboness
Copy link
Contributor Author

uboness commented Jul 31, 2014

@spinscale thanks for the review... I'd like have the plugin manager implemented in a separate PR - its own PR as part of the work there is to change how it works (right now, aside from being a mess, it's also not consistent with its supported options).

If we find things missing in the infra, we can always add those later.

@uboness
Copy link
Contributor Author

uboness commented Aug 2, 2014

updated based on comments by @spinscale

  • change help file resolution... command help file should now be named:
<tool_name>-<cmd_name>.help

CliTool is a base class for command-line interface tools (such as the plugin manager and potentially others). It supports the following:
  - single or multi command tool
  - help printing infrastructure (based on help files)
  - consistent mechanism of parsing arguments (based on commons-cli lib)
  - separation of argument parsing and command execution (for easier unit testing)
  - terminal abstraction (will use System.console() when available)
@uboness uboness merged commit 5ccc7be into elastic:master Aug 2, 2014
@uboness uboness removed the review label Aug 2, 2014
@clintongormley clintongormley changed the title Added a cli infrastructure Internal: Added a cli infrastructure Sep 8, 2014
@clintongormley clintongormley changed the title Internal: Added a cli infrastructure Added a cli infrastructure Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants