-
Notifications
You must be signed in to change notification settings - Fork 160
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
add ros2 param describe #367
Conversation
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
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.
LGTM once the flake8 blank line is clear.
Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
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.
LGTM. Since we can know the list of parameters, I think it would make for a nice feature if no parameter names are provided, then we describe all of them.
We could query the list with an additional service call. Unfortunately the |
But I guess the tool knows about them from an additional service call since auto-completion is working? |
Not sure I understand but currently we don't call the list service. While it is being invoked during completion it is not run during the actual command. As I said: it can certainly be added (with the extra call time). Feel free to do so in a new PR. |
Fixes #362.
I chose to use the parameter name as the item without indentation and all other keys are then indented one level. That works nicely for multiple parameters and doesn't use extra empty lines. See example output: