From 1fa026e5bdb22b51acaefa4fc379fb215386daac Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 19 Oct 2026 19:46:22 +0000 Subject: [PATCH] Preserve alias context in usage errors --- binding.go | 56 ++++++++++-- command.go | 6 +- docs/roadmap/implementation.md | 11 ++- execute.go | 20 ++++- route_test.go | 159 +++++++++++++++++++++++++++++++++ 5 files changed, 239 insertions(+), 13 deletions(-) diff --git a/binding.go b/binding.go index 8e48171..55b3f99 100644 --- a/binding.go +++ b/binding.go @@ -1,6 +1,7 @@ package commandkit import ( + "errors" "fmt" "strconv" ) @@ -22,10 +23,10 @@ func ValueFlag[I, V any](meta FlagMeta, defaultValue *V, parse func(string) (V, binding.prepare = func(values []string) (preparedValue[I], error) { value := fallback supplied := len(values) > 0 - for _, raw := range values { + for index, raw := range values { parsed, err := parse(raw) if err != nil { - return preparedValue[I]{}, err + return preparedValue[I]{}, &flagOccurrenceError{index: index, cause: err} } value = parsed } @@ -47,10 +48,10 @@ func BoolFlag[I any](meta FlagMeta, defaultValue bool, set func(*I, Supplied[boo binding.prepare = func(values []string) (preparedValue[I], error) { value := defaultValue supplied := len(values) > 0 - for _, raw := range values { + for index, raw := range values { parsed, err := strconv.ParseBool(raw) if err != nil { - return preparedValue[I]{}, err + return preparedValue[I]{}, &flagOccurrenceError{index: index, cause: err} } value = parsed } @@ -76,10 +77,10 @@ func RepeatFlag[I, V any](meta FlagMeta, defaults []V, parse func(string) (V, er supplied := len(values) > 0 if supplied { parsedValues = make([]V, 0, len(values)) - for _, raw := range values { + for index, raw := range values { parsed, err := parse(raw) if err != nil { - return preparedValue[I]{}, err + return preparedValue[I]{}, &flagOccurrenceError{index: index, cause: err} } parsedValues = append(parsedValues, parsed) } @@ -158,6 +159,28 @@ type preparedInput[I any] struct { values []preparedValue[I] } +type flagOccurrenceError struct { + index int + cause error +} + +func (err *flagOccurrenceError) Error() string { return err.cause.Error() } + +func (err *flagOccurrenceError) Unwrap() error { return err.cause } + +type flagConversionError struct { + binding string + spelling string + occurrence int + cause error +} + +func (err *flagConversionError) Error() string { + return fmt.Sprintf("flag %s occurrence %d: %v", err.spelling, err.occurrence, err.cause) +} + +func (err *flagConversionError) Unwrap() error { return err.cause } + func (p preparedInput[I]) build() I { var input I if p.newInput != nil { @@ -173,15 +196,36 @@ func (p preparedInput[I]) build() I { // creates an input value. When enforceRequired is false, missing required flags // and operands are allowed so metadata help can validate supplied syntax. func prepareInput[I any](spec CommandSpec[I], flagValues [][]string, operands []string, enforceRequired bool) (preparedInput[I], error) { + return prepareInputWithSpellings(spec, flagValues, nil, operands, enforceRequired) +} + +func prepareInputWithSpellings[I any](spec CommandSpec[I], flagValues, flagSpellings [][]string, 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)) } + if flagSpellings == nil { + flagSpellings = make([][]string, len(spec.Flags)) + } + if len(flagSpellings) != len(spec.Flags) { + 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)) missingRequired := "" for i, flag := range spec.Flags { binding := flag.binding value, err := binding.prepare(flagValues[i]) if err != nil { + var occurrenceErr *flagOccurrenceError + if errors.As(err, &occurrenceErr) { + spelling := "--" + binding.meta.Name + if occurrenceErr.index >= 0 && occurrenceErr.index < len(flagSpellings[i]) && flagSpellings[i][occurrenceErr.index] != "" { + spelling = flagSpellings[i][occurrenceErr.index] + } + return preparedInput[I]{}, &flagConversionError{ + binding: binding.meta.Name, spelling: spelling, + occurrence: occurrenceErr.index + 1, cause: occurrenceErr.cause, + } + } return preparedInput[I]{}, fmt.Errorf("flag --%s: %w", binding.meta.Name, err) } preparedFlags[i] = value diff --git a/command.go b/command.go index 8ce5788..4234450 100644 --- a/command.go +++ b/command.go @@ -170,7 +170,7 @@ type commandPayload interface { hasRun() bool bridgeFlags() []commandmeta.Flag bridgeArguments() []commandmeta.Argument - prepare([][]string, []string, bool) (func(*execution) Outcome, error) + prepare([][]string, [][]string, []string, bool) (func(*execution) Outcome, error) } type typedCommand[I any] struct { @@ -217,8 +217,8 @@ func (c typedCommand[I]) bridgeArguments() []commandmeta.Argument { return arguments } -func (c typedCommand[I]) prepare(flagValues [][]string, operands []string, enforceRequired bool) (func(*execution) Outcome, error) { - prepared, err := prepareInput(c.spec, flagValues, operands, enforceRequired) +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) if err != nil { return nil, err } diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 6b5ab3e..ed385ea 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Application-owned aliases implementation plan -Status: in progress; Stages 1–10 completed. +Status: in progress; Stages 1–11 completed. ## Authority, baseline, and scope @@ -561,6 +561,15 @@ spellings; wrapped `errors.Is`/`errors.As`; usage paths with canonical annotatio unknown routes and flags; cancellation; output and diagnostic failures. Assert semantic fragments and identities, not entire backend wording snapshots. +**Completed:** Flag conversion failures now retain the binding, entered +spelling, occurrence number, and original wrapped cause. Usage diagnostics use +entered alias paths and identify resolved canonical commands; unresolved routes +receive no invented canonical label. Successful and failed registration during +execution preserve the selected route. Verified with root, ckconfig, and +ckprompt suites under the local workspace, including existing cancellation, +diagnostic deduplication, writer failure, short-write, and incomplete-report +coverage. + ## Stage 12: Verify registration and routing interactions end to end **Work** diff --git a/execute.go b/execute.go index 1980ada..259ae8d 100644 --- a/execute.go +++ b/execute.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io" + "strings" internalcobra "gitea.maximumdirect.net/eric/CommandKit/internal/cobra" commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta" @@ -40,12 +41,12 @@ func (a *App) Execute(ctx context.Context, args []string, streams Streams) Outco return current.present(current.renderHelp(parsed.Path), parsed.Path) } if parsed.HelpRequested || !payload.hasRun() { - if _, err := payload.prepare(parsed.FlagValues, parsed.Operands, false); err != nil { + if _, err := payload.prepare(parsed.FlagValues, parsed.FlagSpellings, parsed.Operands, false); err != nil { return current.present(Usage(err), parsed.Path) } return current.present(current.renderHelp(parsed.Path), parsed.Path) } - run, err := payload.prepare(parsed.FlagValues, parsed.Operands, true) + run, err := payload.prepare(parsed.FlagValues, parsed.FlagSpellings, parsed.Operands, true) if err != nil { return current.present(Usage(err), parsed.Path) } @@ -190,7 +191,20 @@ func (current *execution) usageLine(path []string) string { } func (current *execution) present(outcome Outcome, path []string) Outcome { - return current.app.presentWithUsageLine(outcome, path, current.streams, current.usageLine) + usageLine := func(path []string) string { return current.usageLine(path) } + if current.route.kind == routeCanonical && !current.route.enteredPath.Equal(current.route.canonicalPath) { + path = clonePath(current.route.enteredPath) + usageLine = func([]string) string { return current.routeUsageLine() } + } + return current.app.presentWithUsageLine(outcome, path, current.streams, usageLine) +} + +func (current *execution) routeUsageLine() string { + line := current.usageLine(clonePath(current.route.canonicalPath)) + canonical := strings.TrimSpace(current.app.identity.Name + " " + current.route.canonicalPath.String()) + entered := strings.TrimSpace(current.app.identity.Name + " " + current.route.enteredPath.String()) + line = strings.Replace(line, "Usage: "+canonical, "Usage: "+entered, 1) + return line + "\nAlias for: " + canonical } func normalizeStreams(streams Streams) Streams { diff --git a/route_test.go b/route_test.go index 0c09556..f2c868e 100644 --- a/route_test.go +++ b/route_test.go @@ -3,6 +3,8 @@ package commandkit import ( "bytes" "context" + "errors" + "fmt" "io" "reflect" "strings" @@ -316,6 +318,163 @@ func TestExecuteAppliesCanonicalCommandFlagAliasesAfterRouting(t *testing.T) { } } +func TestAliasConversionErrorsRetainOccurrenceAndEnteredUsage(t *testing.T) { + colorFailure := errors.New("invalid color") + tagFailure := errors.New("invalid tag") + type input struct { + color Supplied[string] + tags Supplied[[]string] + } + command := NewCommand(CommandSpec[input]{ + Name: "inspect", + Flags: []Flag[input]{ + ValueFlag(FlagMeta{Name: "color"}, (*string)(nil), func(value string) (string, error) { + if value == "bad" { + return "", colorFailure + } + return value, nil + }, func(target *input, value Supplied[string]) { target.color = value }), + RepeatFlag(FlagMeta{Name: "tag"}, nil, func(value string) (string, error) { + if value == "bad" { + return "", tagFailure + } + return value, nil + }, func(target *input, value Supplied[[]string]) { target.tags = value }), + }, + Run: func(Invocation, input) Outcome { return Success() }, + }) + app, err := New(Identity{Name: "Example"}, command, + Alias("shortcut", "inspect"), + FlagAliasFor("inspect", "--colour", "--color"), + FlagAliasFor("inspect", "--labels", "--tag"), + ) + if err != nil { + t.Fatal(err) + } + + for _, test := range []struct { + name string + args []string + cause error + binding string + spelling string + occurrence int + }{ + { + name: "invalid scalar before valid overwrite", + args: []string{"shortcut", "--colour=bad", "--color=good"}, + cause: colorFailure, + binding: "color", + spelling: "--colour", + occurrence: 1, + }, + { + name: "invalid repeat occurrence after mixed spelling", + args: []string{"shortcut", "--labels=first", "--tag=bad"}, + cause: tagFailure, + binding: "tag", + spelling: "--tag", + occurrence: 2, + }, + } { + t.Run(test.name, func(t *testing.T) { + var diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), test.args, Streams{Out: io.Discard, Err: &diagnostics}) + if outcome.Kind != OutcomeUsage { + t.Fatalf("outcome kind = %v, want usage", outcome.Kind) + } + if !errors.Is(outcome.Cause, test.cause) { + t.Fatalf("outcome cause = %v, want wrapped %v", outcome.Cause, test.cause) + } + var conversionErr *flagConversionError + if !errors.As(outcome.Cause, &conversionErr) { + t.Fatalf("outcome cause %T does not expose flag conversion context", outcome.Cause) + } + if conversionErr.binding != test.binding || conversionErr.spelling != test.spelling || conversionErr.occurrence != test.occurrence { + t.Fatalf("conversion error = %#v, want binding/spelling/occurrence for %s %d", conversionErr, test.spelling, test.occurrence) + } + for _, fragment := range []string{fmt.Sprintf("flag %s occurrence %d: %s", test.spelling, test.occurrence, test.cause), "Usage: Example shortcut", "Alias for: Example inspect"} { + if !strings.Contains(diagnostics.String(), fragment) { + t.Errorf("diagnostics %q do not contain %q", diagnostics.String(), fragment) + } + } + }) + } +} + +func TestAliasUsageSurvivesRegistrationDuringExecution(t *testing.T) { + for _, test := range []struct { + name string + register func(*App) error + wantErr bool + }{ + { + name: "successful registration", + register: func(app *App) error { + return app.Register(NewCommand(CommandSpec[struct{}]{Name: "late", Run: func(Invocation, struct{}) Outcome { return Success() }})) + }, + }, + { + name: "failed registration", + register: func(app *App) error { return app.Register(Alias("shortcut", "other")) }, + wantErr: true, + }, + } { + t.Run(test.name, func(t *testing.T) { + var app *App + var registrationErr error + command := NewCommand(CommandSpec[struct{}]{ + Name: "inspect", + Run: func(Invocation, struct{}) Outcome { + registrationErr = test.register(app) + return Usage(errors.New("operation rejected")) + }, + }) + var err error + app, err = New(Identity{Name: "Example"}, command, Alias("shortcut", "inspect")) + if err != nil { + t.Fatal(err) + } + var diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), []string{"shortcut"}, Streams{Out: io.Discard, Err: &diagnostics}) + if outcome.Kind != OutcomeUsage { + t.Fatalf("outcome kind = %v, want usage", outcome.Kind) + } + if (registrationErr != nil) != test.wantErr { + t.Fatalf("registration error = %v, want error %t", registrationErr, test.wantErr) + } + for _, fragment := range []string{"operation rejected", "Usage: Example shortcut", "Alias for: Example inspect"} { + if !strings.Contains(diagnostics.String(), fragment) { + t.Errorf("diagnostics %q do not contain %q", diagnostics.String(), fragment) + } + } + }) + } +} + +func TestUnresolvedRouteUsageHasNoCanonicalAliasAnnotation(t *testing.T) { + app := routeTestApp(t) + var diagnostics bytes.Buffer + outcome := app.Execute(context.Background(), []string{"missing", "route"}, Streams{Out: io.Discard, Err: &diagnostics}) + if outcome.Kind != OutcomeUsage { + t.Fatalf("outcome kind = %v, want usage", outcome.Kind) + } + if strings.Contains(diagnostics.String(), "Alias for:") { + t.Fatalf("unresolved route diagnostics invented a canonical target: %q", diagnostics.String()) + } + + diagnostics.Reset() + outcome = app.Execute(context.Background(), []string{"r", "full", "--unknown"}, Streams{Out: io.Discard, Err: &diagnostics}) + if outcome.Kind != OutcomeUsage { + t.Fatalf("unknown flag outcome kind = %v, want usage", outcome.Kind) + } + for _, fragment := range []string{"unknown flag", "Usage: Example r full", "Alias for: Example inspect"} { + if !strings.Contains(diagnostics.String(), fragment) { + t.Errorf("unknown flag diagnostics %q do not contain %q", diagnostics.String(), fragment) + } + } +} + func TestExecuteAppliesGlobalHelpAliasAtApplicationRoot(t *testing.T) { app, err := New(Identity{Name: "Example"}, FlagAlias("--hlep", "--help")) if err != nil {