From 795c4f6b29797989b1c43ab4c87fd22b9dac181a Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 28 Oct 2026 20:32:38 +0100 Subject: [PATCH] feat(bonfire): add FlagBool and read every bare switch with it (IN-04, quick-261028-rqd) --- docs/console/writing-commands.md | 8 ++--- modules/bonfire/README.md | 5 +-- modules/bonfire/command.go | 18 +++++++++++ modules/bonfire/command_test.go | 37 +++++++++++++++++++++++ modules/bonfire/example_test.go | 4 +-- modules/bonfire/list.go | 4 +-- modules/cabana/commands.go | 2 +- modules/cabana/token_commands.go | 8 ++--- modules/conga/commands.go | 15 +++------ modules/lagoon/commands.go | 23 +++----------- modules/lagoon/keygen.go | 2 +- modules/lighthouse/centrifugo/commands.go | 6 +--- modules/surf/routelist_command.go | 2 +- modules/surf/summary_test.go | 2 +- scripts/check-phase19.8.sh | 4 +-- 15 files changed, 86 insertions(+), 54 deletions(-) diff --git a/docs/console/writing-commands.md b/docs/console/writing-commands.md index 46043fa8..479e1d81 100644 --- a/docs/console/writing-commands.md +++ b/docs/console/writing-commands.md @@ -14,11 +14,11 @@ A `bonfire.Command` has a name, a description, its positional arguments and flag - The name is in `namespace:verb` form, such as `blog:publish`. Plugin commands must use this form; only a few framework commands have bare names. - Each `bonfire.Arg` is a positional argument with a name, a description and a `Required` marker. The usage line shows required arguments as `` and optional ones as `[name]`. -- Each `bonfire.Flag` is a string flag. Set `bonfire.Flag.Bare` for a switch such as `--dry-run` that stores `true` when given alone, and `bonfire.Flag.Repeatable` for a flag that can be given several times. +- Each `bonfire.Flag` is a string flag. Set `bonfire.Flag.Bare` for a switch such as `--dry-run` that stores `true` when given alone, and read it with `bonfire.FlagBool`, which takes the values `strconv.ParseBool` accepts, so `--dry-run=1` and `--dry-run=false` work too. Set `bonfire.Flag.Repeatable` for a flag that can be given several times. - `bonfire.Command.Run` receives the context, a `bonfire.Input` and a `bonfire.Output`. - `bonfire.Command.Long` is optional extended help, shown under "Help:" by `help ` and ` --help`. `bonfire.Command.Examples` and `bonfire.Command.Hidden` are described below. Set `bonfire.Command.Passthrough` for a command that forwards to another program: flag parsing is turned off, so every argument after the command name, including `--help`, `-h` and `--`, reaches `bonfire.Input.Args` unchanged. Declared flags then appear in the usage text only, and declared arguments are not count-checked. -Read arguments with `bonfire.Input.Argument`, scalar and bare flags with `bonfire.Input.Flag`, and repeatable flags with `bonfire.Input.Flags`, which returns the values in the order given: +Read arguments with `bonfire.Input.Argument`, scalar flags with `bonfire.Input.Flag`, bare switches with `bonfire.FlagBool`, and repeatable flags with `bonfire.Input.Flags`, which returns the values in the order given: ```go src=modules/bonfire/example_test.go#ExampleCatalog publish := bonfire.Command{ @@ -31,8 +31,8 @@ publish := bonfire.Command{ }, Run: func(ctx context.Context, in bonfire.Input, out bonfire.Output) error { slug, _ := in.Argument("slug") - dryRun, _ := in.Flag("dry-run") - out.Printf("publish %s, tags %v, dry run %s\n", slug, in.Flags("tag"), dryRun) + dryRun := bonfire.FlagBool(in, "dry-run") + out.Printf("publish %s, tags %v, dry run %v\n", slug, in.Flags("tag"), dryRun) return nil }, } diff --git a/modules/bonfire/README.md b/modules/bonfire/README.md index 4f59628d..6d346f48 100644 --- a/modules/bonfire/README.md +++ b/modules/bonfire/README.md @@ -25,7 +25,7 @@ Use cases: - Command values with a description, positional arguments (`bonfire.Arg`) and string flags (`bonfire.Flag`), collected from plugins or the tool itself. - Command name validation: plugin commands must use the `namespace:verb` form (for example `blog:import`); `build`, `dev`, `new`, `serve` and `migrate` are the only bare names accepted. Invalid names make `bonfire.NewRoot` fail with `bonfire.ErrCommandName`. - Usage strings and argument-count checks derived from the declared arguments (`` for required, `[name]` for optional). -- Scalar flags, bare flags (`bonfire.Flag.Bare`, so `--force` alone stores `true`) and ordered repeatable flags (`bonfire.Flag.Repeatable`, read back through `bonfire.Input.Flags`). +- Scalar flags, bare flags (`bonfire.Flag.Bare`, so `--force` alone stores `true`, read as a switch with `bonfire.FlagBool`, which also accepts `--force=1` and `--force=false`) and ordered repeatable flags (`bonfire.Flag.Repeatable`, read back through `bonfire.Input.Flags`). - Styled status lines: `bonfire.Output.Info`, `bonfire.Output.Success`, `bonfire.Output.Warning` and `bonfire.Output.Error` (the last one writes to the error stream). - Widgets on the charm stack, ported from SSU's terminal UI: box-drawn tables (`bonfire.Output.Table`, lipgloss), a spinner around a function (`bonfire.Output.Spinner`), a gradient progress bar (`bonfire.Output.Progress`, whose `bonfire.Progress` also implements `bonfire.ItemProgress`) and a multi-row process view for bulk loops (`bonfire.Process`), each a bubbletea program that keeps stdin and signals to the command: Ctrl+C or SIGTERM during a view restores the cursor and the terminal, then stops the command as before (or reaches its own signal handler). Log lines, and output the command prints through the console that started the view, appear above a running view (`bonfire.PrintAbove`); other consoles (a `bonfire.Call` buffer, a console on a file) keep writing to their own writers. When the context given to `bonfire.Process` is cancelled, the view ends with its last frame and later lines print plainly below it. Table cells, the item names and errors of `bonfire.Process` (in the process view and in its plain lines), the process view's operation label, and failure lines are stripped of control characters (`bonfire.Sanitize`). - Prompts: `bonfire.Output.Ask`, `bonfire.Output.Confirm` and `bonfire.Output.Secret` use huh fields on an interactive terminal, and `bonfire.Output.Choice` an arrow-key selector with a confirmation screen. Off a terminal they read lines from stdin as before (a piped password still works), return their defaults when input ends, and `bonfire.Output.Confirm` returns its default without asking. A cancelled prompt returns `bonfire.ErrAborted`. `bonfire.ConfirmProceed` asks before a destructive step only on an interactive terminal. @@ -74,7 +74,7 @@ func main() { }, Run: func(ctx context.Context, in bonfire.Input, out bonfire.Output) error { url, _ := in.Argument("url") - _, dryRun := in.Flag("dry-run") + dryRun := bonfire.FlagBool(in, "dry-run") return out.Spinner("Importing "+url, func() error { out.Table([]string{"Tag"}, [][]string{{"news"}}) if dryRun { @@ -117,6 +117,7 @@ if catalog.Has("blog:import") { | `bonfire.Flag` | A string flag; `bonfire.Flag.Bare` allows the flag without a value, `bonfire.Flag.Repeatable` makes it an ordered multi-value flag. | | `bonfire.Arg` | A positional argument with a name, description and required marker. | | `bonfire.Input` | Parsed view handed to `bonfire.Command.Run`: `bonfire.Input.Args`, `bonfire.Input.Argument`, `bonfire.Input.Flag` and `bonfire.Input.Flags`. | +| `bonfire.FlagBool` | Reports whether a bare switch is on: set and true under `strconv.ParseBool` (`--x`, `--x=1`, `--x=true`); unset, empty or unparsable (`--x=yes`) values and a nil input are false. | | `bonfire.Output` | Injected console: printing, status lines, tables, spinner, progress bar and prompts. | | `bonfire.Progress` | A progress bar advanced from inside `bonfire.Output.Progress`. | | `bonfire.NewRoot` | Builds the Cobra root command for a binary from a slice of commands, using the process stdin, with the global options, the `list` command and the command list on a bare call; takes `bonfire.RootOption` values. When the output is the process stdout, the error block goes to stderr. | diff --git a/modules/bonfire/command.go b/modules/bonfire/command.go index 99bbbef6..5e017517 100644 --- a/modules/bonfire/command.go +++ b/modules/bonfire/command.go @@ -3,6 +3,7 @@ package bonfire import ( "context" "errors" + "strconv" "strings" "github.com/spf13/cobra" @@ -134,6 +135,23 @@ func (in cobraInput) Flag(name string) (string, bool) { return val, true } +// FlagBool reports whether the bare switch name is on: the flag is set and +// its value parses with strconv.ParseBool as true (--name, --name=true, +// --name=1, --name=t, TRUE, True). Unset, empty and unparsable values +// (--name=yes) are false, and a nil Input has no flags. Read every Bare +// flag with it, so --json=1 works like --json. +func FlagBool(in Input, name string) bool { + if in == nil { + return false + } + v, ok := in.Flag(name) + if !ok { + return false + } + on, err := strconv.ParseBool(v) + return err == nil && on +} + func validCommandName(name string) bool { switch name { case "build", "dev", "new", "serve", "migrate": diff --git a/modules/bonfire/command_test.go b/modules/bonfire/command_test.go index 97e65363..42b1e199 100644 --- a/modules/bonfire/command_test.go +++ b/modules/bonfire/command_test.go @@ -115,3 +115,40 @@ func TestHelpShowsLongOrDescription(t *testing.T) { }) } } + +// TestFlagBool (IN-04): FlagBool reads a bare switch with +// strconv.ParseBool; unset, empty or unparsable values are false, and a +// nil Input has no flags. +func TestFlagBool(t *testing.T) { + cases := []struct { + args []string + want bool + }{ + {nil, false}, + {[]string{"--x"}, true}, + {[]string{"--x=1"}, true}, + {[]string{"--x=t"}, true}, + {[]string{"--x=TRUE"}, true}, + {[]string{"--x=True"}, true}, + {[]string{"--x=false"}, false}, + {[]string{"--x=0"}, false}, + {[]string{"--x="}, false}, + {[]string{"--x=yes"}, false}, + } + for _, tc := range cases { + var got bool + cmd := Command{Name: "acme:switch", Flags: []Flag{{Name: "x", Bare: true}}, Run: func(_ context.Context, in Input, _ Output) error { + got = FlagBool(in, "x") + return nil + }} + if err := Call(context.Background(), []Command{cmd}, "acme:switch", tc.args, nil); err != nil { + t.Fatalf("%q: %v", tc.args, err) + } + if got != tc.want { + t.Errorf("FlagBool with %q = %v, want %v", tc.args, got, tc.want) + } + } + if FlagBool(nil, "x") { + t.Error("FlagBool(nil) = true") + } +} diff --git a/modules/bonfire/example_test.go b/modules/bonfire/example_test.go index a1709bf3..ee8c3b06 100644 --- a/modules/bonfire/example_test.go +++ b/modules/bonfire/example_test.go @@ -38,8 +38,8 @@ func ExampleCatalog() { }, Run: func(ctx context.Context, in bonfire.Input, out bonfire.Output) error { slug, _ := in.Argument("slug") - dryRun, _ := in.Flag("dry-run") - out.Printf("publish %s, tags %v, dry run %s\n", slug, in.Flags("tag"), dryRun) + dryRun := bonfire.FlagBool(in, "dry-run") + out.Printf("publish %s, tags %v, dry run %v\n", slug, in.Flags("tag"), dryRun) return nil }, } diff --git a/modules/bonfire/list.go b/modules/bonfire/list.go index 8669e216..83eed176 100644 --- a/modules/bonfire/list.go +++ b/modules/bonfire/list.go @@ -62,11 +62,11 @@ func usageName(name string) string { func (st *rootState) listCommand() Command { c := builtinCommand("list") c.Run = func(_ context.Context, in Input, _ Output) error { - all, _ := in.Flag("all") + all := FlagBool(in, "all") format, _ := in.Flag("format") switch format { case "", "txt": - return st.renderList(st.globals(), all == "true") + return st.renderList(st.globals(), all) case "json": return st.renderJSON() default: diff --git a/modules/cabana/commands.go b/modules/cabana/commands.go index 17abcec2..acfd4029 100644 --- a/modules/cabana/commands.go +++ b/modules/cabana/commands.go @@ -82,7 +82,7 @@ func adminCreate(ctx context.Context, app *backpack.App, in bonfire.Input, out b login = email } roleCode := strings.TrimSpace(flagValue(in, "role")) - superuser := flagValue(in, "superuser") == "true" + superuser := bonfire.FlagBool(in, "superuser") return withAdminDB(ctx, app, func(gdb *gorm.DB) error { return gdb.WithContext(ctx).Transaction(func(tx *gorm.DB) error { roleID, err := roleIDByCode(tx, roleCode) diff --git a/modules/cabana/token_commands.go b/modules/cabana/token_commands.go index 26873532..2097aa62 100644 --- a/modules/cabana/token_commands.go +++ b/modules/cabana/token_commands.go @@ -147,7 +147,7 @@ func adminTokenCreate(ctx context.Context, app *backpack.App, in bonfire.Input, if details != nil { return fmt.Errorf("cabana: %s", firstDetail(details)) } - if flagValue(in, "json") == "true" && row != nil { + if bonfire.FlagBool(in, "json") && row != nil { return printJSON(out, map[string]any{ "id": row.ID, "name": row.Name, @@ -163,7 +163,7 @@ func adminTokenCreate(ctx context.Context, app *backpack.App, in bonfire.Input, func adminTokenList(ctx context.Context, app *backpack.App, in bonfire.Input, out bonfire.Output) error { identifier := argValue(in, "identifier") - showRevoked := flagValue(in, "revoked") == "true" + showRevoked := bonfire.FlagBool(in, "revoked") return withAdminDB(ctx, app, func(gdb *gorm.DB) error { q := gdb.WithContext(ctx).Model(&BackendAPIToken{}).Order("created_at DESC").Order("id DESC") if identifier != "" { @@ -214,7 +214,7 @@ func adminTokenList(ctx context.Context, app *backpack.App, in bonfire.Input, ou formatTimePtr(row.RevokedAt), }) } - if flagValue(in, "json") == "true" { + if bonfire.FlagBool(in, "json") { return printJSON(out, list) } out.Table(headers, table) @@ -296,7 +296,7 @@ func adminTokenScopes(ctx context.Context, app *backpack.App, in bonfire.Input, "permissions": permissions, }) } - if flagValue(in, "json") == "true" { + if bonfire.FlagBool(in, "json") { return printJSON(out, list) } out.Table(headers, rows) diff --git a/modules/conga/commands.go b/modules/conga/commands.go index 40e703da..399d3bf3 100644 --- a/modules/conga/commands.go +++ b/modules/conga/commands.go @@ -8,7 +8,6 @@ import ( "os" "os/signal" "slices" - "strconv" "strings" "syscall" "time" @@ -84,10 +83,8 @@ scheduled queue.`, Bare: true, }}, Run: func(ctx context.Context, in bonfire.Input, out bonfire.Output) error { - if v, ok := in.Flag("once"); ok { - if once, _ := strconv.ParseBool(v); once { - return scheduleRunOnce(ctx, app, plugins, scheduleNow(), out) - } + if bonfire.FlagBool(in, "once") { + return scheduleRunOnce(ctx, app, plugins, scheduleNow(), out) } return withDB(ctx, app, func() error { return runScheduler(ctx, app, plugins, out) @@ -382,12 +379,8 @@ func clearQueue(ctx context.Context, app *backpack.App, queue string, out bonfir // --force is set (D-28). Off a terminal, under -n or -q and in // bonfire.Call it never asks. A "no" returns a Reported error (exit 1). func confirmProduction(app *backpack.App, in bonfire.Input, out bonfire.Output, question string) error { - if in != nil { - if v, ok := in.Flag("force"); ok { - if force, _ := strconv.ParseBool(v); force { - return nil - } - } + if bonfire.FlagBool(in, "force") { + return nil } if app == nil || app.Config == nil || app.Config.Environment() != "production" { return nil diff --git a/modules/lagoon/commands.go b/modules/lagoon/commands.go index acb472c6..569c5e97 100644 --- a/modules/lagoon/commands.go +++ b/modules/lagoon/commands.go @@ -29,26 +29,13 @@ var ( // forceDescription describes the --force flag of the destructive commands. const forceDescription = "Skip the confirmation asked in production on a terminal" -// flagOn reports whether the bare flag name is set; a nil Input has none. -func flagOn(in bonfire.Input, name string) bool { - if in == nil { - return false - } - v, ok := in.Flag(name) - if !ok { - return false - } - on, _ := strconv.ParseBool(v) - return on -} - // confirmProduction asks question before a destructive step when the // environment is production and out is an interactive terminal, unless // --force is set (D-28). Off a terminal, under -n or -q and in // bonfire.Call it never asks, so deploys over SSH proceed as before. A // "no" prints cancelled and returns a Reported error (exit 1). func confirmProduction(app *backpack.App, in bonfire.Input, out bonfire.Output, question, cancelled string) error { - if flagOn(in, "force") || app == nil || app.Config == nil || app.Config.Environment() != "production" { + if bonfire.FlagBool(in, "force") || app == nil || app.Config == nil || app.Config.Environment() != "production" { return nil } ok, err := bonfire.ConfirmProceed(out, question) @@ -156,7 +143,7 @@ asks.`, if err != nil { return err } - if flagOn(in, "pretend") { + if bonfire.FlagBool(in, "pretend") { for _, ref := range pending { out.Println(ref.Plugin + ": " + ref.ID) } @@ -242,7 +229,7 @@ question.`, if err != nil { return err } - if flagOn(in, "pretend") { + if bonfire.FlagBool(in, "pretend") { if id == "" { out.Info(fmt.Sprintf("%s has no applied migration to roll back", plugin)) return nil @@ -283,7 +270,7 @@ sets with its table and whether it is applied, for scripts.`, if err != nil { return err } - if flagOn(in, "json") { + if bonfire.FlagBool(in, "json") { pending, err := PendingMigrations(gdb, plugins) if err != nil { return err @@ -338,7 +325,7 @@ asks first; --force skips the question.`, } before := time.Now().Add(-time.Duration(days) * 24 * time.Hour) return withDB(ctx, app, func(gdb *gorm.DB) error { - if flagOn(in, "pretend") { + if bonfire.FlagBool(in, "pretend") { var n int64 if err := gdb.WithContext(ctx).Model(&DeferredBinding{}).Where("created_at < ?", before).Count(&n).Error; err != nil { return fmt.Errorf("lagoon: count deferred bindings: %w", err) diff --git a/modules/lagoon/keygen.go b/modules/lagoon/keygen.go index cd0476cf..4ed3a576 100644 --- a/modules/lagoon/keygen.go +++ b/modules/lagoon/keygen.go @@ -43,7 +43,7 @@ first. With -q only the key is printed; --json prints {"key": "..."}.`, return err } switch { - case flagOn(in, "json"): + case bonfire.FlagBool(in, "json"): return printJSON(out, map[string]string{"key": key}) case bonfire.IsQuiet(out): out.Println(key) diff --git a/modules/lighthouse/centrifugo/commands.go b/modules/lighthouse/centrifugo/commands.go index bfd666a3..61d02af2 100644 --- a/modules/lighthouse/centrifugo/commands.go +++ b/modules/lighthouse/centrifugo/commands.go @@ -61,11 +61,7 @@ monitoring scripts.`, cfg = LoadConfig(nil) } client := NewClient(cfg, nil) - asJSON := false - if in != nil { - v, _ := in.Flag("json") - asJSON = v == "true" - } + asJSON := bonfire.FlagBool(in, "json") if asJSON { info := client.DebugInfo() res := healthJSON{APIURL: info.APIURL, Enabled: info.Enabled, APIKeySet: info.APIKeySet} diff --git a/modules/surf/routelist_command.go b/modules/surf/routelist_command.go index 73406a46..a048769a 100644 --- a/modules/surf/routelist_command.go +++ b/modules/surf/routelist_command.go @@ -47,7 +47,7 @@ scripts.`, } infos := r.Routes() if in != nil { - if v, _ := in.Flag(jsonFlagName); v == "true" { + if bonfire.FlagBool(in, jsonFlagName) { list := make([]routeJSON, 0, len(infos)) for _, rt := range infos { mw := rt.Middleware diff --git a/modules/surf/summary_test.go b/modules/surf/summary_test.go index df10eb52..757bef34 100644 --- a/modules/surf/summary_test.go +++ b/modules/surf/summary_test.go @@ -326,7 +326,7 @@ func TestServeGracefulNil(t *testing.T) { func TestRouteListJSON(t *testing.T) { app := backpack.New(writeHTTPConfig(t, "body_limits:\n default_bytes: 1024\n upload_bytes: 1024\n")) cmd := RouteListCommand(app, []party.Plugin{blogPostsPlugin{}}) - for _, args := range [][]string{{"--json"}, {"--json", "-q"}} { + for _, args := range [][]string{{"--json"}, {"--json", "-q"}, {"--json=1"}} { var buf bytes.Buffer if err := bonfire.Call(context.Background(), []bonfire.Command{cmd}, "route:list", args, &buf); err != nil { t.Fatal(err) diff --git a/scripts/check-phase19.8.sh b/scripts/check-phase19.8.sh index 7e385bda..272cc17e 100755 --- a/scripts/check-phase19.8.sh +++ b/scripts/check-phase19.8.sh @@ -131,9 +131,9 @@ BONFIRE_TESTS=( TestProcessViewStripsControl TestTableStripsControl # quick 261022-4z8 (re-review fixes): IN-15, WR-09. TestNestedProcessBuildsNoProgram TestPrintAboveAfterCancel - # quick 261028-rqd (info findings): IN-03, IN-06, IN-02. + # quick 261028-rqd (info findings): IN-03, IN-06, IN-02, IN-04. TestPluginErrorWordedUnknownCommand TestExecuteKeepsRootHelp - TestErrorBlockUsesParsedGlobals + TestErrorBlockUsesParsedGlobals TestFlagBool ) DOCSITE_TESTS=( # Plan 04: the docs checker reads past the global options.