Uh oh!
There was an error while loading. Please reload this page.
context: add shell-completion for context-names - #6016
Merged
Conversation
thaJeztah
commented
Apr 16, 2025
Comment on lines
7
to
10
| "github.com/docker/cli/cli/command/formatter" | ||
| "github.com/docker/cli/cli/context/store" | ||
| "github.com/docker/docker/api/types/container" | ||
| "github.com/docker/docker/api/types/image" | ||
| "github.com/docker/docker/api/types/network" |
MemberAuthor
There was a problem hiding this comment.
Starting to consider if we should have sub-packages here, otherwise cli/command/completion will pull in too many other packages. We can either use cli/command/completion/xxx or alternatively, per command (cli/command/container/completion). Or .. maybe it should live elsewhere 🤔
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
thaJeztah
commented
Apr 16, 2025
Comment on lines
+55
to
+59
| // ContextNames implements shell completion for context-names. | ||
| func ContextNames(dockerCLI contextProvider) func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) { | ||
| return func(*cobra.Command, []string, string) ([]string, cobra.ShellCompDirective) { | ||
| // TODO(thaJeztah): implement function similar to [store.Names] to (also) include descriptions. | ||
| names, _ := store.Names(dockerCLI.ContextStore()) |
MemberAuthor
There was a problem hiding this comment.
Missing some options here to differentiate for different commands;
- some only accept a single arg
- some should not include the current context (probably)
- ideally, we'd be able to also add the context's description (which could be useful, if short that is)
thaJeztah
commented
Apr 16, 2025
| names[i] += "\tcurrent" | ||
| } | ||
| } | ||
| return names, cobra.ShellCompDirectiveNoFileComp |
MemberAuthor
There was a problem hiding this comment.
Should we have "no space at end" option for positional arguments?
thaJeztahforce-pushed
the
context_completion
branch
3 times, most recently
from
April 17, 2025 08:11
b04c106 to
1a7d7d6ComparethaJeztah
marked this pull request as ready for review
April 17, 2025 08:12
Benehiko
approved these changes
Apr 17, 2025
thaJeztah
commented
Apr 17, 2025
MemberAuthor
I have one more small enhancement to not add a space after the limit is reached (and we don't use file-completion after it); let me push |
thaJeztah
commented
Apr 17, 2025
MemberAuthor
Oh, actually; not sure that works; let's go with what we have here for now |
thaJeztah
commented
Apr 17, 2025
Uh oh!
There was an error while loading. Please reload this page.
For now, these are not exported and included in the cli/commands/contexts package; a copy of this also lives in cmd/docker, but we need to find a good place for these completions, as some of them bring in additional dependencies. Commands that accept multiple arguments provide completion, but removing duplicates: docker context inspect<TAB> default desktop-linux (current) production tcd docker context inspec default<TAB> desktop-linux (current) production tcd docker context inspect default tcd<TAB> desktop-linux (current) production For "context export", we provide completion for the first argument, after which file-completion is provided: # provides context names completion for the first argument docker context export production<TAB> default desktop-linux (current) production tcd # then provides completion for filenames docker context export desktop-linux<TAB> build/ man/ TESTING.md cli/ docker.Makefile go.mod ... Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztahforce-pushed
the
context_completion
branch
from
April 17, 2025 08:32
1a7d7d6 to
6fd72c6ComparethaJeztah
commented
Apr 17, 2025
Comment on lines
+1
to
+2
| // FIXME(thaJeztah): remove once we are a module; the go:build directive prevents go from downgrading language version to go1.16: | ||
| //go:build go1.22 |
MemberAuthor
There was a problem hiding this comment.
Note to self; need to update #5868 after this is merged.
vvoland
approved these changes
Apr 17, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For now, these are not exported and included in the cli/commands/contexts
package; a copy of this also lives in cmd/docker, but we need to find a
good place for these completions, as some of them bring in additional
dependencies.
Commands that accept multiple arguments provide completion, but removing
duplicates:
For "context export", we provide completion for the first argument, after
which file-completion is provided:
- What I did
- How I did it
- How to verify it
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)