Skip to content

Add bash completion for docker plugin upgrade#30823

Merged
mlaventure merged 1 commit intomoby:masterfrom
albers:completion-plugin-upgrade
Feb 13, 2017
Merged

Add bash completion for docker plugin upgrade#30823
mlaventure merged 1 commit intomoby:masterfrom
albers:completion-plugin-upgrade

Conversation

@albers
Copy link
Copy Markdown
Member

@albers albers commented Feb 8, 2017

Ref #29414
As the corresponding feature will be added in 1.13.1, this PR should go there as well.
Ping @sdurrheimer for zsh completion

Notes for reviewers:
The first non-option is completed as <plugin name:tag>. Completions that include : require __ltrim_colon_completions so that they work on the part after the colon.
The following argument is completed to the available plugin names without the tag, but with a colon appended.

Copy link
Copy Markdown
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐸
/cc @thaJeztah @mlaventure

Copy link
Copy Markdown
Contributor

@mlaventure mlaventure left a comment

Choose a reason for hiding this comment

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

LGTM, but:

This only works on the master.

On the 1.13.1 tag and 1.13.x, bash correctly completes upgrade but afterwards it doesn't complete the available plugins.

@albers if we were to do a 1.13.2 do you know what other commit would need to be cherry-picked into 1.13.x for this to work?

@albers
Copy link
Copy Markdown
Member Author

albers commented Feb 13, 2017

@mlaventure For me, the PR works if cherry-picked onto 1.13.x. Maybe you forgot to install a plugin when you checked?

@mlaventure
Copy link
Copy Markdown
Contributor

hum, I had vieux/sshfs installed. Let me double checked, maybe I had the old binary running.

Comment thread contrib/completion/bash/docker Outdated
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@mlaventure --timeout is nonsense here (though it doesn't harm), I'll fix it in a moment.

Signed-off-by: Harald Albers <github@albersweb.de>
@albers albers force-pushed the completion-plugin-upgrade branch from d64774e to 2c52ec8 Compare February 13, 2017 18:37
@mlaventure
Copy link
Copy Markdown
Contributor

@albers I have the issue with all the plugin commands that should complete on the plugin names actually.

I tried on top of e5a90d4

@albers
Copy link
Copy Markdown
Member Author

albers commented Feb 13, 2017

@mlaventure Maybe #30867?

@albers
Copy link
Copy Markdown
Member Author

albers commented Feb 13, 2017

@mlaventure That's really strange. Maybe I'm doing sonething wrong. Here's what I did:

$ git branch
* master
$ git fetch origin 1.13.x
From https://github.com/docker/docker
 * branch            1.13.x     -> FETCH_HEAD
$ git checkout 1.13.x
Branch 1.13.x set up to track remote branch 1.13.x from origin.
Switched to a new branch '1.13.x'
$ git fetch albers completion-plugin-upgrade
From https://github.com/albers/docker
 * branch            completion-plugin-upgrade -> FETCH_HEAD
$ git cherry-pick 2c52ec8403d721e567a7c3c129fc38c650b85ba4
[1.13.x 32428de] Add bash completion for `docker plugin upgrade`
 Date: Wed Feb 8 13:40:13 2017 +0100
 1 file changed, 20 insertions(+)
$ git log --oneline -2
32428de Add bash completion for `docker plugin upgrade`
e5a90d4 Merge pull request #30875 from albers/fix-30858
$ make binary shell BIND_DIR=.

In a separate shell

$ docker exec -ti $(docker ps -lq) dockerd

Back in DIND

root@1af0ae499ea0:~# docker plugin install --grant-all-permissions vieux/sshfs
latest: Pulling from vieux/sshfs
86b5589884b3: Download complete
Digest: sha256:e5e584b1a2d0855d0be5817506e07f774c7c248db42015caa1e605a60c256007
Status: Downloaded newer image for vieux/sshfs:latest
Installed plugin vieux/sshfs

root@1af0ae499ea0:~# docker plugin upgrade <tab>  # works!

@mlaventure
Copy link
Copy Markdown
Contributor

@albers rebuilt my docker-dev image (I tend to keep them around a while), it seems to work now. Not sure what was wrong.

Let's get this merged :)

@mlaventure mlaventure merged commit c9fa3ee into moby:master Feb 13, 2017
@albers
Copy link
Copy Markdown
Member Author

albers commented Feb 13, 2017

@mlaventure Thanks! Are you working on Ubuntu?

@mlaventure
Copy link
Copy Markdown
Contributor

I am yes, still on 14.04.5 though

@GordonTheTurtle GordonTheTurtle added this to the 1.14.0 milestone Feb 13, 2017
@thaJeztah thaJeztah modified the milestones: 1.13.2, 1.14.0 Feb 13, 2017
@albers albers deleted the completion-plugin-upgrade branch February 14, 2017 08:23
thaJeztah pushed a commit to thaJeztah/docker that referenced this pull request Feb 18, 2017
Add bash completion for `docker plugin upgrade`
(cherry picked from commit c9fa3ee)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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.

5 participants