Skip to content

cli: fix builder persistent flag#818

Merged
tonistiigi merged 1 commit intodocker:masterfrom
crazy-max:fix-builder-flag
Nov 4, 2021
Merged

cli: fix builder persistent flag#818
tonistiigi merged 1 commit intodocker:masterfrom
crazy-max:fix-builder-flag

Conversation

@crazy-max
Copy link
Copy Markdown
Member

--builder flag should not be persistent for all subcommands.

Before

$ docker buildx version --help

Usage:  docker buildx version

Show buildx version information

Options:
      --builder string   Override the configured builder instance

Now

$ docker buildx version --help

Usage:  docker buildx version

Show buildx version information

Signed-off-by: CrazyMax [email protected]

@tonistiigi
Copy link
Copy Markdown
Member

It is somewhat nice though that you can do buildx="buildx --builder=foo" and then $buildx build or any other command.

@crazy-max
Copy link
Copy Markdown
Member Author

crazy-max commented Nov 2, 2021

will instead hide the builder flags for subcommand that don't need it as discussed.

Comment thread go.mod Outdated
Comment on lines +8 to +12
// we could use cmd.SetHelpFunc to override the helper
// but, it's not enough because we also want the generated
// docs to be updated, so we override the flag instead
cmd.Flags().String(h, "", "")
_ = cmd.Flags().MarkHidden(h)
Copy link
Copy Markdown
Member Author

@crazy-max crazy-max Nov 3, 2021

Choose a reason for hiding this comment

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

would have preferred to use SetHelpFunc like:

cmd.SetHelpFunc(func(ccmd *cobra.Command, args []string) {
	for _, h := range hidden {
		_ = ccmd.Flags().MarkHidden(h)
	}
	cmd.SetHelpFunc(nil)
	cmd.HelpFunc()(ccmd, args)
})

but it cannot be handled for docs generation unfortunately.

@crazy-max crazy-max marked this pull request as draft November 3, 2021 22:05
Comment thread commands/imagetools/inspect.go Outdated

_ = flags
// hide builder persistent flag for this command
cobrautil.HideInheritedFlags(cmd, "builder")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I found an use-case for this in #825

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

rebased since latest changes

@crazy-max crazy-max marked this pull request as ready for review November 4, 2021 16:32
@tonistiigi tonistiigi merged commit 5c4e3fc into docker:master Nov 4, 2021
@crazy-max crazy-max deleted the fix-builder-flag branch November 5, 2021 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants