-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: add flag and label completion for bazel commands #373
Conversation
a7e6604
to
3723cbd
Compare
Thanks for the PR! I tried it and out read over the code and its a great start to getting this feature in. Performance is my main concern with this. I measured ~2.5 seconds on my machine when I hit tab on this one before zsh showed the completion prompt:
and the number of options returned is too large to be helpful 🙄 Another test with only a single valid completion,
=>
also took ~2.5s. We've been floating the idea around of vendoring in the bazel flags for each Bazel version so that we don't have to to ask Bazel itself what the flags are every time aspect runs. That should reduce the time to getting completion results.... although I haven't measured what ate up the ~2.5 seconds above. It might partially be a function of how many flags bazel has. Also regarding performance, the behind the scenes aspect runs when you hit Target completion comes with a bit overhead as well for reading and parsing files from disk. Perhaps some caching could be done here to speed things up. Bazel docs for zsh completion mention caching as an option:
To start I think we should measure why completion performance is poor and see what can be done to improve it. I think we should also separate flag completion & target completion into separate PRs with flags coming first. Making target completion fast seems like a harder problem that can be punted to a follow-up once we have flags working fast. |
That is orders of magnitudes slower than what I observe in bash:
Is this maybe an issue with zsh?
While this is true, I think this is the least bad option. Not reporting flags is worse IMO.
Do you want to vendor this in the repository? If so, any preferred location where to store it?
I don't really observe what you are seeing locally. Target completion is pretty fast for me.
|
Oh interesting that bash is that much faster zsh. I will check again with bash and report back. |
@gregmagolan any news here? I really want to start using aspect CLI for its plugin system. But without completion, I will have a hard time introducing it at my day job. |
b951e36
to
967eec4
Compare
pkg/bazel/bazel_flags.go
Outdated
// List of all commands with label as inputs. To compile the list, you can | ||
// use the following command: | ||
// | ||
// bazel help completion | grep 'ARGUMENT="label"' |
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 like there are label-test
and label-bin
variants so the correct grep is | grep 'ARGUMENT="label'
(without the 2nd double quote) to catch the test
, coverage
and run
commands.
I'll push a commit to update the list
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.
$ bazel help completion | grep 'ARGUMENT="label'
BAZEL_COMMAND_AQUERY_ARGUMENT="label"
BAZEL_COMMAND_BUILD_ARGUMENT="label"
BAZEL_COMMAND_COVERAGE_ARGUMENT="label-test"
BAZEL_COMMAND_CQUERY_ARGUMENT="label"
BAZEL_COMMAND_FETCH_ARGUMENT="label"
BAZEL_COMMAND_MOBILE_INSTALL_ARGUMENT="label"
BAZEL_COMMAND_PRINT_ACTION_ARGUMENT="label"
BAZEL_COMMAND_QUERY_ARGUMENT="label"
BAZEL_COMMAND_RUN_ARGUMENT="label-bin"
BAZEL_COMMAND_TEST_ARGUMENT="label-test"
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.
Done.
} | ||
var flags []string | ||
bazelFlags.VisitAll(func(f *pflag.Flag) { | ||
flags = append(flags, "--"+f.Name) |
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.
hmm. suppose the --noable
flags should technically be handled as well but that will nearly double the flag count 🤔
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.
The --noable flags should be take care of:
bazel run //cmd/aspect -- __complete build "--" | grep '\-\-no'
...
--notrim_test_configuration
--nouse_action_cache
--nouse_ijars
--nouse_singlejar_apkbuilder
--nouse_top_level_targets_for_symlinks
--nouse_workers_with_dexbuilder
--noverbose_explanations
--noverbose_failures
--nowatchfs
--noworker_quit_after_build
--noworker_sandboxing
--noworker_verbose
--nozip_undeclared_test_outputs
Performance looks good with bash. Wonder what is up with zsh? |
The |
Overall this is great. 🙏 for the PR and thanks for your patience. The only issues I found were the zsh performance (which we can land without fixing), the missing Performance is fine with bash so we don't need any optimizations there. Detecting the package corresponding to the cwd (to handle |
Life got busy. I will try to give this another look this weekend. |
2d480a0
to
3e4d674
Compare
Rebased, resolved conflicts & force pushed |
@gregmagolan @oncilla any updates on this one? |
This change adds auto completion for bazel flags on all the bazel sub-commands. ``` aspect build --sand<tab><tab> --sandbox_add_mount_pair --sandbox_explicit_pseudoterminal --sandbox_base --sandbox_fake_hostname --sandbox_block_path --sandbox_fake_username --sandbox_debug --sandbox_tmpfs_path --sandbox_default_allow_network --sandbox_writable_path ``` Flag completion for aspect specific flags is not affected: ``` aspect build --aspect:<tab><tab> config (User-specified Aspect CLI config file. /dev/null indic…) interactive (Interactive mode (e.g. prompts for user input) ```
The logic is similar to the bazel logic: https://github.com/bazelbuild/bazel/blob/d1bbf4b7698e019c11b7d1c483eb0b4959954060/scripts/bazel-complete-template.bash#L272-L278 However, not everything is behaving exactly the same, e.g., a custom `BAZEL_COMPLETION_PACKAGE_PATH` is not supported yet. However, in most cases the solution will just work.
3e4d674
to
5f3c19c
Compare
I finally found some time to work on this. Let me know what you think |
@alexeagle @gregmagolan any thing I can do to move this forward? |
Thanks for the reminding @oncilla. I finally have a breather from Aspect Workflows trials & sales this week since we're at a company offsite and told all of our customers we'll be busy this week. Just pulled down your PR. I'll sync it internally and get it landed there this week. |
I've synced this PR internally and improved the completion logic there. There were some bugs and I ended up simplifying the logic and handling more cases such as labels with Thanks for the inspiration @oncilla |
@gregmagolan awesome! Can't wait to drop my hacky bazel completion script. |
Inspired by #373 with a re-write + simplication + improvement of the completion logic from the original. To enable bash completion: ``` $ brew install bash-completion $ aspect completion bash > $(brew --prefix)/etc/bash_completion.d/aspect # add to your .bash_profile: if [[ -r "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" ]]; then . "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" echo "bash completion loaded" fi ``` See https://tecadmin.net/enable-bash-completion-on-macos/ for more info. Demos: ``` $ aspect <tab><tab> analyze-profile (Analyze build profile data) info (Display runtime info about the bazel server) aquery (Query the action graph) init (Create a new Bazel workspace) build (Build the specified targets) license (Prints the license of this software.) canonicalize-flags (Present a list of bazel options in a canonical form) lint (Run configured linters over the dependency graph.) clean (Remove the output tree) outputs (Print paths to declared output files) completion (Generate the autocompletion script for the specified shell) print (Print syntax elements from BUILD files) config (Displays details of configurations.) query (Query the dependency graph, ignoring configuration flags) configure (Auto-configure Bazel by updating BUILD files) run (Build a single target and run it with the given arguments) coverage (Same as 'test', but also generates a code coverage report.) shutdown (Stop the bazel server) cquery (Query the dependency graph, honoring configuration flags) support (Interactive, human-escalated support for Bazel problems) docs (Open documentation in the browser) test (Build the specified targets and run all test targets among them) fetch (Fetch external repositories that are prerequisites to the targets) version (Print the versions of Aspect CLI and Bazel) help ``` ``` $ aspect test <tab><tab> :test :test2 foo foo/bar foo/fum/bar ``` ``` $ aspect test foo<tab><tab> foo foo/bar foo/fum/bar foo:foo foo:fum/test foo:test foo:test2 ``` ``` $ aspect test foo/<tab><tab> foo/bar foo/fum/bar ``` ``` $ aspect test //foo<tab><tab> //foo //foo/bar //foo/fum/bar //foo:foo //foo:fum/test //foo:test //foo:test2 ``` ``` $ aspect test //foo/<tab><tab> //foo/bar //foo/fum/bar ``` ``` $ aspect test @//foo<tab><tab> @//foo @//foo/bar @//foo/fum/bar @//foo:foo @//foo:fum/test @//foo:test @//foo:test2 ``` ``` $ aspect test @//foo/<tab><tab> @//foo/bar @//foo/fum/bar ``` ``` $ aspect test @@//foo<tab><tab> @@//foo @@//foo/bar @@//foo/fum/bar @@//foo:foo @@//foo:fum/test @@//foo:test @@//foo:test2 ``` ``` $ aspect test @@//foo/<tab><tab> @@//foo/bar @@//foo/fum/bar ``` ``` $ cd foo $ aspect test <tab><tab> :foo :fum/test :test :test2 bar fum/bar ``` GitOrigin-RevId: 4044b8d8bb34dfad160cb1d4728bf13d10cb3b9c
Inspired by #373 with a re-write + simplication + improvement of the completion logic from the original. To enable bash completion: ``` $ brew install bash-completion $ aspect completion bash > $(brew --prefix)/etc/bash_completion.d/aspect # add to your .bash_profile: if [[ -r "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" ]]; then . "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" echo "bash completion loaded" fi ``` See https://tecadmin.net/enable-bash-completion-on-macos/ for more info. Demos: ``` $ aspect <tab><tab> analyze-profile (Analyze build profile data) info (Display runtime info about the bazel server) aquery (Query the action graph) init (Create a new Bazel workspace) build (Build the specified targets) license (Prints the license of this software.) canonicalize-flags (Present a list of bazel options in a canonical form) lint (Run configured linters over the dependency graph.) clean (Remove the output tree) outputs (Print paths to declared output files) completion (Generate the autocompletion script for the specified shell) print (Print syntax elements from BUILD files) config (Displays details of configurations.) query (Query the dependency graph, ignoring configuration flags) configure (Auto-configure Bazel by updating BUILD files) run (Build a single target and run it with the given arguments) coverage (Same as 'test', but also generates a code coverage report.) shutdown (Stop the bazel server) cquery (Query the dependency graph, honoring configuration flags) support (Interactive, human-escalated support for Bazel problems) docs (Open documentation in the browser) test (Build the specified targets and run all test targets among them) fetch (Fetch external repositories that are prerequisites to the targets) version (Print the versions of Aspect CLI and Bazel) help ``` ``` $ aspect test <tab><tab> :test :test2 foo foo/bar foo/fum/bar ``` ``` $ aspect test foo<tab><tab> foo foo/bar foo/fum/bar foo:foo foo:fum/test foo:test foo:test2 ``` ``` $ aspect test foo/<tab><tab> foo/bar foo/fum/bar ``` ``` $ aspect test //foo<tab><tab> //foo //foo/bar //foo/fum/bar //foo:foo //foo:fum/test //foo:test //foo:test2 ``` ``` $ aspect test //foo/<tab><tab> //foo/bar //foo/fum/bar ``` ``` $ aspect test @//foo<tab><tab> @//foo @//foo/bar @//foo/fum/bar @//foo:foo @//foo:fum/test @//foo:test @//foo:test2 ``` ``` $ aspect test @//foo/<tab><tab> @//foo/bar @//foo/fum/bar ``` ``` $ aspect test @@//foo<tab><tab> @@//foo @@//foo/bar @@//foo/fum/bar @@//foo:foo @@//foo:fum/test @@//foo:test @@//foo:test2 ``` ``` $ aspect test @@//foo/<tab><tab> @@//foo/bar @@//foo/fum/bar ``` ``` $ cd foo $ aspect test <tab><tab> :foo :fum/test :test :test2 bar fum/bar ``` GitOrigin-RevId: 4044b8d8bb34dfad160cb1d4728bf13d10cb3b9c
Inspired by #373 with a re-write + simplication + improvement of the completion logic from the original. To enable bash completion: ``` $ brew install bash-completion $ aspect completion bash > $(brew --prefix)/etc/bash_completion.d/aspect # add to your .bash_profile: if [[ -r "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" ]]; then . "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" echo "bash completion loaded" fi ``` See https://tecadmin.net/enable-bash-completion-on-macos/ for more info. Demos: ``` $ aspect <tab><tab> analyze-profile (Analyze build profile data) info (Display runtime info about the bazel server) aquery (Query the action graph) init (Create a new Bazel workspace) build (Build the specified targets) license (Prints the license of this software.) canonicalize-flags (Present a list of bazel options in a canonical form) lint (Run configured linters over the dependency graph.) clean (Remove the output tree) outputs (Print paths to declared output files) completion (Generate the autocompletion script for the specified shell) print (Print syntax elements from BUILD files) config (Displays details of configurations.) query (Query the dependency graph, ignoring configuration flags) configure (Auto-configure Bazel by updating BUILD files) run (Build a single target and run it with the given arguments) coverage (Same as 'test', but also generates a code coverage report.) shutdown (Stop the bazel server) cquery (Query the dependency graph, honoring configuration flags) support (Interactive, human-escalated support for Bazel problems) docs (Open documentation in the browser) test (Build the specified targets and run all test targets among them) fetch (Fetch external repositories that are prerequisites to the targets) version (Print the versions of Aspect CLI and Bazel) help ``` ``` $ aspect test <tab><tab> :test :test2 foo foo/bar foo/fum/bar ``` ``` $ aspect test foo<tab><tab> foo foo/bar foo/fum/bar foo:foo foo:fum/test foo:test foo:test2 ``` ``` $ aspect test foo/<tab><tab> foo/bar foo/fum/bar ``` ``` $ aspect test //foo<tab><tab> //foo //foo/bar //foo/fum/bar //foo:foo //foo:fum/test //foo:test //foo:test2 ``` ``` $ aspect test //foo/<tab><tab> //foo/bar //foo/fum/bar ``` ``` $ aspect test @//foo<tab><tab> @//foo @//foo/bar @//foo/fum/bar @//foo:foo @//foo:fum/test @//foo:test @//foo:test2 ``` ``` $ aspect test @//foo/<tab><tab> @//foo/bar @//foo/fum/bar ``` ``` $ aspect test @@//foo<tab><tab> @@//foo @@//foo/bar @@//foo/fum/bar @@//foo:foo @@//foo:fum/test @@//foo:test @@//foo:test2 ``` ``` $ aspect test @@//foo/<tab><tab> @@//foo/bar @@//foo/fum/bar ``` ``` $ cd foo $ aspect test <tab><tab> :foo :fum/test :test :test2 bar fum/bar ``` GitOrigin-RevId: 4044b8d8bb34dfad160cb1d4728bf13d10cb3b9c
Inspired by #373 with a re-write + simplication + improvement of the completion logic from the original. To enable bash completion: ``` $ brew install bash-completion $ aspect completion bash > $(brew --prefix)/etc/bash_completion.d/aspect # add to your .bash_profile: if [[ -r "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" ]]; then . "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" echo "bash completion loaded" fi ``` See https://tecadmin.net/enable-bash-completion-on-macos/ for more info. Demos: ``` $ aspect <tab><tab> analyze-profile (Analyze build profile data) info (Display runtime info about the bazel server) aquery (Query the action graph) init (Create a new Bazel workspace) build (Build the specified targets) license (Prints the license of this software.) canonicalize-flags (Present a list of bazel options in a canonical form) lint (Run configured linters over the dependency graph.) clean (Remove the output tree) outputs (Print paths to declared output files) completion (Generate the autocompletion script for the specified shell) print (Print syntax elements from BUILD files) config (Displays details of configurations.) query (Query the dependency graph, ignoring configuration flags) configure (Auto-configure Bazel by updating BUILD files) run (Build a single target and run it with the given arguments) coverage (Same as 'test', but also generates a code coverage report.) shutdown (Stop the bazel server) cquery (Query the dependency graph, honoring configuration flags) support (Interactive, human-escalated support for Bazel problems) docs (Open documentation in the browser) test (Build the specified targets and run all test targets among them) fetch (Fetch external repositories that are prerequisites to the targets) version (Print the versions of Aspect CLI and Bazel) help ``` ``` $ aspect test <tab><tab> :test :test2 foo foo/bar foo/fum/bar ``` ``` $ aspect test foo<tab><tab> foo foo/bar foo/fum/bar foo:foo foo:fum/test foo:test foo:test2 ``` ``` $ aspect test foo/<tab><tab> foo/bar foo/fum/bar ``` ``` $ aspect test //foo<tab><tab> //foo //foo/bar //foo/fum/bar //foo:foo //foo:fum/test //foo:test //foo:test2 ``` ``` $ aspect test //foo/<tab><tab> //foo/bar //foo/fum/bar ``` ``` $ aspect test @//foo<tab><tab> @//foo @//foo/bar @//foo/fum/bar @//foo:foo @//foo:fum/test @//foo:test @//foo:test2 ``` ``` $ aspect test @//foo/<tab><tab> @//foo/bar @//foo/fum/bar ``` ``` $ aspect test @@//foo<tab><tab> @@//foo @@//foo/bar @@//foo/fum/bar @@//foo:foo @@//foo:fum/test @@//foo:test @@//foo:test2 ``` ``` $ aspect test @@//foo/<tab><tab> @@//foo/bar @@//foo/fum/bar ``` ``` $ cd foo $ aspect test <tab><tab> :foo :fum/test :test :test2 bar fum/bar ``` GitOrigin-RevId: 4044b8d8bb34dfad160cb1d4728bf13d10cb3b9c
Inspired by #373 with a re-write + simplication + improvement of the completion logic from the original. To enable bash completion: ``` $ brew install bash-completion $ aspect completion bash > $(brew --prefix)/etc/bash_completion.d/aspect # add to your .bash_profile: if [[ -r "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" ]]; then . "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" echo "bash completion loaded" fi ``` See https://tecadmin.net/enable-bash-completion-on-macos/ for more info. Demos: ``` $ aspect <tab><tab> analyze-profile (Analyze build profile data) info (Display runtime info about the bazel server) aquery (Query the action graph) init (Create a new Bazel workspace) build (Build the specified targets) license (Prints the license of this software.) canonicalize-flags (Present a list of bazel options in a canonical form) lint (Run configured linters over the dependency graph.) clean (Remove the output tree) outputs (Print paths to declared output files) completion (Generate the autocompletion script for the specified shell) print (Print syntax elements from BUILD files) config (Displays details of configurations.) query (Query the dependency graph, ignoring configuration flags) configure (Auto-configure Bazel by updating BUILD files) run (Build a single target and run it with the given arguments) coverage (Same as 'test', but also generates a code coverage report.) shutdown (Stop the bazel server) cquery (Query the dependency graph, honoring configuration flags) support (Interactive, human-escalated support for Bazel problems) docs (Open documentation in the browser) test (Build the specified targets and run all test targets among them) fetch (Fetch external repositories that are prerequisites to the targets) version (Print the versions of Aspect CLI and Bazel) help ``` ``` $ aspect test <tab><tab> :test :test2 foo foo/bar foo/fum/bar ``` ``` $ aspect test foo<tab><tab> foo foo/bar foo/fum/bar foo:foo foo:fum/test foo:test foo:test2 ``` ``` $ aspect test foo/<tab><tab> foo/bar foo/fum/bar ``` ``` $ aspect test //foo<tab><tab> //foo //foo/bar //foo/fum/bar //foo:foo //foo:fum/test //foo:test //foo:test2 ``` ``` $ aspect test //foo/<tab><tab> //foo/bar //foo/fum/bar ``` ``` $ aspect test @//foo<tab><tab> @//foo @//foo/bar @//foo/fum/bar @//foo:foo @//foo:fum/test @//foo:test @//foo:test2 ``` ``` $ aspect test @//foo/<tab><tab> @//foo/bar @//foo/fum/bar ``` ``` $ aspect test @@//foo<tab><tab> @@//foo @@//foo/bar @@//foo/fum/bar @@//foo:foo @@//foo:fum/test @@//foo:test @@//foo:test2 ``` ``` $ aspect test @@//foo/<tab><tab> @@//foo/bar @@//foo/fum/bar ``` ``` $ cd foo $ aspect test <tab><tab> :foo :fum/test :test :test2 bar fum/bar ``` GitOrigin-RevId: 4044b8d8bb34dfad160cb1d4728bf13d10cb3b9c
@oncilla This is now landed, synced back out to this repo and included in the 5.9.7 release. Please give it a try. There are still a few oddities with the suggested completion labels (or lack of labels) that buildozer returns for some BUILD files but generally the behaviour is very good from my testing. |
To be fair, the regular bazel completion is also not great. E.g., macros are not expanded for me etc. I will give it a try on Monday |
It's expected that macros aren't expanded, because Bazel loading phase is too slow to run in tab complete. It just sees the syntax of the BUILD files. |
This change adds auto completion for bazel flags on all the bazel sub-commands.
Flag completion for aspect specific flags is not affected:
This change also adds auto completion support for startup flags:
Furthermore, this change adds label completion on the appropriate commands.
The logic is similar to the bazel logic:
https://github.com/bazelbuild/bazel/blob/d1bbf4b7698e019c11b7d1c483eb0b4959954060/scripts/bazel-complete-template.bash#L272-L278
However, not everything is behaving exactly the same, e.g., a custom
BAZEL_COMPLETION_PACKAGE_PATH
is not supported yet.For regular setups, this solution should work without any issues though.
fixes #355