Output help message

[fatih]

Currently we only output the dynamically generated flags and environments if something goes wrong. There is no manual way of outputing the flags. For example if we only use the env loader, there is again no way to output the help message, because currently the help message is only defined inside the flag load.

There a two ways to do it.
#1. Create a new interface

We create a new interface which every loader could implement:

type Helper interface {
   Help() string
}

If this is implemented via a loader, we'll have another function that will print those:

func PrintHelp(helper ...Helper) {}
func PrintHelpWriter(w io.Writer, helper ...Helper) {}

//usage
multiconfig.PrintHelp(flagLoader, envLoader)

This is totally optional, but at least it would imense helpful as we could generate those dynamically, as we do for envs, flags, etc...
#2. Extend the Loader interface

This is will add a new method to the existing Loader interface:

type Loader interface {
    Load(s interface{}) error
    Help() string
}

After that, if loader.Load(s) fails it will output the help messages automatically (because the loader has access to the Help() method now. Or we could manually invoke to output the help messages.

Opinions ?

[cihangir]

Creating a new Helper interface + adding a new parameter to the current loaders will give the most extensibility.

type EnvironmentLoader struct {
    Prefix string
    CamelCase bool
        Usage func()
}

Having Usage as a property will help with custom usage messages. Implementer can easily set usage function. If Usage is nil we will generate it while Loading

[fatih]

If Usage is nil we will generate it while Loading

The problem is, loading is not context sensitive. All the loader knows, there is something that has the Load(s interface{}) error method. Nothing is known for this particular type. That's why I think we need to extend the Loader (which I btw didn't like that much, but I'm not against it).

For example mitcellh/cli does have a help method just for it:

https://godoc.org/github.com/mitchellh/cli#Command

type Command interface {
    // Help should return long-form help text that includes the command-line
    // usage, a brief few sentences explaining the function of the command,
    // and the complete list of flags the command accepts.
    Help() string

    // Run should run the actual command with the given CLI instance and
    // command-line arguments. It should return the exit status when it is
    // finished.
    Run(args []string) int

    // Synopsis should return a one-line, short synopsis of the command.
    // This should be less than 50 characters ideally.
    Synopsis() string
}

Having Help() method attached to the Loader interface will make the use more powerful, as we have access to to that particular method. Btw, we still can have Usage func() for all loaders to override the dynamically generated messages.

[jszwedko]

I'm 👍 on changing the Loader interface.

[client9]

Hi guys. Im ok with changing the Loader interface, but as mentioned in #33 I don't see how this helps with flagsets since its configured to use the existing golang flags package and uses the default flags output in a help request. Highly likely I'm missing something here but in

https://github.com/koding/multiconfig/blob/master/flag.go

in private method func (f *FlagLoader) processField(flagSet *flag.FlagSet, fieldName string, field *structs.Field) error {

contains the following snippet:

        // we only can get the value from expored fields, unexported fields panics
        if field.IsExported() {
            flagSet.Var(newFieldValue(field), flagName(fieldName), flagUsage(fieldName))
        }

which then calls another private method

func flagUsage(name string) string { return fmt.Sprintf("Change value of %s.", name) }

func flagName(name string) string { return strings.ToLower(name) }

There is no way to nicely alter the documentation per flag My "solution" is to copy the entire file just to rewrite flagUsage . This seems awkward at best.

Any thoughts on a more general solution to this part. We could

1. make an interface but that probably doesnt make sense for anything but the golang flag library
2. has a public field map[string]string that maps flagName to usage string.. defaulting to flagUsage if nothing found
3. Pass in or set a function to over-ride flagUsage If nil, uses existing function.

This last one seems simplest and matches the existing system of setting values

        &multiconfig.FlagLoader{CamelCase: true, EnvPrefix: envPrefix, UsageFN: MY FUNCTION},

and maybe about 5 LOC to impliment.

I'm happy to do the work in any of them, but would love some direction on the approach you are most comfortable with.

thanks for your time!

nickg

[client9]

hi @fatih I'm sure you are busy but any thoughts on the above proposal to allow customization of FlagLoader ? Regards, --nickg

[fatih]

@client9 Sorry I was on vacation. I think the best way is to provide a Usage field to the FlagLoader. If it's present, we'll use that one. Another thing would be to improve adding help message via field tags. Such as :

type T struct {
    Port `flagUsage: "Port is used to specify the server port"`
}

But this is not related with your 3 point. So let's do first 3, after that we can improve the auto generated usage.

[client9]

@fatih no problem on delay. Vacation is good. I'm bouncing between a new projects on github, but I hope to get a pull request to you today for review. thanks! nickg

update: 8 days later... ok take 2 on doing this.

[client9]

Hello!

its super weird how flexible this package is (thank you!!), but yet, we still can't customize help or add in our own Usage()/Help() functions (i.e. a Null loader that is only there to print a opening or closing statement).

I'll be working on the

type Loader interface {
    Load(s interface{}) error
    Help() string
}

solution.

let me know if thats a problem! Or if you are working on an alternate solution.

regards,

n

[client9]

Hello!
Adding the Help() interface turned out to be not so hard., but there are some backwards compatibility issues that may or may not concern you. I tried to keep backwards compatibility and not introduce new changes or help formats.

Let's do the easy ones first:

+++ b/multiconfig.go
@@ -16,6 +16,7 @@ import (
 type Loader interface {
        // Load loads the source into the config defined by struct s
        Load(s interface{}) error
+       Help() string
 }

+// Help will return the concatentated help messages of the loaders
+func (m multiLoader) Help() string {
+       out := ""
+       for _, loader := range m {
+               out += loader.Help()
+       }
+       return out
+}

+// Help returns an empty string
+func (t *TagLoader) Help() string {
+       return ""
+}
+
+// Help returns an empty string
+func (t *TOMLLoader) Help() string {
+       return ""
+}
+
+// Help returns an empty string
+func (j *JSONLoader) Help() string {
+       return ""
+}
+

Ok thats simple. FlagLoader isn't much harder

+func (f *FlagLoader) Help() string {
+       out := new(bytes.Buffer)
+       f.flagSet.SetOutput(out)
+       f.flagSet.PrintDefaults()
+       return out.String()
+}
+

env.go has a slightly more complicated Help() but is more or less 100% backwards compatible since it already had a PrintEnv() function that printed to stderr. It mostly re-arranging existing code.

So adding the Help() interface isn't hard... and ALL those changes are backwards compatible or benign.

the tricky part is using the Help() interface. The current flag.go does an exit on error and uses stdlib flag behavior. To make things work I did this:

 // Load loads the source into the config defined by struct s
 func (f *FlagLoader) Load(s interface{}) error {
        strct := structs.New(s)
        structName := strct.Name()

-       flagSet := flag.NewFlagSet(structName, flag.ExitOnError)
+       flagSet := flag.NewFlagSet(structName, flag.ContinueOnError)
        f.flagSet = flagSet

        for _, field := range strct.Fields() {
                f.processField(field.Name(), field)
        }

-       flagSet.Usage = func() {
-               fmt.Fprintf(os.Stderr, "Usage of %s:\n", os.Args[0])
-               flagSet.PrintDefaults()
-               fmt.Fprintf(os.Stderr, "\nGenerated environment variables:\n")
-               e := &EnvironmentLoader{
-                       Prefix:    f.EnvPrefix,
-                       CamelCase: f.CamelCase,
-               }
-               e.PrintEnvs(s)
-               fmt.Println("")
-       }
+       // need to prevent default printing of flag usage
+       flagSet.Usage = func() {}

and to use it, I changed my app from

config := &Config{}
        l := multiconfig.MultiLoader(configLoaders...)
        err := l.Load(config)  // <------ if -help was done, this called os.Exit(2) :-(
        if err != nil {
               return nil, err
       }

to

        config := &Config{}
        l := multiconfig.MultiLoader(configLoaders...)
        err := l.Load(config)
        if err != nil {
                if err != flag.ErrHelp {  // <----- golang stdlib flag specific
                        return nil, err
                }
                // if we are here, we asked for help.
                // and I can add a header, footer whatever.
                fmt.Println(cliLoader.Help())
                fmt.Println(envLoader.Help())

                os.Exit(2)
        }

So to keep backwards compatibility I recommend, we add to FlagLoader a new field for ErrorHandling

https://golang.org/pkg/flag/#ErrorHandling

with the default of "ExitOnError // Call os.Exit(2)."

This keeps the current behavior,

If it is set to "ContinueOnError ErrorHandling = iota // Return a descriptive error." Then it does the behavior listed in the diff.

That should make everyone happy ?

thoughts welcome!

n

[client9]

Ok now that #43 was merged in I can make PR for better review.

coming soon.

n

[client9]

Hmmm, I failed in my git commit message.

see pull request!

#47