adds urfave/cli/v2 flag parser #330
Conversation
joicemjoseph
changed the title
There was a problem hiding this comment.
Firstly, Great work!
I was going to create an issue to support urfave/cli I'm very glad I didn't and looked at PR's first.
I am currently using v3, I left a few comments here, let me know your opinion.
If you want to stay with v2, I will gladly make a PR for v3, but I suggest you also version this one so they can both live on the same repo under the same name :)
| "strings" | ||
|
|
||
| "github.com/knadh/koanf/maps" | ||
| "github.com/urfave/cli/v2" |
There was a problem hiding this comment.
v3 is on the verge of being released:
It may well be worth it to either version this provider behind /v2 or jump to /v3 right away.
There was a problem hiding this comment.
As v1, v2 are actively maintained (and widely used), and v3 is about to release, it will be really nice to have support for all the versions urfave/cli versions natively on koanf. But, this PR adds support only for urfave/cli/v2.
I am still confused how different versions of the same provider (here, cliflag) can be structured and maintained. Then I saw your feature request #333. I have added my comments/thoughts in that issue. Please help me how to structure different versions of urfave/cli inside koanf.
|
|
||
| // CliFlag implements a cli.Flag command line provider. | ||
| type CliFlag struct { | ||
| ctx *cli.Context |
There was a problem hiding this comment.
v3 will help to remove this custom context and rely on context.Context instead.
https://github.com/urfave/cli/tree/main/docs/v3
It also makes a much saner API for all the methods below.
Passing the cli.Command could also result in a less opinionated way of parsing flags in koanf.
| cmdPath = append(cmdPath, cmd.Command.Name) | ||
| prefix := strings.Join(cmdPath, p.delim) | ||
| p.processFlags(cmd.Command.Flags, prefix, out) | ||
| } |
There was a problem hiding this comment.
I think the user should provide flag to key mappings.
This leads to an opinionated way to create the returned map keys.
I understand the intention to travel all the command and parse all flags but maybe that is not what any user would want.
I have looked into urfave/cli to see if there is a way to indicate which flags will be used as config and which aren't, but there isn't a way for this. So maybe there is a way to let user of Koanf the ability to say which flags are or aren't to be handled.
There was a problem hiding this comment.
As per my understanding, you are suggesting to have a filter while reading the flags.
I was referring how other flags are parsed. spf13/pflag is supported by koanf as a flag provider.
Here, with posflag, there are ways to modify given keys. Also, none of other koanf flag providers have an option to filter flags. My personal preference is also not to filter while reading the flags.
filtering can be achieved using
ko := koanf.New(.)
_ = ko.Load(cliflag.Provider(ctx, "."), nil)
_ = ko.Unmarshal("app", &cfg)
// or
someStr := ko.MustString("app.some_string")Please share your thoughts
|
@joicemjoseph how's this PR coming along? |
|
i will make changes as discussed above and update it by today. |
No description provided.