Skip to content
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

completion: fix / add completion for service names and node-names #5848

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Feb 20, 2025

completion: use service names, and support DOCKER_COMPLETION_SHOW_SERVICE_IDS

Change completion for services to use names by default, and bring back
support for the DOCKER_COMPLETION_SHOW_SERVICE_IDS env-var

# DOCKER_COMPLETION_SHOW_SERVICE_IDS
# "no" - Show names only (default)
# "yes" - Show names and ids

Before this patch:

docker service ps
c9vrp2pwni9gx5ghat20rjpcy  hmthf0tqws9xpmd87ok7diqly

With this patch:

docker service ps<TAB>
databaseservice  webservice

export DOCKER_COMPLETION_SHOW_SERVICE_IDS=yes
docker service ps<TAB>
c9vrp2pwni9gx5ghat20rjpcy  databaseservice            hmthf0tqws9xpmd87ok7diqly  webservice

completion: add completion for node names

Change completion for nodes to use names by default, and bring back
support for the DOCKER_COMPLETION_SHOW_NODE_IDS env-var

# DOCKER_COMPLETION_SHOW_NODE_IDS

With this patch:

docker node ps <tab>
docker-desktop            self

export DOCKER_COMPLETION_SHOW_NODE_IDS=yes
docker node ps <TAB>
docker-desktop             qyeriqk20al6hy4y869d08ff5  self

completion: add completion for docker node flags

With this patch:

docker node update --role
manager  worker

docker node update --availability
active  drain   pause

completion: add completion for docker service flags

Not all flags have completions yet, and for those that don't have completion,
we disable completion to prevent it completing with filenames.

- Human readable description for the release notes

Fix shell completion suggesting IDs instead of names for services and nodes

- A picture of a cute animal (not mandatory but encouraged)

@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2025

Codecov Report

Attention: Patch coverage is 64.84375% with 45 lines in your changes missing coverage. Please review.

Project coverage is 58.81%. Comparing base (f9ced58) to head (8f55738).
Report is 7 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #5848    +/-   ##
========================================
  Coverage   58.81%   58.81%            
========================================
  Files         349      350     +1     
  Lines       29556    29676   +120     
========================================
+ Hits        17382    17455    +73     
- Misses      11189    11236    +47     
  Partials      985      985            

…VICE_IDS

Change completion for services to use names by default, and bring back
support for the `DOCKER_COMPLETION_SHOW_SERVICE_IDS` env-var
https://github.com/docker/cli/blob/f9ced58158d5e0b358052432244b483774a1983d/contrib/completion/bash/docker#L41-L43

Before this patch:

    docker service ps
    c9vrp2pwni9gx5ghat20rjpcy  hmthf0tqws9xpmd87ok7diqly

With this patch:

    docker service ps<TAB>
    databaseservice  webservice

    export DOCKER_COMPLETION_SHOW_SERVICE_IDS=yes
    docker service ps<TAB>
    c9vrp2pwni9gx5ghat20rjpcy  databaseservice            hmthf0tqws9xpmd87ok7diqly  webservice

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Copy link
Collaborator

@vvoland vvoland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vvoland vvoland added the kind/bugfix PR's that fix bugs label Feb 20, 2025
Comment on lines 21 to 22
ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
return completeNodeNames(dockerCli)(cmd, args, toComplete)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
return completeNodeNames(dockerCli)(cmd, args, toComplete)
ValidArgsFunction: completeNodeNames(dockerCli)

nit

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh! Good call; wouldn't surprise me if we did the same in other places (sometimes the closures trip me up for sure!)

Change completion for nodes to use names by default, and bring back
support for the `DOCKER_COMPLETION_SHOW_NODE_IDS` env-var
https://github.com/docker/cli/blob/f9ced58158d5e0b358052432244b483774a1983d/contrib/completion/bash/docker#L38

With this patch:

    docker node ps <tab>
    docker-desktop            self

    export DOCKER_COMPLETION_SHOW_NODE_IDS=yes
    docker node ps <TAB>
    docker-desktop             qyeriqk20al6hy4y869d08ff5  self

Signed-off-by: Sebastiaan van Stijn <[email protected]>
With this patch:

    docker node update --role
    manager  worker

    docker node update --availability
    active  drain   pause

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Not all flags have completions yet, and for those that don't have completion,
we disable completion to prevent it completing with filenames.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the improve_swarm_completion branch from 147a3cb to 8f55738 Compare February 20, 2025 17:33
@thaJeztah thaJeztah modified the milestones: 29.0.0, 28.0.1 Feb 20, 2025
@neersighted neersighted merged commit eb48cad into docker:master Feb 20, 2025
89 checks passed
@thaJeztah thaJeztah deleted the improve_swarm_completion branch February 20, 2025 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[28.0.0] bash autocompletion works different
4 participants