-
Notifications
You must be signed in to change notification settings - Fork 2k
Update Attach, Build, Commit, Cp, Create subcommand fish completions #1005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This is great!
I noticed some minor issues, and left suggestions for follow ups: if you can amend those commits 🤗
contrib/completion/fish/docker.fish
Outdated
| complete -c docker -A -f -n '__fish_seen_subcommand_from attach' -l help -d 'Print usage' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from attach' -l no-stdin -d 'Do not attach STDIN' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from attach' -l sig-proxy -d 'Proxy all received signals to the process (non-TTY mode only). SIGCHLD, SIGKILL, and SIGSTOP are not proxied.' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from attach' -l sig-proxy -d ' Proxy all received signals to the process' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's some leading whitespace in the description (' Proxy all..)
contrib/completion/fish/docker.fish
Outdated
| complete -c docker -f -n '__fish_docker_no_subcommand' -a cp -d "Copy files/folders between a container and the local filesystem" | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from cp' -s a -l archive -d 'Archive mode (copy all uid/gid information)' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from cp' -s L -l follow-link -d 'Always follow symbol link in SRC_PATH | ||
| ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there's a line-break before the closing quote
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -s v -l volume -d 'Bind mount a volume' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l volumes-from -d 'Mount volumes from the specified container(s)' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -s w -l workdir -d 'Working directory inside the container' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l blkio-weight -d 'Block IO (relative weight), between 10 and 1000, or 0 to disable (default 0)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering why all these moved to the end, and not in alphabetical order 😅
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l cap-add -d 'Add Linux capabilities' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l cap-drop -d 'Drop Linux capabilities' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l cidfile -d 'Write the container ID to the file' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l cpuset -d 'CPUs in which to allow execution (0-3, 0,1)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See question below on alphabetical order 🤗
contrib/completion/fish/docker.fish
Outdated
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -s t -l tty -d 'Allocate a pseudo-TTY' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -s u -l user -d 'Username or UID' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -s v -l volume -d 'Bind mount a volume (e.g., from the host: -v /host:/container, from Docker: -v /container)' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -s u -l user -d ' Username or UID (format: <name|uid>[:<group|gid>])' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leading whitespace in the description (' Username ....)
contrib/completion/fish/docker.fish
Outdated
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l userns -d 'User namespace to use' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l uts -d 'UTS namespace to use' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l volume-driver -d 'Optional volume driver for the container' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l volumes--from -d 'Mount volumes from the specified container(s)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm. Not for this PR, but we should change the description, and remove the (s) from container(s); while you can specify the --volumes-from option multiple times, each flag only accepts a single container.
Perhaps we should also have a look at the Mount volumes from ..., because those volumes aren't really "owned" by a container; --volumes-from merely copies the volume settings (source, dest) from the specified containers.
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l sysctl -d 'Sysctl options' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l tmpfs -d 'Mount a tmpfs directory' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l ulimit -d 'Ulimit options' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from create' -l userns -d 'User namespace to use' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also not for this PR, but this option may need a better description (perhaps mentioning valid values)
Signed-off-by: Kyle Spiers <kyle@spiers.me>
Signed-off-by: Kyle Spiers <kyle@spiers.me>
Signed-off-by: Kyle Spiers <kyle@spiers.me>
Signed-off-by: Kyle Spiers <kyle@spiers.me>
Signed-off-by: Kyle Spiers <kyle@spiers.me>
|
@thaJeztah This should be good to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
| complete -c docker -A -f -n '__fish_seen_subcommand_from attach' -l detach-keys -d 'Override the key sequence for detaching a container' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from attach' -l help -d 'Print usage' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from attach' -l no-stdin -d 'Do not attach STDIN' | ||
| complete -c docker -A -f -n '__fish_seen_subcommand_from attach' -l sig-proxy -d 'Proxy all received signals to the process (non-TTY mode only). SIGCHLD, SIGKILL, and SIGSTOP are not proxied.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps (follow-up) we should add the extra information back to the actual flag description (sounds useful information);
(non-TTY mode only). SIGCHLD, SIGKILL, and SIGSTOP are not proxied.
|
ping @vdemeester @silvin-lubecki PTAL |
|
ping @vdemeester PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🐯

Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.

continuing on #977.
Two things I noticed while doing this:
Docker build
--targetis the only option description that has a periodDocker cp
--follow-linkdescription should probably say symbolic links not symbol link