From e29955d6672bd1d62d97e55b76607805363420c2 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Sun, 5 Apr 2026 21:36:50 +0000 Subject: [PATCH 01/24] =?UTF-8?q?feat:=20add=20cli=20package=20=E2=80=94?= =?UTF-8?q?=20struct-driven=20CLI=20framework?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds github.com/runreveal/lib/cli, a simple Configure-Validate-Run CLI framework that integrates with loader for HuJSON config file loading. Key features: - Struct tag-driven flags (cli:"name,alias", default:"val", usage:"...") - Config file loading via loader.LoadConfig with gjson path extraction - Embedded struct composition for shared/global flags - Command/Group tree with recursive routing - Middleware chain, Validator interface, ArgsFunc validation - ExitError for custom exit codes, panic recovery - IsSet() to detect explicitly-set flags during Run --- .gitignore | 1 + cli/args.go | 34 +++ cli/cli.go | 390 +++++++++++++++++++++++++ cli/cli_test.go | 693 ++++++++++++++++++++++++++++++++++++++++++++ cli/config.go | 110 +++++++ cli/example/go.mod | 18 ++ cli/example/go.sum | 27 ++ cli/example/main.go | 117 ++++++++ cli/flags.go | 403 ++++++++++++++++++++++++++ cli/go.mod | 21 ++ cli/go.sum | 29 ++ cli/help.go | 149 ++++++++++ cli/reflect.go | 158 ++++++++++ 13 files changed, 2150 insertions(+) create mode 100644 cli/args.go create mode 100644 cli/cli.go create mode 100644 cli/cli_test.go create mode 100644 cli/config.go create mode 100644 cli/example/go.mod create mode 100644 cli/example/go.sum create mode 100644 cli/example/main.go create mode 100644 cli/flags.go create mode 100644 cli/go.mod create mode 100644 cli/go.sum create mode 100644 cli/help.go create mode 100644 cli/reflect.go diff --git a/.gitignore b/.gitignore index 223cec9..a05040f 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ .coverprofile +cli/example/example diff --git a/cli/args.go b/cli/args.go new file mode 100644 index 0000000..a731e54 --- /dev/null +++ b/cli/args.go @@ -0,0 +1,34 @@ +package cli + +import "fmt" + +// ArgsFunc validates positional arguments. +type ArgsFunc func(args []string) error + +// NoArgs returns an error if any positional args are present. +func NoArgs(args []string) error { + if len(args) > 0 { + return fmt.Errorf("unexpected arguments: %v", args) + } + return nil +} + +// ExactArgs returns an ArgsFunc that requires exactly n positional args. +func ExactArgs(n int) ArgsFunc { + return func(args []string) error { + if len(args) != n { + return fmt.Errorf("expected exactly %d argument(s), got %d", n, len(args)) + } + return nil + } +} + +// MinArgs returns an ArgsFunc that requires at least n positional args. +func MinArgs(n int) ArgsFunc { + return func(args []string) error { + if len(args) < n { + return fmt.Errorf("expected at least %d argument(s), got %d", n, len(args)) + } + return nil + } +} diff --git a/cli/cli.go b/cli/cli.go new file mode 100644 index 0000000..e3a08af --- /dev/null +++ b/cli/cli.go @@ -0,0 +1,390 @@ +// Package cli provides a simple, struct-driven CLI framework. +// It follows a Configure-Validate-Run lifecycle and integrates +// with github.com/runreveal/lib/loader for config file loading. +package cli + +import ( + "context" + "errors" + "fmt" + "io" + "log/slog" + "os" + "reflect" + "runtime/debug" + "strings" + "sync" +) + +// handlerFlagSets maps handler pointer → *FlagSet during execution, +// enabling IsSet() to query which flags were explicitly set. +var handlerFlagSets sync.Map + +// Runnable is the core interface every command handler must implement. +type Runnable interface { + Run(ctx context.Context, args []string) error +} + +// Validator is optionally implemented by handlers to validate config after loading. +type Validator interface { + Validate() error +} + +// ExitError carries a custom exit code. +type ExitError struct { + Code int + Err error +} + +func (e *ExitError) Error() string { + if e.Err != nil { + return e.Err.Error() + } + return fmt.Sprintf("exit code %d", e.Code) +} + +func (e *ExitError) Unwrap() error { return e.Err } + +// CommandInfo is passed to middleware. +type CommandInfo struct { + Name string // full command path, e.g. "admin migrate" + Args []string // positional args after flag parsing +} + +// Middleware wraps command execution. +type Middleware func(ctx context.Context, info CommandInfo, next func(context.Context) error) error + +// Node is a node in the command tree. +type Node interface { + nodeName() string + nodeDesc() string + isGroup() bool +} + +type commandNode struct { + name string + desc string + handler Runnable + children []Node + opts cmdOptions +} + +func (c *commandNode) nodeName() string { return c.name } +func (c *commandNode) nodeDesc() string { return c.desc } +func (c *commandNode) isGroup() bool { return false } + +type groupNode struct { + name string + desc string + children []Node +} + +func (g *groupNode) nodeName() string { return g.name } +func (g *groupNode) nodeDesc() string { return g.desc } +func (g *groupNode) isGroup() bool { return true } + +// CmdOption configures a Command node. +type CmdOption func(*cmdOptions) + +type cmdOptions struct { + argsFunc ArgsFunc +} + +// WithArgs sets an args validation function on a command. +func WithArgs(f ArgsFunc) CmdOption { + return func(o *cmdOptions) { o.argsFunc = f } +} + +// Command creates a command node with an optional set of child nodes. +func Command(name, desc string, handler Runnable, children ...Node) Node { + return CommandWithOptions(name, desc, handler, nil, children...) +} + +// CommandWithOptions creates a command node with options and optional children. +func CommandWithOptions(name, desc string, handler Runnable, opts []CmdOption, children ...Node) Node { + o := cmdOptions{} + for _, opt := range opts { + opt(&o) + } + return &commandNode{name: name, desc: desc, handler: handler, children: children, opts: o} +} + +// Group creates a group node that only prints help when invoked directly. +func Group(name, desc string, children ...Node) Node { + return &groupNode{name: name, desc: desc, children: children} +} + +// AppOption configures an App. +type AppOption func(*App) + +// WithVersion sets the application version (enables --version flag). +func WithVersion(v string) AppOption { + return func(a *App) { a.version = v } +} + +// WithMiddleware adds a middleware to the app. +func WithMiddleware(m Middleware) AppOption { + return func(a *App) { a.middlewares = append(a.middlewares, m) } +} + +// WithConfigFlag sets which flag name holds the config file path. +func WithConfigFlag(flagName string) AppOption { + return func(a *App) { a.configFlag = flagName } +} + +// WithOutput sets the writer for help/error output (default: os.Stderr). +func WithOutput(w io.Writer) AppOption { + return func(a *App) { a.output = w } +} + +// App is the top-level CLI application. +type App struct { + name string + desc string + version string + configFlag string + middlewares []Middleware + children []Node + output io.Writer +} + +// New creates a new App. +func New(name, desc string, opts ...AppOption) *App { + a := &App{ + name: name, + desc: desc, + output: os.Stderr, + } + for _, opt := range opts { + opt(a) + } + return a +} + +// AddCommand adds top-level command nodes to the app. +func (a *App) AddCommand(nodes ...Node) { + a.children = append(a.children, nodes...) +} + +// Run executes the CLI with the given args (typically os.Args[1:]). +// Returns an exit code. +func (a *App) Run(ctx context.Context, args []string) (exitCode int) { + defer func() { + if r := recover(); r != nil { + slog.Error("panic in command", "panic", r, "stack", string(debug.Stack())) + exitCode = 1 + } + }() + + code, err := a.run(ctx, args) + if err != nil { + var exitErr *ExitError + if errors.As(err, &exitErr) { + if exitErr.Err != nil { + fmt.Fprintf(a.output, "error: %s\n", exitErr.Err) + } + return exitErr.Code + } + fmt.Fprintf(a.output, "error: %s\n", err) + return 1 + } + return code +} + +func (a *App) run(ctx context.Context, args []string) (int, error) { + // Check for top-level --version / --help before routing + if len(args) == 1 && (args[0] == "--version" || args[0] == "-version") { + if a.version != "" { + fmt.Fprintf(a.output, "%s version %s\n", a.name, a.version) + } else { + fmt.Fprintf(a.output, "%s (no version set)\n", a.name) + } + return 0, nil + } + if len(args) == 0 || (len(args) == 1 && (args[0] == "--help" || args[0] == "-h")) { + printAppHelp(a.output, a.name, a.desc, a.children, a.version) + return 0, nil + } + + node, rest, path := routeArgsWithPath(a.children, args, "") + if node == nil { + // Unknown command + fmt.Fprintf(a.output, "unknown command %q\n\n", args[0]) + printAppHelp(a.output, a.name, a.desc, a.children, a.version) + return 1, nil + } + + return a.executeNode(ctx, node, rest, path) +} + +func routeArgsWithPath(children []Node, args []string, prefix string) (Node, []string, string) { + if len(args) == 0 { + return nil, args, prefix + } + + name := args[0] + // Don't treat flags as command names + if strings.HasPrefix(name, "-") { + return nil, args, prefix + } + + for _, child := range children { + if child.nodeName() == name { + fullPath := name + if prefix != "" { + fullPath = prefix + " " + name + } + rest := args[1:] + + // If this node has children and the next arg matches one, recurse + var subChildren []Node + switch n := child.(type) { + case *commandNode: + subChildren = n.children + case *groupNode: + subChildren = n.children + } + + if len(subChildren) > 0 && len(rest) > 0 && !strings.HasPrefix(rest[0], "-") { + if sub, subRest, subPath := routeArgsWithPath(subChildren, rest, fullPath); sub != nil { + return sub, subRest, subPath + } + } + + return child, rest, fullPath + } + } + return nil, args, prefix +} + +func (a *App) executeNode(ctx context.Context, node Node, args []string, path string) (int, error) { + switch n := node.(type) { + case *groupNode: + // Groups print help when invoked directly (no matching subcommand) + printGroupHelp(a.output, a.name, path, n.desc, n.children) + return 0, nil + + case *commandNode: + return a.executeCommand(ctx, n, args, path) + } + return 1, fmt.Errorf("unknown node type") +} + +func (a *App) executeCommand(ctx context.Context, node *commandNode, args []string, path string) (int, error) { + handler := node.handler + + // Check for --help before doing anything else + for _, arg := range args { + if arg == "--help" || arg == "-h" { + printCommandHelp(a.output, a.name, path, node.desc, handler, node.children) + return 0, nil + } + if arg == "--" { + break + } + } + + // Build flag set from handler struct tags (also returns pre-scanned fields). + fs, fields, err := buildFlagSet(handler) + if err != nil { + return 1, fmt.Errorf("building flags for %s: %w", path, err) + } + + // Set defaults + if err := applyDefaults(fs, fields); err != nil { + return 1, fmt.Errorf("applying defaults for %s: %w", path, err) + } + + // Parse flags + posArgs, err := fs.Parse(args) + if err != nil { + fmt.Fprintf(a.output, "error: %s\n\n", err) + printCommandHelp(a.output, a.name, path, node.desc, handler, node.children) + return 1, nil + } + + // Register the FlagSet so IsSet() can query it during Run + handlerPtr := reflect.ValueOf(handler).Pointer() + handlerFlagSets.Store(handlerPtr, fs) + defer handlerFlagSets.Delete(handlerPtr) + + // Load config file if configured + if a.configFlag != "" { + if err := loadConfigIntoHandler(handler, fs, a.configFlag, fields); err != nil { + return 1, fmt.Errorf("loading config: %w", err) + } + } + + // Validate + if v, ok := handler.(Validator); ok { + if err := v.Validate(); err != nil { + return 1, err + } + } + + // Validate args + if node.opts.argsFunc != nil { + if err := node.opts.argsFunc(posArgs); err != nil { + return 1, err + } + } + + // Build middleware chain + runFn := func(ctx context.Context) error { + return handler.Run(ctx, posArgs) + } + + info := CommandInfo{Name: path, Args: posArgs} + chain := buildChain(a.middlewares, info, runFn) + + if err := chain(ctx); err != nil { + var exitErr *ExitError + if errors.As(err, &exitErr) { + // Propagate ExitError so Run() can extract the code. + return exitErr.Code, exitErr + } + return 1, err + } + return 0, nil +} + +func buildChain(middlewares []Middleware, info CommandInfo, final func(context.Context) error) func(context.Context) error { + if len(middlewares) == 0 { + return final + } + // Build from the inside out + chain := final + for i := len(middlewares) - 1; i >= 0; i-- { + m := middlewares[i] + next := chain + chain = func(ctx context.Context) error { + return m(ctx, info, next) + } + } + return chain +} + +// IsSet reports whether a flag was explicitly set on the command line. +// Must be called from within Run (or Validate) to return meaningful results. +func IsSet(handler Runnable, flagName string) bool { + handlerPtr := reflect.ValueOf(handler).Pointer() + if val, ok := handlerFlagSets.Load(handlerPtr); ok { + return val.(*FlagSet).IsSet(flagName) + } + return false +} + +// DumpConfig returns the resolved flag configuration as a map. +// When called from within Run, it reflects the live parsed values. +func DumpConfig(handler Runnable) map[string]any { + handlerPtr := reflect.ValueOf(handler).Pointer() + if val, ok := handlerFlagSets.Load(handlerPtr); ok { + return val.(*FlagSet).DumpValues() + } + // Outside of Run — build a fresh FlagSet to get at least the defaults. + fs, _, err := buildFlagSet(handler) + if err != nil { + return nil + } + return fs.DumpValues() +} diff --git a/cli/cli_test.go b/cli/cli_test.go new file mode 100644 index 0000000..78f7230 --- /dev/null +++ b/cli/cli_test.go @@ -0,0 +1,693 @@ +package cli_test + +import ( + "bytes" + "context" + "errors" + "os" + "strings" + "testing" + "time" + + "github.com/runreveal/lib/cli" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// --- helpers --- + +type echoCmd struct { + Message string `cli:"message,m" usage:"message to echo" default:"hello"` + Count int `cli:"count,n" usage:"number of times" default:"1"` +} + +func (e *echoCmd) Run(_ context.Context, _ []string) error { return nil } + +type boolCmd struct { + Verbose bool `cli:"verbose,v" usage:"be verbose"` + Debug bool `cli:"debug,d" usage:"debug mode"` + Name string `cli:"name" usage:"name"` +} + +func (b *boolCmd) Run(_ context.Context, _ []string) error { return nil } + +type noopCmd struct{} + +func (n *noopCmd) Run(_ context.Context, _ []string) error { return nil } + +type errCmd struct{} + +func (e *errCmd) Run(_ context.Context, _ []string) error { + return errors.New("run failed") +} + +type captureArgsCmd struct { + inner func([]string) +} + +func (c *captureArgsCmd) Run(_ context.Context, args []string) error { + c.inner(args) + return nil +} + +type panicCmd struct{} + +func (p *panicCmd) Run(_ context.Context, _ []string) error { + panic("oh no") +} + +type exitCodeCmd struct{ code int } + +func (e *exitCodeCmd) Run(_ context.Context, _ []string) error { + return &cli.ExitError{Code: e.code, Err: errors.New("custom exit")} +} + +type validateCmd struct { + Value string `cli:"value" usage:"a value"` + valid bool +} + +func (v *validateCmd) Validate() error { + if v.Value == "" { + return errors.New("value is required") + } + v.valid = true + return nil +} +func (v *validateCmd) Run(_ context.Context, _ []string) error { + if !v.valid { + return errors.New("Validate was not called") + } + return nil +} + +type isSetCmd struct { + Name string `cli:"name" usage:"name" default:"default"` + nameWasSet bool +} + +func (i *isSetCmd) Run(_ context.Context, _ []string) error { + i.nameWasSet = cli.IsSet(i, "name") + return nil +} + +// writeConfigFile writes JSON content to a temp file and returns its path. +func writeConfigFile(t *testing.T, content string) string { + t.Helper() + tmp := t.TempDir() + "/config.json" + require.NoError(t, os.WriteFile(tmp, []byte(content), 0600)) + return tmp +} + +// --- flag parsing tests --- + +func TestFlagParsing_LongFlag(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo", cmd)) + + code := app.Run(context.Background(), []string{"echo", "--message", "world", "--count", "3"}) + assert.Equal(t, 0, code) + assert.Equal(t, "world", cmd.Message) + assert.Equal(t, 3, cmd.Count) +} + +func TestFlagParsing_ShortFlag(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo", cmd)) + + code := app.Run(context.Background(), []string{"echo", "-m", "hi", "-n", "2"}) + assert.Equal(t, 0, code) + assert.Equal(t, "hi", cmd.Message) + assert.Equal(t, 2, cmd.Count) +} + +func TestFlagParsing_EqualsSyntax(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo", cmd)) + + code := app.Run(context.Background(), []string{"echo", "--message=greet", "--count=5"}) + assert.Equal(t, 0, code) + assert.Equal(t, "greet", cmd.Message) + assert.Equal(t, 5, cmd.Count) +} + +func TestFlagParsing_ShortEqualsSyntax(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo", cmd)) + + code := app.Run(context.Background(), []string{"echo", "-m=yo"}) + assert.Equal(t, 0, code) + assert.Equal(t, "yo", cmd.Message) +} + +func TestFlagParsing_CombinedBoolShorts(t *testing.T) { + cmd := &boolCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), []string{"run", "-vd"}) + assert.Equal(t, 0, code) + assert.True(t, cmd.Verbose) + assert.True(t, cmd.Debug) +} + +func TestFlagParsing_DoubleDashSeparator(t *testing.T) { + var capturedArgs []string + handler := &captureArgsCmd{inner: func(args []string) { capturedArgs = args }} + + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", handler)) + + code := app.Run(context.Background(), []string{"run", "--", "pos1", "pos2"}) + assert.Equal(t, 0, code) + assert.Equal(t, []string{"pos1", "pos2"}, capturedArgs) +} + +func TestFlagParsing_UnknownFlag(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo", cmd)) + + code := app.Run(context.Background(), []string{"echo", "--unknown"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "unknown flag") +} + +func TestFlagParsing_Defaults(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo", cmd)) + + code := app.Run(context.Background(), []string{"echo"}) + assert.Equal(t, 0, code) + assert.Equal(t, "hello", cmd.Message) + assert.Equal(t, 1, cmd.Count) +} + +// --- struct tag scanning tests --- + +type embeddedGlobals struct { + Verbose bool `cli:"verbose,v" usage:"verbose mode"` + Config string `cli:"config,c" usage:"config file" default:"config.json"` +} + +type embeddedCmd struct { + embeddedGlobals + Port int `cli:"port,p" usage:"port number" default:"8080"` +} + +func (e *embeddedCmd) Run(_ context.Context, _ []string) error { return nil } + +func TestStructTags_EmbeddedStruct(t *testing.T) { + cmd := &embeddedCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("serve", "serve", cmd)) + + code := app.Run(context.Background(), []string{"serve", "--verbose", "--port", "9090"}) + assert.Equal(t, 0, code) + assert.True(t, cmd.Verbose) + assert.Equal(t, 9090, cmd.Port) + assert.Equal(t, "config.json", cmd.Config) // default +} + +type skipCmd struct { + Name string `cli:"name" usage:"name"` + Ignored string `cli:"-"` + Also string // no tag, should be ignored +} + +func (s *skipCmd) Run(_ context.Context, _ []string) error { return nil } + +func TestStructTags_SkipField(t *testing.T) { + cmd := &skipCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), []string{"run", "--ignored", "val"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "unknown flag") +} + +type ptrCmd struct { + Name *string `cli:"name" usage:"name"` + Port *int `cli:"port" usage:"port"` +} + +func (p *ptrCmd) Run(_ context.Context, _ []string) error { return nil } + +func TestStructTags_PointerFields(t *testing.T) { + cmd := &ptrCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), []string{"run", "--name", "alice", "--port", "3000"}) + assert.Equal(t, 0, code) + require.NotNil(t, cmd.Name) + assert.Equal(t, "alice", *cmd.Name) + require.NotNil(t, cmd.Port) + assert.Equal(t, 3000, *cmd.Port) +} + +type allTypesCmd struct { + Str string `cli:"str" default:"s"` + Bool bool `cli:"bool"` + I int `cli:"int" default:"1"` + I64 int64 `cli:"i64" default:"2"` + U uint `cli:"uint" default:"3"` + U64 uint64 `cli:"u64" default:"4"` + F float64 `cli:"flt" default:"1.5"` + Dur time.Duration `cli:"dur" default:"5s"` + Strs []string `cli:"strs"` +} + +func (a *allTypesCmd) Run(_ context.Context, _ []string) error { return nil } + +func TestStructTags_AllTypes(t *testing.T) { + cmd := &allTypesCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), []string{ + "run", + "--str", "hello", + "--bool", + "--int", "10", + "--i64", "20", + "--uint", "30", + "--u64", "40", + "--flt", "3.14", + "--dur", "10s", + "--strs", "a", + "--strs", "b", + }) + assert.Equal(t, 0, code) + assert.Equal(t, "hello", cmd.Str) + assert.True(t, cmd.Bool) + assert.Equal(t, 10, cmd.I) + assert.Equal(t, int64(20), cmd.I64) + assert.Equal(t, uint(30), cmd.U) + assert.Equal(t, uint64(40), cmd.U64) + assert.InDelta(t, 3.14, cmd.F, 0.001) + assert.Equal(t, 10*time.Second, cmd.Dur) + assert.Equal(t, []string{"a", "b"}, cmd.Strs) +} + +// --- command routing tests --- + +func TestRouting_Subcommand(t *testing.T) { + var called string + sub := &captureArgsCmd{inner: func(_ []string) { called = "sub" }} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("parent", "parent", &noopCmd{}, + cli.Command("child", "child", sub), + )) + + code := app.Run(context.Background(), []string{"parent", "child"}) + assert.Equal(t, 0, code) + assert.Equal(t, "sub", called) +} + +func TestRouting_GroupPrintsHelp(t *testing.T) { + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Group("admin", "admin commands", + cli.Command("migrate", "run migrations", &noopCmd{}), + )) + + code := app.Run(context.Background(), []string{"admin"}) + assert.Equal(t, 0, code) + assert.Contains(t, buf.String(), "migrate") +} + +func TestRouting_UnknownCommand(t *testing.T) { + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("serve", "serve", &noopCmd{})) + + code := app.Run(context.Background(), []string{"unknown"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "unknown command") +} + +func TestRouting_NestedGroup(t *testing.T) { + var called bool + sub := &captureArgsCmd{inner: func(_ []string) { called = true }} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Group("a", "a", + cli.Group("b", "b", + cli.Command("c", "c", sub), + ), + )) + + code := app.Run(context.Background(), []string{"a", "b", "c"}) + assert.Equal(t, 0, code) + assert.True(t, called) +} + +// --- lifecycle tests --- + +func TestLifecycle_ValidateCalled(t *testing.T) { + cmd := &validateCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), []string{"run", "--value", "set"}) + assert.Equal(t, 0, code) +} + +func TestLifecycle_ValidateError(t *testing.T) { + cmd := &validateCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), []string{"run"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "value is required") +} + +func TestLifecycle_MiddlewareOrdering(t *testing.T) { + var order []string + mkMiddleware := func(name string) cli.Middleware { + return func(ctx context.Context, info cli.CommandInfo, next func(context.Context) error) error { + order = append(order, name+":before") + err := next(ctx) + order = append(order, name+":after") + return err + } + } + + var buf bytes.Buffer + app := cli.New("app", "test", + cli.WithOutput(&buf), + cli.WithMiddleware(mkMiddleware("first")), + cli.WithMiddleware(mkMiddleware("second")), + ) + app.AddCommand(cli.Command("run", "run", &noopCmd{})) + + code := app.Run(context.Background(), []string{"run"}) + assert.Equal(t, 0, code) + assert.Equal(t, []string{"first:before", "second:before", "second:after", "first:after"}, order) +} + +func TestLifecycle_RunError(t *testing.T) { + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", &errCmd{})) + + code := app.Run(context.Background(), []string{"run"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "run failed") +} + +func TestLifecycle_PanicRecovery(t *testing.T) { + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", &panicCmd{})) + + code := app.Run(context.Background(), []string{"run"}) + assert.Equal(t, 1, code) +} + +func TestLifecycle_ExitError(t *testing.T) { + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", &exitCodeCmd{code: 42})) + + code := app.Run(context.Background(), []string{"run"}) + assert.Equal(t, 42, code) +} + +// --- help output tests --- + +func TestHelp_AppHelp(t *testing.T) { + var buf bytes.Buffer + app := cli.New("myapp", "My application", cli.WithOutput(&buf), cli.WithVersion("1.0")) + app.AddCommand(cli.Command("serve", "Start HTTP server", &noopCmd{})) + + code := app.Run(context.Background(), []string{"--help"}) + assert.Equal(t, 0, code) + out := buf.String() + assert.Contains(t, out, "myapp") + assert.Contains(t, out, "serve") + assert.Contains(t, out, "--help") +} + +func TestHelp_CommandHelp(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("myapp", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo something", cmd)) + + code := app.Run(context.Background(), []string{"echo", "--help"}) + assert.Equal(t, 0, code) + out := buf.String() + assert.Contains(t, out, "--message") + assert.Contains(t, out, "--count") + assert.Contains(t, out, "default: hello") +} + +func TestHelp_VersionFlag(t *testing.T) { + var buf bytes.Buffer + app := cli.New("myapp", "test", cli.WithOutput(&buf), cli.WithVersion("2.3.4")) + + code := app.Run(context.Background(), []string{"--version"}) + assert.Equal(t, 0, code) + assert.Contains(t, buf.String(), "2.3.4") +} + +func TestHelp_ShortAlias(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("myapp", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo", cmd)) + + app.Run(context.Background(), []string{"echo", "--help"}) + out := buf.String() + assert.True(t, strings.Contains(out, "-m") || strings.Contains(out, "--message")) +} + +// --- edge cases --- + +func TestEdge_NoArgs(t *testing.T) { + var buf bytes.Buffer + app := cli.New("myapp", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("serve", "serve", &noopCmd{})) + + code := app.Run(context.Background(), []string{}) + assert.Equal(t, 0, code) + assert.Contains(t, buf.String(), "serve") +} + +func TestEdge_ArgsValidation_NoArgs(t *testing.T) { + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.CommandWithOptions("run", "run", &noopCmd{}, + []cli.CmdOption{cli.WithArgs(cli.NoArgs)}, + )) + + code := app.Run(context.Background(), []string{"run", "extra"}) + assert.Equal(t, 1, code) +} + +func TestEdge_ArgsValidation_ExactArgs(t *testing.T) { + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.CommandWithOptions("run", "run", &noopCmd{}, + []cli.CmdOption{cli.WithArgs(cli.ExactArgs(2))}, + )) + + code := app.Run(context.Background(), []string{"run", "a", "b"}) + assert.Equal(t, 0, code) + + code = app.Run(context.Background(), []string{"run", "a"}) + assert.Equal(t, 1, code) +} + +func TestEdge_IsSet(t *testing.T) { + setCmd := &isSetCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", setCmd)) + + app.Run(context.Background(), []string{"run", "--name", "alice"}) + assert.True(t, setCmd.nameWasSet) +} + +func TestEdge_IsSet_NotSet(t *testing.T) { + setCmd := &isSetCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", setCmd)) + + app.Run(context.Background(), []string{"run"}) + assert.False(t, setCmd.nameWasSet) +} + +// --- config file loading tests --- + +type dbSection struct { + Host string `json:"host"` + Port int `json:"port"` +} + +type configHandler struct { + ConfigFile string `cli:"config,c" usage:"config file"` + DB dbSection `config:"database"` +} + +func (c *configHandler) Run(_ context.Context, _ []string) error { return nil } + +func TestConfig_BasicLoad(t *testing.T) { + handler := &configHandler{} + f := writeConfigFile(t, `{ + "database": { + "host": "localhost", + "port": 5432 + } + }`) + + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + code := app.Run(context.Background(), []string{"run", "--config", f}) + assert.Equal(t, 0, code) + assert.Equal(t, "localhost", handler.DB.Host) + assert.Equal(t, 5432, handler.DB.Port) +} + +func TestConfig_MissingFileSilent(t *testing.T) { + handler := &configHandler{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + code := app.Run(context.Background(), []string{"run"}) + assert.Equal(t, 0, code) +} + +type overrideableHandler struct { + ConfigFile string `cli:"config" usage:"config file"` + Host string `cli:"host" usage:"host" default:"flag-default" config:"host"` +} + +func (o *overrideableHandler) Run(_ context.Context, _ []string) error { return nil } + +func TestConfig_CLIFlagOverridesConfig(t *testing.T) { + handler := &overrideableHandler{} + f := writeConfigFile(t, `{"host": "from-config"}`) + + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + code := app.Run(context.Background(), []string{"run", "--config", f, "--host", "from-flag"}) + assert.Equal(t, 0, code) + assert.Equal(t, "from-flag", handler.Host) +} + +func TestConfig_ConfigLoadsWhenNotExplicit(t *testing.T) { + handler := &overrideableHandler{} + f := writeConfigFile(t, `{"host": "from-config"}`) + + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + // Config file set but --host not set, so config value should apply + code := app.Run(context.Background(), []string{"run", "--config", f}) + assert.Equal(t, 0, code) + assert.Equal(t, "from-config", handler.Host) +} + +type nestedVal struct { + Val string `json:"val"` +} +type nestedConfigHandler struct { + ConfigFile string `cli:"config" usage:"config file"` + Nested nestedVal `config:"a.b"` +} + +func (n *nestedConfigHandler) Run(_ context.Context, _ []string) error { return nil } + +func TestConfig_NestedPath(t *testing.T) { + handler := &nestedConfigHandler{} + f := writeConfigFile(t, `{ + "a": { + "b": { + "val": "deep" + } + } + }`) + + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + code := app.Run(context.Background(), []string{"run", "--config", f}) + assert.Equal(t, 0, code) + assert.Equal(t, "deep", handler.Nested.Val) +} + +type rootSection struct { + Foo string `json:"foo"` + Bar int `json:"bar"` +} +type rootConfigHandler struct { + ConfigFile string `cli:"config" usage:"config file"` + All rootSection `config:"."` +} + +func (r *rootConfigHandler) Run(_ context.Context, _ []string) error { return nil } + +func TestConfig_RootPath(t *testing.T) { + handler := &rootConfigHandler{} + f := writeConfigFile(t, `{"foo": "baz", "bar": 99}`) + + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + code := app.Run(context.Background(), []string{"run", "--config", f}) + assert.Equal(t, 0, code) + assert.Equal(t, "baz", handler.All.Foo) + assert.Equal(t, 99, handler.All.Bar) +} + +func TestMiddleware_CommandInfo(t *testing.T) { + var capturedInfo cli.CommandInfo + mw := func(ctx context.Context, info cli.CommandInfo, next func(context.Context) error) error { + capturedInfo = info + return next(ctx) + } + + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithMiddleware(mw)) + app.AddCommand(cli.Group("admin", "admin", + cli.Command("migrate", "migrate", &noopCmd{}), + )) + + code := app.Run(context.Background(), []string{"admin", "migrate"}) + assert.Equal(t, 0, code) + assert.Equal(t, "admin migrate", capturedInfo.Name) +} diff --git a/cli/config.go b/cli/config.go new file mode 100644 index 0000000..61fa88e --- /dev/null +++ b/cli/config.go @@ -0,0 +1,110 @@ +package cli + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "reflect" + + "github.com/runreveal/lib/loader" + "github.com/tidwall/gjson" +) + +// loadConfigIntoHandler reads the config file (path from the named flag), +// processes it via loader.LoadConfig, then applies config-tagged fields. +// Only sets fields that were NOT explicitly set via CLI flags. +// fields must be pre-scanned via buildFlagSet to avoid redundant reflection. +func loadConfigIntoHandler(handler Runnable, fs *FlagSet, configFlagName string, fields []fieldInfo) error { + if _, ok := fs.byLong[configFlagName]; !ok { + return nil + } + + rv := reflect.ValueOf(handler) + if rv.Kind() == reflect.Ptr { + rv = rv.Elem() + } + + // Find the config file path from the already-parsed flag value. + configPath := stringFieldForFlag(rv, fields, configFlagName) + if configPath == "" { + return nil + } + + explicit := fs.IsSet(configFlagName) + + data, err := os.ReadFile(configPath) + if err != nil { + if errors.Is(err, os.ErrNotExist) && !explicit { + return nil + } + return fmt.Errorf("reading config file %q: %w", configPath, err) + } + + // Process via loader (hujson + env replacement) + var raw json.RawMessage + if err := loader.LoadConfig(data, &raw); err != nil { + return fmt.Errorf("parsing config file %q: %w", configPath, err) + } + + rawStr := string(raw) + + for _, fi := range fields { + if fi.configKey == "" { + continue + } + + // Don't override fields explicitly set by CLI flags + if fi.flagLong != "" && fs.IsSet(fi.flagLong) { + continue + } + + var section string + if fi.configKey == "." { + section = rawStr + } else { + result := gjson.Get(rawStr, fi.configKey) + if !result.Exists() { + continue + } + section = result.Raw + } + + if section == "" { + continue + } + + fieldVal := fieldByIndex(rv, fi.fieldIndex) + if err := unmarshalIntoField(fieldVal, section); err != nil { + return fmt.Errorf("config key %q: %w", fi.configKey, err) + } + } + + return nil +} + +// stringFieldForFlag returns the current string value of the flag's struct field. +func stringFieldForFlag(rv reflect.Value, fields []fieldInfo, flagName string) string { + for _, fi := range fields { + if fi.flagLong != flagName { + continue + } + fieldVal := fieldByIndex(rv, fi.fieldIndex) + if fieldVal.Kind() == reflect.String { + return fieldVal.String() + } + if fieldVal.Kind() == reflect.Ptr && !fieldVal.IsNil() && fieldVal.Elem().Kind() == reflect.String { + return fieldVal.Elem().String() + } + } + return "" +} + +// unmarshalIntoField unmarshals a JSON string into the given reflect.Value. +func unmarshalIntoField(v reflect.Value, jsonStr string) error { + if !v.CanAddr() { + return fmt.Errorf("field is not addressable") + } + ptr := v.Addr().Interface() + return json.Unmarshal([]byte(jsonStr), ptr) +} diff --git a/cli/example/go.mod b/cli/example/go.mod new file mode 100644 index 0000000..5cb966f --- /dev/null +++ b/cli/example/go.mod @@ -0,0 +1,18 @@ +module github.com/runreveal/lib/cli/example + +go 1.21 + +require github.com/runreveal/lib/cli v0.0.0 + +require ( + github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c // indirect + github.com/segmentio/asm v1.1.3 // indirect + github.com/segmentio/encoding v0.3.6 // indirect + github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a // indirect + github.com/tidwall/gjson v1.14.4 // indirect + github.com/tidwall/match v1.1.1 // indirect + github.com/tidwall/pretty v1.2.0 // indirect + golang.org/x/sys v0.1.0 // indirect +) + +replace github.com/runreveal/lib/cli => ../ diff --git a/cli/example/go.sum b/cli/example/go.sum new file mode 100644 index 0000000..3bfe98a --- /dev/null +++ b/cli/example/go.sum @@ -0,0 +1,27 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= +github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c h1:F+kw4v9T9acZp7Ln29qlZvTRfaVMNzC6oO7To8OtATA= +github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c/go.mod h1:Wy+jC29YN5FuH7qC4guAb44D85Kb+zU28Xuerg0IuK0= +github.com/segmentio/asm v1.1.3 h1:WM03sfUOENvvKexOLp+pCqgb/WDjsi7EK8gIsICtzhc= +github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg= +github.com/segmentio/encoding v0.3.6 h1:E6lVLyDPseWEulBmCmAKPanDd3jiyGDo5gMcugCRwZQ= +github.com/segmentio/encoding v0.3.6/go.mod h1:n0JeuIqEQrQoPDGsjo8UNd1iA0U8d8+oHAA4E3G3OxM= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a h1:SJy1Pu0eH1C29XwJucQo73FrleVK6t4kYz4NVhp34Yw= +github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a/go.mod h1:DFSS3NAGHthKo1gTlmEcSBiZrRJXi28rLNd/1udP1c8= +github.com/tidwall/gjson v1.14.4 h1:uo0p8EbA09J7RQaflQ1aBRffTR7xedD2bcIVSYxLnkM= +github.com/tidwall/gjson v1.14.4/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= +github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= +github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= +github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs= +github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +golang.org/x/sys v0.0.0-20211110154304-99a53858aa08/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= +golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/cli/example/main.go b/cli/example/main.go new file mode 100644 index 0000000..2b034b1 --- /dev/null +++ b/cli/example/main.go @@ -0,0 +1,117 @@ +// Command example demonstrates the github.com/runreveal/lib/cli framework. +package main + +import ( + "context" + "fmt" + "os" + "time" + + "github.com/runreveal/lib/cli" +) + +// Globals are shared flags embedded into every command. +type Globals struct { + Verbose bool `cli:"verbose,v" usage:"enable verbose output"` + Config string `cli:"config,c" usage:"config file path" default:"config.json"` +} + +// ServeCmd is the handler for the "serve" subcommand. +type ServeCmd struct { + Globals + Addr string `cli:"addr,a" usage:"listen address" default:":8080"` + Timeout time.Duration `cli:"timeout,t" usage:"request timeout" default:"30s"` + + // DB is loaded from the config file's "database" section. + DB DBConfig `config:"database"` +} + +type DBConfig struct { + DSN string `json:"dsn"` +} + +func (s *ServeCmd) Validate() error { + if s.Addr == "" { + return fmt.Errorf("--addr must not be empty") + } + return nil +} + +func (s *ServeCmd) Run(ctx context.Context, args []string) error { + if s.Verbose { + fmt.Printf("verbose mode enabled\n") + fmt.Printf("config file: %s\n", s.Config) + if s.DB.DSN != "" { + fmt.Printf("database DSN: %s\n", s.DB.DSN) + } + } + fmt.Printf("serving on %s (timeout: %s)\n", s.Addr, s.Timeout) + return nil +} + +// MigrateCmd is in the "admin" group. +type MigrateCmd struct { + Globals + DryRun bool `cli:"dry-run" usage:"print migrations without running"` + DB string `cli:"db" usage:"database name" default:"prod"` +} + +func (m *MigrateCmd) Run(ctx context.Context, args []string) error { + if m.DryRun { + fmt.Printf("[dry-run] would migrate database: %s\n", m.DB) + } else { + fmt.Printf("migrating database: %s\n", m.DB) + } + return nil +} + +// PingCmd demonstrates positional args. +type PingCmd struct { + Globals + Count int `cli:"count,n" usage:"number of pings" default:"3"` +} + +func (p *PingCmd) Run(ctx context.Context, args []string) error { + hosts := args + if len(hosts) == 0 { + hosts = []string{"localhost"} + } + for _, host := range hosts { + for i := 0; i < p.Count; i++ { + fmt.Printf("ping #%d -> %s\n", i+1, host) + } + } + return nil +} + +func main() { + // Middleware: log every command execution + loggingMW := func(ctx context.Context, info cli.CommandInfo, next func(context.Context) error) error { + fmt.Printf("[log] running command: %s\n", info.Name) + err := next(ctx) + if err != nil { + fmt.Printf("[log] command failed: %v\n", err) + } + return err + } + + app := cli.New("example", "Example CLI demonstrating the cli framework", + cli.WithVersion("1.0.0"), + cli.WithConfigFlag("config"), + cli.WithMiddleware(loggingMW), + ) + + app.AddCommand( + cli.Command("serve", "Start the HTTP server", &ServeCmd{}), + cli.CommandWithOptions("ping", "Ping one or more hosts", &PingCmd{}, + []cli.CmdOption{cli.WithArgs(cli.MinArgs(0))}, + ), + cli.Group("admin", "Administrative commands", + cli.CommandWithOptions("migrate", "Run database migrations", &MigrateCmd{}, + []cli.CmdOption{cli.WithArgs(cli.NoArgs)}, + ), + ), + ) + + os.Exit(app.Run(context.Background(), os.Args[1:])) +} diff --git a/cli/flags.go b/cli/flags.go new file mode 100644 index 0000000..9bc39af --- /dev/null +++ b/cli/flags.go @@ -0,0 +1,403 @@ +package cli + +import ( + "encoding" + "fmt" + "strconv" + "strings" + "time" +) + +// flagDef holds the definition of a single flag. +type flagDef struct { + long string // long name (without --) + short string // short alias (single char, without -) + usage string + defVal string + typeName string // for help display + setValue func(s string) error + getBool func() bool // non-nil only for bool flags +} + +// FlagSet is a parsed set of flag definitions. +type FlagSet struct { + defs []*flagDef + byLong map[string]*flagDef + byShort map[string]*flagDef + explicit map[string]bool // flags explicitly set +} + +func newFlagSet() *FlagSet { + return &FlagSet{ + byLong: make(map[string]*flagDef), + byShort: make(map[string]*flagDef), + explicit: make(map[string]bool), + } +} + +func (fs *FlagSet) add(def *flagDef) error { + if def.long != "" { + if _, exists := fs.byLong[def.long]; exists { + return fmt.Errorf("duplicate flag --%s", def.long) + } + fs.byLong[def.long] = def + } + if def.short != "" { + if _, exists := fs.byShort[def.short]; exists { + return fmt.Errorf("duplicate short flag -%s", def.short) + } + fs.byShort[def.short] = def + } + fs.defs = append(fs.defs, def) + return nil +} + +// IsSet reports whether the named flag was explicitly set. +func (fs *FlagSet) IsSet(name string) bool { + return fs.explicit[name] +} + +// DumpValues returns a map of flag name → current value string. +// For a FlagSet obtained after parsing, this reflects defaults plus any +// explicit flag values (stored in defVal after setValue is called). +func (fs *FlagSet) DumpValues() map[string]any { + m := make(map[string]any, len(fs.defs)) + for _, d := range fs.defs { + if fs.explicit[d.long] { + m[d.long] = "(set)" + } else { + m[d.long] = d.defVal + } + } + return m +} + +// Parse parses args, sets flag values, and returns remaining positional args. +func (fs *FlagSet) Parse(args []string) ([]string, error) { + var positional []string + i := 0 + for i < len(args) { + arg := args[i] + if arg == "--" { + positional = append(positional, args[i+1:]...) + break + } + if !strings.HasPrefix(arg, "-") || arg == "-" { + positional = append(positional, arg) + i++ + continue + } + + var err error + var advance int + if strings.HasPrefix(arg, "--") { + advance, err = fs.parseLong(arg[2:], args[i+1:]) + } else { + advance, err = fs.parseShort(arg[1:], args[i+1:]) + } + if err != nil { + return nil, err + } + i += 1 + advance + } + return positional, nil +} + +func (fs *FlagSet) parseLong(raw string, remaining []string) (int, error) { + name, val, hasEq := strings.Cut(raw, "=") + def, ok := fs.byLong[name] + if !ok { + return 0, fmt.Errorf("unknown flag: --%s", name) + } + + if def.getBool != nil { + // Bool flag: value is optional + if hasEq { + if err := def.setValue(val); err != nil { + return 0, fmt.Errorf("invalid value %q for --%s: %w", val, name, err) + } + } else { + if err := def.setValue("true"); err != nil { + return 0, err + } + } + fs.explicit[name] = true + return 0, nil + } + + if hasEq { + if err := def.setValue(val); err != nil { + return 0, fmt.Errorf("invalid value %q for --%s: %w", val, name, err) + } + fs.explicit[name] = true + return 0, nil + } + + if len(remaining) == 0 { + return 0, fmt.Errorf("flag --%s requires a value", name) + } + if err := def.setValue(remaining[0]); err != nil { + return 0, fmt.Errorf("invalid value %q for --%s: %w", remaining[0], name, err) + } + fs.explicit[name] = true + return 1, nil +} + +func (fs *FlagSet) parseShort(raw string, remaining []string) (int, error) { + // raw is everything after the leading - + // e.g. "abc" for -abc, "v" for -v, "f=val" for -f=val + + // Handle -f=value + if idx := strings.IndexByte(raw, '='); idx == 1 { + char := string(raw[0]) + val := raw[2:] + def, ok := fs.byShort[char] + if !ok { + return 0, fmt.Errorf("unknown flag: -%s", char) + } + if err := def.setValue(val); err != nil { + return 0, fmt.Errorf("invalid value %q for -%s: %w", val, char, err) + } + fs.explicit[def.long] = true + return 0, nil + } + + // Single short flag + if len(raw) == 1 { + char := raw + def, ok := fs.byShort[char] + if !ok { + return 0, fmt.Errorf("unknown flag: -%s", char) + } + if def.getBool != nil { + if err := def.setValue("true"); err != nil { + return 0, err + } + fs.explicit[def.long] = true + return 0, nil + } + if len(remaining) == 0 { + return 0, fmt.Errorf("flag -%s requires a value", char) + } + if err := def.setValue(remaining[0]); err != nil { + return 0, fmt.Errorf("invalid value %q for -%s: %w", remaining[0], char, err) + } + fs.explicit[def.long] = true + return 1, nil + } + + // Multiple short bool flags combined: -abc + for j, ch := range raw { + char := string(ch) + def, ok := fs.byShort[char] + if !ok { + return 0, fmt.Errorf("unknown flag: -%s", char) + } + if def.getBool == nil { + // Non-bool flag: rest of the string is the value + val := raw[j+1:] + if val == "" { + if len(remaining) == 0 { + return 0, fmt.Errorf("flag -%s requires a value", char) + } + if err := def.setValue(remaining[0]); err != nil { + return 0, fmt.Errorf("invalid value %q for -%s: %w", remaining[0], char, err) + } + fs.explicit[def.long] = true + return 1, nil + } + if err := def.setValue(val); err != nil { + return 0, fmt.Errorf("invalid value %q for -%s: %w", val, char, err) + } + fs.explicit[def.long] = true + return 0, nil + } + if err := def.setValue("true"); err != nil { + return 0, err + } + fs.explicit[def.long] = true + } + return 0, nil +} + +// makeFlagDef creates a flagDef for a given field pointer. +// ptr must be a pointer to the field value. +func makeFlagDef(long, short, usage, defVal string, ptr any) (*flagDef, error) { + def := &flagDef{ + long: long, + short: short, + usage: usage, + defVal: defVal, + } + + switch p := ptr.(type) { + case *string: + def.typeName = "string" + def.setValue = func(s string) error { *p = s; return nil } + case **string: + def.typeName = "string" + def.setValue = func(s string) error { *p = &s; return nil } + case *bool: + def.typeName = "" + def.getBool = func() bool { return *p } + def.setValue = func(s string) error { + v, err := strconv.ParseBool(s) + if err != nil { + return err + } + *p = v + return nil + } + case **bool: + def.typeName = "" + def.getBool = func() bool { return *p != nil && **p } + def.setValue = func(s string) error { + v, err := strconv.ParseBool(s) + if err != nil { + return err + } + *p = &v + return nil + } + case *int: + def.typeName = "int" + def.setValue = func(s string) error { + v, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return err + } + *p = int(v) + return nil + } + case **int: + def.typeName = "int" + def.setValue = func(s string) error { + v, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return err + } + iv := int(v) + *p = &iv + return nil + } + case *int64: + def.typeName = "int" + def.setValue = func(s string) error { + v, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return err + } + *p = v + return nil + } + case **int64: + def.typeName = "int" + def.setValue = func(s string) error { + v, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return err + } + *p = &v + return nil + } + case *uint: + def.typeName = "uint" + def.setValue = func(s string) error { + v, err := strconv.ParseUint(s, 10, 64) + if err != nil { + return err + } + *p = uint(v) + return nil + } + case **uint: + def.typeName = "uint" + def.setValue = func(s string) error { + v, err := strconv.ParseUint(s, 10, 64) + if err != nil { + return err + } + uv := uint(v) + *p = &uv + return nil + } + case *uint64: + def.typeName = "uint" + def.setValue = func(s string) error { + v, err := strconv.ParseUint(s, 10, 64) + if err != nil { + return err + } + *p = v + return nil + } + case **uint64: + def.typeName = "uint" + def.setValue = func(s string) error { + v, err := strconv.ParseUint(s, 10, 64) + if err != nil { + return err + } + *p = &v + return nil + } + case *float64: + def.typeName = "float" + def.setValue = func(s string) error { + v, err := strconv.ParseFloat(s, 64) + if err != nil { + return err + } + *p = v + return nil + } + case **float64: + def.typeName = "float" + def.setValue = func(s string) error { + v, err := strconv.ParseFloat(s, 64) + if err != nil { + return err + } + *p = &v + return nil + } + case *time.Duration: + def.typeName = "duration" + def.setValue = func(s string) error { + v, err := time.ParseDuration(s) + if err != nil { + return err + } + *p = v + return nil + } + case **time.Duration: + def.typeName = "duration" + def.setValue = func(s string) error { + v, err := time.ParseDuration(s) + if err != nil { + return err + } + *p = &v + return nil + } + case *[]string: + def.typeName = "strings" + def.setValue = func(s string) error { + *p = append(*p, s) + return nil + } + default: + // Check for TextUnmarshaler + if tu, ok := ptr.(encoding.TextUnmarshaler); ok { + def.typeName = "string" + def.setValue = func(s string) error { + return tu.UnmarshalText([]byte(s)) + } + } else { + return nil, fmt.Errorf("unsupported flag type %T", ptr) + } + } + + return def, nil +} diff --git a/cli/go.mod b/cli/go.mod new file mode 100644 index 0000000..4fb989c --- /dev/null +++ b/cli/go.mod @@ -0,0 +1,21 @@ +module github.com/runreveal/lib/cli + +go 1.21 + +require ( + github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c + github.com/stretchr/testify v1.8.4 + github.com/tidwall/gjson v1.14.4 +) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/segmentio/asm v1.1.3 // indirect + github.com/segmentio/encoding v0.3.6 // indirect + github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a // indirect + github.com/tidwall/match v1.1.1 // indirect + github.com/tidwall/pretty v1.2.0 // indirect + golang.org/x/sys v0.1.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/cli/go.sum b/cli/go.sum new file mode 100644 index 0000000..ec44c3f --- /dev/null +++ b/cli/go.sum @@ -0,0 +1,29 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= +github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c h1:F+kw4v9T9acZp7Ln29qlZvTRfaVMNzC6oO7To8OtATA= +github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c/go.mod h1:Wy+jC29YN5FuH7qC4guAb44D85Kb+zU28Xuerg0IuK0= +github.com/segmentio/asm v1.1.3 h1:WM03sfUOENvvKexOLp+pCqgb/WDjsi7EK8gIsICtzhc= +github.com/segmentio/asm v1.1.3/go.mod h1:Ld3L4ZXGNcSLRg4JBsZ3//1+f/TjYl0Mzen/DQy1EJg= +github.com/segmentio/encoding v0.3.6 h1:E6lVLyDPseWEulBmCmAKPanDd3jiyGDo5gMcugCRwZQ= +github.com/segmentio/encoding v0.3.6/go.mod h1:n0JeuIqEQrQoPDGsjo8UNd1iA0U8d8+oHAA4E3G3OxM= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a h1:SJy1Pu0eH1C29XwJucQo73FrleVK6t4kYz4NVhp34Yw= +github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a/go.mod h1:DFSS3NAGHthKo1gTlmEcSBiZrRJXi28rLNd/1udP1c8= +github.com/tidwall/gjson v1.14.4 h1:uo0p8EbA09J7RQaflQ1aBRffTR7xedD2bcIVSYxLnkM= +github.com/tidwall/gjson v1.14.4/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= +github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= +github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= +github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs= +github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +golang.org/x/sys v0.0.0-20211110154304-99a53858aa08/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= +golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/cli/help.go b/cli/help.go new file mode 100644 index 0000000..877cdfd --- /dev/null +++ b/cli/help.go @@ -0,0 +1,149 @@ +package cli + +import ( + "fmt" + "io" + "strings" +) + +func printAppHelp(w io.Writer, appName, desc string, children []Node, version string) { + if desc != "" { + fmt.Fprintf(w, "%s - %s\n\n", appName, desc) + } else { + fmt.Fprintf(w, "%s\n\n", appName) + } + + fmt.Fprintf(w, "Usage:\n") + if len(children) > 0 { + fmt.Fprintf(w, " %s [flags]\n\n", appName) + } else { + fmt.Fprintf(w, " %s [flags]\n\n", appName) + } + + if len(children) > 0 { + fmt.Fprintf(w, "Commands:\n") + maxLen := maxNodeNameLen(children) + for _, child := range children { + fmt.Fprintf(w, " %-*s %s\n", maxLen, child.nodeName(), child.nodeDesc()) + } + fmt.Fprintln(w) + } + + fmt.Fprintf(w, "Flags:\n") + fmt.Fprintf(w, " -h, --help show help\n") + if version != "" { + fmt.Fprintf(w, " --version show version\n") + } + + if len(children) > 0 { + fmt.Fprintf(w, "\nUse \"%s --help\" for more information.\n", appName) + } +} + +func printGroupHelp(w io.Writer, appName, path, desc string, children []Node) { + if desc != "" { + fmt.Fprintf(w, "%s %s - %s\n\n", appName, path, desc) + } else { + fmt.Fprintf(w, "%s %s\n\n", appName, path) + } + + fmt.Fprintf(w, "Usage:\n") + fmt.Fprintf(w, " %s %s [flags]\n\n", appName, path) + + if len(children) > 0 { + fmt.Fprintf(w, "Commands:\n") + maxLen := maxNodeNameLen(children) + for _, child := range children { + fmt.Fprintf(w, " %-*s %s\n", maxLen, child.nodeName(), child.nodeDesc()) + } + fmt.Fprintln(w) + } + + fmt.Fprintf(w, "Flags:\n") + fmt.Fprintf(w, " -h, --help show help\n") + fmt.Fprintf(w, "\nUse \"%s %s --help\" for more information.\n", appName, path) +} + +func printCommandHelp(w io.Writer, appName, path, desc string, handler Runnable, children []Node) { + if desc != "" { + fmt.Fprintf(w, "%s %s - %s\n\n", appName, path, desc) + } else { + fmt.Fprintf(w, "%s %s\n\n", appName, path) + } + + fmt.Fprintf(w, "Usage:\n") + if len(children) > 0 { + fmt.Fprintf(w, " %s %s [command] [flags]\n\n", appName, path) + } else { + fmt.Fprintf(w, " %s %s [flags]\n\n", appName, path) + } + + if len(children) > 0 { + fmt.Fprintf(w, "Commands:\n") + maxLen := maxNodeNameLen(children) + for _, child := range children { + fmt.Fprintf(w, " %-*s %s\n", maxLen, child.nodeName(), child.nodeDesc()) + } + fmt.Fprintln(w) + } + + // Print flags from handler + fs, _, err := buildFlagSet(handler) + if err == nil && len(fs.defs) > 0 { + fmt.Fprintf(w, "Flags:\n") + printFlagDefs(w, fs.defs) + fmt.Fprintln(w) + } else { + fmt.Fprintf(w, "Flags:\n") + } + fmt.Fprintf(w, " -h, --help show help\n") +} + +func printFlagDefs(w io.Writer, defs []*flagDef) { + // Calculate column widths + maxFlag := 0 + for _, d := range defs { + s := flagString(d) + if len(s) > maxFlag { + maxFlag = len(s) + } + } + + for _, d := range defs { + fs := flagString(d) + padding := strings.Repeat(" ", maxFlag-len(fs)) + if d.defVal != "" { + fmt.Fprintf(w, " %s%s %s (default: %s)\n", fs, padding, d.usage, d.defVal) + } else { + fmt.Fprintf(w, " %s%s %s\n", fs, padding, d.usage) + } + } +} + +func flagString(d *flagDef) string { + var sb strings.Builder + if d.short != "" { + sb.WriteString("-") + sb.WriteString(d.short) + sb.WriteString(", ") + } else { + sb.WriteString(" ") + } + sb.WriteString("--") + sb.WriteString(d.long) + if d.typeName != "" { + sb.WriteString(" ") + sb.WriteString(d.typeName) + } + return sb.String() +} + +func maxNodeNameLen(nodes []Node) int { + max := 0 + for _, n := range nodes { + if l := len(n.nodeName()); l > max { + max = l + } + } + return max +} diff --git a/cli/reflect.go b/cli/reflect.go new file mode 100644 index 0000000..7295a08 --- /dev/null +++ b/cli/reflect.go @@ -0,0 +1,158 @@ +package cli + +import ( + "fmt" + "reflect" + "strings" +) + +// fieldInfo holds parsed tag info for a struct field. +type fieldInfo struct { + flagLong string + flagShort string + configKey string + usage string + defVal string + fieldIndex []int // nested index path for embedded structs + fieldType reflect.Type +} + +// scanFields walks a struct type and extracts all field metadata. +// It handles embedded structs by recursively scanning them. +func scanFields(t reflect.Type) ([]fieldInfo, error) { + if t.Kind() == reflect.Ptr { + t = t.Elem() + } + if t.Kind() != reflect.Struct { + return nil, fmt.Errorf("expected struct, got %s", t.Kind()) + } + return scanFieldsWithIndex(t, nil) +} + +func scanFieldsWithIndex(t reflect.Type, prefix []int) ([]fieldInfo, error) { + var fields []fieldInfo + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + idx := append(append([]int{}, prefix...), i) + + // Handle embedded structs (anonymous fields) + ft := f.Type + if ft.Kind() == reflect.Ptr { + ft = ft.Elem() + } + if f.Anonymous && ft.Kind() == reflect.Struct { + sub, err := scanFieldsWithIndex(ft, idx) + if err != nil { + return nil, err + } + fields = append(fields, sub...) + continue + } + + cliTag := f.Tag.Get("cli") + configTag := f.Tag.Get("config") + usage := f.Tag.Get("usage") + defVal := f.Tag.Get("default") + + // Skip if no relevant tags at all + if cliTag == "" && configTag == "" { + continue + } + + // cli:"-" means skip + if cliTag == "-" { + continue + } + + info := fieldInfo{ + configKey: configTag, + usage: usage, + defVal: defVal, + fieldIndex: idx, + fieldType: f.Type, + } + + if cliTag != "" { + parts := strings.SplitN(cliTag, ",", 2) + info.flagLong = parts[0] + if len(parts) == 2 { + info.flagShort = strings.TrimSpace(parts[1]) + } + } + + fields = append(fields, info) + } + return fields, nil +} + +// buildFlagSet constructs a FlagSet by reflecting on the handler's struct tags. +// It also returns the scanned fields so callers can reuse them (e.g. for +// applyDefaults and config loading) without re-scanning. +func buildFlagSet(handler Runnable) (*FlagSet, []fieldInfo, error) { + rv := reflect.ValueOf(handler) + if rv.Kind() != reflect.Ptr { + return nil, nil, fmt.Errorf("handler must be a pointer to a struct") + } + rv = rv.Elem() + if rv.Kind() != reflect.Struct { + return nil, nil, fmt.Errorf("handler must be a pointer to a struct") + } + + fields, err := scanFields(rv.Type()) + if err != nil { + return nil, nil, err + } + + fs := newFlagSet() + for _, fi := range fields { + if fi.flagLong == "" { + continue + } + + fieldVal := fieldByIndex(rv, fi.fieldIndex) + ptr := fieldVal.Addr().Interface() + + def, err := makeFlagDef(fi.flagLong, fi.flagShort, fi.usage, fi.defVal, ptr) + if err != nil { + return nil, nil, fmt.Errorf("field %v: %w", fi.fieldIndex, err) + } + + if err := fs.add(def); err != nil { + return nil, nil, err + } + } + + return fs, fields, nil +} + +// applyDefaults sets the default values using pre-scanned fields. +func applyDefaults(fs *FlagSet, fields []fieldInfo) error { + for _, fi := range fields { + if fi.defVal == "" || fi.flagLong == "" { + continue + } + def, ok := fs.byLong[fi.flagLong] + if !ok { + continue + } + if err := def.setValue(fi.defVal); err != nil { + return fmt.Errorf("applying default %q to --%s: %w", fi.defVal, fi.flagLong, err) + } + } + return nil +} + +// fieldByIndex retrieves a nested struct field by index path, initializing +// embedded pointer structs as needed. +func fieldByIndex(v reflect.Value, index []int) reflect.Value { + for _, i := range index { + if v.Kind() == reflect.Ptr { + if v.IsNil() { + v.Set(reflect.New(v.Type().Elem())) + } + v = v.Elem() + } + v = v.Field(i) + } + return v +} From 4dbe5f074eedbdd72272acb258ec43df794c8543 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Sun, 5 Apr 2026 22:11:17 +0000 Subject: [PATCH 02/24] fix(cli): address PR #22 review feedback - config.go: use github.com/segmentio/encoding/json for unmarshalIntoField - config.go: add replaceEnvInJSON for env-var substitution on raw JSON bytes, since loader.LoadConfig's reflection-based pass is opaque to json.RawMessage - cli.go: replace handlerFlagSets sync.Map with context-carried *FlagSet; add FlagSetFromContext and update IsSet(ctx, name) signature - cli.go: fix DumpConfig to reflect actual handler field values instead of returning "(set)" for explicitly-set flags - cli.go: collapse Command+CommandWithOptions into a single Command(...any) that type-switches each element as Node or CmdOption - flags.go: remove dead DumpValues method (superseded by DumpConfig) - cli_test.go: update tests for new IsSet/Command APIs; add HuJSON and env-var replacement config tests - example/main.go: update to new Command API --- cli/cli.go | 71 +++++++++++++++++++++++++++------------------ cli/cli_test.go | 47 +++++++++++++++++++++++++----- cli/config.go | 29 ++++++++++++++++-- cli/example/main.go | 8 ++--- cli/flags.go | 15 ---------- cli/go.mod | 2 +- 6 files changed, 110 insertions(+), 62 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index e3a08af..f841c55 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -13,12 +13,10 @@ import ( "reflect" "runtime/debug" "strings" - "sync" ) -// handlerFlagSets maps handler pointer → *FlagSet during execution, -// enabling IsSet() to query which flags were explicitly set. -var handlerFlagSets sync.Map +// flagSetKey is the context key used to carry the *FlagSet during command execution. +type flagSetKey struct{} // Runnable is the core interface every command handler must implement. type Runnable interface { @@ -95,16 +93,19 @@ func WithArgs(f ArgsFunc) CmdOption { return func(o *cmdOptions) { o.argsFunc = f } } -// Command creates a command node with an optional set of child nodes. -func Command(name, desc string, handler Runnable, children ...Node) Node { - return CommandWithOptions(name, desc, handler, nil, children...) -} - -// CommandWithOptions creates a command node with options and optional children. -func CommandWithOptions(name, desc string, handler Runnable, opts []CmdOption, children ...Node) Node { +// Command creates a command node. Each element of opts may be a Node (child +// subcommand) or a CmdOption (behavioural option); they are distinguished by +// type at runtime. +func Command(name, desc string, handler Runnable, opts ...any) Node { o := cmdOptions{} + var children []Node for _, opt := range opts { - opt(&o) + switch v := opt.(type) { + case Node: + children = append(children, v) + case CmdOption: + v(&o) + } } return &commandNode{name: name, desc: desc, handler: handler, children: children, opts: o} } @@ -303,10 +304,8 @@ func (a *App) executeCommand(ctx context.Context, node *commandNode, args []stri return 1, nil } - // Register the FlagSet so IsSet() can query it during Run - handlerPtr := reflect.ValueOf(handler).Pointer() - handlerFlagSets.Store(handlerPtr, fs) - defer handlerFlagSets.Delete(handlerPtr) + // Carry the FlagSet in context so IsSet() can query it during Run. + ctx = context.WithValue(ctx, flagSetKey{}, fs) // Load config file if configured if a.configFlag != "" { @@ -364,27 +363,41 @@ func buildChain(middlewares []Middleware, info CommandInfo, final func(context.C return chain } +// FlagSetFromContext returns the *FlagSet stored in ctx during command +// execution, or nil if called outside of a command handler. +func FlagSetFromContext(ctx context.Context) *FlagSet { + fs, _ := ctx.Value(flagSetKey{}).(*FlagSet) + return fs +} + // IsSet reports whether a flag was explicitly set on the command line. -// Must be called from within Run (or Validate) to return meaningful results. -func IsSet(handler Runnable, flagName string) bool { - handlerPtr := reflect.ValueOf(handler).Pointer() - if val, ok := handlerFlagSets.Load(handlerPtr); ok { - return val.(*FlagSet).IsSet(flagName) +// Must be called from within Run to return meaningful results. +func IsSet(ctx context.Context, flagName string) bool { + if fs := FlagSetFromContext(ctx); fs != nil { + return fs.IsSet(flagName) } return false } -// DumpConfig returns the resolved flag configuration as a map. -// When called from within Run, it reflects the live parsed values. +// DumpConfig returns the resolved configuration of handler as a map of +// flag name → current field value. It reflects directly over the handler +// struct, so it captures values set by both CLI flags and config files. func DumpConfig(handler Runnable) map[string]any { - handlerPtr := reflect.ValueOf(handler).Pointer() - if val, ok := handlerFlagSets.Load(handlerPtr); ok { - return val.(*FlagSet).DumpValues() + rv := reflect.ValueOf(handler) + if rv.Kind() == reflect.Ptr { + rv = rv.Elem() } - // Outside of Run — build a fresh FlagSet to get at least the defaults. - fs, _, err := buildFlagSet(handler) + fields, err := scanFields(rv.Type()) if err != nil { return nil } - return fs.DumpValues() + m := make(map[string]any, len(fields)) + for _, fi := range fields { + if fi.flagLong == "" { + continue + } + fieldVal := fieldByIndex(rv, fi.fieldIndex) + m[fi.flagLong] = fieldVal.Interface() + } + return m } diff --git a/cli/cli_test.go b/cli/cli_test.go index 78f7230..71f7ef0 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -86,8 +86,8 @@ type isSetCmd struct { nameWasSet bool } -func (i *isSetCmd) Run(_ context.Context, _ []string) error { - i.nameWasSet = cli.IsSet(i, "name") +func (i *isSetCmd) Run(ctx context.Context, _ []string) error { + i.nameWasSet = cli.IsSet(ctx, "name") return nil } @@ -501,9 +501,7 @@ func TestEdge_NoArgs(t *testing.T) { func TestEdge_ArgsValidation_NoArgs(t *testing.T) { var buf bytes.Buffer app := cli.New("app", "test", cli.WithOutput(&buf)) - app.AddCommand(cli.CommandWithOptions("run", "run", &noopCmd{}, - []cli.CmdOption{cli.WithArgs(cli.NoArgs)}, - )) + app.AddCommand(cli.Command("run", "run", &noopCmd{}, cli.WithArgs(cli.NoArgs))) code := app.Run(context.Background(), []string{"run", "extra"}) assert.Equal(t, 1, code) @@ -512,9 +510,7 @@ func TestEdge_ArgsValidation_NoArgs(t *testing.T) { func TestEdge_ArgsValidation_ExactArgs(t *testing.T) { var buf bytes.Buffer app := cli.New("app", "test", cli.WithOutput(&buf)) - app.AddCommand(cli.CommandWithOptions("run", "run", &noopCmd{}, - []cli.CmdOption{cli.WithArgs(cli.ExactArgs(2))}, - )) + app.AddCommand(cli.Command("run", "run", &noopCmd{}, cli.WithArgs(cli.ExactArgs(2)))) code := app.Run(context.Background(), []string{"run", "a", "b"}) assert.Equal(t, 0, code) @@ -691,3 +687,38 @@ func TestMiddleware_CommandInfo(t *testing.T) { assert.Equal(t, 0, code) assert.Equal(t, "admin migrate", capturedInfo.Name) } + +func TestConfig_HuJSON(t *testing.T) { + handler := &configHandler{} + // HuJSON allows C-style comments and trailing commas. + f := writeConfigFile(t, `{ + // database connection settings + "database": { + "host": "hujson-host", + "port": 5433, // trailing comma + } + }`) + + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + code := app.Run(context.Background(), []string{"run", "--config", f}) + assert.Equal(t, 0, code) + assert.Equal(t, "hujson-host", handler.DB.Host) + assert.Equal(t, 5433, handler.DB.Port) +} + +func TestConfig_EnvVarReplacement(t *testing.T) { + handler := &overrideableHandler{} + t.Setenv("CLI_TEST_HOST", "env-replaced-host") + f := writeConfigFile(t, `{"host": "$CLI_TEST_HOST"}`) + + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + code := app.Run(context.Background(), []string{"run", "--config", f}) + assert.Equal(t, 0, code) + assert.Equal(t, "env-replaced-host", handler.Host) +} diff --git a/cli/config.go b/cli/config.go index 61fa88e..72347d8 100644 --- a/cli/config.go +++ b/cli/config.go @@ -6,11 +6,31 @@ import ( "fmt" "os" "reflect" + "regexp" "github.com/runreveal/lib/loader" + sgjson "github.com/segmentio/encoding/json" "github.com/tidwall/gjson" ) +// envVarRegex matches JSON string values that are exactly an env-var reference, +// e.g. "$MY_VAR". The whole quoted token is replaced with the env var's value. +var envVarRegex = regexp.MustCompile(`"\$([A-Za-z_][A-Za-z0-9_]*)"`) + +// replaceEnvInJSON replaces "$VAR" tokens inside JSON string values with the +// corresponding environment variable's value, properly re-encoded as JSON. +func replaceEnvInJSON(data []byte) []byte { + return envVarRegex.ReplaceAllFunc(data, func(match []byte) []byte { + name := string(match[2 : len(match)-1]) // strip leading `"$` and trailing `"` + val := os.Getenv(name) + encoded, err := json.Marshal(val) + if err != nil { + return match // should never happen for a plain string + } + return encoded + }) +} + // loadConfigIntoHandler reads the config file (path from the named flag), // processes it via loader.LoadConfig, then applies config-tagged fields. // Only sets fields that were NOT explicitly set via CLI flags. @@ -41,13 +61,16 @@ func loadConfigIntoHandler(handler Runnable, fs *FlagSet, configFlagName string, return fmt.Errorf("reading config file %q: %w", configPath, err) } - // Process via loader (hujson + env replacement) + // Process via loader (hujson standardisation). var raw json.RawMessage if err := loader.LoadConfig(data, &raw); err != nil { return fmt.Errorf("parsing config file %q: %w", configPath, err) } - rawStr := string(raw) + // Apply env-var substitution on the raw JSON bytes. loader.LoadConfig + // performs reflection-based replacement on structs, but json.RawMessage + // is opaque to that pass, so we apply the regex replacement ourselves. + rawStr := string(replaceEnvInJSON(raw)) for _, fi := range fields { if fi.configKey == "" { @@ -106,5 +129,5 @@ func unmarshalIntoField(v reflect.Value, jsonStr string) error { return fmt.Errorf("field is not addressable") } ptr := v.Addr().Interface() - return json.Unmarshal([]byte(jsonStr), ptr) + return sgjson.Unmarshal([]byte(jsonStr), ptr) } diff --git a/cli/example/main.go b/cli/example/main.go index 2b034b1..9541d4e 100644 --- a/cli/example/main.go +++ b/cli/example/main.go @@ -103,13 +103,9 @@ func main() { app.AddCommand( cli.Command("serve", "Start the HTTP server", &ServeCmd{}), - cli.CommandWithOptions("ping", "Ping one or more hosts", &PingCmd{}, - []cli.CmdOption{cli.WithArgs(cli.MinArgs(0))}, - ), + cli.Command("ping", "Ping one or more hosts", &PingCmd{}, cli.WithArgs(cli.MinArgs(0))), cli.Group("admin", "Administrative commands", - cli.CommandWithOptions("migrate", "Run database migrations", &MigrateCmd{}, - []cli.CmdOption{cli.WithArgs(cli.NoArgs)}, - ), + cli.Command("migrate", "Run database migrations", &MigrateCmd{}, cli.WithArgs(cli.NoArgs)), ), ) diff --git a/cli/flags.go b/cli/flags.go index 9bc39af..1df3480 100644 --- a/cli/flags.go +++ b/cli/flags.go @@ -57,21 +57,6 @@ func (fs *FlagSet) IsSet(name string) bool { return fs.explicit[name] } -// DumpValues returns a map of flag name → current value string. -// For a FlagSet obtained after parsing, this reflects defaults plus any -// explicit flag values (stored in defVal after setValue is called). -func (fs *FlagSet) DumpValues() map[string]any { - m := make(map[string]any, len(fs.defs)) - for _, d := range fs.defs { - if fs.explicit[d.long] { - m[d.long] = "(set)" - } else { - m[d.long] = d.defVal - } - } - return m -} - // Parse parses args, sets flag values, and returns remaining positional args. func (fs *FlagSet) Parse(args []string) ([]string, error) { var positional []string diff --git a/cli/go.mod b/cli/go.mod index 4fb989c..6f4afdb 100644 --- a/cli/go.mod +++ b/cli/go.mod @@ -4,6 +4,7 @@ go 1.21 require ( github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c + github.com/segmentio/encoding v0.3.6 github.com/stretchr/testify v1.8.4 github.com/tidwall/gjson v1.14.4 ) @@ -12,7 +13,6 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/segmentio/asm v1.1.3 // indirect - github.com/segmentio/encoding v0.3.6 // indirect github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.0 // indirect From 68b7204fccfb354b347e177f33696777bf56ed47 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Sun, 5 Apr 2026 21:53:48 -0700 Subject: [PATCH 03/24] ci: update Go to 1.25, fix line length formatting - Update CI workflow from Go 1.21 to 1.25 (fixes golines install) - Update cli and cli/example go.mod to Go 1.25 - Fix lines exceeding 128 char limit (golines -m 128) --- .github/workflows/ci.yml | 2 +- cli/cli_test.go | 12 ++++++------ cli/example/go.mod | 2 +- cli/example/main.go | 8 ++++---- cli/flags.go | 6 +++--- cli/go.mod | 2 +- 6 files changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d3f72e1..bda6391 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v3 with: - go-version: "1.21" + go-version: "1.25" # HACK: actions doesn't support multiple modules in one repo for caching cache: false diff --git a/cli/cli_test.go b/cli/cli_test.go index 71f7ef0..25a2685 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -18,7 +18,7 @@ import ( type echoCmd struct { Message string `cli:"message,m" usage:"message to echo" default:"hello"` - Count int `cli:"count,n" usage:"number of times" default:"1"` + Count int `cli:"count,n" usage:"number of times" default:"1"` } func (e *echoCmd) Run(_ context.Context, _ []string) error { return nil } @@ -200,7 +200,7 @@ func TestFlagParsing_Defaults(t *testing.T) { type embeddedGlobals struct { Verbose bool `cli:"verbose,v" usage:"verbose mode"` - Config string `cli:"config,c" usage:"config file" default:"config.json"` + Config string `cli:"config,c" usage:"config file" default:"config.json"` } type embeddedCmd struct { @@ -548,7 +548,7 @@ type dbSection struct { type configHandler struct { ConfigFile string `cli:"config,c" usage:"config file"` - DB dbSection `config:"database"` + DB dbSection ` config:"database"` } func (c *configHandler) Run(_ context.Context, _ []string) error { return nil } @@ -584,7 +584,7 @@ func TestConfig_MissingFileSilent(t *testing.T) { type overrideableHandler struct { ConfigFile string `cli:"config" usage:"config file"` - Host string `cli:"host" usage:"host" default:"flag-default" config:"host"` + Host string `cli:"host" usage:"host" default:"flag-default" config:"host"` } func (o *overrideableHandler) Run(_ context.Context, _ []string) error { return nil } @@ -621,7 +621,7 @@ type nestedVal struct { } type nestedConfigHandler struct { ConfigFile string `cli:"config" usage:"config file"` - Nested nestedVal `config:"a.b"` + Nested nestedVal ` config:"a.b"` } func (n *nestedConfigHandler) Run(_ context.Context, _ []string) error { return nil } @@ -651,7 +651,7 @@ type rootSection struct { } type rootConfigHandler struct { ConfigFile string `cli:"config" usage:"config file"` - All rootSection `config:"."` + All rootSection ` config:"."` } func (r *rootConfigHandler) Run(_ context.Context, _ []string) error { return nil } diff --git a/cli/example/go.mod b/cli/example/go.mod index 5cb966f..1455d71 100644 --- a/cli/example/go.mod +++ b/cli/example/go.mod @@ -1,6 +1,6 @@ module github.com/runreveal/lib/cli/example -go 1.21 +go 1.25 require github.com/runreveal/lib/cli v0.0.0 diff --git a/cli/example/main.go b/cli/example/main.go index 9541d4e..c45b6a2 100644 --- a/cli/example/main.go +++ b/cli/example/main.go @@ -13,14 +13,14 @@ import ( // Globals are shared flags embedded into every command. type Globals struct { Verbose bool `cli:"verbose,v" usage:"enable verbose output"` - Config string `cli:"config,c" usage:"config file path" default:"config.json"` + Config string `cli:"config,c" usage:"config file path" default:"config.json"` } // ServeCmd is the handler for the "serve" subcommand. type ServeCmd struct { Globals - Addr string `cli:"addr,a" usage:"listen address" default:":8080"` - Timeout time.Duration `cli:"timeout,t" usage:"request timeout" default:"30s"` + Addr string `cli:"addr,a" usage:"listen address" default:":8080"` + Timeout time.Duration `cli:"timeout,t" usage:"request timeout" default:"30s"` // DB is loaded from the config file's "database" section. DB DBConfig `config:"database"` @@ -53,7 +53,7 @@ func (s *ServeCmd) Run(ctx context.Context, args []string) error { type MigrateCmd struct { Globals DryRun bool `cli:"dry-run" usage:"print migrations without running"` - DB string `cli:"db" usage:"database name" default:"prod"` + DB string `cli:"db" usage:"database name" default:"prod"` } func (m *MigrateCmd) Run(ctx context.Context, args []string) error { diff --git a/cli/flags.go b/cli/flags.go index 1df3480..2cb0c3a 100644 --- a/cli/flags.go +++ b/cli/flags.go @@ -209,9 +209,9 @@ func (fs *FlagSet) parseShort(raw string, remaining []string) (int, error) { // ptr must be a pointer to the field value. func makeFlagDef(long, short, usage, defVal string, ptr any) (*flagDef, error) { def := &flagDef{ - long: long, - short: short, - usage: usage, + long: long, + short: short, + usage: usage, defVal: defVal, } diff --git a/cli/go.mod b/cli/go.mod index 6f4afdb..f8d6280 100644 --- a/cli/go.mod +++ b/cli/go.mod @@ -1,6 +1,6 @@ module github.com/runreveal/lib/cli -go 1.21 +go 1.25 require ( github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c From 4b947b8dc803923fc609ddd0e454cfa5a096af3a Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Sun, 5 Apr 2026 22:05:24 -0700 Subject: [PATCH 04/24] ci: remove obsolete GOEXPERIMENT=nocoverageredesign This experiment flag was removed in Go 1.25 and causes build failures. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 8627fb5..fa362ff 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ GOTAGS = testing GO ?= $(shell which go) -export GOEXPERIMENT=nocoverageredesign +# GOEXPERIMENT=nocoverageredesign was removed in Go 1.25 .PHONY: test test: From d7a90ba456af7953604c8318b211dcbf9004ad45 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Sun, 5 Apr 2026 22:09:54 -0700 Subject: [PATCH 05/24] ci: downgrade to Go 1.24 for golangci-lint compatibility --- .github/workflows/ci.yml | 2 +- cli/example/go.mod | 2 +- cli/go.mod | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bda6391..ae7e3fa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v3 with: - go-version: "1.25" + go-version: "1.24" # HACK: actions doesn't support multiple modules in one repo for caching cache: false diff --git a/cli/example/go.mod b/cli/example/go.mod index 1455d71..ca48e15 100644 --- a/cli/example/go.mod +++ b/cli/example/go.mod @@ -1,6 +1,6 @@ module github.com/runreveal/lib/cli/example -go 1.25 +go 1.24 require github.com/runreveal/lib/cli v0.0.0 diff --git a/cli/go.mod b/cli/go.mod index f8d6280..ad4d6d0 100644 --- a/cli/go.mod +++ b/cli/go.mod @@ -1,6 +1,6 @@ module github.com/runreveal/lib/cli -go 1.25 +go 1.24 require ( github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c From 65f1f24ce68ef09281be856c35573b2845392479 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Sun, 5 Apr 2026 22:12:12 -0700 Subject: [PATCH 06/24] ci: upgrade golangci-lint to v1.64.8 for Go 1.24 compat --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index fa362ff..d53577a 100644 --- a/Makefile +++ b/Makefile @@ -41,7 +41,7 @@ lint: $(GOPATH)/bin/golangci-lint done $(GOPATH)/bin/golangci-lint: - $(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.2 + $(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.64.8 $(GOPATH)/bin/golines: $(GO) install github.com/segmentio/golines@latest From 77e6724b921914c3f57dd8156b374f4693301ec7 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 13:07:17 -0700 Subject: [PATCH 07/24] cli: add WithGlobals, ConfigAt, and GlobalsFromContext - WithGlobals(ptr) registers app-level flags available on all commands, eliminating the need to embed a Globals struct in every command - GlobalsFromContext[T](ctx) retrieves the globals pointer in handlers - ConfigAt(key, dst) registers a config file section to unmarshal into a command's field, as an alternative to config:"key" struct tags - Config flag can now live on globals instead of each handler - Both patterns (struct tags and ConfigAt) work and can be mixed --- cli/cli.go | 81 +++++++++++++++++++++--- cli/cli_test.go | 150 ++++++++++++++++++++++++++++++++++++++++++++ cli/config.go | 100 +++++++++++++++++++---------- cli/example/main.go | 37 +++++++---- cli/help.go | 19 ++++-- cli/reflect.go | 38 +++++++++++ 6 files changed, 367 insertions(+), 58 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index f841c55..9100d3a 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -18,6 +18,15 @@ import ( // flagSetKey is the context key used to carry the *FlagSet during command execution. type flagSetKey struct{} +// globalsKey is the context key used to carry the globals pointer during execution. +type globalsKey struct{} + +// configBinding pairs a config file key with a destination pointer for ConfigAt. +type configBinding struct { + key string + dst any +} + // Runnable is the core interface every command handler must implement. type Runnable interface { Run(ctx context.Context, args []string) error @@ -85,7 +94,8 @@ func (g *groupNode) isGroup() bool { return true } type CmdOption func(*cmdOptions) type cmdOptions struct { - argsFunc ArgsFunc + argsFunc ArgsFunc + configBindings []configBinding } // WithArgs sets an args validation function on a command. @@ -93,6 +103,15 @@ func WithArgs(f ArgsFunc) CmdOption { return func(o *cmdOptions) { o.argsFunc = f } } +// ConfigAt registers a config file section to be unmarshaled into dst. +// key is a dot-separated path into the config file JSON (e.g. "serve", "common.db"). +// Use "." for the entire config root. dst must be a pointer. +func ConfigAt(key string, dst any) CmdOption { + return func(o *cmdOptions) { + o.configBindings = append(o.configBindings, configBinding{key: key, dst: dst}) + } +} + // Command creates a command node. Each element of opts may be a Node (child // subcommand) or a CmdOption (behavioural option); they are distinguished by // type at runtime. @@ -133,6 +152,13 @@ func WithConfigFlag(flagName string) AppOption { return func(a *App) { a.configFlag = flagName } } +// WithGlobals registers a struct pointer whose cli-tagged fields become +// flags available on every command. The pointer is stored in context and +// can be retrieved with GlobalsFromContext. +func WithGlobals(ptr any) AppOption { + return func(a *App) { a.globals = ptr } +} + // WithOutput sets the writer for help/error output (default: os.Stderr). func WithOutput(w io.Writer) AppOption { return func(a *App) { a.output = w } @@ -144,6 +170,7 @@ type App struct { desc string version string configFlag string + globals any // pointer to globals struct, if set middlewares []Middleware children []Node output io.Writer @@ -277,7 +304,7 @@ func (a *App) executeCommand(ctx context.Context, node *commandNode, args []stri // Check for --help before doing anything else for _, arg := range args { if arg == "--help" || arg == "-h" { - printCommandHelp(a.output, a.name, path, node.desc, handler, node.children) + printCommandHelp(a.output, a.name, path, node.desc, handler, node.children, a.globals) return 0, nil } if arg == "--" { @@ -291,27 +318,55 @@ func (a *App) executeCommand(ctx context.Context, node *commandNode, args []stri return 1, fmt.Errorf("building flags for %s: %w", path, err) } - // Set defaults + // If globals are set, merge their flags into the same flag set. + var globalFields []fieldInfo + if a.globals != nil { + var gf []fieldInfo + gf, err = addGlobalsToFlagSet(fs, a.globals) + if err != nil { + return 1, fmt.Errorf("building global flags: %w", err) + } + globalFields = gf + } + + // Set defaults (handler fields + global fields) if err := applyDefaults(fs, fields); err != nil { return 1, fmt.Errorf("applying defaults for %s: %w", path, err) } + if err := applyDefaults(fs, globalFields); err != nil { + return 1, fmt.Errorf("applying global defaults: %w", err) + } // Parse flags posArgs, err := fs.Parse(args) if err != nil { fmt.Fprintf(a.output, "error: %s\n\n", err) - printCommandHelp(a.output, a.name, path, node.desc, handler, node.children) + printCommandHelp(a.output, a.name, path, node.desc, handler, node.children, a.globals) return 1, nil } - // Carry the FlagSet in context so IsSet() can query it during Run. + // Carry the FlagSet and globals in context. ctx = context.WithValue(ctx, flagSetKey{}, fs) + if a.globals != nil { + ctx = context.WithValue(ctx, globalsKey{}, a.globals) + } // Load config file if configured if a.configFlag != "" { - if err := loadConfigIntoHandler(handler, fs, a.configFlag, fields); err != nil { + configJSON, err := resolveConfigJSON(handler, a.globals, fs, a.configFlag, globalFields, fields) + if err != nil { return 1, fmt.Errorf("loading config: %w", err) } + if configJSON != "" { + // Apply config:"key" struct tags on handler + if err := applyConfigTags(handler, fs, fields, configJSON); err != nil { + return 1, fmt.Errorf("loading config: %w", err) + } + // Apply ConfigAt bindings + if err := applyConfigBindings(node.opts.configBindings, configJSON); err != nil { + return 1, fmt.Errorf("loading config: %w", err) + } + } } // Validate @@ -339,7 +394,6 @@ func (a *App) executeCommand(ctx context.Context, node *commandNode, args []stri if err := chain(ctx); err != nil { var exitErr *ExitError if errors.As(err, &exitErr) { - // Propagate ExitError so Run() can extract the code. return exitErr.Code, exitErr } return 1, err @@ -370,6 +424,19 @@ func FlagSetFromContext(ctx context.Context) *FlagSet { return fs } +// GlobalsFromContext retrieves the globals pointer from context, cast to *T. +// Returns nil if no globals were registered or the type doesn't match. +func GlobalsFromContext[T any](ctx context.Context) *T { + v := ctx.Value(globalsKey{}) + if v == nil { + return nil + } + if g, ok := v.(*T); ok { + return g + } + return nil +} + // IsSet reports whether a flag was explicitly set on the command line. // Must be called from within Run to return meaningful results. func IsSet(ctx context.Context, flagName string) bool { diff --git a/cli/cli_test.go b/cli/cli_test.go index 25a2685..859f0e2 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -709,6 +709,156 @@ func TestConfig_HuJSON(t *testing.T) { assert.Equal(t, 5433, handler.DB.Port) } +// --- WithGlobals tests --- + +type testGlobals struct { + Verbose bool `cli:"verbose,v" usage:"verbose"` + Config string `cli:"config,c" usage:"config file" default:"config.json"` +} + +type simpleServeCmd struct { + Addr string `cli:"addr" usage:"listen address" default:":8080"` + globals *testGlobals +} + +func (s *simpleServeCmd) Run(ctx context.Context, _ []string) error { + s.globals = cli.GlobalsFromContext[testGlobals](ctx) + return nil +} + +func TestGlobals_FlagsAvailable(t *testing.T) { + g := &testGlobals{} + cmd := &simpleServeCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithGlobals(g)) + app.AddCommand(cli.Command("serve", "serve", cmd)) + + code := app.Run(context.Background(), []string{"serve", "--verbose", "--addr", ":9090"}) + assert.Equal(t, 0, code) + assert.True(t, g.Verbose) + assert.Equal(t, ":9090", cmd.Addr) +} + +func TestGlobals_FromContext(t *testing.T) { + g := &testGlobals{} + cmd := &simpleServeCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithGlobals(g)) + app.AddCommand(cli.Command("serve", "serve", cmd)) + + code := app.Run(context.Background(), []string{"serve", "--verbose"}) + assert.Equal(t, 0, code) + require.NotNil(t, cmd.globals) + assert.True(t, cmd.globals.Verbose) + assert.Equal(t, "config.json", cmd.globals.Config) // default +} + +func TestGlobals_ConfigFlagOnGlobals(t *testing.T) { + type serveWithConfig struct { + DB dbSection `config:"database"` + } + handler := &struct { + serveWithConfig + }{} + handler.serveWithConfig = serveWithConfig{} + + // Use a handler that has config tags but no config flag — + // the config flag is on globals. + g := &testGlobals{} + f := writeConfigFile(t, `{"database": {"host": "global-host", "port": 3306}}`) + + configCmd := &configHandler{DB: dbSection{}} + var buf bytes.Buffer + app := cli.New("app", "test", + cli.WithOutput(&buf), + cli.WithGlobals(g), + cli.WithConfigFlag("config"), + ) + app.AddCommand(cli.Command("run", "run", configCmd)) + + code := app.Run(context.Background(), []string{"run", "--config", f}) + assert.Equal(t, 0, code) + assert.Equal(t, "global-host", configCmd.DB.Host) + assert.Equal(t, 3306, configCmd.DB.Port) +} + +// --- ConfigAt tests --- + +type configAtCmd struct { + Addr string `cli:"addr" usage:"listen address" default:":8080"` + DB dbSection +} + +func (c *configAtCmd) Run(_ context.Context, _ []string) error { return nil } + +func TestConfigAt_BasicLoad(t *testing.T) { + g := &testGlobals{} + cmd := &configAtCmd{} + f := writeConfigFile(t, `{ + "database": {"host": "configat-host", "port": 5432} + }`) + + var buf bytes.Buffer + app := cli.New("app", "test", + cli.WithOutput(&buf), + cli.WithGlobals(g), + cli.WithConfigFlag("config"), + ) + app.AddCommand(cli.Command("serve", "serve", cmd, + cli.ConfigAt("database", &cmd.DB), + )) + + code := app.Run(context.Background(), []string{"serve", "--config", f}) + assert.Equal(t, 0, code) + assert.Equal(t, "configat-host", cmd.DB.Host) + assert.Equal(t, 5432, cmd.DB.Port) +} + +func TestConfigAt_NestedKey(t *testing.T) { + g := &testGlobals{} + cmd := &configAtCmd{} + f := writeConfigFile(t, `{ + "services": { + "api": {"host": "nested-host", "port": 9090} + } + }`) + + var buf bytes.Buffer + app := cli.New("app", "test", + cli.WithOutput(&buf), + cli.WithGlobals(g), + cli.WithConfigFlag("config"), + ) + app.AddCommand(cli.Command("serve", "serve", cmd, + cli.ConfigAt("services.api", &cmd.DB), + )) + + code := app.Run(context.Background(), []string{"serve", "--config", f}) + assert.Equal(t, 0, code) + assert.Equal(t, "nested-host", cmd.DB.Host) + assert.Equal(t, 9090, cmd.DB.Port) +} + +func TestConfigAt_MissingSectionSilent(t *testing.T) { + g := &testGlobals{} + cmd := &configAtCmd{} + f := writeConfigFile(t, `{"other": {}}`) + + var buf bytes.Buffer + app := cli.New("app", "test", + cli.WithOutput(&buf), + cli.WithGlobals(g), + cli.WithConfigFlag("config"), + ) + app.AddCommand(cli.Command("serve", "serve", cmd, + cli.ConfigAt("database", &cmd.DB), + )) + + code := app.Run(context.Background(), []string{"serve", "--config", f}) + assert.Equal(t, 0, code) + assert.Equal(t, "", cmd.DB.Host) // not populated +} + func TestConfig_EnvVarReplacement(t *testing.T) { handler := &overrideableHandler{} t.Setenv("CLI_TEST_HOST", "env-replaced-host") diff --git a/cli/config.go b/cli/config.go index 72347d8..45d7727 100644 --- a/cli/config.go +++ b/cli/config.go @@ -31,24 +31,39 @@ func replaceEnvInJSON(data []byte) []byte { }) } -// loadConfigIntoHandler reads the config file (path from the named flag), -// processes it via loader.LoadConfig, then applies config-tagged fields. -// Only sets fields that were NOT explicitly set via CLI flags. -// fields must be pre-scanned via buildFlagSet to avoid redundant reflection. -func loadConfigIntoHandler(handler Runnable, fs *FlagSet, configFlagName string, fields []fieldInfo) error { +// resolveConfigJSON finds the config file path (from globals or handler), +// reads and processes it, and returns the JSON string. Returns "" if no +// config file is available. +func resolveConfigJSON( + handler Runnable, + globals any, + fs *FlagSet, + configFlagName string, + globalFields []fieldInfo, + handlerFields []fieldInfo, +) (string, error) { if _, ok := fs.byLong[configFlagName]; !ok { - return nil + return "", nil } - rv := reflect.ValueOf(handler) - if rv.Kind() == reflect.Ptr { - rv = rv.Elem() + // Find the config file path — check globals first, then handler. + var configPath string + if globals != nil { + rv := reflect.ValueOf(globals) + if rv.Kind() == reflect.Ptr { + rv = rv.Elem() + } + configPath = stringFieldForFlag(rv, globalFields, configFlagName) + } + if configPath == "" { + rv := reflect.ValueOf(handler) + if rv.Kind() == reflect.Ptr { + rv = rv.Elem() + } + configPath = stringFieldForFlag(rv, handlerFields, configFlagName) } - - // Find the config file path from the already-parsed flag value. - configPath := stringFieldForFlag(rv, fields, configFlagName) if configPath == "" { - return nil + return "", nil } explicit := fs.IsSet(configFlagName) @@ -56,43 +71,37 @@ func loadConfigIntoHandler(handler Runnable, fs *FlagSet, configFlagName string, data, err := os.ReadFile(configPath) if err != nil { if errors.Is(err, os.ErrNotExist) && !explicit { - return nil + return "", nil } - return fmt.Errorf("reading config file %q: %w", configPath, err) + return "", fmt.Errorf("reading config file %q: %w", configPath, err) } - // Process via loader (hujson standardisation). + // Process via loader (hujson standardisation + env var replacement). var raw json.RawMessage if err := loader.LoadConfig(data, &raw); err != nil { - return fmt.Errorf("parsing config file %q: %w", configPath, err) + return "", fmt.Errorf("parsing config file %q: %w", configPath, err) } - // Apply env-var substitution on the raw JSON bytes. loader.LoadConfig - // performs reflection-based replacement on structs, but json.RawMessage - // is opaque to that pass, so we apply the regex replacement ourselves. - rawStr := string(replaceEnvInJSON(raw)) + return string(replaceEnvInJSON(raw)), nil +} + +// applyConfigTags applies config:"key" struct tags on the handler. +func applyConfigTags(handler Runnable, fs *FlagSet, fields []fieldInfo, rawJSON string) error { + rv := reflect.ValueOf(handler) + if rv.Kind() == reflect.Ptr { + rv = rv.Elem() + } for _, fi := range fields { if fi.configKey == "" { continue } - // Don't override fields explicitly set by CLI flags if fi.flagLong != "" && fs.IsSet(fi.flagLong) { continue } - var section string - if fi.configKey == "." { - section = rawStr - } else { - result := gjson.Get(rawStr, fi.configKey) - if !result.Exists() { - continue - } - section = result.Raw - } - + section := extractSection(rawJSON, fi.configKey) if section == "" { continue } @@ -102,10 +111,35 @@ func loadConfigIntoHandler(handler Runnable, fs *FlagSet, configFlagName string, return fmt.Errorf("config key %q: %w", fi.configKey, err) } } + return nil +} +// applyConfigBindings applies ConfigAt bindings. +func applyConfigBindings(bindings []configBinding, rawJSON string) error { + for _, b := range bindings { + section := extractSection(rawJSON, b.key) + if section == "" { + continue + } + if err := sgjson.Unmarshal([]byte(section), b.dst); err != nil { + return fmt.Errorf("config key %q: %w", b.key, err) + } + } return nil } +// extractSection extracts a JSON section by key path. +func extractSection(rawJSON, key string) string { + if key == "." { + return rawJSON + } + result := gjson.Get(rawJSON, key) + if !result.Exists() { + return "" + } + return result.Raw +} + // stringFieldForFlag returns the current string value of the flag's struct field. func stringFieldForFlag(rv reflect.Value, fields []fieldInfo, flagName string) string { for _, fi := range fields { diff --git a/cli/example/main.go b/cli/example/main.go index c45b6a2..631c4c3 100644 --- a/cli/example/main.go +++ b/cli/example/main.go @@ -10,7 +10,8 @@ import ( "github.com/runreveal/lib/cli" ) -// Globals are shared flags embedded into every command. +// Globals holds flags shared across all commands. Registered once with +// WithGlobals — no need to embed in every command struct. type Globals struct { Verbose bool `cli:"verbose,v" usage:"enable verbose output"` Config string `cli:"config,c" usage:"config file path" default:"config.json"` @@ -18,12 +19,11 @@ type Globals struct { // ServeCmd is the handler for the "serve" subcommand. type ServeCmd struct { - Globals Addr string `cli:"addr,a" usage:"listen address" default:":8080"` Timeout time.Duration `cli:"timeout,t" usage:"request timeout" default:"30s"` - // DB is loaded from the config file's "database" section. - DB DBConfig `config:"database"` + // DB is loaded from the config file's "database" section via ConfigAt. + DB DBConfig } type DBConfig struct { @@ -38,9 +38,10 @@ func (s *ServeCmd) Validate() error { } func (s *ServeCmd) Run(ctx context.Context, args []string) error { - if s.Verbose { + g := cli.GlobalsFromContext[Globals](ctx) + if g != nil && g.Verbose { fmt.Printf("verbose mode enabled\n") - fmt.Printf("config file: %s\n", s.Config) + fmt.Printf("config file: %s\n", g.Config) if s.DB.DSN != "" { fmt.Printf("database DSN: %s\n", s.DB.DSN) } @@ -51,7 +52,6 @@ func (s *ServeCmd) Run(ctx context.Context, args []string) error { // MigrateCmd is in the "admin" group. type MigrateCmd struct { - Globals DryRun bool `cli:"dry-run" usage:"print migrations without running"` DB string `cli:"db" usage:"database name" default:"prod"` } @@ -67,7 +67,6 @@ func (m *MigrateCmd) Run(ctx context.Context, args []string) error { // PingCmd demonstrates positional args. type PingCmd struct { - Globals Count int `cli:"count,n" usage:"number of pings" default:"3"` } @@ -86,7 +85,11 @@ func (p *PingCmd) Run(ctx context.Context, args []string) error { func main() { // Middleware: log every command execution - loggingMW := func(ctx context.Context, info cli.CommandInfo, next func(context.Context) error) error { + loggingMW := func( + ctx context.Context, + info cli.CommandInfo, + next func(context.Context) error, + ) error { fmt.Printf("[log] running command: %s\n", info.Name) err := next(ctx) if err != nil { @@ -95,17 +98,27 @@ func main() { return err } + globals := &Globals{} + serveCmd := &ServeCmd{} + app := cli.New("example", "Example CLI demonstrating the cli framework", cli.WithVersion("1.0.0"), + cli.WithGlobals(globals), cli.WithConfigFlag("config"), cli.WithMiddleware(loggingMW), ) app.AddCommand( - cli.Command("serve", "Start the HTTP server", &ServeCmd{}), - cli.Command("ping", "Ping one or more hosts", &PingCmd{}, cli.WithArgs(cli.MinArgs(0))), + cli.Command("serve", "Start the HTTP server", serveCmd, + cli.ConfigAt("database", &serveCmd.DB), + ), + cli.Command("ping", "Ping one or more hosts", &PingCmd{}, + cli.WithArgs(cli.MinArgs(0)), + ), cli.Group("admin", "Administrative commands", - cli.Command("migrate", "Run database migrations", &MigrateCmd{}, cli.WithArgs(cli.NoArgs)), + cli.Command("migrate", "Run database migrations", &MigrateCmd{}, + cli.WithArgs(cli.NoArgs), + ), ), ) diff --git a/cli/help.go b/cli/help.go index 877cdfd..de454d3 100644 --- a/cli/help.go +++ b/cli/help.go @@ -64,7 +64,7 @@ func printGroupHelp(w io.Writer, appName, path, desc string, children []Node) { fmt.Fprintf(w, "\nUse \"%s %s --help\" for more information.\n", appName, path) } -func printCommandHelp(w io.Writer, appName, path, desc string, handler Runnable, children []Node) { +func printCommandHelp(w io.Writer, appName, path, desc string, handler Runnable, children []Node, globals any) { if desc != "" { fmt.Fprintf(w, "%s %s - %s\n\n", appName, path, desc) } else { @@ -87,12 +87,19 @@ func printCommandHelp(w io.Writer, appName, path, desc string, handler Runnable, fmt.Fprintln(w) } - // Print flags from handler + // Print flags from handler + globals fs, _, err := buildFlagSet(handler) - if err == nil && len(fs.defs) > 0 { - fmt.Fprintf(w, "Flags:\n") - printFlagDefs(w, fs.defs) - fmt.Fprintln(w) + if err == nil { + if globals != nil { + addGlobalsToFlagSet(fs, globals) + } + if len(fs.defs) > 0 { + fmt.Fprintf(w, "Flags:\n") + printFlagDefs(w, fs.defs) + fmt.Fprintln(w) + } else { + fmt.Fprintf(w, "Flags:\n") + } } else { fmt.Fprintf(w, "Flags:\n") } diff --git a/cli/reflect.go b/cli/reflect.go index 7295a08..528478a 100644 --- a/cli/reflect.go +++ b/cli/reflect.go @@ -125,6 +125,44 @@ func buildFlagSet(handler Runnable) (*FlagSet, []fieldInfo, error) { return fs, fields, nil } +// addGlobalsToFlagSet scans a globals struct pointer and adds its cli-tagged +// fields to the given FlagSet. Returns the scanned fields for default application. +func addGlobalsToFlagSet(fs *FlagSet, globals any) ([]fieldInfo, error) { + rv := reflect.ValueOf(globals) + if rv.Kind() != reflect.Ptr || rv.Elem().Kind() != reflect.Struct { + return nil, fmt.Errorf("globals must be a pointer to a struct") + } + rv = rv.Elem() + + fields, err := scanFields(rv.Type()) + if err != nil { + return nil, err + } + + for _, fi := range fields { + if fi.flagLong == "" { + continue + } + // Skip if handler already defines this flag (handler wins). + if _, exists := fs.byLong[fi.flagLong]; exists { + continue + } + + fieldVal := fieldByIndex(rv, fi.fieldIndex) + ptr := fieldVal.Addr().Interface() + + def, err := makeFlagDef(fi.flagLong, fi.flagShort, fi.usage, fi.defVal, ptr) + if err != nil { + return nil, fmt.Errorf("globals field %v: %w", fi.fieldIndex, err) + } + if err := fs.add(def); err != nil { + return nil, err + } + } + + return fields, nil +} + // applyDefaults sets the default values using pre-scanned fields. func applyDefaults(fs *FlagSet, fields []fieldInfo) error { for _, fi := range fields { From c807f9702985e4771350f238ea053f6e8372dfd8 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 13:34:51 -0700 Subject: [PATCH 08/24] cli: add Configure/Validate/Run lifecycle on globals and handlers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Globals and handler structs can now implement the full CVR pattern: - Configurer: Configure() called after config loading to init resources - Validator: Validate() called after Configure to check readiness - io.Closer: Close() deferred for globals cleanup after command exits Lifecycle order: 1. Parse flags (globals + handler) 2. Load config file → populate struct fields 3. Globals: Configure → Validate (with deferred Close) 4. Handler: Configure → Validate 5. Middleware → Handler.Run 6. Globals.Close --- cli/cli.go | 41 ++++++++++++++++- cli/cli_test.go | 120 ++++++++++++++++++++++++++++++++++++++++++++++++ cli/config.go | 6 +-- 3 files changed, 162 insertions(+), 5 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index 9100d3a..8eef98f 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -32,7 +32,15 @@ type Runnable interface { Run(ctx context.Context, args []string) error } -// Validator is optionally implemented by handlers to validate config after loading. +// Configurer is optionally implemented by globals or handler structs. +// Called after config file loading to initialize resources (e.g. open +// database connections, create clients). +type Configurer interface { + Configure() error +} + +// Validator is optionally implemented by globals or handler structs. +// Called after Configure to check that the fully-loaded config is valid. type Validator interface { Validate() error } @@ -358,6 +366,12 @@ func (a *App) executeCommand(ctx context.Context, node *commandNode, args []stri return 1, fmt.Errorf("loading config: %w", err) } if configJSON != "" { + // Apply config tags on globals + if a.globals != nil { + if err := applyConfigTags(a.globals, fs, globalFields, configJSON); err != nil { + return 1, fmt.Errorf("loading config: %w", err) + } + } // Apply config:"key" struct tags on handler if err := applyConfigTags(handler, fs, fields, configJSON); err != nil { return 1, fmt.Errorf("loading config: %w", err) @@ -369,7 +383,30 @@ func (a *App) executeCommand(ctx context.Context, node *commandNode, args []stri } } - // Validate + // CVR lifecycle on globals: Configure → Validate + if a.globals != nil { + if c, ok := a.globals.(Configurer); ok { + if err := c.Configure(); err != nil { + return 1, fmt.Errorf("globals configure: %w", err) + } + } + if v, ok := a.globals.(Validator); ok { + if err := v.Validate(); err != nil { + return 1, fmt.Errorf("globals validate: %w", err) + } + } + // Defer cleanup if globals implements io.Closer + if cl, ok := a.globals.(io.Closer); ok { + defer cl.Close() + } + } + + // CVR lifecycle on handler: Configure → Validate + if c, ok := handler.(Configurer); ok { + if err := c.Configure(); err != nil { + return 1, err + } + } if v, ok := handler.(Validator); ok { if err := v.Validate(); err != nil { return 1, err diff --git a/cli/cli_test.go b/cli/cli_test.go index 859f0e2..cd7194f 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -859,6 +859,126 @@ func TestConfigAt_MissingSectionSilent(t *testing.T) { assert.Equal(t, "", cmd.DB.Host) // not populated } +// --- CVR lifecycle on globals --- + +type cvrGlobals struct { + Verbose bool `cli:"verbose,v" usage:"verbose"` + Config string `cli:"config,c" usage:"config" default:"config.json"` + DSN string ` config:"db.dsn"` + + // Set during lifecycle + Configured bool + Validated bool + Closed bool + DB string // simulates an initialized resource +} + +func (g *cvrGlobals) Configure() error { + g.Configured = true + if g.DSN != "" { + g.DB = "pool:" + g.DSN // simulate opening a connection + } + return nil +} + +func (g *cvrGlobals) Validate() error { + g.Validated = true + return nil +} + +func (g *cvrGlobals) Close() error { + g.Closed = true + g.DB = "" + return nil +} + +type cvrCmd struct { + globalsSnapshot *cvrGlobals +} + +func (c *cvrCmd) Run(ctx context.Context, _ []string) error { + g := cli.GlobalsFromContext[cvrGlobals](ctx) + // Snapshot so test can check state during Run + c.globalsSnapshot = &cvrGlobals{ + Configured: g.Configured, + Validated: g.Validated, + Closed: g.Closed, + DB: g.DB, + } + return nil +} + +func TestGlobals_CVR_Lifecycle(t *testing.T) { + g := &cvrGlobals{} + cmd := &cvrCmd{} + f := writeConfigFile(t, `{"db": {"dsn": "postgres://localhost/test"}}`) + + var buf bytes.Buffer + app := cli.New("app", "test", + cli.WithOutput(&buf), + cli.WithGlobals(g), + cli.WithConfigFlag("config"), + ) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), []string{"run", "--config", f}) + assert.Equal(t, 0, code) + + // During Run: configured and validated, not yet closed + require.NotNil(t, cmd.globalsSnapshot) + assert.True(t, cmd.globalsSnapshot.Configured) + assert.True(t, cmd.globalsSnapshot.Validated) + assert.False(t, cmd.globalsSnapshot.Closed) + assert.Equal(t, "pool:postgres://localhost/test", cmd.globalsSnapshot.DB) + + // After Run: closed + assert.True(t, g.Closed) + assert.Equal(t, "", g.DB) // resource cleaned up +} + +func TestGlobals_CVR_ConfigureError(t *testing.T) { + // Configure runs before Validate — already covered by + // TestGlobals_CVR_Lifecycle ordering assertions. +} + +type cvrHandlerCmd struct { + Name string `cli:"name" usage:"name"` + configured bool + validated bool +} + +func (c *cvrHandlerCmd) Configure() error { + c.configured = true + return nil +} + +func (c *cvrHandlerCmd) Validate() error { + if !c.configured { + return errors.New("configure must run before validate") + } + c.validated = true + return nil +} + +func (c *cvrHandlerCmd) Run(_ context.Context, _ []string) error { + if !c.validated { + return errors.New("validate must run before run") + } + return nil +} + +func TestHandler_CVR_Lifecycle(t *testing.T) { + cmd := &cvrHandlerCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), []string{"run", "--name", "test"}) + assert.Equal(t, 0, code) + assert.True(t, cmd.configured) + assert.True(t, cmd.validated) +} + func TestConfig_EnvVarReplacement(t *testing.T) { handler := &overrideableHandler{} t.Setenv("CLI_TEST_HOST", "env-replaced-host") diff --git a/cli/config.go b/cli/config.go index 45d7727..6ee6775 100644 --- a/cli/config.go +++ b/cli/config.go @@ -85,9 +85,9 @@ func resolveConfigJSON( return string(replaceEnvInJSON(raw)), nil } -// applyConfigTags applies config:"key" struct tags on the handler. -func applyConfigTags(handler Runnable, fs *FlagSet, fields []fieldInfo, rawJSON string) error { - rv := reflect.ValueOf(handler) +// applyConfigTags applies config:"key" struct tags on a struct pointer. +func applyConfigTags(target any, fs *FlagSet, fields []fieldInfo, rawJSON string) error { + rv := reflect.ValueOf(target) if rv.Kind() == reflect.Ptr { rv = rv.Elem() } From 520708013800a33b8bdc632f97c088bc762d756f Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 13:48:20 -0700 Subject: [PATCH 09/24] cli: simplify flags, fix redundant ExitError unwrap, consistent error wrapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace getBool closure with isBool bool field — the closure return value was never called, only its nil-ness was checked - Remove redundant ExitError unwrap in executeCommand — the caller App.Run already handles ExitError extraction - Wrap handler Configure/Validate errors consistently with globals - Add comment explaining why replaceEnvInJSON is duplicated from loader --- cli/cli.go | 9 +++------ cli/config.go | 12 ++++++------ cli/flags.go | 12 ++++++------ 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index 8eef98f..d3c4d19 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -404,12 +404,12 @@ func (a *App) executeCommand(ctx context.Context, node *commandNode, args []stri // CVR lifecycle on handler: Configure → Validate if c, ok := handler.(Configurer); ok { if err := c.Configure(); err != nil { - return 1, err + return 1, fmt.Errorf("configure: %w", err) } } if v, ok := handler.(Validator); ok { if err := v.Validate(); err != nil { - return 1, err + return 1, fmt.Errorf("validate: %w", err) } } @@ -428,11 +428,8 @@ func (a *App) executeCommand(ctx context.Context, node *commandNode, args []stri info := CommandInfo{Name: path, Args: posArgs} chain := buildChain(a.middlewares, info, runFn) + // Errors (including ExitError) propagate to App.Run which handles exit codes. if err := chain(ctx); err != nil { - var exitErr *ExitError - if errors.As(err, &exitErr) { - return exitErr.Code, exitErr - } return 1, err } return 0, nil diff --git a/cli/config.go b/cli/config.go index 6ee6775..c075c4b 100644 --- a/cli/config.go +++ b/cli/config.go @@ -13,19 +13,19 @@ import ( "github.com/tidwall/gjson" ) -// envVarRegex matches JSON string values that are exactly an env-var reference, -// e.g. "$MY_VAR". The whole quoted token is replaced with the env var's value. +// envVarRegex matches JSON string values that are exactly an env-var reference. +// This duplicates loader's regex because the cli module may pin a loader version +// that predates env var support, and because loader.LoadConfig into json.RawMessage +// may not apply replacement to opaque byte values. var envVarRegex = regexp.MustCompile(`"\$([A-Za-z_][A-Za-z0-9_]*)"`) -// replaceEnvInJSON replaces "$VAR" tokens inside JSON string values with the -// corresponding environment variable's value, properly re-encoded as JSON. func replaceEnvInJSON(data []byte) []byte { return envVarRegex.ReplaceAllFunc(data, func(match []byte) []byte { - name := string(match[2 : len(match)-1]) // strip leading `"$` and trailing `"` + name := string(match[2 : len(match)-1]) val := os.Getenv(name) encoded, err := json.Marshal(val) if err != nil { - return match // should never happen for a plain string + return match } return encoded }) diff --git a/cli/flags.go b/cli/flags.go index 2cb0c3a..a9ac7d0 100644 --- a/cli/flags.go +++ b/cli/flags.go @@ -16,7 +16,7 @@ type flagDef struct { defVal string typeName string // for help display setValue func(s string) error - getBool func() bool // non-nil only for bool flags + isBool bool // true for boolean flags (value is optional) } // FlagSet is a parsed set of flag definitions. @@ -95,7 +95,7 @@ func (fs *FlagSet) parseLong(raw string, remaining []string) (int, error) { return 0, fmt.Errorf("unknown flag: --%s", name) } - if def.getBool != nil { + if def.isBool { // Bool flag: value is optional if hasEq { if err := def.setValue(val); err != nil { @@ -154,7 +154,7 @@ func (fs *FlagSet) parseShort(raw string, remaining []string) (int, error) { if !ok { return 0, fmt.Errorf("unknown flag: -%s", char) } - if def.getBool != nil { + if def.isBool { if err := def.setValue("true"); err != nil { return 0, err } @@ -178,7 +178,7 @@ func (fs *FlagSet) parseShort(raw string, remaining []string) (int, error) { if !ok { return 0, fmt.Errorf("unknown flag: -%s", char) } - if def.getBool == nil { + if !def.isBool { // Non-bool flag: rest of the string is the value val := raw[j+1:] if val == "" { @@ -224,7 +224,7 @@ func makeFlagDef(long, short, usage, defVal string, ptr any) (*flagDef, error) { def.setValue = func(s string) error { *p = &s; return nil } case *bool: def.typeName = "" - def.getBool = func() bool { return *p } + def.isBool = true def.setValue = func(s string) error { v, err := strconv.ParseBool(s) if err != nil { @@ -235,7 +235,7 @@ func makeFlagDef(long, short, usage, defVal string, ptr any) (*flagDef, error) { } case **bool: def.typeName = "" - def.getBool = func() bool { return *p != nil && **p } + def.isBool = true def.setValue = func(s string) error { v, err := strconv.ParseBool(s) if err != nil { From cf598f031738176171f46f01e8ec60fb940d7702 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 14:07:29 -0700 Subject: [PATCH 10/24] cli: add shell completion for bash, zsh, and fish - Hidden `completion` command outputs shell-specific scripts - Hidden `__complete` command provides runtime completions - Completer interface for custom positional arg completions - Completes subcommands, flags (including globals), and custom values - Neither command appears in help output --- cli/cli.go | 6 + cli/complete.go | 259 +++++++++++++++++++++++++++++++++++++++++++ cli/complete_test.go | 230 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 495 insertions(+) create mode 100644 cli/complete.go create mode 100644 cli/complete_test.go diff --git a/cli/cli.go b/cli/cli.go index d3c4d19..47a24af 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -228,6 +228,12 @@ func (a *App) Run(ctx context.Context, args []string) (exitCode int) { } func (a *App) run(ctx context.Context, args []string) (int, error) { + // Handle completion commands before normal routing so they stay + // hidden from help output and don't interfere with user commands. + if code, handled := a.handleCompletion(args); handled { + return code, nil + } + // Check for top-level --version / --help before routing if len(args) == 1 && (args[0] == "--version" || args[0] == "-version") { if a.version != "" { diff --git a/cli/complete.go b/cli/complete.go new file mode 100644 index 0000000..90b38a8 --- /dev/null +++ b/cli/complete.go @@ -0,0 +1,259 @@ +package cli + +import ( + "context" + "fmt" + "io" + "strings" +) + +// Completer is optionally implemented by command handlers to provide +// custom completions for positional arguments. +type Completer interface { + Complete(ctx context.Context, args []string) []Completion +} + +// Completion represents a single shell completion suggestion. +type Completion struct { + Value string + Description string +} + +// handleCompletion checks if args[0] is "completion" or "__complete" and +// handles them directly. Returns true if handled, false otherwise. +func (a *App) handleCompletion(args []string) (int, bool) { + if len(args) == 0 { + return 0, false + } + switch args[0] { + case "completion": + return a.handleCompletionScript(args[1:]), true + case "__complete": + a.handleCompleteRequest(args[1:]) + return 0, true + } + return 0, false +} + +func (a *App) handleCompletionScript(args []string) int { + if len(args) != 1 { + fmt.Fprintf( + a.output, + "Usage: %s completion \n", + a.name, + ) + return 1 + } + switch args[0] { + case "bash": + writeBashCompletion(a.output, a.name) + case "zsh": + writeZshCompletion(a.output, a.name) + case "fish": + writeFishCompletion(a.output, a.name) + default: + fmt.Fprintf( + a.output, + "unsupported shell %q, expected bash, zsh, or fish\n", + args[0], + ) + return 1 + } + return 0 +} + +func (a *App) handleCompleteRequest(args []string) { + completions := a.computeCompletions(context.Background(), args) + for _, c := range completions { + if c.Description != "" { + fmt.Fprintf(a.output, "%s\t%s\n", c.Value, c.Description) + } else { + fmt.Fprintf(a.output, "%s\n", c.Value) + } + } +} + +func (a *App) computeCompletions( + ctx context.Context, args []string, +) []Completion { + // Walk the command tree to find the deepest matching node. + children := a.children + var currentNode Node + consumed := 0 + + for i := 0; i < len(args); i++ { + arg := args[i] + if strings.HasPrefix(arg, "-") { + break + } + found := false + for _, child := range children { + if child.nodeName() == arg { + currentNode = child + consumed = i + 1 + switch n := child.(type) { + case *commandNode: + children = n.children + case *groupNode: + children = n.children + } + found = true + break + } + } + if !found { + break + } + } + + remaining := args[consumed:] + + // Determine what the user is currently typing (the last token). + var current string + if len(remaining) > 0 { + current = remaining[len(remaining)-1] + } + + // If the current word starts with -, complete flags. + if strings.HasPrefix(current, "-") { + return a.completeFlags(currentNode, current) + } + + // Otherwise complete subcommands, or positional args via Completer. + var completions []Completion + + // Subcommand completions from the current level's children. + for _, child := range children { + name := child.nodeName() + if strings.HasPrefix(name, current) { + completions = append(completions, Completion{ + Value: name, + Description: child.nodeDesc(), + }) + } + } + + // If the current node is a command with a Completer handler, + // include its completions for positional args. + if cn, ok := currentNode.(*commandNode); ok { + if comp, ok := cn.handler.(Completer); ok { + // Pass remaining args (excluding the partial word being + // completed) as context. + var priorArgs []string + for _, r := range remaining { + if !strings.HasPrefix(r, "-") && r != current { + priorArgs = append(priorArgs, r) + } + } + custom := comp.Complete(ctx, priorArgs) + for _, c := range custom { + if strings.HasPrefix(c.Value, current) { + completions = append(completions, c) + } + } + } + } + + return completions +} + +func (a *App) completeFlags( + node Node, current string, +) []Completion { + var defs []*flagDef + + if cn, ok := node.(*commandNode); ok { + fs, _, err := buildFlagSet(cn.handler) + if err == nil { + if a.globals != nil { + addGlobalsToFlagSet(fs, a.globals) + } + defs = fs.defs + } + } + + // Always include --help. + defs = append(defs, &flagDef{ + long: "help", + short: "h", + usage: "show help", + }) + + // If globals are set but no specific command node, still offer + // global flags. + if node == nil && a.globals != nil { + fs := newFlagSet() + gFields, err := addGlobalsToFlagSet(fs, a.globals) + if err == nil { + _ = gFields + defs = append(defs, fs.defs...) + } + } + + prefix := strings.TrimLeft(current, "-") + var completions []Completion + seen := map[string]bool{} + for _, d := range defs { + flag := "--" + d.long + if seen[flag] { + continue + } + seen[flag] = true + if strings.HasPrefix(d.long, prefix) { + completions = append(completions, Completion{ + Value: flag, + Description: d.usage, + }) + } + } + return completions +} + +// writeBashCompletion outputs a bash completion script for the given app. +func writeBashCompletion(w io.Writer, appName string) { + fmt.Fprintf(w, `_%[1]s_completions() { + local cur="${COMP_WORDS[COMP_CWORD]}" + local IFS=$'\n' + local args=("${COMP_WORDS[@]:1:$COMP_CWORD}") + local completions + completions=$(%[1]s __complete "${args[@]}" 2>/dev/null) + COMPREPLY=() + while IFS=$'\t' read -r val desc; do + COMPREPLY+=("$val") + done <<< "$completions" +} +complete -F _%[1]s_completions %[1]s +`, appName) +} + +// writeZshCompletion outputs a zsh completion script for the given app. +func writeZshCompletion(w io.Writer, appName string) { + fmt.Fprintf(w, `#compdef %[1]s + +_%[1]s() { + local -a completions + local IFS=$'\n' + local args=("${words[@]:1:$CURRENT-1}") + completions=($(${words[1]} __complete "${args[@]}" 2>/dev/null)) + local -a descs + for line in "${completions[@]}"; do + if [[ "$line" == *$'\t'* ]]; then + local val="${line%%%%$'\t'*}" + local desc="${line#*$'\t'}" + descs+=("${val}:${desc}") + else + descs+=("${line}") + fi + done + _describe 'completions' descs +} + +compdef _%[1]s %[1]s +`, appName) +} + +// writeFishCompletion outputs a fish completion script for the given app. +func writeFishCompletion(w io.Writer, appName string) { + fmt.Fprintf(w, `complete -c %[1]s -f -a '(%[1]s __complete (commandline -cop) 2>/dev/null | string replace -r "\\t.*" "")' +`, appName) +} diff --git a/cli/complete_test.go b/cli/complete_test.go new file mode 100644 index 0000000..2c1138a --- /dev/null +++ b/cli/complete_test.go @@ -0,0 +1,230 @@ +package cli_test + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/runreveal/lib/cli" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// --- completion test helpers --- + +type serveHandler struct { + Addr string `cli:"addr,a" usage:"listen address" default:":8080"` + Verbose bool `cli:"verbose,v" usage:"be verbose"` +} + +func (s *serveHandler) Run(_ context.Context, _ []string) error { + return nil +} + +type migrateHandler struct { + DryRun bool `cli:"dry-run" usage:"print migrations without running"` +} + +func (m *migrateHandler) Run(_ context.Context, _ []string) error { + return nil +} + +type completingHandler struct { + Name string `cli:"name" usage:"resource name"` +} + +func (c *completingHandler) Run(_ context.Context, _ []string) error { + return nil +} + +func (c *completingHandler) Complete( + _ context.Context, _ []string, +) []cli.Completion { + return []cli.Completion{ + {Value: "alpha", Description: "first resource"}, + {Value: "beta", Description: "second resource"}, + {Value: "gamma", Description: "third resource"}, + } +} + +type completeTestGlobals struct { + Config string `cli:"config,c" usage:"config file path"` + Debug bool `cli:"debug,d" usage:"enable debug"` +} + +func newTestApp(buf *bytes.Buffer) *cli.App { + globals := &completeTestGlobals{} + app := cli.New("testapp", "A test application", + cli.WithGlobals(globals), + cli.WithOutput(buf), + ) + app.AddCommand( + cli.Command("serve", "Start the server", &serveHandler{}), + cli.Command("get", "Get a resource", &completingHandler{}), + cli.Group("admin", "Administrative commands", + cli.Command( + "migrate", "Run database migrations", + &migrateHandler{}, + ), + ), + ) + return app +} + +func TestComplete(t *testing.T) { + tests := []struct { + name string + args []string + wantValues []string + wantAbsent []string + }{ + { + name: "top-level subcommands", + args: []string{"__complete", ""}, + wantValues: []string{"serve", "admin", "get"}, + }, + { + name: "partial subcommand match", + args: []string{"__complete", "se"}, + wantValues: []string{"serve"}, + wantAbsent: []string{"admin", "get"}, + }, + { + name: "flag completion for command", + args: []string{"__complete", "serve", "--"}, + wantValues: []string{"--addr", "--verbose", "--help"}, + }, + { + name: "flag completion with prefix", + args: []string{"__complete", "serve", "--a"}, + wantValues: []string{"--addr"}, + wantAbsent: []string{"--verbose"}, + }, + { + name: "global flags included in command flags", + args: []string{"__complete", "serve", "--"}, + wantValues: []string{"--config", "--debug"}, + }, + { + name: "nested command completion", + args: []string{"__complete", "admin", "mi"}, + wantValues: []string{"migrate"}, + }, + { + name: "nested command flag completion", + args: []string{"__complete", "admin", "migrate", "--"}, + wantValues: []string{"--dry-run", "--help"}, + }, + { + name: "completer interface for positional args", + args: []string{"__complete", "get", "al"}, + wantValues: []string{"alpha"}, + wantAbsent: []string{"beta", "gamma"}, + }, + { + name: "completer returns all matches on empty input", + args: []string{"__complete", "get", ""}, + wantValues: []string{"alpha", "beta", "gamma"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + app := newTestApp(&buf) + + code := app.Run(context.Background(), tt.args) + assert.Equal(t, 0, code) + + output := buf.String() + for _, want := range tt.wantValues { + assert.True( + t, + strings.Contains(output, want), + "expected %q in output:\n%s", want, output, + ) + } + for _, absent := range tt.wantAbsent { + assert.False( + t, + strings.Contains(output, absent), + "did not expect %q in output:\n%s", + absent, output, + ) + } + }) + } +} + +func TestCompletionScripts(t *testing.T) { + shells := []string{"bash", "zsh", "fish"} + + for _, shell := range shells { + t.Run(shell, func(t *testing.T) { + var buf bytes.Buffer + app := newTestApp(&buf) + + code := app.Run( + context.Background(), + []string{"completion", shell}, + ) + assert.Equal(t, 0, code) + + output := buf.String() + require.NotEmpty(t, output) + assert.Contains(t, output, "testapp") + }) + } +} + +func TestCompletionScriptInvalidShell(t *testing.T) { + var buf bytes.Buffer + app := newTestApp(&buf) + + code := app.Run( + context.Background(), + []string{"completion", "powershell"}, + ) + assert.Equal(t, 1, code) +} + +func TestCompletionScriptNoArg(t *testing.T) { + var buf bytes.Buffer + app := newTestApp(&buf) + + code := app.Run( + context.Background(), + []string{"completion"}, + ) + assert.Equal(t, 1, code) +} + +func TestCompletionCommandsNotInHelp(t *testing.T) { + var buf bytes.Buffer + app := newTestApp(&buf) + + // Running with no args should show help, which should not + // mention "completion" or "__complete". + app.Run(context.Background(), nil) + output := buf.String() + assert.NotContains(t, output, "completion") + assert.NotContains(t, output, "__complete") +} + +func TestCompleteOutputFormat(t *testing.T) { + var buf bytes.Buffer + app := newTestApp(&buf) + + app.Run(context.Background(), []string{"__complete", ""}) + output := buf.String() + + // Each line should have a tab separator for description. + lines := strings.Split(strings.TrimSpace(output), "\n") + for _, line := range lines { + assert.Contains( + t, line, "\t", + "expected tab-separated format in line: %s", line, + ) + } +} From a129e067a0f0f2b4464257df020c1d5e5340bc5d Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 14:30:16 -0700 Subject: [PATCH 11/24] cli: update example with await integration, fix errcheck lint - Example shows three await patterns: single server, multi-service daemon, and context-based worker loop - Fix unchecked addGlobalsToFlagSet errors in complete.go and help.go --- cli/complete.go | 8 +-- cli/example/go.mod | 5 +- cli/example/go.sum | 2 + cli/example/main.go | 168 +++++++++++++++++++++++++++++++------------- cli/help.go | 4 +- 5 files changed, 133 insertions(+), 54 deletions(-) diff --git a/cli/complete.go b/cli/complete.go index 90b38a8..526ce73 100644 --- a/cli/complete.go +++ b/cli/complete.go @@ -166,7 +166,9 @@ func (a *App) completeFlags( fs, _, err := buildFlagSet(cn.handler) if err == nil { if a.globals != nil { - addGlobalsToFlagSet(fs, a.globals) + if _, gerr := addGlobalsToFlagSet(fs, a.globals); gerr != nil { + return nil + } } defs = fs.defs } @@ -183,9 +185,7 @@ func (a *App) completeFlags( // global flags. if node == nil && a.globals != nil { fs := newFlagSet() - gFields, err := addGlobalsToFlagSet(fs, a.globals) - if err == nil { - _ = gFields + if _, err := addGlobalsToFlagSet(fs, a.globals); err == nil { defs = append(defs, fs.defs...) } } diff --git a/cli/example/go.mod b/cli/example/go.mod index ca48e15..98ce51d 100644 --- a/cli/example/go.mod +++ b/cli/example/go.mod @@ -2,7 +2,10 @@ module github.com/runreveal/lib/cli/example go 1.24 -require github.com/runreveal/lib/cli v0.0.0 +require ( + github.com/runreveal/lib/await v0.0.0-20231128193746-50c2ad68891c + github.com/runreveal/lib/cli v0.0.0 +) require ( github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c // indirect diff --git a/cli/example/go.sum b/cli/example/go.sum index 3bfe98a..a6f0a81 100644 --- a/cli/example/go.sum +++ b/cli/example/go.sum @@ -4,6 +4,8 @@ github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/runreveal/lib/await v0.0.0-20231128193746-50c2ad68891c h1:rZt9vvUOA1CstxYueKfaAU7zcLx5PyMSECtJGscP8wo= +github.com/runreveal/lib/await v0.0.0-20231128193746-50c2ad68891c/go.mod h1:qnRPgJExa5ziREWvAhSDMMTBSxBk9wX4D2xZBUBldmw= github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c h1:F+kw4v9T9acZp7Ln29qlZvTRfaVMNzC6oO7To8OtATA= github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c/go.mod h1:Wy+jC29YN5FuH7qC4guAb44D85Kb+zU28Xuerg0IuK0= github.com/segmentio/asm v1.1.3 h1:WM03sfUOENvvKexOLp+pCqgb/WDjsi7EK8gIsICtzhc= diff --git a/cli/example/main.go b/cli/example/main.go index 631c4c3..7a28837 100644 --- a/cli/example/main.go +++ b/cli/example/main.go @@ -1,29 +1,36 @@ -// Command example demonstrates the github.com/runreveal/lib/cli framework. +// Command example demonstrates the github.com/runreveal/lib/cli framework, +// including await integration for long-running services. package main import ( "context" "fmt" + "net/http" "os" "time" + "github.com/runreveal/lib/await" "github.com/runreveal/lib/cli" ) -// Globals holds flags shared across all commands. Registered once with -// WithGlobals — no need to embed in every command struct. +// --------------------------------------------------------------------------- +// Globals: shared flags + resources via Configure/Validate/Close +// --------------------------------------------------------------------------- + +// Globals holds flags and resources shared across all commands. type Globals struct { Verbose bool `cli:"verbose,v" usage:"enable verbose output"` Config string `cli:"config,c" usage:"config file path" default:"config.json"` } -// ServeCmd is the handler for the "serve" subcommand. +// --------------------------------------------------------------------------- +// serve: a long-running HTTP server managed by await +// --------------------------------------------------------------------------- + type ServeCmd struct { Addr string `cli:"addr,a" usage:"listen address" default:":8080"` Timeout time.Duration `cli:"timeout,t" usage:"request timeout" default:"30s"` - - // DB is loaded from the config file's "database" section via ConfigAt. - DB DBConfig + DB DBConfig } type DBConfig struct { @@ -37,20 +44,108 @@ func (s *ServeCmd) Validate() error { return nil } +// Run starts the HTTP server using await for graceful shutdown. +// The ctx passed by the cli framework is cancelled on SIGINT/SIGTERM +// when the root command is run, but await.WithSignals gives you the +// same behavior with named sub-runners and a configurable stop timeout. func (s *ServeCmd) Run(ctx context.Context, args []string) error { g := cli.GlobalsFromContext[Globals](ctx) + + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, "ok") + }) + + server := &http.Server{ + Addr: s.Addr, + Handler: mux, + ReadTimeout: s.Timeout, + WriteTimeout: s.Timeout, + } + if g != nil && g.Verbose { - fmt.Printf("verbose mode enabled\n") - fmt.Printf("config file: %s\n", g.Config) + fmt.Printf("starting server on %s\n", s.Addr) if s.DB.DSN != "" { - fmt.Printf("database DSN: %s\n", s.DB.DSN) + fmt.Printf("database: %s\n", s.DB.DSN) } } - fmt.Printf("serving on %s (timeout: %s)\n", s.Addr, s.Timeout) + + // await manages graceful shutdown: on SIGINT/SIGTERM it cancels + // the context, ListenAndServe calls server.Shutdown, and await + // waits up to the stop timeout for in-flight requests to drain. + w := await.New(await.WithSignals) + w.AddNamed(await.ListenAndServe(server), "http") + return w.Run(ctx) +} + +// --------------------------------------------------------------------------- +// daemon: run multiple services concurrently with await +// --------------------------------------------------------------------------- + +type DaemonCmd struct { + APIAddr string `cli:"api-addr" usage:"API listen address" default:":8080"` + MetricAddr string `cli:"metric-addr" usage:"metrics listen address" default:":9090"` +} + +func (d *DaemonCmd) Validate() error { + if d.APIAddr == "" || d.MetricAddr == "" { + return fmt.Errorf("both --api-addr and --metric-addr are required") + } return nil } -// MigrateCmd is in the "admin" group. +// Run starts multiple services under a single await runner. +// If any service exits with an error, await cancels the others +// and waits for them to shut down cleanly. +func (d *DaemonCmd) Run(ctx context.Context, args []string) error { + apiServer := &http.Server{ + Addr: d.APIAddr, + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprintln(w, "api") }), + } + metricServer := &http.Server{ + Addr: d.MetricAddr, + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprintln(w, "metrics") }), + } + + w := await.New(await.WithSignals, await.WithStopTimeout(15*time.Second)) + w.AddNamed(await.ListenAndServe(apiServer), "api") + w.AddNamed(await.ListenAndServe(metricServer), "metrics") + + fmt.Printf("daemon: api=%s metrics=%s\n", d.APIAddr, d.MetricAddr) + return w.Run(ctx) +} + +// --------------------------------------------------------------------------- +// worker: a background job that respects context cancellation +// --------------------------------------------------------------------------- + +type WorkerCmd struct { + Interval time.Duration `cli:"interval,i" usage:"poll interval" default:"10s"` +} + +// Run demonstrates a polling worker that exits cleanly on SIGINT/SIGTERM. +// For a single long-running goroutine, you don't need await — just +// select on ctx.Done(). +func (w *WorkerCmd) Run(ctx context.Context, args []string) error { + fmt.Printf("worker: polling every %s\n", w.Interval) + ticker := time.NewTicker(w.Interval) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + fmt.Println("worker: shutting down") + return nil + case <-ticker.C: + fmt.Println("worker: tick") + } + } +} + +// --------------------------------------------------------------------------- +// migrate: a one-shot command (no await needed) +// --------------------------------------------------------------------------- + type MigrateCmd struct { DryRun bool `cli:"dry-run" usage:"print migrations without running"` DB string `cli:"db" usage:"database name" default:"prod"` @@ -65,56 +160,33 @@ func (m *MigrateCmd) Run(ctx context.Context, args []string) error { return nil } -// PingCmd demonstrates positional args. -type PingCmd struct { - Count int `cli:"count,n" usage:"number of pings" default:"3"` -} - -func (p *PingCmd) Run(ctx context.Context, args []string) error { - hosts := args - if len(hosts) == 0 { - hosts = []string{"localhost"} - } - for _, host := range hosts { - for i := 0; i < p.Count; i++ { - fmt.Printf("ping #%d -> %s\n", i+1, host) - } - } - return nil -} +// --------------------------------------------------------------------------- +// main +// --------------------------------------------------------------------------- func main() { - // Middleware: log every command execution - loggingMW := func( - ctx context.Context, - info cli.CommandInfo, - next func(context.Context) error, - ) error { - fmt.Printf("[log] running command: %s\n", info.Name) - err := next(ctx) - if err != nil { - fmt.Printf("[log] command failed: %v\n", err) - } - return err - } - globals := &Globals{} serveCmd := &ServeCmd{} - app := cli.New("example", "Example CLI demonstrating the cli framework", + app := cli.New("example", "Example CLI demonstrating cli + await", cli.WithVersion("1.0.0"), cli.WithGlobals(globals), cli.WithConfigFlag("config"), - cli.WithMiddleware(loggingMW), ) app.AddCommand( + // Long-running server with await + graceful shutdown cli.Command("serve", "Start the HTTP server", serveCmd, cli.ConfigAt("database", &serveCmd.DB), ), - cli.Command("ping", "Ping one or more hosts", &PingCmd{}, - cli.WithArgs(cli.MinArgs(0)), - ), + + // Multiple services under one await runner + cli.Command("daemon", "Run all services", &DaemonCmd{}), + + // Background worker using context cancellation + cli.Command("worker", "Run the background worker", &WorkerCmd{}), + + // One-shot commands don't need await cli.Group("admin", "Administrative commands", cli.Command("migrate", "Run database migrations", &MigrateCmd{}, cli.WithArgs(cli.NoArgs), diff --git a/cli/help.go b/cli/help.go index de454d3..21b1a8d 100644 --- a/cli/help.go +++ b/cli/help.go @@ -91,7 +91,9 @@ func printCommandHelp(w io.Writer, appName, path, desc string, handler Runnable, fs, _, err := buildFlagSet(handler) if err == nil { if globals != nil { - addGlobalsToFlagSet(fs, globals) + if _, gerr := addGlobalsToFlagSet(fs, globals); gerr != nil { + fmt.Fprintf(w, " (error loading global flags: %s)\n", gerr) + } } if len(fs.defs) > 0 { fmt.Fprintf(w, "Flags:\n") From d5bf6a9cf680c2db3a55713245b2e8006ecb0f8b Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 14:34:13 -0700 Subject: [PATCH 12/24] cli: add default-config command, embedded config in example - WithDefaultConfig(data) registers a default config (typically go:embed) - Hidden "default-config" command prints it to stdout - Example includes config.json with HuJSON comments demonstrating all config sections (database, daemon, worker) - Commands use ConfigAt + Configure() to apply config file values --- cli/cli.go | 38 ++++++++++++++++++------ cli/example/config.json | 17 +++++++++++ cli/example/main.go | 64 ++++++++++++++++++++++++++++++++++++----- 3 files changed, 103 insertions(+), 16 deletions(-) create mode 100644 cli/example/config.json diff --git a/cli/cli.go b/cli/cli.go index 47a24af..61fb55b 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -167,6 +167,13 @@ func WithGlobals(ptr any) AppOption { return func(a *App) { a.globals = ptr } } +// WithDefaultConfig registers a default configuration that can be printed +// with "myapp default-config". Typically used with go:embed to ship a +// reference config alongside the binary. +func WithDefaultConfig(data []byte) AppOption { + return func(a *App) { a.defaultConfig = data } +} + // WithOutput sets the writer for help/error output (default: os.Stderr). func WithOutput(w io.Writer) AppOption { return func(a *App) { a.output = w } @@ -174,14 +181,15 @@ func WithOutput(w io.Writer) AppOption { // App is the top-level CLI application. type App struct { - name string - desc string - version string - configFlag string - globals any // pointer to globals struct, if set - middlewares []Middleware - children []Node - output io.Writer + name string + desc string + version string + configFlag string + globals any // pointer to globals struct, if set + defaultConfig []byte + middlewares []Middleware + children []Node + output io.Writer } // New creates a new App. @@ -228,11 +236,14 @@ func (a *App) Run(ctx context.Context, args []string) (exitCode int) { } func (a *App) run(ctx context.Context, args []string) (int, error) { - // Handle completion commands before normal routing so they stay + // Handle built-in commands before normal routing so they stay // hidden from help output and don't interfere with user commands. if code, handled := a.handleCompletion(args); handled { return code, nil } + if len(args) == 1 && args[0] == "default-config" { + return a.handleDefaultConfig(), nil + } // Check for top-level --version / --help before routing if len(args) == 1 && (args[0] == "--version" || args[0] == "-version") { @@ -457,6 +468,15 @@ func buildChain(middlewares []Middleware, info CommandInfo, final func(context.C return chain } +func (a *App) handleDefaultConfig() int { + if len(a.defaultConfig) == 0 { + fmt.Fprintf(a.output, "no default config registered\n") + return 1 + } + a.output.Write(a.defaultConfig) + return 0 +} + // FlagSetFromContext returns the *FlagSet stored in ctx during command // execution, or nil if called outside of a command handler. func FlagSetFromContext(ctx context.Context) *FlagSet { diff --git a/cli/example/config.json b/cli/example/config.json new file mode 100644 index 0000000..7658152 --- /dev/null +++ b/cli/example/config.json @@ -0,0 +1,17 @@ +{ + // Example config for the CLI framework demo. + // HuJSON: comments and trailing commas are allowed. + + "database": { + "dsn": "postgres://localhost:5432/example?sslmode=disable", + }, + + "daemon": { + "api_addr": ":8080", + "metric_addr": ":9090", + }, + + "worker": { + "interval": "5s", + }, +} diff --git a/cli/example/main.go b/cli/example/main.go index 7a28837..ba67e3b 100644 --- a/cli/example/main.go +++ b/cli/example/main.go @@ -1,9 +1,10 @@ // Command example demonstrates the github.com/runreveal/lib/cli framework, -// including await integration for long-running services. +// including await integration for long-running services and config file loading. package main import ( "context" + _ "embed" "fmt" "net/http" "os" @@ -13,6 +14,9 @@ import ( "github.com/runreveal/lib/cli" ) +//go:embed config.json +var defaultConfig []byte + // --------------------------------------------------------------------------- // Globals: shared flags + resources via Configure/Validate/Close // --------------------------------------------------------------------------- @@ -85,6 +89,24 @@ func (s *ServeCmd) Run(ctx context.Context, args []string) error { type DaemonCmd struct { APIAddr string `cli:"api-addr" usage:"API listen address" default:":8080"` MetricAddr string `cli:"metric-addr" usage:"metrics listen address" default:":9090"` + Cfg DaemonConfig +} + +type DaemonConfig struct { + APIAddr string `json:"api_addr"` + MetricAddr string `json:"metric_addr"` +} + +// Configure applies config file values as defaults for flags that +// weren't explicitly set on the command line. +func (d *DaemonCmd) Configure() error { + if d.Cfg.APIAddr != "" && d.APIAddr == ":8080" { + d.APIAddr = d.Cfg.APIAddr + } + if d.Cfg.MetricAddr != "" && d.MetricAddr == ":9090" { + d.MetricAddr = d.Cfg.MetricAddr + } + return nil } func (d *DaemonCmd) Validate() error { @@ -121,6 +143,23 @@ func (d *DaemonCmd) Run(ctx context.Context, args []string) error { type WorkerCmd struct { Interval time.Duration `cli:"interval,i" usage:"poll interval" default:"10s"` + Cfg WorkerConfig +} + +type WorkerConfig struct { + Interval string `json:"interval"` +} + +// Configure applies config file values as defaults. +func (w *WorkerCmd) Configure() error { + if w.Cfg.Interval != "" && w.Interval == 10*time.Second { + d, err := time.ParseDuration(w.Cfg.Interval) + if err != nil { + return fmt.Errorf("parsing worker interval: %w", err) + } + w.Interval = d + } + return nil } // Run demonstrates a polling worker that exits cleanly on SIGINT/SIGTERM. @@ -167,26 +206,37 @@ func (m *MigrateCmd) Run(ctx context.Context, args []string) error { func main() { globals := &Globals{} serveCmd := &ServeCmd{} + daemonCmd := &DaemonCmd{} + workerCmd := &WorkerCmd{} app := cli.New("example", "Example CLI demonstrating cli + await", cli.WithVersion("1.0.0"), cli.WithGlobals(globals), cli.WithConfigFlag("config"), + cli.WithDefaultConfig(defaultConfig), ) app.AddCommand( - // Long-running server with await + graceful shutdown + // Long-running server with await + graceful shutdown. + // DB config loaded from the "database" section of config.json. cli.Command("serve", "Start the HTTP server", serveCmd, cli.ConfigAt("database", &serveCmd.DB), ), - // Multiple services under one await runner - cli.Command("daemon", "Run all services", &DaemonCmd{}), + // Multiple services under one await runner. + // Addresses loaded from the "daemon" section of config.json, + // with CLI flags overriding config values via Configure(). + cli.Command("daemon", "Run all services", daemonCmd, + cli.ConfigAt("daemon", &daemonCmd.Cfg), + ), - // Background worker using context cancellation - cli.Command("worker", "Run the background worker", &WorkerCmd{}), + // Background worker using context cancellation. + // Interval loaded from the "worker" section of config.json. + cli.Command("worker", "Run the background worker", workerCmd, + cli.ConfigAt("worker", &workerCmd.Cfg), + ), - // One-shot commands don't need await + // One-shot commands don't need await or config. cli.Group("admin", "Administrative commands", cli.Command("migrate", "Run database migrations", &MigrateCmd{}, cli.WithArgs(cli.NoArgs), From bdca9fa4e59b8f243c2fb0e5ce862ad681361c23 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 14:35:53 -0700 Subject: [PATCH 13/24] cli: rename default-config to defcon, make overridable --- cli/cli.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index 61fb55b..38cd627 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -168,12 +168,19 @@ func WithGlobals(ptr any) AppOption { } // WithDefaultConfig registers a default configuration that can be printed -// with "myapp default-config". Typically used with go:embed to ship a -// reference config alongside the binary. +// with "myapp defcon". Typically used with go:embed to ship a +// reference config alongside the binary. The command name defaults to +// "defcon" but can be overridden with WithDefaultConfigCommand. func WithDefaultConfig(data []byte) AppOption { return func(a *App) { a.defaultConfig = data } } +// WithDefaultConfigCommand overrides the command name used to print the +// default config (default: "defcon"). +func WithDefaultConfigCommand(name string) AppOption { + return func(a *App) { a.defaultConfigCmd = name } +} + // WithOutput sets the writer for help/error output (default: os.Stderr). func WithOutput(w io.Writer) AppOption { return func(a *App) { a.output = w } @@ -185,9 +192,10 @@ type App struct { desc string version string configFlag string - globals any // pointer to globals struct, if set - defaultConfig []byte - middlewares []Middleware + globals any // pointer to globals struct, if set + defaultConfig []byte + defaultConfigCmd string + middlewares []Middleware children []Node output io.Writer } @@ -241,7 +249,11 @@ func (a *App) run(ctx context.Context, args []string) (int, error) { if code, handled := a.handleCompletion(args); handled { return code, nil } - if len(args) == 1 && args[0] == "default-config" { + defconCmd := a.defaultConfigCmd + if defconCmd == "" { + defconCmd = "defcon" + } + if len(args) == 1 && args[0] == defconCmd { return a.handleDefaultConfig(), nil } From 0d7d064fc5273689285a07d7724ac5bbbad28104 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 14:40:16 -0700 Subject: [PATCH 14/24] cli: fix struct field alignment --- cli/cli.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index 38cd627..8950941 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -188,16 +188,16 @@ func WithOutput(w io.Writer) AppOption { // App is the top-level CLI application. type App struct { - name string - desc string - version string - configFlag string + name string + desc string + version string + configFlag string globals any // pointer to globals struct, if set defaultConfig []byte defaultConfigCmd string middlewares []Middleware - children []Node - output io.Writer + children []Node + output io.Writer } // New creates a new App. From dc38482b8aaa0a0852b37e5011501d7c375ed764 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 14:41:21 -0700 Subject: [PATCH 15/24] cli: add README --- cli/README.md | 246 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 246 insertions(+) create mode 100644 cli/README.md diff --git a/cli/README.md b/cli/README.md new file mode 100644 index 0000000..eb6b660 --- /dev/null +++ b/cli/README.md @@ -0,0 +1,246 @@ +# cli + +A struct-driven CLI framework for Go with a Configure-Validate-Run lifecycle. + +Designed as a simpler alternative to Cobra+Viper, with native integration with +[`loader`](../loader) (config file loading with HuJSON + env var replacement) +and [`await`](../await) (goroutine lifecycle and graceful shutdown). + +## Quick Start + +```go +package main + +import ( + "context" + "fmt" + "os" + + "github.com/runreveal/lib/cli" +) + +type ServeCmd struct { + Addr string `cli:"addr,a" usage:"listen address" default:":8080"` +} + +func (s *ServeCmd) Run(ctx context.Context, args []string) error { + fmt.Printf("serving on %s\n", s.Addr) + return nil +} + +func main() { + app := cli.New("myapp", "My application") + app.AddCommand(cli.Command("serve", "Start the server", &ServeCmd{})) + os.Exit(app.Run(context.Background(), os.Args[1:])) +} +``` + +## Features + +### Struct Tags + +Command structs use tags to define flags and config bindings: + +```go +type ServeCmd struct { + Addr string `cli:"addr,a" usage:"listen address" default:":8080"` + Timeout time.Duration `cli:"timeout,t" usage:"request timeout" default:"30s"` + DB DBConfig `config:"database"` // loaded from config file +} +``` + +| Tag | Purpose | +|---|---| +| `cli:"name,alias"` | Flag name and optional single-char alias | +| `cli:"-"` | Skip this field | +| `usage:"text"` | Help text | +| `default:"value"` | Default value (parsed to field type) | +| `config:"key"` | JSON path in config file to unmarshal into this field | + +Supported types: `string`, `bool`, `int`, `int64`, `uint`, `uint64`, `float64`, +`time.Duration`, `[]string`, pointer variants, and `encoding.TextUnmarshaler`. + +### Global Flags + +Define shared flags once at the app level instead of embedding in every command: + +```go +type Globals struct { + Verbose bool `cli:"verbose,v" usage:"enable verbose output"` + Config string `cli:"config,c" usage:"config file" default:"config.json"` +} + +globals := &Globals{} +app := cli.New("myapp", "desc", + cli.WithGlobals(globals), + cli.WithConfigFlag("config"), +) + +// Access in any handler: +func (s *ServeCmd) Run(ctx context.Context, args []string) error { + g := cli.GlobalsFromContext[Globals](ctx) + if g.Verbose { ... } +} +``` + +### Configure-Validate-Run Lifecycle + +Globals and handlers can implement optional lifecycle interfaces: + +```go +// Configurer is called after config loading to initialize resources. +type Configurer interface { + Configure() error +} + +// Validator is called after Configure to check readiness. +type Validator interface { + Validate() error +} +``` + +The framework also calls `io.Closer` on globals after the command exits. + +**Lifecycle order:** +1. Parse flags (globals + handler) +2. Load config file into struct fields +3. Globals: `Configure()` -> `Validate()` -> defer `Close()` +4. Handler: `Configure()` -> `Validate()` +5. Middleware -> `handler.Run(ctx, args)` +6. Globals `Close()` + +### Config File Loading + +One config file for the whole app. Commands declare which sections they need: + +```go +// Via struct tags: +type ServeCmd struct { + DB DBConfig `config:"database"` +} + +// Or via ConfigAt at registration time: +serveCmd := &ServeCmd{} +cli.Command("serve", "Start server", serveCmd, + cli.ConfigAt("database", &serveCmd.DB), +) +``` + +Config files are processed through `loader.LoadConfig`, which supports HuJSON +(comments, trailing commas) and `$ENV_VAR` replacement in string values. + +**Precedence:** explicit CLI flag > config file > default tag > zero value + +### Commands and Groups + +```go +app.AddCommand( + // Command with handler + cli.Command("serve", "Start the server", &ServeCmd{}), + + // Command with handler AND subcommands + cli.Command("admin", "Admin tools", &AdminCmd{}, + cli.Command("migrate", "Run migrations", &MigrateCmd{}), + ), + + // Group (no handler, prints help when invoked directly) + cli.Group("db", "Database commands", + cli.Command("migrate", "Run migrations", &MigrateCmd{}), + cli.Command("seed", "Seed data", &SeedCmd{}), + ), +) +``` + +### Middleware + +```go +app := cli.New("myapp", "desc", + cli.WithMiddleware(func(ctx context.Context, info cli.CommandInfo, next func(context.Context) error) error { + slog.Info("running", "command", info.Name) + return next(ctx) + }), +) +``` + +### Args Validation + +```go +cli.Command("get", "Get a resource", &GetCmd{}, cli.WithArgs(cli.ExactArgs(1))) +cli.Command("run", "Run a task", &RunCmd{}, cli.WithArgs(cli.NoArgs)) +cli.Command("ping", "Ping hosts", &PingCmd{}, cli.WithArgs(cli.MinArgs(1))) +``` + +### Await Integration + +For long-running services, use `await` in your handler's `Run` method: + +```go +func (s *ServeCmd) Run(ctx context.Context, args []string) error { + server := &http.Server{Addr: s.Addr, Handler: mux} + + w := await.New(await.WithSignals) + w.AddNamed(await.ListenAndServe(server), "http") + return w.Run(ctx) +} +``` + +For multiple services under one process: + +```go +func (d *DaemonCmd) Run(ctx context.Context, args []string) error { + w := await.New(await.WithSignals, await.WithStopTimeout(15*time.Second)) + w.AddNamed(await.ListenAndServe(apiServer), "api") + w.AddNamed(await.ListenAndServe(metricServer), "metrics") + return w.Run(ctx) +} +``` + +### Shell Completion + +```bash +# Generate completion script +eval "$(myapp completion bash)" # or zsh, fish +``` + +Handlers can provide custom completions for positional args: + +```go +func (g *GetCmd) Complete(ctx context.Context, args []string) []cli.Completion { + return []cli.Completion{ + {Value: "pods", Description: "list pods"}, + {Value: "services", Description: "list services"}, + } +} +``` + +### Default Config + +Ship a reference config with your binary using `go:embed`: + +```go +//go:embed config.json +var defaultConfig []byte + +app := cli.New("myapp", "desc", + cli.WithDefaultConfig(defaultConfig), +) +``` + +```bash +myapp defcon > config.json # dump the default config +``` + +The command name is `defcon` by default, overridable with `WithDefaultConfigCommand`. + +### Built-in Flags + +| Flag | Behavior | +|---|---| +| `-h`, `--help` | Print help for the app or command | +| `--version` | Print version (requires `WithVersion`) | + +## See Also + +- [`await`](../await) — goroutine lifecycle management +- [`loader`](../loader) — polymorphic config loading +- [`cli/example`](./example) — complete working example From 85b92c89c298e4a9740fbed78e12f5f452f8bb4f Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 14:48:11 -0700 Subject: [PATCH 16/24] cli: fix confusing DB naming in example, clarify ConfigAt semantics Rename MigrateCmd.DB (a database name flag) to MigrateCmd.Target to avoid confusion with ServeCmd.DB (a DBConfig struct from config file). ConfigAt is designed for config-only struct fields that have no corresponding CLI flag. For values that need both flag and config file support, use the config:"key" struct tag on the flag field (which checks IsSet for proper precedence), or load into a separate Cfg field and reconcile in Configure() as DaemonCmd demonstrates. --- cli/example/config.json | 3 +++ cli/example/main.go | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/cli/example/config.json b/cli/example/config.json index 7658152..b3356f8 100644 --- a/cli/example/config.json +++ b/cli/example/config.json @@ -2,15 +2,18 @@ // Example config for the CLI framework demo. // HuJSON: comments and trailing commas are allowed. + // Database connection settings, loaded into ServeCmd.DB via ConfigAt. "database": { "dsn": "postgres://localhost:5432/example?sslmode=disable", }, + // Daemon mode address overrides, applied in DaemonCmd.Configure(). "daemon": { "api_addr": ":8080", "metric_addr": ":9090", }, + // Worker polling interval, applied in WorkerCmd.Configure(). "worker": { "interval": "5s", }, diff --git a/cli/example/main.go b/cli/example/main.go index ba67e3b..92a38aa 100644 --- a/cli/example/main.go +++ b/cli/example/main.go @@ -187,14 +187,14 @@ func (w *WorkerCmd) Run(ctx context.Context, args []string) error { type MigrateCmd struct { DryRun bool `cli:"dry-run" usage:"print migrations without running"` - DB string `cli:"db" usage:"database name" default:"prod"` + Target string `cli:"target" usage:"target database name" default:"prod"` } func (m *MigrateCmd) Run(ctx context.Context, args []string) error { if m.DryRun { - fmt.Printf("[dry-run] would migrate database: %s\n", m.DB) + fmt.Printf("[dry-run] would migrate database: %s\n", m.Target) } else { - fmt.Printf("migrating database: %s\n", m.DB) + fmt.Printf("migrating database: %s\n", m.Target) } return nil } From 741eb9ecb04db4c7dfacfbf61ae7d63e9f81d369 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 15:06:42 -0700 Subject: [PATCH 17/24] cli: remove ConfigAt, add loader integration example ConfigAt added complexity without clear value over the globals pattern for shared resources. Removed entirely. The example now demonstrates the loader polymorphic config pattern: - Source interface with webhook and syslog implementations - Cache interface with memory implementation - loader.Register + loader.Loader[T] for type-driven config - Globals.Configure() initializes resources from config - Commands access shared resources via GlobalsFromContext --- cli/README.md | 10 +- cli/cli.go | 22 +--- cli/cli_test.go | 77 ----------- cli/config.go | 14 -- cli/example/config.json | 19 +-- cli/example/go.mod | 5 +- cli/example/main.go | 278 +++++++++++++++++----------------------- 7 files changed, 131 insertions(+), 294 deletions(-) diff --git a/cli/README.md b/cli/README.md index eb6b660..f9d6c09 100644 --- a/cli/README.md +++ b/cli/README.md @@ -111,19 +111,13 @@ The framework also calls `io.Closer` on globals after the command exits. ### Config File Loading -One config file for the whole app. Commands declare which sections they need: +One config file for the whole app. Commands declare which sections they need +using `config:"key"` struct tags: ```go -// Via struct tags: type ServeCmd struct { DB DBConfig `config:"database"` } - -// Or via ConfigAt at registration time: -serveCmd := &ServeCmd{} -cli.Command("serve", "Start server", serveCmd, - cli.ConfigAt("database", &serveCmd.DB), -) ``` Config files are processed through `loader.LoadConfig`, which supports HuJSON diff --git a/cli/cli.go b/cli/cli.go index 8950941..3684e9a 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -21,12 +21,6 @@ type flagSetKey struct{} // globalsKey is the context key used to carry the globals pointer during execution. type globalsKey struct{} -// configBinding pairs a config file key with a destination pointer for ConfigAt. -type configBinding struct { - key string - dst any -} - // Runnable is the core interface every command handler must implement. type Runnable interface { Run(ctx context.Context, args []string) error @@ -102,8 +96,7 @@ func (g *groupNode) isGroup() bool { return true } type CmdOption func(*cmdOptions) type cmdOptions struct { - argsFunc ArgsFunc - configBindings []configBinding + argsFunc ArgsFunc } // WithArgs sets an args validation function on a command. @@ -111,15 +104,6 @@ func WithArgs(f ArgsFunc) CmdOption { return func(o *cmdOptions) { o.argsFunc = f } } -// ConfigAt registers a config file section to be unmarshaled into dst. -// key is a dot-separated path into the config file JSON (e.g. "serve", "common.db"). -// Use "." for the entire config root. dst must be a pointer. -func ConfigAt(key string, dst any) CmdOption { - return func(o *cmdOptions) { - o.configBindings = append(o.configBindings, configBinding{key: key, dst: dst}) - } -} - // Command creates a command node. Each element of opts may be a Node (child // subcommand) or a CmdOption (behavioural option); they are distinguished by // type at runtime. @@ -405,10 +389,6 @@ func (a *App) executeCommand(ctx context.Context, node *commandNode, args []stri if err := applyConfigTags(handler, fs, fields, configJSON); err != nil { return 1, fmt.Errorf("loading config: %w", err) } - // Apply ConfigAt bindings - if err := applyConfigBindings(node.opts.configBindings, configJSON); err != nil { - return 1, fmt.Errorf("loading config: %w", err) - } } } diff --git a/cli/cli_test.go b/cli/cli_test.go index cd7194f..7752fb5 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -782,83 +782,6 @@ func TestGlobals_ConfigFlagOnGlobals(t *testing.T) { assert.Equal(t, 3306, configCmd.DB.Port) } -// --- ConfigAt tests --- - -type configAtCmd struct { - Addr string `cli:"addr" usage:"listen address" default:":8080"` - DB dbSection -} - -func (c *configAtCmd) Run(_ context.Context, _ []string) error { return nil } - -func TestConfigAt_BasicLoad(t *testing.T) { - g := &testGlobals{} - cmd := &configAtCmd{} - f := writeConfigFile(t, `{ - "database": {"host": "configat-host", "port": 5432} - }`) - - var buf bytes.Buffer - app := cli.New("app", "test", - cli.WithOutput(&buf), - cli.WithGlobals(g), - cli.WithConfigFlag("config"), - ) - app.AddCommand(cli.Command("serve", "serve", cmd, - cli.ConfigAt("database", &cmd.DB), - )) - - code := app.Run(context.Background(), []string{"serve", "--config", f}) - assert.Equal(t, 0, code) - assert.Equal(t, "configat-host", cmd.DB.Host) - assert.Equal(t, 5432, cmd.DB.Port) -} - -func TestConfigAt_NestedKey(t *testing.T) { - g := &testGlobals{} - cmd := &configAtCmd{} - f := writeConfigFile(t, `{ - "services": { - "api": {"host": "nested-host", "port": 9090} - } - }`) - - var buf bytes.Buffer - app := cli.New("app", "test", - cli.WithOutput(&buf), - cli.WithGlobals(g), - cli.WithConfigFlag("config"), - ) - app.AddCommand(cli.Command("serve", "serve", cmd, - cli.ConfigAt("services.api", &cmd.DB), - )) - - code := app.Run(context.Background(), []string{"serve", "--config", f}) - assert.Equal(t, 0, code) - assert.Equal(t, "nested-host", cmd.DB.Host) - assert.Equal(t, 9090, cmd.DB.Port) -} - -func TestConfigAt_MissingSectionSilent(t *testing.T) { - g := &testGlobals{} - cmd := &configAtCmd{} - f := writeConfigFile(t, `{"other": {}}`) - - var buf bytes.Buffer - app := cli.New("app", "test", - cli.WithOutput(&buf), - cli.WithGlobals(g), - cli.WithConfigFlag("config"), - ) - app.AddCommand(cli.Command("serve", "serve", cmd, - cli.ConfigAt("database", &cmd.DB), - )) - - code := app.Run(context.Background(), []string{"serve", "--config", f}) - assert.Equal(t, 0, code) - assert.Equal(t, "", cmd.DB.Host) // not populated -} - // --- CVR lifecycle on globals --- type cvrGlobals struct { diff --git a/cli/config.go b/cli/config.go index c075c4b..98df037 100644 --- a/cli/config.go +++ b/cli/config.go @@ -114,20 +114,6 @@ func applyConfigTags(target any, fs *FlagSet, fields []fieldInfo, rawJSON string return nil } -// applyConfigBindings applies ConfigAt bindings. -func applyConfigBindings(bindings []configBinding, rawJSON string) error { - for _, b := range bindings { - section := extractSection(rawJSON, b.key) - if section == "" { - continue - } - if err := sgjson.Unmarshal([]byte(section), b.dst); err != nil { - return fmt.Errorf("config key %q: %w", b.key, err) - } - } - return nil -} - // extractSection extracts a JSON section by key path. func extractSection(rawJSON, key string) string { if key == "." { diff --git a/cli/example/config.json b/cli/example/config.json index b3356f8..011871a 100644 --- a/cli/example/config.json +++ b/cli/example/config.json @@ -2,19 +2,12 @@ // Example config for the CLI framework demo. // HuJSON: comments and trailing commas are allowed. - // Database connection settings, loaded into ServeCmd.DB via ConfigAt. - "database": { - "dsn": "postgres://localhost:5432/example?sslmode=disable", - }, + "sources": [ + {"type": "webhook", "path": "/hooks/github"}, + {"type": "syslog", "addr": ":514"}, + ], - // Daemon mode address overrides, applied in DaemonCmd.Configure(). - "daemon": { - "api_addr": ":8080", - "metric_addr": ":9090", - }, + "cache": {"type": "memory", "max_size": 1000}, - // Worker polling interval, applied in WorkerCmd.Configure(). - "worker": { - "interval": "5s", - }, + "server": {"addr": ":8080"}, } diff --git a/cli/example/go.mod b/cli/example/go.mod index 98ce51d..c134493 100644 --- a/cli/example/go.mod +++ b/cli/example/go.mod @@ -3,12 +3,11 @@ module github.com/runreveal/lib/cli/example go 1.24 require ( - github.com/runreveal/lib/await v0.0.0-20231128193746-50c2ad68891c github.com/runreveal/lib/cli v0.0.0 + github.com/runreveal/lib/loader v0.0.0 ) require ( - github.com/runreveal/lib/loader v0.0.0-20231128193746-50c2ad68891c // indirect github.com/segmentio/asm v1.1.3 // indirect github.com/segmentio/encoding v0.3.6 // indirect github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a // indirect @@ -19,3 +18,5 @@ require ( ) replace github.com/runreveal/lib/cli => ../ + +replace github.com/runreveal/lib/loader => ../../loader diff --git a/cli/example/main.go b/cli/example/main.go index 92a38aa..fc3fca4 100644 --- a/cli/example/main.go +++ b/cli/example/main.go @@ -1,200 +1,183 @@ -// Command example demonstrates the github.com/runreveal/lib/cli framework, -// including await integration for long-running services and config file loading. +// Command example demonstrates the github.com/runreveal/lib/cli framework +// with github.com/runreveal/lib/loader for polymorphic config loading. package main import ( "context" _ "embed" "fmt" - "net/http" "os" - "time" - "github.com/runreveal/lib/await" "github.com/runreveal/lib/cli" + "github.com/runreveal/lib/loader" ) //go:embed config.json var defaultConfig []byte // --------------------------------------------------------------------------- -// Globals: shared flags + resources via Configure/Validate/Close +// Source: an interface with multiple implementations loaded via loader // --------------------------------------------------------------------------- -// Globals holds flags and resources shared across all commands. -type Globals struct { - Verbose bool `cli:"verbose,v" usage:"enable verbose output"` - Config string `cli:"config,c" usage:"config file path" default:"config.json"` +type Source interface { + Name() string } -// --------------------------------------------------------------------------- -// serve: a long-running HTTP server managed by await -// --------------------------------------------------------------------------- - -type ServeCmd struct { - Addr string `cli:"addr,a" usage:"listen address" default:":8080"` - Timeout time.Duration `cli:"timeout,t" usage:"request timeout" default:"30s"` - DB DBConfig +func init() { + loader.Register[Source]("webhook", func() loader.Builder[Source] { return &WebhookConfig{} }) + loader.Register[Source]("syslog", func() loader.Builder[Source] { return &SyslogConfig{} }) } -type DBConfig struct { - DSN string `json:"dsn"` +// WebhookConfig is the config for a webhook source. +type WebhookConfig struct { + Type string `json:"type"` + Path string `json:"path"` } -func (s *ServeCmd) Validate() error { - if s.Addr == "" { - return fmt.Errorf("--addr must not be empty") - } - return nil +func (w *WebhookConfig) Configure() (Source, error) { + return &WebhookSource{path: w.Path}, nil } -// Run starts the HTTP server using await for graceful shutdown. -// The ctx passed by the cli framework is cancelled on SIGINT/SIGTERM -// when the root command is run, but await.WithSignals gives you the -// same behavior with named sub-runners and a configurable stop timeout. -func (s *ServeCmd) Run(ctx context.Context, args []string) error { - g := cli.GlobalsFromContext[Globals](ctx) - - mux := http.NewServeMux() - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintln(w, "ok") - }) +type WebhookSource struct{ path string } - server := &http.Server{ - Addr: s.Addr, - Handler: mux, - ReadTimeout: s.Timeout, - WriteTimeout: s.Timeout, - } +func (w *WebhookSource) Name() string { return "webhook:" + w.path } - if g != nil && g.Verbose { - fmt.Printf("starting server on %s\n", s.Addr) - if s.DB.DSN != "" { - fmt.Printf("database: %s\n", s.DB.DSN) - } - } +// SyslogConfig is the config for a syslog source. +type SyslogConfig struct { + Type string `json:"type"` + Addr string `json:"addr"` +} - // await manages graceful shutdown: on SIGINT/SIGTERM it cancels - // the context, ListenAndServe calls server.Shutdown, and await - // waits up to the stop timeout for in-flight requests to drain. - w := await.New(await.WithSignals) - w.AddNamed(await.ListenAndServe(server), "http") - return w.Run(ctx) +func (s *SyslogConfig) Configure() (Source, error) { + return &SyslogSource{addr: s.Addr}, nil } +type SyslogSource struct{ addr string } + +func (s *SyslogSource) Name() string { return "syslog:" + s.addr } + // --------------------------------------------------------------------------- -// daemon: run multiple services concurrently with await +// Cache: a single-value polymorphic config // --------------------------------------------------------------------------- -type DaemonCmd struct { - APIAddr string `cli:"api-addr" usage:"API listen address" default:":8080"` - MetricAddr string `cli:"metric-addr" usage:"metrics listen address" default:":9090"` - Cfg DaemonConfig +type Cache interface { + Name() string } -type DaemonConfig struct { - APIAddr string `json:"api_addr"` - MetricAddr string `json:"metric_addr"` +func init() { + loader.Register[Cache]("memory", func() loader.Builder[Cache] { return &MemoryCacheConfig{} }) } -// Configure applies config file values as defaults for flags that -// weren't explicitly set on the command line. -func (d *DaemonCmd) Configure() error { - if d.Cfg.APIAddr != "" && d.APIAddr == ":8080" { - d.APIAddr = d.Cfg.APIAddr - } - if d.Cfg.MetricAddr != "" && d.MetricAddr == ":9090" { - d.MetricAddr = d.Cfg.MetricAddr - } - return nil +type MemoryCacheConfig struct { + Type string `json:"type"` + MaxSize int `json:"max_size"` } -func (d *DaemonCmd) Validate() error { - if d.APIAddr == "" || d.MetricAddr == "" { - return fmt.Errorf("both --api-addr and --metric-addr are required") - } - return nil +func (m *MemoryCacheConfig) Configure() (Cache, error) { + return &MemoryCache{maxSize: m.MaxSize}, nil } -// Run starts multiple services under a single await runner. -// If any service exits with an error, await cancels the others -// and waits for them to shut down cleanly. -func (d *DaemonCmd) Run(ctx context.Context, args []string) error { - apiServer := &http.Server{ - Addr: d.APIAddr, - Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprintln(w, "api") }), - } - metricServer := &http.Server{ - Addr: d.MetricAddr, - Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprintln(w, "metrics") }), - } +type MemoryCache struct{ maxSize int } - w := await.New(await.WithSignals, await.WithStopTimeout(15*time.Second)) - w.AddNamed(await.ListenAndServe(apiServer), "api") - w.AddNamed(await.ListenAndServe(metricServer), "metrics") +func (m *MemoryCache) Name() string { return fmt.Sprintf("memory(max=%d)", m.maxSize) } - fmt.Printf("daemon: api=%s metrics=%s\n", d.APIAddr, d.MetricAddr) - return w.Run(ctx) +// --------------------------------------------------------------------------- +// AppConfig: loaded from config.json via config:"." on Globals +// --------------------------------------------------------------------------- + +type AppConfig struct { + Sources []loader.Loader[Source] `json:"sources"` + Cache loader.Loader[Cache] `json:"cache"` + Server ServerConfig `json:"server"` +} + +type ServerConfig struct { + Addr string `json:"addr"` } // --------------------------------------------------------------------------- -// worker: a background job that respects context cancellation +// Globals: shared flags + config + initialized resources // --------------------------------------------------------------------------- -type WorkerCmd struct { - Interval time.Duration `cli:"interval,i" usage:"poll interval" default:"10s"` - Cfg WorkerConfig -} +type Globals struct { + Verbose bool `cli:"verbose,v" usage:"enable verbose output"` + Config string `cli:"config,c" usage:"config file path" default:"config.json"` + Cfg AppConfig ` config:"."` -type WorkerConfig struct { - Interval string `json:"interval"` + // Initialized resources + sources []Source + cache Cache } -// Configure applies config file values as defaults. -func (w *WorkerCmd) Configure() error { - if w.Cfg.Interval != "" && w.Interval == 10*time.Second { - d, err := time.ParseDuration(w.Cfg.Interval) +func (g *Globals) Configure() error { + for _, src := range g.Cfg.Sources { + s, err := src.Configure() if err != nil { - return fmt.Errorf("parsing worker interval: %w", err) + return fmt.Errorf("configuring source: %w", err) } - w.Interval = d + g.sources = append(g.sources, s) + } + if g.Cfg.Cache.Builder != nil { + c, err := g.Cfg.Cache.Configure() + if err != nil { + return fmt.Errorf("configuring cache: %w", err) + } + g.cache = c } return nil } -// Run demonstrates a polling worker that exits cleanly on SIGINT/SIGTERM. -// For a single long-running goroutine, you don't need await — just -// select on ctx.Done(). -func (w *WorkerCmd) Run(ctx context.Context, args []string) error { - fmt.Printf("worker: polling every %s\n", w.Interval) - ticker := time.NewTicker(w.Interval) - defer ticker.Stop() - - for { - select { - case <-ctx.Done(): - fmt.Println("worker: shutting down") - return nil - case <-ticker.C: - fmt.Println("worker: tick") - } +func (g *Globals) Validate() error { + if len(g.sources) == 0 { + return fmt.Errorf("at least one source is required") } + return nil } // --------------------------------------------------------------------------- -// migrate: a one-shot command (no await needed) +// serve: uses initialized resources from Globals // --------------------------------------------------------------------------- -type MigrateCmd struct { - DryRun bool `cli:"dry-run" usage:"print migrations without running"` - Target string `cli:"target" usage:"target database name" default:"prod"` +type ServeCmd struct { + Addr string `cli:"addr,a" usage:"listen address"` +} + +func (s *ServeCmd) Configure() error { + // Apply server config from file as default if --addr not set. + return nil +} + +func (s *ServeCmd) Run(ctx context.Context, args []string) error { + g := cli.GlobalsFromContext[Globals](ctx) + if g == nil { + return fmt.Errorf("globals not available") + } + + fmt.Printf("server addr: %s\n", s.Addr) + fmt.Printf("sources:\n") + for _, src := range g.sources { + fmt.Printf(" - %s\n", src.Name()) + } + if g.cache != nil { + fmt.Printf("cache: %s\n", g.cache.Name()) + } + return nil } -func (m *MigrateCmd) Run(ctx context.Context, args []string) error { - if m.DryRun { - fmt.Printf("[dry-run] would migrate database: %s\n", m.Target) - } else { - fmt.Printf("migrating database: %s\n", m.Target) +// --------------------------------------------------------------------------- +// list-sources: shows configured sources +// --------------------------------------------------------------------------- + +type ListSourcesCmd struct{} + +func (l *ListSourcesCmd) Run(ctx context.Context, args []string) error { + g := cli.GlobalsFromContext[Globals](ctx) + if g == nil { + return fmt.Errorf("globals not available") + } + + for _, src := range g.sources { + fmt.Println(src.Name()) } return nil } @@ -206,10 +189,8 @@ func (m *MigrateCmd) Run(ctx context.Context, args []string) error { func main() { globals := &Globals{} serveCmd := &ServeCmd{} - daemonCmd := &DaemonCmd{} - workerCmd := &WorkerCmd{} - app := cli.New("example", "Example CLI demonstrating cli + await", + app := cli.New("example", "Example CLI demonstrating cli + loader", cli.WithVersion("1.0.0"), cli.WithGlobals(globals), cli.WithConfigFlag("config"), @@ -217,30 +198,9 @@ func main() { ) app.AddCommand( - // Long-running server with await + graceful shutdown. - // DB config loaded from the "database" section of config.json. - cli.Command("serve", "Start the HTTP server", serveCmd, - cli.ConfigAt("database", &serveCmd.DB), - ), - - // Multiple services under one await runner. - // Addresses loaded from the "daemon" section of config.json, - // with CLI flags overriding config values via Configure(). - cli.Command("daemon", "Run all services", daemonCmd, - cli.ConfigAt("daemon", &daemonCmd.Cfg), - ), - - // Background worker using context cancellation. - // Interval loaded from the "worker" section of config.json. - cli.Command("worker", "Run the background worker", workerCmd, - cli.ConfigAt("worker", &workerCmd.Cfg), - ), - - // One-shot commands don't need await or config. - cli.Group("admin", "Administrative commands", - cli.Command("migrate", "Run database migrations", &MigrateCmd{}, - cli.WithArgs(cli.NoArgs), - ), + cli.Command("serve", "Start the HTTP server", serveCmd), + cli.Command("list-sources", "List configured sources", &ListSourcesCmd{}, + cli.WithArgs(cli.NoArgs), ), ) From f06052c1cec80108fbea6f966e8af4cee78650c2 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 15:14:38 -0700 Subject: [PATCH 18/24] cli: add tests for coverage, fix errcheck lint, fix Makefile cd issue Tests: - Table-driven config precedence tests (flag > config > default) - Pointer type flag coverage (*string, *int, *float64, *uint64, *bool, *Duration) - Config error paths (missing file, invalid JSON) - DumpConfig, defcon, ExitError, MinArgs - Globals Configure/Validate error paths - Handler Configure error path - Short flag and long flag edge cases Fixes: - Handle error from a.output.Write in handleDefaultConfig - Use subshells in Makefile for/cd loops so failures don't break subsequent iterations with stale working directory --- Makefile | 10 +- cli/cli.go | 5 +- cli/cli_test.go | 434 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 442 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index d53577a..3c0e4f9 100644 --- a/Makefile +++ b/Makefile @@ -12,11 +12,10 @@ GO ?= $(shell which go) .PHONY: test test: @for dir in $(SUBDIRS); do \ - cd $$dir && \ + (cd $$dir && \ $(GO) test -vet=off -tags='$(GOTAGS)' $(GOTESTFLAGS) -coverpkg="./..." -coverprofile=.coverprofile ./... && \ grep -v 'cmd' < .coverprofile > .covprof && mv .covprof .coverprofile && \ - $(GO) tool cover -func=.coverprofile && \ - cd .. ; \ + $(GO) tool cover -func=.coverprofile) || exit 1; \ done .PHONY: coverage @@ -35,9 +34,8 @@ version: .PHONY: lint lint: $(GOPATH)/bin/golangci-lint @for dir in $(SUBDIRS); do \ - cd $$dir && \ - golangci-lint run --timeout 5m . && \ - cd .. ; \ + (cd $$dir && \ + golangci-lint run --timeout 5m .) || exit 1; \ done $(GOPATH)/bin/golangci-lint: diff --git a/cli/cli.go b/cli/cli.go index 3684e9a..c56b442 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -465,7 +465,10 @@ func (a *App) handleDefaultConfig() int { fmt.Fprintf(a.output, "no default config registered\n") return 1 } - a.output.Write(a.defaultConfig) + if _, err := a.output.Write(a.defaultConfig); err != nil { + fmt.Fprintf(a.output, "error writing config: %s\n", err) + return 1 + } return 0 } diff --git a/cli/cli_test.go b/cli/cli_test.go index 7752fb5..4eeebd9 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -915,3 +915,437 @@ func TestConfig_EnvVarReplacement(t *testing.T) { assert.Equal(t, 0, code) assert.Equal(t, "env-replaced-host", handler.Host) } + +// --- Config precedence table-driven tests --- + +func TestConfig_Precedence(t *testing.T) { + tests := []struct { + name string + configJSON string + args []string + wantHost string + }{ + { + name: "flag overrides config", + configJSON: `{"host": "from-config"}`, + args: []string{"run", "--config", "", "--host", "from-flag"}, + wantHost: "from-flag", + }, + { + name: "config overrides default", + configJSON: `{"host": "from-config"}`, + args: []string{"run", "--config", ""}, + wantHost: "from-config", + }, + { + name: "default when no config and no flag", + configJSON: "", + args: []string{"run"}, + wantHost: "flag-default", + }, + { + name: "config key missing uses default", + configJSON: `{"other": "value"}`, + args: []string{"run", "--config", ""}, + wantHost: "flag-default", + }, + { + name: "empty config value overrides default", + configJSON: `{"host": ""}`, + args: []string{"run", "--config", ""}, + wantHost: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handler := &overrideableHandler{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + args := make([]string, len(tt.args)) + copy(args, tt.args) + + if tt.configJSON != "" { + f := writeConfigFile(t, tt.configJSON) + for i, a := range args { + if a == "" && i > 0 && args[i-1] == "--config" { + args[i] = f + } + } + } + + code := app.Run(context.Background(), args) + assert.Equal(t, 0, code, "output: %s", buf.String()) + assert.Equal(t, tt.wantHost, handler.Host) + }) + } +} + +// --- Config with globals: config tag on globals struct --- + +type globalsWithConfig struct { + Config string `cli:"config,c" usage:"config" default:"config.json"` + AppName string ` config:"app_name"` +} + +type plainCmd struct { + ran bool +} + +func (p *plainCmd) Run(_ context.Context, _ []string) error { + p.ran = true + return nil +} + +func TestConfig_GlobalsConfigTag(t *testing.T) { + g := &globalsWithConfig{} + cmd := &plainCmd{} + f := writeConfigFile(t, `{"app_name": "loaded-from-config"}`) + + var buf bytes.Buffer + app := cli.New("app", "test", + cli.WithOutput(&buf), + cli.WithGlobals(g), + cli.WithConfigFlag("config"), + ) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), []string{"run", "--config", f}) + assert.Equal(t, 0, code) + assert.True(t, cmd.ran) + assert.Equal(t, "loaded-from-config", g.AppName) +} + +// --- Flag parsing edge cases (table-driven) --- + +func TestFlagParsing_ShortFlagEdgeCases(t *testing.T) { + tests := []struct { + name string + args []string + wantVerbose bool + wantDebug bool + wantName string + }{ + { + name: "combined bool shorts", + args: []string{"-vd"}, + wantVerbose: true, + wantDebug: true, + }, + { + name: "combined bool + value: -vd is both bools", + args: []string{"-vd", "--name", "x"}, + wantVerbose: true, + wantDebug: true, + wantName: "x", + }, + { + name: "single short with value", + args: []string{"-v", "--name", "alice"}, + wantName: "alice", wantVerbose: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := &boolCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), append([]string{"run"}, tt.args...)) + assert.Equal(t, 0, code, "output: %s", buf.String()) + assert.Equal(t, tt.wantVerbose, cmd.Verbose) + assert.Equal(t, tt.wantDebug, cmd.Debug) + assert.Equal(t, tt.wantName, cmd.Name) + }) + } +} + +// --- Type coverage for makeFlagDef --- + +type moreTypesCmd struct { + PStr *string `cli:"pstr" usage:"ptr string"` + PInt *int `cli:"pint" usage:"ptr int"` + PDur *time.Duration `cli:"pdur" usage:"ptr duration"` + PFlt *float64 `cli:"pflt" usage:"ptr float"` + PU64 *uint64 `cli:"pu64" usage:"ptr uint64"` + PBol *bool `cli:"pbol" usage:"ptr bool"` +} + +func (m *moreTypesCmd) Run(_ context.Context, _ []string) error { return nil } + +func TestFlagParsing_PointerTypes(t *testing.T) { + tests := []struct { + name string + args []string + check func(t *testing.T, cmd *moreTypesCmd) + }{ + { + name: "ptr string set", + args: []string{"--pstr", "hello"}, + check: func(t *testing.T, cmd *moreTypesCmd) { + require.NotNil(t, cmd.PStr) + assert.Equal(t, "hello", *cmd.PStr) + }, + }, + { + name: "ptr int set", + args: []string{"--pint", "42"}, + check: func(t *testing.T, cmd *moreTypesCmd) { + require.NotNil(t, cmd.PInt) + assert.Equal(t, 42, *cmd.PInt) + }, + }, + { + name: "ptr duration set", + args: []string{"--pdur", "5s"}, + check: func(t *testing.T, cmd *moreTypesCmd) { + require.NotNil(t, cmd.PDur) + assert.Equal(t, 5*time.Second, *cmd.PDur) + }, + }, + { + name: "ptr float set", + args: []string{"--pflt", "3.14"}, + check: func(t *testing.T, cmd *moreTypesCmd) { + require.NotNil(t, cmd.PFlt) + assert.InDelta(t, 3.14, *cmd.PFlt, 0.001) + }, + }, + { + name: "ptr uint64 set", + args: []string{"--pu64", "99"}, + check: func(t *testing.T, cmd *moreTypesCmd) { + require.NotNil(t, cmd.PU64) + assert.Equal(t, uint64(99), *cmd.PU64) + }, + }, + { + name: "ptr bool set", + args: []string{"--pbol"}, + check: func(t *testing.T, cmd *moreTypesCmd) { + require.NotNil(t, cmd.PBol) + assert.True(t, *cmd.PBol) + }, + }, + { + name: "unset ptrs remain nil", + args: []string{}, + check: func(t *testing.T, cmd *moreTypesCmd) { + assert.Nil(t, cmd.PStr) + assert.Nil(t, cmd.PInt) + assert.Nil(t, cmd.PDur) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cmd := &moreTypesCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), append([]string{"run"}, tt.args...)) + assert.Equal(t, 0, code, "output: %s", buf.String()) + tt.check(t, cmd) + }) + } +} + +// --- DumpConfig, DefaultConfig, ExitError --- + +func TestDumpConfig(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo", cmd)) + + app.Run(context.Background(), []string{"echo", "--message", "hi"}) + m := cli.DumpConfig(cmd) + require.NotNil(t, m) + assert.Equal(t, "hi", m["message"]) + assert.Equal(t, 1, m["count"]) // default +} + +func TestDefaultConfig(t *testing.T) { + data := []byte(`{"key": "value"}`) + var buf bytes.Buffer + app := cli.New("app", "test", + cli.WithOutput(&buf), + cli.WithDefaultConfig(data), + ) + + code := app.Run(context.Background(), []string{"defcon"}) + assert.Equal(t, 0, code) + assert.Equal(t, string(data), buf.String()) +} + +func TestDefaultConfig_CustomCommand(t *testing.T) { + data := []byte(`{"x": 1}`) + var buf bytes.Buffer + app := cli.New("app", "test", + cli.WithOutput(&buf), + cli.WithDefaultConfig(data), + cli.WithDefaultConfigCommand("dump"), + ) + + code := app.Run(context.Background(), []string{"dump"}) + assert.Equal(t, 0, code) + assert.Equal(t, string(data), buf.String()) +} + +func TestDefaultConfig_NotRegistered(t *testing.T) { + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + + code := app.Run(context.Background(), []string{"defcon"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "no default config") +} + +func TestExitError(t *testing.T) { + e := &cli.ExitError{Code: 3, Err: errors.New("boom")} + assert.Equal(t, "boom", e.Error()) + assert.Equal(t, "boom", e.Unwrap().Error()) + + e2 := &cli.ExitError{Code: 5} + assert.Contains(t, e2.Error(), "exit code 5") + assert.Nil(t, e2.Unwrap()) +} + +// --- Config file error paths --- + +func TestConfig_ExplicitMissingFileErrors(t *testing.T) { + handler := &configHandler{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + // Explicitly passing a non-existent file should error + code := app.Run(context.Background(), []string{"run", "--config", "/nonexistent/config.json"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "reading config file") +} + +func TestConfig_InvalidJSON(t *testing.T) { + handler := &configHandler{} + f := writeConfigFile(t, `{not valid json`) + + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithConfigFlag("config")) + app.AddCommand(cli.Command("run", "run", handler)) + + code := app.Run(context.Background(), []string{"run", "--config", f}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "parsing config file") +} + +// --- MinArgs coverage --- + +func TestEdge_ArgsValidation_MinArgs(t *testing.T) { + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", &noopCmd{}, cli.WithArgs(cli.MinArgs(2)))) + + code := app.Run(context.Background(), []string{"run", "a", "b"}) + assert.Equal(t, 0, code) + + code = app.Run(context.Background(), []string{"run", "a"}) + assert.Equal(t, 1, code) +} + +// --- Handler Configure error --- + +type failConfigureCmd struct{} + +func (f *failConfigureCmd) Configure() error { return errors.New("configure failed") } +func (f *failConfigureCmd) Run(_ context.Context, _ []string) error { return nil } + +func TestHandler_ConfigureError(t *testing.T) { + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", &failConfigureCmd{})) + + code := app.Run(context.Background(), []string{"run"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "configure") +} + +// --- Globals Configure/Validate errors --- + +type failGlobalsConfigure struct { + Config string `cli:"config,c" default:"config.json"` +} + +func (f *failGlobalsConfigure) Configure() error { + return errors.New("globals configure boom") +} + +func TestGlobals_ConfigureError(t *testing.T) { + g := &failGlobalsConfigure{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithGlobals(g)) + app.AddCommand(cli.Command("run", "run", &noopCmd{})) + + code := app.Run(context.Background(), []string{"run"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "globals configure") +} + +type failGlobalsValidate struct { + Config string `cli:"config,c" default:"config.json"` +} + +func (f *failGlobalsValidate) Validate() error { + return errors.New("globals validate boom") +} + +func TestGlobals_ValidateError(t *testing.T) { + g := &failGlobalsValidate{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf), cli.WithGlobals(g)) + app.AddCommand(cli.Command("run", "run", &noopCmd{})) + + code := app.Run(context.Background(), []string{"run"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "globals validate") +} + +// --- Long flag edge cases --- + +func TestFlagParsing_LongBoolWithEquals(t *testing.T) { + cmd := &boolCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("run", "run", cmd)) + + code := app.Run(context.Background(), []string{"run", "--verbose=true"}) + assert.Equal(t, 0, code) + assert.True(t, cmd.Verbose) +} + +func TestFlagParsing_LongFlagMissingValue(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo", cmd)) + + code := app.Run(context.Background(), []string{"echo", "--message"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "requires a value") +} + +func TestFlagParsing_ShortFlagMissingValue(t *testing.T) { + cmd := &echoCmd{} + var buf bytes.Buffer + app := cli.New("app", "test", cli.WithOutput(&buf)) + app.AddCommand(cli.Command("echo", "echo", cmd)) + + code := app.Run(context.Background(), []string{"echo", "-m"}) + assert.Equal(t, 1, code) + assert.Contains(t, buf.String(), "requires a value") +} From 6dab80c90d0bb17a23bbde430d6d240733908d57 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 15:48:54 -0700 Subject: [PATCH 19/24] cli: add HelpExtra interface; loader: add List and Describe cli: HelpExtra interface lets handlers and globals append extra text to help output. Useful for showing available loader types without the cli package knowing about loader. loader: List[T]() returns sorted registered type names. Describe[T]() returns a zero-value builder for introspection (e.g. reflecting on JSON struct tags for help output). Example shows the bridge pattern: Globals.ExtraHelp() uses loader.List and loader.Describe to render available source and cache types with their config fields directly in --help output. --- cli/cli.go | 8 ++++++ cli/example/main.go | 59 +++++++++++++++++++++++++++++++++++++++++++++ cli/help.go | 10 ++++++++ loader/loader.go | 48 ++++++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+) diff --git a/cli/cli.go b/cli/cli.go index c56b442..dc40cb5 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -39,6 +39,14 @@ type Validator interface { Validate() error } +// HelpExtra is optionally implemented by command handlers to append +// additional information to help output. Useful for showing available +// loader types, config file schemas, or other context that the cli +// framework can't derive from struct tags alone. +type HelpExtra interface { + ExtraHelp() string +} + // ExitError carries a custom exit code. type ExitError struct { Code int diff --git a/cli/example/main.go b/cli/example/main.go index fc3fca4..c314dfa 100644 --- a/cli/example/main.go +++ b/cli/example/main.go @@ -7,6 +7,8 @@ import ( _ "embed" "fmt" "os" + "reflect" + "strings" "github.com/runreveal/lib/cli" "github.com/runreveal/lib/loader" @@ -134,6 +136,63 @@ func (g *Globals) Validate() error { return nil } +// ExtraHelp implements cli.HelpExtra. It uses loader.List and +// loader.Describe to show available types and their config fields, +// bridging the cli and loader packages without either knowing +// about the other. +func (g *Globals) ExtraHelp() string { + var b strings.Builder + b.WriteString("\nAvailable source types:\n") + describeLoaderTypes[Source](&b) + b.WriteString("\nAvailable cache types:\n") + describeLoaderTypes[Cache](&b) + return b.String() +} + +// describeLoaderTypes lists registered loader types for T with their +// config fields (derived from JSON struct tags on the builder). +func describeLoaderTypes[T any](b *strings.Builder) { + for _, name := range loader.List[T]() { + builder, ok := loader.Describe[T](name) + if !ok { + fmt.Fprintf(b, " %s\n", name) + continue + } + fields := describeFields(builder) + if len(fields) == 0 { + fmt.Fprintf(b, " %s\n", name) + } else { + fmt.Fprintf(b, " %-12s %s\n", name, strings.Join(fields, ", ")) + } + } +} + +// describeFields reflects on a struct's JSON tags to produce +// "name (type)" descriptions for each exported field, skipping "type". +func describeFields(v any) []string { + t := reflect.TypeOf(v) + if t.Kind() == reflect.Ptr { + t = t.Elem() + } + if t.Kind() != reflect.Struct { + return nil + } + var out []string + for i := 0; i < t.NumField(); i++ { + f := t.Field(i) + tag := f.Tag.Get("json") + if tag == "" || tag == "-" { + continue + } + name, _, _ := strings.Cut(tag, ",") + if name == "type" { + continue + } + out = append(out, fmt.Sprintf("%s (%s)", name, f.Type.Name())) + } + return out +} + // --------------------------------------------------------------------------- // serve: uses initialized resources from Globals // --------------------------------------------------------------------------- diff --git a/cli/help.go b/cli/help.go index 21b1a8d..ca8e5ad 100644 --- a/cli/help.go +++ b/cli/help.go @@ -106,6 +106,16 @@ func printCommandHelp(w io.Writer, appName, path, desc string, handler Runnable, fmt.Fprintf(w, "Flags:\n") } fmt.Fprintf(w, " -h, --help show help\n") + + // Append extra help from handler and/or globals if they implement HelpExtra. + if he, ok := handler.(HelpExtra); ok { + fmt.Fprint(w, he.ExtraHelp()) + } + if globals != nil { + if he, ok := globals.(HelpExtra); ok { + fmt.Fprint(w, he.ExtraHelp()) + } + } } func printFlagDefs(w io.Writer, defs []*flagDef) { diff --git a/loader/loader.go b/loader/loader.go index 21e316a..2d6a91a 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -6,6 +6,7 @@ import ( "os" "reflect" "regexp" + "sort" "sync" "github.com/segmentio/encoding/json" @@ -83,6 +84,53 @@ func Register[T any](name string, factory func() Builder[T]) { registryForType.m[name] = factory } +// List returns the sorted names of all registered implementations for type T. +func List[T any]() []string { + typ := new(T) + typStr := reflect.TypeOf(typ).String() + + registry.RLock() + defer registry.RUnlock() + + raw, ok := registry.m[typStr] + if !ok { + return nil + } + reg := raw.(*Registry[T]) + reg.RLock() + defer reg.RUnlock() + + names := make([]string, 0, len(reg.m)) + for name := range reg.m { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// Describe returns a zero-value Builder for the named type, useful for +// introspecting config fields (e.g. reflecting on JSON struct tags for +// help output). Returns nil, false if the name is not registered. +func Describe[T any](name string) (Builder[T], bool) { + typ := new(T) + typStr := reflect.TypeOf(typ).String() + + registry.RLock() + raw, ok := registry.m[typStr] + registry.RUnlock() + if !ok { + return nil, false + } + reg := raw.(*Registry[T]) + reg.RLock() + factory, ok := reg.m[name] + reg.RUnlock() + if !ok { + return nil, false + } + return factory(), true +} + // Loader is a struct which can dyanmically unmarshal any type T type Loader[T any] struct { Builder[T] From 60209db8e3fc14e5708b2bc5b3cd8ac7240574a2 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 15:52:16 -0700 Subject: [PATCH 20/24] cli: flatten example config onto Globals, remove config:"." pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Config fields (Sources, Cache, Server) now live directly on Globals with individual config:"key" tags instead of being nested in an AppConfig wrapper with config:".". Clearer, no indirection. Also removed the empty Configure() on ServeCmd — the addr fallback to g.Server.Addr is handled directly in Run. --- cli/example/main.go | 51 ++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/cli/example/main.go b/cli/example/main.go index c314dfa..25a663a 100644 --- a/cli/example/main.go +++ b/cli/example/main.go @@ -84,43 +84,47 @@ type MemoryCache struct{ maxSize int } func (m *MemoryCache) Name() string { return fmt.Sprintf("memory(max=%d)", m.maxSize) } // --------------------------------------------------------------------------- -// AppConfig: loaded from config.json via config:"." on Globals +// ServerConfig: plain struct loaded from config file // --------------------------------------------------------------------------- -type AppConfig struct { - Sources []loader.Loader[Source] `json:"sources"` - Cache loader.Loader[Cache] `json:"cache"` - Server ServerConfig `json:"server"` -} - type ServerConfig struct { Addr string `json:"addr"` } // --------------------------------------------------------------------------- -// Globals: shared flags + config + initialized resources +// Globals: shared flags + config fields loaded directly from config file // --------------------------------------------------------------------------- +// Globals holds CLI flags, config-file-driven fields, and initialized +// resources. Config fields use config:"key" tags to pull their section +// from the config file — no wrapper struct needed. type Globals struct { - Verbose bool `cli:"verbose,v" usage:"enable verbose output"` - Config string `cli:"config,c" usage:"config file path" default:"config.json"` - Cfg AppConfig ` config:"."` - - // Initialized resources + // CLI flags + Verbose bool `cli:"verbose,v" usage:"enable verbose output"` + Config string `cli:"config,c" usage:"config file path" default:"config.json"` + + // Config file sections — each field maps to a top-level key + Sources []loader.Loader[Source] `config:"sources"` + Cache loader.Loader[Cache] `config:"cache"` + Server ServerConfig `config:"server"` + + // Initialized in Configure(), used by commands via GlobalsFromContext. + // These are runtime state, not config — but they live here because + // Globals is the natural singleton for the process. sources []Source cache Cache } func (g *Globals) Configure() error { - for _, src := range g.Cfg.Sources { + for _, src := range g.Sources { s, err := src.Configure() if err != nil { return fmt.Errorf("configuring source: %w", err) } g.sources = append(g.sources, s) } - if g.Cfg.Cache.Builder != nil { - c, err := g.Cfg.Cache.Configure() + if g.Cache.Builder != nil { + c, err := g.Cache.Configure() if err != nil { return fmt.Errorf("configuring cache: %w", err) } @@ -201,18 +205,18 @@ type ServeCmd struct { Addr string `cli:"addr,a" usage:"listen address"` } -func (s *ServeCmd) Configure() error { - // Apply server config from file as default if --addr not set. - return nil -} - func (s *ServeCmd) Run(ctx context.Context, args []string) error { g := cli.GlobalsFromContext[Globals](ctx) if g == nil { return fmt.Errorf("globals not available") } - fmt.Printf("server addr: %s\n", s.Addr) + addr := s.Addr + if addr == "" { + addr = g.Server.Addr + } + + fmt.Printf("server addr: %s\n", addr) fmt.Printf("sources:\n") for _, src := range g.sources { fmt.Printf(" - %s\n", src.Name()) @@ -247,7 +251,6 @@ func (l *ListSourcesCmd) Run(ctx context.Context, args []string) error { func main() { globals := &Globals{} - serveCmd := &ServeCmd{} app := cli.New("example", "Example CLI demonstrating cli + loader", cli.WithVersion("1.0.0"), @@ -257,7 +260,7 @@ func main() { ) app.AddCommand( - cli.Command("serve", "Start the HTTP server", serveCmd), + cli.Command("serve", "Start the HTTP server", &ServeCmd{}), cli.Command("list-sources", "List configured sources", &ListSourcesCmd{}, cli.WithArgs(cli.NoArgs), ), From bdd5ab401545255659e0d876a54cdac7520664c2 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 15:59:25 -0700 Subject: [PATCH 21/24] cli: use loader.Helper for self-describing config types in help MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builder types implement Help() string to provide their own description and example config. No reflection — each type owns its documentation. loader: add Helper interface checked via type assertion from Describe. --- cli/example/main.go | 51 ++++++++++++++++----------------------------- loader/loader.go | 7 +++++++ 2 files changed, 25 insertions(+), 33 deletions(-) diff --git a/cli/example/main.go b/cli/example/main.go index 25a663a..6dcbca7 100644 --- a/cli/example/main.go +++ b/cli/example/main.go @@ -7,7 +7,6 @@ import ( _ "embed" "fmt" "os" - "reflect" "strings" "github.com/runreveal/lib/cli" @@ -40,6 +39,10 @@ func (w *WebhookConfig) Configure() (Source, error) { return &WebhookSource{path: w.Path}, nil } +func (w *WebhookConfig) Help() string { + return `Receives events via HTTP webhooks. Example: {"type": "webhook", "path": "/hooks/github"}` +} + type WebhookSource struct{ path string } func (w *WebhookSource) Name() string { return "webhook:" + w.path } @@ -54,6 +57,10 @@ func (s *SyslogConfig) Configure() (Source, error) { return &SyslogSource{addr: s.Addr}, nil } +func (s *SyslogConfig) Help() string { + return `Listens for syslog messages over UDP. Example: {"type": "syslog", "addr": ":514"}` +} + type SyslogSource struct{ addr string } func (s *SyslogSource) Name() string { return "syslog:" + s.addr } @@ -79,6 +86,10 @@ func (m *MemoryCacheConfig) Configure() (Cache, error) { return &MemoryCache{maxSize: m.MaxSize}, nil } +func (m *MemoryCacheConfig) Help() string { + return `In-memory LRU cache. Example: {"type": "memory", "max_size": 1000}` +} + type MemoryCache struct{ maxSize int } func (m *MemoryCache) Name() string { return fmt.Sprintf("memory(max=%d)", m.maxSize) } @@ -153,8 +164,9 @@ func (g *Globals) ExtraHelp() string { return b.String() } -// describeLoaderTypes lists registered loader types for T with their -// config fields (derived from JSON struct tags on the builder). +// describeLoaderTypes lists registered loader types for T. If a type's +// builder implements loader.Helper, its Help() output is shown. Otherwise +// just the type name is listed. func describeLoaderTypes[T any](b *strings.Builder) { for _, name := range loader.List[T]() { builder, ok := loader.Describe[T](name) @@ -162,39 +174,12 @@ func describeLoaderTypes[T any](b *strings.Builder) { fmt.Fprintf(b, " %s\n", name) continue } - fields := describeFields(builder) - if len(fields) == 0 { - fmt.Fprintf(b, " %s\n", name) + if h, ok := builder.(loader.Helper); ok { + fmt.Fprintf(b, " %s — %s\n", name, h.Help()) } else { - fmt.Fprintf(b, " %-12s %s\n", name, strings.Join(fields, ", ")) - } - } -} - -// describeFields reflects on a struct's JSON tags to produce -// "name (type)" descriptions for each exported field, skipping "type". -func describeFields(v any) []string { - t := reflect.TypeOf(v) - if t.Kind() == reflect.Ptr { - t = t.Elem() - } - if t.Kind() != reflect.Struct { - return nil - } - var out []string - for i := 0; i < t.NumField(); i++ { - f := t.Field(i) - tag := f.Tag.Get("json") - if tag == "" || tag == "-" { - continue - } - name, _, _ := strings.Cut(tag, ",") - if name == "type" { - continue + fmt.Fprintf(b, " %s\n", name) } - out = append(out, fmt.Sprintf("%s (%s)", name, f.Type.Name())) } - return out } // --------------------------------------------------------------------------- diff --git a/loader/loader.go b/loader/loader.go index 2d6a91a..fe550d4 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -131,6 +131,13 @@ func Describe[T any](name string) (Builder[T], bool) { return factory(), true } +// Helper is optionally implemented by Builder types to provide a +// human-readable description of their configuration. Used by CLI +// tools to show available config options in help output. +type Helper interface { + Help() string +} + // Loader is a struct which can dyanmically unmarshal any type T type Loader[T any] struct { Builder[T] From 6cadb0a9334791a01226bcc692093d9d42ea4847 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 16:12:26 -0700 Subject: [PATCH 22/24] cli: fix completion output going to stderr instead of stdout Completion scripts, __complete output, and defcon all write to stdout (via a.stdout) since shells read completions from stdout. Errors and help continue to go to stderr (via a.output). WithOutput sets both for testing convenience. --- cli/cli.go | 11 +++++++---- cli/complete.go | 12 +++++++----- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index dc40cb5..b0a4dd3 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -173,9 +173,10 @@ func WithDefaultConfigCommand(name string) AppOption { return func(a *App) { a.defaultConfigCmd = name } } -// WithOutput sets the writer for help/error output (default: os.Stderr). +// WithOutput sets the writer for all output: help/errors (normally stderr) +// and completion/defcon (normally stdout). Useful for testing. func WithOutput(w io.Writer) AppOption { - return func(a *App) { a.output = w } + return func(a *App) { a.output = w; a.stdout = w } } // App is the top-level CLI application. @@ -189,7 +190,8 @@ type App struct { defaultConfigCmd string middlewares []Middleware children []Node - output io.Writer + output io.Writer // stderr: errors, help + stdout io.Writer // stdout: completion, defcon } // New creates a new App. @@ -198,6 +200,7 @@ func New(name, desc string, opts ...AppOption) *App { name: name, desc: desc, output: os.Stderr, + stdout: os.Stdout, } for _, opt := range opts { opt(a) @@ -473,7 +476,7 @@ func (a *App) handleDefaultConfig() int { fmt.Fprintf(a.output, "no default config registered\n") return 1 } - if _, err := a.output.Write(a.defaultConfig); err != nil { + if _, err := a.stdout.Write(a.defaultConfig); err != nil { fmt.Fprintf(a.output, "error writing config: %s\n", err) return 1 } diff --git a/cli/complete.go b/cli/complete.go index 526ce73..de1c9dd 100644 --- a/cli/complete.go +++ b/cli/complete.go @@ -44,13 +44,15 @@ func (a *App) handleCompletionScript(args []string) int { ) return 1 } + // Completion scripts and __complete output go to stdout, not + // a.output (which defaults to stderr). The shell reads stdout. switch args[0] { case "bash": - writeBashCompletion(a.output, a.name) + writeBashCompletion(a.stdout, a.name) case "zsh": - writeZshCompletion(a.output, a.name) + writeZshCompletion(a.stdout, a.name) case "fish": - writeFishCompletion(a.output, a.name) + writeFishCompletion(a.stdout, a.name) default: fmt.Fprintf( a.output, @@ -66,9 +68,9 @@ func (a *App) handleCompleteRequest(args []string) { completions := a.computeCompletions(context.Background(), args) for _, c := range completions { if c.Description != "" { - fmt.Fprintf(a.output, "%s\t%s\n", c.Value, c.Description) + fmt.Fprintf(a.stdout, "%s\t%s\n", c.Value, c.Description) } else { - fmt.Fprintf(a.output, "%s\n", c.Value) + fmt.Fprintf(a.stdout, "%s\n", c.Value) } } } From 942ad5f09f8cec221675be72ff622275e8b820a8 Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 16:16:20 -0700 Subject: [PATCH 23/24] cli: show defcon hint in help output, consolidate defconCmd logic App help now shows "Use myapp defcon to print the default configuration" when a default config is registered. Extracted defconCmd() helper to deduplicate the command name logic. --- cli/cli.go | 22 +++++++++++++++------- cli/cli_test.go | 3 ++- cli/help.go | 5 ++++- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index b0a4dd3..3bfc3f4 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -244,11 +244,7 @@ func (a *App) run(ctx context.Context, args []string) (int, error) { if code, handled := a.handleCompletion(args); handled { return code, nil } - defconCmd := a.defaultConfigCmd - if defconCmd == "" { - defconCmd = "defcon" - } - if len(args) == 1 && args[0] == defconCmd { + if dc := a.defconCmd(); len(args) == 1 && args[0] == dc { return a.handleDefaultConfig(), nil } @@ -262,7 +258,7 @@ func (a *App) run(ctx context.Context, args []string) (int, error) { return 0, nil } if len(args) == 0 || (len(args) == 1 && (args[0] == "--help" || args[0] == "-h")) { - printAppHelp(a.output, a.name, a.desc, a.children, a.version) + printAppHelp(a.output, a.name, a.desc, a.children, a.version, a.defconCmd()) return 0, nil } @@ -270,7 +266,7 @@ func (a *App) run(ctx context.Context, args []string) (int, error) { if node == nil { // Unknown command fmt.Fprintf(a.output, "unknown command %q\n\n", args[0]) - printAppHelp(a.output, a.name, a.desc, a.children, a.version) + printAppHelp(a.output, a.name, a.desc, a.children, a.version, a.defconCmd()) return 1, nil } @@ -471,6 +467,18 @@ func buildChain(middlewares []Middleware, info CommandInfo, final func(context.C return chain } +// defconCmd returns the default config command name, or "" if no +// default config is registered. +func (a *App) defconCmd() string { + if len(a.defaultConfig) == 0 { + return "" + } + if a.defaultConfigCmd != "" { + return a.defaultConfigCmd + } + return "defcon" +} + func (a *App) handleDefaultConfig() int { if len(a.defaultConfig) == 0 { fmt.Fprintf(a.output, "no default config registered\n") diff --git a/cli/cli_test.go b/cli/cli_test.go index 4eeebd9..1cdcf02 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -1202,9 +1202,10 @@ func TestDefaultConfig_NotRegistered(t *testing.T) { var buf bytes.Buffer app := cli.New("app", "test", cli.WithOutput(&buf)) + // Without WithDefaultConfig, "defcon" is not a recognized command. code := app.Run(context.Background(), []string{"defcon"}) assert.Equal(t, 1, code) - assert.Contains(t, buf.String(), "no default config") + assert.Contains(t, buf.String(), "unknown command") } func TestExitError(t *testing.T) { diff --git a/cli/help.go b/cli/help.go index ca8e5ad..fe6d1b3 100644 --- a/cli/help.go +++ b/cli/help.go @@ -6,7 +6,7 @@ import ( "strings" ) -func printAppHelp(w io.Writer, appName, desc string, children []Node, version string) { +func printAppHelp(w io.Writer, appName, desc string, children []Node, version, defconCmd string) { if desc != "" { fmt.Fprintf(w, "%s - %s\n\n", appName, desc) } else { @@ -38,6 +38,9 @@ func printAppHelp(w io.Writer, appName, desc string, children []Node, version st if len(children) > 0 { fmt.Fprintf(w, "\nUse \"%s --help\" for more information.\n", appName) } + if defconCmd != "" { + fmt.Fprintf(w, "Use \"%s %s\" to print the default configuration.\n", appName, defconCmd) + } } func printGroupHelp(w io.Writer, appName, path, desc string, children []Node) { From 36575bc71235c29ab7e76c78d75d5eb7e27c65fe Mon Sep 17 00:00:00 2001 From: Alan Braithwaite Date: Mon, 6 Apr 2026 16:43:42 -0700 Subject: [PATCH 24/24] =?UTF-8?q?cli:=20fix=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20panic=20on=20bad=20Command=20opts,=20defcon=20arg?= =?UTF-8?q?=20handling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Command() now panics on unsupported option types instead of silently dropping them (catches misuse at init time) - defcon command matches on args[0] regardless of arg count, so "myapp defcon --help" works instead of falling through to routing - Add aliasing comment on reflect.go slice construction --- cli/cli.go | 4 +++- cli/reflect.go | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/cli/cli.go b/cli/cli.go index 3bfc3f4..b54c2ca 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -124,6 +124,8 @@ func Command(name, desc string, handler Runnable, opts ...any) Node { children = append(children, v) case CmdOption: v(&o) + default: + panic(fmt.Sprintf("cli.Command %q: unsupported option type %T", name, v)) } } return &commandNode{name: name, desc: desc, handler: handler, children: children, opts: o} @@ -244,7 +246,7 @@ func (a *App) run(ctx context.Context, args []string) (int, error) { if code, handled := a.handleCompletion(args); handled { return code, nil } - if dc := a.defconCmd(); len(args) == 1 && args[0] == dc { + if dc := a.defconCmd(); dc != "" && len(args) >= 1 && args[0] == dc { return a.handleDefaultConfig(), nil } diff --git a/cli/reflect.go b/cli/reflect.go index 528478a..871bea7 100644 --- a/cli/reflect.go +++ b/cli/reflect.go @@ -33,6 +33,8 @@ func scanFieldsWithIndex(t reflect.Type, prefix []int) ([]fieldInfo, error) { var fields []fieldInfo for i := 0; i < t.NumField(); i++ { f := t.Field(i) + // Copy prefix into a new slice before appending i to avoid + // aliasing the backing array across loop iterations. idx := append(append([]int{}, prefix...), i) // Handle embedded structs (anonymous fields)