diff --git a/.github/labeler.yml b/.github/labeler.yml index 0f0bc3c9..0db3be27 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -1,17 +1,24 @@ # changes to documentation generation -"area/docs-generation": doc/**/* +"area/docs-generation": +- changed-files: + - any-glob-to-any-file: 'doc/**' # changes to the core cobra command "area/cobra-command": -- any: ['./cobra.go', './cobra_test.go', './*command*.go'] +- changed-files: + - any-glob-to-any-file: ['./cobra.go', './cobra_test.go', './*command*.go'] # changes made to command flags/args -"area/flags": ./args*.go +"area/flags": +- changed-files: + - any-glob-to-any-file: './args*.go' # changes to Github workflows -"area/github": .github/**/* +"area/github": +- changed-files: + - any-glob-to-any-file: '.github/**' # changes to shell completions "area/shell-completion": - - ./*completions* - +- changed-files: + - any-glob-to-any-file: './*completions*' diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml index 17f451fd..cc5c5400 100644 --- a/.github/workflows/labeler.yml +++ b/.github/workflows/labeler.yml @@ -12,7 +12,7 @@ jobs: pull-requests: write # for actions/labeler to add labels to PRs runs-on: ubuntu-latest steps: - - uses: actions/labeler@v4 + - uses: actions/labeler@v5 with: repo-token: "${{ github.token }}" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 498e3bec..21f81e0c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,7 +18,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - run: >- docker run @@ -39,17 +39,15 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - - uses: actions/setup-go@v3 + - uses: actions/setup-go@v5 with: - go-version: '^1.20' + go-version: '^1.22' check-latest: true cache: true - - uses: actions/checkout@v3 - - - uses: golangci/golangci-lint-action@v3.6.0 + - uses: golangci/golangci-lint-action@v4.0.0 with: version: latest args: --verbose @@ -67,13 +65,15 @@ jobs: - 18 - 19 - 20 + - 21 + - 22 name: '${{ matrix.platform }} | 1.${{ matrix.go }}.x' runs-on: ${{ matrix.platform }}-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - - uses: actions/setup-go@v3 + - uses: actions/setup-go@v5 with: go-version: 1.${{ matrix.go }}.x cache: true @@ -107,9 +107,9 @@ jobs: unzip mingw-w64-x86_64-go - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - - uses: actions/cache@v3 + - uses: actions/cache@v4 with: path: ~/go/pkg/mod key: ${{ runner.os }}-${{ matrix.go }}-${{ hashFiles('**/go.sum') }} diff --git a/.golangci.yml b/.golangci.yml index 2578d94b..7b6d3021 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,7 +19,7 @@ linters: disable-all: true enable: #- bodyclose - - deadcode + # - deadcode ! deprecated since v1.49.0; replaced by 'unused' #- depguard #- dogsled #- dupl @@ -29,20 +29,17 @@ linters: - gas #- gochecknoinits - goconst - #- gocritic + - gocritic #- gocyclo #- gofmt - goimports - - golint #- gomnd #- goprintffuncname #- gosec - #- gosimple + - gosimple - govet - ineffassign - - interfacer #- lll - - maligned - megacheck #- misspell #- nakedret @@ -51,12 +48,12 @@ linters: #- rowserrcheck #- scopelint #- staticcheck - - structcheck - #- stylecheck + #- structcheck ! deprecated since v1.49.0; replaced by 'unused' + - stylecheck #- typecheck - unconvert #- unparam - #- unused - - varcheck + - unused + # - varcheck ! deprecated since v1.49.0; replaced by 'unused' #- whitespace fast: false diff --git a/active_help.go b/active_help.go index 2d023943..25c30e3c 100644 --- a/active_help.go +++ b/active_help.go @@ -17,15 +17,14 @@ package cobra import ( "fmt" "os" - "strings" ) const ( activeHelpMarker = "_activeHelp_ " // The below values should not be changed: programs will be using them explicitly // in their user documentation, and users will be using them explicitly. - activeHelpEnvVarSuffix = "_ACTIVE_HELP" - activeHelpGlobalEnvVar = "COBRA_ACTIVE_HELP" + activeHelpEnvVarSuffix = "ACTIVE_HELP" + activeHelpGlobalEnvVar = configEnvVarGlobalPrefix + "_" + activeHelpEnvVarSuffix activeHelpGlobalDisable = "0" ) @@ -42,7 +41,7 @@ func AppendActiveHelp(compArray []string, activeHelpStr string) []string { // GetActiveHelpConfig returns the value of the ActiveHelp environment variable // _ACTIVE_HELP where is the name of the root command in upper -// case, with all - replaced by _. +// case, with all non-ASCII-alphanumeric characters replaced by `_`. // It will always return "0" if the global environment variable COBRA_ACTIVE_HELP // is set to "0". func GetActiveHelpConfig(cmd *Command) string { @@ -55,9 +54,7 @@ func GetActiveHelpConfig(cmd *Command) string { // activeHelpEnvVar returns the name of the program-specific ActiveHelp environment // variable. It has the format _ACTIVE_HELP where is the name of the -// root command in upper case, with all - replaced by _. +// root command in upper case, with all non-ASCII-alphanumeric characters replaced by `_`. func activeHelpEnvVar(name string) string { - // This format should not be changed: users will be using it explicitly. - activeHelpEnvVar := strings.ToUpper(fmt.Sprintf("%s%s", name, activeHelpEnvVarSuffix)) - return strings.ReplaceAll(activeHelpEnvVar, "-", "_") + return configEnvVar(name, activeHelpEnvVarSuffix) } diff --git a/args.go b/args.go index e79ec33a..ed1e70ce 100644 --- a/args.go +++ b/args.go @@ -52,9 +52,9 @@ func OnlyValidArgs(cmd *Command, args []string) error { if len(cmd.ValidArgs) > 0 { // Remove any description that may be included in ValidArgs. // A description is following a tab character. - var validArgs []string + validArgs := make([]string, 0, len(cmd.ValidArgs)) for _, v := range cmd.ValidArgs { - validArgs = append(validArgs, strings.Split(v, "\t")[0]) + validArgs = append(validArgs, strings.SplitN(v, "\t", 2)[0]) } for _, v := range args { if !stringInSlice(v, validArgs) { diff --git a/bash_completions.go b/bash_completions.go index 8a531518..f4d198cb 100644 --- a/bash_completions.go +++ b/bash_completions.go @@ -597,19 +597,16 @@ func writeRequiredFlag(buf io.StringWriter, cmd *Command) { if nonCompletableFlag(flag) { return } - for key := range flag.Annotations { - switch key { - case BashCompOneRequiredFlag: - format := " must_have_one_flag+=(\"--%s" - if flag.Value.Type() != "bool" { - format += "=" - } - format += cbn - WriteStringAndCheck(buf, fmt.Sprintf(format, flag.Name)) + if _, ok := flag.Annotations[BashCompOneRequiredFlag]; ok { + format := " must_have_one_flag+=(\"--%s" + if flag.Value.Type() != "bool" { + format += "=" + } + format += cbn + WriteStringAndCheck(buf, fmt.Sprintf(format, flag.Name)) - if len(flag.Shorthand) > 0 { - WriteStringAndCheck(buf, fmt.Sprintf(" must_have_one_flag+=(\"-%s"+cbn, flag.Shorthand)) - } + if len(flag.Shorthand) > 0 { + WriteStringAndCheck(buf, fmt.Sprintf(" must_have_one_flag+=(\"-%s"+cbn, flag.Shorthand)) } } }) @@ -621,7 +618,7 @@ func writeRequiredNouns(buf io.StringWriter, cmd *Command) { for _, value := range cmd.ValidArgs { // Remove any description that may be included following a tab character. // Descriptions are not supported by bash completion. - value = strings.Split(value, "\t")[0] + value = strings.SplitN(value, "\t", 2)[0] WriteStringAndCheck(buf, fmt.Sprintf(" must_have_one_noun+=(%q)\n", value)) } if cmd.ValidArgsFunction != nil { diff --git a/cobra.go b/cobra.go index f23f5092..e0b0947b 100644 --- a/cobra.go +++ b/cobra.go @@ -43,9 +43,10 @@ var initializers []func() var finalizers []func() const ( - defaultPrefixMatching = false - defaultCommandSorting = true - defaultCaseInsensitive = false + defaultPrefixMatching = false + defaultCommandSorting = true + defaultCaseInsensitive = false + defaultTraverseRunHooks = false ) // EnablePrefixMatching allows setting automatic prefix matching. Automatic prefix matching can be a dangerous thing @@ -60,6 +61,10 @@ var EnableCommandSorting = defaultCommandSorting // EnableCaseInsensitive allows case-insensitive commands names. (case sensitive by default) var EnableCaseInsensitive = defaultCaseInsensitive +// EnableTraverseRunHooks executes persistent pre-run and post-run hooks from all parents. +// By default this is disabled, which means only the first run hook to be found is executed. +var EnableTraverseRunHooks = defaultTraverseRunHooks + // MousetrapHelpText enables an information splash screen on Windows // if the CLI is started from explorer.exe. // To disable the mousetrap, just set this variable to blank string (""). @@ -188,8 +193,6 @@ func ld(s, t string, ignoreCase bool) int { d := make([][]int, len(s)+1) for i := range d { d[i] = make([]int, len(t)+1) - } - for i := range d { d[i][0] = i } for j := range d[0] { diff --git a/cobra_test.go b/cobra_test.go index fbb07f9b..2bba461c 100644 --- a/cobra_test.go +++ b/cobra_test.go @@ -40,3 +40,185 @@ func TestAddTemplateFunctions(t *testing.T) { t.Errorf("Expected UsageString: %v\nGot: %v", expected, got) } } + +func TestLevenshteinDistance(t *testing.T) { + tests := []struct { + name string + s string + t string + ignoreCase bool + expected int + }{ + { + name: "Equal strings (case-sensitive)", + s: "hello", + t: "hello", + ignoreCase: false, + expected: 0, + }, + { + name: "Equal strings (case-insensitive)", + s: "Hello", + t: "hello", + ignoreCase: true, + expected: 0, + }, + { + name: "Different strings (case-sensitive)", + s: "kitten", + t: "sitting", + ignoreCase: false, + expected: 3, + }, + { + name: "Different strings (case-insensitive)", + s: "Kitten", + t: "Sitting", + ignoreCase: true, + expected: 3, + }, + { + name: "Empty strings", + s: "", + t: "", + ignoreCase: false, + expected: 0, + }, + { + name: "One empty string", + s: "abc", + t: "", + ignoreCase: false, + expected: 3, + }, + { + name: "Both empty strings", + s: "", + t: "", + ignoreCase: true, + expected: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Act + got := ld(tt.s, tt.t, tt.ignoreCase) + + // Assert + if got != tt.expected { + t.Errorf("Expected ld: %v\nGot: %v", tt.expected, got) + } + }) + } +} + +func TestStringInSlice(t *testing.T) { + tests := []struct { + name string + a string + list []string + expected bool + }{ + { + name: "String in slice (case-sensitive)", + a: "apple", + list: []string{"orange", "banana", "apple", "grape"}, + expected: true, + }, + { + name: "String not in slice (case-sensitive)", + a: "pear", + list: []string{"orange", "banana", "apple", "grape"}, + expected: false, + }, + { + name: "String in slice (case-insensitive)", + a: "APPLE", + list: []string{"orange", "banana", "apple", "grape"}, + expected: false, + }, + { + name: "Empty slice", + a: "apple", + list: []string{}, + expected: false, + }, + { + name: "Empty string", + a: "", + list: []string{"orange", "banana", "apple", "grape"}, + expected: false, + }, + { + name: "Empty strings match", + a: "", + list: []string{"orange", ""}, + expected: true, + }, + { + name: "Empty string in empty slice", + a: "", + list: []string{}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Act + got := stringInSlice(tt.a, tt.list) + + // Assert + if got != tt.expected { + t.Errorf("Expected stringInSlice: %v\nGot: %v", tt.expected, got) + } + }) + } +} + +func TestRpad(t *testing.T) { + tests := []struct { + name string + inputString string + padding int + expected string + }{ + { + name: "Padding required", + inputString: "Hello", + padding: 10, + expected: "Hello ", + }, + { + name: "No padding required", + inputString: "World", + padding: 5, + expected: "World", + }, + { + name: "Empty string", + inputString: "", + padding: 8, + expected: " ", + }, + { + name: "Zero padding", + inputString: "cobra", + padding: 0, + expected: "cobra", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Act + got := rpad(tt.inputString, tt.padding) + + // Assert + if got != tt.expected { + t.Errorf("Expected rpad: %v\nGot: %v", tt.expected, got) + } + }) + } +} diff --git a/command.go b/command.go index e426810f..0419cb15 100644 --- a/command.go +++ b/command.go @@ -31,8 +31,9 @@ import ( ) const ( - FlagSetByCobraAnnotation = "cobra_annotation_flag_set_by_cobra" - trueString = "true" + FlagSetByCobraAnnotation = "cobra_annotation_flag_set_by_cobra" + CommandDisplayNameAnnotation = "cobra_annotation_command_display_name" + trueString = "true" ) // FParseErrWhitelist configures Flag parse errors to be ignored @@ -102,7 +103,7 @@ type Command struct { Deprecated string // Annotations are key/value pairs that can be used by applications to identify or - // group commands. + // group commands or set special options. Annotations map[string]string // Version defines the version for this command. If this value is non-empty and the command does not @@ -118,6 +119,8 @@ type Command struct { // * PostRun() // * PersistentPostRun() // All functions get the same args, the arguments after the command name. + // The *PreRun and *PostRun functions will only be executed if the Run function of the current + // command has been declared. // // PersistentPreRun: children of this command will inherit and execute. PersistentPreRun func(cmd *Command, args []string) @@ -152,8 +155,10 @@ type Command struct { // pflags contains persistent flags. pflags *flag.FlagSet // lflags contains local flags. + // This field does not represent internal state, it's used as a cache to optimise LocalFlags function call lflags *flag.FlagSet // iflags contains inherited flags. + // This field does not represent internal state, it's used as a cache to optimise InheritedFlags function call iflags *flag.FlagSet // parentsPflags is all persistent flags of cmd's parents. parentsPflags *flag.FlagSet @@ -184,6 +189,9 @@ type Command struct { // versionTemplate is the version template defined by user. versionTemplate string + // errPrefix is the error message prefix defined by user. + errPrefix string + // inReader is a reader defined by the user that replaces stdin inReader io.Reader // outWriter is a writer defined by the user that replaces stdout @@ -349,6 +357,11 @@ func (c *Command) SetVersionTemplate(s string) { c.versionTemplate = s } +// SetErrPrefix sets error message prefix to be used. Application can use it to set custom prefix. +func (c *Command) SetErrPrefix(s string) { + c.errPrefix = s +} + // SetGlobalNormalizationFunc sets a normalization function to all flag sets and also to child commands. // The user should not have a cyclic dependency on commands. func (c *Command) SetGlobalNormalizationFunc(n func(f *flag.FlagSet, name string) flag.NormalizedName) { @@ -598,6 +611,18 @@ func (c *Command) VersionTemplate() string { ` } +// ErrPrefix return error message prefix for the command +func (c *Command) ErrPrefix() string { + if c.errPrefix != "" { + return c.errPrefix + } + + if c.HasParent() { + return c.parent.ErrPrefix() + } + return "Error:" +} + func hasNoOptDefVal(name string, fs *flag.FlagSet) bool { flag := fs.Lookup(name) if flag == nil { @@ -684,7 +709,7 @@ Loop: // This is not a flag or a flag value. Check to see if it matches what we're looking for, and if so, // return the args, excluding the one at this position. if s == x { - ret := []string{} + ret := make([]string, 0, len(args)-1) ret = append(ret, args[:pos]...) ret = append(ret, args[pos+1:]...) return ret @@ -732,14 +757,14 @@ func (c *Command) findSuggestions(arg string) string { if c.SuggestionsMinimumDistance <= 0 { c.SuggestionsMinimumDistance = 2 } - suggestionsString := "" + var sb strings.Builder if suggestions := c.SuggestionsFor(arg); len(suggestions) > 0 { - suggestionsString += "\n\nDid you mean this?\n" + sb.WriteString("\n\nDid you mean this?\n") for _, s := range suggestions { - suggestionsString += fmt.Sprintf("\t%v\n", s) + _, _ = fmt.Fprintf(&sb, "\t%v\n", s) } } - return suggestionsString + return sb.String() } func (c *Command) findNext(next string) *Command { @@ -755,7 +780,9 @@ func (c *Command) findNext(next string) *Command { } if len(matches) == 1 { - return matches[0] + // Temporarily disable gosec G602, which produces a false positive. + // See https://github.com/securego/gosec/issues/1005. + return matches[0] // #nosec G602 } return nil @@ -849,7 +876,7 @@ func (c *Command) ArgsLenAtDash() int { func (c *Command) execute(a []string) (err error) { if c == nil { - return fmt.Errorf("Called Execute() on a nil Command") + return fmt.Errorf("called Execute() on a nil Command") } if len(c.Deprecated) > 0 { @@ -913,15 +940,31 @@ func (c *Command) execute(a []string) (err error) { return err } + parents := make([]*Command, 0, 5) for p := c; p != nil; p = p.Parent() { + if EnableTraverseRunHooks { + // When EnableTraverseRunHooks is set: + // - Execute all persistent pre-runs from the root parent till this command. + // - Execute all persistent post-runs from this command till the root parent. + parents = append([]*Command{p}, parents...) + } else { + // Otherwise, execute only the first found persistent hook. + parents = append(parents, p) + } + } + for _, p := range parents { if p.PersistentPreRunE != nil { if err := p.PersistentPreRunE(c, argWoFlags); err != nil { return err } - break + if !EnableTraverseRunHooks { + break + } } else if p.PersistentPreRun != nil { p.PersistentPreRun(c, argWoFlags) - break + if !EnableTraverseRunHooks { + break + } } } if c.PreRunE != nil { @@ -958,10 +1001,14 @@ func (c *Command) execute(a []string) (err error) { if err := p.PersistentPostRunE(c, argWoFlags); err != nil { return err } - break + if !EnableTraverseRunHooks { + break + } } else if p.PersistentPostRun != nil { p.PersistentPostRun(c, argWoFlags) - break + if !EnableTraverseRunHooks { + break + } } } @@ -1051,7 +1098,7 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { c = cmd } if !c.SilenceErrors { - c.PrintErrln("Error:", err.Error()) + c.PrintErrln(c.ErrPrefix(), err.Error()) c.PrintErrf("Run '%v --help' for usage.\n", c.CommandPath()) } return c, err @@ -1080,7 +1127,7 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { // If root command has SilenceErrors flagged, // all subcommands should respect it if !cmd.SilenceErrors && !c.SilenceErrors { - c.PrintErrln("Error:", err.Error()) + c.PrintErrln(cmd.ErrPrefix(), err.Error()) } // If root command has SilenceUsage flagged, @@ -1143,10 +1190,11 @@ func (c *Command) InitDefaultHelpFlag() { c.mergePersistentFlags() if c.Flags().Lookup("help") == nil { usage := "help for " - if c.Name() == "" { + name := c.displayName() + if name == "" { usage += "this command" } else { - usage += c.Name() + usage += name } c.Flags().BoolP("help", "h", false, usage) _ = c.Flags().SetAnnotation("help", FlagSetByCobraAnnotation, []string{trueString}) @@ -1192,7 +1240,7 @@ func (c *Command) InitDefaultHelpCmd() { Use: "help [command]", 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.`, +Simply type ` + c.displayName() + ` help [path to command] for full details.`, ValidArgsFunction: func(c *Command, args []string, toComplete string) ([]string, ShellCompDirective) { var completions []string cmd, _, e := c.Root().Find(args) @@ -1400,16 +1448,24 @@ func (c *Command) CommandPath() string { if c.HasParent() { return c.Parent().CommandPath() + " " + c.Name() } + return c.displayName() +} + +func (c *Command) displayName() string { + if displayName, ok := c.Annotations[CommandDisplayNameAnnotation]; ok { + return displayName + } return c.Name() } // UseLine puts out the full usage for a given command (including parents). func (c *Command) UseLine() string { var useline string + use := strings.Replace(c.Use, c.Name(), c.displayName(), 1) if c.HasParent() { - useline = c.parent.CommandPath() + " " + c.Use + useline = c.parent.CommandPath() + " " + use } else { - useline = c.Use + useline = use } if c.DisableFlagsInUseLine { return useline @@ -1422,6 +1478,7 @@ func (c *Command) UseLine() string { // DebugFlags used to determine which flags have been assigned to which commands // and which persist. +// nolint:goconst func (c *Command) DebugFlags() { c.Println("DebugFlags called on", c.Name()) var debugflags func(*Command) @@ -1611,7 +1668,7 @@ func (c *Command) GlobalNormalizationFunc() func(f *flag.FlagSet, name string) f // to this command (local and persistent declared here and by all parents). func (c *Command) Flags() *flag.FlagSet { if c.flags == nil { - c.flags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.flags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1622,10 +1679,11 @@ func (c *Command) Flags() *flag.FlagSet { } // LocalNonPersistentFlags are flags specific to this command which will NOT persist to subcommands. +// This function does not modify the flags of the current command, it's purpose is to return the current state. func (c *Command) LocalNonPersistentFlags() *flag.FlagSet { persistentFlags := c.PersistentFlags() - out := flag.NewFlagSet(c.Name(), flag.ContinueOnError) + out := flag.NewFlagSet(c.displayName(), flag.ContinueOnError) c.LocalFlags().VisitAll(func(f *flag.Flag) { if persistentFlags.Lookup(f.Name) == nil { out.AddFlag(f) @@ -1635,11 +1693,12 @@ func (c *Command) LocalNonPersistentFlags() *flag.FlagSet { } // LocalFlags returns the local FlagSet specifically set in the current command. +// This function does not modify the flags of the current command, it's purpose is to return the current state. func (c *Command) LocalFlags() *flag.FlagSet { c.mergePersistentFlags() if c.lflags == nil { - c.lflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.lflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1662,11 +1721,12 @@ func (c *Command) LocalFlags() *flag.FlagSet { } // InheritedFlags returns all flags which were inherited from parent commands. +// This function does not modify the flags of the current command, it's purpose is to return the current state. func (c *Command) InheritedFlags() *flag.FlagSet { c.mergePersistentFlags() if c.iflags == nil { - c.iflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.iflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1687,6 +1747,7 @@ func (c *Command) InheritedFlags() *flag.FlagSet { } // NonInheritedFlags returns all flags which were not inherited from parent commands. +// This function does not modify the flags of the current command, it's purpose is to return the current state. func (c *Command) NonInheritedFlags() *flag.FlagSet { return c.LocalFlags() } @@ -1694,7 +1755,7 @@ func (c *Command) NonInheritedFlags() *flag.FlagSet { // PersistentFlags returns the persistent FlagSet specifically set in the current command. func (c *Command) PersistentFlags() *flag.FlagSet { if c.pflags == nil { - c.pflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.pflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) if c.flagErrorBuf == nil { c.flagErrorBuf = new(bytes.Buffer) } @@ -1707,9 +1768,9 @@ func (c *Command) PersistentFlags() *flag.FlagSet { func (c *Command) ResetFlags() { c.flagErrorBuf = new(bytes.Buffer) c.flagErrorBuf.Reset() - c.flags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.flags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) c.flags.SetOutput(c.flagErrorBuf) - c.pflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.pflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) c.pflags.SetOutput(c.flagErrorBuf) c.lflags = nil @@ -1826,7 +1887,7 @@ func (c *Command) mergePersistentFlags() { // If c.parentsPflags == nil, it makes new. func (c *Command) updateParentsPflags() { if c.parentsPflags == nil { - c.parentsPflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + c.parentsPflags = flag.NewFlagSet(c.displayName(), flag.ContinueOnError) c.parentsPflags.SetOutput(c.flagErrorBuf) c.parentsPflags.SortFlags = false } diff --git a/command_test.go b/command_test.go index d333f071..40a95664 100644 --- a/command_test.go +++ b/command_test.go @@ -18,7 +18,7 @@ import ( "bytes" "context" "fmt" - "io/ioutil" + "io" "os" "reflect" "strings" @@ -366,6 +366,67 @@ func TestAliasPrefixMatching(t *testing.T) { EnablePrefixMatching = defaultPrefixMatching } +// TestPlugin checks usage as plugin for another command such as kubectl. The +// executable is `kubectl-plugin`, but we run it as `kubectl plugin`. The help +// text should reflect the way we run the command. +func TestPlugin(t *testing.T) { + cmd := &Command{ + Use: "kubectl-plugin", + Args: NoArgs, + Annotations: map[string]string{ + CommandDisplayNameAnnotation: "kubectl plugin", + }, + Run: emptyRun, + } + + cmdHelp, err := executeCommand(cmd, "-h") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + checkStringContains(t, cmdHelp, "kubectl plugin [flags]") + checkStringContains(t, cmdHelp, "help for kubectl plugin") +} + +// TestPlugin checks usage as plugin with sub commands. +func TestPluginWithSubCommands(t *testing.T) { + rootCmd := &Command{ + Use: "kubectl-plugin", + Args: NoArgs, + Annotations: map[string]string{ + CommandDisplayNameAnnotation: "kubectl plugin", + }, + } + + subCmd := &Command{Use: "sub [flags]", Args: NoArgs, Run: emptyRun} + rootCmd.AddCommand(subCmd) + + rootHelp, err := executeCommand(rootCmd, "-h") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + checkStringContains(t, rootHelp, "kubectl plugin [command]") + checkStringContains(t, rootHelp, "help for kubectl plugin") + checkStringContains(t, rootHelp, "kubectl plugin [command] --help") + + childHelp, err := executeCommand(rootCmd, "sub", "-h") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + checkStringContains(t, childHelp, "kubectl plugin sub [flags]") + checkStringContains(t, childHelp, "help for sub") + + helpHelp, err := executeCommand(rootCmd, "help", "-h") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + checkStringContains(t, helpHelp, "kubectl plugin help [path to command]") + checkStringContains(t, helpHelp, "kubectl plugin help [command]") +} + // TestChildSameName checks the correct behaviour of cobra in cases, // when an application with name "foo" and with subcommand "foo" // is executed with args "foo foo". @@ -438,7 +499,7 @@ func TestFlagLong(t *testing.T) { output, err := executeCommand(c, "--intf=7", "--sf=abc", "one", "--", "two") if output != "" { - t.Errorf("Unexpected output: %v", err) + t.Errorf("Unexpected output: %v", output) } if err != nil { t.Errorf("Unexpected error: %v", err) @@ -475,7 +536,7 @@ func TestFlagShort(t *testing.T) { output, err := executeCommand(c, "-i", "7", "-sabc", "one", "two") if output != "" { - t.Errorf("Unexpected output: %v", err) + t.Errorf("Unexpected output: %v", output) } if err != nil { t.Errorf("Unexpected error: %v", err) @@ -504,7 +565,7 @@ func TestChildFlag(t *testing.T) { output, err := executeCommand(rootCmd, "child", "-i7") if output != "" { - t.Errorf("Unexpected output: %v", err) + t.Errorf("Unexpected output: %v", output) } if err != nil { t.Errorf("Unexpected error: %v", err) @@ -1111,6 +1172,39 @@ func TestShorthandVersionTemplate(t *testing.T) { checkStringContains(t, output, "customized version: 1.0.0") } +func TestRootErrPrefixExecutedOnSubcommand(t *testing.T) { + rootCmd := &Command{Use: "root", Run: emptyRun} + rootCmd.SetErrPrefix("root error prefix:") + rootCmd.AddCommand(&Command{Use: "sub", Run: emptyRun}) + + output, err := executeCommand(rootCmd, "sub", "--unknown-flag") + if err == nil { + t.Errorf("Expected error") + } + + checkStringContains(t, output, "root error prefix: unknown flag: --unknown-flag") +} + +func TestRootAndSubErrPrefix(t *testing.T) { + rootCmd := &Command{Use: "root", Run: emptyRun} + subCmd := &Command{Use: "sub", Run: emptyRun} + rootCmd.AddCommand(subCmd) + rootCmd.SetErrPrefix("root error prefix:") + subCmd.SetErrPrefix("sub error prefix:") + + if output, err := executeCommand(rootCmd, "--unknown-root-flag"); err == nil { + t.Errorf("Expected error") + } else { + checkStringContains(t, output, "root error prefix: unknown flag: --unknown-root-flag") + } + + if output, err := executeCommand(rootCmd, "sub", "--unknown-sub-flag"); err == nil { + t.Errorf("Expected error") + } else { + checkStringContains(t, output, "sub error prefix: unknown flag: --unknown-sub-flag") + } +} + func TestVersionFlagExecutedOnSubcommand(t *testing.T) { rootCmd := &Command{Use: "root", Version: "1.0.0"} rootCmd.AddCommand(&Command{Use: "sub", Run: emptyRun}) @@ -1509,57 +1603,73 @@ func TestHooks(t *testing.T) { } func TestPersistentHooks(t *testing.T) { - var ( - parentPersPreArgs string - parentPreArgs string - parentRunArgs string - parentPostArgs string - parentPersPostArgs string - ) + EnableTraverseRunHooks = true + testPersistentHooks(t, []string{ + "parent PersistentPreRun", + "child PersistentPreRun", + "child PreRun", + "child Run", + "child PostRun", + "child PersistentPostRun", + "parent PersistentPostRun", + }) - var ( - childPersPreArgs string - childPreArgs string - childRunArgs string - childPostArgs string - childPersPostArgs string - ) + EnableTraverseRunHooks = false + testPersistentHooks(t, []string{ + "child PersistentPreRun", + "child PreRun", + "child Run", + "child PostRun", + "child PersistentPostRun", + }) +} + +func testPersistentHooks(t *testing.T, expectedHookRunOrder []string) { + var hookRunOrder []string + + validateHook := func(args []string, hookName string) { + hookRunOrder = append(hookRunOrder, hookName) + got := strings.Join(args, " ") + if onetwo != got { + t.Errorf("Expected %s %q, got %q", hookName, onetwo, got) + } + } parentCmd := &Command{ Use: "parent", PersistentPreRun: func(_ *Command, args []string) { - parentPersPreArgs = strings.Join(args, " ") + validateHook(args, "parent PersistentPreRun") }, PreRun: func(_ *Command, args []string) { - parentPreArgs = strings.Join(args, " ") + validateHook(args, "parent PreRun") }, Run: func(_ *Command, args []string) { - parentRunArgs = strings.Join(args, " ") + validateHook(args, "parent Run") }, PostRun: func(_ *Command, args []string) { - parentPostArgs = strings.Join(args, " ") + validateHook(args, "parent PostRun") }, PersistentPostRun: func(_ *Command, args []string) { - parentPersPostArgs = strings.Join(args, " ") + validateHook(args, "parent PersistentPostRun") }, } childCmd := &Command{ Use: "child", PersistentPreRun: func(_ *Command, args []string) { - childPersPreArgs = strings.Join(args, " ") + validateHook(args, "child PersistentPreRun") }, PreRun: func(_ *Command, args []string) { - childPreArgs = strings.Join(args, " ") + validateHook(args, "child PreRun") }, Run: func(_ *Command, args []string) { - childRunArgs = strings.Join(args, " ") + validateHook(args, "child Run") }, PostRun: func(_ *Command, args []string) { - childPostArgs = strings.Join(args, " ") + validateHook(args, "child PostRun") }, PersistentPostRun: func(_ *Command, args []string) { - childPersPostArgs = strings.Join(args, " ") + validateHook(args, "child PersistentPostRun") }, } parentCmd.AddCommand(childCmd) @@ -1572,41 +1682,13 @@ func TestPersistentHooks(t *testing.T) { t.Errorf("Unexpected error: %v", err) } - for _, v := range []struct { - name string - got string - }{ - // TODO: currently PersistentPreRun* defined in parent does not - // run if the matching child subcommand has PersistentPreRun. - // If the behavior changes (https://github.com/spf13/cobra/issues/252) - // this test must be fixed. - {"parentPersPreArgs", parentPersPreArgs}, - {"parentPreArgs", parentPreArgs}, - {"parentRunArgs", parentRunArgs}, - {"parentPostArgs", parentPostArgs}, - // TODO: currently PersistentPostRun* defined in parent does not - // run if the matching child subcommand has PersistentPostRun. - // If the behavior changes (https://github.com/spf13/cobra/issues/252) - // this test must be fixed. - {"parentPersPostArgs", parentPersPostArgs}, - } { - if v.got != "" { - t.Errorf("Expected blank %s, got %q", v.name, v.got) - } - } - - for _, v := range []struct { - name string - got string - }{ - {"childPersPreArgs", childPersPreArgs}, - {"childPreArgs", childPreArgs}, - {"childRunArgs", childRunArgs}, - {"childPostArgs", childPostArgs}, - {"childPersPostArgs", childPersPostArgs}, - } { - if v.got != onetwo { - t.Errorf("Expected %s %q, got %q", v.name, onetwo, v.got) + for idx, exp := range expectedHookRunOrder { + if len(hookRunOrder) > idx { + if act := hookRunOrder[idx]; act != exp { + t.Errorf("Expected %q at %d, got %q", exp, idx, act) + } + } else { + t.Errorf("Expected %q at %d, got nothing", exp, idx) } } } @@ -2041,12 +2123,12 @@ func TestCommandPrintRedirection(t *testing.T) { t.Error(err) } - gotErrBytes, err := ioutil.ReadAll(errBuff) + gotErrBytes, err := io.ReadAll(errBuff) if err != nil { t.Error(err) } - gotOutBytes, err := ioutil.ReadAll(outBuff) + gotOutBytes, err := io.ReadAll(outBuff) if err != nil { t.Error(err) } @@ -2726,7 +2808,7 @@ func TestFind(t *testing.T) { func TestUnknownFlagShouldReturnSameErrorRegardlessOfArgPosition(t *testing.T) { testCases := [][]string{ - //{"--unknown", "--namespace", "foo", "child", "--bar"}, // FIXME: This test case fails, returning the error `unknown command "foo" for "root"` instead of the expected error `unknown flag: --unknown` + // {"--unknown", "--namespace", "foo", "child", "--bar"}, // FIXME: This test case fails, returning the error `unknown command "foo" for "root"` instead of the expected error `unknown flag: --unknown` {"--namespace", "foo", "--unknown", "child", "--bar"}, {"--namespace", "foo", "child", "--unknown", "--bar"}, {"--namespace", "foo", "child", "--bar", "--unknown"}, diff --git a/completions.go b/completions.go index 58bc770a..d60d8c2f 100644 --- a/completions.go +++ b/completions.go @@ -17,6 +17,8 @@ package cobra import ( "fmt" "os" + "regexp" + "strconv" "strings" "sync" @@ -145,6 +147,20 @@ func (c *Command) RegisterFlagCompletionFunc(flagName string, f func(cmd *Comman return nil } +// GetFlagCompletionFunc returns the completion function for the given flag of the command, if available. +func (c *Command) GetFlagCompletionFunc(flagName string) (func(*Command, []string, string) ([]string, ShellCompDirective), bool) { + flag := c.Flag(flagName) + if flag == nil { + return nil, false + } + + flagCompletionMutex.RLock() + defer flagCompletionMutex.RUnlock() + + completionFunc, exists := flagCompletionFunctions[flag] + return completionFunc, exists +} + // Returns a string listing the different directive enabled in the specified parameter func (d ShellCompDirective) string() string { var directives []string @@ -197,24 +213,29 @@ func (c *Command) initCompleteCmd(args []string) { // 2- Even without completions, we need to print the directive } - noDescriptions := (cmd.CalledAs() == ShellCompNoDescRequestCmd) + noDescriptions := cmd.CalledAs() == ShellCompNoDescRequestCmd + if !noDescriptions { + if doDescriptions, err := strconv.ParseBool(getEnvConfig(cmd, configEnvVarSuffixDescriptions)); err == nil { + noDescriptions = !doDescriptions + } + } + noActiveHelp := GetActiveHelpConfig(finalCmd) == activeHelpGlobalDisable + out := finalCmd.OutOrStdout() for _, comp := range completions { - if GetActiveHelpConfig(finalCmd) == activeHelpGlobalDisable { - // Remove all activeHelp entries in this case - if strings.HasPrefix(comp, activeHelpMarker) { - continue - } + if noActiveHelp && strings.HasPrefix(comp, activeHelpMarker) { + // Remove all activeHelp entries if it's disabled. + continue } if noDescriptions { // Remove any description that may be included following a tab character. - comp = strings.Split(comp, "\t")[0] + comp = strings.SplitN(comp, "\t", 2)[0] } // Make sure we only write the first line to the output. // This is needed if a description contains a linebreak. // Otherwise the shell scripts will interpret the other lines as new flags // and could therefore provide a wrong completion. - comp = strings.Split(comp, "\n")[0] + comp = strings.SplitN(comp, "\n", 2)[0] // Finally trim the completion. This is especially important to get rid // of a trailing tab when there are no description following it. @@ -223,14 +244,14 @@ func (c *Command) initCompleteCmd(args []string) { // although there is no description). comp = strings.TrimSpace(comp) - // Print each possible completion to stdout for the completion script to consume. - fmt.Fprintln(finalCmd.OutOrStdout(), comp) + // Print each possible completion to the output for the completion script to consume. + fmt.Fprintln(out, comp) } // As the last printout, print the completion directive for the completion script to parse. // The directive integer must be that last character following a single colon (:). // The completion script expects : - fmt.Fprintf(finalCmd.OutOrStdout(), ":%d\n", directive) + fmt.Fprintf(out, ":%d\n", directive) // Print some helpful info to stderr for the user to understand. // Output from stderr must be ignored by the completion script. @@ -277,15 +298,19 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi } if err != nil { // Unable to find the real command. E.g., someInvalidCmd - return c, []string{}, ShellCompDirectiveDefault, fmt.Errorf("Unable to find a command for arguments: %v", trimmedArgs) + return c, []string{}, ShellCompDirectiveDefault, fmt.Errorf("unable to find a command for arguments: %v", trimmedArgs) } finalCmd.ctx = c.ctx // These flags are normally added when `execute()` is called on `finalCmd`, // however, when doing completion, we don't call `finalCmd.execute()`. - // Let's add the --help and --version flag ourselves. - finalCmd.InitDefaultHelpFlag() - finalCmd.InitDefaultVersionFlag() + // Let's add the --help and --version flag ourselves but only if the finalCmd + // has not disabled flag parsing; if flag parsing is disabled, it is up to the + // finalCmd itself to handle the completion of *all* flags. + if !finalCmd.DisableFlagParsing { + finalCmd.InitDefaultHelpFlag() + finalCmd.InitDefaultVersionFlag() + } // Check if we are doing flag value completion before parsing the flags. // This is important because if we are completing a flag value, we need to also @@ -389,6 +414,11 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi finalCmd.InheritedFlags().VisitAll(func(flag *pflag.Flag) { doCompleteFlags(flag) }) + // Try to complete non-inherited flags even if DisableFlagParsing==true. + // This allows programs to tell Cobra about flags for completion even + // if the actual parsing of flags is not done by Cobra. + // For instance, Helm uses this to provide flag name completion for + // some of its plugins. finalCmd.NonInheritedFlags().VisitAll(func(flag *pflag.Flag) { doCompleteFlags(flag) }) @@ -893,3 +923,34 @@ func CompError(msg string) { func CompErrorln(msg string) { CompError(fmt.Sprintf("%s\n", msg)) } + +// These values should not be changed: users will be using them explicitly. +const ( + configEnvVarGlobalPrefix = "COBRA" + configEnvVarSuffixDescriptions = "COMPLETION_DESCRIPTIONS" +) + +var configEnvVarPrefixSubstRegexp = regexp.MustCompile(`[^A-Z0-9_]`) + +// configEnvVar returns the name of the program-specific configuration environment +// variable. It has the format _ where is the name of the +// root command in upper case, with all non-ASCII-alphanumeric characters replaced by `_`. +func configEnvVar(name, suffix string) string { + // This format should not be changed: users will be using it explicitly. + v := strings.ToUpper(fmt.Sprintf("%s_%s", name, suffix)) + v = configEnvVarPrefixSubstRegexp.ReplaceAllString(v, "_") + return v +} + +// getEnvConfig returns the value of the configuration environment variable +// _ where is the name of the root command in upper +// case, with all non-ASCII-alphanumeric characters replaced by `_`. +// If the value is empty or not set, the value of the environment variable +// COBRA_ is returned instead. +func getEnvConfig(cmd *Command, suffix string) string { + v := os.Getenv(configEnvVar(cmd.Root().Name(), suffix)) + if v == "" { + v = os.Getenv(configEnvVar(configEnvVarGlobalPrefix, suffix)) + } + return v +} diff --git a/completions_test.go b/completions_test.go index 75f652cf..28acb236 100644 --- a/completions_test.go +++ b/completions_test.go @@ -17,7 +17,10 @@ package cobra import ( "bytes" "context" + "fmt" + "os" "strings" + "sync" "testing" ) @@ -2040,6 +2043,114 @@ func TestFlagCompletionWorksRootCommandAddedAfterFlags(t *testing.T) { } } +func TestFlagCompletionForPersistentFlagsCalledFromSubCmd(t *testing.T) { + rootCmd := &Command{Use: "root", Run: emptyRun} + rootCmd.PersistentFlags().String("string", "", "test string flag") + _ = rootCmd.RegisterFlagCompletionFunc("string", func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{"myval"}, ShellCompDirectiveDefault + }) + + childCmd := &Command{ + Use: "child", + Run: emptyRun, + ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{"--validarg", "test"}, ShellCompDirectiveDefault + }, + } + childCmd.Flags().Bool("bool", false, "test bool flag") + rootCmd.AddCommand(childCmd) + + // Test that persistent flag completion works for the subcmd + output, err := executeCommand(rootCmd, ShellCompRequestCmd, "child", "--string", "") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + expected := strings.Join([]string{ + "myval", + ":0", + "Completion ended with directive: ShellCompDirectiveDefault", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } +} + +// This test tries to register flag completion concurrently to make sure the +// code handles concurrency properly. +// This was reported as a problem when tests are run concurrently: +// https://github.com/spf13/cobra/issues/1320 +// +// NOTE: this test can sometimes pass even if the code were to not handle +// concurrency properly. This is not great but the important part is that +// it should never fail. Therefore, if the tests fails sometimes, we will +// still be able to know there is a problem. +func TestFlagCompletionConcurrentRegistration(t *testing.T) { + rootCmd := &Command{Use: "root", Run: emptyRun} + const maxFlags = 50 + for i := 1; i < maxFlags; i += 2 { + flagName := fmt.Sprintf("flag%d", i) + rootCmd.Flags().String(flagName, "", fmt.Sprintf("test %s flag on root", flagName)) + } + + childCmd := &Command{ + Use: "child", + Run: emptyRun, + } + for i := 2; i <= maxFlags; i += 2 { + flagName := fmt.Sprintf("flag%d", i) + childCmd.Flags().String(flagName, "", fmt.Sprintf("test %s flag on child", flagName)) + } + + rootCmd.AddCommand(childCmd) + + // Register completion in different threads to test concurrency. + var wg sync.WaitGroup + for i := 1; i <= maxFlags; i++ { + index := i + flagName := fmt.Sprintf("flag%d", i) + wg.Add(1) + go func() { + defer wg.Done() + cmd := rootCmd + if index%2 == 0 { + cmd = childCmd + } + _ = cmd.RegisterFlagCompletionFunc(flagName, func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{fmt.Sprintf("flag%d", index)}, ShellCompDirectiveDefault + }) + }() + } + + wg.Wait() + + // Test that flag completion works for each flag + for i := 1; i <= 6; i++ { + var output string + var err error + flagName := fmt.Sprintf("flag%d", i) + + if i%2 == 1 { + output, err = executeCommand(rootCmd, ShellCompRequestCmd, "--"+flagName, "") + } else { + output, err = executeCommand(rootCmd, ShellCompRequestCmd, "child", "--"+flagName, "") + } + + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + expected := strings.Join([]string{ + flagName, + ":0", + "Completion ended with directive: ShellCompDirectiveDefault", ""}, "\n") + + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } + } +} + func TestFlagCompletionInGoWithDesc(t *testing.T) { rootCmd := &Command{ Use: "root", @@ -2658,8 +2769,6 @@ func TestCompleteWithDisableFlagParsing(t *testing.T) { expected := strings.Join([]string{ "--persistent", "-p", - "--help", - "-h", "--nonPersistent", "-n", "--flag", @@ -2866,6 +2975,104 @@ func TestCompletionForGroupedFlags(t *testing.T) { } } +func TestCompletionForOneRequiredGroupFlags(t *testing.T) { + getCmd := func() *Command { + rootCmd := &Command{ + Use: "root", + Run: emptyRun, + } + childCmd := &Command{ + Use: "child", + ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{"subArg"}, ShellCompDirectiveNoFileComp + }, + Run: emptyRun, + } + rootCmd.AddCommand(childCmd) + + rootCmd.PersistentFlags().Int("ingroup1", -1, "ingroup1") + rootCmd.PersistentFlags().String("ingroup2", "", "ingroup2") + + childCmd.Flags().Bool("ingroup3", false, "ingroup3") + childCmd.Flags().Bool("nogroup", false, "nogroup") + + // Add flags to a group + childCmd.MarkFlagsOneRequired("ingroup1", "ingroup2", "ingroup3") + + return rootCmd + } + + // Each test case uses a unique command from the function above. + testcases := []struct { + desc string + args []string + expectedOutput string + }{ + { + desc: "flags in group suggested without - prefix", + args: []string{"child", ""}, + expectedOutput: strings.Join([]string{ + "--ingroup1", + "--ingroup2", + "--ingroup3", + "subArg", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"), + }, + { + desc: "flags in group suggested with - prefix", + args: []string{"child", "-"}, + expectedOutput: strings.Join([]string{ + "--ingroup1", + "--ingroup2", + "--ingroup3", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"), + }, + { + desc: "when any flag in group present, other flags in group not suggested without - prefix", + args: []string{"child", "--ingroup2", "value", ""}, + expectedOutput: strings.Join([]string{ + "subArg", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"), + }, + { + desc: "when all flags in group present, flags not suggested without - prefix", + args: []string{"child", "--ingroup1", "8", "--ingroup2", "value2", "--ingroup3", ""}, + expectedOutput: strings.Join([]string{ + "subArg", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"), + }, + { + desc: "group ignored if some flags not applicable", + args: []string{"--ingroup2", "value", ""}, + expectedOutput: strings.Join([]string{ + "child", + "completion", + "help", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"), + }, + } + + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + c := getCmd() + args := []string{ShellCompNoDescRequestCmd} + args = append(args, tc.args...) + output, err := executeCommand(c, args...) + switch { + case err == nil && output != tc.expectedOutput: + t.Errorf("expected: %q, got: %q", tc.expectedOutput, output) + case err != nil: + t.Errorf("Unexpected error %q", err) + } + }) + } +} + func TestCompletionForMutuallyExclusiveFlags(t *testing.T) { getCmd := func() *Command { rootCmd := &Command{ @@ -2991,8 +3198,26 @@ func TestCompletionCobraFlags(t *testing.T) { return []string{"extra3"}, ShellCompDirectiveNoFileComp }, } + childCmd4 := &Command{ + Use: "child4", + Version: "1.1.1", + Run: emptyRun, + ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{"extra4"}, ShellCompDirectiveNoFileComp + }, + DisableFlagParsing: true, + } + childCmd5 := &Command{ + Use: "child5", + Version: "1.1.1", + Run: emptyRun, + ValidArgsFunction: func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{"extra5"}, ShellCompDirectiveNoFileComp + }, + DisableFlagParsing: true, + } - rootCmd.AddCommand(childCmd, childCmd2, childCmd3) + rootCmd.AddCommand(childCmd, childCmd2, childCmd3, childCmd4, childCmd5) _ = childCmd.Flags().Bool("bool", false, "A bool flag") _ = childCmd.MarkFlagRequired("bool") @@ -3004,6 +3229,10 @@ func TestCompletionCobraFlags(t *testing.T) { // Have a command that only adds its own -v flag _ = childCmd3.Flags().BoolP("verbose", "v", false, "Not a version flag") + // Have a command that DisablesFlagParsing but that also adds its own help and version flags + _ = childCmd5.Flags().BoolP("help", "h", false, "My own help") + _ = childCmd5.Flags().BoolP("version", "v", false, "My own version") + return rootCmd } @@ -3134,6 +3363,26 @@ func TestCompletionCobraFlags(t *testing.T) { ":4", "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"), }, + { + desc: "no completion for --help/-h and --version/-v flags when DisableFlagParsing=true", + args: []string{"child4", "-"}, + expectedOutput: strings.Join([]string{ + "extra4", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"), + }, + { + desc: "completions for program-defined --help/-h and --version/-v flags even when DisableFlagParsing=true", + args: []string{"child5", "-"}, + expectedOutput: strings.Join([]string{ + "--help", + "-h", + "--version", + "-v", + "extra5", + ":4", + "Completion ended with directive: ShellCompDirectiveNoFileComp", ""}, "\n"), + }, } for _, tc := range testcases { @@ -3215,3 +3464,284 @@ Completion ended with directive: ShellCompDirectiveNoFileComp }) } } + +func TestGetFlagCompletion(t *testing.T) { + rootCmd := &Command{Use: "root", Run: emptyRun} + + rootCmd.Flags().String("rootflag", "", "root flag") + _ = rootCmd.RegisterFlagCompletionFunc("rootflag", func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{"rootvalue"}, ShellCompDirectiveKeepOrder + }) + + rootCmd.PersistentFlags().String("persistentflag", "", "persistent flag") + _ = rootCmd.RegisterFlagCompletionFunc("persistentflag", func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{"persistentvalue"}, ShellCompDirectiveDefault + }) + + childCmd := &Command{Use: "child", Run: emptyRun} + + childCmd.Flags().String("childflag", "", "child flag") + _ = childCmd.RegisterFlagCompletionFunc("childflag", func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + return []string{"childvalue"}, ShellCompDirectiveNoFileComp | ShellCompDirectiveNoSpace + }) + + rootCmd.AddCommand(childCmd) + + testcases := []struct { + desc string + cmd *Command + flagName string + exists bool + comps []string + directive ShellCompDirective + }{ + { + desc: "get flag completion function for command", + cmd: rootCmd, + flagName: "rootflag", + exists: true, + comps: []string{"rootvalue"}, + directive: ShellCompDirectiveKeepOrder, + }, + { + desc: "get persistent flag completion function for command", + cmd: rootCmd, + flagName: "persistentflag", + exists: true, + comps: []string{"persistentvalue"}, + directive: ShellCompDirectiveDefault, + }, + { + desc: "get flag completion function for child command", + cmd: childCmd, + flagName: "childflag", + exists: true, + comps: []string{"childvalue"}, + directive: ShellCompDirectiveNoFileComp | ShellCompDirectiveNoSpace, + }, + { + desc: "get persistent flag completion function for child command", + cmd: childCmd, + flagName: "persistentflag", + exists: true, + comps: []string{"persistentvalue"}, + directive: ShellCompDirectiveDefault, + }, + { + desc: "cannot get flag completion function for local parent flag", + cmd: childCmd, + flagName: "rootflag", + exists: false, + }, + } + + for _, tc := range testcases { + t.Run(tc.desc, func(t *testing.T) { + compFunc, exists := tc.cmd.GetFlagCompletionFunc(tc.flagName) + if tc.exists != exists { + t.Errorf("Unexpected result looking for flag completion function") + } + + if exists { + comps, directive := compFunc(tc.cmd, []string{}, "") + if strings.Join(tc.comps, " ") != strings.Join(comps, " ") { + t.Errorf("Unexpected completions %q", comps) + } + if tc.directive != directive { + t.Errorf("Unexpected directive %q", directive) + } + } + }) + } +} + +func TestGetEnvConfig(t *testing.T) { + testCases := []struct { + desc string + use string + suffix string + cmdVar string + globalVar string + cmdVal string + globalVal string + expected string + }{ + { + desc: "Command envvar overrides global", + use: "root", + suffix: "test", + cmdVar: "ROOT_TEST", + globalVar: "COBRA_TEST", + cmdVal: "cmd", + globalVal: "global", + expected: "cmd", + }, + { + desc: "Missing/empty command envvar falls back to global", + use: "root", + suffix: "test", + cmdVar: "ROOT_TEST", + globalVar: "COBRA_TEST", + cmdVal: "", + globalVal: "global", + expected: "global", + }, + { + desc: "Missing/empty command and global envvars fall back to empty", + use: "root", + suffix: "test", + cmdVar: "ROOT_TEST", + globalVar: "COBRA_TEST", + cmdVal: "", + globalVal: "", + expected: "", + }, + { + desc: "Periods in command use transform to underscores in env var name", + use: "foo.bar", + suffix: "test", + cmdVar: "FOO_BAR_TEST", + globalVar: "COBRA_TEST", + cmdVal: "cmd", + globalVal: "global", + expected: "cmd", + }, + { + desc: "Dashes in command use transform to underscores in env var name", + use: "quux-BAZ", + suffix: "test", + cmdVar: "QUUX_BAZ_TEST", + globalVar: "COBRA_TEST", + cmdVal: "cmd", + globalVal: "global", + expected: "cmd", + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + // Could make env handling cleaner with t.Setenv with Go >= 1.17 + err := os.Setenv(tc.cmdVar, tc.cmdVal) + defer func() { + assertNoErr(t, os.Unsetenv(tc.cmdVar)) + }() + assertNoErr(t, err) + err = os.Setenv(tc.globalVar, tc.globalVal) + defer func() { + assertNoErr(t, os.Unsetenv(tc.globalVar)) + }() + assertNoErr(t, err) + cmd := &Command{Use: tc.use} + got := getEnvConfig(cmd, tc.suffix) + if got != tc.expected { + t.Errorf("expected: %q, got: %q", tc.expected, got) + } + }) + } +} + +func TestDisableDescriptions(t *testing.T) { + rootCmd := &Command{ + Use: "root", + Run: emptyRun, + } + + childCmd := &Command{ + Use: "thechild", + Short: "The child command", + Run: emptyRun, + } + rootCmd.AddCommand(childCmd) + + specificDescriptionsEnvVar := configEnvVar(rootCmd.Name(), configEnvVarSuffixDescriptions) + globalDescriptionsEnvVar := configEnvVar(configEnvVarGlobalPrefix, configEnvVarSuffixDescriptions) + + const ( + descLineWithDescription = "first\tdescription" + descLineWithoutDescription = "first" + ) + childCmd.ValidArgsFunction = func(cmd *Command, args []string, toComplete string) ([]string, ShellCompDirective) { + comps := []string{descLineWithDescription} + return comps, ShellCompDirectiveDefault + } + + testCases := []struct { + desc string + globalEnvValue string + specificEnvValue string + expectedLine string + }{ + { + "No env variables set", + "", + "", + descLineWithDescription, + }, + { + "Global value false", + "false", + "", + descLineWithoutDescription, + }, + { + "Specific value false", + "", + "false", + descLineWithoutDescription, + }, + { + "Both values false", + "false", + "false", + descLineWithoutDescription, + }, + { + "Both values true", + "true", + "true", + descLineWithDescription, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + if err := os.Setenv(specificDescriptionsEnvVar, tc.specificEnvValue); err != nil { + t.Errorf("Unexpected error setting %s: %v", specificDescriptionsEnvVar, err) + } + if err := os.Setenv(globalDescriptionsEnvVar, tc.globalEnvValue); err != nil { + t.Errorf("Unexpected error setting %s: %v", globalDescriptionsEnvVar, err) + } + + var run = func() { + output, err := executeCommand(rootCmd, ShellCompRequestCmd, "thechild", "") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + expected := strings.Join([]string{ + tc.expectedLine, + ":0", + "Completion ended with directive: ShellCompDirectiveDefault", ""}, "\n") + if output != expected { + t.Errorf("expected: %q, got: %q", expected, output) + } + } + + run() + + // For empty cases, test also unset state + if tc.specificEnvValue == "" { + if err := os.Unsetenv(specificDescriptionsEnvVar); err != nil { + t.Errorf("Unexpected error unsetting %s: %v", specificDescriptionsEnvVar, err) + } + run() + } + if tc.globalEnvValue == "" { + if err := os.Unsetenv(globalDescriptionsEnvVar); err != nil { + t.Errorf("Unexpected error unsetting %s: %v", globalDescriptionsEnvVar, err) + } + run() + } + }) + } +} diff --git a/doc/man_docs.go b/doc/man_docs.go index b8c15ce8..2138f248 100644 --- a/doc/man_docs.go +++ b/doc/man_docs.go @@ -133,7 +133,7 @@ func fillHeader(header *GenManHeader, name string, disableAutoGen bool) error { } header.Date = &now } - header.date = (*header.Date).Format("Jan 2006") + header.date = header.Date.Format("Jan 2006") if header.Source == "" && !disableAutoGen { header.Source = "Auto generated by spf13/cobra" } diff --git a/doc/man_docs_test.go b/doc/man_docs_test.go index c111d455..a4435e6e 100644 --- a/doc/man_docs_test.go +++ b/doc/man_docs_test.go @@ -150,7 +150,7 @@ func TestGenManSeeAlso(t *testing.T) { } } -func TestManPrintFlagsHidesShortDeperecated(t *testing.T) { +func TestManPrintFlagsHidesShortDeprecated(t *testing.T) { c := &cobra.Command{} c.Flags().StringP("foo", "f", "default", "Foo flag") assertNoErr(t, c.Flags().MarkShorthandDeprecated("foo", "don't use it no more")) diff --git a/doc/md_docs.go b/doc/md_docs.go index c4a27c00..12592223 100644 --- a/doc/md_docs.go +++ b/doc/md_docs.go @@ -27,6 +27,8 @@ import ( "github.com/spf13/cobra" ) +const markdownExtension = ".md" + func printOptions(buf *bytes.Buffer, cmd *cobra.Command, name string) error { flags := cmd.NonInheritedFlags() flags.SetOutput(buf) @@ -83,7 +85,7 @@ func GenMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string) if cmd.HasParent() { parent := cmd.Parent() pname := parent.CommandPath() - link := pname + ".md" + link := pname + markdownExtension link = strings.ReplaceAll(link, " ", "_") buf.WriteString(fmt.Sprintf("* [%s](%s)\t - %s\n", pname, linkHandler(link), parent.Short)) cmd.VisitParents(func(c *cobra.Command) { @@ -101,7 +103,7 @@ func GenMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string) continue } cname := name + " " + child.Name() - link := cname + ".md" + link := cname + markdownExtension link = strings.ReplaceAll(link, " ", "_") buf.WriteString(fmt.Sprintf("* [%s](%s)\t - %s\n", cname, linkHandler(link), child.Short)) } @@ -126,7 +128,7 @@ func GenMarkdownTree(cmd *cobra.Command, dir string) error { return GenMarkdownTreeCustom(cmd, dir, emptyStr, identity) } -// GenMarkdownTreeCustom is the the same as GenMarkdownTree, but +// GenMarkdownTreeCustom is the same as GenMarkdownTree, but // with custom filePrepender and linkHandler. func GenMarkdownTreeCustom(cmd *cobra.Command, dir string, filePrepender, linkHandler func(string) string) error { for _, c := range cmd.Commands() { @@ -138,7 +140,7 @@ func GenMarkdownTreeCustom(cmd *cobra.Command, dir string, filePrepender, linkHa } } - basename := strings.ReplaceAll(cmd.CommandPath(), " ", "_") + ".md" + basename := strings.ReplaceAll(cmd.CommandPath(), " ", "_") + markdownExtension filename := filepath.Join(dir, basename) f, err := os.Create(filename) if err != nil { diff --git a/doc/rest_docs.go b/doc/rest_docs.go index 2cca6fd7..c33acc2b 100644 --- a/doc/rest_docs.go +++ b/doc/rest_docs.go @@ -140,7 +140,7 @@ func GenReSTTree(cmd *cobra.Command, dir string) error { return GenReSTTreeCustom(cmd, dir, emptyStr, defaultLinkHandler) } -// GenReSTTreeCustom is the the same as GenReSTTree, but +// GenReSTTreeCustom is the same as GenReSTTree, but // with custom filePrepender and linkHandler. func GenReSTTreeCustom(cmd *cobra.Command, dir string, filePrepender func(string) string, linkHandler func(string, string) string) error { for _, c := range cmd.Commands() { diff --git a/doc/util.go b/doc/util.go index 0aaa07a1..4de4ceee 100644 --- a/doc/util.go +++ b/doc/util.go @@ -40,7 +40,7 @@ func hasSeeAlso(cmd *cobra.Command) bool { // that do not contain \n. func forceMultiLine(s string) string { if len(s) > 60 && !strings.Contains(s, "\n") { - s = s + "\n" + s += "\n" } return s } diff --git a/flag_groups.go b/flag_groups.go index b35fde15..560612fd 100644 --- a/flag_groups.go +++ b/flag_groups.go @@ -23,8 +23,9 @@ import ( ) const ( - requiredAsGroup = "cobra_annotation_required_if_others_set" - mutuallyExclusive = "cobra_annotation_mutually_exclusive" + requiredAsGroupAnnotation = "cobra_annotation_required_if_others_set" + oneRequiredAnnotation = "cobra_annotation_one_required" + mutuallyExclusiveAnnotation = "cobra_annotation_mutually_exclusive" ) // MarkFlagsRequiredTogether marks the given flags with annotations so that Cobra errors @@ -36,7 +37,23 @@ func (c *Command) MarkFlagsRequiredTogether(flagNames ...string) { if f == nil { panic(fmt.Sprintf("Failed to find flag %q and mark it as being required in a flag group", v)) } - if err := c.Flags().SetAnnotation(v, requiredAsGroup, append(f.Annotations[requiredAsGroup], strings.Join(flagNames, " "))); err != nil { + if err := c.Flags().SetAnnotation(v, requiredAsGroupAnnotation, append(f.Annotations[requiredAsGroupAnnotation], strings.Join(flagNames, " "))); err != nil { + // Only errs if the flag isn't found. + panic(err) + } + } +} + +// MarkFlagsOneRequired marks the given flags with annotations so that Cobra errors +// if the command is invoked without at least one flag from the given set of flags. +func (c *Command) MarkFlagsOneRequired(flagNames ...string) { + c.mergePersistentFlags() + for _, v := range flagNames { + f := c.Flags().Lookup(v) + if f == nil { + panic(fmt.Sprintf("Failed to find flag %q and mark it as being in a one-required flag group", v)) + } + if err := c.Flags().SetAnnotation(v, oneRequiredAnnotation, append(f.Annotations[oneRequiredAnnotation], strings.Join(flagNames, " "))); err != nil { // Only errs if the flag isn't found. panic(err) } @@ -53,13 +70,13 @@ func (c *Command) MarkFlagsMutuallyExclusive(flagNames ...string) { panic(fmt.Sprintf("Failed to find flag %q and mark it as being in a mutually exclusive flag group", v)) } // Each time this is called is a single new entry; this allows it to be a member of multiple groups if needed. - if err := c.Flags().SetAnnotation(v, mutuallyExclusive, append(f.Annotations[mutuallyExclusive], strings.Join(flagNames, " "))); err != nil { + if err := c.Flags().SetAnnotation(v, mutuallyExclusiveAnnotation, append(f.Annotations[mutuallyExclusiveAnnotation], strings.Join(flagNames, " "))); err != nil { panic(err) } } } -// ValidateFlagGroups validates the mutuallyExclusive/requiredAsGroup logic and returns the +// ValidateFlagGroups validates the mutuallyExclusive/oneRequired/requiredAsGroup logic and returns the // first error encountered. func (c *Command) ValidateFlagGroups() error { if c.DisableFlagParsing { @@ -71,15 +88,20 @@ func (c *Command) ValidateFlagGroups() error { // groupStatus format is the list of flags as a unique ID, // then a map of each flag name and whether it is set or not. groupStatus := map[string]map[string]bool{} + oneRequiredGroupStatus := map[string]map[string]bool{} mutuallyExclusiveGroupStatus := map[string]map[string]bool{} flags.VisitAll(func(pflag *flag.Flag) { - processFlagForGroupAnnotation(flags, pflag, requiredAsGroup, groupStatus) - processFlagForGroupAnnotation(flags, pflag, mutuallyExclusive, mutuallyExclusiveGroupStatus) + processFlagForGroupAnnotation(flags, pflag, requiredAsGroupAnnotation, groupStatus) + processFlagForGroupAnnotation(flags, pflag, oneRequiredAnnotation, oneRequiredGroupStatus) + processFlagForGroupAnnotation(flags, pflag, mutuallyExclusiveAnnotation, mutuallyExclusiveGroupStatus) }) if err := validateRequiredFlagGroups(groupStatus); err != nil { return err } + if err := validateOneRequiredFlagGroups(oneRequiredGroupStatus); err != nil { + return err + } if err := validateExclusiveFlagGroups(mutuallyExclusiveGroupStatus); err != nil { return err } @@ -108,7 +130,7 @@ func processFlagForGroupAnnotation(flags *flag.FlagSet, pflag *flag.Flag, annota continue } - groupStatus[group] = map[string]bool{} + groupStatus[group] = make(map[string]bool, len(flagnames)) for _, name := range flagnames { groupStatus[group][name] = false } @@ -142,6 +164,27 @@ func validateRequiredFlagGroups(data map[string]map[string]bool) error { return nil } +func validateOneRequiredFlagGroups(data map[string]map[string]bool) error { + keys := sortedKeys(data) + for _, flagList := range keys { + flagnameAndStatus := data[flagList] + var set []string + for flagname, isSet := range flagnameAndStatus { + if isSet { + set = append(set, flagname) + } + } + if len(set) >= 1 { + continue + } + + // Sort values, so they can be tested/scripted against consistently. + sort.Strings(set) + return fmt.Errorf("at least one of the flags in the group [%v] is required", flagList) + } + return nil +} + func validateExclusiveFlagGroups(data map[string]map[string]bool) error { keys := sortedKeys(data) for _, flagList := range keys { @@ -176,6 +219,7 @@ func sortedKeys(m map[string]map[string]bool) []string { // enforceFlagGroupsForCompletion will do the following: // - when a flag in a group is present, other flags in the group will be marked required +// - when none of the flags in a one-required group are present, all flags in the group will be marked required // - when a flag in a mutually exclusive group is present, other flags in the group will be marked as hidden // This allows the standard completion logic to behave appropriately for flag groups func (c *Command) enforceFlagGroupsForCompletion() { @@ -185,10 +229,12 @@ func (c *Command) enforceFlagGroupsForCompletion() { flags := c.Flags() groupStatus := map[string]map[string]bool{} + oneRequiredGroupStatus := map[string]map[string]bool{} mutuallyExclusiveGroupStatus := map[string]map[string]bool{} c.Flags().VisitAll(func(pflag *flag.Flag) { - processFlagForGroupAnnotation(flags, pflag, requiredAsGroup, groupStatus) - processFlagForGroupAnnotation(flags, pflag, mutuallyExclusive, mutuallyExclusiveGroupStatus) + processFlagForGroupAnnotation(flags, pflag, requiredAsGroupAnnotation, groupStatus) + processFlagForGroupAnnotation(flags, pflag, oneRequiredAnnotation, oneRequiredGroupStatus) + processFlagForGroupAnnotation(flags, pflag, mutuallyExclusiveAnnotation, mutuallyExclusiveGroupStatus) }) // If a flag that is part of a group is present, we make all the other flags @@ -204,6 +250,26 @@ func (c *Command) enforceFlagGroupsForCompletion() { } } + // If none of the flags of a one-required group are present, we make all the flags + // of that group required so that the shell completion suggests them automatically + for flagList, flagnameAndStatus := range oneRequiredGroupStatus { + isSet := false + + for _, isSet = range flagnameAndStatus { + if isSet { + break + } + } + + // None of the flags of the group are set, mark all flags in the group + // as required + if !isSet { + for _, fName := range strings.Split(flagList, " ") { + _ = c.MarkFlagRequired(fName) + } + } + } + // If a flag that is mutually exclusive to others is present, we hide the other // flags of that group so the shell completion does not suggest them for flagList, flagnameAndStatus := range mutuallyExclusiveGroupStatus { diff --git a/flag_groups_test.go b/flag_groups_test.go index bf988d73..cffa8552 100644 --- a/flag_groups_test.go +++ b/flag_groups_test.go @@ -43,13 +43,15 @@ func TestValidateFlagGroups(t *testing.T) { // Each test case uses a unique command from the function above. testcases := []struct { - desc string - flagGroupsRequired []string - flagGroupsExclusive []string - subCmdFlagGroupsRequired []string - subCmdFlagGroupsExclusive []string - args []string - expectErr string + desc string + flagGroupsRequired []string + flagGroupsOneRequired []string + flagGroupsExclusive []string + subCmdFlagGroupsRequired []string + subCmdFlagGroupsOneRequired []string + subCmdFlagGroupsExclusive []string + args []string + expectErr string }{ { desc: "No flags no problem", @@ -62,6 +64,11 @@ func TestValidateFlagGroups(t *testing.T) { flagGroupsRequired: []string{"a b c"}, args: []string{"--a=foo"}, expectErr: "if any flags in the group [a b c] are set they must all be set; missing [b c]", + }, { + desc: "One-required flag group not satisfied", + flagGroupsOneRequired: []string{"a b"}, + args: []string{"--c=foo"}, + expectErr: "at least one of the flags in the group [a b] is required", }, { desc: "Exclusive flag group not satisfied", flagGroupsExclusive: []string{"a b c"}, @@ -72,6 +79,11 @@ func TestValidateFlagGroups(t *testing.T) { flagGroupsRequired: []string{"a b c", "a d"}, args: []string{"--c=foo", "--d=foo"}, expectErr: `if any flags in the group [a b c] are set they must all be set; missing [a b]`, + }, { + desc: "Multiple one-required flag group not satisfied returns first error", + flagGroupsOneRequired: []string{"a b", "d e"}, + args: []string{"--c=foo", "--f=foo"}, + expectErr: `at least one of the flags in the group [a b] is required`, }, { desc: "Multiple exclusive flag group not satisfied returns first error", flagGroupsExclusive: []string{"a b c", "a d"}, @@ -82,32 +94,57 @@ func TestValidateFlagGroups(t *testing.T) { flagGroupsRequired: []string{"a d", "a b", "a c"}, args: []string{"--a=foo"}, expectErr: `if any flags in the group [a b] are set they must all be set; missing [b]`, + }, { + desc: "Validation of one-required groups occurs on groups in sorted order", + flagGroupsOneRequired: []string{"d e", "a b", "f g"}, + args: []string{"--c=foo"}, + expectErr: `at least one of the flags in the group [a b] is required`, }, { desc: "Validation of exclusive groups occurs on groups in sorted order", flagGroupsExclusive: []string{"a d", "a b", "a c"}, args: []string{"--a=foo", "--b=foo", "--c=foo"}, expectErr: `if any flags in the group [a b] are set none of the others can be; [a b] were all set`, }, { - desc: "Persistent flags utilize both features and can fail required groups", + desc: "Persistent flags utilize required and exclusive groups and can fail required groups", flagGroupsRequired: []string{"a e", "e f"}, flagGroupsExclusive: []string{"f g"}, args: []string{"--a=foo", "--f=foo", "--g=foo"}, expectErr: `if any flags in the group [a e] are set they must all be set; missing [e]`, }, { - desc: "Persistent flags utilize both features and can fail mutually exclusive groups", + desc: "Persistent flags utilize one-required and exclusive groups and can fail one-required groups", + flagGroupsOneRequired: []string{"a b", "e f"}, + flagGroupsExclusive: []string{"e f"}, + args: []string{"--e=foo"}, + expectErr: `at least one of the flags in the group [a b] is required`, + }, { + desc: "Persistent flags utilize required and exclusive groups and can fail mutually exclusive groups", flagGroupsRequired: []string{"a e", "e f"}, flagGroupsExclusive: []string{"f g"}, args: []string{"--a=foo", "--e=foo", "--f=foo", "--g=foo"}, expectErr: `if any flags in the group [f g] are set none of the others can be; [f g] were all set`, }, { - desc: "Persistent flags utilize both features and can pass", + desc: "Persistent flags utilize required and exclusive groups and can pass", flagGroupsRequired: []string{"a e", "e f"}, flagGroupsExclusive: []string{"f g"}, args: []string{"--a=foo", "--e=foo", "--f=foo"}, + }, { + desc: "Persistent flags utilize one-required and exclusive groups and can pass", + flagGroupsOneRequired: []string{"a e", "e f"}, + flagGroupsExclusive: []string{"f g"}, + args: []string{"--a=foo", "--e=foo", "--f=foo"}, }, { desc: "Subcmds can use required groups using inherited flags", subCmdFlagGroupsRequired: []string{"e subonly"}, args: []string{"subcmd", "--e=foo", "--subonly=foo"}, + }, { + desc: "Subcmds can use one-required groups using inherited flags", + subCmdFlagGroupsOneRequired: []string{"e subonly"}, + args: []string{"subcmd", "--e=foo", "--subonly=foo"}, + }, { + desc: "Subcmds can use one-required groups using inherited flags and fail one-required groups", + subCmdFlagGroupsOneRequired: []string{"e subonly"}, + args: []string{"subcmd"}, + expectErr: "at least one of the flags in the group [e subonly] is required", }, { desc: "Subcmds can use exclusive groups using inherited flags", subCmdFlagGroupsExclusive: []string{"e subonly"}, @@ -130,12 +167,18 @@ func TestValidateFlagGroups(t *testing.T) { for _, flagGroup := range tc.flagGroupsRequired { c.MarkFlagsRequiredTogether(strings.Split(flagGroup, " ")...) } + for _, flagGroup := range tc.flagGroupsOneRequired { + c.MarkFlagsOneRequired(strings.Split(flagGroup, " ")...) + } for _, flagGroup := range tc.flagGroupsExclusive { c.MarkFlagsMutuallyExclusive(strings.Split(flagGroup, " ")...) } for _, flagGroup := range tc.subCmdFlagGroupsRequired { sub.MarkFlagsRequiredTogether(strings.Split(flagGroup, " ")...) } + for _, flagGroup := range tc.subCmdFlagGroupsOneRequired { + sub.MarkFlagsOneRequired(strings.Split(flagGroup, " ")...) + } for _, flagGroup := range tc.subCmdFlagGroupsExclusive { sub.MarkFlagsMutuallyExclusive(strings.Split(flagGroup, " ")...) } diff --git a/go.mod b/go.mod index 6361d742..8c80da01 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/spf13/cobra go 1.15 require ( - github.com/cpuguy83/go-md2man/v2 v2.0.2 + github.com/cpuguy83/go-md2man/v2 v2.0.4 github.com/inconshreveable/mousetrap v1.1.0 github.com/spf13/pflag v1.0.5 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 5ccb69dd..ab40b433 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/cpuguy83/go-md2man/v2 v2.0.2 h1:p1EgwI/C7NhT0JmVkwCD2ZBK8j4aeHQX2pMHHBfMQ6w= -github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/cpuguy83/go-md2man/v2 v2.0.4 h1:wfIWP927BUkWJb2NmU/kNDYIBTh/ziUX91+lVfRxZq4= +github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= diff --git a/powershell_completions.go b/powershell_completions.go index 55195193..a830b7bc 100644 --- a/powershell_completions.go +++ b/powershell_completions.go @@ -28,8 +28,8 @@ import ( func genPowerShellComp(buf io.StringWriter, name string, includeDesc bool) { // Variables should not contain a '-' or ':' character nameForVar := name - nameForVar = strings.Replace(nameForVar, "-", "_", -1) - nameForVar = strings.Replace(nameForVar, ":", "_", -1) + nameForVar = strings.ReplaceAll(nameForVar, "-", "_") + nameForVar = strings.ReplaceAll(nameForVar, ":", "_") compCmd := ShellCompRequestCmd if !includeDesc { diff --git a/site/content/active_help.md b/site/content/active_help.md index 5e7f59af..d72acc72 100644 --- a/site/content/active_help.md +++ b/site/content/active_help.md @@ -92,7 +92,7 @@ Allowing to configure Active Help is entirely optional; you can use Active Help The way to configure Active Help is to use the program's Active Help environment variable. That variable is named `_ACTIVE_HELP` where `` is the name of your -program in uppercase with any `-` replaced by an `_`. The variable should be set by the user to whatever +program in uppercase with any non-ASCII-alphanumeric characters replaced by an `_`. The variable should be set by the user to whatever Active Help configuration values are supported by the program. For example, say `helm` has chosen to support three levels for Active Help: `on`, `off`, `local`. Then a user @@ -140,7 +140,7 @@ details for your users. Debugging your Active Help code is done in the same way as debugging your dynamic completion code, which is with Cobra's hidden `__complete` command. Please refer to [debugging shell completion](shell_completions.md#debugging) for details. -When debugging with the `__complete` command, if you want to specify different Active Help configurations, you should use the active help environment variable. That variable is named `_ACTIVE_HELP` where any `-` is replaced by an `_`. For example, we can test deactivating some Active Help as shown below: +When debugging with the `__complete` command, if you want to specify different Active Help configurations, you should use the active help environment variable. That variable is named `_ACTIVE_HELP` where any non-ASCII-alphanumeric characters are replaced by an `_`. For example, we can test deactivating some Active Help as shown below: ``` $ HELM_ACTIVE_HELP=1 bin/helm __complete install wordpress bitnami/h bitnami/haproxy diff --git a/site/content/completions/_index.md b/site/content/completions/_index.md index 4efad290..02257ade 100644 --- a/site/content/completions/_index.md +++ b/site/content/completions/_index.md @@ -393,6 +393,9 @@ $ source <(helm completion bash --no-descriptions) $ helm completion [tab][tab] bash fish powershell zsh ``` + +Setting the `_COMPLETION_DESCRIPTIONS` environment variable (falling back to `COBRA_COMPLETION_DESCRIPTIONS` if empty or not set) to a [falsey value](https://pkg.go.dev/strconv#ParseBool) achieves the same. `` is the name of your program with all non-ASCII-alphanumeric characters replaced by `_`. + ## Bash completions ### Dependencies diff --git a/site/content/projects_using_cobra.md b/site/content/projects_using_cobra.md index 8a291eb2..52e4e807 100644 --- a/site/content/projects_using_cobra.md +++ b/site/content/projects_using_cobra.md @@ -12,6 +12,7 @@ - [Datree](https://github.com/datreeio/datree) - [Delve](https://github.com/derekparker/delve) - [Docker (distribution)](https://github.com/docker/distribution) +- [Encore](https://encore.dev) - [Etcd](https://etcd.io/) - [Gardener](https://github.com/gardener/gardenctl) - [Giant Swarm's gsctl](https://github.com/giantswarm/gsctl) @@ -23,6 +24,7 @@ - [GoReleaser](https://goreleaser.com) - [Helm](https://helm.sh) - [Hugo](https://gohugo.io) +- [Incus](https://linuxcontainers.org/incus/) - [Infracost](https://github.com/infracost/infracost) - [Istio](https://istio.io) - [Kool](https://github.com/kool-dev/kool) @@ -30,6 +32,7 @@ - [Kubescape](https://github.com/kubescape/kubescape) - [KubeVirt](https://github.com/kubevirt/kubevirt) - [Linkerd](https://linkerd.io/) +- [LXC](https://github.com/canonical/lxd) - [Mattermost-server](https://github.com/mattermost/mattermost-server) - [Mercure](https://mercure.rocks/) - [Meroxa CLI](https://github.com/meroxa/cli) @@ -55,10 +58,12 @@ - [Scaleway CLI](https://github.com/scaleway/scaleway-cli) - [Sia](https://github.com/SiaFoundation/siad) - [Skaffold](https://skaffold.dev/) +- [Taikun](https://taikun.cloud/) - [Tendermint](https://github.com/tendermint/tendermint) - [Twitch CLI](https://github.com/twitchdev/twitch-cli) - [UpCloud CLI (`upctl`)](https://github.com/UpCloudLtd/upcloud-cli) - [Vitess](https://vitess.io) - VMware's [Tanzu Community Edition](https://github.com/vmware-tanzu/community-edition) & [Tanzu Framework](https://github.com/vmware-tanzu/tanzu-framework) - [Werf](https://werf.io/) +- [Zarf](https://github.com/defenseunicorns/zarf) - [ZITADEL](https://github.com/zitadel/zitadel) diff --git a/site/content/user_guide.md b/site/content/user_guide.md index 04971431..93e87d66 100644 --- a/site/content/user_guide.md +++ b/site/content/user_guide.md @@ -3,7 +3,7 @@ While you are welcome to provide your own organization, typically a Cobra-based application will follow the following organizational structure: -``` +```test ▾ appName/ ▾ cmd/ add.go @@ -301,6 +301,7 @@ command := cobra.Command{ ### Bind Flags with Config You can also bind your flags with [viper](https://github.com/spf13/viper): + ```go var author string @@ -320,12 +321,14 @@ More in [viper documentation](https://github.com/spf13/viper#working-with-flags) Flags are optional by default. If instead you wish your command to report an error when a flag has not been set, mark it as required: + ```go rootCmd.Flags().StringVarP(&Region, "region", "r", "", "AWS region (required)") rootCmd.MarkFlagRequired("region") ``` Or, for persistent flags: + ```go rootCmd.PersistentFlags().StringVarP(&Region, "region", "r", "", "AWS region (required)") rootCmd.MarkPersistentFlagRequired("region") @@ -335,6 +338,7 @@ rootCmd.MarkPersistentFlagRequired("region") If you have different flags that must be provided together (e.g. if they provide the `--username` flag they MUST provide the `--password` flag as well) then Cobra can enforce that requirement: + ```go rootCmd.Flags().StringVarP(&u, "username", "u", "", "Username (required if password is set)") rootCmd.Flags().StringVarP(&pw, "password", "p", "", "Password (required if username is set)") @@ -343,13 +347,24 @@ rootCmd.MarkFlagsRequiredTogether("username", "password") You can also prevent different flags from being provided together if they represent mutually exclusive options such as specifying an output format as either `--json` or `--yaml` but never both: + ```go rootCmd.Flags().BoolVar(&ofJson, "json", false, "Output in JSON") rootCmd.Flags().BoolVar(&ofYaml, "yaml", false, "Output in YAML") rootCmd.MarkFlagsMutuallyExclusive("json", "yaml") ``` -In both of these cases: +If you want to require at least one flag from a group to be present, you can use `MarkFlagsOneRequired`. +This can be combined with `MarkFlagsMutuallyExclusive` to enforce exactly one flag from a given group: + +```go +rootCmd.Flags().BoolVar(&ofJson, "json", false, "Output in JSON") +rootCmd.Flags().BoolVar(&ofYaml, "yaml", false, "Output in YAML") +rootCmd.MarkFlagsOneRequired("json", "yaml") +rootCmd.MarkFlagsMutuallyExclusive("json", "yaml") +``` + +In these cases: - both local and persistent flags can be used - **NOTE:** the group is only enforced on commands where every flag is defined - a flag may appear in multiple groups @@ -419,7 +434,7 @@ by not providing a 'Run' for the 'rootCmd'. We have only defined one flag for a single command. -More documentation about flags is available at https://github.com/spf13/pflag +More documentation about flags is available at https://github.com/spf13/pflag. ```go package main @@ -587,9 +602,15 @@ Running an application with the '--version' flag will print the version to stdou the version template. The template can be customized using the `cmd.SetVersionTemplate(s string)` function. +## Error Message Prefix + +Cobra prints an error message when receiving a non-nil error value. +The default error message is `Error: `. +The Prefix, `Error:` can be customized using the `cmd.SetErrPrefix(s string)` function. + ## PreRun and PostRun Hooks -It is possible to run functions before or after the main `Run` function of your command. The `PersistentPreRun` and `PreRun` functions will be executed before `Run`. `PersistentPostRun` and `PostRun` will be executed after `Run`. The `Persistent*Run` functions will be inherited by children if they do not declare their own. These functions are run in the following order: +It is possible to run functions before or after the main `Run` function of your command. The `PersistentPreRun` and `PreRun` functions will be executed before `Run`. `PersistentPostRun` and `PostRun` will be executed after `Run`. The `Persistent*Run` functions will be inherited by children if they do not declare their own. The `*PreRun` and `*PostRun` functions will only be executed if the `Run` function of the current command has been declared. These functions are run in the following order: - `PersistentPreRun` - `PreRun` @@ -672,6 +693,10 @@ Inside subCmd PostRun with args: [arg1 arg2] Inside subCmd PersistentPostRun with args: [arg1 arg2] ``` +By default, only the first persistent hook found in the command chain is executed. +That is why in the above output, the `rootCmd PersistentPostRun` was not called for a child command. +Set `EnableTraverseRunHooks` global variable to `true` if you want to execute all parents' persistent hooks. + ## Suggestions when "unknown command" happens Cobra will print automatic suggestions when "unknown command" errors happen. This allows Cobra to behave similarly to the `git` command when a typo happens. For example: @@ -703,7 +728,7 @@ command.SuggestionsMinimumDistance = 1 You can also explicitly set names for which a given command will be suggested using the `SuggestFor` attribute. This allows suggestions for strings that are not close in terms of string distance, but make sense in your set of commands but for which you don't want aliases. Example: -``` +```bash $ kubectl remove Error: unknown command "remove" for "kubectl" @@ -729,3 +754,57 @@ Read more about it in [Shell Completions](completions/_index.md). Cobra makes use of the shell-completion system to define a framework allowing you to provide Active Help to your users. Active Help are messages (hints, warnings, etc) printed as the program is being used. Read more about it in [Active Help](active_help.md). + +## Creating a plugin + +When creating a plugin for tools like *kubectl*, the executable is named +`kubectl-myplugin`, but it is used as `kubectl myplugin`. To fix help +messages and completions, annotate the root command with the +`cobra.CommandDisplayNameAnnotation` annotation. + +### Example kubectl plugin + +```go +package main + +import ( + "fmt" + + "github.com/spf13/cobra" +) + +func main() { + rootCmd := &cobra.Command{ + Use: "kubectl-myplugin", + Annotations: map[string]string{ + cobra.CommandDisplayNameAnnotation: "kubectl myplugin", + }, + } + subCmd := &cobra.Command{ + Use: "subcmd", + Run: func(cmd *cobra.Command, args []string) { + fmt.Println("kubectl myplugin subcmd") + }, + } + rootCmd.AddCommand(subCmd) + rootCmd.Execute() +} +``` + +Example run as a kubectl plugin: + +```bash +$ kubectl myplugin +Usage: + kubectl myplugin [command] + +Available Commands: + completion Generate the autocompletion script for the specified shell + help Help about any command + subcmd + +Flags: + -h, --help help for kubectl myplugin + +Use "kubectl myplugin [command] --help" for more information about a command. +```