From 85e80973903ad0f073b0102cdd8804ab9d959ee5 Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Sun, 9 Jul 2023 16:09:58 +0900 Subject: [PATCH 01/13] DisableFlagParsing in helpCommand set to true --- command.go | 1 + 1 file changed, 1 insertion(+) diff --git a/command.go b/command.go index 01f7c6f1c..d0a57f643 100644 --- a/command.go +++ b/command.go @@ -1190,6 +1190,7 @@ func (c *Command) InitDefaultHelpCmd() { Short: "Help about any command", Long: `Help provides help for any command in the application. Simply type ` + c.Name() + ` help [path to command] for full details.`, + DisableFlagParsing: true, ValidArgsFunction: func(c *Command, args []string, toComplete string) ([]string, ShellCompDirective) { var completions []string cmd, _, e := c.Root().Find(args) From 43e4ad49f76b7e98b4d6b3c869adca936faf4f06 Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Sun, 9 Jul 2023 16:10:56 +0900 Subject: [PATCH 02/13] DisableFlagParsing in bash, zsh, fish, powershell set to true --- completions.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/completions.go b/completions.go index ee38c4d0b..f8b5167f6 100644 --- a/completions.go +++ b/completions.go @@ -713,6 +713,7 @@ You will need to start a new shell for this setup to take effect. `, c.Root().Name()), Args: NoArgs, DisableFlagsInUseLine: true, + DisableFlagParsing: true, ValidArgsFunction: NoFileCompletions, RunE: func(cmd *Command, args []string) error { return cmd.Root().GenBashCompletionV2(out, !noDesc) @@ -748,8 +749,9 @@ To load completions for every new session, execute once: You will need to start a new shell for this setup to take effect. `, c.Root().Name()), - Args: NoArgs, - ValidArgsFunction: NoFileCompletions, + Args: NoArgs, + DisableFlagParsing: true, + ValidArgsFunction: NoFileCompletions, RunE: func(cmd *Command, args []string) error { if noDesc { return cmd.Root().GenZshCompletionNoDesc(out) @@ -776,8 +778,9 @@ To load completions for every new session, execute once: You will need to start a new shell for this setup to take effect. `, c.Root().Name()), - Args: NoArgs, - ValidArgsFunction: NoFileCompletions, + Args: NoArgs, + DisableFlagParsing: true, + ValidArgsFunction: NoFileCompletions, RunE: func(cmd *Command, args []string) error { return cmd.Root().GenFishCompletion(out, !noDesc) }, @@ -798,8 +801,9 @@ To load completions in your current shell session: To load completions for every new session, add the output of the above command to your powershell profile. `, c.Root().Name()), - Args: NoArgs, - ValidArgsFunction: NoFileCompletions, + Args: NoArgs, + DisableFlagParsing: true, + ValidArgsFunction: NoFileCompletions, RunE: func(cmd *Command, args []string) error { if noDesc { return cmd.Root().GenPowerShellCompletion(out) From 314c9b86c2348847b079ae7e841f876556753062 Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Sun, 9 Jul 2023 16:11:47 +0900 Subject: [PATCH 03/13] add TestHelpCommandExecutedWithPersistentRequiredFlags --- command_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/command_test.go b/command_test.go index 0212f5ae9..ac435e096 100644 --- a/command_test.go +++ b/command_test.go @@ -885,6 +885,18 @@ func TestHelpCommandExecuted(t *testing.T) { checkStringContains(t, output, rootCmd.Long) } +func TestHelpCommandExecutedWithPersistentRequiredFlags(t *testing.T) { + rootCmd := &Command{Use: "root", Run: emptyRun} + rootCmd.PersistentFlags().Bool("foo", false, "") + childCmd := &Command{Use: "child", Run: emptyRun} + rootCmd.AddCommand(childCmd) + assertNoErr(t, rootCmd.MarkPersistentFlagRequired("foo")) + + if _, err := executeCommand(rootCmd, "help"); err != nil { + t.Errorf("unexpected error: %v", err) + } +} + func TestHelpCommandExecutedOnChild(t *testing.T) { rootCmd := &Command{Use: "root", Run: emptyRun} childCmd := &Command{Use: "child", Long: "Long description", Run: emptyRun} From 10b0388a6e2b8097ddc11fa05c1dd717a60c4512 Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Sun, 9 Jul 2023 16:12:28 +0900 Subject: [PATCH 04/13] add test to check required flags will be ignored for bash, zsh, fish, powershell completion --- completions_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/completions_test.go b/completions_test.go index 0588da0f1..c9274a8a9 100644 --- a/completions_test.go +++ b/completions_test.go @@ -2447,6 +2447,17 @@ func TestDefaultCompletionCmd(t *testing.T) { rootCmd.CompletionOptions.HiddenDefaultCmd = false // Remove completion command for the next test removeCompCmd(rootCmd) + + // Test that required flag will be ignored + rootCmd.PersistentFlags().Bool("foo", false, "") + assertNoErr(t, rootCmd.MarkPersistentFlagRequired("foo")) + for _, shell := range []string{"bash", "fish", "powershell", "zsh"} { + if _, err = executeCommand(rootCmd, compCmdName, shell); err != nil { + t.Errorf("Unexpected error: %v", err) + } + } + // Remove completion command for the next test + removeCompCmd(rootCmd) } func TestCompleteCompletion(t *testing.T) { From 4b81cd58b291b854423c33b0633a658872c7e9e8 Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Tue, 11 Jul 2023 00:02:09 +0900 Subject: [PATCH 05/13] add PersistentPreRun for help command Disable any persistent required flags for the help command --- command.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/command.go b/command.go index d0a57f643..0de3ec802 100644 --- a/command.go +++ b/command.go @@ -1190,7 +1190,6 @@ func (c *Command) InitDefaultHelpCmd() { Short: "Help about any command", Long: `Help provides help for any command in the application. Simply type ` + c.Name() + ` help [path to command] for full details.`, - DisableFlagParsing: true, ValidArgsFunction: func(c *Command, args []string, toComplete string) ([]string, ShellCompDirective) { var completions []string cmd, _, e := c.Root().Find(args) @@ -1210,6 +1209,15 @@ Simply type ` + c.Name() + ` help [path to command] for full details.`, } return completions, ShellCompDirectiveNoFileComp }, + PersistentPreRun: func(cmd *Command, args []string) { + cmd.Flags().VisitAll(func(pflag *flag.Flag) { + requiredAnnotation, found := pflag.Annotations[BashCompOneRequiredFlag] + if found && requiredAnnotation[0] == "true" { + // Disable any persistent required flags for the help command + pflag.Annotations[BashCompOneRequiredFlag] = []string{"false"} + } + }) + }, Run: func(c *Command, args []string) { cmd, _, e := c.Root().Find(args) if cmd == nil || e != nil { From 94a3cb14391954971fd697497a7e8d1515bffda3 Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Tue, 11 Jul 2023 00:05:27 +0900 Subject: [PATCH 06/13] add PersistentPreRun for completion command disable any persistent required flags for the completion command --- completions.go | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/completions.go b/completions.go index f8b5167f6..92e17f686 100644 --- a/completions.go +++ b/completions.go @@ -681,6 +681,15 @@ See each sub-command's help for details on how to use the generated script. ValidArgsFunction: NoFileCompletions, Hidden: c.CompletionOptions.HiddenDefaultCmd, GroupID: c.completionCommandGroupID, + PersistentPreRun: func(cmd *Command, args []string) { + cmd.Flags().VisitAll(func(flag *pflag.Flag) { + requiredAnnotation, found := flag.Annotations[BashCompOneRequiredFlag] + if found && requiredAnnotation[0] == "true" { + // Disable any persistent required flags for the completion command + flag.Annotations[BashCompOneRequiredFlag] = []string{"false"} + } + }) + }, } c.AddCommand(completionCmd) @@ -713,7 +722,6 @@ You will need to start a new shell for this setup to take effect. `, c.Root().Name()), Args: NoArgs, DisableFlagsInUseLine: true, - DisableFlagParsing: true, ValidArgsFunction: NoFileCompletions, RunE: func(cmd *Command, args []string) error { return cmd.Root().GenBashCompletionV2(out, !noDesc) @@ -749,9 +757,8 @@ To load completions for every new session, execute once: You will need to start a new shell for this setup to take effect. `, c.Root().Name()), - Args: NoArgs, - DisableFlagParsing: true, - ValidArgsFunction: NoFileCompletions, + Args: NoArgs, + ValidArgsFunction: NoFileCompletions, RunE: func(cmd *Command, args []string) error { if noDesc { return cmd.Root().GenZshCompletionNoDesc(out) @@ -778,9 +785,8 @@ To load completions for every new session, execute once: You will need to start a new shell for this setup to take effect. `, c.Root().Name()), - Args: NoArgs, - DisableFlagParsing: true, - ValidArgsFunction: NoFileCompletions, + Args: NoArgs, + ValidArgsFunction: NoFileCompletions, RunE: func(cmd *Command, args []string) error { return cmd.Root().GenFishCompletion(out, !noDesc) }, @@ -801,9 +807,8 @@ To load completions in your current shell session: To load completions for every new session, add the output of the above command to your powershell profile. `, c.Root().Name()), - Args: NoArgs, - DisableFlagParsing: true, - ValidArgsFunction: NoFileCompletions, + Args: NoArgs, + ValidArgsFunction: NoFileCompletions, RunE: func(cmd *Command, args []string) error { if noDesc { return cmd.Root().GenPowerShellCompletion(out) From 2d1eeaf16971bf69cff27d8eb9ee52797f59115d Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Fri, 14 Jul 2023 00:38:08 +0900 Subject: [PATCH 07/13] call Root PersitentPreRun on HelpCommand --- command.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/command.go b/command.go index 0de3ec802..43916321d 100644 --- a/command.go +++ b/command.go @@ -1217,6 +1217,9 @@ Simply type ` + c.Name() + ` help [path to command] for full details.`, pflag.Annotations[BashCompOneRequiredFlag] = []string{"false"} } }) + // Adding PersistentPreRun on sub-commands prevents root's PersistentPreRun from being called. + // So it is intentionally called here. + cmd.Root().PersistentPreRun(cmd.Root(), args) }, Run: func(c *Command, args []string) { cmd, _, e := c.Root().Find(args) From ef1328f8cf91b17683b544369bd1c07841b7bb8e Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Fri, 14 Jul 2023 00:38:29 +0900 Subject: [PATCH 08/13] call Root PersistentPreRun on Completion Command --- completions.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/completions.go b/completions.go index 92e17f686..23d09bc73 100644 --- a/completions.go +++ b/completions.go @@ -689,6 +689,9 @@ See each sub-command's help for details on how to use the generated script. flag.Annotations[BashCompOneRequiredFlag] = []string{"false"} } }) + // Adding PersistentPreRun on sub-commands prevents root's PersistentPreRun from being called. + // So it is intentionally called here. + cmd.Root().PersistentPreRun(cmd.Root(), args) }, } c.AddCommand(completionCmd) From ba4e21df7a7ad8784ccda320fe015618636139a8 Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Fri, 14 Jul 2023 00:39:10 +0900 Subject: [PATCH 09/13] add TestPersistentPreRunHooksForHelpCommand --- command_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/command_test.go b/command_test.go index ac435e096..d333f0712 100644 --- a/command_test.go +++ b/command_test.go @@ -1611,6 +1611,25 @@ func TestPersistentHooks(t *testing.T) { } } +func TestPersistentPreRunHooksForHelpCommand(t *testing.T) { + executed := false + + rootCmd := &Command{ + Use: "root", + PersistentPreRun: func(*Command, []string) { executed = true }, + Run: emptyRun, + } + childCmd := &Command{Use: "child", Run: emptyRun} + rootCmd.AddCommand(childCmd) + + if _, err := executeCommand(rootCmd, "help"); err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !executed { + t.Error("Root PersistentPreRun should have been executed") + } +} + // Related to https://github.com/spf13/cobra/issues/521. func TestGlobalNormFuncPropagation(t *testing.T) { normFunc := func(f *pflag.FlagSet, name string) pflag.NormalizedName { From 3bb59c6775f700c30f31fb2a8741dbd45aa333bf Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Fri, 14 Jul 2023 00:39:35 +0900 Subject: [PATCH 10/13] add TestPersistentPreRunHooksForCompletionCommand --- completions_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/completions_test.go b/completions_test.go index c9274a8a9..75f652cfa 100644 --- a/completions_test.go +++ b/completions_test.go @@ -2460,6 +2460,31 @@ func TestDefaultCompletionCmd(t *testing.T) { removeCompCmd(rootCmd) } +func TestPersistentPreRunHooksForCompletionCommand(t *testing.T) { + executed := false + + rootCmd := &Command{ + Use: "root", + PersistentPreRun: func(*Command, []string) { executed = true }, + Run: emptyRun, + } + subCmd := &Command{ + Use: "sub", + Run: emptyRun, + } + rootCmd.AddCommand(subCmd) + + for _, shell := range []string{"bash", "fish", "powershell", "zsh"} { + if _, err := executeCommand(rootCmd, compCmdName, shell); err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !executed { + t.Error("Root PersistentPreRun should have been executed") + } + executed = false + } +} + func TestCompleteCompletion(t *testing.T) { rootCmd := &Command{Use: "root", Args: NoArgs, Run: emptyRun} subCmd := &Command{ From b370e7b9af77863720df919db204f20097186aaf Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Fri, 14 Jul 2023 23:50:21 +0900 Subject: [PATCH 11/13] fix help command: use PersistentPreRunE instead of PersistentPreRun --- command.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/command.go b/command.go index 43916321d..acb3752dd 100644 --- a/command.go +++ b/command.go @@ -1209,7 +1209,7 @@ Simply type ` + c.Name() + ` help [path to command] for full details.`, } return completions, ShellCompDirectiveNoFileComp }, - PersistentPreRun: func(cmd *Command, args []string) { + PersistentPreRunE: func(cmd *Command, args []string) error { cmd.Flags().VisitAll(func(pflag *flag.Flag) { requiredAnnotation, found := pflag.Annotations[BashCompOneRequiredFlag] if found && requiredAnnotation[0] == "true" { @@ -1219,7 +1219,12 @@ Simply type ` + c.Name() + ` help [path to command] for full details.`, }) // Adding PersistentPreRun on sub-commands prevents root's PersistentPreRun from being called. // So it is intentionally called here. - cmd.Root().PersistentPreRun(cmd.Root(), args) + if cmd.Root().PersistentPreRunE != nil { + return cmd.Root().PersistentPreRunE(cmd, args) + } else if cmd.Root().PersistentPreRun != nil { + cmd.Root().PersistentPreRun(cmd, args) + } + return nil }, Run: func(c *Command, args []string) { cmd, _, e := c.Root().Find(args) From 46a2ba8c4179e52e76c08a3fab78af888bdf38a6 Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Fri, 14 Jul 2023 23:50:49 +0900 Subject: [PATCH 12/13] fix completion command: use PersistentPreRunE instead of PersistentPreRun --- completions.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/completions.go b/completions.go index 23d09bc73..58bc770a2 100644 --- a/completions.go +++ b/completions.go @@ -681,7 +681,7 @@ See each sub-command's help for details on how to use the generated script. ValidArgsFunction: NoFileCompletions, Hidden: c.CompletionOptions.HiddenDefaultCmd, GroupID: c.completionCommandGroupID, - PersistentPreRun: func(cmd *Command, args []string) { + PersistentPreRunE: func(cmd *Command, args []string) error { cmd.Flags().VisitAll(func(flag *pflag.Flag) { requiredAnnotation, found := flag.Annotations[BashCompOneRequiredFlag] if found && requiredAnnotation[0] == "true" { @@ -691,7 +691,12 @@ See each sub-command's help for details on how to use the generated script. }) // Adding PersistentPreRun on sub-commands prevents root's PersistentPreRun from being called. // So it is intentionally called here. - cmd.Root().PersistentPreRun(cmd.Root(), args) + if cmd.Root().PersistentPreRunE != nil { + return cmd.Root().PersistentPreRunE(cmd, args) + } else if cmd.Root().PersistentPreRun != nil { + cmd.Root().PersistentPreRun(cmd, args) + } + return nil }, } c.AddCommand(completionCmd) From f3125ff6fa42a0351c4c2b6871d79e40660e3cf1 Mon Sep 17 00:00:00 2001 From: Jun Nishimura Date: Sat, 15 Jul 2023 00:56:13 +0900 Subject: [PATCH 13/13] define trueString constant --- command.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/command.go b/command.go index acb3752dd..e426810f8 100644 --- a/command.go +++ b/command.go @@ -30,7 +30,10 @@ import ( flag "github.com/spf13/pflag" ) -const FlagSetByCobraAnnotation = "cobra_annotation_flag_set_by_cobra" +const ( + FlagSetByCobraAnnotation = "cobra_annotation_flag_set_by_cobra" + trueString = "true" +) // FParseErrWhitelist configures Flag parse errors to be ignored type FParseErrWhitelist flag.ParseErrorsWhitelist @@ -1109,7 +1112,7 @@ func (c *Command) ValidateRequiredFlags() error { if !found { return } - if (requiredAnnotation[0] == "true") && !pflag.Changed { + if (requiredAnnotation[0] == trueString) && !pflag.Changed { missingFlagNames = append(missingFlagNames, pflag.Name) } }) @@ -1146,7 +1149,7 @@ func (c *Command) InitDefaultHelpFlag() { usage += c.Name() } c.Flags().BoolP("help", "h", false, usage) - _ = c.Flags().SetAnnotation("help", FlagSetByCobraAnnotation, []string{"true"}) + _ = c.Flags().SetAnnotation("help", FlagSetByCobraAnnotation, []string{trueString}) } } @@ -1172,7 +1175,7 @@ func (c *Command) InitDefaultVersionFlag() { } else { c.Flags().Bool("version", false, usage) } - _ = c.Flags().SetAnnotation("version", FlagSetByCobraAnnotation, []string{"true"}) + _ = c.Flags().SetAnnotation("version", FlagSetByCobraAnnotation, []string{trueString}) } } @@ -1212,7 +1215,7 @@ Simply type ` + c.Name() + ` help [path to command] for full details.`, PersistentPreRunE: func(cmd *Command, args []string) error { cmd.Flags().VisitAll(func(pflag *flag.Flag) { requiredAnnotation, found := pflag.Annotations[BashCompOneRequiredFlag] - if found && requiredAnnotation[0] == "true" { + if found && requiredAnnotation[0] == trueString { // Disable any persistent required flags for the help command pflag.Annotations[BashCompOneRequiredFlag] = []string{"false"} }