diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index f0cc33c..efe0910 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Invocation reporting implementation plan -Status: Stages 1–5 complete. Stage 6 is next. +Status: Stages 1–6 complete. Stage 7 is next. ## Objective and authority @@ -237,7 +237,7 @@ migration restores root-package buildability. ## Stage 6: Preserve occurrence order through the normal parser -Status: pending. +Status: complete. Primary files: [internal/cobra/flags.go](../../internal/cobra/flags.go), [internal/cobra/parse.go](../../internal/cobra/parse.go), and neutral metadata. @@ -269,6 +269,10 @@ explicit progress before/after mode flags, values containing reporting-like tokens, terminators, and malformed paths. Retain coverage for input spelling provenance and consumer binding conversion. +Focused internal package suites pass. Root reducer tests are included, but their +execution is deferred until the planned API migration restores root-package +buildability. + ## Stage 7: Connect invocation execution to one reporter Status: pending. diff --git a/internal/cobra/flags.go b/internal/cobra/flags.go index e87fabe..540386a 100644 --- a/internal/cobra/flags.go +++ b/internal/cobra/flags.go @@ -7,7 +7,7 @@ import ( commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta" ) -func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []commandmeta.FlagAlias) ([]string, map[string][]string, *commandmeta.Failure) { +func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []commandmeta.FlagAlias) ([]string, map[string][]string, []commandmeta.FlagOccurrence, *commandmeta.Failure) { canonical := make(map[string]commandmeta.Flag, len(flags)*2+2) help := commandmeta.Flag{Name: "help", Short: "h", Boolean: true} canonical["--help"] = help @@ -27,7 +27,9 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm } spellings := make(map[string][]string) + occurrences := make([]commandmeta.FlagOccurrence, 0, len(argv)) interpreted := make([]string, 0, len(argv)) + recordingOccurrences := true for index := 0; index < len(argv); { token := argv[index] if token == "--" { @@ -48,22 +50,34 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm } if !ok { interpreted = append(interpreted, token) + recordingOccurrences = false index++ continue } spellings[flag.Name] = append(spellings[flag.Name], original) if equal >= 0 { - interpreted = append(interpreted, "--"+flag.Name+"="+body[equal+1:]) + value := body[equal+1:] + interpreted = append(interpreted, "--"+flag.Name+"="+value) + if recordingOccurrences { + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) + } index++ continue } if !flag.Boolean && index+1 < len(argv) { - interpreted = append(interpreted, "--"+flag.Name+"="+argv[index+1]) + value := argv[index+1] + interpreted = append(interpreted, "--"+flag.Name+"="+value) + if recordingOccurrences { + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) + } index += 2 continue } if !flag.Boolean { - return interpreted, spellings, &commandmeta.Failure{Message: fmt.Sprintf("flag needs an argument: %s", original)} + return interpreted, spellings, occurrences, &commandmeta.Failure{Message: fmt.Sprintf("flag needs an argument: %s", original)} + } + if recordingOccurrences { + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: "true"}) } interpreted = append(interpreted, "--"+flag.Name) index++ @@ -84,15 +98,23 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm } if !ok { interpreted = append(interpreted, "-"+token[offset:]) + recordingOccurrences = false break } spellings[flag.Name] = append(spellings[flag.Name], original) suffix := token[offset+1:] if flag.Boolean { if strings.HasPrefix(suffix, "=") { - interpreted = append(interpreted, "--"+flag.Name+"="+suffix[1:]) + value := suffix[1:] + interpreted = append(interpreted, "--"+flag.Name+"="+value) + if recordingOccurrences { + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) + } break } + if recordingOccurrences { + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: "true"}) + } interpreted = append(interpreted, "--"+flag.Name) offset++ continue @@ -103,15 +125,22 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm value = value[1:] } interpreted = append(interpreted, "--"+flag.Name+"="+value) + if recordingOccurrences { + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) + } break } if index+1 < len(argv) { - interpreted = append(interpreted, "--"+flag.Name+"="+argv[index+1]) + value := argv[index+1] + interpreted = append(interpreted, "--"+flag.Name+"="+value) + if recordingOccurrences { + occurrences = append(occurrences, commandmeta.FlagOccurrence{Name: flag.Name, Spelling: original, RawValue: value}) + } index += 2 consumedNext = true break } - return interpreted, spellings, &commandmeta.Failure{Message: fmt.Sprintf("flag needs an argument: %s", original)} + return interpreted, spellings, occurrences, &commandmeta.Failure{Message: fmt.Sprintf("flag needs an argument: %s", original)} } if !consumedNext { index++ @@ -121,5 +150,5 @@ func interpretFlagTokens(argv []string, flags []commandmeta.Flag, aliases []comm interpreted = append(interpreted, token) index++ } - return interpreted, spellings, nil + return interpreted, spellings, occurrences, nil } diff --git a/internal/cobra/flags_test.go b/internal/cobra/flags_test.go index 8a5695e..0ffebb7 100644 --- a/internal/cobra/flags_test.go +++ b/internal/cobra/flags_test.go @@ -32,6 +32,74 @@ func flagGrammarOptions() ParseOptions { }} } +func reportingFlagTree() *commandmeta.Command { + return &commandmeta.Command{Name: "app", Children: []*commandmeta.Command{{ + Name: "run", Runnable: true, CoreFlags: commandmeta.ReportingFlags(), + Flags: []commandmeta.Flag{{Name: "message"}}, + }}} +} + +func reportingFlagOptions() ParseOptions { + return ParseOptions{FlagAliases: []commandmeta.FlagAlias{ + {Source: "-q", Target: "--quiet"}, + {Source: "-v", Target: "--verbose"}, + {Source: "-d", Target: "--debug"}, + }} +} + +func TestParseReportsFlagOccurrencesInEnteredOrder(t *testing.T) { + result := Parse(reportingFlagTree(), []string{ + "run", "--verbose=true", "-qd", "--progress=false", "--message=--quiet", "--", "--debug", + }, reportingFlagOptions()) + if result.Failure != nil { + t.Fatalf("parse failed: %s", result.Failure.Message) + } + want := []commandmeta.FlagOccurrence{ + {Name: "verbose", Spelling: "--verbose", RawValue: "true"}, + {Name: "quiet", Spelling: "-q", RawValue: "true"}, + {Name: "debug", Spelling: "-d", RawValue: "true"}, + {Name: "progress", Spelling: "--progress", RawValue: "false"}, + {Name: "message", Spelling: "--message", RawValue: "--quiet"}, + } + if !reflect.DeepEqual(result.FlagOccurrences, want) { + t.Fatalf("flag occurrences = %#v, want %#v", result.FlagOccurrences, want) + } + if !reflect.DeepEqual(result.FlagValues, [][]string{{"--quiet"}}) || !reflect.DeepEqual(result.FlagSpellings, [][]string{{"--message"}}) { + t.Fatalf("consumer values or spellings changed: %#v, %#v", result.FlagValues, result.FlagSpellings) + } +} + +func TestParseFlagOccurrencesRetainOnlyConsumedPrefixOnFailure(t *testing.T) { + result := Parse(reportingFlagTree(), []string{"run", "--verbose", "--unknown", "--quiet"}, reportingFlagOptions()) + if result.Failure == nil || len(result.FlagOccurrences) != 1 || result.FlagOccurrences[0].Name != "verbose" { + t.Fatalf("unknown flag result = %#v", result) + } + + result = Parse(reportingFlagTree(), []string{"run", "-qd", "--message"}, reportingFlagOptions()) + if result.Failure == nil || len(result.FlagOccurrences) != 2 || result.FlagOccurrences[0].Name != "quiet" || result.FlagOccurrences[1].Name != "debug" { + t.Fatalf("missing value result = %#v", result) + } + + result = Parse(reportingFlagTree(), []string{"missing", "--quiet"}, reportingFlagOptions()) + if result.Failure == nil || len(result.FlagOccurrences) != 0 { + t.Fatalf("malformed path result = %#v", result) + } +} + +func TestParseFlagOccurrenceKeepsOptionLookingValueAndConsumesTerminator(t *testing.T) { + result := Parse(reportingFlagTree(), []string{"run", "--message", "--quiet", "--", "--verbose"}, reportingFlagOptions()) + if result.Failure != nil { + t.Fatalf("parse failed: %s", result.Failure.Message) + } + want := []commandmeta.FlagOccurrence{{Name: "message", Spelling: "--message", RawValue: "--quiet"}} + if !reflect.DeepEqual(result.FlagOccurrences, want) { + t.Fatalf("flag occurrences = %#v, want %#v", result.FlagOccurrences, want) + } + if !reflect.DeepEqual(result.Operands, []string{"--verbose"}) { + t.Fatalf("operands = %#v", result.Operands) + } +} + func TestParseCanonicalAndAliasShortGrammar(t *testing.T) { for _, test := range []struct { name string diff --git a/internal/cobra/parse.go b/internal/cobra/parse.go index 9cdb59b..0b28c47 100644 --- a/internal/cobra/parse.go +++ b/internal/cobra/parse.go @@ -80,7 +80,7 @@ func (session *Session) Parse(argv []string) commandmeta.Result { localFlags := make([]commandmeta.Flag, 0, len(selectedNode.Flags)+len(selectedNode.CoreFlags)) localFlags = append(localFlags, selectedNode.Flags...) localFlags = append(localFlags, selectedNode.CoreFlags...) - interpreted, spellings, interpretationFailure := interpretFlagTokens(remaining, localFlags, session.flagAliases) + interpreted, spellings, occurrences, interpretationFailure := interpretFlagTokens(remaining, localFlags, session.flagAliases) for _, flag := range localFlags { if value := selected.Flags().Lookup(flag.Name); value != nil { value.Value.(*rawValue).queueSpellings(spellings[flag.Name]) @@ -90,14 +90,17 @@ func (session *Session) Parse(argv []string) commandmeta.Result { helpFlag.Value.(*rawValue).queueSpellings(spellings["help"]) } if err := selected.ParseFlags(interpreted); err != nil { + result.FlagOccurrences = occurrences return failure(result, err.Error()) } // Parse the completed prefix first so an earlier backend failure keeps its // precedence over a missing value discovered while interpreting aliases. if interpretationFailure != nil { + result.FlagOccurrences = occurrences result.Failure = interpretationFailure return result } + result.FlagOccurrences = occurrences operands := selected.Flags().Args() result.Operands = append([]string(nil), operands...) @@ -113,6 +116,15 @@ func (session *Session) Parse(argv []string) commandmeta.Result { if helpFlag := selected.Flags().Lookup("help"); helpFlag != nil { helpRequested, err := helpFlag.Value.(*rawValue).lastBool() if err != nil { + for index, occurrence := range result.FlagOccurrences { + if occurrence.Name != "help" { + continue + } + if _, parseErr := strconv.ParseBool(occurrence.RawValue); parseErr != nil { + result.FlagOccurrences = result.FlagOccurrences[:index+1] + break + } + } return failure(result, err.Error()) } result.HelpRequested = helpRequested diff --git a/internal/commandmeta/metadata.go b/internal/commandmeta/metadata.go index 9220cff..f71b49b 100644 --- a/internal/commandmeta/metadata.go +++ b/internal/commandmeta/metadata.go @@ -91,15 +91,41 @@ type Failure struct { Usage bool } +// FlagOccurrence records one parsed spelling and its unconverted value. +type FlagOccurrence struct { + Name string // canonical flag name + Spelling string // entered long spelling or individual short spelling + RawValue string // unconverted value collected by the parser +} + +// ReportingMode selects visibility for shared reporting messages. +type ReportingMode uint8 + +const ( + ReportingModeNormal ReportingMode = iota + ReportingModeQuiet + ReportingModeVerbose + ReportingModeDebug +) + +// ReportingSelection contains the resolved reporting options for one parse. +type ReportingSelection struct { + Mode ReportingMode + Progress bool + ProgressSet bool +} + // Result contains the selected command and raw input collected by the adapter. type Result struct { - Key Key - Path []string - HelpRequested bool - FlagValues [][]string - FlagSpellings [][]string - Operands []string - Failure *Failure + Key Key + Path []string + HelpRequested bool + FlagValues [][]string + FlagSpellings [][]string + FlagOccurrences []FlagOccurrence + Reporting ReportingSelection + Operands []string + Failure *Failure } // ParseOptions makes parser context and streams explicit. diff --git a/parser.go b/parser.go index 36da825..161cb0a 100644 --- a/parser.go +++ b/parser.go @@ -1,7 +1,9 @@ package commandkit import ( + "fmt" "sort" + "strconv" internalcobra "gitea.maximumdirect.net/eric/CommandKit/internal/cobra" commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta" @@ -12,7 +14,47 @@ func (a *App) parse(args []string, options commandmeta.ParseOptions) commandmeta if a == nil { return commandmeta.Result{Failure: &commandmeta.Failure{Message: "cannot parse commands on a nil app"}} } - return internalcobra.Parse(a.routeSnapshot().metadata, args, options) + result := internalcobra.Parse(a.routeSnapshot().metadata, args, options) + return reduceReportingOccurrences(result) +} + +func reduceReportingOccurrences(result commandmeta.Result) commandmeta.Result { + selection := commandmeta.ReportingSelection{} + for index, occurrence := range result.FlagOccurrences { + if !commandmeta.IsReportingFlag(occurrence.Name) { + continue + } + enabled, err := strconv.ParseBool(occurrence.RawValue) + if err != nil { + result.FlagOccurrences = result.FlagOccurrences[:index+1] + result.Reporting = selection + result.Failure = &commandmeta.Failure{ + Message: fmt.Sprintf("invalid boolean value %q for %s", occurrence.RawValue, occurrence.Spelling), + Usage: true, + } + return result + } + mode := commandmeta.ReportingModeNormal + switch occurrence.Name { + case "quiet": + mode = commandmeta.ReportingModeQuiet + case "verbose": + mode = commandmeta.ReportingModeVerbose + case "debug": + mode = commandmeta.ReportingModeDebug + case "progress": + selection.Progress = enabled + selection.ProgressSet = true + continue + } + if enabled { + selection.Mode = mode + } else if selection.Mode == mode { + selection.Mode = commandmeta.ReportingModeNormal + } + } + result.Reporting = selection + return result } func (a *App) routeSnapshot() *routingSnapshot { diff --git a/parser_reporting_test.go b/parser_reporting_test.go new file mode 100644 index 0000000..0ddc6a9 --- /dev/null +++ b/parser_reporting_test.go @@ -0,0 +1,120 @@ +package commandkit + +import ( + "reflect" + "testing" + + commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta" +) + +func newReportingParseApp(t *testing.T) *App { + t.Helper() + app, err := New(Identity{Name: "Example"}, + flagAliasCommand("run", stringFlag("message", "")), + FlagAlias("-q", "--quiet"), + FlagAlias("-v", "--verbose"), + FlagAlias("-d", "--debug"), + ) + if err != nil { + t.Fatal(err) + } + return app +} + +func TestReportingModeReductionFollowsFlagOccurrenceOrder(t *testing.T) { + for _, test := range []struct { + name string + args []string + mode commandmeta.ReportingMode + }{ + {"quiet then verbose", []string{"run", "--quiet", "--verbose"}, commandmeta.ReportingModeVerbose}, + {"debug then quiet", []string{"run", "--debug", "--quiet"}, commandmeta.ReportingModeQuiet}, + {"repeated verbose", []string{"run", "--verbose", "--verbose"}, commandmeta.ReportingModeVerbose}, + {"clear selected repeated mode", []string{"run", "--verbose", "--verbose=false"}, commandmeta.ReportingModeNormal}, + {"clear other mode has no effect", []string{"run", "--quiet", "--verbose=false"}, commandmeta.ReportingModeQuiet}, + {"clear current after switch", []string{"run", "--verbose", "--quiet", "--quiet=false"}, commandmeta.ReportingModeNormal}, + {"disabled quiet then debug", []string{"run", "--quiet=false", "--debug"}, commandmeta.ReportingModeDebug}, + {"disabled quiet leaves verbose", []string{"run", "--verbose", "--quiet=false"}, commandmeta.ReportingModeVerbose}, + } { + t.Run(test.name, func(t *testing.T) { + result := newReportingParseApp(t).parse(test.args, commandmeta.ParseOptions{}) + if result.Failure != nil || result.Reporting.Mode != test.mode { + t.Fatalf("parse result = %#v, want mode %d", result, test.mode) + } + }) + } +} + +func TestProgressSelectionIsIndependentAndUsesLastOccurrence(t *testing.T) { + for _, test := range []struct { + name string + args []string + mode commandmeta.ReportingMode + progress bool + progressSet bool + }{ + {"explicit false before debug", []string{"run", "--progress=false", "--debug"}, commandmeta.ReportingModeDebug, false, true}, + {"explicit false after debug", []string{"run", "--debug", "--progress=false"}, commandmeta.ReportingModeDebug, false, true}, + {"explicit true in verbose", []string{"run", "--verbose", "--progress"}, commandmeta.ReportingModeVerbose, true, true}, + {"quiet keeps independent setting", []string{"run", "--progress", "--quiet"}, commandmeta.ReportingModeQuiet, true, true}, + {"unset by default", []string{"run", "--verbose"}, commandmeta.ReportingModeVerbose, false, false}, + } { + t.Run(test.name, func(t *testing.T) { + result := newReportingParseApp(t).parse(test.args, commandmeta.ParseOptions{}) + selection := result.Reporting + if result.Failure != nil || selection.Mode != test.mode || selection.Progress != test.progress || selection.ProgressSet != test.progressSet { + t.Fatalf("parse selection = %#v, failure = %v", selection, result.Failure) + } + }) + } +} + +func TestReportingParseRetainsAliasesAndConsumedPrefix(t *testing.T) { + app := newReportingParseApp(t) + result := app.parse([]string{"run", "--verbose", "-q"}, commandmeta.ParseOptions{}) + if result.Failure != nil || result.Reporting.Mode != commandmeta.ReportingModeQuiet { + t.Fatalf("aliased selection = %#v", result) + } + want := []commandmeta.FlagOccurrence{ + {Name: "verbose", Spelling: "--verbose", RawValue: "true"}, + {Name: "quiet", Spelling: "-q", RawValue: "true"}, + } + if !reflect.DeepEqual(result.FlagOccurrences, want) { + t.Fatalf("occurrences = %#v, want %#v", result.FlagOccurrences, want) + } + + result = app.parse([]string{"run", "--verbose", "--unknown", "--quiet"}, commandmeta.ParseOptions{}) + if result.Failure == nil || result.Reporting.Mode != commandmeta.ReportingModeVerbose || len(result.FlagOccurrences) != 1 { + t.Fatalf("unknown flag partial selection = %#v", result) + } + + result = app.parse([]string{"run", "--verbose", "--quiet=invalid", "--debug"}, commandmeta.ParseOptions{}) + if result.Failure == nil || !result.Failure.Usage || result.Reporting.Mode != commandmeta.ReportingModeVerbose || len(result.FlagOccurrences) != 2 { + t.Fatalf("invalid boolean partial selection = %#v", result) + } +} + +func TestReportingParsePreservesValuesHelpAndTerminator(t *testing.T) { + app := newReportingParseApp(t) + result := app.parse([]string{"run", "--message=--quiet", "--verbose", "--", "--debug"}, commandmeta.ParseOptions{}) + if result.Failure != nil || result.Reporting.Mode != commandmeta.ReportingModeVerbose || !reflect.DeepEqual(result.Operands, []string{"--debug"}) { + t.Fatalf("option-looking value or terminator = %#v", result) + } + if len(result.FlagOccurrences) != 2 || result.FlagOccurrences[0].Name != "message" || result.FlagOccurrences[0].RawValue != "--quiet" { + t.Fatalf("reporting-like value became a flag occurrence: %#v", result.FlagOccurrences) + } + + result = app.parse([]string{"run", "--help=bad", "--quiet"}, commandmeta.ParseOptions{}) + if result.Failure == nil || result.Reporting.Mode != commandmeta.ReportingModeNormal || len(result.FlagOccurrences) != 1 || result.FlagOccurrences[0].Name != "help" { + t.Fatalf("invalid help boolean consumed later flags: %#v", result) + } + result = app.parse([]string{"run", "--help=false", "--quiet"}, commandmeta.ParseOptions{}) + if result.Failure != nil || result.HelpRequested || result.Reporting.Mode != commandmeta.ReportingModeQuiet { + t.Fatalf("valid false help boolean changed handling: %#v", result) + } + + result = app.parse([]string{"missing", "--quiet"}, commandmeta.ParseOptions{}) + if result.Failure == nil || len(result.FlagOccurrences) != 0 { + t.Fatalf("malformed path produced flag occurrences: %#v", result) + } +}