Skip to content

Commit

Permalink
Changes from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
dearchap committed Nov 18, 2022
1 parent dca84fb commit e6a80b5
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 32 deletions.
18 changes: 11 additions & 7 deletions flag_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,19 @@ import (
)

type FlagConfig interface {
IntBase() int
GetNumberBase() int
GetCount() *int
}

type ValueCreator[T any] interface {
Create(T, *T, FlagConfig) flag.Value
}

// Float64Flag is a flag with type float64
// FlagBase[T,VC] is a generic flag base which can be used
// as a boilerplate to implement the most common interfaces
// used by urfave/cli. T specifies the types and VC specifies
// a value creator which can be used to get the correct flag.Value
// for that type
type FlagBase[T any, VC ValueCreator[T]] struct {
Name string

Expand All @@ -34,9 +38,9 @@ type FlagBase[T any, VC ValueCreator[T]] struct {
Aliases []string
EnvVars []string

TakesFile bool
Base int
Count *int
TakesFile bool
NumberBase int
Count *int

Action func(*Context, T) error

Expand All @@ -46,8 +50,8 @@ type FlagBase[T any, VC ValueCreator[T]] struct {
value flag.Value
}

func (f *FlagBase[T, V]) IntBase() int {
return f.Base
func (f *FlagBase[T, V]) GetNumberBase() int {
return f.NumberBase
}

func (f *FlagBase[T, V]) GetCount() *int {
Expand Down
2 changes: 1 addition & 1 deletion flag_int.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func (i intValue) Create(val int, p *int, c FlagConfig) flag.Value {
*p = val
return &intValue{
val: p,
base: c.IntBase(),
base: c.GetNumberBase(),
}
}

Expand Down
2 changes: 1 addition & 1 deletion flag_int64.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func (i int64Value) Create(val int64, p *int64, c FlagConfig) flag.Value {
*p = val
return &int64Value{
val: p,
base: c.IntBase(),
base: c.GetNumberBase(),
}
}

Expand Down
24 changes: 12 additions & 12 deletions flag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,10 @@ func TestFlagsFromEnv(t *testing.T) {
{"foobar", 0, &Int64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int64 value from environment variable "SECONDS" for flag seconds: .*`},

{"1", 1, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"08", 8, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 10}, ""},
{"755", 493, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 8}, ""},
{"deadBEEF", 3735928559, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 16}, ""},
{"08", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 0}, `could not parse "08" as int value from environment variable "SECONDS" for flag seconds: .*`},
{"08", 8, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 10}, ""},
{"755", 493, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 8}, ""},
{"deadBEEF", 3735928559, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 16}, ""},
{"08", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 0}, `could not parse "08" as int value from environment variable "SECONDS" for flag seconds: .*`},
{"1.2", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as int value from environment variable "SECONDS" for flag seconds: .*`},
{"foobar", 0, &IntFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as int value from environment variable "SECONDS" for flag seconds: .*`},

Expand Down Expand Up @@ -199,18 +199,18 @@ func TestFlagsFromEnv(t *testing.T) {
{"foo,bar", newSetStringSlice("foo", "bar"), &StringSliceFlag{Name: "names", EnvVars: []string{"NAMES"}}, ""},

{"1", uint(1), &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"08", uint(8), &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 10}, ""},
{"755", uint(493), &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 8}, ""},
{"deadBEEF", uint(3735928559), &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 16}, ""},
{"08", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 0}, `could not parse "08" as uint value from environment variable "SECONDS" for flag seconds: .*`},
{"08", uint(8), &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 10}, ""},
{"755", uint(493), &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 8}, ""},
{"deadBEEF", uint(3735928559), &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 16}, ""},
{"08", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 0}, `could not parse "08" as uint value from environment variable "SECONDS" for flag seconds: .*`},
{"1.2", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint value from environment variable "SECONDS" for flag seconds: .*`},
{"foobar", 0, &UintFlag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint value from environment variable "SECONDS" for flag seconds: .*`},

{"1", uint64(1), &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, ""},
{"08", uint64(8), &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 10}, ""},
{"755", uint64(493), &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 8}, ""},
{"deadBEEF", uint64(3735928559), &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 16}, ""},
{"08", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}, Base: 0}, `could not parse "08" as uint64 value from environment variable "SECONDS" for flag seconds: .*`},
{"08", uint64(8), &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 10}, ""},
{"755", uint64(493), &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 8}, ""},
{"deadBEEF", uint64(3735928559), &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 16}, ""},
{"08", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}, NumberBase: 0}, `could not parse "08" as uint64 value from environment variable "SECONDS" for flag seconds: .*`},
{"1.2", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "1.2" as uint64 value from environment variable "SECONDS" for flag seconds: .*`},
{"foobar", 0, &Uint64Flag{Name: "seconds", EnvVars: []string{"SECONDS"}}, `could not parse "foobar" as uint64 value from environment variable "SECONDS" for flag seconds: .*`},

Expand Down
2 changes: 1 addition & 1 deletion flag_uint.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func (i uintValue) Create(val uint, p *uint, c FlagConfig) flag.Value {
*p = val
return &uintValue{
val: p,
base: c.IntBase(),
base: c.GetNumberBase(),
}
}

Expand Down
2 changes: 1 addition & 1 deletion flag_uint64.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func (i uint64Value) Create(val uint64, p *uint64, c FlagConfig) flag.Value {
*p = val
return &uint64Value{
val: p,
base: c.IntBase(),
base: c.GetNumberBase(),
}
}

Expand Down
23 changes: 16 additions & 7 deletions godoc-current.txt
Original file line number Diff line number Diff line change
Expand Up @@ -784,15 +784,18 @@ type FlagBase[T any, VC ValueCreator[T]] struct {
Aliases []string
EnvVars []string

TakesFile bool
Base int
Count *int
TakesFile bool
NumberBase int
Count *int

Action func(*Context, T) error

// Has unexported fields.
}
Float64Flag is a flag with type float64
FlagBase[T,VC] is a generic flag base which can be used as a boilerplate
to implement the most common interfaces used by urfave/cli. T specifies the
types and VC specifies a value creator which can be used to get the correct
flag.Value for that type

func (f *FlagBase[T, V]) Apply(set *flag.FlagSet) error
Apply populates the flag given the flag set and environment
Expand All @@ -811,15 +814,15 @@ func (f *FlagBase[T, V]) GetDefaultText() string
func (f *FlagBase[T, V]) GetEnvVars() []string
GetEnvVars returns the env vars for this flag

func (f *FlagBase[T, V]) GetNumberBase() int

func (f *FlagBase[T, V]) GetUsage() string
GetUsage returns the usage string for the flag

func (f *FlagBase[T, V]) GetValue() string
GetValue returns the flags value as string representation and an empty
string if the flag takes no value at all.

func (f *FlagBase[T, V]) IntBase() int

func (f *FlagBase[T, V]) IsRequired() bool
IsRequired returns whether or not the flag is required

Expand Down Expand Up @@ -850,7 +853,7 @@ type FlagCategories interface {
FlagCategories interface allows for category manipulation

type FlagConfig interface {
IntBase() int
GetNumberBase() int
GetCount() *int
}

Expand Down Expand Up @@ -955,6 +958,8 @@ func (f *Float64SliceFlag) GetCategory() string
func (f *Float64SliceFlag) GetDefaultText() string
GetDefaultText returns the default text for this flag

func (f *Float64SliceFlag) GetDestination() []float64

func (f *Float64SliceFlag) GetEnvVars() []string
GetEnvVars returns the env vars for this flag

Expand Down Expand Up @@ -983,6 +988,10 @@ func (f *Float64SliceFlag) Names() []string
func (f *Float64SliceFlag) RunAction(c *Context) error
RunAction executes flag action if set

func (f *Float64SliceFlag) SetDestination(slice []float64)

func (f *Float64SliceFlag) SetValue(slice []float64)

func (f *Float64SliceFlag) String() string
String returns a readable representation of this value (for usage defaults)

Expand Down
4 changes: 2 additions & 2 deletions sliceflag.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func unwrapFlagValue(v flag.Value) flag.Value {

// NOTE: the methods below are in this file to make use of the build constraint

/*func (f *Float64SliceFlag) SetValue(slice []float64) {
func (f *Float64SliceFlag) SetValue(slice []float64) {
f.Value = newSliceFlagValue(NewFloat64Slice, slice)
}

Expand All @@ -242,7 +242,7 @@ func (f *Float64SliceFlag) GetDestination() []float64 {
return destination.Value()
}
return nil
}*/
}

func (f *Int64SliceFlag) SetValue(slice []int64) {
f.Value = newSliceFlagValue(NewInt64Slice, slice)
Expand Down

0 comments on commit e6a80b5

Please sign in to comment.