urfave / cli

A simple, fast, and fun package for building command line apps in Go
https://cli.urfave.org
MIT License
22.36k stars 1.71k forks source link

Ensure primary library usage is via struct literals #1774

Open meatballhat opened 1 year ago

meatballhat commented 1 year ago

One of the strongest differentiators of this library is (arguably! (I'm arguing)) how declarative it is. Ensuring the primary usage of the library is via struct literals will support this differentiation, imho.

In practice, this can certainly become awkward to the point of being a bad user experience. For example, consider the "value source" implementation Sources: EnvVars(...) that replaces the v2 EnvVars: []string{...}. The EnvVars function is not a literal declaration of a type, but the usage is expected to be within a literal declaration of a Flag type.

Judgement calls must be made!

bartekpacia commented 6 months ago

Just to make sure I understand it correctly:

If I understood this correctly, then I agree with it.

meatballhat commented 6 months ago

@bartekpacia The example you're giving is actually one that I'm not sure about 😅

In the v1 API, there were a few "stringly typed" fields that would be processed with strings.Split("a,b", ",") and strings.TrimSpace, which resulted in some annoying and bug-inducing patterns like having to process the strings in many places.

In v2 we made sure to require []string{"a", "b"} instead of "a,b" and added fields like Aliases to allow Name to remain a string. We also saw a lot of activity in ./altsrc that contributed to more awkward dependency management issues. I mention this because it's related to the existence of the v3 Sources field.

In v3, I am hoping we can accommodate environment variables, files of various formats, and whatever else we haven't thought of, all via the Sources field on each flag type. The inclusion of environment variables in this goal means that accepting something more v2-ish like EnvVars: []string{...} introduces other tradeoffs around precedence, merging, egad, and I'm currently favoring trying to use only the Sources field.

So! I don't want to be too strict about any of these rules, whatever that may mean at the time. I hope that we can lean more heavily into the "declarative" differentiator of this library. I don't want to reintroduce a separate EnvVars field if we can help it, so I think this means that I'm favoring a smaller API surface area over being declarative everywhere.

WDYT?

dearchap commented 2 months ago

Is this still an issue ? We've fixed ValueSourceChain so that sources can be defined as

    Sources: cli.EnvVars("a", "b", c"),

or

    Sources: cli.ValueSourceChain{
       Chain: []cli.ValueSource {
            cli.EnvVar("a"),
            cli.EnvVar("b"),
            cli.EnvVar("c"),