Skip to content

docker swarm init: fix help printing #4403

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

Open
hernandanielg wants to merge 2 commits into
base: master
Choose a base branch
from
+4 −1
Open

docker swarm init: fix help printing #4403

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e01bda
prevent `tryRunPluginHelp()` execution on non-1st-level commands.
Jul 6, 2023
b0cd429
Merge branch 'docker:master' into fix-swarm-init-help
hernandanielg Apr 19, 2024
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
5 changes: 4 additions & 1 deletion cmd/docker/docker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,10 @@ func setHelpFunc(dockerCli command.Cli, cmd *cobra.Command) {
return
}

if len(args) >= 1 {
// commands are chained in the form `rootCmd 1stCmd 2ndCmd...`
// if RootCmd and ParentCmd are the same
// then my command is 1st level command
if len(args) >= 1 && ccmd.Root() == ccmd.Parent() {
Copy link
Copy Markdown
Collaborator

@vvoland vvoland Jul 13, 2023

Choose a reason for hiding this comment

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

In my case, Parent is always nil, so this check is never true, making it effectively if false {

I haven't fully followed the whole code yet, but it looks to me like we can just explicitly delete that whole branch:

docker <plugin> ... --help will be forwarded to the plugin cli directly so it won't even be handled here.

docker help <plugin> is already handled by setupHelpCommand

Or are there any other ways to "call for help" that I'm not aware of?

Copy link
Copy Markdown
Contributor Author

@hernandanielg hernandanielg Jul 13, 2023

Choose a reason for hiding this comment

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

I think 2nd condition (the one I added) makes sense only if len(args) is > 1 , situations like docker foo bar so bar won't be considered to check if it's a plugin... but yeah I agree with you, maybe we can remove the whole thing, I'll make some tests and modify this PR (bear with me)

err := tryRunPluginHelp(dockerCli, ccmd, args)
if err == nil {
return
Expand Down
Loading