-
Notifications
You must be signed in to change notification settings - Fork 30
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
#396 Display the tools help output on 'ide help <tool>' #434
Conversation
Pull Request Test Coverage Report for Build 9743644633Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9743646050Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9759176562Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your additions. Looks good to me. Can be merged as soon as my small CR's are addressed.
cli/src/main/java/com/devonfw/tools/ide/commandlet/HelpCommandlet.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/tool/ToolCommandlet.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/tool/ToolCommandlet.java
Outdated
Show resolved
Hide resolved
Pull Request Test Coverage Report for Build 9778567475Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
If possible, please add a simple test to increase the test coverage here too. Just a check for a tool starting its simulated help command if it was found should suffice I guess. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slskiba thanks for your PR. Well done - your solution looks good to me. 👍
I left a review comment for design improvement. Please have a look. Then we can merge.
cli/src/main/java/com/devonfw/tools/ide/commandlet/HelpCommandlet.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slskiba thanks. Ready for merge now 👍
Pull Request Test Coverage Report for Build 9808878877Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Resolves #396.
While initially #396 sounded like a great idea, it might be troublesome in practice. Taking for example aws,
aws help
prints an incredibly long list of services (380+) to use with the aws cli, cluttering the output immensly.I therefore suggest the following solution to this issue:
printToolHelp
inToolCommandlet
runs the tool with the provided help command as input. The help command is by default set to null inToolCommandlet
, so if a tool does not specify otherwise, no tool-specific help is provided.<tool> help
output can overrideToolCommandlet
'sprintToolHelp
(e.g. AWS CLI, instead of printing 400+ lines of usage help, we can advise the user to useaws help
for detailed usage help)Example output for
ide help aws
:Example output for
ide help gh
: