Skip to content
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

[18.09 backport] Add bash completion for experimental CLI commands (manifest) #1542

Merged

Conversation

thaJeztah
Copy link
Member

Backport of #1252 for 18.09

git checkout -b 18.09_backport_completion_cli_experimental upstream/18.09
git cherry-pick -s -S -x 564d4da06e74bbb2f4ca73dd7d84c070ddfa36ae
git cherry-pick -s -S -x a183c952c6ca3b732f559154b03b066436a2b052
git cherry-pick -s -S -x 0fb4256a00d932d9fe5bc918cb4582bf24f6567d

cherry-pick was clean; no conflicts

This adds bash completion for docker manifest and its subcommands.

Completion for the manifest command is only activated if experimental CLI features are enabled (either by setting "experimental": "enabled" in .docker/config.json or the env var DOCKER_CLI_EXPERIMENTAL=enabled). This configuration setting is harvested from the output of docker version.

Before adding the third configuration setting that is extracted from docker version's output , I refactored the existing logic to a more consistent and clearer state. There is only one call to docker version. It is executed lazily and caches its result.

Note: The initial concept also included gathering the orchestrator in this step, but this piece of information went bananas in #1137.

In the context of docker version and its output, I changed the domain language to use server instead of daemon because this is the term used there (bounded context). daemon is better suited for local calls whereas server is more appropriate for (potentially) remote calls.

Edit: The list of valid os/arch values was taken from validOSArches in util.go. A comment was left as a reminder to keep both files in sync.

This preapares bash completion for more context sensitivity:

- experimental cli features
- orchestrator specific features

Also renames _daemon_ to _server_ where used in context of `docker version`
because the fields there are grouped unter _Server_.

Signed-off-by: Harald Albers <github@albersweb.de>
(cherry picked from commit 564d4da)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This is needed for implementing bash completion for the `docker manifest`
command family.

Signed-off-by: Harald Albers <github@albersweb.de>
(cherry picked from commit a183c95)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Harald Albers <github@albersweb.de>
(cherry picked from commit 0fb4256)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-io
Copy link

Codecov Report

Merging #1542 into 18.09 will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##            18.09    #1542   +/-   ##
=======================================
  Coverage   54.06%   54.06%           
=======================================
  Files         290      290           
  Lines       19403    19403           
=======================================
  Hits        10490    10490           
  Misses       8241     8241           
  Partials      672      672

Copy link
Collaborator

@albers albers left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah
Copy link
Member Author

Thanks for reviewing 🤗

@vdemeester @andrewhsu PTAL

Copy link
Collaborator

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

Copy link
Contributor

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewhsu andrewhsu merged commit 12834ee into docker:18.09 Dec 3, 2018
@thaJeztah thaJeztah deleted the 18.09_backport_completion_cli_experimental branch December 3, 2018 22:26
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.

6 participants