Option new methods

[pierrec]

Hopefully, this is helpful for everyone...

• added Option.Field() method to recover the struct field associated with the option
• added Option.IsSetDefault() to help distinguish cases where the option was really set or set with default values

[jessevdk]

Can you explain what the use cases for these are.

[pierrec]

Sure.

For some programs that have a cli interface for which options can be saved into a file, you may want to have the order of preference for the options to be:

1. set from cli
2. set from file
3. default value

For instance:
myprog has the following options:

var config struct {
  Path string `long:"path" description:"path to the config file"`
  LogLevel string `long:"path" description:"logging level" default:"info"`
}

1. First case: options are specified on the command line
   ./myprog --config=my.ini --loglevel debug
   => the config is:

struct {
  Path: "my.ini"
  LogLevel "debug"
}

1. Second case: options are specified in a file
   if the my.ini file contains:

[Application Options]
LogLevel = warning

./myprog --config=my.ini
=> the config is:

struct {
  Path: "my.ini"
  LogLevel "warning"
}

1. Third case: options are not specified anywhere
   ./myprog --config=my.ini
   => the config is:

struct {
  Path: "my.ini"
  LogLevel "info"
}

So basically, I first use a Parser to get the options from cli, then a second Parser to get the ones from the config file, then I fuse the results by comparing if the Option is set in the file with Option.IsSet() or set on the cli with Option.IsSetDefault(), and if not, I do a Option.Field().Set(iniOption.Field()).
Hence my requirement for those 2 new methods.

[jessevdk]

Does this solve it: #120 ?

[pierrec]

Ah yes it does, although it is not straightforward.

Here is a working example based on the one in #120:

package main

import (
    "fmt"
    "log"

    "github.com/jessevdk/go-flags"
)

type opts struct {
    Config string `long:"config" no-ini:"true"`
    Host   string `long:"host" default:"localhost"`
}

func main() {
    options := &opts{}

    // parse cli options
    p := flags.NewParser(options, flags.Default)
    if _, err := p.Parse(); err != nil {
        log.Fatal(err)
    }

    if options.Config == "" {
        // no config file: nothing else to do
        fmt.Printf("%+v\n", options)
        return
    }

    // config file specified:
    // - load it in a separate options struct
    // - any set file option overwrites the default cli one
    inip := flags.NewParser(&opts{}, flags.Default)
    if err := flags.NewIniParser(inip).ParseFile(options.Config); err != nil {
        log.Fatal(err)
    }

    // compare and set all options
    var walk func([]*flags.Group)
    walk = func(groups []*flags.Group) {
        for _, g := range groups {
            walk(g.Groups())
            for _, o := range g.Options() {
                if !o.IsSet() {
                    // option not set in the config file, ignore
                    continue
                }
                // cli corresponding option
                oo := p.Command.Group.FindOptionByLongName(o.LongNameWithNamespace())
                if !oo.IsSetDefault() {
                    // cli option set, it prevails
                    continue
                }
                // file option overwrites cli default value
                oo.Field().Set(o.Field())
            }
        }
    }
    walk(inip.Command.Groups())

    fmt.Printf("%+v\n", options)
}

The output:

$ ./tmp
&{Config: Host:localhost}
$ ./tmp --host google.com
&{Config: Host:google.com}
$ cat a.ini 
Host = github.com
$ ./tmp --config a.ini 
&{Config:a.ini Host:github.com}
$ ./tmp --config a.ini --host google.com
&{Config:a.ini Host:google.com}
$ ./tmp --host google.com --config a.ini 
&{Config:a.ini Host:google.com}

[pierrec]

Actually, I think it may be possible to have a better approach for this without having to expose any new method, but only having a new tag such as is-ini to identify an ini path. You could potentially have more than one.

type opts struct {
    Config string `long:"config" no-ini:"true" is-ini:"true"`
    Host   string `long:"host" default:"localhost"`
}

The options would be set in order of priority: cli, ini file, default value.

The steps would be:

1. load options with ParseArgs(), flagging them internally with isSetDefault if the default option was defined and applied
2. parse any ini file found during step 1 using a cloned structure, aborting on any error
3. for every option, if it is not set or set with its default value, check every ini file for that option. If it is defined, then use that value and continue to the next option, that is, the first option set in the ini file would be used.

Let me know what you think. I can provide another pull request with this implementation and associated tests, although it may take a couple of days as I am busy with other things.

[jessevdk]

Why didn't you try the approach as shown in #120 (comment), it's pretty simple and straightforward?

[pierrec]

Because I want the cli options to always override my ini ones, no matter the order in which they appear.

[jessevdk]

I would rather have an option on the ini parser that determines whether it will override already set options or not.

[pierrec]

Ok, something like this? This is straightforward to implement and simplifies my code too ;)

type opts struct {
    Config string `long:"config" no-ini:"true"`
    Host   string `long:"host" default:"localhost" ini-override:"true"`
}

[pierrec]

Any thought on the last commit?

[jessevdk]

Sorry for taking so long, but the idea I think was not to have an additional tag, but an option on the actual IniParser.

[pierrec]

Such as:

type IniParser struct {
  KeepCLI bool
}

?

[jessevdk]

Yes, I think parseAsDefaults might be a better name.

[pierrec]

Ok, finally found some time to do this: the option ParseAsDefaults has been added to IniParser that override default values when set.

[jessevdk]

I don't really like this, can you put isSetDefault on a separate line?

[jessevdk]

Does this need to be public?

[jessevdk]

Does this need to be public?