From 5081c7f3a35974d84c0bb53d2c33da106cb6fab1 Mon Sep 17 00:00:00 2001 From: Mike Fridman Date: Fri, 21 Aug 2026 14:12:50 +0200 Subject: [PATCH 1/2] chore: modernize and simplify codebase --- cli.go | 236 +++++++++--------------------------- cli_test.go | 62 ++-------- examples/cmd/task/main.go | 1 - examples/cmd/task/tasks.go | 2 +- flagtype/doc.go | 19 +-- flagtype/enum.go | 10 +- flagtype/flagtype_test.go | 1 - flagtype/regexp.go | 5 +- flagtype/string_map.go | 16 +-- flagtype/string_slice.go | 5 +- flagtype/url.go | 5 +- graceful/graceful.go | 147 ++++------------------ graceful/graceful_test.go | 5 - internal/helpdoc/helpdoc.go | 7 +- internal/usage/help.go | 37 ++---- parse.go | 41 +------ pkg/suggest/suggest.go | 61 +++++----- pkg/textutil/textutil.go | 1 + xflag/doc.go | 5 +- xflag/parse_test.go | 1 - 20 files changed, 141 insertions(+), 526 deletions(-) diff --git a/cli.go b/cli.go index 88de39b..c9f07b4 100644 --- a/cli.go +++ b/cli.go @@ -1,40 +1,5 @@ // Package cli builds command-line programs on top of the standard library [flag] package. It adds -// nested subcommands and lets users place flags anywhere in command arguments. -// -// Features: -// - Nested subcommands via [Command.SubCommands] -// - Flags placed anywhere on the command line -// - Parent flags inherited by child commands -// - Type-safe flag access via [State.GetFlag] -// - Generated help, replaceable per command via [Command.Help] -// - "Did you mean" suggestions for misspelled subcommands -// -// Quick example: -// -// root := &cli.Command{ -// Name: "echo", -// Usage: "echo [flags] ...", -// Summary: "Print text", -// Description: "echo prints the provided text.", -// Flags: cli.FlagsFunc(func(f *flag.FlagSet) { -// f.Bool("c", false, "capitalize the input") -// }), -// Exec: func(ctx context.Context, s *cli.State) error { -// output := strings.Join(s.Args, " ") -// if s.GetFlag[bool]("c") { -// output = strings.ToUpper(output) -// } -// fmt.Fprintln(s.Stdout, output) -// return nil -// }, -// } -// if err := cli.ParseAndRun(ctx, root, os.Args[1:], nil); err != nil { -// fmt.Fprintf(os.Stderr, "error: %v\n", err) -// os.Exit(1) -// } -// -// The API is small on purpose. cli uses the standard library flag package instead of replacing it, -// so most of what you write is your program. +// nested subcommands, flags anywhere, inherited flags, generated help, and type-safe flag access. package cli import ( @@ -44,8 +9,10 @@ import ( "fmt" "io" "os" + "path/filepath" "runtime" "runtime/debug" + "slices" "strconv" "strings" "sync" @@ -55,87 +22,48 @@ import ( "github.com/pressly/cli/xflag" ) -// Command describes a single command in the CLI. -// -// Pass a Command to [ParseAndRun] (or [Parse] and [Run]) to run a program. To add a subcommand, -// list it in another command's [Command.SubCommands]. +// Command describes a command in a CLI. type Command struct { - // Name is the word users type to pick this command. It must start with a letter and can contain - // letters, digits, dashes, or underscores. For the root command it is also the program name - // shown in help. + // Name identifies the command. It must start with a letter and contain only letters, digits, + // dashes, or underscores. Name string - // Usage replaces the usage line shown at the top of help. Set it to show the expected - // arguments. The default usage line shows only the command path, plus "[flags]" when the - // command has flags. - // - // A common convention is to write required values as "", optional values as "[name]", and - // repeated values with "...". + // Usage overrides the generated usage line. Angle brackets usually mark required arguments, + // square brackets optional arguments, and an ellipsis repeated arguments. // - // Example: "todo list [flags]" - // Example: "todo add [flags]" - // Example: "todo remove " - // Example: "echo [flags] ..." - // Example: "serve [flags] [addr]" + // Usage: "echo [flags] ..." Usage string - // Summary is the one-line description shown next to this command in its parent's command list. - // It is also shown at the top of this command's own help when [Command.Description] is empty. - // - // Most commands only need Summary. Use [Command.Description] when one line is not enough. + // Summary is the one-line description used in command lists and, when Description is empty, in + // the command's help. Summary string - // Description is the longer help text shown at the top of this command's own help. Use it to - // explain behavior, defaults, or anything else worth knowing. - // - // When [Command.Summary] is empty, the first line of Description is used in command lists - // instead. + // Description is the command's longer help text. Its first line is used in command lists when + // Summary is empty. Description string - // Help replaces the built-in help text for this command. Leave it nil to use the default help. - // - // The function is given the command and returns the full help string. Help is used for --help - // and for [UsageErrorf] errors. Each command can set its own Help, and only the selected - // command's Help is called. + // Help overrides the generated help for this command. Help func(*Command) string - // Flags holds this command's flags as a standard library [flag.FlagSet]. Build it with - // [flag.NewFlagSet], or use [FlagsFunc] to define flags inline. - // - // Subcommands inherit these flags unless they are marked [FlagConfig.Local] in - // [Command.FlagConfigs]. Read flag values inside [Command.Exec] with [State.GetFlag]. + // Flags holds this command's [flag.FlagSet]. Subcommands inherit these flags unless they are + // marked [FlagConfig.Local]. Flags *flag.FlagSet - // FlagConfigs adds extra behavior to flags already defined in [Command.Flags]. See [FlagConfig] - // for the available options. - // - // Each entry must point to a flag defined in [Command.Flags]. Otherwise [Parse] returns an - // error. + // FlagConfigs adds behavior to flags already defined in Flags. FlagConfigs []FlagConfig - // SubCommands are the commands users can pick after this command's name. - // - // When a command has SubCommands, the first non-flag argument must match one of them. An - // unknown name returns an "unknown command" error with suggestions. Commands without - // SubCommands pass any non-flag arguments through to [State.Args]. Leave [Command.Exec] nil on - // a command that only groups subcommands; selecting it without a child returns a usage error. + // SubCommands are the commands available below this command. A command that only groups + // subcommands may leave Exec nil. SubCommands []*Command - // Exec is the function that runs when this command is picked. It is given a [State] holding the - // parsed inputs the command needs. - // - // Return [UsageErrorf] for bad arguments or flag combinations so [Run] prints the command's - // help to stderr. Return a normal error for everything else; [Run] returns it without printing - // help. + // Exec runs the selected command. Return [UsageErrorf] for invalid arguments or flag + // combinations so [Run] prints the command's help. Exec func(ctx context.Context, s *State) error state *State } -// Path returns the list of commands from the root down to this command. It is usually called inside -// [Command.Exec] as s.Cmd.Path() to build error messages that include the full command path. -// -// Path returns nil if called before [Parse]. +// Path returns the parsed command path from root to this command, or nil before [Parse]. func (c *Command) Path() []*Command { if c.state == nil { return nil @@ -143,67 +71,50 @@ func (c *Command) Path() []*Command { return c.state.path } -// FlagConfig adds extra behavior to a single flag already defined in [Command.Flags]. It is used as -// an entry in [Command.FlagConfigs]. +// FlagConfig adds behavior to a flag already defined in [Command.Flags]. type FlagConfig struct { - // Name is the long flag name as registered in the command's [flag.FlagSet]. + // Name is the flag's registered name. Name string - // Short is a one-letter alias for the flag, such as "v" so users can type -v instead of - // --verbose. Both forms are shown in help. + // Short is a one-letter alias, such as "v" for --verbose. Short string - // Required, when true, makes [Parse] fail unless the user sets the flag. The default value is - // not enough; the user must pass it. + // Required makes [Parse] fail unless the user explicitly sets the flag. Required bool - // Local, when true, keeps the flag on this command only and stops it from being inherited by - // subcommands. Parent flags are inherited by default. + // Local prevents subcommands from inheriting the flag. Local bool } // FlagName ties a flag name to the type returned by [State.GetFlag]. type FlagName[T any] string -// State is the value passed to [Command.Exec]. It holds the parsed inputs the command needs to run. +// State contains the parsed inputs passed to [Command.Exec]. type State struct { - // Args holds the positional arguments left after the command name and flags are parsed. - // Anything after "--" is included as-is, even if it looks like a flag. + // Args holds positional arguments. Anything after "--" is included as-is. Args []string - // Stdin, Stdout, and Stderr are the streams to use in your command code instead of os.Stdin, - // os.Stdout, and os.Stderr. Tests can swap them via [RunOptions]. + // Stdin, Stdout, and Stderr are the command's streams. Stdin io.Reader Stdout, Stderr io.Writer - // Cmd is the command that was picked. Call Cmd.Path() to get the full list of commands from the - // root down, useful for error messages that include the command path. + // Cmd is the selected command. Cmd *Command - // path is the command hierarchy from the root command to the current command. The root command - // is the first element in the path, and the terminal command is the last element. path []*Command } -// RunOptions replaces the standard streams used by [Run] and [ParseAndRun]. Pass nil for normal -// programs to use os.Stdin, os.Stdout, and os.Stderr. -// -// Use RunOptions in tests, or anywhere you need to capture output or supply your own input. +// RunOptions replaces the standard streams used by [Run] and [ParseAndRun]. type RunOptions struct { - // Stdin, Stdout, and Stderr replace os.Stdin, os.Stdout, and os.Stderr when set. A nil field - // falls back to its os equivalent. + // Nil fields default to the corresponding os stream. Stdin io.Reader Stdout, Stderr io.Writer } -// FlagsFunc creates a [flag.FlagSet] inline so you don't have to make one and assign it separately. -// The returned FlagSet uses [flag.ContinueOnError], so parsing errors are returned instead of being -// fatal. +// FlagsFunc builds a [flag.FlagSet] inline using [flag.ContinueOnError]. // // Flags: cli.FlagsFunc(func(f *flag.FlagSet) { // f.Bool("verbose", false, "enable verbose output") -// f.String("output", "", "output file") -// f.Int("count", 0, "number of items") // }), func FlagsFunc(fn func(f *flag.FlagSet)) (fset *flag.FlagSet) { fset = flag.NewFlagSet("", flag.ContinueOnError) @@ -217,8 +128,8 @@ func FlagsFunc(fn func(f *flag.FlagSet)) (fset *flag.FlagSet) { return fset } -// GetFlag returns a flag value as T, searching the picked command before its parents. Unknown names -// and type mismatches are programming errors: GetFlag panics, and [Run] returns the error. +// GetFlag returns a flag value as T, searching the selected command before its parents. Unknown +// names and type mismatches are programming errors: GetFlag panics, and [Run] returns the error. // // verbose := s.GetFlag[bool]("verbose") // const count FlagName[int] = "count" @@ -228,9 +139,7 @@ func (s *State) GetFlag[T any](name FlagName[T]) T { panic(&internalError{err: errors.New("state is nil")}) } flagName := string(name) - // Try to find the flag in each command's flag set, starting from the current command - for i := len(s.path) - 1; i >= 0; i-- { - cmd := s.path[i] + for _, cmd := range slices.Backward(s.path) { if cmd.Flags == nil { continue } @@ -247,13 +156,11 @@ func (s *State) GetFlag[T any](name FlagName[T]) T { value, *new(T), ) - // Flag exists but type doesn't match - this is an internal error panic(&internalError{err: err}) } } } - // If flag not found anywhere in hierarchy, panic with helpful message err := fmt.Errorf("flag %q not found in command %q flag set", formatFlagName(flagName), getCommandPath(s.path), @@ -261,12 +168,8 @@ func (s *State) GetFlag[T any](name FlagName[T]) T { panic(&internalError{err: err}) } -// Parse picks the right command and parses its flags from args, but does not run [Command.Exec]. -// Use Parse with [Run] when you need to do work between parsing and running. For the common case, -// call [ParseAndRun]. -// -// Parse returns [flag.ErrHelp] when the user passes -h or --help. You have to print the help -// yourself when this happens. [ParseAndRun] does it for you. +// Parse selects a command and parses its flags without running it. It returns [flag.ErrHelp] for -h +// or --help; [ParseAndRun] handles that case automatically. func Parse(root *Command, args []string) error { if root == nil { return errors.New("root command is nil") @@ -296,7 +199,6 @@ func Parse(root *Command, args []string) error { root.state.Cmd = current 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" { return flag.ErrHelp @@ -305,7 +207,6 @@ func Parse(root *Command, args []string) error { 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) } @@ -326,12 +227,8 @@ func Parse(root *Command, args []string) error { return nil } -// Run runs the command picked by a previous call to [Parse]. Use Run only when you call [Parse] -// separately. For the common case, use [ParseAndRun]. -// -// If [Command.Exec] returns an error created by [UsageErrorf], Run prints the command's help to -// stderr and returns the error you passed to [UsageErrorf]. Other errors are returned as-is. A nil -// ctx defaults to [context.Background]. +// Run executes the command selected by [Parse]. Usage errors print help to stderr; other errors are +// returned as-is. A nil ctx uses [context.Background]. func Run(ctx context.Context, root *Command, options *RunOptions) error { if ctx == nil { ctx = context.Background() @@ -344,7 +241,6 @@ func Run(ctx context.Context, root *Command, options *RunOptions) error { } cmd := root.terminal() if cmd == nil { - // This should never happen, but if it does, it's likely a bug in the Parse function. return errors.New("no terminal command found") } @@ -354,17 +250,13 @@ func Run(ctx context.Context, root *Command, options *RunOptions) error { return run(ctx, cmd, root.state) } -// ParseAndRun parses args, picks the right command, and runs its [Command.Exec]. This is the normal -// way to start a CLI program: +// ParseAndRun parses args and runs the selected command. It prints help and returns nil for -h or +// --help. // // if err := cli.ParseAndRun(ctx, root, os.Args[1:], nil); err != nil { // fmt.Fprintf(os.Stderr, "error: %v\n", err) // os.Exit(1) // } -// -// When the user passes -h or --help, ParseAndRun prints the picked command's help to stdout and -// returns nil. Use [Parse] and [Run] separately when you need to do work between parsing and -// running, such as setting up resources based on parsed flags. func ParseAndRun(ctx context.Context, root *Command, args []string, options *RunOptions) error { if err := Parse(root, args); err != nil { if errors.Is(err, flag.ErrHelp) { @@ -372,8 +264,7 @@ func ParseAndRun(ctx context.Context, root *Command, args []string, options *Run _, _ = fmt.Fprintln(options.Stdout, help(root)) return nil } - var usageErr *usageError - if errors.As(err, &usageErr) { + if usageErr, ok := errors.AsType[*usageError](err); ok { options = checkAndSetRunOptions(options) _, _ = fmt.Fprintf(options.Stderr, "%s\n\n", help(root)) return usageErr.Unwrap() @@ -383,15 +274,12 @@ func ParseAndRun(ctx context.Context, root *Command, args []string, options *Run return Run(ctx, root, options) } -// UsageErrorf returns an error that means the command was used incorrectly. Return it from -// [Command.Exec] when the command itself was right but the arguments or flag combination are wrong: +// UsageErrorf returns an error for invalid command arguments or flag combinations. [Run] prints the +// command's help before returning the underlying error. // // if len(s.Args) == 0 { // return cli.UsageErrorf("must supply a name") // } -// -// When [Run] sees a UsageErrorf error, it prints the command's help to stderr and returns the error -// message you passed in. Return a normal error if you do not want help printed. func UsageErrorf(format string, args ...any) error { return &usageError{err: fmt.Errorf(format, args...)} } @@ -413,9 +301,7 @@ func run(ctx context.Context, cmd *Command, state *State) (retErr error) { if r := recover(); r != nil { switch err := r.(type) { case error: - // If error is from cli package (e.g., flag type mismatch), don't add location info - var intErr *internalError - if errors.As(err, &intErr) { + if _, ok := errors.AsType[*internalError](err); ok { retErr = err } else { retErr = fmt.Errorf("panic: %v\n\n%s", err, location(4)) @@ -426,8 +312,7 @@ func run(ctx context.Context, cmd *Command, state *State) (retErr error) { } }() err := cmd.Exec(ctx, state) - var usageErr *usageError - if errors.As(err, &usageErr) { + if usageErr, ok := errors.AsType[*usageError](err); ok { _, _ = fmt.Fprintf(state.Stderr, "%s\n\n", help(state.Cmd)) return usageErr.Unwrap() } @@ -529,8 +414,7 @@ func helpFlagConfigs(configs []FlagConfig) []helpdoc.FlagConfig { return out } -// internalError is a marker type for errors that originate from the cli package itself. These are -// programming errors (e.g., flag type mismatches) that should be caught during development. +// internalError marks programmer errors that Run returns without adding a panic location. type internalError struct { err error } @@ -568,12 +452,9 @@ func (c *Command) terminal() *Command { if c.state == nil || len(c.state.path) == 0 { return c } - // Get the last command in the path - this is our terminal command return c.state.path[len(c.state.path)-1] } -// findSubCommand searches for a subcommand by name and returns it if found. Returns nil if no -// subcommand with the given name exists. func (c *Command) findSubCommand(name string) *Command { for _, sub := range c.SubCommands { if strings.EqualFold(sub.Name, name) { @@ -625,7 +506,7 @@ func getGoModuleName() string { func location(skip int) string { var pcs [1]uintptr - // Need to add 2 to skip to account for this function and runtime.Callers + // Skip location and runtime.Callers. n := runtime.Callers(skip+2, pcs[:]) if n == 0 { return "unknown:0" @@ -633,22 +514,13 @@ func location(skip int) string { frame, _ := runtime.CallersFrames(pcs[:n]).Next() - // 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 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() + fn := strings.TrimPrefix(frame.Function, mod+"/") + file := filepath.Base(frame.File) if mod != "" { - if idx := strings.Index(file, mod+"/"); idx != -1 { - file = file[idx+len(mod)+1:] - } else { - file = file[strings.LastIndex(file, "/")+1:] + if _, relative, ok := strings.Cut(frame.File, mod+"/"); ok { + file = relative } - } else { - file = file[strings.LastIndex(file, "/")+1:] } return fn + " " + file + ":" + strconv.Itoa(frame.Line) diff --git a/cli_test.go b/cli_test.go index 4be7e45..de88345 100644 --- a/cli_test.go +++ b/cli_test.go @@ -13,7 +13,7 @@ import ( "github.com/stretchr/testify/require" ) -// testState is a helper struct to hold the commands for testing +// testState defines this command tree: // // root --verbose --version // ├── add --dry-run @@ -350,7 +350,6 @@ func TestParse(t *testing.T) { require.ErrorContains(t, err, `command "todo nested hello": required flags "-mandatory-flag, -another-mandatory-flag" not set`) } { - // Correct type - true s := newTestState() err := Parse(s.root, []string{"nested", "hello", "--mandatory-flag=true", "--another-mandatory-flag", "some-value"}) require.NoError(t, err) @@ -360,7 +359,6 @@ func TestParse(t *testing.T) { require.True(t, s.root.state.GetFlag[bool]("mandatory-flag")) } { - // Correct type - false s := newTestState() err := Parse(s.root, []string{"nested", "hello", "--mandatory-flag=false", "--another-mandatory-flag=some-value"}) require.NoError(t, err) @@ -369,7 +367,6 @@ func TestParse(t *testing.T) { require.False(t, s.root.state.GetFlag[bool]("mandatory-flag")) } { - // Incorrect type s := newTestState() err := Parse(s.root, []string{"nested", "hello", "--mandatory-flag=not-a-bool"}) require.Error(t, err) @@ -634,7 +631,7 @@ func TestParse(t *testing.T) { t.Run("many subcommands", func(t *testing.T) { t.Parallel() var subcommands []*Command - for i := 0; i < 25; i++ { + for i := range 25 { subcommands = append(subcommands, &Command{ Name: "cmd" + string(rune('a'+i%26)), Exec: func(ctx context.Context, s *State) error { return nil }, @@ -658,10 +655,8 @@ 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 err := Parse(cmd, []string{"duplicate"}) require.NoError(t, err) - // Just ensure it doesn't crash and can parse the first match }) t.Run("flag config for non-existent flag", func(t *testing.T) { t.Parallel() @@ -698,7 +693,7 @@ func TestParse(t *testing.T) { Exec: func(ctx context.Context, s *State) error { return nil }, } var longArgList []string - for i := 0; i < 100; i++ { + for i := range 100 { longArgList = append(longArgList, "arg"+string(rune('0'+i%10))) } err := Parse(cmd, longArgList) @@ -762,7 +757,6 @@ func TestParse(t *testing.T) { 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", root.state.GetFlag[string]("port")) @@ -782,7 +776,6 @@ func TestParse(t *testing.T) { 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") @@ -886,10 +879,8 @@ func TestShortFlags(t *testing.T) { }, Exec: func(ctx context.Context, s *State) error { return nil }, } - // Use short flag err := Parse(cmd, []string{"-c", "42"}) require.NoError(t, err) - // Both short and long name should return the same value require.Equal(t, 42, cmd.state.GetFlag[int]("count")) }) @@ -985,12 +976,10 @@ func TestLocalFlags(t *testing.T) { SubCommands: []*Command{child}, Exec: func(ctx context.Context, s *State) error { return nil }, } - // --version on child should fail because it's local to root err := Parse(root, []string{"child", "--version"}) require.Error(t, err) require.ErrorContains(t, err, "flag provided but not defined") - // --verbose on child should still work (not local) root2 := &Command{ Name: "root", Flags: FlagsFunc(func(f *flag.FlagSet) { @@ -1045,11 +1034,9 @@ func TestLocalFlags(t *testing.T) { SubCommands: []*Command{child}, Exec: func(ctx context.Context, s *State) error { return nil }, } - // Child command should not require parent's local required flag err := Parse(root, []string{"child"}) require.NoError(t, err) - // But root command itself should still require it root2 := &Command{ Name: "root", Flags: FlagsFunc(func(f *flag.FlagSet) { @@ -1090,11 +1077,8 @@ func TestLocalFlags(t *testing.T) { require.ErrorIs(t, err, flag.ErrHelp) usage := help(root) - // --verbose should appear in inherited flags (not local) assert.Contains(t, usage, "--verbose") - // --version should NOT appear (local to root, not inherited) assert.NotContains(t, usage, "--version") - // --dry-run should appear in local flags assert.Contains(t, usage, "--dry-run") }) @@ -1115,7 +1099,6 @@ func TestLocalFlags(t *testing.T) { SubCommands: []*Command{child}, Exec: func(ctx context.Context, s *State) error { return nil }, } - // Short alias -V should also not work on child err := Parse(root, []string{"child", "-V"}) require.Error(t, err) require.ErrorContains(t, err, "flag provided but not defined") @@ -1169,14 +1152,12 @@ func TestCommandPath(t *testing.T) { 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) }) @@ -1227,7 +1208,6 @@ func TestCommandPath(t *testing.T) { Exec: func(ctx context.Context, s *State) error { return nil }, } - // Path should return nil before parsing path := cmd.Path() require.Nil(t, path) }) @@ -1249,7 +1229,6 @@ func TestCommandPath(t *testing.T) { SubCommands: []*Command{parent}, } - // Parse to parent level, not child err := Parse(root, []string{"parent"}) require.NoError(t, err) @@ -1261,7 +1240,6 @@ func TestCommandPath(t *testing.T) { 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) }) @@ -1282,7 +1260,6 @@ func TestCommandPath(t *testing.T) { SubCommands: []*Command{child1, child2}, } - // Parse to first child err := Parse(root, []string{"child1"}) require.NoError(t, err) @@ -1294,7 +1271,6 @@ func TestCommandPath(t *testing.T) { 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) @@ -1353,7 +1329,6 @@ func TestCommandPath(t *testing.T) { SubCommands: []*Command{parent}, } - // Parse multiple times to different levels err := Parse(root, []string{"parent"}) require.NoError(t, err) @@ -1447,7 +1422,6 @@ func TestTerminalCommand(t *testing.T) { 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) }) @@ -1469,7 +1443,6 @@ func TestTerminalCommand(t *testing.T) { SubCommands: []*Command{parent}, } - // Parse only to parent level err := Parse(root, []string{"parent"}) require.NoError(t, err) @@ -1540,13 +1513,11 @@ func TestRun(t *testing.T) { } err := Parse(root, nil) require.NoError(t, err) - // Run the command 3 times - for i := 0; i < 3; i++ { + for range 3 { err := Run(context.Background(), root, nil) require.NoError(t, err) } require.Equal(t, 3, count) - // Run with dry-run flag err = Parse(root, []string{"--dry-run"}) require.NoError(t, err) err = Run(context.Background(), root, nil) @@ -1610,7 +1581,6 @@ func TestRun(t *testing.T) { f.String("value", "default", "test value") }), Exec: func(ctx context.Context, s *State) error { - // Simulate concurrent access to state go func() { _ = s.GetFlag[string]("value") }() @@ -1659,17 +1629,14 @@ 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, root.state.GetFlag[int]("int")) - // Test min int err = Parse(root, []string{"--int", "-2147483648"}) require.NoError(t, err) require.Equal(t, -2147483648, root.state.GetFlag[int]("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, root.state.GetFlag[int]("int")) @@ -1677,10 +1644,8 @@ func TestRun(t *testing.T) { 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) { @@ -1817,7 +1782,6 @@ func TestStateGetFlag(t *testing.T) { require.True(t, ok) assert.ErrorContains(t, err, `flag "-version" not found in command "root" flag set`) }() - // Panic because author tried to access a flag that doesn't exist in any of the commands _ = state.GetFlag[string]("version") }) t.Run("flag type mismatch", func(t *testing.T) { @@ -1835,7 +1799,6 @@ func TestStateGetFlag(t *testing.T) { require.True(t, ok) assert.ErrorContains(t, err, `type mismatch for flag "-version" in command "root": registered string, requested int`) }() - // Panic because author tried to access a registered flag with the wrong type _ = state.GetFlag[int]("version") }) } @@ -2138,7 +2101,7 @@ func TestUsageGeneration(t *testing.T) { t.Parallel() var subcommands []*Command - for i := 0; i < 10; i++ { + for i := range 10 { subcommands = append(subcommands, &Command{ Name: "cmd" + string(rune('0'+i)), Description: "command number " + string(rune('0'+i)), @@ -2157,7 +2120,7 @@ func TestUsageGeneration(t *testing.T) { output := help(cmd) require.Contains(t, output, "manychildren") - for i := 0; i < 10; i++ { + for i := range 10 { require.Contains(t, output, "cmd"+string(rune('0'+i))) require.Contains(t, output, "command number "+string(rune('0'+i))) } @@ -2208,7 +2171,6 @@ func TestUsageGeneration(t *testing.T) { require.Contains(t, output, "root command") require.Contains(t, output, "parent") require.Contains(t, output, "parent command") - // Child should not appear in root's usage require.NotContains(t, output, "child") require.NotContains(t, output, "nested child command") }) @@ -2257,7 +2219,6 @@ func TestUsageGeneration(t *testing.T) { Exec: func(ctx context.Context, s *State) error { return nil }, } - // Usage should work even before parsing and show flags output := help(cmd) require.NotEmpty(t, output) require.Contains(t, output, "Flags:") @@ -2450,7 +2411,6 @@ func TestFlagHelp(t *testing.T) { require.Contains(t, output, "configuration file path") require.Contains(t, output, "number of worker threads") - // Non-zero defaults are shown require.Contains(t, output, "(default: /etc/config)") require.Contains(t, output, "(default: 4)") }) @@ -2473,15 +2433,12 @@ func TestFlagHelp(t *testing.T) { require.NoError(t, err) output := help(cmd) - // Zero-value defaults should not appear require.NotContains(t, output, "(default: false)") require.NotContains(t, output, "(default: 0)") require.NotContains(t, output, "(default: )") - // But non-bool flags should still have type hints require.Contains(t, output, "-output string") require.Contains(t, output, "-count int") require.Contains(t, output, "-rate float64") - // Bool flags should NOT have a type hint require.NotContains(t, output, "-verbose bool") }) @@ -2505,9 +2462,7 @@ func TestFlagHelp(t *testing.T) { output := help(cmd) require.Contains(t, output, "(required)") - // Required flag should not also show a default require.NotContains(t, output, "(default: )") - // Non-required flag with non-zero default should show default require.Contains(t, output, "(default: stdout)") }) @@ -2535,7 +2490,7 @@ func TestFlagHelp(t *testing.T) { require.Contains(t, output, "(required)") inFlags := false - for _, line := range strings.Split(output, "\n") { + for line := range strings.SplitSeq(output, "\n") { if line == "Flags:" { inFlags = true continue @@ -2570,10 +2525,8 @@ func TestFlagHelp(t *testing.T) { require.NoError(t, err) output := help(cmd) - // Flags with short aliases show both forms require.Contains(t, output, "-v, --verbose") require.Contains(t, output, "-o, --output string") - // Flags without short aliases are padded to align with double-dash require.Contains(t, output, " --config string") }) @@ -2593,7 +2546,6 @@ func TestFlagHelp(t *testing.T) { require.NoError(t, err) output := help(cmd) - // Without any short flags, no extra padding should be added require.Contains(t, output, " --verbose") require.Contains(t, output, " --config string") require.NotContains(t, output, " --verbose") diff --git a/examples/cmd/task/main.go b/examples/cmd/task/main.go index df77a08..5557fec 100644 --- a/examples/cmd/task/main.go +++ b/examples/cmd/task/main.go @@ -229,7 +229,6 @@ func taskRemove() *cli.Command { return nil } } - // add a confirmation prompt return Save(file, &TaskList{}) } return nil diff --git a/examples/cmd/task/tasks.go b/examples/cmd/task/tasks.go index 06592e3..69dfd70 100644 --- a/examples/cmd/task/tasks.go +++ b/examples/cmd/task/tasks.go @@ -13,7 +13,7 @@ type Task struct { ID int `json:"id,omitempty"` Text string `json:"text,omitempty"` Tags []string `json:"tags,omitempty"` - Created time.Time `json:"created,omitempty"` + Created time.Time `json:"created"` Status Status `json:"status,omitempty"` } diff --git a/flagtype/doc.go b/flagtype/doc.go index 7c0c144..5f5c8b8 100644 --- a/flagtype/doc.go +++ b/flagtype/doc.go @@ -1,27 +1,14 @@ -// Package flagtype provides common [flag.Value] implementations for use with [flag.FlagSet.Var]. -// -// All types implement [flag.Getter] so they work with [cli.State.GetFlag]. -// -// The following types are available: -// - [StringSlice] - repeatable flag that collects values into []string -// - [Enum] - restricts values to a predefined set, retrieved as string -// - [EnumDefault] - like [Enum] but with an initial default value -// - [StringMap] - repeatable flag that parses key=value pairs into map[string]string -// - [URL] - parses and validates a URL (must have scheme and host), retrieved as *url.URL -// - [Regexp] - compiles a regular expression, retrieved as *regexp.Regexp -// -// Example registration: +// Package flagtype provides common [flag.Value] implementations. Each also implements [flag.Getter] +// for use with cli.State.GetFlag. // // Flags: cli.FlagsFunc(func(f *flag.FlagSet) { // f.Var(flagtype.StringSlice(), "tag", "add a tag (repeatable)") // f.Var(flagtype.Enum("json", "yaml", "table"), "format", "output format") -// f.Var(flagtype.EnumDefault("sql", []string{"sql", "go"}), "type", "migration type") // f.Var(flagtype.StringMap(), "label", "key=value pair (repeatable)") // }) // -// Example retrieval in Exec: +// Inside Exec: // // tags := s.GetFlag[[]string]("tag") // format := s.GetFlag[string]("format") -// labels := s.GetFlag[map[string]string]("label") package flagtype diff --git a/flagtype/enum.go b/flagtype/enum.go index 9ae2827..277a419 100644 --- a/flagtype/enum.go +++ b/flagtype/enum.go @@ -12,18 +12,12 @@ type enumValue struct { allowed []string } -// Enum returns a [flag.Value] that restricts the flag to one of the allowed values. If a value not -// in the allowed list is provided, an error is returned listing valid options. -// -// Use [cli.State.GetFlag] with type string to retrieve the value. +// Enum returns a [flag.Value] restricted to allowed. func Enum(allowed ...string) flag.Value { return &enumValue{allowed: allowed} } -// EnumDefault is like [Enum] but sets an initial default value. The default must be one of the -// allowed values, otherwise EnumDefault panics. -// -// Use [cli.State.GetFlag] with type string to retrieve the value. +// EnumDefault is like [Enum] with a default value. It panics if defaultVal is not allowed. func EnumDefault(defaultVal string, allowed []string) flag.Value { if !slices.Contains(allowed, defaultVal) { panic(fmt.Sprintf("flagtype: default value %q is not in allowed values: %s", diff --git a/flagtype/flagtype_test.go b/flagtype/flagtype_test.go index c6ecf4a..edfc073 100644 --- a/flagtype/flagtype_test.go +++ b/flagtype/flagtype_test.go @@ -232,7 +232,6 @@ func TestRegexp(t *testing.T) { }) } -// nopWriter discards all writes, used to suppress flag.FlagSet error output in tests. type nopWriter struct{} func (nopWriter) Write(p []byte) (int, error) { return len(p), nil } diff --git a/flagtype/regexp.go b/flagtype/regexp.go index e82dcde..e865e91 100644 --- a/flagtype/regexp.go +++ b/flagtype/regexp.go @@ -9,10 +9,7 @@ type regexpValue struct { re *regexp.Regexp } -// Regexp returns a [flag.Value] that compiles the flag value as a regular expression. If the -// pattern is invalid, an error is returned. -// -// Use [cli.State.GetFlag] with type *regexp.Regexp to retrieve the value. +// Regexp returns a [flag.Value] that compiles its input as a regular expression. func Regexp() flag.Value { return ®expValue{} } diff --git a/flagtype/string_map.go b/flagtype/string_map.go index fc5a1da..8f3018c 100644 --- a/flagtype/string_map.go +++ b/flagtype/string_map.go @@ -3,7 +3,8 @@ package flagtype import ( "flag" "fmt" - "sort" + "maps" + "slices" "strings" ) @@ -11,11 +12,7 @@ type stringMapValue struct { m map[string]string } -// StringMap returns a [flag.Value] that parses key=value pairs into a map. The flag can be repeated -// to add multiple entries, like --label=env=prod --label=tier=web. The value is split on the first -// "=" character, so values may contain additional "=" characters. -// -// Use [cli.State.GetFlag] with type map[string]string to retrieve the value. +// StringMap returns a repeatable [flag.Value] that parses key=value pairs. Values may contain "=". func StringMap() flag.Value { return &stringMapValue{} } @@ -24,12 +21,7 @@ func (v *stringMapValue) String() string { if v.m == nil { return "" } - // Sort keys for deterministic output. - keys := make([]string, 0, len(v.m)) - for k := range v.m { - keys = append(keys, k) - } - sort.Strings(keys) + keys := slices.Sorted(maps.Keys(v.m)) pairs := make([]string, 0, len(keys)) for _, k := range keys { pairs = append(pairs, k+"="+v.m[k]) diff --git a/flagtype/string_slice.go b/flagtype/string_slice.go index 66a246a..8385d79 100644 --- a/flagtype/string_slice.go +++ b/flagtype/string_slice.go @@ -9,10 +9,7 @@ type stringSliceValue struct { vals []string } -// StringSlice returns a [flag.Value] that collects values into a string slice. Each time the flag -// is set, the value is appended. This allows repeatable flags like --tag=foo --tag=bar. -// -// Use [cli.State.GetFlag] with type []string to retrieve the value. +// StringSlice returns a repeatable [flag.Value] that collects values into a string slice. func StringSlice() flag.Value { return &stringSliceValue{} } diff --git a/flagtype/url.go b/flagtype/url.go index 9756ff6..bb6a89a 100644 --- a/flagtype/url.go +++ b/flagtype/url.go @@ -10,10 +10,7 @@ type urlValue struct { u *url.URL } -// URL returns a [flag.Value] that parses the flag value as a URL. The URL must have both a scheme -// and a host, otherwise an error is returned. -// -// Use [cli.State.GetFlag] with type *url.URL to retrieve the value. +// URL returns a [flag.Value] that requires a URL with a scheme and host. func URL() flag.Value { return &urlValue{} } diff --git a/graceful/graceful.go b/graceful/graceful.go index 9fa8891..2f781a6 100644 --- a/graceful/graceful.go +++ b/graceful/graceful.go @@ -1,51 +1,18 @@ -// Package graceful provides utilities for running long-lived processes with predictable, -// well-behaved shutdown semantics. It wraps a user-provided function with signal handling, context -// cancellation, timeouts, and standardized exit codes. +// Package graceful runs long-lived processes with signal handling and timeouts. // -// On the first SIGINT/SIGTERM, the context passed to the run function is canceled, giving the -// process an opportunity to shut down cleanly. A second signal forces an immediate exit. Optional -// timeouts bound both the maximum run duration (WithRunTimeout) and the total shutdown period -// (WithTerminationTimeout). For scenarios requiring immediate termination on the first signal, use -// WithImmediateTermination to bypass the graceful shutdown phase. +// The first interrupt cancels the run context; a second exits immediately. Run exits with status 0 +// on success, 1 on error, 124 on shutdown timeout, and 130 on forced shutdown. // -// Exit codes: -// - 0: successful completion -// - 1: run function returned an error -// - 124: shutdown timeout exceeded -// - 130: forced shutdown (second signal or immediate termination) -// -// Example: HTTP server +// Example: // // server := &http.Server{ -// Addr: ":8080", +// Addr: ":8080", // Handler: mux, // } -// // graceful.Run( -// graceful.ListenAndServe(server, 15*time.Second), // HTTP draining period -// graceful.WithTerminationTimeout(30*time.Second), // overall shutdown limit -// ) -// -// Example: batch job with a hard deadline -// -// graceful.Run(func(ctx context.Context) error { -// return processBatch(ctx) -// }, graceful.WithRunTimeout(1*time.Hour)) -// -// Example: worker with both limits -// -// graceful.Run(func(ctx context.Context) error { -// return runWorker(ctx) -// }, -// graceful.WithRunTimeout(24*time.Hour), +// graceful.ListenAndServe(server, 15*time.Second), // graceful.WithTerminationTimeout(30*time.Second), // ) -// -// Example: immediate termination on first signal -// -// graceful.Run(func(ctx context.Context) error { -// return runTask(ctx) -// }, graceful.WithImmediateTermination()) package graceful import ( @@ -62,14 +29,13 @@ import ( "time" ) -// osExit is a variable that can be mocked in tests. +// osExit is replaced in tests. var osExit = os.Exit func exit(code int) { osExit(code) } -// Run the provided function with signal handling and optional timeouts. See package documentation -// for details on signal handling, timeouts, and exit codes. -func Run(run func(context.Context) error, opts ...Option) { +// Run calls fn with signal handling and optional timeouts, then exits with the documented status. +func Run(fn func(context.Context) error, opts ...Option) { cfg := config{ stderr: os.Stderr, } @@ -77,11 +43,9 @@ func Run(run func(context.Context) error, opts ...Option) { opt(&cfg) } - // Main cancellation context (first signal) ctx, stop := signal.NotifyContext(context.Background(), interrupt()...) defer stop() - // Apply run timeout if configured if cfg.runTimeout > 0 { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, cfg.runTimeout) @@ -90,12 +54,11 @@ func Run(run func(context.Context) error, opts ...Option) { done := make(chan error, 1) go func() { - done <- run(ctx) + done <- fn(ctx) }() select { case err := <-done: - // fn completed before any signal if err != nil { if cfg.logger != nil { cfg.logger.Error("function error", slog.Any("error", err)) @@ -107,7 +70,6 @@ func Run(run func(context.Context) error, opts ...Option) { exit(0) case <-ctx.Done(): - // Check if immediate termination is requested if cfg.immediateTermination { msg := "immediate termination" if cfg.logger != nil { @@ -118,7 +80,7 @@ func Run(run func(context.Context) error, opts ...Option) { exit(130) } - // First signal received - NOW set up second signal detector + // Listen for a second signal only after the first has canceled ctx. second := make(chan os.Signal, 1) signal.Notify(second, interrupt()...) defer signal.Stop(second) @@ -130,7 +92,7 @@ func Run(run func(context.Context) error, opts ...Option) { _, _ = fmt.Fprintln(cfg.stderr, msg) } - // Set up shutdown timeout if configured + // A nil channel disables the timeout case below. var timeoutChan <-chan time.Time if cfg.shutdownTimeout > 0 { timer := time.NewTimer(cfg.shutdownTimeout) @@ -140,7 +102,6 @@ func Run(run func(context.Context) error, opts ...Option) { select { case err := <-done: - // fn completed during graceful shutdown if err != nil { if cfg.logger != nil { cfg.logger.Error("function error", "error", err) @@ -152,7 +113,6 @@ func Run(run func(context.Context) error, opts ...Option) { exit(0) case <-second: - // Second signal received msg := "forced shutdown" if cfg.logger != nil { cfg.logger.Warn(msg) @@ -162,7 +122,6 @@ func Run(run func(context.Context) error, opts ...Option) { exit(130) case <-timeoutChan: - // Shutdown timeout expired msg := "shutdown timeout exceeded" if cfg.logger != nil { cfg.logger.Error(msg) @@ -174,43 +133,14 @@ func Run(run func(context.Context) error, opts ...Option) { } } -// ListenAndServe runs an *http.Server under the lifecycle managed by graceful.Run. It starts the -// server, waits for ctx cancellation (SIGINT/SIGTERM), and then performs a graceful shutdown using -// http.Server.Shutdown. -// -// Shutdown behavior follows standard net/http semantics: -// - new connections are refused once shutdown begins -// - in-flight requests are allowed to finish normally -// - shutdownGrace bounds how long the server waits for draining -// -// ListenAndServe does not propagate the initial shutdown signal into handler contexts. Requests are -// only cancelled if the client disconnects or if shutdownGrace expires. This matches typical -// production environments and avoids mid-request interruptions. -// -// Two timeouts are involved: -// - shutdownGrace: how long the HTTP server may drain connections -// - graceful.WithTerminationTimeout: the total process shutdown budget -// -// Example: -// -// server := &http.Server{ -// Addr: ":8080", -// Handler: mux, -// } -// -// graceful.Run( -// graceful.ListenAndServe(server, 15*time.Second), // server draining period -// graceful.WithTerminationTimeout(25*time.Second), // total shutdown limit -// ) +// ListenAndServe runs srv until ctx is canceled, then drains it for up to shutdownGrace. The +// initial cancellation is not propagated to handler contexts. func ListenAndServe(srv *http.Server, shutdownGrace time.Duration) func(context.Context) error { return func(ctx context.Context) error { var wg sync.WaitGroup serverErr := make(chan error, 1) - // Run the HTTP/HTTPS server - wg.Add(1) - go func() { - defer wg.Done() + wg.Go(func() { var err error if srv.TLSConfig != nil { err = srv.ListenAndServeTLS("", "") @@ -220,9 +150,8 @@ func ListenAndServe(srv *http.Server, shutdownGrace time.Duration) func(context. if err != nil && err != http.ErrServerClosed { serverErr <- fmt.Errorf("listen: %w", err) } - }() + }) - // Wait for context cancellation or server error select { case err := <-serverErr: wg.Wait() @@ -231,20 +160,18 @@ func ListenAndServe(srv *http.Server, shutdownGrace time.Duration) func(context. shutdownCtx, cancel := context.WithTimeout(context.Background(), shutdownGrace) defer cancel() - // Shutdown the server if err := srv.Shutdown(shutdownCtx); err != nil { wg.Wait() return err } - // Wait for the server goroutine to finish wg.Wait() return nil } } } -// Option configures the Handle function. +// Option configures [Run]. type Option func(*config) type config struct { @@ -255,16 +182,14 @@ type config struct { immediateTermination bool } -// WithStderr sets the writer for error output. Defaults to os.Stderr if not specified. If a logger -// is configured via WithLogger, the logger takes precedence over stderr for messages. +// WithStderr sets the error output. A logger configured by [WithLogger] takes precedence. func WithStderr(w io.Writer) Option { return func(c *config) { c.stderr = w } } -// WithLogger sets an optional slog.Logger for structured logging. When provided, the logger is used -// instead of fmt.Fprintln to stderr for all messages (shutdown notifications, errors, etc.). +// WithLogger sends lifecycle messages to logger instead of stderr. // // To disable all logging output, pass a logger with a discard handler: // @@ -275,54 +200,28 @@ func WithLogger(logger *slog.Logger) Option { } } -// WithRunTimeout sets the maximum time the run function may execute. When the timeout expires, the -// context passed to the run function is canceled. If the function does not exit on cancellation, it -// will eventually be stopped by the termination timeout or a second interrupt signal. -// -// A zero or negative duration means no limit. -// -// Example: -// -// graceful.Run(processBatch, graceful.WithRunTimeout(1*time.Hour)) +// WithRunTimeout cancels the run context after d. A non-positive duration disables the limit. func WithRunTimeout(d time.Duration) Option { return func(c *config) { c.runTimeout = d } } -// WithTerminationTimeout sets the maximum time the process may spend shutting down after the first -// interrupt signal. If this timeout expires, the process exits with code 124. -// -// This bounds the total shutdown phase (server draining, cleanup, background work). A zero or -// negative duration means no limit. -// -// Example: -// -// graceful.Run(fn, graceful.WithTerminationTimeout(30*time.Second)) +// WithTerminationTimeout exits with status 124 if shutdown takes longer than d. A non-positive +// duration disables the limit. func WithTerminationTimeout(d time.Duration) Option { return func(c *config) { c.shutdownTimeout = d } } -// WithImmediateTermination configures the process to exit immediately on the first interrupt -// signal, without waiting for a second signal. By default, graceful shutdown allows a second Ctrl+C -// to force immediate termination. This option disables that behavior. -// -// When enabled, the first SIGINT/SIGTERM will cause the process to exit with code 130 immediately, -// without waiting for the run function to complete gracefully. -// -// Example: -// -// graceful.Run(fn, graceful.WithImmediateTermination()) +// WithImmediateTermination exits with status 130 as soon as the run context is canceled. func WithImmediateTermination() Option { return func(c *config) { c.immediateTermination = true } } -// interrupt returns the list of signals to listen for interrupt events. On Unix-like systems, this -// includes SIGINT and SIGTERM. On Windows, only os.interrupt is included. func interrupt() []os.Signal { signals := []os.Signal{os.Interrupt} if runtime.GOOS != "windows" { diff --git a/graceful/graceful_test.go b/graceful/graceful_test.go index 6ebfad7..278a82c 100644 --- a/graceful/graceful_test.go +++ b/graceful/graceful_test.go @@ -10,7 +10,6 @@ import ( "time" ) -// captureExitCode intercepts os.Exit calls and returns the exit code. func captureExitCode(t *testing.T, fn func()) int { t.Helper() @@ -34,7 +33,6 @@ func captureExitCode(t *testing.T, fn func()) int { } } -// sendSignal sends a signal after a channel is closed func sendSignal(trigger <-chan struct{}, delay time.Duration) { <-trigger if delay > 0 { @@ -108,7 +106,6 @@ func TestRun_GracefulCompletionAfterSignal(t *testing.T) { code := captureExitCode(t, func() { go sendSignal(started, 0) - // Simulate cleanup completing after signal go func() { <-started time.Sleep(20 * time.Millisecond) @@ -179,8 +176,6 @@ func TestRun_ImmediateTermination(t *testing.T) { Run(func(ctx context.Context) error { close(started) <-ctx.Done() - // Even though we block forever here, WithImmediateTermination should cause immediate - // exit without waiting for function completion select {} }, WithImmediateTermination()) }) diff --git a/internal/helpdoc/helpdoc.go b/internal/helpdoc/helpdoc.go index 12274d2..1d7d986 100644 --- a/internal/helpdoc/helpdoc.go +++ b/internal/helpdoc/helpdoc.go @@ -255,7 +255,7 @@ func commandListSummary(cmd Command) string { } func firstLine(text string) string { - for _, line := range strings.Split(text, "\n") { + for line := range strings.SplitSeq(text, "\n") { line = strings.TrimSpace(line) if line != "" { return line @@ -354,10 +354,7 @@ func writeItems(w io.Writer, items []Item) (int64, error) { } summaryIndent := maxNameLen + 6 - wrapWidth := defaultTerminalWidth - summaryIndent - if wrapWidth < 20 { - wrapWidth = 20 - } + wrapWidth := max(defaultTerminalWidth-summaryIndent, 20) for _, item := range items { if item.Summary == "" { diff --git a/internal/usage/help.go b/internal/usage/help.go index 2f6d346..1499c0e 100644 --- a/internal/usage/help.go +++ b/internal/usage/help.go @@ -1,8 +1,4 @@ -// Package usage contains experimental building blocks for command help documents. -// -// It is internal while the public usage API is still being refined. Generated help prefers -// cli.Command.Description for the command's long help text and cli.Command.Summary for command -// lists, with fallbacks for simple commands that only set one field. +// Package usage builds and renders command help. package usage import ( @@ -28,46 +24,32 @@ type Item struct { } // Text returns an untitled paragraph block. -// -// Use Text for descriptions, notes, or closing hints. func Text(lines ...string) Block { return Block{block: helpdoc.Text(lines...)} } // Lines returns a titled block of indented lines. -// -// Use Lines for sections such as Usage or Examples where each line should stand on its own. func Lines(heading string, lines ...string) Block { return Block{block: helpdoc.Lines(heading, lines...)} } -// List returns a titled list of name/summary pairs. -// -// Use List for aligned sections such as commands, flags, or named examples. +// List returns a titled list of name and summary pairs. func List(heading string, items ...Item) Block { return Block{block: helpdoc.List(heading, helpItems(items)...)} } -// String renders the full help document as a string. -// -// Use String when returning help from cli.Command.Help or when comparing help text in tests. +// String renders the document. func (d Document) String() string { return d.helpdoc().String() } -// WriteTo writes the help document to w. -// -// Use WriteTo when streaming help directly to stdout, stderr, or another writer. +// WriteTo writes the document to w. func (d Document) WriteTo(w io.Writer) (int64, error) { return d.helpdoc().WriteTo(w) } -// New returns the default help document for cmd. -// -// Use New from cli.Command.Help when you want to keep the built-in help layout and add or reorder -// sections before returning the final string. The document starts with Description when set, or -// Summary otherwise. Subcommand lists use Summary when set, or the first line of Description -// otherwise. Use New, not Help, inside a cli.Command.Help hook so the hook does not call itself. +// New returns the default help document for cmd. Use it inside a [cli.Command.Help] hook to avoid +// calling the hook recursively. func New(cmd *cli.Command) Document { cmd = resolveCommand(cmd) if cmd == nil { @@ -76,12 +58,7 @@ func New(cmd *cli.Command) Document { return fromHelpDoc(helpdoc.New(helpPath(cmd))) } -// Help returns help text for cmd. -// -// Use Help when handling flag.ErrHelp yourself after calling cli.Parse directly. It returns the -// same text cli.ParseAndRun prints for --help: if the resolved command has a cli.Command.Help hook, -// Help returns that hook's output; otherwise, it renders the default document from New. Inside a -// cli.Command.Help hook, use New instead. +// Help returns custom help for cmd when configured, otherwise the default help from [New]. func Help(cmd *cli.Command) string { cmd = resolveCommand(cmd) if cmd == nil { diff --git a/parse.go b/parse.go index f2f77a5..1f56d85 100644 --- a/parse.go +++ b/parse.go @@ -10,8 +10,6 @@ import ( "strings" ) -// 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 == "--" { @@ -21,8 +19,6 @@ func splitAtDelimiter(args []string) (argsToParse, remaining []string) { 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 { @@ -33,29 +29,22 @@ func resolveCommandPath(root *Command, argsToParse []string) (*Command, error) { for i < len(argsToParse) { arg := argsToParse[i] - // Skip flags and their values if strings.HasPrefix(arg, "-") { - // For formats like -flag=x or --flag=x if strings.Contains(arg, "=") { i++ continue } - // 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. Also check short flag aliases from FlagConfigs. + // A parent flag may appear before a subcommand, so inspect the full path before + // deciding whether the next argument is its value. name := strings.TrimLeft(arg, "-") skipValue := false for _, cmd := range root.state.path { localFlags := localFlagSet(cmd.FlagConfigs) - // Skip local flags on ancestor commands (any command already in the path is an - // ancestor of the not-yet-resolved terminal command). if localFlags[name] { continue } - // First try direct lookup. f := cmd.Flags.Lookup(name) - // If not found, check if it's a short alias. if f == nil { for _, flagConfig := range cmd.FlagConfigs { if flagConfig.Short == name { @@ -75,7 +64,6 @@ func resolveCommandPath(root *Command, argsToParse []string) (*Command, error) { } } if skipValue { - // Skip both flag and its value i += 2 continue } @@ -83,7 +71,6 @@ func resolveCommandPath(root *Command, argsToParse []string) (*Command, error) { continue } - // Try to traverse to subcommand if len(current.SubCommands) > 0 { if sub := current.findSubCommand(arg); sub != nil { root.state.path = append(slices.Clone(root.state.path), sub) @@ -112,9 +99,7 @@ func clearCommandState(cmd *Command) { } } -// 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. Short flag -// aliases from FlagConfigs are also registered, sharing the same Value as their long counterpart. +// combineFlags adds child flags first so they take precedence over inherited flags. func combineFlags(path []*Command) *flag.FlagSet { combined := flag.NewFlagSet(path[0].Name, flag.ContinueOnError) combined.SetOutput(io.Discard) @@ -135,7 +120,6 @@ func combineFlags(path []*Command) *flag.FlagSet { if combined.Lookup(f.Name) == nil { combined.Var(f.Value, f.Name, f.Usage) } - // Register the short alias pointing to the same Value. if short, ok := shortMap[f.Name]; ok { if combined.Lookup(short) == nil { combined.Var(f.Value, short, f.Usage) @@ -146,7 +130,6 @@ func combineFlags(path []*Command) *flag.FlagSet { return combined } -// localFlagSet builds a set of flag names that are marked as local in FlagConfigs. func localFlagSet(configs []FlagConfig) map[string]bool { m := make(map[string]bool, len(configs)) for _, flagConfig := range configs { @@ -157,7 +140,6 @@ func localFlagSet(configs []FlagConfig) map[string]bool { return m } -// shortFlagMap builds a map from long flag name to short alias from FlagConfigs. func shortFlagMap(configs []FlagConfig) map[string]string { m := make(map[string]string, len(configs)) for _, flagConfig := range configs { @@ -168,11 +150,8 @@ func shortFlagMap(configs []FlagConfig) map[string]string { return m } -// checkRequiredFlags verifies that all flags marked as required in FlagConfigs 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. + // Visit reports flags explicitly set by the user, including explicit zero values. setFlags := make(map[string]struct{}) combined.Visit(func(f *flag.Flag) { setFlags[f.Name] = struct{}{} @@ -185,7 +164,6 @@ func checkRequiredFlags(path []*Command, combined *flag.FlagSet) error { if !flagConfig.Required { continue } - // Skip required-flag checks for local flags on ancestor commands. if flagConfig.Local && i < terminalIdx { continue } @@ -207,14 +185,10 @@ func checkRequiredFlags(path []*Command, combined *flag.FlagSet) error { return nil } -// collectArgs strips resolved command names from the parsed positional args and appends any args -// that appeared after the "--" delimiter. +// collectArgs removes the resolved command path and restores arguments after "--". 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 - chainIdx := 1 // Skip root + chainIdx := 1 // The root name is not part of args. for startIdx < len(parsed) && chainIdx < len(path) { if strings.EqualFold(parsed[startIdx], path[chainIdx].Name) { startIdx++ @@ -276,9 +250,6 @@ func commandDefinitionError(path []string, err error) error { return fmt.Errorf("command %q: %w", strings.Join(path, " "), err) } -// validateFlagConfigs checks that each FlagConfig entry refers to a flag that exists in the -// command's FlagSet, that Short aliases are single ASCII letters, and that no two entries share the -// same Short alias. func validateFlagConfigs(cmd *Command) error { if len(cmd.FlagConfigs) == 0 { return nil diff --git a/pkg/suggest/suggest.go b/pkg/suggest/suggest.go index 9822d04..8890502 100644 --- a/pkg/suggest/suggest.go +++ b/pkg/suggest/suggest.go @@ -1,47 +1,46 @@ package suggest import ( - "sort" + "slices" "strings" ) -// threshold is the minimum similarity score required for a string to be considered similar. const threshold = 0.5 -// FindSimilar returns a list of similar strings to the target string from a list of candidates. +type suggestion struct { + name string + score float64 +} + +// FindSimilar returns up to maxResults candidates similar to target. func FindSimilar(target string, candidates []string, maxResults int) []string { - // Early returns for invalid inputs if target == "" || maxResults <= 0 { return []string{} } - suggestions := make([]struct { - name string - score float64 - }, 0, len(candidates)) + suggestions := make([]suggestion, 0, len(candidates)) - // Calculate similarity scores for _, name := range candidates { score := calculateSimilarity(target, name) - if score > threshold { // Only include reasonably similar commands - suggestions = append(suggestions, struct { - name string - score float64 - }{name, score}) + if score > threshold { + suggestions = append(suggestions, suggestion{name, score}) } } - sort.Slice(suggestions, func(i, j int) bool { - if suggestions[i].score == suggestions[j].score { - return suggestions[i].name < suggestions[j].name + slices.SortFunc(suggestions, func(a, b suggestion) int { + if a.score > b.score { + return -1 + } + if a.score < b.score { + return 1 } - return suggestions[i].score > suggestions[j].score + return strings.Compare(a.name, b.name) }) - // Get top N suggestions - result := make([]string, 0, maxResults) - for i := 0; i < len(suggestions) && i < maxResults; i++ { - result = append(result, suggestions[i].name) + limit := min(maxResults, len(suggestions)) + result := make([]string, limit) + for i, suggestion := range suggestions[:limit] { + result[i] = suggestion.name } return result @@ -51,22 +50,15 @@ func calculateSimilarity(a, b string) float64 { a = strings.ToLower(a) b = strings.ToLower(b) - // Perfect match if a == b { - return 1.0 + return 1 } - // Prefix match bonus if strings.HasPrefix(b, a) { return 0.9 } - // Calculate Levenshtein distance distance := levenshteinDistance(a, b) maxLen := float64(max(len(a), len(b))) - - // Convert distance to similarity score (0 to 1) - similarity := 1.0 - float64(distance)/maxLen - - return similarity + return 1 - float64(distance)/maxLen } func levenshteinDistance(a, b string) int { @@ -96,9 +88,10 @@ func levenshteinDistance(a, b string) int { cost = 0 } matrix[i][j] = min( - matrix[i-1][j]+1, // deletion - min(matrix[i][j-1]+1, // insertion - matrix[i-1][j-1]+cost)) // substitution + matrix[i-1][j]+1, + matrix[i][j-1]+1, + matrix[i-1][j-1]+cost, + ) } } diff --git a/pkg/textutil/textutil.go b/pkg/textutil/textutil.go index e32c6df..d6e23b7 100644 --- a/pkg/textutil/textutil.go +++ b/pkg/textutil/textutil.go @@ -2,6 +2,7 @@ package textutil import "strings" +// Wrap wraps text to width at word boundaries. func Wrap(text string, width int) []string { words := strings.Fields(text) var ( diff --git a/xflag/doc.go b/xflag/doc.go index 2a0821d..2f1a543 100644 --- a/xflag/doc.go +++ b/xflag/doc.go @@ -1,5 +1,2 @@ -// Package xflag extends the standard library's flag package to support parsing flags interleaved -// with positional arguments. By default, Go's flag package stops parsing flags at the first -// non-flag argument, which is unintuitive for most CLI users. This package provides [ParseToEnd] as -// a drop-in replacement that handles flags anywhere in the argument list. +// Package xflag parses flags interspersed with positional arguments. package xflag diff --git a/xflag/parse_test.go b/xflag/parse_test.go index a58a4c7..5b2a747 100644 --- a/xflag/parse_test.go +++ b/xflag/parse_test.go @@ -119,7 +119,6 @@ func TestParseToEnd(t *testing.T) { fs, c := newFlagset() err := ParseToEnd(fs, []string{"arg1", "arg2", "arg3"}) require.NoError(t, err) - // All flags should retain defaults. require.Equal(t, config{flag1: "asdf", flag2: "qwerty", flag3: false, flag4: true}, *c) require.Equal(t, 0, fs.NFlag()) require.Equal(t, []string{"arg1", "arg2", "arg3"}, fs.Args()) From 8591067d28ef32c1d973e6d2ea8d9e1ca19478f5 Mon Sep 17 00:00:00 2001 From: Mike Fridman Date: Fri, 21 Aug 2026 14:26:05 +0200 Subject: [PATCH 2/2] docs: expand doc comments and add examples --- cli.go | 24 ++++++++++++++++++++++-- flagtype/enum.go | 5 +++-- flagtype/regexp.go | 3 ++- flagtype/string_map.go | 3 ++- flagtype/string_slice.go | 2 +- flagtype/url.go | 3 ++- graceful/graceful.go | 27 +++++++++++++++++++++++++-- 7 files changed, 57 insertions(+), 10 deletions(-) diff --git a/cli.go b/cli.go index c9f07b4..22ce30a 100644 --- a/cli.go +++ b/cli.go @@ -1,5 +1,24 @@ // Package cli builds command-line programs on top of the standard library [flag] package. It adds // nested subcommands, flags anywhere, inherited flags, generated help, and type-safe flag access. +// +// root := &cli.Command{ +// Name: "echo", +// Flags: cli.FlagsFunc(func(f *flag.FlagSet) { +// f.Bool("capitalize", false, "capitalize the input") +// }), +// Exec: func(ctx context.Context, s *cli.State) error { +// text := strings.Join(s.Args, " ") +// if s.GetFlag[bool]("capitalize") { +// text = strings.ToUpper(text) +// } +// fmt.Fprintln(s.Stdout, text) +// return nil +// }, +// } +// if err := cli.ParseAndRun(ctx, root, os.Args[1:], nil); err != nil { +// fmt.Fprintln(os.Stderr, err) +// os.Exit(1) +// } package cli import ( @@ -42,14 +61,15 @@ type Command struct { // Summary is empty. Description string - // Help overrides the generated help for this command. + // Help overrides the generated help for --help and [UsageErrorf] errors on this command. Help func(*Command) string // Flags holds this command's [flag.FlagSet]. Subcommands inherit these flags unless they are // marked [FlagConfig.Local]. Flags *flag.FlagSet - // FlagConfigs adds behavior to flags already defined in Flags. + // FlagConfigs adds behavior to flags already defined in Flags. Each config must name a flag in + // Flags. FlagConfigs []FlagConfig // SubCommands are the commands available below this command. A command that only groups diff --git a/flagtype/enum.go b/flagtype/enum.go index 277a419..07a4d3b 100644 --- a/flagtype/enum.go +++ b/flagtype/enum.go @@ -12,12 +12,13 @@ type enumValue struct { allowed []string } -// Enum returns a [flag.Value] restricted to allowed. +// Enum returns a [flag.Value] restricted to allowed. Its value is retrieved as a string. func Enum(allowed ...string) flag.Value { return &enumValue{allowed: allowed} } -// EnumDefault is like [Enum] with a default value. It panics if defaultVal is not allowed. +// EnumDefault is like [Enum] with a default value, also retrieved as a string. It panics if +// defaultVal is not allowed. func EnumDefault(defaultVal string, allowed []string) flag.Value { if !slices.Contains(allowed, defaultVal) { panic(fmt.Sprintf("flagtype: default value %q is not in allowed values: %s", diff --git a/flagtype/regexp.go b/flagtype/regexp.go index e865e91..dc4deb9 100644 --- a/flagtype/regexp.go +++ b/flagtype/regexp.go @@ -9,7 +9,8 @@ type regexpValue struct { re *regexp.Regexp } -// Regexp returns a [flag.Value] that compiles its input as a regular expression. +// Regexp returns a [flag.Value] that compiles its input as a regular expression. Its value is +// retrieved as *regexp.Regexp. func Regexp() flag.Value { return ®expValue{} } diff --git a/flagtype/string_map.go b/flagtype/string_map.go index 8f3018c..7e5cb3f 100644 --- a/flagtype/string_map.go +++ b/flagtype/string_map.go @@ -12,7 +12,8 @@ type stringMapValue struct { m map[string]string } -// StringMap returns a repeatable [flag.Value] that parses key=value pairs. Values may contain "=". +// StringMap returns a repeatable [flag.Value] that parses key=value pairs. Values may contain "="; +// the result is retrieved as map[string]string. func StringMap() flag.Value { return &stringMapValue{} } diff --git a/flagtype/string_slice.go b/flagtype/string_slice.go index 8385d79..6f9914f 100644 --- a/flagtype/string_slice.go +++ b/flagtype/string_slice.go @@ -9,7 +9,7 @@ type stringSliceValue struct { vals []string } -// StringSlice returns a repeatable [flag.Value] that collects values into a string slice. +// StringSlice returns a repeatable [flag.Value] retrieved as []string. func StringSlice() flag.Value { return &stringSliceValue{} } diff --git a/flagtype/url.go b/flagtype/url.go index bb6a89a..b544dd0 100644 --- a/flagtype/url.go +++ b/flagtype/url.go @@ -10,7 +10,8 @@ type urlValue struct { u *url.URL } -// URL returns a [flag.Value] that requires a URL with a scheme and host. +// URL returns a [flag.Value] that requires a URL with a scheme and host. Its value is retrieved as +// *url.URL. func URL() flag.Value { return &urlValue{} } diff --git a/graceful/graceful.go b/graceful/graceful.go index 2f781a6..e571805 100644 --- a/graceful/graceful.go +++ b/graceful/graceful.go @@ -134,7 +134,8 @@ func Run(fn func(context.Context) error, opts ...Option) { } // ListenAndServe runs srv until ctx is canceled, then drains it for up to shutdownGrace. The -// initial cancellation is not propagated to handler contexts. +// initial cancellation is not propagated to handler contexts. shutdownGrace only bounds HTTP +// draining; [WithTerminationTimeout] bounds the entire shutdown. func ListenAndServe(srv *http.Server, shutdownGrace time.Duration) func(context.Context) error { return func(ctx context.Context) error { var wg sync.WaitGroup @@ -200,7 +201,14 @@ func WithLogger(logger *slog.Logger) Option { } } -// WithRunTimeout cancels the run context after d. A non-positive duration disables the limit. +// WithRunTimeout cancels the run context after d. It does not force the run function to return; use +// [WithTerminationTimeout] to bound shutdown. A non-positive duration disables the limit. +// +// For a batch job with a hard deadline: +// +// graceful.Run(func(ctx context.Context) error { +// return processBatch(ctx) +// }, graceful.WithRunTimeout(1*time.Hour)) func WithRunTimeout(d time.Duration) Option { return func(c *config) { c.runTimeout = d @@ -209,6 +217,15 @@ func WithRunTimeout(d time.Duration) Option { // WithTerminationTimeout exits with status 124 if shutdown takes longer than d. A non-positive // duration disables the limit. +// +// To bound both a worker's run time and shutdown: +// +// graceful.Run(func(ctx context.Context) error { +// return runWorker(ctx) +// }, +// graceful.WithRunTimeout(24*time.Hour), +// graceful.WithTerminationTimeout(30*time.Second), +// ) func WithTerminationTimeout(d time.Duration) Option { return func(c *config) { c.shutdownTimeout = d @@ -216,6 +233,12 @@ func WithTerminationTimeout(d time.Duration) Option { } // WithImmediateTermination exits with status 130 as soon as the run context is canceled. +// +// To exit on the first signal: +// +// graceful.Run(func(ctx context.Context) error { +// return runTask(ctx) +// }, graceful.WithImmediateTermination()) func WithImmediateTermination() Option { return func(c *config) { c.immediateTermination = true