Skip to content

Commit 0a36683

Browse files
More performance wins (#212)
1 parent a30cda3 commit 0a36683

7 files changed

Lines changed: 542 additions & 594 deletions

File tree

command.go

Lines changed: 33 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -379,47 +379,37 @@ func (cmd *Command) hasShortFlag(name string) bool {
379379
// findRequestedCommand uses the raw arguments and the command tree to determine what
380380
// (if any) subcommand is being requested and return that command along with the arguments
381381
// that were meant for it.
382+
//
383+
// On the first descent into a subcommand it snapshots args into a working
384+
// slice that we own; subsequent levels then mutate that slice in place via
385+
// [slices.Delete]. The original cmd.rawArgs is never touched, so re-Execute
386+
// on the same Command still sees pristine input.
382387
func findRequestedCommand(cmd *Command, args []string) (*Command, []string) {
383-
// The next non-flag argument (if any) is the first immediate subcommand
384-
// e.g. in 'go mod tidy' we're looking for 'mod'.
385-
nextSubCommand, ok := firstNonFlagArg(cmd, args)
386-
if !ok {
387-
// No non-flag arguments, so we must already be either at the root command
388-
// or the correct subcommand
389-
return cmd, args
390-
}
391-
392-
// Lookup this immediate subcommand by name and if we find it, recursively call
393-
// this function so we eventually end up at the end of the command tree with
394-
// the right arguments
395-
next := findSubCommand(cmd, nextSubCommand)
396-
if next != nil {
397-
return findRequestedCommand(next, argsMinusFirstX(args, nextSubCommand))
398-
}
388+
owned := false
389+
390+
for {
391+
// The next non-flag argument (if any) is the immediate subcommand
392+
// e.g. in 'go mod tidy' we're looking for 'mod'.
393+
idx, ok := firstNonFlagArg(cmd, args)
394+
if !ok {
395+
return cmd, args
396+
}
399397

400-
// Found it
401-
return cmd, args
402-
}
398+
next := findSubCommand(cmd, args[idx])
399+
if next == nil {
400+
return cmd, args
401+
}
403402

404-
// argsMinusFirstX removes only the first x from args. Otherwise, commands that look like
405-
// openshift admin policy add-role-to-user admin my-user, lose the admin argument (arg[4]).
406-
//
407-
// The input slice is not mutated so that repeated Execute calls on the same
408-
// Command see the original rawArgs.
409-
func argsMinusFirstX(args []string, x string) []string {
410-
// Note: this is borrowed from Cobra but ours is a lot simpler because we don't support
411-
// persistent flags
412-
for i, arg := range args {
413-
if arg == x {
414-
result := make([]string, 0, len(args)-1)
415-
result = append(result, args[:i]...)
416-
result = append(result, args[i+1:]...)
417-
418-
return result
403+
if !owned {
404+
working := make([]string, len(args))
405+
copy(working, args)
406+
args = working
407+
owned = true
419408
}
420-
}
421409

422-
return args
410+
args = slices.Delete(args, idx, idx+1)
411+
cmd = next
412+
}
423413
}
424414

425415
// findSubCommand searches the immediate subcommands of cmd by name, looking for next.
@@ -435,37 +425,37 @@ func findSubCommand(cmd *Command, next string) *Command {
435425
return nil
436426
}
437427

438-
// firstNonFlagArg walks args and returns the first positional (non-flag)
439-
// argument along with a boolean indicating whether one was found.
428+
// firstNonFlagArg walks args and returns the index of the first positional
429+
// (non-flag) argument along with a boolean indicating whether one was found.
440430
//
441431
// It consumes flag-value pairs (e.g. '--flag value' or '-f value') so they
442432
// aren't mistaken for positional arguments, and stops at '--'.
443-
func firstNonFlagArg(cmd *Command, args []string) (arg string, ok bool) {
433+
func firstNonFlagArg(cmd *Command, args []string) (idx int, ok bool) {
444434
for i := 0; i < len(args); i++ {
445435
a := args[i]
446436
switch {
447437
case a == "--":
448438
// "--" terminates the flags
449-
return "", false
439+
return -1, false
450440
case strings.HasPrefix(a, "--") && !strings.Contains(a, "=") && !cmd.hasFlag(a[2:]):
451441
// If '--flag value' then skip value
452442
fallthrough
453443
case strings.HasPrefix(a, "-") && !strings.Contains(a, "=") && len(a) == 2 && !cmd.hasShortFlag(a[1:]):
454444
// '-f value' skip the value too. If there isn't one, we're done.
455445
if i+1 >= len(args) {
456-
return "", false
446+
return -1, false
457447
}
458448

459449
i++
460450

461451
continue
462452
case a != "" && !strings.HasPrefix(a, "-"):
463453
// First valid positional arg
464-
return a, true
454+
return i, true
465455
}
466456
}
467457

468-
return "", false
458+
return -1, false
469459
}
470460

471461
// showHelp is the default for a command's helpFunc.

0 commit comments

Comments
 (0)