gazebosim / gz-tools

Command line tools for the Gazebo libraries.
https://gazebosim.org
Apache License 2.0
14 stars 18 forks source link

Bash completion for subcommand flags #87

Closed mabelzhang closed 2 years ago

mabelzhang commented 2 years ago

🎉 New feature

Part of https://github.com/ignitionrobotics/ign-tools/issues/1 Test with any or all of these subcommands (ign --commands shows these 11):

Summary

Demonstration of tab completion for ign-gui (libraries, Ruby parsing) and ign-plugin (executables, CLI11 parsing). When we are happy with these two, it is easy to extend to other ign-* libraries.

Doing tab completion this way means it's purely at the bash interaction level, independent of how the actual flags are parsed and executed. Regardless of whether the flag implementation is Ruby or CLI11, the tab completion happens before any of those are incurred. gui and plugin are chosen so that we validate that here.

I took the liberty of adding comments to the existing code, because it took me a long time to understand even with online tutorials, this being my first time writing a bash completion script. Thought the comments might help the next person.

This set of PRs is for functionality, and the bash scripts are manually sourced for now. The scripts will need to be installed for tab completion to happen out of the box, which is more about infrastructure and will be in a separate PR.

Test it

1efb619 installs a single script to source everything:

. install/share/gz/gz.completion

Then you should get tab completion when you type ign <subcommand> -<tab>. See those PRs for sample outputs.

Checklist

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.

mabelzhang commented 2 years ago

Do not merge this PR until we have a PR to install all the scripts and source them at once. Otherwise, this script will be calling functions that don't exist. This script is already being installed, so the new changes will probably break the original tab completion.

The other two PRs can be merged, as those scripts are new and not being used anywhere else.

mabelzhang commented 2 years ago

1efb619 adds installation, which is also added to the ign-gui and ign-plugin PRs. Let me know if it's better to break off installation into separate PRs (it would be another set of 3 PRs, 1 in each repo).

Scripts are installed to the directory install/share/ignition/ignition.completion.d/, which are sourced by the script installed at install/share/ignition/ignition.completion. The user needs to source it manually:

. install/share/ignition/ignition.completion

I don't know if the way I've written this script is the most secure way to source bash scripts. What if somebody adds a random script matching the name pattern to that directory? I think it probably needs to be versioned too, like the Ruby scripts? If one version of a library adds a new flag, then the bash script will need to have that new flag just for that version. If that turns out to be complicated, maybe it's better to break off installation into separate PRs to figure those things out.

Either way, this is probably still not compatible with https://github.com/ignition-release/ign-tools-release/pull/4/files . I think bash completion requires a single file. We probably still have to pipe all the repos' individual scripts to a single script before installing to /usr/share/bash-completion/completions/ign.

mabelzhang commented 2 years ago

To push this along before I'm gone next week, I've started opening PRs in more repos, following the installation method in this comment https://github.com/gazebosim/gz-tools/pull/87#discussion_r877692827 that worked for gui and plugin.

mabelzhang commented 2 years ago

All PRs have now been opened for second-level flags. All ready for another pass. The main change is that the flags are no longer hard coded in the tests.

scpeters commented 2 years ago

I noticed the nightly builds are unstable since this was merged forward since the newly installed completion files are not handled by the current debian metadata

we should update the release repos for tools1 and tools2