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

Add bash completion for available plugins #4094

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Mar 13, 2023

closes #4089

- What I did

This is a follow-up of #4089 (comment) to enable bash completion for available plugins.

- How I did it

- How to verify it

$ mkdir -p ~/.local/share/bash-completion/completions
$ cp ./contrib/completion/bash/docker ~/.local/share/bash-completion/completions/docker
$ _completion_loader docker

$ docker info --format '{{json .ClientInfo.Plugins}}' | jq
[
  {
    "SchemaVersion": "0.1.0",
    "Vendor": "Docker Inc.",
    "Version": "v0.10.0-rc2-175-g00bb9c5c.m",
    "ShortDescription": "Docker Buildx",
    "Name": "buildx",
    "Path": "/home/crazy/.docker/cli-plugins/docker-buildx",
    "ShadowedPaths": [
      "/usr/libexec/docker/cli-plugins/docker-buildx"
    ]
  },
  {
    "SchemaVersion": "0.1.0",
    "Vendor": "Docker Inc.",
    "Version": "v2.16.0",
    "ShortDescription": "Docker Compose",
    "Name": "compose",
    "Path": "/usr/libexec/docker/cli-plugins/docker-compose"
  },
  {
    "SchemaVersion": "0.1.0",
    "Vendor": "Docker Inc.",
    "Version": "v0.23.0",
    "ShortDescription": "Docker Scan",
    "Name": "scan",
    "Path": "/usr/libexec/docker/cli-plugins/docker-scan"
  }
]

$ docker build [tab][tab]
build    builder  buildx

$ docker buildx [tab][tab]
bake        create      help        inspect     prune       stop        version     
build       du          imagetools  ls          rm          use

$ docker buildx b [tab][tab]
bake   build

$ docker co [tab][tab]
commit     compose    config     container  context

$ docker compose [tab][tab]
build    cp       down     exec     kill     ls       port     pull     restart  run      stop     unpause  version  
config   create   events   images   logs     pause    ps       push     rm       start    top      up

To make it works with buildx plugin you need docker/buildx#1674

- Description for the changelog

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

cc @silvin-lubecki @ulyssessouza

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

Not a shell script expert here, but the logic looks good 👍

Copy link
Collaborator

@albers albers left a comment

Choose a reason for hiding this comment

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

This is really, really cool.

I have to admit that I do not fully understand __docker_complete_plugin() but I assume this is basically an idiom for cobra generated completions and should be correct.

I did not find any issues with the completions.

Thanks @crazy-max.

contrib/completion/bash/docker Outdated Show resolved Hide resolved
contrib/completion/bash/docker Outdated Show resolved Hide resolved
contrib/completion/bash/docker Outdated Show resolved Hide resolved
contrib/completion/bash/docker Show resolved Hide resolved
@crazy-max
Copy link
Member Author

crazy-max commented Mar 16, 2023

@albers Thanks for the review. I made the suggested changed.

I think in follow-up we could use the same logic but for all docker commands so we would only need a single function to handle completion like the __docker_complete_plugin() one does.

I also have a branch to handle this for fish and zsh.

e.g., for fish:

# plugins
set PLUGINS_PATH (docker info --format '{{range .ClientInfo.Plugins}}{{.Path}},{{end}}')

function __fish_docker_plugin_complete --description 'Print plugins commands and subcomands' -a plugin_path
    set -l resultArray $plugin_path "__completeNoDesc"
    for i in (commandline -opc)
        if set -q i
            set -a resultArray "''"
        else
            set -a resultArray $i
        end
    end
    set -l result (eval printf '%s ' $resultArray 2> /dev/null | grep -v '^:[0-9]*$')
    if set -q i
        return 1
    end
    echo $result
    return 0
end

for plugin_path in (string split ',' $PLUGINS_PATH)
    set plugin_name (string trim $plugin_path)
    set plugin_name (string trim -l 'docker-' $plugin_name)
    set plugin_name (echo $plugin_name | sed 's/\.[^.]*$//')
    complete -c docker -f -n '__fish_docker_no_subcommand' -a $plugin_name
    complete -c docker -A -f -n "__fish_seen_subcommand_from $plugin_name" -a "(__fish_docker_plugin_complete $plugin_path)"
end

Not tested and needs more work I think. I'm not an expert of this shell 😅.

@codecov-commenter
Copy link

codecov-commenter commented Mar 16, 2023

Codecov Report

Merging #4094 (aa0aa4a) into master (cb5463a) will decrease coverage by 0.02%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4094      +/-   ##
==========================================
- Coverage   59.15%   59.13%   -0.02%     
==========================================
  Files         285      285              
  Lines       24725    24738      +13     
==========================================
+ Hits        14627    14630       +3     
- Misses       9212     9221       +9     
- Partials      886      887       +1     

Copy link
Collaborator

@albers albers left a comment

Choose a reason for hiding this comment

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

Thanks @crazy-max. Very impressive.

Signed-off-by: CrazyMax <github@crazymax.dev>
@crazy-max
Copy link
Member Author

PTAL @thaJeztah @vvoland 🙏

Copy link
Contributor

@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.

Just tested and it seems to work perfectly, thanks!

Copy link
Contributor

@ulyssessouza ulyssessouza left a comment

Choose a reason for hiding this comment

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

LGTM! 🥳

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

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.

None yet

9 participants