Skip to content

Commit

Permalink
mv flag.SetBy -> value.UpdatedBy()
Browse files Browse the repository at this point in the history
Move more mutable state out of flags!
  • Loading branch information
bbkane committed Dec 29, 2024
1 parent 2bbf4e9 commit da8bf7c
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 23 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ below this description) is likely unreleased.

## Changed

- Moved `SetBy` into the `Value` interface - this allows `Flag` to be readonly and we need to update `SetBy` anyway
- 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

# v0.0.25

Expand Down
19 changes: 7 additions & 12 deletions app_parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,7 @@ func resolveFlag(
}
}

if fl.SetBy == "" && len(strValues) > 0 {

if fl.Value.UpdatedBy() == value.UpdatedByUnset && len(strValues) > 0 {
_, isScalar := val.(value.ScalarValue)
if isScalar && len(strValues) > 1 {
return fmt.Errorf("flag error: %v: flag passed multiple times, it's value (type %v), can only be updated once", name, fl.Value.Description())
Expand All @@ -224,14 +223,13 @@ func resolveFlag(
if err != nil {
return fmt.Errorf("error updating flag %v from passed flag value %v: %w", name, v, err)
}
fl.SetBy = "passedflag"
}
}
}

// update from config
{
if canUpdate && fl.SetBy == "" && configReader != nil {
if canUpdate && fl.Value.UpdatedBy() == value.UpdatedByUnset && configReader != nil {
fpr, err := configReader.Search(fl.ConfigPath)
if err != nil {
return err
Expand All @@ -247,7 +245,6 @@ func resolveFlag(
err,
)
}
fl.SetBy = "config"
} else {
v, ok := fl.Value.(value.SliceValue)
if !ok {
Expand All @@ -264,7 +261,6 @@ func resolveFlag(
return fmt.Errorf("could not update container type value: err: %w", err)
}
}
fl.SetBy = "config"
fl.Value = v
}
}
Expand All @@ -273,15 +269,14 @@ func resolveFlag(

// update from envvars
{
if canUpdate && fl.SetBy == "" && len(fl.EnvVars) > 0 {
if canUpdate && fl.Value.UpdatedBy() == value.UpdatedByUnset && len(fl.EnvVars) > 0 {
for _, e := range fl.EnvVars {
val, exists := lookupEnv(e)
if exists {
err := fl.Value.Update(val, value.UpdatedByEnvVar)
if err != nil {
return fmt.Errorf("error updating flag %v from envvar %v: %w", name, val, err)
}
fl.SetBy = "envvar"
break // stop looking for envvars
}

Expand All @@ -291,9 +286,8 @@ func resolveFlag(

// update from default
{
if canUpdate && fl.SetBy == "" && fl.Value.HasDefault() {
if canUpdate && fl.Value.UpdatedBy() == value.UpdatedByUnset && fl.Value.HasDefault() {
fl.Value.ReplaceFromDefault(value.UpdatedByDefault)
fl.SetBy = "appdefault"
}
}

Expand Down Expand Up @@ -467,7 +461,8 @@ func (app *App) parseWithOptHolder(parseOptHolder ParseOptHolder) (*ParseResult,
}

if !gar.HelpPassed {
if fl.Required && fl.SetBy == "" {
// TODO: do I need all the checks
if fl.Required && fl.Value.UpdatedBy() == value.UpdatedByUnset {
return nil, fmt.Errorf("flag required but not set: %s", name)
}
}
Expand All @@ -488,7 +483,7 @@ func (app *App) parseWithOptHolder(parseOptHolder ParseOptHolder) (*ParseResult,

pfs := make(command.PassedFlags)
for name, fl := range ftar.AllowedFlags {
if fl.SetBy != "" {
if fl.Value.UpdatedBy() != value.UpdatedByUnset {
pfs[string(name)] = fl.Value.Get()
}
}
Expand Down
4 changes: 0 additions & 4 deletions flag/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ type Flag struct {
// IsCommandFlag is set when parsing. Set to true if the flag was attached to a command (as opposed to being inherited from a section)
IsCommandFlag bool

// SetBy might be set when parsing. Possible values: appdefault, config, passedflag
SetBy string

// Value is set when parsing. Use SetBy != "" to determine whether a value was actually passed instead of being empty
Value value.Value
}
Expand All @@ -90,7 +87,6 @@ func New(helpShort HelpShort, empty value.EmptyConstructor, opts ...FlagOpt) Fla
EnvVars: nil,
Required: false,
IsCommandFlag: false,
SetBy: "",
UnsetSentinel: "",
Value: nil,
}
Expand Down
8 changes: 4 additions & 4 deletions help/detailed/detailed.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,14 @@ func detailedPrintFlag(w io.Writer, color *gocolor.Color, name flag.Name, f *fla
)
}

if f.SetBy != "" {
if f.Value.UpdatedBy() != value.UpdatedByUnset {
switch v := f.Value.(type) {
case value.DictValue:
fmt.Fprintf(
w,
" %s (set by %s):\n",
color.Add(color.Bold, "currentvalue"),
color.Add(color.Bold, f.SetBy),
color.Add(color.Bold, string(f.Value.UpdatedBy())),
)
m := v.StringMap()
for _, key := range common.SortedKeys(m) {
Expand All @@ -145,7 +145,7 @@ func detailedPrintFlag(w io.Writer, color *gocolor.Color, name flag.Name, f *fla
fmt.Fprintf(w,
" %s (set by %s) :\n",
color.Add(color.Bold, "currentvalue"),
color.Add(color.Bold, f.SetBy),
color.Add(color.Bold, string(f.Value.UpdatedBy())),
)

for i, e := range v.StringSlice() {
Expand All @@ -166,7 +166,7 @@ func detailedPrintFlag(w io.Writer, color *gocolor.Color, name flag.Name, f *fla
w,
" %s (set by %s) : %s\n",
color.Add(color.Bold, "currentvalue"),
color.Add(color.Bold, f.SetBy),
color.Add(color.Bold, string(f.Value.UpdatedBy())),
v.String(),
)
default:
Expand Down
4 changes: 4 additions & 0 deletions value/dict/dict.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ func (v *dictValue[_]) Update(s string, u value.UpdatedBy) error {
return nil
}

func (v *dictValue[_]) UpdatedBy() value.UpdatedBy {
return v.updatedBy
}

func (v *dictValue[_]) ReplaceFromDefault(u value.UpdatedBy) {
if v.hasDefault {
v.vals = v.defaultVals
Expand Down
4 changes: 4 additions & 0 deletions value/scalar/scalar.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ func (v *scalarValue[T]) Update(s string, u value.UpdatedBy) error {
return nil
}

func (v *scalarValue[_]) UpdatedBy() value.UpdatedBy {
return v.updatedBy
}

func (v *scalarValue[_]) ReplaceFromDefault(u value.UpdatedBy) {
if v.defaultVal != nil {
v.updatedBy = u
Expand Down
4 changes: 4 additions & 0 deletions value/slice/slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ func (v *sliceValue[_]) Update(s string, u value.UpdatedBy) error {
return nil
}

func (v *sliceValue[_]) UpdatedBy() value.UpdatedBy {
return v.updatedBy
}

func (v *sliceValue[_]) ReplaceFromDefault(u value.UpdatedBy) {
if v.hasDefault {
v.vals = v.defaultVals
Expand Down
7 changes: 5 additions & 2 deletions value/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ type UpdatedBy string

const (
UpdatedByUnset UpdatedBy = ""
UpdatedByDefault UpdatedBy = "default"
UpdatedByDefault UpdatedBy = "appdefault"
UpdatedByEnvVar UpdatedBy = "envvar"
UpdatedByFlag UpdatedBy = "flag"
UpdatedByFlag UpdatedBy = "passedflag"
UpdatedByConfig UpdatedBy = "config"
)

Expand Down Expand Up @@ -41,6 +41,9 @@ type Value interface {
// and replaces scalar Values
Update(string, UpdatedBy) error

// UpdatedBy returns the source of the last update
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)
}
Expand Down

0 comments on commit da8bf7c

Please sign in to comment.