Skip to content

Zsh completion fixes#788

Open
segevfiner wants to merge 1 commit into
docker:masterfrom
segevfiner:zsh-completion-fixes
Open

Zsh completion fixes#788
segevfiner wants to merge 1 commit into
docker:masterfrom
segevfiner:zsh-completion-fixes

Conversation

@segevfiner
Copy link
Copy Markdown

@segevfiner segevfiner commented Jan 5, 2018

- 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
  1. 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
Copy link
Copy Markdown

codecov-io commented Jan 5, 2018

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

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

segevfiner commented Jan 5, 2018

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

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

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

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

@segevfiner
Copy link
Copy Markdown
Author

segevfiner commented Apr 30, 2018 via email

@thaJeztah
Copy link
Copy Markdown
Member

ping @jphuynh @sdurrheimer PTAL 🤗

@ratijas
Copy link
Copy Markdown

ratijas commented Nov 27, 2020

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

jphuynh commented Nov 27, 2020

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>
@thaJeztah thaJeztah force-pushed the zsh-completion-fixes branch from ec601dd to 74e4649 Compare May 13, 2026 14:57
@thaJeztah thaJeztah requested a review from Copilot May 13, 2026 14:58
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 rm completion logic from checking words to 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
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.

8 participants