-
Notifications
You must be signed in to change notification settings - Fork 18
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 subcommand flags #87
Conversation
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
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. |
Signed-off-by: Mabel Zhang <[email protected]>
1efb619 adds installation, which is also added to the Scripts are installed to the directory
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? 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 |
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
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 #87 (comment) that worked for gui and plugin. |
Signed-off-by: Mabel Zhang <[email protected]>
Signed-off-by: Mabel Zhang <[email protected]>
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.
Just one last thing
All PRs have now been opened for second-level flags. |
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 |
🎉 New feature
Part of #1
Test with any or all of these subcommands (
ign --commands
shows these 11):plugin
Bash completion for flags gz-plugin#81gui
Bash completion for flags gz-gui#392topic
service
Bash completion for flags gz-transport#312log
is a third level subcommandand not yet implemented, see Bash completion for flags gz-transport#312 (comment). Adds the third level commands for the log gz-transport#451gazebo
model
Bash completion for flags gz-sim#1504msg
Bash completion for flags gz-msgs#254launch
Bash completion for flags gz-launch#167fuel
also has three levels, and the third level subcommand might have to come latersdf
Bash completion for flags sdformat#1042Summary
Demonstration of tab completion for
ign-gui
(libraries, Ruby parsing) andign-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
andplugin
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:
Then you should get tab completion when you type
ign <subcommand> -<tab>
. See those PRs for sample outputs.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.