Zsh completion updates

[sdurrheimer]

Hello,

Trying to catch up missing changes for the zsh completion.

TODO

• docker daemon
• docker volume Add volume api #14242
• move --dns-opt to daemon command when Add support for DNS options #16031 is merged
• log driver options ? log driver - Interpolate fields into log tag #15384

@vincentbernat

[vincentbernat]

Currently, all good for me!

[calavera]

Thanks @sdurrheimer. Let me know when this is ready to merge.

[sdurrheimer]

@calavera I'm encountering a issue with the docker daemon command. Since it is not listed in the --help commands section, the current zsh completion system for commands is not working for it.
Is it envisagable to add it, it would be a shame to hack the current completion system just to add the daemon command.

[calavera]

oh, that's weird. I had the feeling that the command was in the help too. Maybe @tiborvass can clarify that, I don't know if it's a bug.

[tiborvass]

@sdurrheimer it indeed changed, but it is listed on platforms that support it. On platforms that only have clients (e.g. darwin), it is not listed.

On Linux:

$ docker --help
Usage: docker [OPTIONS] COMMAND [arg...]
       docker daemon [ --help | ... ]
       docker [ -h | --help | -v | --version ]
...

On OS X:

$ docker --help
Usage: docker [OPTIONS] COMMAND [arg...]
       docker [ -h | --help | -v | --version ]
...

[sdurrheimer]

@tiborvass This is not the issue I described, sorry for my poor explanation.

What I was saying is that the daemon command is not listed in the Commands: section like this example:

Commands:
    attach    Attach to a running container
    build     Build an image from a Dockerfile
    commit    Create a new image from a container's changes
    cp        Copy files/folders from a container to a HOSTDIR or to STDOUT
    create    Create a new container
    diff      Inspect changes on a container's filesystem
    events    Get real time events from the server
    exec      Run a command in a running container
    export    Export a container's filesystem as a tar archive
    history   Show the history of an image
    images    List images
    import    Import the contents from a tarball to create a filesystem image
    info      Display system-wide information
    inspect   Return low-level information on a container or image
    kill      Kill a running container
    load      Load an image from a tar archive or STDIN
    login     Register or log in to a Docker registry
    logout    Log out from a Docker registry
    logs      Fetch the logs of a container
    pause     Pause all processes within a container
    port      List port mappings or a specific mapping for the CONTAINER
    ps        List containers
    pull      Pull an image or a repository from a registry
    push      Push an image or a repository to a registry
    rename    Rename a container
    restart   Restart a running container
    rm        Remove one or more containers
    rmi       Remove one or more images
    run       Run a command in a new container
    save      Save an image(s) to a tar archive
    search    Search the Docker Hub for images
    start     Start one or more stopped containers
    stats     Display a live stream of container(s) resource usage statistics
    stop      Stop a running container
    tag       Tag an image into a repository
    top       Display the running processes of a container
    unpause   Unpause all processes within a container
    version   Show the Docker version information
    wait      Block until a container stops, then print its exit code

Run 'docker COMMAND --help' for more information on a command.

The current zsh completion system for commands is automatic, based on this list.

[tiborvass]

@sdurrheimer right, I'm afraid the completion script will have to add daemon manually to its list if it's in the second line of usage. The reason it cannot be in that list of commands is because the options have to be put after daemon and not before like for the others. Think of it as two different binaries :S

[sdurrheimer]

Ok I will add it manually like the help command.

[sdurrheimer]

The recent merge of PR #15421 is causing issues with autocompletion of running/stopped containers.

By truncating image names, the zsh completion is grouping containers badly.

Example:

docker exec -t -i
 -- containers --
pyrenex_bdd_1          999b57e6f353  3ac7035179e0                                                                
pyrenex_nginx-proxy_1                              --      37 seconds, docker.netap                                
pyrenex_cache_1        c5187d4ca23f  3b5f33325b43                                                                
pyrenex_session_1                                  --      36 seconds, memcached                                   
pyrenex_solr_1         8d3fa2cc5fe9                --      37 seconds, pyrenex_solr                                
pyrenex_web_1          5e37016fee7c                --      36 seconds, docker.netap

In this example, pyrenex_bdd_1 and pyrenex_nginx-proxy_1 container shouldn't be on the same group, they don't have the same image. But with with the truncated name, it appears as docker.netap

Since the script is using docker ps to generate this container completion, maybe adding the --no-trunc option would solve the problem.
But in fact, adding this options create others issues:

• full length IDs make the completion unreadable.
• the NAMES has now / and , which makes the containers completion returning invalid completion values.

Example:

$ docker exec -t -i
 -- containers --
05ba23eab2f6f752caab0adbf97d99c922f314a2ca1149331ed4e5bc04710220  pyrenex_bdd_1      bdd_1            bdd      bdd                                                                                                                       
pyrenex_bdd_1                                                     bdd_1                                               --         2 hours, docker.netapsys.fr/debian7-mariadb:5                                                             
0acb160951ae990c2d090400d071dc93e146865ef35add27352e0cdb2057fadd                                                                                                                                                                         
747c86d9aec3cb0031c7bf3590b5e7e7805eb32a2ace9e7572ffbff79c90ecab  pyrenex_cache_1    pyrenex_cache_1  cache_1  cache                                                                                                                     
pyrenex_session_1                                                 pyrenex_session_1  session_1        session         --         2 hours, memcached                                                                                        
8f7748a74690f99aa42e7705f0ba9e5ee54d293391f8c8458e17ff36863d0e6c                                                      --         2 hours, docker.netapsys.fr/magento:apache2-php5.5                                                        
b9e1518c2d67c1eb5dc750753e0e4998c30aac47f03c39fd692fd97b8a0834cf                                                      --         2 hours, docker.netapsys.fr/jwilder-nginx-proxy                                                           
ea8d5eecfa6db22b81f5d65871dd5d3c4248ecc02b29f6030a6c81395a8d1f26  pyrenex_solr_1     pyrenex_solr_1   solr_1   solr   --         2 hours, pyrenex_solr                                                                                     
debian7-mariadb:5 

Here we can see invalid names like bdd_1, bdd etc.

All those problems would be solved by not truncating image names, relevant PR #15472.

[vincentbernat]

@sdurrheimer I did not test with Docker 1.8 but I did introduce a bug in one of my previous commit. I have fixed it here: felixr/docker-zsh-completion@cd0f1b9

Also, have a look at this commit to avoid using "-1" when we know that a field is the last: felixr/docker-zsh-completion@4ccd292

And I have also done this: felixr/docker-zsh-completion@8bf9041

It works with 1.7. I think that the bogus names that you see are due to my first bug.

[sdurrheimer]

@vincentbernat Oh ok, I've added them to this PR if it's ok for you. I will also test them if those commits solves the problem with 1.9.0-dev.

[jessfraz]

how about we do two parts and try to get in what you have now then you can make another PR with the other changes so they are smaller to review and this isnt waiting around :)

[thaJeztah]

Same question as @jfrazelle (#15915 (comment)); I think you're doing great work, but often smaller, incremental PRs make it easier to review, and get merged. (Just a bit concerned we try to do too much in a single PR, and at the pace Docker is moving, it's hard to get everything right) 👍

[sdurrheimer]

@jfrazelle @thaJeztah I totally agree. The projet is moving fast and I was not able to follow the progression lately.
So with this PR is was trying to catch up the delay for the 1.9.0 release.

I still need to do the container naming modification discussed with @vincentbernat.

I was hoping the #16031 to be merged before mine since it will create a conflict in mine (or vice versa).

@vincentbernat Do you want to copy those changes into felixr/docker-zsh-completion repository, so that we can reference them in the squashed commit message ?

[vincentbernat]

I have pushed all your commits to felixr/docker-zsh-completion.

They are:

felixr/docker-zsh-completion@7f996bf82d5c: Add docker daemon command to zsh completion
felixr/docker-zsh-completion@c8ce164538e4: Add docker volume command and subcommands in zsh completion
felixr/docker-zsh-completion@bdc12610e677: Add support for kernel memory limit in zsh completion
felixr/docker-zsh-completion@d286ccd8c602: Add --ulimit flag to build command to zsh completion
felixr/docker-zsh-completion@6a503b44a490: Filter zsh completions of inspect command by --type
felixr/docker-zsh-completion@a1f39f89ac34: Add --config flag to zsh completion
felixr/docker-zsh-completion@5f77b29f4e8d: Add docker ps --format flag to zsh completion
felixr/docker-zsh-completion@259ea005c52c: Remove -h help flag from subcommands in zsh completion
felixr/docker-zsh-completion@6ae6279857f8: Add --privileged flag to docker exec command in zsh completion

felixr/docker-zsh-completion@b721bbbb9e27: Only keep the last name for a running/stopped container
felixr/docker-zsh-completion@8bf9041c4eb2: Use "docker ps --no-trunc" to build completion.
felixr/docker-zsh-completion@cd0f1b9719b9: Fix container completion by name.
felixr/docker-zsh-completion@4ccd29228909: Be more generic when parsing "docker ps" output.

And if you could review this one, we would be in sync:

felixr/docker-zsh-completion@6791b3edf378: Fix "docker run --stop-signal" completion

[sdurrheimer]

Note that I did use ${...##,} instead of ${...%%,} The later one is keeping the first name, not the last one (and its prefix was removed later due to the Swarm stuff).

@vincentbernat ${...##*,} is not good either.
Here a example:

NAMES
pyrenex_solr_1,pyrenex_web_1/pyrenex_solr_1,pyrenex_web_1/solr,pyrenex_web_1/solr_1

With ${...##*,} the completion result is solr_1 instead of pyrenex_solr_1.

[vincentbernat]

I don't find the examples you posted earlier (GitHub should show outdated comments somewhere, but I don't see them). Is the "good" name always the first one? Then, yes, in this case, turn ${...##*,} back to ${...%%,*}. If no, we'll figure something out.

[sdurrheimer]

@vincentbernat Yeah true. This is kind of odd.
In my examples I posted earlier, the good container name is the last one. But in my example of yesterday, the good one is the first one.

[vincentbernat]

Taking the one without / in it is easy. However, to be able to handle Swarm, all of them could have on slash. Let me think a bit about it (some time around the day).

[vincentbernat]

Here is my take: felixr/docker-zsh-completion@961f628d4376. Tell me if it works as expected for you.

[sdurrheimer]

@vincentbernat It works, but I have tested with only one of my examples yet.

It's kind of hacky and harder to maintain. Maybe should we try to convince the docker staff to not truncate the image name in regular docker ps (#15472). It's is not usefull at all and force us to do hacky script (--no-trunc fault).

P.S: I've just rebased to resolve recent conflict with --dns-opt new option. (#16031)

[vincentbernat]

Yes, if they can revert this change, it would be great. Meanwhile, this hack should work as expected. I hope that #15472 will make it before a new release so that we won't have to keep the hack at all. Otherwise, we may have to keep it a bit (as the completion is in contrib, I believe some people may fetch it directly because it lags a bit behind, but I may be wrong).

[sdurrheimer]

Ok, I've squashed those commits into one. So @calavera it's ready to be merged.

We will see later if the --no-trunc image name issue will be fixed or not.

[calavera]

OMG so many changes, thanks @sdurrheimer. We should really try to be better at keeping zsh completion in sync. Specially because I use zsh 😬

LGTM

[tiborvass]

LGTM