Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

albers
Copy link
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
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
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
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
Contributor

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

Copy link
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
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
Member Author

albers commented Feb 13, 2017

@mlaventure Maybe #30867?

@albers
Copy link
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
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
Member Author

albers commented Feb 13, 2017

@mlaventure Thanks! Are you working on Ubuntu?

@mlaventure
Copy link
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.