From 7159f52f4d357e8df2fa694477536f676c77c75b Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 7 Nov 2026 03:49:34 +0000 Subject: [PATCH] Preserve ordered flag occurrences --- binding.go | 76 ++++++++++++++++- command.go | 13 ++- docs/roadmap/implementation.md | 6 +- execute.go | 4 +- flag_values.go | 34 ++++++-- occurrence_test.go | 152 +++++++++++++++++++++++++++++++++ 6 files changed, 273 insertions(+), 12 deletions(-) create mode 100644 occurrence_test.go diff --git a/binding.go b/binding.go index 21b0a29..6b479c8 100644 --- a/binding.go +++ b/binding.go @@ -3,6 +3,7 @@ package commandkit import ( "errors" "fmt" + "sort" "strconv" commandmeta "go.maximumdirect.net/commandkit/internal/commandmeta" @@ -38,6 +39,48 @@ func ValueFlag[I, V any](meta FlagMeta, defaultValue *V, parse func(string) (V, return Flag[I]{binding: binding} } +// OccurrenceFlag declares a repeatable flag whose setter runs once for each +// supplied occurrence. Occurrence setters run in command-line order across +// participating bindings; an omitted flag does not call its setter. +func OccurrenceFlag[I, V any](meta FlagMeta, parse func(string) (V, error), set func(*I, Supplied[V])) Flag[I] { + binding := flagBinding[I]{identity: newBindingIdentity(), meta: meta, repeated: true} + if parse == nil || set == nil { + return Flag[I]{binding: binding} + } + binding.valid = true + binding.prepare = func(values []string) (preparedValue[I], error) { + result := preparedValue[I]{identity: binding.identity, supplied: len(values) > 0} + for index, raw := range values { + value, err := parse(raw) + if err != nil { + return preparedValue[I]{}, &flagOccurrenceError{index: index, cause: err} + } + ordinal := index + result.ordered = append(result.ordered, preparedOccurrence[I]{ordinal: &ordinal, apply: func(input *I) { set(input, Supplied[V]{Value: value, Set: true}) }}) + } + return result, nil + } + return Flag[I]{binding: binding} +} + +// RenameFlag returns an independent binding with a new long name and no short +// spelling. Parsing, defaults, completion, requiredness, and setters are retained. +func RenameFlag[I any](flag Flag[I], name string) Flag[I] { + binding := flag.binding + binding.meta.Name = name + binding.meta.Short = "" + binding.identity = newBindingIdentity() + if prepare := binding.prepare; prepare != nil { + identity := binding.identity + binding.prepare = func(values []string) (preparedValue[I], error) { + value, err := prepare(values) + value.identity = identity + return value, err + } + } + return Flag[I]{binding: binding} +} + // BoolFlag declares a boolean flag. A bare flag is supplied by the parser as // "true"; explicit values are converted with strconv.ParseBool. Set distinguishes // an omitted flag from an explicit false value. @@ -189,7 +232,9 @@ func (p preparedInput[I]) build() I { input = p.newInput() } for _, value := range p.values { - value.apply(&input) + if value.apply != nil { + value.apply(&input) + } } return input } @@ -202,6 +247,10 @@ func prepareInput[I any](spec CommandSpec[I], flagValues [][]string, operands [] } func prepareInputWithSpellings[I any](spec CommandSpec[I], flagValues, flagSpellings [][]string, operands []string, enforceRequired bool) (preparedInput[I], error) { + return prepareInputWithOccurrences(spec, flagValues, flagSpellings, nil, operands, enforceRequired) +} + +func prepareInputWithOccurrences[I any](spec CommandSpec[I], flagValues, flagSpellings [][]string, occurrences []commandmeta.FlagOccurrence, operands []string, enforceRequired bool) (preparedInput[I], error) { if len(flagValues) != len(spec.Flags) { return preparedInput[I]{}, fmt.Errorf("received %d flag value sets for %d bindings", len(flagValues), len(spec.Flags)) } @@ -212,6 +261,12 @@ func prepareInputWithSpellings[I any](spec CommandSpec[I], flagValues, flagSpell return preparedInput[I]{}, fmt.Errorf("received %d flag spelling sets for %d bindings", len(flagSpellings), len(spec.Flags)) } preparedFlags := make([]preparedValue[I], len(spec.Flags)) + ordinals := make(map[string][]int) + for index, occurrence := range occurrences { + ordinals[occurrence.Name] = append(ordinals[occurrence.Name], index) + } + var ordered []preparedOccurrence[I] + fallbackOrdinal := 0 missingRequired := "" for i, flag := range spec.Flags { binding := flag.binding @@ -231,6 +286,21 @@ func prepareInputWithSpellings[I any](spec CommandSpec[I], flagValues, flagSpell return preparedInput[I]{}, fmt.Errorf("flag --%s: %w", binding.meta.Name, err) } preparedFlags[i] = value + for index, occurrence := range value.ordered { + if occurrences != nil { + positions := ordinals[binding.meta.Name] + if index >= len(positions) { + return preparedInput[I]{}, fmt.Errorf("missing occurrence position for flag --%s", binding.meta.Name) + } + *occurrence.ordinal = positions[index] + } else { + *occurrence.ordinal = fallbackOrdinal + fallbackOrdinal++ + } + if occurrence.apply != nil { + ordered = append(ordered, occurrence) + } + } if enforceRequired && binding.meta.Required && !binding.deferredRequired && !value.supplied && missingRequired == "" { missingRequired = binding.meta.Name } @@ -243,6 +313,10 @@ func prepareInputWithSpellings[I any](spec CommandSpec[I], flagValues, flagSpell return preparedInput[I]{}, err } preparedValues := append([]preparedValue[I](nil), preparedFlags...) + sort.SliceStable(ordered, func(i, j int) bool { return *ordered[i].ordinal < *ordered[j].ordinal }) + for _, occurrence := range ordered { + preparedValues = append(preparedValues, preparedValue[I]{apply: occurrence.apply}) + } for i, argument := range spec.Arguments { binding := argument.binding value, err := binding.prepare(argumentValues[i]) diff --git a/command.go b/command.go index 1f34aa1..14187df 100644 --- a/command.go +++ b/command.go @@ -127,6 +127,13 @@ type preparedValue[I any] struct { identity *bindingIdentity supplied bool apply func(*I) + ordered []preparedOccurrence[I] +} + +type preparedOccurrence[I any] struct { + // Captured bindings share this position with the outer prepared occurrence. + ordinal *int + apply func(*I) } type flagBinding[I any] struct { @@ -186,7 +193,7 @@ type commandPayload interface { hasRun() bool bridgeFlags() []commandmeta.Flag bridgeArguments() []commandmeta.Argument - prepare([][]string, [][]string, []string, bool) (func(*execution) Outcome, error) + prepare([][]string, [][]string, []commandmeta.FlagOccurrence, []string, bool) (func(*execution) Outcome, error) } type typedCommand[I any] struct { @@ -235,8 +242,8 @@ func (c typedCommand[I]) bridgeArguments() []commandmeta.Argument { return arguments } -func (c typedCommand[I]) prepare(flagValues, flagSpellings [][]string, operands []string, enforceRequired bool) (func(*execution) Outcome, error) { - prepared, err := prepareInputWithSpellings(c.spec, flagValues, flagSpellings, operands, enforceRequired) +func (c typedCommand[I]) prepare(flagValues, flagSpellings [][]string, occurrences []commandmeta.FlagOccurrence, operands []string, enforceRequired bool) (func(*execution) Outcome, error) { + prepared, err := prepareInputWithOccurrences(c.spec, flagValues, flagSpellings, occurrences, operands, enforceRequired) if err != nil { return nil, err } diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index a09edc9..4580cdd 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Shared reporting and PromptKit v0.20.0 implementation plan -Status: implementation in progress; Stages 1–5 complete. +Status: implementation in progress; Stages 1–6 complete. ## Scope, authority, and execution @@ -213,6 +213,10 @@ conversion API. Each integration will enforce its flag's outer shape. ## Stage 6: Ordered typed occurrences and reusable binding identity +**Complete.** Ordered typed occurrences and independently renamed bindings are +implemented, including captured subset replay. Focused binding tests and the +root workspace suite (`GOTOOLCHAIN=local go test ./...` with Go 1.27.0) passed. + Extend [binding.go](../../binding.go), [flag_values.go](../../flag_values.go), and the neutral preparation path to retain ordinal positions from existing `FlagOccurrences`. Add: diff --git a/execute.go b/execute.go index 7abce02..a302061 100644 --- a/execute.go +++ b/execute.go @@ -59,13 +59,13 @@ func (a *App) Execute(ctx context.Context, args []string, streams Streams) Outco if !ok || payload == nil { outcome = current.renderHelp(parsed.Path) } else if parsed.HelpRequested || !payload.hasRun() { - if _, err := payload.prepare(parsed.FlagValues, parsed.FlagSpellings, parsed.Operands, false); err != nil { + if _, err := payload.prepare(parsed.FlagValues, parsed.FlagSpellings, parsed.FlagOccurrences, parsed.Operands, false); err != nil { outcome = classifyInputError(err) } else { outcome = current.renderHelp(parsed.Path) } } else { - run, err := payload.prepare(parsed.FlagValues, parsed.FlagSpellings, parsed.Operands, true) + run, err := payload.prepare(parsed.FlagValues, parsed.FlagSpellings, parsed.FlagOccurrences, parsed.Operands, true) if err != nil { outcome = classifyInputError(err) } else { diff --git a/flag_values.go b/flag_values.go index c9efdb3..757a2bd 100644 --- a/flag_values.go +++ b/flag_values.go @@ -3,6 +3,7 @@ package commandkit import ( "errors" "fmt" + "sort" ) // ErrFlagInput identifies a missing required captured flag selected for an input. @@ -13,7 +14,8 @@ var ErrFlagWiring = errors.New("flag wiring is invalid") // FlagValues stores invocation-local converted values and presence for captured // flags. It does not retain an input instance; values can be applied to -// independently initialized destinations. +// independently initialized destinations. Captured occurrence flags retain +// their relative command-line order across the selected bindings. type FlagValues[I any] struct { values map[*bindingIdentity]preparedValue[I] } @@ -42,7 +44,7 @@ func CaptureFlags[Outer, I any](flags []Flag[I], target func(*Outer) *FlagValues if err != nil { return preparedValue[Outer]{}, err } - return preparedValue[Outer]{ + result := preparedValue[Outer]{ identity: identity, supplied: prepared.supplied, apply: func(outer *Outer) { @@ -55,7 +57,11 @@ func CaptureFlags[Outer, I any](flags []Flag[I], target func(*Outer) *FlagValues } store.values[inner.binding.identity] = prepared }, - }, nil + } + for _, occurrence := range prepared.ordered { + result.ordered = append(result.ordered, preparedOccurrence[Outer]{ordinal: occurrence.ordinal}) + } + return result, nil } } captured[index] = Flag[Outer]{binding: binding} @@ -66,11 +72,14 @@ func CaptureFlags[Outer, I any](flags []Flag[I], target func(*Outer) *FlagValues // ApplyTo applies selected original captured flags to an initialized // destination. A missing captured binding or nil destination matches // ErrFlagWiring. A required selected value that was not supplied matches -// ErrFlagInput. Repeated core-owned values are copied for each application. +// ErrFlagInput. Ordinary selected values are applied first, followed by selected +// occurrence values in command-line order. Repeated core-owned values are copied +// for each application. func (values FlagValues[I]) ApplyTo(target *I, selected ...Flag[I]) error { if target == nil { return fmt.Errorf("%w: destination is nil", ErrFlagWiring) } + preparedValues := make([]preparedValue[I], 0, len(selected)) for _, flag := range selected { binding := flag.binding if binding.identity == nil { @@ -83,7 +92,22 @@ func (values FlagValues[I]) ApplyTo(target *I, selected ...Flag[I]) error { if binding.meta.Required && !prepared.supplied { return fmt.Errorf("flag --%s: %w", binding.meta.Name, ErrFlagInput) } - prepared.apply(target) + preparedValues = append(preparedValues, prepared) + } + var ordered []preparedOccurrence[I] + for _, prepared := range preparedValues { + if prepared.apply != nil { + prepared.apply(target) + } + for _, occurrence := range prepared.ordered { + if occurrence.apply != nil { + ordered = append(ordered, occurrence) + } + } + } + sort.SliceStable(ordered, func(i, j int) bool { return *ordered[i].ordinal < *ordered[j].ordinal }) + for _, occurrence := range ordered { + occurrence.apply(target) } return nil } diff --git a/occurrence_test.go b/occurrence_test.go new file mode 100644 index 0000000..7dfe018 --- /dev/null +++ b/occurrence_test.go @@ -0,0 +1,152 @@ +package commandkit + +import ( + "errors" + "fmt" + "reflect" + "strings" + "testing" +) + +type occurrenceInput struct{ events []string } + +func occurrenceFor(name string, required bool, calls *int) Flag[occurrenceInput] { + return OccurrenceFlag(FlagMeta{Name: name, Required: required, Short: "x"}, func(raw string) (string, error) { + *calls++ + if raw == "bad" { + return "", errors.New("invalid occurrence") + } + return raw, nil + }, func(input *occurrenceInput, value Supplied[string]) { + input.events = append(input.events, name+"="+value.Value) + }) +} + +func TestOccurrenceFlagsApplyInEnteredOrder(t *testing.T) { + calls, factories := 0, 0 + first := occurrenceFor("first", false, &calls) + second := RenameFlag(occurrenceFor("second", false, &calls), "second") + ordinary := ValueFlag(FlagMeta{Name: "ordinary"}, strptr("default"), func(s string) (string, error) { return s, nil }, func(i *occurrenceInput, v Supplied[string]) { i.events = append(i.events, "ordinary="+v.Value) }) + arg := ValueArgument(ArgumentMeta{Name: "ARG"}, false, func(s string) (string, error) { return s, nil }, func(i *occurrenceInput, v Supplied[string]) { i.events = append(i.events, "operand="+v.Value) }) + var got []string + cmd := NewCommand(CommandSpec[occurrenceInput]{Name: "run", Flags: []Flag[occurrenceInput]{first, second, ordinary}, Arguments: []Argument[occurrenceInput]{arg}, NewInput: func() occurrenceInput { factories++; return occurrenceInput{} }, CheckInput: func(i occurrenceInput) error { got = append([]string(nil), i.events...); return nil }, Run: func(Invocation, occurrenceInput) Outcome { return Success() }}) + app, err := New(Identity{Name: "Example"}, cmd, FlagAliasFor("run", "--other", "--second")) + if err != nil { + t.Fatal(err) + } + result := app.Execute(t.Context(), []string{"run", "--other=a", "--first=", "--second=b", "--ordinary=z", "tail"}, Streams{}) + want := []string{"ordinary=z", "second=a", "first=", "second=b", "operand=tail"} + if result.Status != 0 || !reflect.DeepEqual(got, want) || calls != 3 || factories != 1 { + t.Fatalf("%#v events=%q calls=%d factories=%d", result, got, calls, factories) + } + result = app.Execute(t.Context(), []string{"run", "--first=bad"}, Streams{}) + if result.Kind != OutcomeUsage || factories != 1 || !strings.Contains(result.Explanation.Message, "--first") { + t.Fatalf("invalid result %#v factories=%d", result, factories) + } + result = app.Execute(t.Context(), []string{"run"}, Streams{}) + if result.Status != 0 || !reflect.DeepEqual(got, []string{"ordinary=default", "operand="}) { + t.Fatalf("fresh %#v, %q", result, got) + } +} + +type occurrenceOuter struct{ flags FlagValues[occurrenceInput] } + +func TestCapturedOccurrencesReplaySelectedOrder(t *testing.T) { + calls := 0 + a := occurrenceFor("alpha", false, &calls) + b := RenameFlag(a, "beta") + missing := RenameFlag(occurrenceFor("required", true, &calls), "required") + originals := []Flag[occurrenceInput]{a, b, missing} + captured := CaptureFlags(originals, func(o *occurrenceOuter) *FlagValues[occurrenceInput] { return &o.flags }) + var stored FlagValues[occurrenceInput] + cmd := NewCommand(CommandSpec[occurrenceOuter]{Name: "run", Flags: captured, Run: func(_ Invocation, i occurrenceOuter) Outcome { stored = i.flags; return Success() }}) + app, err := New(Identity{Name: "Example"}, cmd, FlagAliasFor("run", "--b", "--beta")) + if err != nil { + t.Fatal(err) + } + result := app.Execute(t.Context(), []string{"run", "--b=1", "--alpha=2", "--beta=3", "--alpha=4"}, Streams{}) + if result.Status != 0 || calls != 4 { + t.Fatalf("%#v calls=%d", result, calls) + } + var got occurrenceInput + if err := stored.ApplyTo(&got, a, b); err != nil { + t.Fatal(err) + } + want := []string{"alpha=1", "alpha=2", "alpha=3", "alpha=4"} + if !reflect.DeepEqual(got.events, want) { + t.Fatalf("events %q want %q", got.events, want) + } + var subset occurrenceInput + if err := stored.ApplyTo(&subset, b); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(subset.events, []string{"alpha=1", "alpha=3"}) || calls != 4 { + t.Fatalf("subset %q calls=%d", subset.events, calls) + } + var repeated occurrenceInput + if err := stored.ApplyTo(&repeated, a, b); err != nil { + t.Fatal(err) + } + got.events[0] = "changed" + if !reflect.DeepEqual(repeated.events, want) { + t.Fatalf("shared state %q", repeated.events) + } + if err := stored.ApplyTo(&repeated, missing); !errors.Is(err, ErrFlagInput) { + t.Fatalf("required error %v", err) + } + other := RenameFlag(a, "other") + if err := stored.ApplyTo(&repeated, other); !errors.Is(err, ErrFlagWiring) { + t.Fatalf("identity error %v", err) + } +} + +func TestRenameFlagKeepsBindingBehavior(t *testing.T) { + parse := func(s string) (string, error) { return strings.ToUpper(s), nil } + original := ValueFlag(FlagMeta{Name: "original", Short: "o", Required: true, Completion: CompleteChoices(CompletionChoice{Value: "one"})}, strptr("default"), parse, func(i *occurrenceInput, v Supplied[string]) { + i.events = append(i.events, fmt.Sprint(v.Value, ":", v.Set)) + }) + renamed := RenameFlag(original, "renamed") + if renamed.binding.identity == original.binding.identity || renamed.binding.meta.Short != "" || !renamed.binding.meta.Required || !reflect.DeepEqual(renamed.binding.meta.Completion, original.binding.meta.Completion) { + t.Fatalf("renamed %#v", renamed.binding.meta) + } + value, err := renamed.binding.prepare(nil) + if err != nil { + t.Fatal(err) + } + var input occurrenceInput + value.apply(&input) + if !reflect.DeepEqual(input.events, []string{"default:false"}) { + t.Fatalf("default %q", input.events) + } + if _, err := New(Identity{Name: "Example"}, NewCommand(CommandSpec[occurrenceInput]{Name: "run", Flags: []Flag[occurrenceInput]{original, RenameFlag(original, "bad name")}})); err == nil { + t.Fatal("invalid name accepted") + } +} + +func TestOccurrenceRequiredAndAliasedConversion(t *testing.T) { + parseErr := errors.New("cannot parse") + builds := 0 + flag := OccurrenceFlag(FlagMeta{Name: "item", Required: true}, func(raw string) (string, error) { + if raw == "bad" { + return "", parseErr + } + return raw, nil + }, func(*occurrenceInput, Supplied[string]) {}) + cmd := NewCommand(CommandSpec[occurrenceInput]{Name: "run", Flags: []Flag[occurrenceInput]{flag}, NewInput: func() occurrenceInput { builds++; return occurrenceInput{} }, Run: func(Invocation, occurrenceInput) Outcome { return Success() }}) + app, err := New(Identity{Name: "Example"}, cmd, FlagAliasFor("run", "--entry", "--item")) + if err != nil { + t.Fatal(err) + } + missing := app.Execute(t.Context(), []string{"run"}, Streams{}) + if missing.Kind != OutcomeUsage || builds != 0 { + t.Fatalf("missing: %#v", missing) + } + invalid := app.Execute(t.Context(), []string{"run", "--item=ok", "--entry=bad"}, Streams{}) + if invalid.Kind != OutcomeUsage || !errors.Is(invalid.Cause, parseErr) || !strings.Contains(invalid.Explanation.Message, "--entry occurrence 2") || builds != 0 { + t.Fatalf("invalid: %#v", invalid) + } + help := app.Execute(t.Context(), []string{"run", "--help"}, Streams{}) + if help.Status != 0 || builds != 0 { + t.Fatalf("help: %#v", help) + } +}