fix(bonfire): read the =value forms of the global flags and use the parsed globals in the error block (IN-02, quick-261028-rqd)

modules/bonfire/README.md

@@ -41,7 +41,7 @@ Use cases:
 - Command state for helpers: `bonfire.GlobalsOf`, `bonfire.VerbosityOf`, `bonfire.IsQuiet` and `bonfire.Interactive` read the running command's global options from its `bonfire.Output`.
 - Passthrough commands (`bonfire.Command.Passthrough`) skip flag parsing, so every argument, including `--help`, reaches `bonfire.Input.Args` unchanged; this suits commands that forward to another program. `bonfire.Command.Long` adds extended help text that the help command shows instead of the description.
 - Artisan-style command list: a bare binary (or `--help`, or `list`) prints a version header (`bonfire.Version.Header`, read from the build information by `bonfire.BuildVersion`), the usage line, the global options and the commands, top-level names first and then one block per namespace, with aligned descriptions.
-- Global options on every command: `-v`/`-vv`/`-vvv` (`--verbose`), `-q` (`--quiet`, implies `-n`), `--ansi`/`--no-ansi` (last wins), `-n` (`--no-interaction`) and `--env`. `bonfire.PeekGlobals` reads them from the raw arguments before the root exists, so the generated main can pick the configuration environment and the log verbosity before plugins boot; `bonfire.OnGlobals` hooks receive the parsed `bonfire.Globals` before every command runs (roots built by `bonfire.Call` run no hook).
+- Global options on every command: `-v`/`-vv`/`-vvv` (`--verbose`), `-q` (`--quiet`, implies `-n`), `--ansi`/`--no-ansi` (last wins), `-n` (`--no-interaction`) and `--env`. `bonfire.PeekGlobals` reads them from the raw arguments before the root exists, so the generated main can pick the configuration environment and the log verbosity before plugins boot; it reads the `=value` forms the way pflag does (`--verbose=2`, `-v=3`, `--quiet=false`, `--ansi=false`), and a value-taking command flag followed by a global-looking token (`--name -v`) is read as the global before boot, while the command and its error block use cobra's reading; `bonfire.OnGlobals` hooks receive the parsed `bonfire.Globals` before every command runs (roots built by `bonfire.Call` run no hook).
 - Artisan-style help: `help <command>` and `<command> --help` print Description, Usage, Arguments, Options with `[default: ...]` and "(multiple values allowed)", the global options, Help (`bonfire.Command.Long`) and Examples (`bonfire.Command.Examples`, printed after `bin/<name>` or `summer`).
 - Hidden commands (`bonfire.Command.Hidden`) run but stay out of the list unless `list --all`.
 - JSON catalog: `list --format=json` prints every command, hidden ones flagged, with arguments, options, defaults and examples as a `bonfire.Listing` (`bonfire.Describe` builds it from a command slice).
@@ -125,7 +125,7 @@ if catalog.Has("blog:import") {
 | `bonfire.WithVersion` | Sets the `bonfire.Version` shown in the list header; without it the root reads `bonfire.BuildVersion`. |
 | `bonfire.OnGlobals` | Registers a function that receives the parsed `bonfire.Globals` before every command of the root runs. |
 | `bonfire.Globals` | The global options: `Verbosity` (0 to 3), `Quiet`, `Color` (a `bonfire.ColorMode`), `NoInteraction` and `Env`. |
-| `bonfire.PeekGlobals` | Reads `bonfire.Globals` from raw arguments without failing; stops at `--`. |
+| `bonfire.PeekGlobals` | Reads `bonfire.Globals` from raw arguments without failing, `=value` forms included; stops at `--`. |
 | `bonfire.Globals.Args` | Turns `bonfire.Globals` back into flags (`-vv`, `-q`, `-n`, `--ansi` or `--no-ansi`, `--env=<value>`) for forwarding. |
 | `bonfire.Version` | Application and framework versions of a binary (`App`, `Framework`, `Local`). |
 | `bonfire.BuildVersion` | Reads the `bonfire.Version` from the binary's build information. |

modules/bonfire/execute.go

@@ -56,10 +56,16 @@ func Execute(ctx context.Context, root *cobra.Command, args []string) int {
 			lipgloss.EnableLegacyWindowsANSI(f)
 		}
 	}
-	err := root.ExecuteContext(ctx)
+	parsed := &parsedGlobals{}
+	err := root.ExecuteContext(context.WithValue(ctx, parsedGlobalsKey{}, parsed))
 	if err == nil {
 		return ExitOK
 	}
+	if parsed.set {
+		// cobra parsed the command's flags: use its reading, which knows
+		// which tokens are the values of command flags.
+		g = parsed.g
+	}
 	err = unknownCommand(root, args, err)
 	name := root.Name()
 	RenderError(root.ErrOrStderr(), name, wrongDirectory(name, err, args, ""), g)
@@ -79,6 +85,18 @@ func Fail(w io.Writer, name string, err error, g Globals) int {
 	return ExitFailure
 }
 
+// parsedGlobalsKey is the context key of the parsedGlobals Execute
+// installs.
+type parsedGlobalsKey struct{}
+
+// parsedGlobals receives the globals the root's PersistentPreRunE applied,
+// so Execute's error block can use cobra's reading of the command line
+// instead of PeekGlobals. Call never installs one.
+type parsedGlobals struct {
+	g   Globals
+	set bool
+}
+
 func exitCode(err error) int {
 	var carrier *exitCodeError
 	switch {

modules/bonfire/globals.go

@@ -48,11 +48,32 @@ var (
 // exists, so the generated main can pick the configuration environment and
 // the log verbosity before config loads and plugins boot. The scan is
 // tolerant: it never fails, ignores every other token and stops at "--".
-// Quiet also sets NoInteraction.
+// It reads the =value forms the way pflag does (--verbose=2, -v=3,
+// --quiet=false, -n=1, --ansi=false), ignoring a value pflag would reject.
+// Quiet also sets NoInteraction. A value-taking command flag followed by a
+// global-looking token (--name -v) is read as the global before boot; the
+// command and its error block use cobra's reading.
 func PeekGlobals(args []string) Globals {
 	var g Globals
 	for i := 0; i < len(args); i++ {
 		a := args[i]
+		if name, value, ok := strings.Cut(a, "="); ok && strings.HasPrefix(name, "--") {
+			switch strings.TrimPrefix(name, "--") {
+			case flagVerbose:
+				peekCount(&g.Verbosity, value)
+			case flagQuiet:
+				peekBool(&g.Quiet, value)
+			case flagNoInteraction:
+				peekBool(&g.NoInteraction, value)
+			case flagANSI:
+				peekColor(&g.Color, ColorAlways, value)
+			case flagNoANSI:
+				peekColor(&g.Color, ColorNever, value)
+			case flagEnv:
+				g.Env = value
+			}
+			continue
+		}
 		switch {
 		case a == "--":
 			return g.normalize()
@@ -71,8 +92,6 @@ func PeekGlobals(args []string) Globals {
 				g.Env = args[i+1]
 				i++
 			}
-		case strings.HasPrefix(a, "--"+flagEnv+"="):
-			g.Env = strings.TrimPrefix(a, "--"+flagEnv+"=")
 		case isShorthandCluster(a):
 			g.peekShorthands(a[1:])
 		}
@@ -80,17 +99,60 @@ func PeekGlobals(args []string) Globals {
 	return g.normalize()
 }
 
+// peekCount applies a --verbose=N or -v=N value the way pflag's count flag
+// does: the value replaces the count; an unparsable value is ignored.
+func peekCount(n *int, value string) {
+	if v, err := strconv.ParseInt(value, 0, 0); err == nil {
+		*n = int(v)
+	}
+}
+
+// peekBool applies a bool flag's =value the way pflag does; an unparsable
+// value is ignored.
+func peekBool(b *bool, value string) {
+	if v, err := strconv.ParseBool(value); err == nil {
+		*b = v
+	}
+}
+
+// peekColor applies --ansi=B or --no-ansi=B the way colorFlag.Set does:
+// true sets the flag's mode, false resets the mode to ColorAuto only when it
+// is the flag's own; an unparsable value is ignored.
+func peekColor(mode *ColorMode, set ColorMode, value string) {
+	on, err := strconv.ParseBool(value)
+	switch {
+	case err != nil:
+	case on:
+		*mode = set
+	case *mode == set:
+		*mode = ColorAuto
+	}
+}
+
 // isShorthandCluster reports a single-dash token such as -v, -vvv or -qn.
 func isShorthandCluster(a string) bool {
 	return len(a) >= 2 && a[0] == '-' && a[1] != '-'
 }
 
 // peekShorthands reads a cluster of shorthands the way pflag does (-qn is
-// -q -n, -vvq is -v -v -q). It stops at the first letter that is not a
-// global shorthand: that may be a command flag whose value is the rest of
-// the token.
+// -q -n, -vvq is -v -v -q, -v=3 sets the count, -qn=false turns -n off). It
+// stops at the first letter that is not a global shorthand: that may be a
+// command flag whose value is the rest of the token.
 func (g *Globals) peekShorthands(cluster string) {
-	for _, r := range cluster {
+	for i := 0; i < len(cluster); i++ {
+		r := cluster[i]
+		if i+1 < len(cluster) && cluster[i+1] == '=' {
+			value := cluster[i+2:]
+			switch r {
+			case 'v':
+				peekCount(&g.Verbosity, value)
+			case 'q':
+				peekBool(&g.Quiet, value)
+			case 'n':
+				peekBool(&g.NoInteraction, value)
+			}
+			return
+		}
 		switch r {
 		case 'v':
 			g.Verbosity++

modules/bonfire/globals_test.go

@@ -3,6 +3,8 @@ package bonfire
 import (
 	"bytes"
 	"context"
+	"errors"
+	"fmt"
 	"slices"
 	"strings"
 	"testing"
@@ -41,6 +43,23 @@ var peekCases = []struct {
 	{"combined -vq", []string{"-vq", "migrate"}, Globals{Verbosity: 1, Quiet: true, NoInteraction: true}, true},
 	{"combined -nvv", []string{"migrate", "-nvv"}, Globals{Verbosity: 2, NoInteraction: true}, true},
 	{"everything", []string{"-vv", "--env=dev", "migrate", "-n", "--no-ansi"}, Globals{Verbosity: 2, NoInteraction: true, Color: ColorNever, Env: "dev"}, true},
+	// IN-02: the =value forms, read with pflag's value rules.
+	{"--verbose=2", []string{"migrate", "--verbose=2"}, Globals{Verbosity: 2}, true},
+	{"-v=3", []string{"migrate", "-v=3"}, Globals{Verbosity: 3}, true},
+	{"-vv=1", []string{"migrate", "-vv=1"}, Globals{Verbosity: 1}, true},
+	{"-v --verbose=2 -v", []string{"migrate", "-v", "--verbose=2", "-v"}, Globals{Verbosity: 3}, true},
+	{"--quiet=true", []string{"migrate", "--quiet=true"}, Globals{Quiet: true, NoInteraction: true}, true},
+	{"-q --quiet=false", []string{"migrate", "-q", "--quiet=false"}, Globals{}, true},
+	{"-q=false", []string{"migrate", "-q=false"}, Globals{}, true},
+	{"--no-interaction=1", []string{"migrate", "--no-interaction=1"}, Globals{NoInteraction: true}, true},
+	{"-n=t", []string{"migrate", "-n=t"}, Globals{NoInteraction: true}, true},
+	{"-qn=false", []string{"migrate", "-qn=false"}, Globals{Quiet: true, NoInteraction: true}, true},
+	{"--ansi=false", []string{"migrate", "--ansi=false"}, Globals{Color: ColorAuto}, true},
+	{"--ansi --ansi=false", []string{"migrate", "--ansi", "--ansi=false"}, Globals{Color: ColorAuto}, true},
+	{"--ansi=true", []string{"migrate", "--ansi=true"}, Globals{Color: ColorAlways}, true},
+	{"--no-ansi=true", []string{"migrate", "--no-ansi=true"}, Globals{Color: ColorNever}, true},
+	{"--no-ansi --ansi=false", []string{"migrate", "--no-ansi", "--ansi=false"}, Globals{Color: ColorNever}, true},
+	{"unparsable values ignored", []string{"migrate", "-v", "--verbose=x", "--quiet=maybe", "--ansi=sure"}, Globals{Verbosity: 1}, false},
 	{"unknown flags ignored", []string{"--bogus", "-x", "migrate", "-", "--env"}, Globals{}, false},
 	{"--env without a value", []string{"migrate", "--env"}, Globals{}, false},
 	{"empty", nil, Globals{}, false},
@@ -121,3 +140,29 @@ func TestGlobalFlagsAgree(t *testing.T) {
 		})
 	}
 }
+
+// TestErrorBlockUsesParsedGlobals (IN-02): once cobra has parsed the
+// command's flags, the error block uses cobra's globals. In
+// "migrate --name -v" the -v is the value of --name, so the block stays at
+// verbosity 0 although PeekGlobals reads -v.
+func TestErrorBlockUsesParsedGlobals(t *testing.T) {
+	plainEnv(t)
+	cmd := Command{
+		Name:        "migrate",
+		Description: "Run the migrations",
+		Flags:       []Flag{{Name: "name", Description: "A name"}},
+		Run: func(_ context.Context, in Input, _ Output) error {
+			if v, _ := in.Flag("name"); v != "-v" {
+				t.Errorf("--name = %q, want -v", v)
+			}
+			return fmt.Errorf("outer: %w", errors.New("inner"))
+		},
+	}
+	_, errOut, code := runGolden(t, []Command{cmd}, "migrate", "--name", "-v")
+	if code != ExitFailure {
+		t.Fatalf("exit %d, want %d\n%s", code, ExitFailure, errOut)
+	}
+	if !strings.Contains(errOut, "Run with -v for details.") || strings.Contains(errOut, "Caused by:") {
+		t.Errorf("block does not use the parsed verbosity 0:\n%s", errOut)
+	}
+}

modules/bonfire/root.go

@@ -152,6 +152,9 @@ func NewRootIO(name string, commands []Command, in io.Reader, out, errW io.Write
 			g = flags.globals()
 		}
 		output.setGlobals(g)
+		if parsed, ok := cmd.Context().Value(parsedGlobalsKey{}).(*parsedGlobals); ok && parsed != nil {
+			parsed.g, parsed.set = g, true
+		}
 		for _, hook := range cfg.hooks {
 			hook(g)
 		}