From bafbed5857f5189a6c769eafc3218ecdae7122c1 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Wed, 25 Dec 2024 17:31:10 -0800 Subject: [PATCH 01/20] partial parse2 work --- app_parse2.go | 155 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 app_parse2.go diff --git a/app_parse2.go b/app_parse2.go new file mode 100644 index 0000000..82d8dee --- /dev/null +++ b/app_parse2.go @@ -0,0 +1,155 @@ +package warg + +import ( + "fmt" + + "go.bbkane.com/warg/command" + "go.bbkane.com/warg/flag" + "go.bbkane.com/warg/section" + "go.bbkane.com/warg/value" +) + +// -- FlagValue + +type FlagValue struct { + SetBy string + Value value.Value +} + +type FlagValueMap map[flag.Name]*FlagValue + +func (m FlagValueMap) ToPassedFlags() command.PassedFlags { + pf := make(command.PassedFlags) + for name, f := range m { + if f.SetBy != "" { + pf[string(name)] = f.Value + } + } + return pf +} + +type ParseResult2 struct { + SectionPath []string + CurrentSection *section.SectionT + + CurrentCommandName command.Name + CurrentCommand *command.Command + + CurrentFlagName flag.Name + CurrentFlag *flag.Flag + + FlagValues FlagValueMap + State ParseState + HelpPassed bool +} + +type ParseState string + +const ( + Parse_ExpectingSectionOrCommand ParseState = "Parse_ExpectingSectionOrCommand" + Parse_ExpectingFlagNameOrEnd ParseState = "Parse_ExpectingFlagNameOrEnd" + Parse_ExpectingFlagValue ParseState = "Parse_ExpectingFlagValue" +) + +func (a *App) parseArgs(args []string) (ParseResult2, error) { + pr := ParseResult2{ + SectionPath: []string{}, + CurrentSection: &a.rootSection, + + CurrentCommandName: "", + CurrentCommand: nil, + + CurrentFlagName: "", + CurrentFlag: nil, + FlagValues: make(FlagValueMap), + + HelpPassed: false, + + State: Parse_ExpectingSectionOrCommand, + } + + // fill the FlagValues map with empty values from the app + for flagName := range a.globalFlags { + val, err := a.globalFlags[flagName].EmptyValueConstructor() + // TODO: make this not an error! + if err != nil { + panic(err) + } + pr.FlagValues[flagName] = &FlagValue{ + SetBy: "", + Value: val, + } + } + + for i, arg := range args { + + // --help or --help must be the last thing passed and can appear at any state we aren't expecting a flag value + if i >= len(args)-2 && + flag.Name(arg) == a.helpFlagName && + pr.State != Parse_ExpectingFlagValue { + + pr.HelpPassed = true + // set the value of --help if an arg was passed, otherwise let it resolve with the rest of them... + if i == len(args)-2 { + err := pr.FlagValues[a.helpFlagName].Value.Update(args[i+1]) + if err != nil { + return pr, fmt.Errorf("error updating help flag: %w", err) + } + pr.FlagValues[a.helpFlagName].SetBy = "passedarg" + } + + return pr, nil + } + + switch pr.State { + case Parse_ExpectingSectionOrCommand: + if childSection, exists := pr.CurrentSection.Sections[section.Name(arg)]; exists { + pr.CurrentSection = &childSection + pr.SectionPath = append(pr.SectionPath, arg) + } else if childCommand, exists := pr.CurrentSection.Commands[command.Name(arg)]; exists { + pr.CurrentCommand = &childCommand + pr.CurrentCommandName = command.Name(arg) + + for flagName := range pr.CurrentCommand.Flags { + _, exists := pr.FlagValues[flagName] + if exists { + // NOTE: move this check to app construction + panic("app flags and command flags cannot share a name: " + flagName) + } + pr.FlagValues[flagName] = &FlagValue{} + } + + pr.State = Parse_ExpectingFlagNameOrEnd + } else { + return pr, fmt.Errorf("expecting section or command, got %s", arg) + } + + case Parse_ExpectingFlagNameOrEnd: + // TODO: handle aliases of flags + if flagFromArg, exists := a.globalFlags[flag.Name(arg)]; exists { + pr.CurrentFlagName = flag.Name(arg) + pr.CurrentFlag = &flagFromArg + pr.State = Parse_ExpectingFlagValue + } else if flagFromArg, exists := pr.CurrentCommand.Flags[flag.Name(arg)]; exists { + pr.CurrentFlagName = flag.Name(arg) + pr.CurrentFlag = &flagFromArg + pr.State = Parse_ExpectingFlagValue + } else { + // return pr, fmt.Errorf("expecting command flag name %v or app flag name %v, got %s", pr.CurrentCommand.ChildrenNames(), a.GlobalFlags.SortedNames(), arg) + return pr, fmt.Errorf("expecting flag name, got %s", arg) + } + + case Parse_ExpectingFlagValue: + err := pr.FlagValues[pr.CurrentFlagName].Value.Update(arg) + if err != nil { + return pr, err + } + pr.FlagValues[pr.CurrentFlagName].SetBy = "passedarg" + pr.State = Parse_ExpectingFlagNameOrEnd + + default: + panic("unexpected state: " + pr.State) + } + } + return pr, nil +} From 8a44125a9b238a259374e9bdebfc24cc55f7b92f Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Sat, 28 Dec 2024 22:05:37 -0800 Subject: [PATCH 02/20] WIP use value.UpdatedBy --- app_parse2.go | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/app_parse2.go b/app_parse2.go index 82d8dee..682d273 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -16,13 +16,13 @@ type FlagValue struct { Value value.Value } -type FlagValueMap map[flag.Name]*FlagValue +type FlagValueMap map[flag.Name]value.Value func (m FlagValueMap) ToPassedFlags() command.PassedFlags { pf := make(command.PassedFlags) - for name, f := range m { - if f.SetBy != "" { - pf[string(name)] = f.Value + for name, v := range m { + if v.UpdatedBy() != value.UpdatedByUnset { + pf[string(name)] = v } } return pf @@ -70,15 +70,8 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { // fill the FlagValues map with empty values from the app for flagName := range a.globalFlags { - val, err := a.globalFlags[flagName].EmptyValueConstructor() - // TODO: make this not an error! - if err != nil { - panic(err) - } - pr.FlagValues[flagName] = &FlagValue{ - SetBy: "", - Value: val, - } + val := a.globalFlags[flagName].EmptyValueConstructor() + pr.FlagValues[flagName] = val } for i, arg := range args { @@ -91,11 +84,10 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { pr.HelpPassed = true // set the value of --help if an arg was passed, otherwise let it resolve with the rest of them... if i == len(args)-2 { - err := pr.FlagValues[a.helpFlagName].Value.Update(args[i+1]) + err := pr.FlagValues[a.helpFlagName].Update(args[i+1], value.UpdatedByFlag) if err != nil { return pr, fmt.Errorf("error updating help flag: %w", err) } - pr.FlagValues[a.helpFlagName].SetBy = "passedarg" } return pr, nil @@ -110,13 +102,13 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { pr.CurrentCommand = &childCommand pr.CurrentCommandName = command.Name(arg) - for flagName := range pr.CurrentCommand.Flags { + for flagName, f := range pr.CurrentCommand.Flags { _, exists := pr.FlagValues[flagName] if exists { // NOTE: move this check to app construction panic("app flags and command flags cannot share a name: " + flagName) } - pr.FlagValues[flagName] = &FlagValue{} + pr.FlagValues[flagName] = f.EmptyValueConstructor() } pr.State = Parse_ExpectingFlagNameOrEnd @@ -140,11 +132,10 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { } case Parse_ExpectingFlagValue: - err := pr.FlagValues[pr.CurrentFlagName].Value.Update(arg) + err := pr.FlagValues[pr.CurrentFlagName].Update(arg, value.UpdatedByFlag) if err != nil { return pr, err } - pr.FlagValues[pr.CurrentFlagName].SetBy = "passedarg" pr.State = Parse_ExpectingFlagNameOrEnd default: From 0c4abf90ae01ed3d396d4f203f41ddd1cd69d52b Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Sun, 29 Dec 2024 03:45:17 -0800 Subject: [PATCH 03/20] WIP - more parse code porting --- app_parse.go | 2 +- app_parse2.go | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) diff --git a/app_parse.go b/app_parse.go index 1d7bea1..2a77100 100644 --- a/app_parse.go +++ b/app_parse.go @@ -229,7 +229,7 @@ func resolveFlag( // update from config { - if canUpdate && fl.Value.UpdatedBy() == value.UpdatedByUnset && configReader != nil { + if canUpdate && fl.Value.UpdatedBy() == value.UpdatedByUnset && configReader != nil && fl.ConfigPath != "" { fpr, err := configReader.Search(fl.ConfigPath) if err != nil { return err diff --git a/app_parse2.go b/app_parse2.go index 682d273..beb33f8 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -4,7 +4,9 @@ import ( "fmt" "go.bbkane.com/warg/command" + "go.bbkane.com/warg/config" "go.bbkane.com/warg/flag" + "go.bbkane.com/warg/path" "go.bbkane.com/warg/section" "go.bbkane.com/warg/value" ) @@ -144,3 +146,127 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { } return pr, nil } + +func resolveFlag2( + flagName flag.Name, + fl flag.Flag, + flagValues FlagValueMap, // this gets updated - all other params are readonly + configReader config.Reader, + lookupEnv LookupFunc, +) (bool, error) { + + // maybe it's set by args already + isSet := flagValues[flagName].UpdatedBy() != value.UpdatedByUnset + if isSet { + return true, nil + } + + // config + if fl.ConfigPath != "" && configReader != nil { + fpr, err := configReader.Search(fl.ConfigPath) + if err != nil { + return false, err + } + if fpr != nil { + if !fpr.IsAggregated { + err := flagValues[flagName].ReplaceFromInterface(fpr.IFace, value.UpdatedByConfig) + if err != nil { + return false, fmt.Errorf( + "could not replace container type value:\nval:\n%#v\nreplacement:\n%#v\nerr: %w", + flagValues[flagName], + fpr.IFace, + err, + ) + } + } else { + v, ok := flagValues[flagName].(value.SliceValue) + if !ok { + return false, fmt.Errorf("could not update scalar value with aggregated value from config: name: %v, configPath: %v", flagName, fl.ConfigPath) + } + under, ok := fpr.IFace.([]interface{}) + if !ok { + return false, fmt.Errorf("expected []interface{}, got: %#v", under) + } + for _, e := range under { + err := v.AppendFromInterface(e, value.UpdatedByConfig) + if err != nil { + return false, fmt.Errorf("could not update container type value: err: %w", err) + } + } + flagValues[flagName] = v + } + } + return true, nil + } + + // envvar + for _, e := range fl.EnvVars { + val, exists := lookupEnv(e) + if exists { + err := flagValues[flagName].Update(val, value.UpdatedByEnvVar) + if err != nil { + return false, fmt.Errorf("error updating flag %v from envvar %v: %w", flagName, val, err) + } + // Use first env var found + return true, nil + } + } + + // default + if fl.Value.HasDefault() { + flagValues[flagName].ReplaceFromDefault(value.UpdatedByDefault) + return true, nil + } + return false, nil +} + +func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValueMap, lookupEnv LookupFunc) error { + // resolve config flag first and try to get a reader + var configReader config.Reader + if a.configFlagName != "" { + resolved, err := resolveFlag2( + a.configFlagName, a.globalFlags[a.configFlagName], flagValues, nil, lookupEnv) + if err != nil { + return fmt.Errorf("resolveFlag error for flag %s: %w", a.configFlagName, err) + } + if resolved { + configPath := flagValues[flag.Name(a.configFlag.ConfigPath)].Get().(path.Path) + configPathStr, err := configPath.Expand() + if err != nil { + return fmt.Errorf("error expanding config path ( %s ) : %w", configPath, err) + } + configReader, err = a.newConfigReader(configPathStr) + if err != nil { + return fmt.Errorf("error reading config path ( %s ) : %w", configPath, err) + } + + } + } + + // resolve app global flags + for flagName, fl := range a.globalFlags { + _, err := resolveFlag2(flagName, fl, flagValues, configReader, lookupEnv) + if err != nil { + return fmt.Errorf("resolveFlag error for flag %s: %w", flagName, err) + } + } + + // resolve current command flags + if currentCommand != nil { // can be nil in the case of --help + for flagName, fl := range currentCommand.Flags { + _, err := resolveFlag2(flagName, fl, flagValues, configReader, lookupEnv) + if err != nil { + return fmt.Errorf("resolveFlag error for flag %s: %w", flagName, err) + } + } + } + + return nil +} + +// next steps: +// - port gzc Parse -> Parse2 +// - make warg.Parse call Parse2 instead of doing the parsing +// - make all the tests pass (unsetsentinel, etc...) +// - delete old parsing code +// - update warg.Parse's signature and make tests pass! From 0c91b1317127909eafceebde237c8305238fbec0 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Sun, 29 Dec 2024 03:59:29 -0800 Subject: [PATCH 04/20] Update TODOs --- app_parse2.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app_parse2.go b/app_parse2.go index beb33f8..b03a1cb 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -268,5 +268,8 @@ func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValue // - port gzc Parse -> Parse2 // - make warg.Parse call Parse2 instead of doing the parsing // - make all the tests pass (unsetsentinel, etc...) +// - test against CLI apps +// - release version // - delete old parsing code -// - update warg.Parse's signature and make tests pass! +// - update warg.Parse's signature and tests +// - actually add tab completion (need to stringify values so they can be suggested as flag values) From a6f5a89ec72621f70e2ce36d856c4d8f2206eba7 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Sun, 29 Dec 2024 21:05:04 -0800 Subject: [PATCH 05/20] Parse now calls Parse2, tests are failing Need to debug tests tomorrow --- app_parse.go | 4 +- app_parse2.go | 157 +++++++++++++++++++++++++++++++++++++++++++++++++- go.mod | 2 +- 3 files changed, 159 insertions(+), 4 deletions(-) diff --git a/app_parse.go b/app_parse.go index 2a77100..2909456 100644 --- a/app_parse.go +++ b/app_parse.go @@ -391,6 +391,8 @@ func NewParseOptHolder(opts ...ParseOpt) ParseOptHolder { func (app *App) parseWithOptHolder(parseOptHolder ParseOptHolder) (*ParseResult, error) { + return app.parseWithOptHolder2(parseOptHolder) + osArgs := parseOptHolder.Args osLookupEnv := parseOptHolder.LookupFunc @@ -540,7 +542,7 @@ func (app *App) parseWithOptHolder(parseOptHolder ParseOptHolder) (*ParseResult, return &pr, nil } } - return nil, fmt.Errorf("some problem with section help: info: %v", helpInfo) + return nil, fmt.Errorf("some problem with command help: info: %v", helpInfo) } else { pr := ParseResult{ diff --git a/app_parse2.go b/app_parse2.go index b03a1cb..4a3373d 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -2,10 +2,12 @@ package warg import ( "fmt" + "slices" "go.bbkane.com/warg/command" "go.bbkane.com/warg/config" "go.bbkane.com/warg/flag" + "go.bbkane.com/warg/help/common" "go.bbkane.com/warg/path" "go.bbkane.com/warg/section" "go.bbkane.com/warg/value" @@ -24,7 +26,7 @@ func (m FlagValueMap) ToPassedFlags() command.PassedFlags { pf := make(command.PassedFlags) for name, v := range m { if v.UpdatedBy() != value.UpdatedByUnset { - pf[string(name)] = v + pf[string(name)] = v.Get() } } return pf @@ -213,7 +215,7 @@ func resolveFlag2( } // default - if fl.Value.HasDefault() { + if flagValues[flagName].HasDefault() { flagValues[flagName].ReplaceFromDefault(value.UpdatedByDefault) return true, nil } @@ -264,6 +266,157 @@ func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValue return nil } +func (a *App) Parse2(args []string, lookupEnv LookupFunc) (*ParseResult2, error) { + pr, err := a.parseArgs(args) + if err != nil { + return nil, fmt.Errorf("Parse error: %w", err) + } + + // --help means we don't need to do a lot of error checking + if pr.HelpPassed { + err = a.resolveFlags(pr.CurrentCommand, pr.FlagValues, lookupEnv) + if err != nil { + return nil, err + } + return &pr, nil + } + + // ok, we're running a real command, let's do the error checking + if pr.State != Parse_ExpectingFlagNameOrEnd { + return nil, fmt.Errorf("unexpected parse state: %s", pr.State) + } + + err = a.resolveFlags(pr.CurrentCommand, pr.FlagValues, lookupEnv) + if err != nil { + return nil, err + } + + missingRequiredFlags := []string{} + for flagName, flag := range a.globalFlags { + if flag.Required && pr.FlagValues[flagName].UpdatedBy() == value.UpdatedByUnset { + missingRequiredFlags = append(missingRequiredFlags, string(flagName)) + } + } + + for flagName, flag := range pr.CurrentCommand.Flags { + if flag.Required && pr.FlagValues[flagName].UpdatedBy() == value.UpdatedByUnset { + missingRequiredFlags = append(missingRequiredFlags, string(flagName)) + } + } + + if len(missingRequiredFlags) > 0 { + return nil, fmt.Errorf("missing but required flags: %s", missingRequiredFlags) + } + + return &pr, nil + +} + +func (app *App) parseWithOptHolder2(parseOptHolder ParseOptHolder) (*ParseResult, error) { + pr2, err := app.Parse2(parseOptHolder.Args[1:], parseOptHolder.LookupFunc) + if err != nil { + return nil, fmt.Errorf("parseWithOptHolder2 err: %w", err) + } + + // build ftar.AvailableFlags - it's a map of string to flag for the app globals + current command. Don't forget to set each flag.IsCommandFlag and Value for now.. + // TODO: + ftarAllowedFlags := make(flag.FlagMap) + for flagName, fl := range app.globalFlags { + fl.Value = pr2.FlagValues[flagName] + fl.IsCommandFlag = false + ftarAllowedFlags.AddFlag(flagName, fl) + } + + // If we're in Parse_ExpectingSectionOrCommand, we haven't received a command + if pr2.State != Parse_ExpectingSectionOrCommand { + for flagName, fl := range pr2.CurrentCommand.Flags { + fl.Value = pr2.FlagValues[flagName] + fl.IsCommandFlag = true + ftarAllowedFlags.AddFlag(flagName, fl) + } + } + + // port pfs + pfs := pr2.FlagValues.ToPassedFlags() + + // port gar.Path + garPath := slices.Concat(pr2.SectionPath, []string{string(pr2.CurrentCommandName)}) + + // TODO: handle aliases and sentinel values later + + if pr2.CurrentCommand == nil { // we got a section + // no legit actions, just print the help + helpInfo := common.HelpInfo{ + AvailableFlags: ftarAllowedFlags, + RootSection: app.rootSection, + } + // We know the helpFlag has a default so this is safe + helpType := ftarAllowedFlags[flag.Name(app.helpFlagName)].Value.Get().(string) + for _, e := range app.helpMappings { + if e.Name == helpType { + pr := ParseResult{ + Context: command.Context{ + AppName: app.name, + Context: parseOptHolder.Context, + Flags: pfs, + Path: garPath, + Stderr: parseOptHolder.Stderr, + Stdout: parseOptHolder.Stdout, + Version: app.version, + }, + Action: e.SectionHelp(pr2.CurrentSection, helpInfo), + } + return &pr, nil + } + } + return nil, fmt.Errorf("some problem with section help: info: %v", helpInfo) + } else if pr2.CurrentCommand != nil { // we got a command + if pr2.HelpPassed { + helpInfo := common.HelpInfo{ + AvailableFlags: ftarAllowedFlags, + RootSection: app.rootSection, + } + // We know the helpFlag has a default so this is safe + helpType := ftarAllowedFlags[flag.Name(app.helpFlagName)].Value.Get().(string) + for _, e := range app.helpMappings { + if e.Name == helpType { + pr := ParseResult{ + Context: command.Context{ + AppName: app.name, + Context: parseOptHolder.Context, + Flags: pfs, + Path: garPath, + Stderr: parseOptHolder.Stderr, + Stdout: parseOptHolder.Stdout, + Version: app.version, + }, + Action: e.CommandHelp(pr2.CurrentCommand, helpInfo), + } + return &pr, nil + } + } + return nil, fmt.Errorf("some problem with command help: info: %v", helpInfo) + } else { + pr := ParseResult{ + Context: command.Context{ + AppName: app.name, + Context: parseOptHolder.Context, + Flags: pfs, + Path: garPath, + Stderr: parseOptHolder.Stderr, + Stdout: parseOptHolder.Stdout, + Version: app.version, + }, + Action: pr2.CurrentCommand.Action, + } + return &pr, nil + } + + } else { + return nil, fmt.Errorf("internal Error: invalid parse state: currentSection == %v, currentCommand == %v", pr2.SectionPath, pr2.CurrentCommandName) + } +} + // next steps: // - port gzc Parse -> Parse2 // - make warg.Parse call Parse2 instead of doing the parsing diff --git a/go.mod b/go.mod index 6dedd67..1f5d2ff 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module go.bbkane.com/warg -go 1.21 +go 1.23 require ( github.com/mattn/go-isatty v0.0.20 From 17e9403629bbaa5813e9f31e65608f43a07ae740 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Tue, 31 Dec 2024 06:39:45 -0800 Subject: [PATCH 06/20] fix more tests --- app_parse2.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/app_parse2.go b/app_parse2.go index 4a3373d..d0f1aa7 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -198,7 +198,6 @@ func resolveFlag2( flagValues[flagName] = v } } - return true, nil } // envvar @@ -232,7 +231,7 @@ func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValue return fmt.Errorf("resolveFlag error for flag %s: %w", a.configFlagName, err) } if resolved { - configPath := flagValues[flag.Name(a.configFlag.ConfigPath)].Get().(path.Path) + configPath := flagValues[a.configFlagName].Get().(path.Path) configPathStr, err := configPath.Expand() if err != nil { return fmt.Errorf("error expanding config path ( %s ) : %w", configPath, err) @@ -313,6 +312,14 @@ func (a *App) Parse2(args []string, lookupEnv LookupFunc) (*ParseResult2, error) } func (app *App) parseWithOptHolder2(parseOptHolder ParseOptHolder) (*ParseResult, error) { + + // --config flag... + // original Parse treats it specially + // Parse2 expects it to be in app.GlobalFlags + if app.configFlag != nil { + app.globalFlags[app.configFlagName] = *app.configFlag + } + pr2, err := app.Parse2(parseOptHolder.Args[1:], parseOptHolder.LookupFunc) if err != nil { return nil, fmt.Errorf("parseWithOptHolder2 err: %w", err) @@ -340,7 +347,10 @@ func (app *App) parseWithOptHolder2(parseOptHolder ParseOptHolder) (*ParseResult pfs := pr2.FlagValues.ToPassedFlags() // port gar.Path - garPath := slices.Concat(pr2.SectionPath, []string{string(pr2.CurrentCommandName)}) + garPath := pr2.SectionPath + if pr2.CurrentCommandName != "" { + garPath = slices.Concat(pr2.SectionPath, []string{string(pr2.CurrentCommandName)}) + } // TODO: handle aliases and sentinel values later From 8c86c83c8abc232c20c5d297eccf58ca22f2e325 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Tue, 31 Dec 2024 18:27:21 -0800 Subject: [PATCH 07/20] Just run help if we're in a section --- app_parse2.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app_parse2.go b/app_parse2.go index d0f1aa7..657925e 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -271,6 +271,11 @@ func (a *App) Parse2(args []string, lookupEnv LookupFunc) (*ParseResult2, error) return nil, fmt.Errorf("Parse error: %w", err) } + // If we're in a section, just print the help + if pr.State == Parse_ExpectingSectionOrCommand { + pr.HelpPassed = true + } + // --help means we don't need to do a lot of error checking if pr.HelpPassed { err = a.resolveFlags(pr.CurrentCommand, pr.FlagValues, lookupEnv) From 2f238b786ca5b50e600ff9fc11de887cf39f6171 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Wed, 1 Jan 2025 07:16:20 -0800 Subject: [PATCH 08/20] Init SectionPath to nil for backcompat --- app_parse2.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app_parse2.go b/app_parse2.go index 657925e..874241e 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -57,7 +57,7 @@ const ( func (a *App) parseArgs(args []string) (ParseResult2, error) { pr := ParseResult2{ - SectionPath: []string{}, + SectionPath: nil, CurrentSection: &a.rootSection, CurrentCommandName: "", From bc1540c936d692702521dd1cea6b5db8b6524327 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Wed, 1 Jan 2025 07:21:35 -0800 Subject: [PATCH 09/20] Make configFlag tests pass I was forgetting to return early on success, so values were being updated by the config, then overwritten by the default --- app_parse2.go | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/app_parse2.go b/app_parse2.go index 874241e..47d86c6 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -155,47 +155,48 @@ func resolveFlag2( flagValues FlagValueMap, // this gets updated - all other params are readonly configReader config.Reader, lookupEnv LookupFunc, -) (bool, error) { +) error { // maybe it's set by args already - isSet := flagValues[flagName].UpdatedBy() != value.UpdatedByUnset - if isSet { - return true, nil + if flagValues[flagName].UpdatedBy() != value.UpdatedByUnset { + return nil } // config if fl.ConfigPath != "" && configReader != nil { fpr, err := configReader.Search(fl.ConfigPath) if err != nil { - return false, err + return err } if fpr != nil { if !fpr.IsAggregated { err := flagValues[flagName].ReplaceFromInterface(fpr.IFace, value.UpdatedByConfig) if err != nil { - return false, fmt.Errorf( + return fmt.Errorf( "could not replace container type value:\nval:\n%#v\nreplacement:\n%#v\nerr: %w", flagValues[flagName], fpr.IFace, err, ) } + return nil } else { v, ok := flagValues[flagName].(value.SliceValue) if !ok { - return false, fmt.Errorf("could not update scalar value with aggregated value from config: name: %v, configPath: %v", flagName, fl.ConfigPath) + return fmt.Errorf("could not update scalar value with aggregated value from config: name: %v, configPath: %v", flagName, fl.ConfigPath) } under, ok := fpr.IFace.([]interface{}) if !ok { - return false, fmt.Errorf("expected []interface{}, got: %#v", under) + return fmt.Errorf("expected []interface{}, got: %#v", under) } for _, e := range under { err := v.AppendFromInterface(e, value.UpdatedByConfig) if err != nil { - return false, fmt.Errorf("could not update container type value: err: %w", err) + return fmt.Errorf("could not update container type value: err: %w", err) } } flagValues[flagName] = v + return nil } } } @@ -206,31 +207,31 @@ func resolveFlag2( if exists { err := flagValues[flagName].Update(val, value.UpdatedByEnvVar) if err != nil { - return false, fmt.Errorf("error updating flag %v from envvar %v: %w", flagName, val, err) + return fmt.Errorf("error updating flag %v from envvar %v: %w", flagName, val, err) } // Use first env var found - return true, nil + return nil } } // default if flagValues[flagName].HasDefault() { flagValues[flagName].ReplaceFromDefault(value.UpdatedByDefault) - return true, nil + return nil } - return false, nil + return nil } func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValueMap, lookupEnv LookupFunc) error { // resolve config flag first and try to get a reader var configReader config.Reader if a.configFlagName != "" { - resolved, err := resolveFlag2( + err := resolveFlag2( a.configFlagName, a.globalFlags[a.configFlagName], flagValues, nil, lookupEnv) if err != nil { return fmt.Errorf("resolveFlag error for flag %s: %w", a.configFlagName, err) } - if resolved { + if flagValues[a.configFlagName].UpdatedBy() != value.UpdatedByUnset { configPath := flagValues[a.configFlagName].Get().(path.Path) configPathStr, err := configPath.Expand() if err != nil { @@ -246,7 +247,7 @@ func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValue // resolve app global flags for flagName, fl := range a.globalFlags { - _, err := resolveFlag2(flagName, fl, flagValues, configReader, lookupEnv) + err := resolveFlag2(flagName, fl, flagValues, configReader, lookupEnv) if err != nil { return fmt.Errorf("resolveFlag error for flag %s: %w", flagName, err) } @@ -255,7 +256,7 @@ func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValue // resolve current command flags if currentCommand != nil { // can be nil in the case of --help for flagName, fl := range currentCommand.Flags { - _, err := resolveFlag2(flagName, fl, flagValues, configReader, lookupEnv) + err := resolveFlag2(flagName, fl, flagValues, configReader, lookupEnv) if err != nil { return fmt.Errorf("resolveFlag error for flag %s: %w", flagName, err) } From 14e3706229ebcafa04db14c259eb829773eb60b0 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Wed, 1 Jan 2025 08:03:33 -0800 Subject: [PATCH 10/20] Update flag name/alias conflict validation --- app.go | 71 +++++++++++++++------------------------- app_validate_ext_test.go | 63 ++++++++++++++++++++++++++++++++--- 2 files changed, 86 insertions(+), 48 deletions(-) diff --git a/app.go b/app.go index 9076bec..4ff5b0e 100644 --- a/app.go +++ b/app.go @@ -2,6 +2,7 @@ package warg import ( + "errors" "fmt" "log" "os" @@ -259,57 +260,39 @@ func LookupMap(m map[string]string) LookupFunc { } } -type flagNameSet map[flag.Name]struct{} - -// addFlags adds a flag's name and alias to the set. Returns an error -// if the name OR alias already exists -func (fs flagNameSet) addFlags(fm flag.FlagMap) error { - for flagName := range fm { - _, exists := fs[flagName] - if exists { - return fmt.Errorf("flag or alias name exists twice: %v", flagName) - } - fs[flagName] = struct{}{} - - alias := fm[flagName].Alias - if alias != "" { - _, exists := fs[alias] - if exists { - return fmt.Errorf("flag or alias name exists twice: %v", alias) - } - fs[alias] = struct{}{} - } - } - return nil -} - -func validateFlags( +// validateFlags2 checks that global and command flag names and aliases start with "-" and are unique. +// It does not need to check the following scenarios: +// +// - global flag names don't collide with global flag names (app will panic when adding the second global flag) - TOOD: ensure there's a test for this +// - command flag names in the same command don't collide with each other (app will panic when adding the second command flag) TODO: ensure there's a test for this +// - command flag names/aliases don't collide with command flag names/aliases in other commands (since only one command will be run, this is not a problem) +func validateFlags2( globalFlags flag.FlagMap, comFlags flag.FlagMap, ) error { - nameSet := make(flagNameSet) - var err error - - err = nameSet.addFlags(globalFlags) - if err != nil { - return err + nameCount := make(map[flag.Name]int) + for name, fl := range globalFlags { + nameCount[name]++ + if fl.Alias != "" { + nameCount[fl.Alias]++ + } } - - err = nameSet.addFlags(comFlags) - if err != nil { - return err + for name, fl := range comFlags { + nameCount[name]++ + if fl.Alias != "" { + nameCount[fl.Alias]++ + } } - - // fmt.Printf("%#v\n", nameSet) - - for name := range nameSet { + var errs []error + for name, count := range nameCount { if !strings.HasPrefix(string(name), "-") { - return fmt.Errorf("flag and alias names must start with '-': %#v", name) + errs = append(errs, fmt.Errorf("flag and alias names must start with '-': %#v", name)) + } + if count > 1 { + errs = append(errs, fmt.Errorf("flag or alias name exists %d times: %v", count, name)) } } - - return nil - + return errors.Join(errs...) } // Validate checks app for creation errors. It checks: @@ -346,7 +329,7 @@ func (app *App) Validate() error { return fmt.Errorf("command names must not start with '-': %#v", name) } - err := validateFlags(app.globalFlags, com.Flags) + err := validateFlags2(app.globalFlags, com.Flags) if err != nil { return err } diff --git a/app_validate_ext_test.go b/app_validate_ext_test.go index ade8110..7d50d10 100644 --- a/app_validate_ext_test.go +++ b/app_validate_ext_test.go @@ -100,7 +100,7 @@ func TestApp_Validate(t *testing.T) { }, { - name: "flagNameAliasConflict", + name: "commandFlagAliasCommandFlagNameConflict", app: warg.New( "newAppName", section.New("", @@ -114,7 +114,62 @@ func TestApp_Validate(t *testing.T) { expectedErr: true, }, { - name: "globalFlagNameAliasConflict", + name: "commandFlagAliasGlobalFlagAliasConflict", + app: warg.New( + "newAppName", + section.New( + "help for test", + section.Command( + "com", + "help for com", + command.DoNothing, + command.Flag( + "--commandflag", + "global flag conflict", + scalar.String(), + flag.Alias("--global"), + ), + ), + ), + warg.SkipValidation(), + warg.GlobalFlag( + "--globalflag", + "global flag", + scalar.String(), + flag.Alias("--global"), + ), + ), + expectedErr: true, + }, + { + name: "commandFlagAliasGlobalFlagNameConflict", + app: warg.New( + "newAppName", + section.New( + "help for test", + section.Command( + "com", + "help for com", + command.DoNothing, + command.Flag( + "--commandflag", + "global flag conflict", + scalar.String(), + flag.Alias("--global"), + ), + ), + ), + warg.SkipValidation(), + warg.GlobalFlag( + "--global", + "global flag", + scalar.String(), + ), + ), + expectedErr: true, + }, + { + name: "commandFlagNameGlobalFlagNameConflict", app: warg.New( "newAppName", section.New( @@ -145,10 +200,10 @@ func TestApp_Validate(t *testing.T) { actualErr := tt.app.Validate() if tt.expectedErr { - require.NotNil(t, actualErr) + require.Error(t, actualErr) return } else { - require.Nil(t, actualErr) + require.NoError(t, actualErr) } }) } From 711d6e55aef534fd084777380ba8134b849f07aa Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Wed, 1 Jan 2025 08:10:01 -0800 Subject: [PATCH 11/20] Add flag alias support to Parse2 --- app_parse2.go | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/app_parse2.go b/app_parse2.go index 47d86c6..c3951f3 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -72,6 +72,13 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { State: Parse_ExpectingSectionOrCommand, } + aliasToFlagName := make(map[flag.Name]flag.Name) + for flagName, fl := range a.globalFlags { + if fl.Alias != "" { + aliasToFlagName[flag.Name(fl.Alias)] = flagName + } + } + // fill the FlagValues map with empty values from the app for flagName := range a.globalFlags { val := a.globalFlags[flagName].EmptyValueConstructor() @@ -106,28 +113,33 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { pr.CurrentCommand = &childCommand pr.CurrentCommandName = command.Name(arg) + // fill the FlagValues map with empty values from the command + // All names in (command flag names, command flag aliases, global flag names, global flag aliases) + // should be unique because app.Validate should have caught any conflicts for flagName, f := range pr.CurrentCommand.Flags { - _, exists := pr.FlagValues[flagName] - if exists { - // NOTE: move this check to app construction - panic("app flags and command flags cannot share a name: " + flagName) - } pr.FlagValues[flagName] = f.EmptyValueConstructor() - } + if f.Alias != "" { + aliasToFlagName[flag.Name(f.Alias)] = flagName + } + + } pr.State = Parse_ExpectingFlagNameOrEnd } else { return pr, fmt.Errorf("expecting section or command, got %s", arg) } case Parse_ExpectingFlagNameOrEnd: - // TODO: handle aliases of flags - if flagFromArg, exists := a.globalFlags[flag.Name(arg)]; exists { - pr.CurrentFlagName = flag.Name(arg) + flagName := flag.Name(arg) + if actualFlagName, exists := aliasToFlagName[flagName]; exists { + flagName = actualFlagName + } + if flagFromArg, exists := a.globalFlags[flagName]; exists { + pr.CurrentFlagName = flagName pr.CurrentFlag = &flagFromArg pr.State = Parse_ExpectingFlagValue - } else if flagFromArg, exists := pr.CurrentCommand.Flags[flag.Name(arg)]; exists { - pr.CurrentFlagName = flag.Name(arg) + } else if flagFromArg, exists := pr.CurrentCommand.Flags[flagName]; exists { + pr.CurrentFlagName = flagName pr.CurrentFlag = &flagFromArg pr.State = Parse_ExpectingFlagValue } else { From 0941dbb5faff697946e232904ffacedf5b020d69 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Wed, 1 Jan 2025 08:22:24 -0800 Subject: [PATCH 12/20] Error if scalar flag is passed twice --- app_parse2.go | 8 ++++++++ app_parse_ext_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/app_parse2.go b/app_parse2.go index c3951f3..bbc3cdd 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -148,6 +148,14 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { } case Parse_ExpectingFlagValue: + // don't allow scalar flags to be passed more than once + // TODO: Move this inside the scalarValue update check + if val, isScalar := pr.FlagValues[pr.CurrentFlagName].(value.ScalarValue); isScalar && val.UpdatedBy() != value.UpdatedByUnset { + return pr, fmt.Errorf("scalar flag %s passed more than once", pr.CurrentFlagName) + } + + // TODO: unset the flag if UnsetSentinel is passed. Search though global flags and command flags, reset the value to unset sentinal and store in the parseResult that it was unset so calls to resolveFlags won't set it... + err := pr.FlagValues[pr.CurrentFlagName].Update(arg, value.UpdatedByFlag) if err != nil { return pr, err diff --git a/app_parse_ext_test.go b/app_parse_ext_test.go index 6742985..8a4bc63 100644 --- a/app_parse_ext_test.go +++ b/app_parse_ext_test.go @@ -400,6 +400,32 @@ func TestApp_Parse(t *testing.T) { expectedPassedFlagValues: command.PassedFlags{"--flag": map[string]bool{"true": true, "false": false}, "--help": "default"}, expectedErr: false, }, + { + name: "scalarFlagPassedTwice", + app: warg.New( + "newAppName", + section.New( + "help for test", + section.Command( + "com", + "help for com1", + command.DoNothing, + command.Flag( + "--flag", + "flag help", + scalar.Int(), + ), + ), + ), + warg.SkipValidation(), + ), + + args: []string{"app", "com", "--flag", "1", "--flag", "2"}, + lookup: warg.LookupMap(nil), + expectedPassedPath: []string{"com"}, + expectedPassedFlagValues: command.PassedFlags{"--flag": int(1), "--help": "default"}, + expectedErr: true, + }, } for _, tt := range tests { From d1f022316b23c654cbd5649deff89a5a2a55358c Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Wed, 1 Jan 2025 15:13:00 -0800 Subject: [PATCH 13/20] Move scalar value update check inside scalar impl --- app_parse.go | 5 ++++- app_parse2.go | 11 ++++------- value/dict/dict.go | 3 ++- value/scalar/scalar.go | 14 ++++++++++++-- value/slice/slice.go | 3 ++- value/value.go | 12 +++++++++++- value/value_ext_test.go | 20 +++++++++++++------- 7 files changed, 48 insertions(+), 20 deletions(-) diff --git a/app_parse.go b/app_parse.go index 2909456..f36aee3 100644 --- a/app_parse.go +++ b/app_parse.go @@ -287,7 +287,10 @@ func resolveFlag( // update from default { if canUpdate && fl.Value.UpdatedBy() == value.UpdatedByUnset && fl.Value.HasDefault() { - fl.Value.ReplaceFromDefault(value.UpdatedByDefault) + err := fl.Value.ReplaceFromDefault(value.UpdatedByDefault) + if err != nil { + return fmt.Errorf("error updating flag %v from default: %w", name, err) + } } } diff --git a/app_parse2.go b/app_parse2.go index bbc3cdd..6abb305 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -148,12 +148,6 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { } case Parse_ExpectingFlagValue: - // don't allow scalar flags to be passed more than once - // TODO: Move this inside the scalarValue update check - if val, isScalar := pr.FlagValues[pr.CurrentFlagName].(value.ScalarValue); isScalar && val.UpdatedBy() != value.UpdatedByUnset { - return pr, fmt.Errorf("scalar flag %s passed more than once", pr.CurrentFlagName) - } - // TODO: unset the flag if UnsetSentinel is passed. Search though global flags and command flags, reset the value to unset sentinal and store in the parseResult that it was unset so calls to resolveFlags won't set it... err := pr.FlagValues[pr.CurrentFlagName].Update(arg, value.UpdatedByFlag) @@ -236,7 +230,10 @@ func resolveFlag2( // default if flagValues[flagName].HasDefault() { - flagValues[flagName].ReplaceFromDefault(value.UpdatedByDefault) + err := flagValues[flagName].ReplaceFromDefault(value.UpdatedByDefault) + if err != nil { + return fmt.Errorf("error updating flag %v from default: %w", flagName, err) + } return nil } return nil diff --git a/value/dict/dict.go b/value/dict/dict.go index fabb56e..3937c81 100644 --- a/value/dict/dict.go +++ b/value/dict/dict.go @@ -147,9 +147,10 @@ func (v *dictValue[_]) UpdatedBy() value.UpdatedBy { return v.updatedBy } -func (v *dictValue[_]) ReplaceFromDefault(u value.UpdatedBy) { +func (v *dictValue[_]) ReplaceFromDefault(u value.UpdatedBy) error { if v.hasDefault { v.vals = v.defaultVals v.updatedBy = u } + return nil } diff --git a/value/scalar/scalar.go b/value/scalar/scalar.go index efa6115..d6360c8 100644 --- a/value/scalar/scalar.go +++ b/value/scalar/scalar.go @@ -84,7 +84,10 @@ func (v *scalarValue[_]) HasDefault() bool { return v.defaultVal != nil } -func (v *scalarValue[_]) ReplaceFromInterface(iFace interface{}, u value.UpdatedBy) error { +func (v *scalarValue[T]) ReplaceFromInterface(iFace interface{}, u value.UpdatedBy) error { + if v.updatedBy != value.UpdatedByUnset { + return value.ErrUpdatedMoreThanOnce[T]{CurrentValue: v.val, UpdatedBy: v.updatedBy} + } val, err := v.inner.FromIFace(iFace) if err != nil { return err @@ -112,6 +115,9 @@ func withinChoices[T comparable](val T, choices []T) bool { } func (v *scalarValue[T]) Update(s string, u value.UpdatedBy) error { + if v.updatedBy != value.UpdatedByUnset { + return value.ErrUpdatedMoreThanOnce[T]{CurrentValue: v.val, UpdatedBy: v.updatedBy} + } val, err := v.inner.FromString(s) if err != nil { return err @@ -128,9 +134,13 @@ func (v *scalarValue[_]) UpdatedBy() value.UpdatedBy { return v.updatedBy } -func (v *scalarValue[_]) ReplaceFromDefault(u value.UpdatedBy) { +func (v *scalarValue[T]) ReplaceFromDefault(u value.UpdatedBy) error { + if v.updatedBy != value.UpdatedByUnset { + return value.ErrUpdatedMoreThanOnce[T]{CurrentValue: v.val, UpdatedBy: v.updatedBy} + } if v.defaultVal != nil { v.updatedBy = u v.val = *v.defaultVal } + return nil } diff --git a/value/slice/slice.go b/value/slice/slice.go index 7547299..98e9ca4 100644 --- a/value/slice/slice.go +++ b/value/slice/slice.go @@ -144,11 +144,12 @@ func (v *sliceValue[_]) UpdatedBy() value.UpdatedBy { return v.updatedBy } -func (v *sliceValue[_]) ReplaceFromDefault(u value.UpdatedBy) { +func (v *sliceValue[_]) ReplaceFromDefault(u value.UpdatedBy) error { if v.hasDefault { v.vals = v.defaultVals v.updatedBy = u } + return nil } func (v *sliceValue[_]) AppendFromInterface(iFace interface{}, u value.UpdatedBy) error { diff --git a/value/value.go b/value/value.go index f7104be..b739dda 100644 --- a/value/value.go +++ b/value/value.go @@ -45,7 +45,7 @@ type Value interface { UpdatedBy() UpdatedBy // ReplaceFromDefault updates the Value from a pre-set default, if one exists. use HasDefault to check whether a default exists - ReplaceFromDefault(u UpdatedBy) + ReplaceFromDefault(u UpdatedBy) error } type ScalarValue interface { @@ -96,3 +96,13 @@ type ErrInvalidChoice[T comparable] struct { func (e ErrInvalidChoice[T]) Error() string { return "invalid choice for value: choices: " + fmt.Sprint(e.Choices) } + +// ErrUpdatedMoreThanOnce is returned when a value is updated more than once. Only applicable to Scalar Values +type ErrUpdatedMoreThanOnce[T comparable] struct { + CurrentValue T + UpdatedBy UpdatedBy +} + +func (e ErrUpdatedMoreThanOnce[T]) Error() string { + return fmt.Sprintf("value already updated to %#v by %s", e.CurrentValue, e.UpdatedBy) +} diff --git a/value/value_ext_test.go b/value/value_ext_test.go index 7b0033f..c207137 100644 --- a/value/value_ext_test.go +++ b/value/value_ext_test.go @@ -10,20 +10,25 @@ import ( ) func TestIntScalar(t *testing.T) { - v := scalar.Int( + constructor := scalar.Int( scalar.Choices(1, 2), scalar.Default(2), - )() + ) + // update, then try to update again + v := constructor() err := v.Update("1", value.UpdatedByFlag) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, v.Get().(int), 1) - err = v.Update("-1", value.UpdatedByFlag) - require.NotNil(t, err) + // Should only be able to be updated once + err = v.Update("1", value.UpdatedByFlag) + require.Error(t, err) require.Equal(t, v.Get().(int), 1) - v.ReplaceFromDefault(value.UpdatedByDefault) + v = constructor() + err = v.ReplaceFromDefault(value.UpdatedByDefault) + require.NoError(t, err) require.Equal(t, v.Get().(int), 2) } @@ -61,7 +66,8 @@ func TestIntSlice(t *testing.T) { v.Get().([]int), ) - v.ReplaceFromDefault(value.UpdatedByFlag) + err = v.ReplaceFromDefault(value.UpdatedByFlag) + require.NoError(t, err) require.Equal( t, []int{1, 1, 1}, From e23b27f532da584aef50624e76ea2b5fd57fe85d Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Wed, 1 Jan 2025 16:04:05 -0800 Subject: [PATCH 14/20] Add UnsetSentinel support Only thing left is fixing the example test --- app_help_ext_test.go | 11 ++ app_parse2.go | 100 ++++++++++++------ app_parse_ext_test.go | 13 ++- example_config_flag_test.go | 2 +- example_flag_value_options_test.go | 2 +- .../TestAppHelp/toplevel/stderr.golden.txt | 0 .../TestAppHelp/toplevel/stdout.golden.txt | 13 +++ 7 files changed, 97 insertions(+), 44 deletions(-) create mode 100644 testdata/TestAppHelp/toplevel/stderr.golden.txt create mode 100644 testdata/TestAppHelp/toplevel/stdout.golden.txt diff --git a/app_help_ext_test.go b/app_help_ext_test.go index efea429..80bf8d5 100644 --- a/app_help_ext_test.go +++ b/app_help_ext_test.go @@ -95,6 +95,17 @@ func TestAppHelp(t *testing.T) { args []string lookup warg.LookupFunc }{ + // toplevel just a toplevel help! + { + name: "toplevel", + app: warg.New( + "grabbit", + grabbitSection(), + warg.SkipValidation(), + ), + args: []string{"grabbit", "--help", "detailed"}, + lookup: warg.LookupMap(nil), + }, // allcommands (no command help) { diff --git a/app_parse2.go b/app_parse2.go index 6abb305..4289c3f 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -32,6 +32,33 @@ func (m FlagValueMap) ToPassedFlags() command.PassedFlags { return pf } +// -- ParseState + +type ParseState string + +const ( + Parse_ExpectingSectionOrCommand ParseState = "Parse_ExpectingSectionOrCommand" + Parse_ExpectingFlagNameOrEnd ParseState = "Parse_ExpectingFlagNameOrEnd" + Parse_ExpectingFlagValue ParseState = "Parse_ExpectingFlagValue" +) + +// -- unsetFlagNameSet + +type UnsetFlagNameSet map[flag.Name]struct{} + +func (u UnsetFlagNameSet) Add(name flag.Name) { + u[name] = struct{}{} +} + +func (u UnsetFlagNameSet) Delete(name flag.Name) { + delete(u, name) +} + +func (u UnsetFlagNameSet) Contains(name flag.Name) bool { + _, exists := u[name] + return exists +} + type ParseResult2 struct { SectionPath []string CurrentSection *section.SectionT @@ -41,20 +68,13 @@ type ParseResult2 struct { CurrentFlagName flag.Name CurrentFlag *flag.Flag + FlagValues FlagValueMap + UnsetFlagNames UnsetFlagNameSet - FlagValues FlagValueMap - State ParseState HelpPassed bool + State ParseState } -type ParseState string - -const ( - Parse_ExpectingSectionOrCommand ParseState = "Parse_ExpectingSectionOrCommand" - Parse_ExpectingFlagNameOrEnd ParseState = "Parse_ExpectingFlagNameOrEnd" - Parse_ExpectingFlagValue ParseState = "Parse_ExpectingFlagValue" -) - func (a *App) parseArgs(args []string) (ParseResult2, error) { pr := ParseResult2{ SectionPath: nil, @@ -66,10 +86,10 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { CurrentFlagName: "", CurrentFlag: nil, FlagValues: make(FlagValueMap), + UnsetFlagNames: make(UnsetFlagNameSet), HelpPassed: false, - - State: Parse_ExpectingSectionOrCommand, + State: Parse_ExpectingSectionOrCommand, } aliasToFlagName := make(map[flag.Name]flag.Name) @@ -134,25 +154,26 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { if actualFlagName, exists := aliasToFlagName[flagName]; exists { flagName = actualFlagName } - if flagFromArg, exists := a.globalFlags[flagName]; exists { - pr.CurrentFlagName = flagName - pr.CurrentFlag = &flagFromArg - pr.State = Parse_ExpectingFlagValue - } else if flagFromArg, exists := pr.CurrentCommand.Flags[flagName]; exists { - pr.CurrentFlagName = flagName - pr.CurrentFlag = &flagFromArg - pr.State = Parse_ExpectingFlagValue - } else { + fl := findFlag(flagName, a.globalFlags, pr.CurrentCommand.Flags) + if fl == nil { // return pr, fmt.Errorf("expecting command flag name %v or app flag name %v, got %s", pr.CurrentCommand.ChildrenNames(), a.GlobalFlags.SortedNames(), arg) return pr, fmt.Errorf("expecting flag name, got %s", arg) } + pr.CurrentFlagName = flagName + pr.CurrentFlag = fl + pr.State = Parse_ExpectingFlagValue case Parse_ExpectingFlagValue: // TODO: unset the flag if UnsetSentinel is passed. Search though global flags and command flags, reset the value to unset sentinal and store in the parseResult that it was unset so calls to resolveFlags won't set it... - - err := pr.FlagValues[pr.CurrentFlagName].Update(arg, value.UpdatedByFlag) - if err != nil { - return pr, err + if arg == pr.CurrentFlag.UnsetSentinel { + pr.FlagValues[pr.CurrentFlagName] = pr.CurrentFlag.EmptyValueConstructor() + pr.UnsetFlagNames.Add(pr.CurrentFlagName) + } else { + err := pr.FlagValues[pr.CurrentFlagName].Update(arg, value.UpdatedByFlag) + if err != nil { + return pr, err + } + pr.UnsetFlagNames.Delete(pr.CurrentFlagName) } pr.State = Parse_ExpectingFlagNameOrEnd @@ -163,16 +184,27 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { return pr, nil } +func findFlag(flagName flag.Name, globalFlags flag.FlagMap, currentCommandFlags flag.FlagMap) *flag.Flag { + if fl, exists := globalFlags[flagName]; exists { + return &fl + } + if fl, exists := currentCommandFlags[flagName]; exists { + return &fl + } + return nil +} + func resolveFlag2( flagName flag.Name, fl flag.Flag, flagValues FlagValueMap, // this gets updated - all other params are readonly configReader config.Reader, lookupEnv LookupFunc, + unsetFlagNames UnsetFlagNameSet, ) error { - // maybe it's set by args already - if flagValues[flagName].UpdatedBy() != value.UpdatedByUnset { + // don't update if its been explicitly unset or already set + if unsetFlagNames.Contains(flagName) || flagValues[flagName].UpdatedBy() != value.UpdatedByUnset { return nil } @@ -239,12 +271,12 @@ func resolveFlag2( return nil } -func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValueMap, lookupEnv LookupFunc) error { +func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValueMap, lookupEnv LookupFunc, unsetFlagNames UnsetFlagNameSet) error { // resolve config flag first and try to get a reader var configReader config.Reader if a.configFlagName != "" { err := resolveFlag2( - a.configFlagName, a.globalFlags[a.configFlagName], flagValues, nil, lookupEnv) + a.configFlagName, a.globalFlags[a.configFlagName], flagValues, nil, lookupEnv, unsetFlagNames) if err != nil { return fmt.Errorf("resolveFlag error for flag %s: %w", a.configFlagName, err) } @@ -264,7 +296,7 @@ func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValue // resolve app global flags for flagName, fl := range a.globalFlags { - err := resolveFlag2(flagName, fl, flagValues, configReader, lookupEnv) + err := resolveFlag2(flagName, fl, flagValues, configReader, lookupEnv, unsetFlagNames) if err != nil { return fmt.Errorf("resolveFlag error for flag %s: %w", flagName, err) } @@ -273,7 +305,7 @@ func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValue // resolve current command flags if currentCommand != nil { // can be nil in the case of --help for flagName, fl := range currentCommand.Flags { - err := resolveFlag2(flagName, fl, flagValues, configReader, lookupEnv) + err := resolveFlag2(flagName, fl, flagValues, configReader, lookupEnv, unsetFlagNames) if err != nil { return fmt.Errorf("resolveFlag error for flag %s: %w", flagName, err) } @@ -296,7 +328,7 @@ func (a *App) Parse2(args []string, lookupEnv LookupFunc) (*ParseResult2, error) // --help means we don't need to do a lot of error checking if pr.HelpPassed { - err = a.resolveFlags(pr.CurrentCommand, pr.FlagValues, lookupEnv) + err = a.resolveFlags(pr.CurrentCommand, pr.FlagValues, lookupEnv, pr.UnsetFlagNames) if err != nil { return nil, err } @@ -308,7 +340,7 @@ func (a *App) Parse2(args []string, lookupEnv LookupFunc) (*ParseResult2, error) return nil, fmt.Errorf("unexpected parse state: %s", pr.State) } - err = a.resolveFlags(pr.CurrentCommand, pr.FlagValues, lookupEnv) + err = a.resolveFlags(pr.CurrentCommand, pr.FlagValues, lookupEnv, pr.UnsetFlagNames) if err != nil { return nil, err } @@ -451,8 +483,6 @@ func (app *App) parseWithOptHolder2(parseOptHolder ParseOptHolder) (*ParseResult } // next steps: -// - port gzc Parse -> Parse2 -// - make warg.Parse call Parse2 instead of doing the parsing // - make all the tests pass (unsetsentinel, etc...) // - test against CLI apps // - release version diff --git a/app_parse_ext_test.go b/app_parse_ext_test.go index 8a4bc63..2d42a65 100644 --- a/app_parse_ext_test.go +++ b/app_parse_ext_test.go @@ -487,8 +487,7 @@ func TestApp_Parse_unsetSetinel(t *testing.T) { expectedErr: false, }, { - // A scalar flag can only be passed once - either to set it to something or to ensure it's unset with unsetSentinel. There's no point in allowing it to be passed multiples times since all desired outcomes can be accomplished with a single pass. - name: "unsetSentinelScalarError", + name: "unsetSentinelScalarUpdate", app: warg.New( "newAppName", section.New( @@ -507,11 +506,11 @@ func TestApp_Parse_unsetSetinel(t *testing.T) { ), warg.SkipValidation(), ), - args: []string{t.Name(), "test", "--flag", "UNSET", "--flag", "justsayno"}, + args: []string{t.Name(), "test", "--flag", "UNSET", "--flag", "setAfter"}, lookup: warg.LookupMap(nil), expectedPassedPath: []string{"test"}, - expectedPassedFlagValues: nil, - expectedErr: true, + expectedPassedFlagValues: command.PassedFlags{"--flag": "setAfter", "--help": "default"}, + expectedErr: false, }, { name: "unsetSentinelSlice", @@ -553,10 +552,10 @@ func TestApp_Parse_unsetSetinel(t *testing.T) { actualPR, actualErr := tt.app.Parse(warg.OverrideArgs(tt.args), warg.OverrideLookupFunc(tt.lookup)) if tt.expectedErr { - require.NotNil(t, actualErr) + require.Error(t, actualErr) return } else { - require.Nil(t, actualErr) + require.NoError(t, actualErr) } require.Equal(t, tt.expectedPassedPath, actualPR.Context.Path) require.Equal(t, tt.expectedPassedFlagValues, actualPR.Context.Flags) diff --git a/example_config_flag_test.go b/example_config_flag_test.go index 8a44cfa..adae95a 100644 --- a/example_config_flag_test.go +++ b/example_config_flag_test.go @@ -68,7 +68,7 @@ func ExampleConfigFlag() { log.Fatalf("write error: %e", err) } app.MustRun( - warg.OverrideArgs([]string{"calc", "-c", "testdata/ExampleConfigFlag/calc.yaml", "add"}), + warg.OverrideArgs([]string{"calc", "add", "-c", "testdata/ExampleConfigFlag/calc.yaml"}), ) // Output: // Sum: 6 diff --git a/example_flag_value_options_test.go b/example_flag_value_options_test.go index 56091eb..551cdd0 100644 --- a/example_flag_value_options_test.go +++ b/example_flag_value_options_test.go @@ -90,7 +90,7 @@ func ExampleApp_Parse_flag_value_options() { log.Fatalf("write error: %e", err) } app.MustRun( - warg.OverrideArgs([]string{"calc", "-c", "testdata/ExampleFlagValueOptions/config.yaml", "show", "--scalar-flag", "b"}), + warg.OverrideArgs([]string{"calc", "show", "-c", "testdata/ExampleFlagValueOptions/config.yaml", "--scalar-flag", "b"}), ) // Output: // --scalar-flag: "b" diff --git a/testdata/TestAppHelp/toplevel/stderr.golden.txt b/testdata/TestAppHelp/toplevel/stderr.golden.txt new file mode 100644 index 0000000..e69de29 diff --git a/testdata/TestAppHelp/toplevel/stdout.golden.txt b/testdata/TestAppHelp/toplevel/stdout.golden.txt new file mode 100644 index 0000000..c68e76b --- /dev/null +++ b/testdata/TestAppHelp/toplevel/stdout.golden.txt @@ -0,0 +1,13 @@ +grab those images! + +Sections: + + config : Change grabbit's config + section2 : another section + section3 : another section + +Commands: + + command2 : another command + command3 : another command + grab : do the grabbity grabbity From 781b4ebea4e38ea6ea08df75f940e48adf9a179b Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Wed, 1 Jan 2025 16:11:11 -0800 Subject: [PATCH 15/20] Allow help flag alias when parsing --- app_help_ext_test.go | 2 +- app_parse2.go | 2 +- .../TestAppHelp/toplevel/stdout.golden.txt | 35 ++++++++++++------- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/app_help_ext_test.go b/app_help_ext_test.go index 80bf8d5..6d8c933 100644 --- a/app_help_ext_test.go +++ b/app_help_ext_test.go @@ -103,7 +103,7 @@ func TestAppHelp(t *testing.T) { grabbitSection(), warg.SkipValidation(), ), - args: []string{"grabbit", "--help", "detailed"}, + args: []string{"grabbit", "-h", "outline"}, lookup: warg.LookupMap(nil), }, diff --git a/app_parse2.go b/app_parse2.go index 4289c3f..7fa4191 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -109,7 +109,7 @@ func (a *App) parseArgs(args []string) (ParseResult2, error) { // --help or --help must be the last thing passed and can appear at any state we aren't expecting a flag value if i >= len(args)-2 && - flag.Name(arg) == a.helpFlagName && + (flag.Name(arg) == a.helpFlagName || flag.Name(arg) == a.helpFlagAlias) && pr.State != Parse_ExpectingFlagValue { pr.HelpPassed = true diff --git a/testdata/TestAppHelp/toplevel/stdout.golden.txt b/testdata/TestAppHelp/toplevel/stdout.golden.txt index c68e76b..bbdccd8 100644 --- a/testdata/TestAppHelp/toplevel/stdout.golden.txt +++ b/testdata/TestAppHelp/toplevel/stdout.golden.txt @@ -1,13 +1,22 @@ -grab those images! - -Sections: - - config : Change grabbit's config - section2 : another section - section3 : another section - -Commands: - - command2 : another command - command3 : another command - grab : do the grabbity grabbity +# grab those images! +grabbit + # another command + command2 + # another command + command3 + # do the grabbity grabbity + grab + # Change grabbit's config + config + # Edit the config. A default config will be created if it doesn't exist + edit + # path to editor + --editor + # another section + section2 + # Dummy command to pass validation + com + # another section + section3 + # Dummy command to pass validation + com From 67ac494044eb5c553bb10cb216687680275836b6 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Thu, 2 Jan 2025 20:53:57 -0800 Subject: [PATCH 16/20] Validation error on command/section name clash Closes #56 , #30 --- app.go | 21 +++++++++++++++++++++ app_parse2.go | 8 -------- app_validate_ext_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/app.go b/app.go index 4ff5b0e..a9fb67d 100644 --- a/app.go +++ b/app.go @@ -322,6 +322,27 @@ func (app *App) Validate() error { return fmt.Errorf("sections must have either child sections or child commands: %#v", secName) } + { + // child section names should not clash with child command names + nameCount := make(map[string]int) + for name := range flatSec.Sec.Commands { + nameCount[string(name)]++ + } + for name := range flatSec.Sec.Sections { + nameCount[string(name)]++ + } + errs := []error{} + for name, count := range nameCount { + if count > 1 { + errs = append(errs, fmt.Errorf("command and section name clash: %s", name)) + } + } + err := errors.Join(errs...) + if err != nil { + return fmt.Errorf("name collision: %w", err) + } + } + for name, com := range flatSec.Sec.Commands { // Commands must not start wtih "-" diff --git a/app_parse2.go b/app_parse2.go index 7fa4191..fcf103e 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -481,11 +481,3 @@ func (app *App) parseWithOptHolder2(parseOptHolder ParseOptHolder) (*ParseResult return nil, fmt.Errorf("internal Error: invalid parse state: currentSection == %v, currentCommand == %v", pr2.SectionPath, pr2.CurrentCommandName) } } - -// next steps: -// - make all the tests pass (unsetsentinel, etc...) -// - test against CLI apps -// - release version -// - delete old parsing code -// - update warg.Parse's signature and tests -// - actually add tab completion (need to stringify values so they can be suggested as flag values) diff --git a/app_validate_ext_test.go b/app_validate_ext_test.go index 7d50d10..bd8cd79 100644 --- a/app_validate_ext_test.go +++ b/app_validate_ext_test.go @@ -194,6 +194,31 @@ func TestApp_Validate(t *testing.T) { ), expectedErr: true, }, + { + name: "commandNameSectionNameConflict", + app: warg.New( + "newAppName", + section.New( + "help for test", + section.Command( + "conflict", + "help for com", + command.DoNothing, + ), + section.Section( + "conflict", + "help for section", + ), + ), + warg.SkipValidation(), + warg.GlobalFlag( + "--global", + "global flag", + scalar.String(), + ), + ), + expectedErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 2b737fae596e0294b2e7c61fa2ca9c31a3971907 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Thu, 2 Jan 2025 21:02:13 -0800 Subject: [PATCH 17/20] Test for passing a bad section/command name --- app_parse_ext_test.go | 50 ++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/app_parse_ext_test.go b/app_parse_ext_test.go index 2d42a65..f51e487 100644 --- a/app_parse_ext_test.go +++ b/app_parse_ext_test.go @@ -284,35 +284,6 @@ func TestApp_Parse(t *testing.T) { expectedPassedFlagValues: nil, expectedErr: true, }, - { - name: "addSectionFlags", - app: func() warg.App { - fm := flag.FlagMap{ - "--flag1": flag.New("--flag1 value", scalar.String()), - "--flag2": flag.New("--flag1 value", scalar.String()), - } - app := warg.New( - "newAppName", - section.New( - "help for section", - section.Command( - "test", - "help for test", - command.DoNothing, - command.ExistingFlags(fm), - ), - ), - warg.SkipValidation(), - ) - return app - }(), - - args: []string{t.Name(), "test", "--flag1", "val1"}, - lookup: warg.LookupMap(nil), - expectedPassedPath: []string{"test"}, - expectedPassedFlagValues: command.PassedFlags{"--flag1": "val1", "--help": "default"}, - expectedErr: false, - }, { name: "addCommandFlags", app: func() warg.App { @@ -400,6 +371,27 @@ func TestApp_Parse(t *testing.T) { expectedPassedFlagValues: command.PassedFlags{"--flag": map[string]bool{"true": true, "false": false}, "--help": "default"}, expectedErr: false, }, + { + name: "passAbsentSection", + app: warg.New( + "newAppName", + section.New( + "help for test", + section.Command( + "com", + "help for com", + command.DoNothing, + ), + ), + warg.SkipValidation(), + ), + + args: []string{"app", "badSectionName"}, + lookup: warg.LookupMap(nil), + expectedPassedPath: []string{"com1"}, + expectedPassedFlagValues: command.PassedFlags{"--help": "default"}, + expectedErr: true, + }, { name: "scalarFlagPassedTwice", app: warg.New( From cc48ea9461fbf99551ca86422fc7e7693ce259bd Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Thu, 2 Jan 2025 21:03:59 -0800 Subject: [PATCH 18/20] Test for passing flag before command --- app_parse_ext_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/app_parse_ext_test.go b/app_parse_ext_test.go index f51e487..7721553 100644 --- a/app_parse_ext_test.go +++ b/app_parse_ext_test.go @@ -418,6 +418,32 @@ func TestApp_Parse(t *testing.T) { expectedPassedFlagValues: command.PassedFlags{"--flag": int(1), "--help": "default"}, expectedErr: true, }, + { + name: "passedFlagBeforeCommand", + app: warg.New( + "newAppName", + section.New( + "help for test", + section.Command( + "com", + "help for com", + command.DoNothing, + command.Flag( + "--flag", + "flag help", + scalar.Int(), + ), + ), + ), + warg.SkipValidation(), + ), + + args: []string{"app", "--flag", "1", "com"}, + lookup: warg.LookupMap(nil), + expectedPassedPath: []string{"com"}, + expectedPassedFlagValues: command.PassedFlags{"--flag": int(1), "--help": "default"}, + expectedErr: true, + }, } for _, tt := range tests { From 24080f195cc254f54a1aaaa046976e692b05102b Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Thu, 2 Jan 2025 22:01:24 -0800 Subject: [PATCH 19/20] Clearer error messages --- app_parse.go | 6 ++---- app_parse2.go | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/app_parse.go b/app_parse.go index f36aee3..d166c26 100644 --- a/app_parse.go +++ b/app_parse.go @@ -393,9 +393,6 @@ func NewParseOptHolder(opts ...ParseOpt) ParseOptHolder { } func (app *App) parseWithOptHolder(parseOptHolder ParseOptHolder) (*ParseResult, error) { - - return app.parseWithOptHolder2(parseOptHolder) - osArgs := parseOptHolder.Args osLookupEnv := parseOptHolder.LookupFunc @@ -571,5 +568,6 @@ func (app *App) parseWithOptHolder(parseOptHolder ParseOptHolder) (*ParseResult, func (app *App) Parse(opts ...ParseOpt) (*ParseResult, error) { parseOptHolder := NewParseOptHolder(opts...) - return app.parseWithOptHolder(parseOptHolder) + return app.parseWithOptHolder2(parseOptHolder) + // return app.parseWithOptHolder(parseOptHolder) } diff --git a/app_parse2.go b/app_parse2.go index fcf103e..953f5e4 100644 --- a/app_parse2.go +++ b/app_parse2.go @@ -318,7 +318,7 @@ func (a *App) resolveFlags(currentCommand *command.Command, flagValues FlagValue func (a *App) Parse2(args []string, lookupEnv LookupFunc) (*ParseResult2, error) { pr, err := a.parseArgs(args) if err != nil { - return nil, fmt.Errorf("Parse error: %w", err) + return nil, fmt.Errorf("Parse args error: %w", err) } // If we're in a section, just print the help @@ -377,7 +377,7 @@ func (app *App) parseWithOptHolder2(parseOptHolder ParseOptHolder) (*ParseResult pr2, err := app.Parse2(parseOptHolder.Args[1:], parseOptHolder.LookupFunc) if err != nil { - return nil, fmt.Errorf("parseWithOptHolder2 err: %w", err) + return nil, fmt.Errorf("Parse err: %w", err) } // build ftar.AvailableFlags - it's a map of string to flag for the app globals + current command. Don't forget to set each flag.IsCommandFlag and Value for now.. From 0c09c4367a2ee8eb4b58eebc0a5ad77f50129b21 Mon Sep 17 00:00:00 2001 From: Benjamin Kane <6081085+bbkane@users.noreply.github.com> Date: Sat, 4 Jan 2025 10:12:27 -0800 Subject: [PATCH 20/20] Update CHANGELOG, WARG_PRE_V0_0_26_PARSE_ALGORITHM Examples: ``` $ go run ./examples/butler --color false -h Parse err: Parse args error: expecting section or command, got --color exit status 64 ``` ```bash $ WARG_PRE_V0_0_26_PARSE_ALGORITHM=1 go run ./examples/butler --color false -h A virtual assistant ... more ``` --- CHANGELOG.md | 26 +++++++++++++++++++++++++- app_parse.go | 4 +++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c48abcc..7aa374c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,31 @@ below this description) is likely unreleased. ## Changed -- Moved `SetBy` into the `Value` interface (`value.UpdatedBy()` - this allows `Flag` to be readonly and and makes the coupling between setting the value and updating `UpdatedBy` explicit +- Moved `SetBy` into the `Value` interface (`value.UpdatedBy()` - this allows +`Flag` to be readonly and and makes the coupling between setting the value and +updating `UpdatedBy` explicit +- Flags must now be the last things passed - i.e. `
+ ...`. In addition, the only flag allowed after a `
` is +the `--help` flag (unfortunately the `--color` flag is NOT currently allowed to +be passed for section help). This simplifies the parsing and will help with tab +completion, and after that's implemented I might try to restore the old +behavior if I get too annoyed with this limitation. Temporarily, the old +behavior can be restored by setting the `WARG_PRE_V0_0_26_PARSE_ALGORITHM` +environment variable, but I plan to remove that in the next version. + +Examples: + +``` +$ go run ./examples/butler --color false -h +Parse err: Parse args error: expecting section or command, got --color +exit status 64 +``` + +``` +$ WARG_PRE_V0_0_26_PARSE_ALGORITHM=1 go run ./examples/butler --color false -h +A virtual assistant +# ... more text ... +``` # v0.0.25 diff --git a/app_parse.go b/app_parse.go index d166c26..da0cc4a 100644 --- a/app_parse.go +++ b/app_parse.go @@ -568,6 +568,8 @@ func (app *App) parseWithOptHolder(parseOptHolder ParseOptHolder) (*ParseResult, func (app *App) Parse(opts ...ParseOpt) (*ParseResult, error) { parseOptHolder := NewParseOptHolder(opts...) + if _, exists := os.LookupEnv("WARG_PRE_V0_0_26_PARSE_ALGORITHM"); exists { + return app.parseWithOptHolder(parseOptHolder) + } return app.parseWithOptHolder2(parseOptHolder) - // return app.parseWithOptHolder(parseOptHolder) }