diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 85e84cf..6961a7e 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -8,6 +8,19 @@ on: types: [opened, synchronize, reopened] jobs: + lint: + name: Lint + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: stable + - name: Run golangci-lint + uses: golangci/golangci-lint-action@v7 + build: name: Build and test runs-on: ubuntu-latest diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..119ec42 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,11 @@ +version: "2" + +linters: + default: standard + exclusions: + paths: + - examples + +formatters: + enable: + - goimports diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..44c35d0 --- /dev/null +++ b/Makefile @@ -0,0 +1,17 @@ +.PHONY: build test lint format ci-test + +build: + go build -v . + +test: + go test $$(go list ./... | grep -v 'examples') -count=1 -v + +lint: + golangci-lint run ./... + +format: + goimports -w $$(find . -name '*.go' -not -path './examples/*') + +ci-test: + go test $$(go list ./... | grep -v 'examples') -count=1 -v -json -cover \ + | tparse -all -follow -sort=elapsed -trimpath=auto diff --git a/graceful/graceful.go b/graceful/graceful.go index c303370..9fa8891 100644 --- a/graceful/graceful.go +++ b/graceful/graceful.go @@ -100,7 +100,7 @@ func Run(run func(context.Context) error, opts ...Option) { if cfg.logger != nil { cfg.logger.Error("function error", slog.Any("error", err)) } else { - fmt.Fprintln(cfg.stderr, err) + _, _ = fmt.Fprintln(cfg.stderr, err) } exit(1) } @@ -113,7 +113,7 @@ func Run(run func(context.Context) error, opts ...Option) { if cfg.logger != nil { cfg.logger.Warn(msg) } else { - fmt.Fprintln(cfg.stderr, msg) + _, _ = fmt.Fprintln(cfg.stderr, msg) } exit(130) } @@ -127,7 +127,7 @@ func Run(run func(context.Context) error, opts ...Option) { if cfg.logger != nil { cfg.logger.Info(msg) } else { - fmt.Fprintln(cfg.stderr, msg) + _, _ = fmt.Fprintln(cfg.stderr, msg) } // Set up shutdown timeout if configured @@ -145,7 +145,7 @@ func Run(run func(context.Context) error, opts ...Option) { if cfg.logger != nil { cfg.logger.Error("function error", "error", err) } else { - fmt.Fprintln(cfg.stderr, err) + _, _ = fmt.Fprintln(cfg.stderr, err) } exit(1) } @@ -157,7 +157,7 @@ func Run(run func(context.Context) error, opts ...Option) { if cfg.logger != nil { cfg.logger.Warn(msg) } else { - fmt.Fprintln(cfg.stderr, msg) + _, _ = fmt.Fprintln(cfg.stderr, msg) } exit(130) @@ -167,7 +167,7 @@ func Run(run func(context.Context) error, opts ...Option) { if cfg.logger != nil { cfg.logger.Error(msg) } else { - fmt.Fprintln(cfg.stderr, msg) + _, _ = fmt.Fprintln(cfg.stderr, msg) } exit(124) } diff --git a/graceful/graceful_test.go b/graceful/graceful_test.go index 244a126..6ebfad7 100644 --- a/graceful/graceful_test.go +++ b/graceful/graceful_test.go @@ -40,7 +40,7 @@ func sendSignal(trigger <-chan struct{}, delay time.Duration) { if delay > 0 { time.Sleep(delay) } - syscall.Kill(syscall.Getpid(), syscall.SIGINT) + _ = syscall.Kill(syscall.Getpid(), syscall.SIGINT) } func TestRun_Success(t *testing.T) { diff --git a/parse.go b/parse.go index d892e67..b8efd54 100644 --- a/parse.go +++ b/parse.go @@ -36,32 +36,62 @@ func Parse(root *Command, args []string) error { // Reset command path but preserve other state root.state.path = []*Command{root} } - // First split args at the -- delimiter if present - var argsToParse []string - var remainingArgs []string + + argsToParse, remainingArgs := splitAtDelimiter(args) + + current, err := resolveCommandPath(root, argsToParse) + if err != nil { + return err + } + current.Flags.Usage = func() { /* suppress default usage */ } + + // Check for help flags after resolving the correct command + for _, arg := range argsToParse { + if arg == "-h" || arg == "--h" || arg == "-help" || arg == "--help" { + // Combine flags first so the help message includes all inherited flags + combineFlags(root.state.path) + return flag.ErrHelp + } + } + + combinedFlags := combineFlags(root.state.path) + + // Let ParseToEnd handle the flag parsing + if err := xflag.ParseToEnd(combinedFlags, argsToParse); err != nil { + return fmt.Errorf("command %q: %w", getCommandPath(root.state.path), err) + } + + if err := checkRequiredFlags(root.state.path, combinedFlags); err != nil { + return err + } + + root.state.Args = collectArgs(root.state.path, combinedFlags.Args(), remainingArgs) + + if current.Exec == nil { + return fmt.Errorf("command %q: no exec function defined", getCommandPath(root.state.path)) + } + return nil +} + +// splitAtDelimiter splits args at the first "--" delimiter. Returns the args before the delimiter +// and any args after it. +func splitAtDelimiter(args []string) (argsToParse, remaining []string) { for i, arg := range args { if arg == "--" { - argsToParse = args[:i] - remainingArgs = args[i+1:] - break + return args[:i], args[i+1:] } } - if argsToParse == nil { - argsToParse = args - } + return args, nil +} +// resolveCommandPath walks argsToParse to resolve the subcommand chain, building root.state.path +// and initializing flag sets along the way. Returns the terminal (deepest) command. +func resolveCommandPath(root *Command, argsToParse []string) (*Command, error) { current := root if current.Flags == nil { current.Flags = flag.NewFlagSet(root.Name, flag.ContinueOnError) } - var commandChain []*Command - commandChain = append(commandChain, root) - // Create combined flags with all parent flags - combinedFlags := flag.NewFlagSet(root.Name, flag.ContinueOnError) - combinedFlags.SetOutput(io.Discard) - - // First pass: process commands and build the flag set i := 0 for i < len(argsToParse) { arg := argsToParse[i] @@ -74,15 +104,24 @@ func Parse(root *Command, args []string) error { continue } - // Check if this flag expects a value + // Check if this flag expects a value across all commands in the chain (not just the + // current command), since flags from ancestor commands are inherited and can appear + // anywhere. name := strings.TrimLeft(arg, "-") - if f := current.Flags.Lookup(name); f != nil { - if _, isBool := f.Value.(interface{ IsBoolFlag() bool }); !isBool { - // Skip both flag and its value - i += 2 - continue + skipValue := false + for _, cmd := range root.state.path { + if f := cmd.Flags.Lookup(name); f != nil { + if _, isBool := f.Value.(interface{ IsBoolFlag() bool }); !isBool { + skipValue = true + } + break } } + if skipValue { + // Skip both flag and its value + i += 2 + continue + } i++ continue } @@ -95,73 +134,55 @@ func Parse(root *Command, args []string) error { sub.Flags = flag.NewFlagSet(sub.Name, flag.ContinueOnError) } current = sub - commandChain = append(commandChain, sub) i++ continue } - return current.formatUnknownCommandError(arg) + return nil, current.formatUnknownCommandError(arg) } break } - current.Flags.Usage = func() { /* suppress default usage */ } - - // Add the help check here, after we've found the correct command - hasHelp := false - for _, arg := range argsToParse { - if arg == "-h" || arg == "--h" || arg == "-help" || arg == "--help" { - hasHelp = true - break - } - } + return current, nil +} - // Add flags in reverse order for proper precedence - for i := len(commandChain) - 1; i >= 0; i-- { - cmd := commandChain[i] +// combineFlags merges flags from the command path into a single FlagSet. Flags are added in reverse +// order (deepest command first) so that child flags take precedence over parent flags. +func combineFlags(path []*Command) *flag.FlagSet { + combined := flag.NewFlagSet(path[0].Name, flag.ContinueOnError) + combined.SetOutput(io.Discard) + for i := len(path) - 1; i >= 0; i-- { + cmd := path[i] if cmd.Flags != nil { cmd.Flags.VisitAll(func(f *flag.Flag) { - if combinedFlags.Lookup(f.Name) == nil { - combinedFlags.Var(f.Value, f.Name, f.Usage) + if combined.Lookup(f.Name) == nil { + combined.Var(f.Value, f.Name, f.Usage) } }) } } - // Make sure to return help only after combining all flags, this way we get the full list of - // flags in the help message! - if hasHelp { - return flag.ErrHelp - } + return combined +} - // Let ParseToEnd handle the flag parsing - if err := xflag.ParseToEnd(combinedFlags, argsToParse); err != nil { - return fmt.Errorf("command %q: %w", getCommandPath(root.state.path), err) - } +// checkRequiredFlags verifies that all flags marked as required in FlagsMetadata were explicitly +// set during parsing. +func checkRequiredFlags(path []*Command, combined *flag.FlagSet) error { + // Build a set of flags that were explicitly set during parsing. Visit (unlike VisitAll) only + // iterates over flags that were actually provided by the user, regardless of their value. + setFlags := make(map[string]struct{}) + combined.Visit(func(f *flag.Flag) { + setFlags[f.Name] = struct{}{} + }) - // Check required flags var missingFlags []string - for _, cmd := range commandChain { - if len(cmd.FlagsMetadata) > 0 { - for _, flagMetadata := range cmd.FlagsMetadata { - if !flagMetadata.Required { - continue - } - flag := combinedFlags.Lookup(flagMetadata.Name) - if flag == nil { - return fmt.Errorf("command %q: internal error: required flag %s not found in flag set", getCommandPath(root.state.path), formatFlagName(flagMetadata.Name)) - } - if _, isBool := flag.Value.(interface{ IsBoolFlag() bool }); isBool { - isSet := false - for _, arg := range argsToParse { - if strings.HasPrefix(arg, "-"+flagMetadata.Name) || strings.HasPrefix(arg, "--"+flagMetadata.Name) { - isSet = true - break - } - } - if !isSet { - missingFlags = append(missingFlags, formatFlagName(flagMetadata.Name)) - } - } else if flag.Value.String() == flag.DefValue { - missingFlags = append(missingFlags, formatFlagName(flagMetadata.Name)) - } + for _, cmd := range path { + for _, flagMetadata := range cmd.FlagsMetadata { + if !flagMetadata.Required { + continue + } + if combined.Lookup(flagMetadata.Name) == nil { + return fmt.Errorf("command %q: internal error: required flag %s not found in flag set", getCommandPath(path), formatFlagName(flagMetadata.Name)) + } + if _, ok := setFlags[flagMetadata.Name]; !ok { + missingFlags = append(missingFlags, formatFlagName(flagMetadata.Name)) } } } @@ -170,40 +191,36 @@ func Parse(root *Command, args []string) error { if len(missingFlags) > 1 { msg += "s" } - return fmt.Errorf("command %q: %s %q not set", getCommandPath(root.state.path), msg, strings.Join(missingFlags, ", ")) + return fmt.Errorf("command %q: %s %q not set", getCommandPath(path), msg, strings.Join(missingFlags, ", ")) } + return nil +} - // Skip past command names in remaining args - parsed := combinedFlags.Args() +// collectArgs strips resolved command names from the parsed positional args and appends any args +// that appeared after the "--" delimiter. +func collectArgs(path []*Command, parsed, remaining []string) []string { + // Skip past command names in remaining args. Only strip the exact command names that were + // resolved during traversal (path[1:], since root never appears in user args), in order and + // only once each. startIdx := 0 - for _, arg := range parsed { - isCommand := false - for _, cmd := range commandChain { - if arg == cmd.Name { - startIdx++ - isCommand = true - break - } - } - if !isCommand { + chainIdx := 1 // Skip root + for startIdx < len(parsed) && chainIdx < len(path) { + if strings.EqualFold(parsed[startIdx], path[chainIdx].Name) { + startIdx++ + chainIdx++ + } else { break } } - // Combine remaining parsed args and everything after delimiter var finalArgs []string if startIdx < len(parsed) { finalArgs = append(finalArgs, parsed[startIdx:]...) } - if len(remainingArgs) > 0 { - finalArgs = append(finalArgs, remainingArgs...) + if len(remaining) > 0 { + finalArgs = append(finalArgs, remaining...) } - root.state.Args = finalArgs - - if current.Exec == nil { - return fmt.Errorf("command %q: no exec function defined", getCommandPath(root.state.path)) - } - return nil + return finalArgs } var validNameRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9_-]*$`) diff --git a/parse_test.go b/parse_test.go index dfeabc6..6e2bb29 100644 --- a/parse_test.go +++ b/parse_test.go @@ -13,11 +13,11 @@ import ( // testState is a helper struct to hold the commands for testing // -// root --verbose --version -// ├── add --dry-run -// └── nested --force -// └── sub --echo -// └── hello --mandatory-flag=false --another-mandatory-flag some-value +// root --verbose --version +// ├── add --dry-run +// └── nested --force +// └── sub --echo +// └── hello --mandatory-flag=false --another-mandatory-flag some-value type testState struct { add *Command nested, sub, hello *Command @@ -498,23 +498,23 @@ func TestParse(t *testing.T) { Exec: func(ctx context.Context, s *State) error { return nil }, } level4 := &Command{ - Name: "level4", + Name: "level4", SubCommands: []*Command{level5}, } level3 := &Command{ - Name: "level3", + Name: "level3", SubCommands: []*Command{level4}, } level2 := &Command{ - Name: "level2", + Name: "level2", SubCommands: []*Command{level3}, } level1 := &Command{ - Name: "level1", + Name: "level1", SubCommands: []*Command{level2}, } root := &Command{ - Name: "root", + Name: "root", SubCommands: []*Command{level1}, } err := Parse(root, []string{"level1", "level2", "level3", "level4", "level5"}) @@ -532,7 +532,7 @@ func TestParse(t *testing.T) { }) } root := &Command{ - Name: "root", + Name: "root", SubCommands: subcommands, } err := Parse(root, []string{"cmda"}) @@ -549,7 +549,7 @@ func TestParse(t *testing.T) { {Name: "duplicate", Exec: func(ctx context.Context, s *State) error { return nil }}, }, } - // This library may not check for duplicate names, so just verify it works + // This library may not check for duplicate names, so just verify it works err := Parse(cmd, []string{"duplicate"}) require.NoError(t, err) // Just ensure it doesn't crash and can parse the first match @@ -596,6 +596,88 @@ func TestParse(t *testing.T) { require.NoError(t, err) require.Equal(t, longArgList, cmd.state.Args) }) + t.Run("positional arg matching command name", func(t *testing.T) { + t.Parallel() + + add := &Command{ + Name: "add", + Exec: func(ctx context.Context, s *State) error { return nil }, + } + root := &Command{ + Name: "mycli", + SubCommands: []*Command{add}, + } + err := Parse(root, []string{"add", "add"}) + require.NoError(t, err) + assert.Equal(t, add, getCommand(t, root)) + // The second "add" is a positional arg, not a command traversal. + assert.Equal(t, []string{"add"}, root.state.Args) + }) + t.Run("ancestor flag value not treated as command", func(t *testing.T) { + t.Parallel() + + child := &Command{ + Name: "child", + Exec: func(ctx context.Context, s *State) error { return nil }, + } + parent := &Command{ + Name: "parent", + SubCommands: []*Command{child}, + } + root := &Command{ + Name: "mycli", + Flags: FlagsFunc(func(f *flag.FlagSet) { + f.String("output", "", "output file") + }), + SubCommands: []*Command{parent}, + } + + // Root flag --output used between parent and child: the value "foo" should be skipped + // during command resolution, not treated as an unknown command. + err := Parse(root, []string{"parent", "--output", "foo", "child"}) + require.NoError(t, err, "ancestor flag value should not be treated as unknown command") + assert.Equal(t, child, getCommand(t, root)) + assert.Equal(t, "foo", GetFlag[string](root.state, "output")) + }) + t.Run("required flag set to default value", func(t *testing.T) { + t.Parallel() + + root := &Command{ + Name: "mycli", + Flags: FlagsFunc(func(f *flag.FlagSet) { + f.String("port", "8080", "port number") + }), + FlagsMetadata: []FlagMetadata{ + {Name: "port", Required: true}, + }, + Exec: func(ctx context.Context, s *State) error { return nil }, + } + + // Explicitly passing the default value should satisfy the required check. + err := Parse(root, []string{"--port", "8080"}) + require.NoError(t, err, "explicitly setting required flag to its default value should not fail") + assert.Equal(t, "8080", GetFlag[string](root.state, "port")) + }) + t.Run("required bool flag prefix match not too broad", func(t *testing.T) { + t.Parallel() + + root := &Command{ + Name: "mycli", + Flags: FlagsFunc(func(f *flag.FlagSet) { + f.Bool("force", false, "force operation") + f.Bool("force-all", false, "force all") + }), + FlagsMetadata: []FlagMetadata{ + {Name: "force", Required: true}, + }, + Exec: func(ctx context.Context, s *State) error { return nil }, + } + + // --force-all should NOT satisfy the required --force flag. + err := Parse(root, []string{"--force-all"}) + require.Error(t, err, "--force-all should not satisfy required --force") + assert.Contains(t, err.Error(), "required flag") + }) t.Run("mixed flags and args in various orders", func(t *testing.T) { t.Parallel() cmd := &Command{ diff --git a/path_test.go b/path_test.go index 33d2c63..274168e 100644 --- a/path_test.go +++ b/path_test.go @@ -12,15 +12,15 @@ func TestCommandPath(t *testing.T) { t.Run("single command path", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "root", Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, nil) require.NoError(t, err) - + path := cmd.Path() require.Len(t, path, 1) require.Equal(t, "root", path[0].Name) @@ -28,30 +28,30 @@ func TestCommandPath(t *testing.T) { t.Run("nested command path", func(t *testing.T) { t.Parallel() - + child := &Command{ Name: "child", Exec: func(ctx context.Context, s *State) error { return nil }, } parent := &Command{ - Name: "parent", + Name: "parent", SubCommands: []*Command{child}, } root := &Command{ - Name: "root", + Name: "root", SubCommands: []*Command{parent}, } - + err := Parse(root, []string{"parent", "child"}) require.NoError(t, err) - + // Test path from root command (which contains state) path := root.Path() require.Len(t, path, 3) require.Equal(t, "root", path[0].Name) require.Equal(t, "parent", path[1].Name) require.Equal(t, "child", path[2].Name) - + // Navigate to terminal command to verify it's the child terminal := root.terminal() require.Equal(t, child, terminal) @@ -59,34 +59,34 @@ func TestCommandPath(t *testing.T) { t.Run("deeply nested command path", func(t *testing.T) { t.Parallel() - + level4 := &Command{ Name: "level4", Exec: func(ctx context.Context, s *State) error { return nil }, } level3 := &Command{ - Name: "level3", + Name: "level3", SubCommands: []*Command{level4}, } level2 := &Command{ - Name: "level2", + Name: "level2", SubCommands: []*Command{level3}, } level1 := &Command{ - Name: "level1", + Name: "level1", SubCommands: []*Command{level2}, } root := &Command{ - Name: "root", + Name: "root", SubCommands: []*Command{level1}, } - + err := Parse(root, []string{"level1", "level2", "level3", "level4"}) require.NoError(t, err) - + terminal := root.terminal() require.Equal(t, level4, terminal) - + path := root.Path() require.Len(t, path, 5) expected := []string{"root", "level1", "level2", "level3", "level4"} @@ -97,12 +97,12 @@ func TestCommandPath(t *testing.T) { t.Run("path before parsing", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "unparsed", Exec: func(ctx context.Context, s *State) error { return nil }, } - + // Path should return nil before parsing path := cmd.Path() require.Nil(t, path) @@ -110,33 +110,33 @@ func TestCommandPath(t *testing.T) { t.Run("path with command hierarchy not executed", func(t *testing.T) { t.Parallel() - + child := &Command{ Name: "child", Exec: func(ctx context.Context, s *State) error { return nil }, } parent := &Command{ - Name: "parent", + Name: "parent", SubCommands: []*Command{child}, - Exec: func(ctx context.Context, s *State) error { return nil }, + Exec: func(ctx context.Context, s *State) error { return nil }, } root := &Command{ - Name: "root", + Name: "root", SubCommands: []*Command{parent}, } - + // Parse to parent level, not child err := Parse(root, []string{"parent"}) require.NoError(t, err) - + terminal := root.terminal() require.Equal(t, parent, terminal) - + path := root.Path() require.Len(t, path, 2) require.Equal(t, "root", path[0].Name) require.Equal(t, "parent", path[1].Name) - + // Child's path should be nil since it hasn't been parsed in context childPath := child.Path() require.Nil(t, childPath) @@ -144,39 +144,39 @@ func TestCommandPath(t *testing.T) { t.Run("multiple sibling commands path", func(t *testing.T) { t.Parallel() - + child1 := &Command{ Name: "child1", Exec: func(ctx context.Context, s *State) error { return nil }, } child2 := &Command{ - Name: "child2", + Name: "child2", Exec: func(ctx context.Context, s *State) error { return nil }, } root := &Command{ - Name: "root", + Name: "root", SubCommands: []*Command{child1, child2}, } - + // Parse to first child err := Parse(root, []string{"child1"}) require.NoError(t, err) - + terminal := root.terminal() require.Equal(t, child1, terminal) - + path := root.Path() require.Len(t, path, 2) require.Equal(t, "root", path[0].Name) require.Equal(t, "child1", path[1].Name) - + // Parse to second child err = Parse(root, []string{"child2"}) require.NoError(t, err) - + terminal = root.terminal() require.Equal(t, child2, terminal) - + path = root.Path() require.Len(t, path, 2) require.Equal(t, "root", path[0].Name) @@ -185,23 +185,23 @@ func TestCommandPath(t *testing.T) { t.Run("command with complex names in path", func(t *testing.T) { t.Parallel() - + child := &Command{ Name: "complex-child_name", Exec: func(ctx context.Context, s *State) error { return nil }, } parent := &Command{ - Name: "parent-with-dashes", + Name: "parent-with-dashes", SubCommands: []*Command{child}, } root := &Command{ - Name: "root_with_underscores", + Name: "root_with_underscores", SubCommands: []*Command{parent}, } - + err := Parse(root, []string{"parent-with-dashes", "complex-child_name"}) require.NoError(t, err) - + path := root.Path() require.Len(t, path, 3) expected := []string{"root_with_underscores", "parent-with-dashes", "complex-child_name"} @@ -212,33 +212,33 @@ func TestCommandPath(t *testing.T) { t.Run("path consistency across multiple parses", func(t *testing.T) { t.Parallel() - + child := &Command{ Name: "child", Exec: func(ctx context.Context, s *State) error { return nil }, } parent := &Command{ - Name: "parent", + Name: "parent", SubCommands: []*Command{child}, - Exec: func(ctx context.Context, s *State) error { return nil }, + Exec: func(ctx context.Context, s *State) error { return nil }, } root := &Command{ - Name: "root", + Name: "root", SubCommands: []*Command{parent}, } - + // Parse multiple times to different levels err := Parse(root, []string{"parent"}) require.NoError(t, err) - + path1 := root.Path() require.Len(t, path1, 2) require.Equal(t, "root", path1[0].Name) require.Equal(t, "parent", path1[1].Name) - + err = Parse(root, []string{"parent", "child"}) require.NoError(t, err) - + path2 := root.Path() require.Len(t, path2, 3) require.Equal(t, "root", path2[0].Name) @@ -252,38 +252,38 @@ func TestTerminalCommand(t *testing.T) { t.Run("terminal command is root", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "root", Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, nil) require.NoError(t, err) - + terminal := cmd.terminal() require.Equal(t, cmd, terminal) }) t.Run("terminal command is nested", func(t *testing.T) { t.Parallel() - + child := &Command{ Name: "child", Exec: func(ctx context.Context, s *State) error { return nil }, } parent := &Command{ - Name: "parent", + Name: "parent", SubCommands: []*Command{child}, } root := &Command{ - Name: "root", + Name: "root", SubCommands: []*Command{parent}, } - + err := Parse(root, []string{"parent", "child"}) require.NoError(t, err) - + terminal := root.terminal() require.Equal(t, child, terminal) require.NotEqual(t, parent, terminal) @@ -292,35 +292,35 @@ func TestTerminalCommand(t *testing.T) { t.Run("terminal command with multiple levels", func(t *testing.T) { t.Parallel() - + deepest := &Command{ Name: "deepest", Exec: func(ctx context.Context, s *State) error { return nil }, } middle := &Command{ - Name: "middle", + Name: "middle", SubCommands: []*Command{deepest}, } root := &Command{ - Name: "root", - SubCommands: []*Command{middle}, + Name: "root", + SubCommands: []*Command{middle}, } - + err := Parse(root, []string{"middle", "deepest"}) require.NoError(t, err) - + terminal := root.terminal() require.Equal(t, deepest, terminal) }) t.Run("terminal command before parsing", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "unparsed", Exec: func(ctx context.Context, s *State) error { return nil }, } - + // terminal() should return the command itself before parsing terminal := cmd.terminal() require.Equal(t, cmd, terminal) @@ -328,25 +328,25 @@ func TestTerminalCommand(t *testing.T) { t.Run("terminal with partial command path", func(t *testing.T) { t.Parallel() - + child := &Command{ - Name: "child", + Name: "child", Exec: func(ctx context.Context, s *State) error { return nil }, } parent := &Command{ - Name: "parent", + Name: "parent", SubCommands: []*Command{child}, - Exec: func(ctx context.Context, s *State) error { return nil }, + Exec: func(ctx context.Context, s *State) error { return nil }, } root := &Command{ - Name: "root", + Name: "root", SubCommands: []*Command{parent}, } - + // Parse only to parent level err := Parse(root, []string{"parent"}) require.NoError(t, err) - + terminal := root.terminal() require.Equal(t, parent, terminal) require.NotEqual(t, child, terminal) diff --git a/run.go b/run.go index 45bfd47..109b7ab 100644 --- a/run.go +++ b/run.go @@ -118,9 +118,23 @@ func location(skip int) string { frame, _ := runtime.CallersFrames(pcs[:n]).Next() - // Trim the module name from both function and file paths for cleaner output + // Trim the module name from function and file paths for cleaner output. + // Function names use the module path directly (e.g., "github.com/pressly/cli.Run"). fn := strings.TrimPrefix(frame.Function, getGoModuleName()+"/") - file := strings.TrimPrefix(frame.File, getGoModuleName()+"/") + // File paths from runtime are absolute (e.g., "/Users/.../cli/run.go"). We want a relative + // path for cleaner output. Try to find the module's import path in the filesystem path + // (works with GOPATH-style layouts), otherwise fall back to just the base filename. + file := frame.File + mod := getGoModuleName() + if mod != "" { + if idx := strings.Index(file, mod+"/"); idx != -1 { + file = file[idx+len(mod)+1:] + } else { + file = file[strings.LastIndex(file, "/")+1:] + } + } else { + file = file[strings.LastIndex(file, "/")+1:] + } return fn + " " + file + ":" + strconv.Itoa(frame.Line) } diff --git a/run_test.go b/run_test.go index 6ce6b39..910cc6c 100644 --- a/run_test.go +++ b/run_test.go @@ -5,6 +5,7 @@ import ( "context" "errors" "flag" + "strings" "testing" "github.com/stretchr/testify/require" @@ -110,7 +111,7 @@ func TestRun(t *testing.T) { } err := Parse(root, nil) require.NoError(t, err) - err = Run(nil, root, nil) + err = Run(nil, root, nil) //nolint:staticcheck // intentionally testing nil context require.Error(t, err) require.Contains(t, err.Error(), "context is nil") }) @@ -173,7 +174,7 @@ func TestRun(t *testing.T) { } err := Parse(root, nil) require.NoError(t, err) - + stdout := bytes.NewBuffer(nil) stderr := bytes.NewBuffer(nil) err = Run(context.Background(), root, &RunOptions{ @@ -194,22 +195,31 @@ func TestRun(t *testing.T) { }), Exec: func(ctx context.Context, s *State) error { return nil }, } - + // Test max int err := Parse(root, []string{"--int", "2147483647"}) require.NoError(t, err) require.Equal(t, 2147483647, GetFlag[int](root.state, "int")) - + // Test min int err = Parse(root, []string{"--int", "-2147483648"}) require.NoError(t, err) require.Equal(t, -2147483648, GetFlag[int](root.state, "int")) - + // Test that parsing still works with large values (may not overflow in Go flag package) err = Parse(root, []string{"--int", "999999999"}) require.NoError(t, err) require.Equal(t, 999999999, GetFlag[int](root.state, "int")) }) + t.Run("location file path is relative", func(t *testing.T) { + t.Parallel() + loc := location(0) + // location returns "funcName file:line" + parts := strings.SplitN(loc, " ", 2) + require.Len(t, parts, 2, "location should return 'func file:line'") + // File path should be relative, not an absolute path + require.False(t, strings.HasPrefix(parts[1], "/"), "file path should be relative, not absolute: %s", parts[1]) + }) t.Run("string flags with special characters", func(t *testing.T) { t.Parallel() root := &Command{ @@ -219,7 +229,7 @@ func TestRun(t *testing.T) { }), Exec: func(ctx context.Context, s *State) error { return nil }, } - + specialValues := []string{ "text with spaces", "text\"with\"quotes", @@ -228,7 +238,7 @@ func TestRun(t *testing.T) { "text\twith\ttabs", "text@with#symbols$", } - + for _, val := range specialValues { err := Parse(root, []string{"--text", val}) require.NoError(t, err) diff --git a/usage_test.go b/usage_test.go index bd56a79..4b60a02 100644 --- a/usage_test.go +++ b/usage_test.go @@ -13,15 +13,15 @@ func TestUsageGeneration(t *testing.T) { t.Run("default usage with no flags", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "simple", Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.NotEmpty(t, output) require.Contains(t, output, "simple") @@ -30,7 +30,7 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage with flags", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "withflags", Flags: FlagsFunc(func(fset *flag.FlagSet) { @@ -40,10 +40,10 @@ func TestUsageGeneration(t *testing.T) { }), Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.Contains(t, output, "withflags") require.Contains(t, output, "-verbose") @@ -56,7 +56,7 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage with subcommands", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "parent", SubCommands: []*Command{ @@ -65,10 +65,10 @@ func TestUsageGeneration(t *testing.T) { }, Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.Contains(t, output, "parent") require.Contains(t, output, "child1") @@ -80,16 +80,16 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage with flags and subcommands", func(t *testing.T) { t.Parallel() - + cmd := &Command{ - Name: "complex", + Name: "complex", ShortHelp: "complex command with flags and subcommands", Flags: FlagsFunc(func(fset *flag.FlagSet) { fset.Bool("global", false, "global flag") }), SubCommands: []*Command{ { - Name: "sub", + Name: "sub", ShortHelp: "subcommand with its own flags", Flags: FlagsFunc(func(fset *flag.FlagSet) { fset.String("local", "", "local flag") @@ -99,10 +99,10 @@ func TestUsageGeneration(t *testing.T) { }, Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.Contains(t, output, "complex") require.Contains(t, output, "complex command with flags and subcommands") @@ -114,20 +114,20 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage with very long descriptions", func(t *testing.T) { t.Parallel() - + longDesc := "This is a very long description that should be wrapped properly when displayed in the usage output to ensure readability and proper formatting" cmd := &Command{ - Name: "longdesc", + Name: "longdesc", ShortHelp: longDesc, Flags: FlagsFunc(func(fset *flag.FlagSet) { fset.String("long-flag", "", longDesc) }), Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.Contains(t, output, "longdesc") require.Contains(t, output, "very long description") @@ -136,7 +136,7 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage with no subcommands but global flags", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "globalonly", Flags: FlagsFunc(func(fset *flag.FlagSet) { @@ -145,10 +145,10 @@ func TestUsageGeneration(t *testing.T) { }), Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.Contains(t, output, "globalonly") require.Contains(t, output, "-debug") @@ -159,25 +159,25 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage with many subcommands", func(t *testing.T) { t.Parallel() - + var subcommands []*Command for i := 0; i < 10; i++ { subcommands = append(subcommands, &Command{ - Name: "cmd" + string(rune('0'+i)), + Name: "cmd" + string(rune('0'+i)), ShortHelp: "command number " + string(rune('0'+i)), - Exec: func(ctx context.Context, s *State) error { return nil }, + Exec: func(ctx context.Context, s *State) error { return nil }, }) } - + cmd := &Command{ - Name: "manychildren", + Name: "manychildren", SubCommands: subcommands, - Exec: func(ctx context.Context, s *State) error { return nil }, + Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.Contains(t, output, "manychildren") for i := 0; i < 10; i++ { @@ -188,15 +188,15 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage with empty command structure", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "empty", Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.Contains(t, output, "empty") require.NotEmpty(t, output) @@ -204,28 +204,28 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage with nested command hierarchy", func(t *testing.T) { t.Parallel() - + child := &Command{ - Name: "child", + Name: "child", ShortHelp: "nested child command", - Exec: func(ctx context.Context, s *State) error { return nil }, + Exec: func(ctx context.Context, s *State) error { return nil }, } parent := &Command{ - Name: "parent", - ShortHelp: "parent command", + Name: "parent", + ShortHelp: "parent command", SubCommands: []*Command{child}, - Exec: func(ctx context.Context, s *State) error { return nil }, + Exec: func(ctx context.Context, s *State) error { return nil }, } root := &Command{ - Name: "root", - ShortHelp: "root command", + Name: "root", + ShortHelp: "root command", SubCommands: []*Command{parent}, - Exec: func(ctx context.Context, s *State) error { return nil }, + Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(root, []string{}) require.NoError(t, err) - + output := DefaultUsage(root) require.Contains(t, output, "root") require.Contains(t, output, "root command") @@ -238,7 +238,7 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage with mixed flag types", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "mixed", Flags: FlagsFunc(func(fset *flag.FlagSet) { @@ -249,16 +249,16 @@ func TestUsageGeneration(t *testing.T) { }), Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.Contains(t, output, "-bool-flag") require.Contains(t, output, "-string-flag") require.Contains(t, output, "-int-flag") require.Contains(t, output, "-float-flag") - + require.Contains(t, output, "boolean flag") require.Contains(t, output, "string flag") require.Contains(t, output, "integer flag") @@ -267,7 +267,7 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage before parsing", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "unparsed", Flags: FlagsFunc(func(fset *flag.FlagSet) { @@ -275,7 +275,7 @@ func TestUsageGeneration(t *testing.T) { }), Exec: func(ctx context.Context, s *State) error { return nil }, } - + // Usage should work even before parsing output := DefaultUsage(cmd) require.NotEmpty(t, output) @@ -284,23 +284,23 @@ func TestUsageGeneration(t *testing.T) { t.Run("usage with custom usage string", func(t *testing.T) { t.Parallel() - + cmd := &Command{ - Name: "custom", + Name: "custom", Usage: "custom [options] ", - Exec: func(ctx context.Context, s *State) error { return nil }, + Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.Contains(t, output, "custom [options] ") }) t.Run("usage with global and local flags", func(t *testing.T) { t.Parallel() - + child := &Command{ Name: "child", Flags: FlagsFunc(func(fset *flag.FlagSet) { @@ -315,10 +315,10 @@ func TestUsageGeneration(t *testing.T) { }), SubCommands: []*Command{child}, } - + err := Parse(parent, []string{"child"}) require.NoError(t, err) - + output := DefaultUsage(parent) require.Contains(t, output, "-local") require.Contains(t, output, "-global") @@ -332,7 +332,7 @@ func TestWriteFlagSection(t *testing.T) { t.Run("writeFlagSection helper function", func(t *testing.T) { t.Parallel() - + // Test the internal behavior through DefaultUsage since writeFlagSection is not exported cmd := &Command{ Name: "test", @@ -343,10 +343,10 @@ func TestWriteFlagSection(t *testing.T) { }), Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.Contains(t, output, "Flags:") require.Contains(t, output, "-verbose") @@ -355,7 +355,7 @@ func TestWriteFlagSection(t *testing.T) { require.Contains(t, output, "enable verbose output") require.Contains(t, output, "configuration file path") require.Contains(t, output, "number of worker threads") - + // Test default values are shown require.Contains(t, output, "(default: /etc/config)") require.Contains(t, output, "(default: 4)") @@ -363,15 +363,15 @@ func TestWriteFlagSection(t *testing.T) { t.Run("no flags section when no flags", func(t *testing.T) { t.Parallel() - + cmd := &Command{ Name: "noflag", Exec: func(ctx context.Context, s *State) error { return nil }, } - + err := Parse(cmd, []string{}) require.NoError(t, err) - + output := DefaultUsage(cmd) require.NotContains(t, output, "Flags:") require.NotContains(t, output, "Global Flags:")