Zsh completion fixes

[segevfiner]

- What I did
I fixed two issues in the Zsh completion script:

1. Complete running containers correctly after docker rm -vf.
2. Fixed docker container update completion which was broken.

- How I did it

1. Checking for -f in $opt_args instead of in $words.
2. Adding a missing $.

- How to verify it

1. Enable option-stacking & try to complete after docker rm -vf when you have running containers:

zstyle ':completion:*:*:docker:*' option-stacking yes
zstyle ':completion:*:*:docker-*:*' option-stacking yes

2. Try to complete after docker container update. It used to spew an error from _arguments.

- Description for the changelog

• Complete running containers correctly after docker rm -vf in Zsh

• Fix docker container update completion in Zsh

P.S. Why is option-stacking (The -s argument to _arguments) even an option/zstyle, and not enabled by default? Whether option stacking is possible, or not, is a property of the docker command, which always accepts them. I think it really should be on by default, and I'm not sure having an option is even necessary.

[codecov-io]

Codecov Report

Merging #788 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master    #788   +/-   ##
======================================
  Coverage    50.9%   50.9%           
======================================
  Files         237     237           
  Lines       15338   15338           
======================================
  Hits         7808    7808           
  Misses       7028    7028           
  Partials      502     502

[thaJeztah]

P.S. Why is option-stacking (The -s argument to _arguments) even an option/zstyle, and not enabled by default? Whether option stacking is possible, or not, is a property of the docker command, which always accepts them. I think it really should be on by default, and I'm not sure having an option is even necessary.

See moby/moby@402caa9 (moby/moby#17124) for an explanation

[segevfiner]

See moby/moby@402caa9 (moby/moby#17124) for an explanation

Reading the Zsh documentation for _arguments (Completion System - Completion Functions). It says that if you include a '=' after the option it will consider it as taking an argument either following a '=' in the same word or in the next word. Maybe including that for all short options that take an argument will fix that?

[thaJeztah]

Technically, all short options take a value; -i=true / -i=false is valid, but it's not documented for boolean options because -i and -i=true are the same (and it's confusing).

But; if you have a solution that works, feel free to open a PR for it so that we can try if it works 👍

[segevfiner]

Guess I jumped to the conclusion that it wasn't included, but it seems the completion script actually already has the required '='. So it's really an issue with Zsh's comparguments (Used by _arguments) at this point. 😔

[thaJeztah]

@segevfiner is there anything left to do in this PR, or do I understand correctly that this should be closed?

[segevfiner]

It can be merged/closed. בתאריך יום ב׳, 30 באפר׳ 2018, 15:04, מאת Sebastiaan van Stijn ‏< [email protected]>:

…

@segevfiner <https://github.com/segevfiner> is there anything left to do in this PR, or do I understand correctly that this should be closed? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#788 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AXlg_4ftCMgLXPbVXTi3NxuZC4NIPjbGks5ttv3BgaJpZM4RU6S6> .

[thaJeztah]

ping @jphuynh @sdurrheimer PTAL 🤗

[ratijas]

Hi guys, sorry for necro-posting, but I'm really interested in the current situation around zsh completions for docker. Why this PR was left on reads?

[jphuynh]

Sorry no particular reason from my side apart that I kind of dropped the ball on that moving out from zsh for a long while. Feel free to review that if it's still relevant. I might have a look later if I find some time.