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.

Sign up for GitHub

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

Jump to bottom

Add a "top-level" annotation to hide persistent flags #1106

Merged
merged 1 commit into from
Jun 1, 2018
Merged

Add a "top-level" annotation to hide persistent flags #1106

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions cmd/docker/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func newDockerCommand(dockerCli *command.DockerCli) *cobra.Command {
// Install persistent flags
persistentFlags := cmd.PersistentFlags()
persistentFlags.StringVar(&opts.Common.Orchestrator, "orchestrator", "", "Orchestrator to use (swarm|kubernetes|all)")
persistentFlags.SetAnnotation("orchestrator", "top-level", []string{"version", "stack"})

setFlagErrorFunc(dockerCli, cmd, flags, opts)

Expand Down Expand Up @@ -245,6 +246,12 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) {
if !isOSTypeSupported(f, osType) || !isVersionSupported(f, clientVersion) {
f.Hidden = true
}
// root command shows all top-level flags
if cmd.Parent() != nil {
Copy link
Member

Choose a reason for hiding this comment

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

I take it the default is false for f.Hidden, therefore --orchestrator is displayed by docker --help (i.e. when cmd.Parent() is nil), is this what we want?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think that's the desired behavior, it's possible to set it as global flag (i.e., docker --orchestrator=foo stack deploy), but when showing help for (e.g.) docker stack deploy we show it as a flag for that command, so: docker stack deploy --orchestrator=foo

Is that what you meant?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I find it a bit odd that it can be set as a global flag given that it then has no effect and is ignored for most of the sub-commands, however it's out of the scope of this PR anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Not a big fan of the top level options as well, although they can be handy for making shell aliases (alias dockerk -> docker --orchestrator=foo --host=bla)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep that's what we want, so "top level" flags are shown in the root command help (docker --help), hidden in all sub commands help but still available, excepting the one declared in the annotations.

if commands, ok := f.Annotations["top-level"]; ok {
f.Hidden = !findCommand(cmd, commands)
}
}
})

for _, subcmd := range cmd.Commands() {
Expand All @@ -259,6 +266,19 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) {
}
}

// Checks if a command or one of its ancestors is in the list
func findCommand(cmd *cobra.Command, commands []string) bool {
if cmd == nil {
return false
}
for _, c := range commands {
if c == cmd.Name() {
return true
}
}
return findCommand(cmd.Parent(), commands)
}

func isSupported(cmd *cobra.Command, details versionDetails) error {
if err := areSubcommandsSupported(cmd, details); err != nil {
return err
Expand Down