Collapse flag interface

[dearchap]

What type of PR is this?

(REQUIRED)

• cleanup

What this PR does / why we need it:

(REQUIRED)

Collapse all the flag interfaces into a single consistent interface

Which issue(s) this PR fixes:

(REQUIRED)

Special notes for your reviewer:

(fill-in or delete this section)

Testing

(fill-in or delete this section)

go test ./...

Release Notes

(REQUIRED)

[meatballhat]

@dearchap thank you for doing this! As I'm thinking through the details here, a few questions most important to me are:

• Does this change make it easier for users of this library to implement custom flag types?

In this form, I don't think so, but I think it could, e.g. if we were to provide an embeddable struct with base method implementations, or allow for embedding the Flag interface and then internally guarding against nil pointer dereferencing from automatic method implementations.

• Does this change make it any easier to maintain this library?

I don't think so 😭 In general, I'm OK with this. I would rather take care of complexity and common problems within the library than require users of the library to build roughly the same code themselves.

Having the separate interfaces available is also useful within this library when using interface checks to branch into optional behaviors, e.g.:

    // where f is an interface type, likely cli.Flag
    if rf, ok := f.(RequiredFlag); ok {
        if rf.IsRequired() {
            // ... more things
        }
    }

[meatballhat]

• Does this change make it easier for users of this library to implement custom flag types?

In this form, I don't think so, but I think it could, e.g. if we were to provide an embeddable struct with base method implementations, or allow for embedding the Flag interface and then internally guarding against nil pointer dereferencing from automatic method implementations.

After thinking about this a bit more, I feel like we already provide this capability by publishing the various base flag types (?) e.g.:

type FruitFlag struct {
    cli.StringFlag

    IsBanana bool
}

func (ff *FruitFlag) IsNecessarilyBanana() bool {
    return ff.IsRequired() && ff.IsBanana && ff.Value == "banana"
}

// ...

var bf cli.Flag = &FruitFlag{
    StringFlag: cli.StringFlag{
        Name: "fruit",
        Value: "apple",
    },
}

[dearchap]

@meatballhat I agree with your points. This is just a starting point to integrate a FlagImpl[T] using generics so that we can get rid of all the generated implementation and have a compact code base. The same FlagImpl[T] can be used by users as well. The reason the Flag interface was split in v1/v2 was because once the initial Flag interface was defined , extra features were added like RequiredInterface, VisibleInterface etc etc and then all the XXFlag structs were retrofited to be compatible with these interfaces. With v3 we have a chance to make a single interface to span all our current use cases. Any flag implementation will necessarily need all of these methods anyway. Why would a flag impl NOT have a GetUsage() implementation. That being said perhaps I jumped the gun here with collapsing flag interfaces. I probably should have started off with the StructImpl[T] to remove generated code. This is in keeping with the pointed described in #833 . If you think it is not needed I am fine with it but then whats the whole point of a v3.x if not to make everything more streamlined.

[meatballhat]

@dearchap thank you for the details! I'm happy to support you in this direction, and I'm very much looking forward to what you have in mind 🎉 🤘