fix(cmd): Hide full suggestions if they are aliases#401
Conversation
Signed-off-by: Cezar Craciunoiu <cezar@unikraft.io>
There was a problem hiding this comment.
Pull request overview
This PR adjusts the CLI’s tab-completion integration to hide redundant alias suggestions in root (and nested) command listings, while keeping aliases available when the user is typing a prefix. It introduces a small wrapper around kong-completion registration, adds targeted unit tests, and updates the README completion examples.
Changes:
- Add
registerCompletionwrapper to filter completion output and hide child-command aliases for “bare tab” completions. - Wire the new completion registration into the root command initialization, preserving the completion exit behavior.
- Add unit tests for parsing
COMP_LINE/COMP_POINTand for the alias-hiding completion behavior; update README completion invocation examples.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| README.md | Updates shell completion script generation examples. |
| internal/cmd/root.go | Switches from direct kongcompletion.Register to the new registerCompletion wrapper (with explicit exit handling). |
| internal/cmd/completion.go | New implementation that captures/filters completion output to hide alias entries in root/nested listings. |
| internal/cmd/completion_test.go | New tests covering COMP_LINE parsing and alias-hiding behavior during completion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Cezar Craciunoiu <cezar@unikraft.io>
craciunoiuc
force-pushed
the
craciunoiuc/fix-completions
branch
from
6b2a7f0 to
6e4cb5d
Compare
| if isComp != tt.wantIsComp { | ||
| t.Fatalf("isCompletion = %v, want %v", isComp, tt.wantIsComp) | ||
| } | ||
| if !slices.Equal(completed, tt.wantCompleted) { | ||
| t.Errorf("completed = %v, want %v", completed, tt.wantCompleted) | ||
| } | ||
| if bareLast != tt.wantBareLast { | ||
| t.Errorf("bareLast = %v, want %v", bareLast, tt.wantBareLast) | ||
| } |
There was a problem hiding this comment.
testify.
Maybe add to AGENTS.md as well.
There was a problem hiding this comment.
Might be worth mimicing the style here: https://github.com/unikraft/cli/blob/f0b830b56a8f6c2b40487cd3fddbdcd99e023d6d/internal/multimetro/key_test.go#L166-L166
That test style is quite clear and easy to verify, the current one here is a bit tricky to grok.
There was a problem hiding this comment.
Right, I always assume it did it like the others and never actually check unit test generation 💫
| // registerCompletion wires up kong-completion, the same way | ||
| // kongcompletion.Register does, but additionally hides command aliases from | ||
| // the root completion listing. | ||
| func registerCompletion(parser *kong.Kong, opt ...kongcompletion.Option) *int { |
There was a problem hiding this comment.
Hmmmm. Is there a kongcompletion opt for this?
If not... is there something we could upstream that would make this more readable? I don't love needing to pull all the code of a dependency in, and run the completion ourself.
The upstream maintainer has actually been really receptive before.
There was a problem hiding this comment.
There is completion-enabled-command-alias.
Maybe that's okay honestly 🤔
Closes: TOOL-1152
Closes: #399