cli icon indicating copy to clipboard operation
cli copied to clipboard

[completion/zsh] add volume completion

Open pseyfert opened this issue 4 years ago • 3 comments

This provides completion for docker run ... -v <TAB>. This closes part of #1997.

  • the completion of docker run -it is discussed in the issue and does not require a fix (maybe a different default, but that's out of scope here)
  • the completion of docker run ... -v is addressed in this PR

As outlined here there are multiple options passed to _directories. The behaviour is that:

  • docker run -v <TAB> inserts a / immediately and will suggest directories relative to /. On my system that means docker run -v <TAB> turns into docker run -v / and the completion list contains elements such as usr,home
  • a : is inserted at the end of every suggestion. depending how the user configured their completion this means they can cycle through insertions of docker run -v /usr: docker run -v /home:and so on (for me the colon is bold face).
  • the : does not need to be removed by the user if they want to go one level higher. hitting space or / will remove the :. I.e. hitting / after the completion inserted docker run -v /usr: turns the command line into docker run -v /usr/. Continued hitting of TAB continues the game and suggests docker run -v /usr/bin:, again with a self-removing :
  • the : persists if the user types :
  • after : in the same word there is no more completion suggestions, the user needs to type the volume mount on their own.
  • after a (such as docker run -v /home or docker run -v /home:/hosthome ) the completion continues to complete arguments to docker run just like it did before this PR.

Verification was trying around on the command line with docker run -it --rm -v <stuff here> debian:testing. I think the suggestions should be absolute paths (the original draft suggested relative paths to the cwd, but docker didn't mount them).

One can probably argue if the treatment of the trailing : is optimal (self removal on space and slash. i caught myself a few times that after having docker run -it --rm -v /home: i tried to continue with /hosthome and wanting to type-v /home:/hosthome but typed /home/hosthome as the / overwrote the visual :).

not an animal, but a cake: cake

pseyfert avatar Mar 04 '21 21:03 pseyfert

Codecov Report

Merging #2998 (35b42ef) into master (70a0015) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2998   +/-   ##
=======================================
  Coverage   57.06%   57.06%           
=======================================
  Files         299      299           
  Lines       18683    18683           
=======================================
  Hits        10662    10662           
  Misses       7155     7155           
  Partials      866      866           

codecov-io avatar Mar 04 '21 22:03 codecov-io

Any maintainer out there willing to review / approve? ( @thaJeztah , tagging you as you appear in the other completion related MRs)

pseyfert avatar Aug 25 '22 18:08 pseyfert

@vvoland thanks for the feedback. I appreciate the effort going for a generated completion feature rather than the manually maintained function. That said, shall we still try to merge the MR here? It improves the completion file while it's still there. And admittedly I don't (yet?) find my way around the cobra completion, so it will take a bit until I can contribute to that.

pseyfert avatar Sep 15 '22 08:09 pseyfert

The only thing I was wondering is if it should complete (named) volumes, and if so, how we could make it either complete those, or local paths (for bind-mounts). Not exactly sure what we currently do for the Bash completion (maybe we don't do it there either)

thaJeztah avatar Oct 27 '22 22:10 thaJeztah

(close/reopen to trigger CI)

thaJeztah avatar Oct 27 '22 22:10 thaJeztah