mcfg: move Path out of ParamValue and into Param, which allows for shuffling around a whole bunch of code to make things make a bit more sense

mcfg/cli.go

@@ -47,7 +47,7 @@ func (cli SourceCLI) Parse(cfg *Cfg) ([]ParamValue, error) {
 		args = os.Args[1:]
 	}
 
-	pvM, err := cli.cliParamVals(cfg)
+	pM, err := cli.cliParams(cfg)
 	if err != nil {
 		return nil, err
 	}
@@ -64,11 +64,11 @@ func (cli SourceCLI) Parse(cfg *Cfg) ([]ParamValue, error) {
 			pvStrVal = arg
 			pvStrValOk = true
 		} else if !cli.DisableHelpPage && arg == cliHelpArg {
-			cli.printHelp(os.Stdout, pvM)
+			cli.printHelp(os.Stdout, pM)
 			os.Stdout.Sync()
 			os.Exit(1)
 		} else {
-			for key, pv = range pvM {
+			for key, pv.Param = range pM {
 				if arg == key {
 					pvOk = true
 					break
@@ -102,7 +102,7 @@ func (cli SourceCLI) Parse(cfg *Cfg) ([]ParamValue, error) {
 			continue
 		}
 
-		pv.Value = fuzzyParse(pv.Param, pvStrVal)
+		pv.Value = pv.Param.fuzzyParse(pvStrVal)
 
 		pvs = append(pvs, pv)
 		key = ""
@@ -117,28 +117,28 @@ func (cli SourceCLI) Parse(cfg *Cfg) ([]ParamValue, error) {
 	return pvs, nil
 }
 
-func (cli SourceCLI) cliParamVals(cfg *Cfg) (map[string]ParamValue, error) {
+func (cli SourceCLI) cliParams(cfg *Cfg) (map[string]Param, error) {
-	m := map[string]ParamValue{}
+	m := map[string]Param{}
-	for _, pv := range cfg.allParamValues() {
+	for _, p := range cfg.allParams() {
-		key := strings.Join(append(pv.Path, pv.Param.Name), cliKeyJoin)
+		key := strings.Join(append(p.Path, p.Name), cliKeyJoin)
-		m[cliKeyPrefix+key] = pv
+		m[cliKeyPrefix+key] = p
 	}
 	return m, nil
 }
 
-func (cli SourceCLI) printHelp(w io.Writer, pvM map[string]ParamValue) {
+func (cli SourceCLI) printHelp(w io.Writer, pM map[string]Param) {
-	type pvEntry struct {
+	type pEntry struct {
 		arg string
-		ParamValue
+		Param
 	}
 
-	pvA := make([]pvEntry, 0, len(pvM))
+	pA := make([]pEntry, 0, len(pM))
-	for arg, pv := range pvM {
+	for arg, p := range pM {
-		pvA = append(pvA, pvEntry{arg: arg, ParamValue: pv})
+		pA = append(pA, pEntry{arg: arg, Param: p})
 	}
 
-	sort.Slice(pvA, func(i, j int) bool {
+	sort.Slice(pA, func(i, j int) bool {
-		return pvA[i].arg < pvA[j].arg
+		return pA[i].arg < pA[j].arg
 	})
 
 	fmtDefaultVal := func(ptr interface{}) string {
@@ -155,16 +155,16 @@ func (cli SourceCLI) printHelp(w io.Writer, pvM map[string]ParamValue) {
 		return fmt.Sprint(val.Interface())
 	}
 
-	for _, pvE := range pvA {
+	for _, p := range pA {
-		fmt.Fprintf(w, "\n%s", pvE.arg)
+		fmt.Fprintf(w, "\n%s", p.arg)
-		if pvE.IsBool {
+		if p.IsBool {
 			fmt.Fprintf(w, " (Flag)")
-		} else if defVal := fmtDefaultVal(pvE.Into); defVal != "" {
+		} else if defVal := fmtDefaultVal(p.Into); defVal != "" {
 			fmt.Fprintf(w, " (Default: %s)", defVal)
 		}
 		fmt.Fprintf(w, "\n")
-		if pvE.Usage != "" {
+		if p.Usage != "" {
-			fmt.Fprintln(w, "\t"+pvE.Usage)
+			fmt.Fprintln(w, "\t"+p.Usage)
 		}
 	}
 	fmt.Fprintf(w, "\n")

mcfg/cli_test.go

@@ -21,9 +21,9 @@ func TestSourceCLIHelp(t *T) {
 	src := SourceCLI{}
 
 	buf := new(bytes.Buffer)
-	pvM, err := src.cliParamVals(cfg)
+	pM, err := src.cliParams(cfg)
 	require.NoError(t, err)
-	SourceCLI{}.printHelp(buf, pvM)
+	SourceCLI{}.printHelp(buf, pM)
 
 	exp := `
 --bar (Flag)

mcfg/env.go

@@ -43,10 +43,10 @@ func (env SourceEnv) Parse(cfg *Cfg) ([]ParamValue, error) {
 		kvs = os.Environ()
 	}
 
-	pvM := map[string]ParamValue{}
+	pM := map[string]Param{}
-	for _, pv := range cfg.allParamValues() {
+	for _, p := range cfg.allParams() {
-		name := env.expectedName(pv.Path, pv.Name)
+		name := env.expectedName(p.Path, p.Name)
-		pvM[name] = pv
+		pM[name] = p
 	}
 
 	pvs := make([]ParamValue, 0, len(kvs))
@@ -56,9 +56,11 @@ func (env SourceEnv) Parse(cfg *Cfg) ([]ParamValue, error) {
 			return nil, fmt.Errorf("malformed environment kv %q", kv)
 		}
 		k, v := split[0], split[1]
-		if pv, ok := pvM[k]; ok {
+		if p, ok := pM[k]; ok {
-			pv.Value = fuzzyParse(pv.Param, v)
+			pvs = append(pvs, ParamValue{
-			pvs = append(pvs, pv)
+				Param: p,
+				Value: p.fuzzyParse(v),
+			})
 		}
 	}
 

mcfg/mcfg.go

@@ -10,7 +10,6 @@ import (
 )
 
 // TODO Sources:
-// - Env
 // - Env file
 // - JSON file
 // - YAML file
@@ -130,14 +129,9 @@ func (c *Cfg) FullName() string {
 }
 
 func (c *Cfg) populateParams(src Source) error {
-	// we allow for nil Source here for tests
+	pvs, err := src.Parse(c)
-	// TODO make Source stub type which tests could use here instead
+	if err != nil {
-	var pvs []ParamValue
+		return err
-	if src != nil {
-		var err error
-		if pvs, err = src.Parse(c); err != nil {
-			return err
-		}
 	}
 
 	// dedupe the ParamValues based on their hashes, with the last ParamValue
@@ -148,11 +142,11 @@ func (c *Cfg) populateParams(src Source) error {
 	}
 
 	// check for required params
-	for _, pv := range c.allParamValues() {
+	for _, p := range c.allParams() {
-		if !pv.Param.Required {
+		if !p.Required {
 			continue
-		} else if _, ok := pvM[pv.hash()]; !ok {
+		} else if _, ok := pvM[p.hash()]; !ok {
-			return fmt.Errorf("param %q is required", pv.displayName())
+			return fmt.Errorf("param %q is required", p.fullName())
 		}
 	}
 

mcfg/param.go

@@ -1,6 +1,9 @@
 package mcfg
 
 import (
+	"crypto/md5"
+	"encoding/hex"
+	"encoding/json"
 	"fmt"
 	"strings"
 
@@ -36,6 +39,39 @@ type Param struct {
 	// json.Unmarshal'd. The value being pointed to also determines the default
 	// value of the parameter.
 	Into interface{}
+
+	// The Path field of the Cfg this Param is attached to. NOTE that this
+	// will be automatically filled in when the Param is added to the Cfg.
+	Path []string
+}
+
+func (p Param) fuzzyParse(v string) json.RawMessage {
+	if p.IsBool {
+		if v == "" || v == "0" || v == "false" {
+			return json.RawMessage("false")
+		}
+		return json.RawMessage("true")
+
+	} else if p.IsString && (v == "" || v[0] != '"') {
+		return json.RawMessage(`"` + v + `"`)
+	}
+
+	return json.RawMessage(v)
+}
+
+func (p Param) fullName() string {
+	return strings.Join(append(p.Path, p.Name), "-")
+}
+
+func (p Param) hash() string {
+	h := md5.New()
+	for _, path := range p.Path {
+		fmt.Fprintf(h, "pathEl:%q\n", path)
+	}
+	fmt.Fprintf(h, "name:%q\n", p.Name)
+	hStr := hex.EncodeToString(h.Sum(nil))
+	// we add the displayName to it to make debugging easier
+	return p.fullName() + "/" + hStr
 }
 
 // ParamAdd adds the given Param to the Cfg. It will panic if a Param of the
@@ -45,9 +81,22 @@ func (c *Cfg) ParamAdd(p Param) {
 	if _, ok := c.Params[p.Name]; ok {
 		panic(fmt.Sprintf("Cfg.Path:%#v name:%q already exists", c.Path, p.Name))
 	}
+	p.Path = c.Path
 	c.Params[p.Name] = p
 }
 
+func (c *Cfg) allParams() []Param {
+	params := make([]Param, 0, len(c.Params))
+	for _, p := range c.Params {
+		params = append(params, p)
+	}
+
+	for _, child := range c.Children {
+		params = append(params, child.allParams()...)
+	}
+	return params
+}
+
 // ParamInt64 returns an *int64 which will be populated once the Cfg is run
 func (c *Cfg) ParamInt64(name string, defaultVal int64, usage string) *int64 {
 	i := defaultVal

mcfg/source.go

@@ -1,67 +1,16 @@
 package mcfg
 
 import (
-	"crypto/md5"
-	"encoding/hex"
 	"encoding/json"
-	"fmt"
-	"strings"
 )
 
-func fuzzyParse(p Param, v string) json.RawMessage {
-	if p.IsBool {
-		if v == "" || v == "0" || v == "false" {
-			return json.RawMessage("false")
-		}
-		return json.RawMessage("true")
-
-	} else if p.IsString && (v == "" || v[0] != '"') {
-		return json.RawMessage(`"` + v + `"`)
-	}
-
-	return json.RawMessage(v)
-}
-
-// TODO moving Path into Param would make a lot more sense
-
 // ParamValue describes a value for a parameter which has been parsed by a
 // Source
 type ParamValue struct {
 	Param
-	Path  []string // nil if root
 	Value json.RawMessage
 }
 
-func (pv ParamValue) displayName() string {
-	return strings.Join(append(pv.Path, pv.Param.Name), "-")
-}
-
-func (pv ParamValue) hash() string {
-	h := md5.New()
-	for _, path := range pv.Path {
-		fmt.Fprintf(h, "pathEl:%q\n", path)
-	}
-	fmt.Fprintf(h, "name:%q\n", pv.Param.Name)
-	hStr := hex.EncodeToString(h.Sum(nil))
-	// we add the displayName to it to make debugging easier
-	return pv.displayName() + "/" + hStr
-}
-
-func (cfg *Cfg) allParamValues() []ParamValue {
-	pvs := make([]ParamValue, 0, len(cfg.Params))
-	for _, param := range cfg.Params {
-		pvs = append(pvs, ParamValue{
-			Param: param,
-			Path:  cfg.Path,
-		})
-	}
-
-	for _, child := range cfg.Children {
-		pvs = append(pvs, child.allParamValues()...)
-	}
-	return pvs
-}
-
 // Source parses ParamValues out of a particular configuration source. The
 // returned []ParamValue may contain duplicates of the same Param's value.
 type Source interface {
@@ -94,10 +43,12 @@ type SourceMap map[string]string
 
 func (m SourceMap) Parse(c *Cfg) ([]ParamValue, error) {
 	pvs := make([]ParamValue, 0, len(m))
-	for _, pv := range c.allParamValues() {
+	for _, p := range c.allParams() {
-		if v, ok := m[pv.displayName()]; ok {
+		if v, ok := m[p.fullName()]; ok {
-			pv.Value = fuzzyParse(pv.Param, v)
+			pvs = append(pvs, ParamValue{
-			pvs = append(pvs, pv)
+				Param: p,
+				Value: p.fuzzyParse(v),
+			})
 		}
 	}
 	return pvs, nil

mcfg/source_test.go

@@ -105,12 +105,10 @@ func (scs srcCommonState) applyCfgAndPV(p srcCommonParams) srcCommonState {
 		// those are only used by Cfg once it has all ParamValues together
 	}
 	thisCfg.ParamAdd(cfgP)
+	cfgP = thisCfg.Params[p.name] // get it back out to get any added fields
 
 	if !p.unset {
-		pv := ParamValue{
+		pv := ParamValue{Param: cfgP}
-			Param: cfgP,
-			Path:  p.path,
-		}
 		if p.isBool {
 			pv.Value = json.RawMessage("true")
 		} else {