Zsh completion fixes#788
Conversation
Codecov Report
@@ Coverage Diff @@
## master #788 +/- ##
======================================
Coverage 50.9% 50.9%
======================================
Files 237 237
Lines 15338 15338
======================================
Hits 7808 7808
Misses 7028 7028
Partials 502 502 |
See moby/moby@402caa9 (moby/moby#17124) for an explanation |
Reading the Zsh documentation for |
|
Technically, all short options take a value; But; if you have a solution that works, feel free to open a PR for it so that we can try if it works 👍 |
|
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 |
889a284 to
ec601dd
Compare
|
@segevfiner is there anything left to do in this PR, or do I understand correctly that this should be closed? |
|
It can be merged/closed.
בתאריך יום ב׳, 30 באפר׳ 2018, 15:04, מאת Sebastiaan van Stijn <
notifications@github.com>:
… @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>
.
|
|
ping @jphuynh @sdurrheimer PTAL 🤗 |
|
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? |
|
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. |
When using option-stacking the -f flag that controls completing running containers can be a part of another word of options, so use opt_args that contains the parsed arguments from _arguments to check for it instead. Signed-off-by: Segev Finer <segev208@gmail.com>
ec601dd to
74e4649
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR updates the Zsh completion script for Docker to improve container-name completion behavior when docker container rm is invoked with --force / -f, particularly in short-option stacking scenarios.
Changes:
- Switches the
docker container rmcompletion logic from checkingwordsto checking parsed options (opt_args) to decide whether to include running containers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case $state in | ||
| (values) | ||
| if [[ ${words[(r)-f]} == -f || ${words[(r)--force]} == --force ]]; then | ||
| if [[ -n ${opt_args[(i)-f]} || -n ${opt_args[(i)--force]} ]]; then |
- What I did
I fixed two issues in the Zsh completion script:
docker rm -vf.docker container updatecompletion which was broken.- How I did it
-fin$opt_argsinstead of in$words.$.- How to verify it
option-stacking& try to complete afterdocker rm -vfwhen you have running containers: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-sargument 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.