From 1f495bb3c893f449f67453eba8f2ab17d61eafd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Tue, 9 May 2023 23:55:29 +0300 Subject: [PATCH 1/6] Avoid redundant string splits There likely isn't actually more than once to split in the source strings in these cases, but avoid doing so anyway as we're only interested in the first. --- args.go | 2 +- bash_completions.go | 2 +- completions.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/args.go b/args.go index e79ec33a8..b7674c881 100644 --- a/args.go +++ b/args.go @@ -54,7 +54,7 @@ func OnlyValidArgs(cmd *Command, args []string) error { // A description is following a tab character. var validArgs []string 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 8a5315184..be835704b 100644 --- a/bash_completions.go +++ b/bash_completions.go @@ -621,7 +621,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/completions.go b/completions.go index 55a9f62c3..b76e54ad8 100644 --- a/completions.go +++ b/completions.go @@ -226,14 +226,14 @@ func (c *Command) initCompleteCmd(args []string) { } 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. From 79fb3dd0cf9abf1c62d1bf48e7df18421e6ab261 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 10 May 2023 00:00:43 +0300 Subject: [PATCH 2/6] Avoid redundant completion output target evaluations The target is not to be changed while outputting completions, so resolve it only once. --- completions.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/completions.go b/completions.go index b76e54ad8..f94018886 100644 --- a/completions.go +++ b/completions.go @@ -217,6 +217,7 @@ func (c *Command) initCompleteCmd(args []string) { } noDescriptions := (cmd.CalledAs() == ShellCompNoDescRequestCmd) + out := finalCmd.OutOrStdout() for _, comp := range completions { if GetActiveHelpConfig(finalCmd) == activeHelpGlobalDisable { // Remove all activeHelp entries in this case @@ -242,14 +243,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. From 3f1c31c21e43ba597e6533ae1208d8cc9fe735ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 10 May 2023 00:03:16 +0300 Subject: [PATCH 3/6] Avoid redundant active help enablement evaluations The enablement state is not to be changed during completion output, so evaluate it only once. --- completions.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/completions.go b/completions.go index f94018886..95e1dc1fb 100644 --- a/completions.go +++ b/completions.go @@ -217,13 +217,12 @@ func (c *Command) initCompleteCmd(args []string) { } noDescriptions := (cmd.CalledAs() == ShellCompNoDescRequestCmd) + 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. From 34416a2637176496d7810a6fd59dfbf7fe612796 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Thu, 5 Oct 2023 23:48:03 +0300 Subject: [PATCH 4/6] Preallocate some slices and maps with known size --- args.go | 2 +- command.go | 2 +- flag_groups.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/args.go b/args.go index b7674c881..ed1e70cea 100644 --- a/args.go +++ b/args.go @@ -52,7 +52,7 @@ 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.SplitN(v, "\t", 2)[0]) } diff --git a/command.go b/command.go index 6866f7d0f..1764a2178 100644 --- a/command.go +++ b/command.go @@ -701,7 +701,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 diff --git a/flag_groups.go b/flag_groups.go index 0671ec5f2..6e9383255 100644 --- a/flag_groups.go +++ b/flag_groups.go @@ -130,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 } From 3c47a5a9c87f431fd871d840dc0464de898bbed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Thu, 5 Oct 2023 23:51:52 +0300 Subject: [PATCH 5/6] Avoid some unnecessary looping --- cobra.go | 2 -- flag_groups.go | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/cobra.go b/cobra.go index f23f5092f..e403e762e 100644 --- a/cobra.go +++ b/cobra.go @@ -188,8 +188,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/flag_groups.go b/flag_groups.go index 6e9383255..2be3b18b1 100644 --- a/flag_groups.go +++ b/flag_groups.go @@ -253,17 +253,17 @@ 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 { - set := 0 + isSet := false - for _, isSet := range flagnameAndStatus { + for _, isSet = range flagnameAndStatus { if isSet { - set++ + break } } // None of the flags of the group are set, mark all flags in the group // as required - if set == 0 { + if !isSet { for _, fName := range strings.Split(flagList, " ") { _ = c.MarkFlagRequired(fName) } From 4585aea30f71d46423b6bffe8bdd359df42c567e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Thu, 5 Oct 2023 23:54:53 +0300 Subject: [PATCH 6/6] Use strings.Builder to construct suggestions --- command.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/command.go b/command.go index 1764a2178..d1fb2bedd 100644 --- a/command.go +++ b/command.go @@ -749,14 +749,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 {