fixup! Cleanup code

coder · ammario · Mar 30, 2023 · Mar 29, 2023 · Mar 29, 2023 · Mar 29, 2023
commit eae71422502d9c12ae106fc1d950700b0f12e026
diff --git a/cli/clibase/cmd.go b/cli/clibase/cmd.go
@@ -218,17 +218,17 @@ func copyFlagSetWithout(fs *pflag.FlagSet, without string) *pflag.FlagSet {
 // run recursively executes the command and its children.
 // allArgs is wired through the stack so that global flags can be accepted
 // anywhere in the command invocation.
-func (i *Invocation) run(state *runState) error {
-	err := i.Command.Options.ParseEnv(i.Environ)
+func (inv *Invocation) run(state *runState) error {
+	err := inv.Command.Options.ParseEnv(inv.Environ)
 	if err != nil {
 		return xerrors.Errorf("parsing env: %w", err)
 	}
 
 	// Now the fun part, argument parsing!
 
 	children := make(map[string]*Cmd)
-	for _, child := range i.Command.Children {
-		child.Parent = i.Command
+	for _, child := range inv.Command.Children {
+		child.Parent = inv.Command
 		for _, name := range append(child.Aliases, child.Name()) {
 			if _, ok := children[name]; ok {
 				return xerrors.Errorf("duplicate command name: %s", name)
@@ -237,42 +237,42 @@ func (i *Invocation) run(state *runState) error {
 		}
 	}
 
-	if i.parsedFlags == nil {
-		i.parsedFlags = pflag.NewFlagSet(i.Command.Name(), pflag.ContinueOnError)
+	if inv.parsedFlags == nil {
+		inv.parsedFlags = pflag.NewFlagSet(inv.Command.Name(), pflag.ContinueOnError)
 		// We handle Usage ourselves.
-		i.parsedFlags.Usage = func() {}
+		inv.parsedFlags.Usage = func() {}
 	}
 
 	// If we find a duplicate flag, we want the deeper command's flag to override
 	// the shallow one. Unfortunately, pflag has no way to remove a flag, so we
 	// have to create a copy of the flagset without a value.
-	i.Command.Options.FlagSet().VisitAll(func(f *pflag.Flag) {
-		if i.parsedFlags.Lookup(f.Name) != nil {
-			i.parsedFlags = copyFlagSetWithout(i.parsedFlags, f.Name)
+	inv.Command.Options.FlagSet().VisitAll(func(f *pflag.Flag) {
+		if inv.parsedFlags.Lookup(f.Name) != nil {
+			inv.parsedFlags = copyFlagSetWithout(inv.parsedFlags, f.Name)
 		}
-		i.parsedFlags.AddFlag(f)
+		inv.parsedFlags.AddFlag(f)
 	})
 
 	var parsedArgs []string
 
-	if !i.Command.RawArgs {
+	if !inv.Command.RawArgs {
 		// Flag parsing will fail on intermediate commands in the command tree,
 		// so we check the error after looking for a child command.
-		state.flagParseErr = i.parsedFlags.Parse(state.allArgs)
-		parsedArgs = i.parsedFlags.Args()
+		state.flagParseErr = inv.parsedFlags.Parse(state.allArgs)
+		parsedArgs = inv.parsedFlags.Args()
 	}
 
 	// Set defaults for flags that weren't set by the user.
-	skipDefaults := make(map[string]struct{}, len(i.Command.Options))
-	for _, opt := range i.Command.Options {
-		if fl := i.parsedFlags.Lookup(opt.Flag); fl != nil && fl.Changed {
-			skipDefaults[opt.Name] = struct{}{}
+	skipDefaults := make(map[int]struct{}, len(inv.Command.Options))
+	for i, opt := range inv.Command.Options {
+		if fl := inv.parsedFlags.Lookup(opt.Flag); fl != nil && fl.Changed {
+			skipDefaults[i] = struct{}{}
 		}
 		if opt.envChanged {
-			skipDefaults[opt.Name] = struct{}{}
+			skipDefaults[i] = struct{}{}
 		}
 	}
-	err = i.Command.Options.SetDefaults(skipDefaults)
+	err = inv.Command.Options.SetDefaults(skipDefaults)
 	if err != nil {
 		return xerrors.Errorf("setting defaults: %w", err)
 	}
@@ -283,64 +283,64 @@ func (i *Invocation) run(state *runState) error {
 	if len(parsedArgs) > state.commandDepth {
 		nextArg := parsedArgs[state.commandDepth]
 		if child, ok := children[nextArg]; ok {
-			child.Parent = i.Command
-			i.Command = child
+			child.Parent = inv.Command
+			inv.Command = child
 			state.commandDepth++
-			return i.run(state)
+			return inv.run(state)
 		}
 	}
 
 	// Flag parse errors are irrelevant for raw args commands.
-	if !i.Command.RawArgs && state.flagParseErr != nil && !errors.Is(state.flagParseErr, pflag.ErrHelp) {
+	if !inv.Command.RawArgs && state.flagParseErr != nil && !errors.Is(state.flagParseErr, pflag.ErrHelp) {
 		return xerrors.Errorf(
 			"parsing flags (%v) for %q: %w",
 			state.allArgs,
-			i.Command.FullName(), state.flagParseErr,
+			inv.Command.FullName(), state.flagParseErr,
 		)
 	}
 
-	if i.Command.RawArgs {
+	if inv.Command.RawArgs {
 		// If we're at the root command, then the name is omitted
 		// from the arguments, so we can just use the entire slice.
 		if state.commandDepth == 0 {
-			i.Args = state.allArgs
+			inv.Args = state.allArgs
 		} else {
-			argPos, err := findArg(i.Command.Name(), state.allArgs, i.parsedFlags)
+			argPos, err := findArg(inv.Command.Name(), state.allArgs, inv.parsedFlags)
 			if err != nil {
 				panic(err)
 			}
-			i.Args = state.allArgs[argPos+1:]
+			inv.Args = state.allArgs[argPos+1:]
 		}
 	} else {
 		// In non-raw-arg mode, we want to skip over flags.
-		i.Args = parsedArgs[state.commandDepth:]
+		inv.Args = parsedArgs[state.commandDepth:]
 	}
 
-	mw := i.Command.Middleware
+	mw := inv.Command.Middleware
 	if mw == nil {
 		mw = Chain()
 	}
 
-	ctx := i.ctx
+	ctx := inv.ctx
 	if ctx == nil {
 		ctx = context.Background()
 	}
 
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
-	i = i.WithContext(ctx)
+	inv = inv.WithContext(ctx)
 
-	if i.Command.Handler == nil || errors.Is(state.flagParseErr, pflag.ErrHelp) {
-		if i.Command.HelpHandler == nil {
-			return xerrors.Errorf("no handler or help for command %s", i.Command.FullName())
+	if inv.Command.Handler == nil || errors.Is(state.flagParseErr, pflag.ErrHelp) {
+		if inv.Command.HelpHandler == nil {
+			return xerrors.Errorf("no handler or help for command %s", inv.Command.FullName())
 		}
-		return i.Command.HelpHandler(i)
+		return inv.Command.HelpHandler(inv)
 	}
 
-	err = mw(i.Command.Handler)(i)
+	err = mw(inv.Command.Handler)(inv)
 	if err != nil {
 		return &RunCommandError{
-			Cmd: i.Command,
+			Cmd: inv.Command,
 			Err: err,
 		}
 	}

diff --git a/cli/clibase/option.go b/cli/clibase/option.go
@@ -146,25 +146,19 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
 	return merr.ErrorOrNil()
 }
 
-// SetDefaults sets the default values for each Option.
-// It should be called before all parsing (e.g. ParseFlags, ParseEnv).
-func (s *OptionSet) SetDefaults(skip map[string]struct{}) error {
+// SetDefaults sets the default values for each Option, skipping values
+// that have already been set as indiciated by the skip map.
+func (s *OptionSet) SetDefaults(skip map[int]struct{}) error {
 	if s == nil {
 		return nil
 	}
 
 	var merr *multierror.Error
 
-	for _, opt := range *s {
+	for i, opt := range *s {
 		// Skip values that may have already been set by the user.
 		if len(skip) > 0 {
-			if opt.Name == "" {
-				merr = multierror.Append(
-					merr, xerrors.Errorf("parse: no Name field set"),
-				)
-				continue
-			}
-			if _, ok := skip[opt.Name]; ok {
+			if _, ok := skip[i]; ok {
 				continue
 			}
 		}