-
Notifications
You must be signed in to change notification settings - Fork 43
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
Bash completion for flags #312
Conversation
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Codecov Report
@@ Coverage Diff @@
## ign-transport8 #312 +/- ##
=================================================
Coverage ? 83.81%
=================================================
Files ? 51
Lines ? 5035
Branches ? 0
=================================================
Hits ? 4220
Misses ? 815
Partials ? 0 Continue to review full report at Codecov.
|
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Just realized there's also the 😅 That logic would go where I say in the comments
In the interest of time, I think I will try to cover the two-level flags for as many libraries as possible, for commands like When we have more time, we can come back to the three-level flags, like |
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
b88b0d0
to
148d144
Compare
Signed-off-by: Louise Poubel <louise@openrobotics.org>
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.
Pushed some changes on b6c5430:
- Apply Detect ign instead of using cmake module to check for ignition-tools #249 to this branch, so it's on track with other libs
- Make sure we install CLI files even if
ign
isn't present - Fix tests accordingly
🎉 New feature
Part of gazebosim/gz-tools#1
Used together with gazebosim/gz-tools#87
Summary
Bash completion, installation, tests.
Small corner cutting for easier forward-porting:ign service
has--help-all
in Fortress+ign topic
has--json-output
in Fortress+This PR targets Citadel, but I've removed those so that the tests don't fail when this is forward-ported to Fortress 😅 We could add a PR to add these back to Citadel - they'd need to be added to both the bash script and the tests. They don't seem like critical flags to be in tab completion, and if a user types--help
, they can still see them.Alternatively, I can add the Citadel-specific flags in this PR, and add a comment to remove them in Fortress?
Update: For correctness, I've gone ahead and do that in 7ccc7a6.
Test it
This works with the installation method in the gz-tools PR described in this comment gazebosim/gz-tools#87 (comment) .
Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-by
messages.