cli icon indicating copy to clipboard operation
cli copied to clipboard

Fix docker plugin suggestions in Zsh completion

Open mcornella opened this issue 3 years ago • 8 comments

Fixes #2761

- What I did

The __docker_plugins function calls docker plugin ls to get its information, but it didn't parse it correctly before, so I fixed it. Without this change, triggering the function would produce the following errors:

__docker_plugins:27: bad math expression: empty string
__docker_plugins:27: bad math expression: empty string
__docker_plugins:27: bad math expression: empty string
__docker_plugins:27: bad math expression: empty string
__docker_plugins:27: bad math expression: empty string

- How I did it

The docker plugin ls command doesn't output a TAG column; instead, its name is ID. So one of the changes is to replace TAG for ID. Also, the relevant part of ID hash is the left one, so we need to use right-padding to trim it to 7 characters. With (l:7:: :::) what we get is the last 7 characters of the hash, which is not what we want.

- How to verify it

  1. If you don't have docker plugins installed, install at least 2. For example:
    docker plugin install vieux/sshfs
    docker plugin install vieux/sshfs:1.3
    docker plugin install grafana/loki-docker-driver:latest --alias loki --grant-all-permissions
    
  2. Run docker plugin ls to see the output:
    $ docker plugin ls
    ID             NAME                 DESCRIPTION               ENABLED
    a237ec8092c3   loki:latest          Loki Logging Driver       true
    23dd56eff145   vieux/sshfs:1.3      sshFS plugin for Docker   true
    84dd2ad7d54e   vieux/sshfs:latest   sshFS plugin for Docker   true
    
  3. Make sure to be using this _docker file for completion. In Zsh, download this _docker file and run this to load it:
    unfunction _docker; autoload -Uz /absolute/path/to/downloaded/_docker
    
  4. Tab-complete docker plugin inspect:
    $ docker plugin inspect [TAB]
    loki:latest         vieux/sshfs:1.3     vieux/sshfs:latest
    

- Description for the changelog

Fix docker plugin Zsh completion by correctly parsing the output of docker plugin ls.

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

cute sleeping baby donkeys

mcornella avatar Jan 01 '21 12:01 mcornella

Codecov Report

Merging #2903 (beb4ede) into master (2291f61) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2903   +/-   ##
=======================================
  Coverage   57.05%   57.05%           
=======================================
  Files         297      297           
  Lines       18653    18653           
=======================================
  Hits        10643    10643           
  Misses       7151     7151           
  Partials      859      859           

codecov-io avatar Jan 01 '21 12:01 codecov-io

Turns out I was wrong. Took making a PR to realise that 😅

The problem is that docker plugin ls used to provide a TAG column with the plugin tag name, but now it's is incorporated into the NAME column. For example, if we install both the latest and 1.3 tags of vieux/sshfs, this is what docker plugin ls looks like:

docker plugin install vieux/sshfs:latest
docker plugin install vieux/sshfs:1.3
$ docker plugin ls
ID             NAME                 DESCRIPTION               ENABLED
a237ec8092c3   loki:latest          Loki Logging Driver       true
23dd56eff145   vieux/sshfs:1.3      sshFS plugin for Docker   true
84dd2ad7d54e   vieux/sshfs:latest   sshFS plugin for Docker   true

With the current proposed change, this is what completion suggests:

$ docker plugin inspect [TAB]
loki         -- latest:a237ec8
vieux/sshfs  -- 1.3:23dd56e

Note that the tag (latest, 1.3) appears in the right side of the completion entry, and vieux/sshfs is only presented once, while two entries should appear, one for each tag. This happens because : is a special character used by _describe to provide a comment for a completion entry, so instead of appearing in the suggested completion it's parsed by _describe. If we quote the colon, then the tag correctly appears in the suggestion:

diff --git a/contrib/completion/zsh/_docker b/contrib/completion/zsh/_docker
index a7d94a3..ee32e82 100644
--- a/contrib/completion/zsh/_docker
+++ b/contrib/completion/zsh/_docker
@@ -1581,7 +1581,7 @@ __docker_plugins() {
 
     # Name
     for line in $lines; do
-        s="${line[${begin[NAME]},${end[NAME]}]%% ##}"
+        s="${${line[${begin[NAME]},${end[NAME]}]%% ##}//:/\:}"
         s="$s:${(r:7:: :::)${${line[${begin[ID]},${end[ID]}]}%% ##}}"
         plugins=($plugins $s)
     done
$ docker plugin inspect [TAB]
loki:latest         -- a237ec8
vieux/sshfs:1.3     -- 23dd56e
vieux/sshfs:latest  -- 84dd2ad

Also, the line in the for that tries to find the TAG column can be removed, since it doesn't provide any relevant information. This is how the completion now looks like:

$ docker plugin inspect [TAB]
loki:latest         vieux/sshfs:1.3     vieux/sshfs:latest

I'll redo the original PR post to match this recent changes.

mcornella avatar Jan 01 '21 16:01 mcornella

Ping @thaJeztah

mcornella avatar Sep 15 '21 14:09 mcornella

Rebased on top of the current master. @thaJeztah this problem still exists, so this PR is still relevant.

mcornella avatar Apr 06 '22 16:04 mcornella

Codecov Report

Merging #2903 (beb4ede) into master (73b05aa) will decrease coverage by 1.94%. The diff coverage is n/a.

:exclamation: Current head beb4ede differs from pull request most recent head f617883. Consider uploading reports for the commit f617883 to get more accurate results

@@            Coverage Diff             @@
##           master    #2903      +/-   ##
==========================================
- Coverage   59.00%   57.05%   -1.95%     
==========================================
  Files         284      297      +13     
  Lines       23839    18653    -5186     
==========================================
- Hits        14066    10643    -3423     
+ Misses       8914     7151    -1763     
  Partials      859      859              

codecov-commenter avatar Apr 06 '22 16:04 codecov-commenter

It would help if someone could merge this! @thaJeztah

obvionaoe avatar Apr 06 '22 17:04 obvionaoe

@thaJeztah gentle ping. This is still an issue 😊

mcornella avatar Apr 20 '22 07:04 mcornella

Is this still being considered, as I'm also suffering from this bug?

toby-griffiths avatar Mar 07 '24 10:03 toby-griffiths