Add dynamic shell completions for ModelKit references #963
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds dynamic shell completions for ModelKit references across multiple CLI commands. The completions intelligently suggest local ModelKit repositories and tags based on the current input context, supporting both repository-only completions (with colon suffix) and full repository:tag completions.
Key changes:
- Implements a new completion system that generates ModelKit reference suggestions based on local repositories
- Adds completion logic that handles repository names, tags, and digest-based references
- Integrates completions into commands like
kit info,kit inspect,kit push,kit remove,kit tag, andkit unpack
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/lib/completion/modelkits.go | New completion module with logic for generating ModelKit reference completions |
| pkg/cmd/unpack/cmd.go | Adds ModelKit completion support for the unpack command |
| pkg/cmd/tag/cmd.go | Adds ModelKit completion for the first argument of tag command |
| pkg/cmd/remove/cmd.go | Adds conditional ModelKit completion when --all flag is not present |
| pkg/cmd/push/cmd.go | Adds ModelKit completion support for push command arguments |
| pkg/cmd/pack/cmd.go | Sets default completion directive to enable file completions |
| pkg/cmd/inspect/cmd.go | Adds conditional ModelKit completion when --remote flag is not present |
| pkg/cmd/info/cmd.go | Adds conditional ModelKit completion when --remote flag is not present |
| cmd/root.go | Adds flag completions for --log-level and --progress options |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pkg/cmd/remove/cmd.go
Outdated
| Example: examples, | ||
| RunE: runCommand(opts), | ||
| ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { | ||
| if slices.Contains(args, "--all") || slices.Contains(args, "--remote") { |
There was a problem hiding this comment.
The slices.Contains(args, "--all") check is incorrect. The args parameter contains positional arguments, not flags. Flags should be checked using cmd.Flags().Changed() or similar methods.
| if slices.Contains(args, "--all") || slices.Contains(args, "--remote") { | |
| if cmd.Flags().Changed("all") || cmd.Flags().Changed("remote") { |
There was a problem hiding this comment.
Looks like copilot caught the issue I see.
There was a problem hiding this comment.
Yeah, that's a good catch; don't know why I didn't test this directly.
| Long: longDesc, | ||
| Example: example, | ||
| RunE: runCommand(opts), | ||
| Args: cobra.RangeArgs(1, 2), |
There was a problem hiding this comment.
The Args field is being set twice - once at line 109 and once removed at line 117. This duplication could cause confusion. Consider keeping the Args assignment in one location.
There was a problem hiding this comment.
Does Copilot not understand that once a line of code is removed, it is no longer run as part of the program?
| return completion.GetLocalModelKitsCompletion(cmd.Context(), toComplete), cobra.ShellCompDirectiveNoFileComp | cobra.ShellCompDirectiveNoSpace | ||
| }, | ||
| } | ||
|
|
There was a problem hiding this comment.
The Args field is being set twice - once at line 109 and once removed at line 117. This duplication could cause confusion. Consider keeping the Args assignment in one location.
amisevsk
force-pushed
the
completions
branch
from
ffc209f to
acf2a2d
Compare
gorkem
left a comment
gorkem
left a comment
There was a problem hiding this comment.
I bumped to the issue with kit info --remote which copilot actually flagged. The rest seems to work well.
Add completions for locally-stored ModelKits to commands where it makes sense. This allows completing ModelKit references in e.g. push commands. The list of ModelKits is generated at run time based on local storage. For commands that work both locally and remotely, we only generate completions when the user could be referring to a local ModelKit. Signed-off-by: Angel Misevski <amisevsk@gmail.com>
* Add flag completions for flags that take one of a few values (e.g. --log-level, --progress) * Remove filesystem completions for most commands, as they don't care about local disk; for commands that do, make sure we print them as usual. Signed-off-by: Angel Misevski <amisevsk@gmail.com>
amisevsk
force-pushed
the
completions
branch
from
acf2a2d to
f76e600
Compare
2 participants
Description
This PR adds dynamic completions for ModelKit references where applicable:
kit info: If the--remoteflag is not specified, generate ModelKit reference completionskit inspect: Same askit infokit push: Generate completions for ModelKit references, supporting both the one-arg and two-arg (kit push [SOURCE] [TARGET]) formatkit remove: Generate completions if the--allflag is not specifiedkit tag: Generate completions for the first argument only; ignore the second argumentkit unpack: Generate completions. Since we can't tell if the intended ModelKit is present locally (since we can unpack from a remote without pulling first), these may be less useful.For ModelKit completions, we generate completions in two steps:
:), suggest repositories for completion, ignoring tags.Additionally, completion based on digests is supported as well. If a ModelKit reference includes an
@symbol (as used with@sha256:...), we suggest digests based on what's in the repo.We also do not suggest filepath completions for commands that do not work on the local directory -- e.g. for
kit packwe support autocompleting local directories, but we suppress those completions forkit push.Finally, this PR adds flag completions for flags that take on a limited set of values:
--progress: Complete tonone,plain, orfancy--log-level: Complete to one of the log level optionsLinked issues
Closes #79
Supercedes #934