Preserve alias context in usage errors
This commit is contained in:
56
binding.go
56
binding.go
@@ -1,6 +1,7 @@
|
|||||||
package commandkit
|
package commandkit
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"strconv"
|
"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) {
|
binding.prepare = func(values []string) (preparedValue[I], error) {
|
||||||
value := fallback
|
value := fallback
|
||||||
supplied := len(values) > 0
|
supplied := len(values) > 0
|
||||||
for _, raw := range values {
|
for index, raw := range values {
|
||||||
parsed, err := parse(raw)
|
parsed, err := parse(raw)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return preparedValue[I]{}, err
|
return preparedValue[I]{}, &flagOccurrenceError{index: index, cause: err}
|
||||||
}
|
}
|
||||||
value = parsed
|
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) {
|
binding.prepare = func(values []string) (preparedValue[I], error) {
|
||||||
value := defaultValue
|
value := defaultValue
|
||||||
supplied := len(values) > 0
|
supplied := len(values) > 0
|
||||||
for _, raw := range values {
|
for index, raw := range values {
|
||||||
parsed, err := strconv.ParseBool(raw)
|
parsed, err := strconv.ParseBool(raw)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return preparedValue[I]{}, err
|
return preparedValue[I]{}, &flagOccurrenceError{index: index, cause: err}
|
||||||
}
|
}
|
||||||
value = parsed
|
value = parsed
|
||||||
}
|
}
|
||||||
@@ -76,10 +77,10 @@ func RepeatFlag[I, V any](meta FlagMeta, defaults []V, parse func(string) (V, er
|
|||||||
supplied := len(values) > 0
|
supplied := len(values) > 0
|
||||||
if supplied {
|
if supplied {
|
||||||
parsedValues = make([]V, 0, len(values))
|
parsedValues = make([]V, 0, len(values))
|
||||||
for _, raw := range values {
|
for index, raw := range values {
|
||||||
parsed, err := parse(raw)
|
parsed, err := parse(raw)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return preparedValue[I]{}, err
|
return preparedValue[I]{}, &flagOccurrenceError{index: index, cause: err}
|
||||||
}
|
}
|
||||||
parsedValues = append(parsedValues, parsed)
|
parsedValues = append(parsedValues, parsed)
|
||||||
}
|
}
|
||||||
@@ -158,6 +159,28 @@ type preparedInput[I any] struct {
|
|||||||
values []preparedValue[I]
|
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 {
|
func (p preparedInput[I]) build() I {
|
||||||
var input I
|
var input I
|
||||||
if p.newInput != nil {
|
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
|
// creates an input value. When enforceRequired is false, missing required flags
|
||||||
// and operands are allowed so metadata help can validate supplied syntax.
|
// 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) {
|
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) {
|
if len(flagValues) != len(spec.Flags) {
|
||||||
return preparedInput[I]{}, fmt.Errorf("received %d flag value sets for %d bindings", 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))
|
preparedFlags := make([]preparedValue[I], len(spec.Flags))
|
||||||
missingRequired := ""
|
missingRequired := ""
|
||||||
for i, flag := range spec.Flags {
|
for i, flag := range spec.Flags {
|
||||||
binding := flag.binding
|
binding := flag.binding
|
||||||
value, err := binding.prepare(flagValues[i])
|
value, err := binding.prepare(flagValues[i])
|
||||||
if err != nil {
|
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)
|
return preparedInput[I]{}, fmt.Errorf("flag --%s: %w", binding.meta.Name, err)
|
||||||
}
|
}
|
||||||
preparedFlags[i] = value
|
preparedFlags[i] = value
|
||||||
|
|||||||
@@ -170,7 +170,7 @@ type commandPayload interface {
|
|||||||
hasRun() bool
|
hasRun() bool
|
||||||
bridgeFlags() []commandmeta.Flag
|
bridgeFlags() []commandmeta.Flag
|
||||||
bridgeArguments() []commandmeta.Argument
|
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 {
|
type typedCommand[I any] struct {
|
||||||
@@ -217,8 +217,8 @@ func (c typedCommand[I]) bridgeArguments() []commandmeta.Argument {
|
|||||||
return arguments
|
return arguments
|
||||||
}
|
}
|
||||||
|
|
||||||
func (c typedCommand[I]) prepare(flagValues [][]string, operands []string, enforceRequired bool) (func(*execution) Outcome, error) {
|
func (c typedCommand[I]) prepare(flagValues, flagSpellings [][]string, operands []string, enforceRequired bool) (func(*execution) Outcome, error) {
|
||||||
prepared, err := prepareInput(c.spec, flagValues, operands, enforceRequired)
|
prepared, err := prepareInputWithSpellings(c.spec, flagValues, flagSpellings, operands, enforceRequired)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
# Application-owned aliases implementation plan
|
# Application-owned aliases implementation plan
|
||||||
|
|
||||||
Status: in progress; Stages 1–10 completed.
|
Status: in progress; Stages 1–11 completed.
|
||||||
|
|
||||||
## Authority, baseline, and scope
|
## 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
|
unknown routes and flags; cancellation; output and diagnostic failures. Assert
|
||||||
semantic fragments and identities, not entire backend wording snapshots.
|
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
|
## Stage 12: Verify registration and routing interactions end to end
|
||||||
|
|
||||||
**Work**
|
**Work**
|
||||||
|
|||||||
20
execute.go
20
execute.go
@@ -5,6 +5,7 @@ import (
|
|||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
|
"strings"
|
||||||
|
|
||||||
internalcobra "gitea.maximumdirect.net/eric/CommandKit/internal/cobra"
|
internalcobra "gitea.maximumdirect.net/eric/CommandKit/internal/cobra"
|
||||||
commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta"
|
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)
|
return current.present(current.renderHelp(parsed.Path), parsed.Path)
|
||||||
}
|
}
|
||||||
if parsed.HelpRequested || !payload.hasRun() {
|
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(Usage(err), parsed.Path)
|
||||||
}
|
}
|
||||||
return current.present(current.renderHelp(parsed.Path), 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 {
|
if err != nil {
|
||||||
return current.present(Usage(err), parsed.Path)
|
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 {
|
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 {
|
func normalizeStreams(streams Streams) Streams {
|
||||||
|
|||||||
159
route_test.go
159
route_test.go
@@ -3,6 +3,8 @@ package commandkit
|
|||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"reflect"
|
"reflect"
|
||||||
"strings"
|
"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) {
|
func TestExecuteAppliesGlobalHelpAliasAtApplicationRoot(t *testing.T) {
|
||||||
app, err := New(Identity{Name: "Example"}, FlagAlias("--hlep", "--help"))
|
app, err := New(Identity{Name: "Example"}, FlagAlias("--hlep", "--help"))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
Reference in New Issue
Block a user