Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add string slice arguments to our parsed_flags library #1852

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: add string slice arguments to our parsed_flags library #1852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8b1baaf
Add support for string slice arguments
Nov 27, 2023
e485bf5
Add the stringslice type
Nov 27, 2023
965a9f2
Add processor
Nov 27, 2023
40b4e8d
Block default strings that aren't empty strings
Nov 27, 2023
6d34b19
Merge branch 'main' into omar/string-slice-args
omar711 Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cli/cli/command_framework/lowlevel/flags/flag_type.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ const (
FlagType_String FlagType = iota // This is intentionally the first value, meaning it will be the emptyval/default
FlagType_Uint32
FlagType_Bool
FlagType_StringSlice
)
28 changes: 25 additions & 3 deletions cli/cli/command_framework/lowlevel/flags/flag_type_processors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ type flagTypeProcessor func(

// Completeness enforced via unit test
var AllFlagTypeProcessors = map[FlagType]flagTypeProcessor{
FlagType_String: processStringFlag,
FlagType_Uint32: processUint32Flag,
FlagType_Bool: processBoolFlag,
FlagType_String: processStringFlag,
FlagType_StringSlice: processStringSliceFlag,
FlagType_Uint32: processUint32Flag,
FlagType_Bool: processBoolFlag,
}

func processStringFlag(
Expand All @@ -45,6 +46,27 @@ func processStringFlag(
return nil
}

func processStringSliceFlag(
flagKey string,
shorthand string,
defaultValueStr string,
usage string,
cobraFlagSet *flag.FlagSet,
) error {

if defaultValueStr != "" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an assumption in the library that defaults will always derive from strings.

We could look to parse string representations of arrays, or add more genral support e.g. for []string here.

However, my thinking is defaults don't make much sense for a slice flag, so just make it clear it won't work, where somebody does specify a default.

return stacktrace.NewError("Default values not supported for string slice flags; please use an empty string")
}

cobraFlagSet.StringSliceP(
flagKey,
shorthand,
[]string{},
usage,
)
return nil
}

func processUint32Flag(
flagKey string,
shorthand string,
Expand Down
12 changes: 8 additions & 4 deletions cli/cli/command_framework/lowlevel/flags/flagtype_enumer.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions cli/cli/command_framework/lowlevel/flags/parsed_flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ func (flags *ParsedFlags) GetString(name string) (string, error) {
}
return value, nil
}

func (flags *ParsedFlags) GetStringSlice(name string) ([]string, error) {
value, err := flags.cmdFlagsSet.GetStringSlice(name)
if err != nil {
return []string{}, stacktrace.Propagate(
err,
"An error occurred getting string slice flag '%v'",
name,
)
}
return value, nil
}

func (flags *ParsedFlags) GetUint32(name string) (uint32, error) {
value, err := flags.cmdFlagsSet.GetUint32(name)
if err != nil {
Expand All @@ -34,6 +47,7 @@ func (flags *ParsedFlags) GetUint32(name string) (uint32, error) {
}
return value, nil
}

func (flags *ParsedFlags) GetBool(name string) (bool, error) {
value, err := flags.cmdFlagsSet.GetBool(name)
if err != nil {
Expand Down