required parameters don't work with altsrc

[costela]

Hi,

Unless I'm somehow overlooking something obvious, it seems the new Required support introduced in #819 doesn't work with altsrc.
The following code (based on the example for altsrc) complains that someflag wasn't supported if Required: true, but works if you comment that line out.

package main

import (
	"fmt"
	"log"
	"os"

	"github.com/urfave/cli"
	"github.com/urfave/cli/altsrc"
)

func main() {
	app := cli.NewApp()

	flags := []cli.Flag{
		altsrc.NewIntFlag(cli.IntFlag{
			Name:     "someflag",
			Required: true,
		}),
		cli.StringFlag{
			Name:  "file",
			Value: "test_cli_yaml.yaml",
		},
	}

	app.Action = func(c *cli.Context) error {
		fmt.Printf("test: %s\n", c.String("someflag"))
		return nil
	}

	app.Before = altsrc.InitInputSourceWithContext(flags, altsrc.NewYamlSourceFromFlagFunc("file"))
	app.Flags = flags

	err := app.Run(os.Args)
	if err != nil {
		log.Fatal(err)
	}
}

And the accompanying YAML file (test_cli_yaml.yaml):

---

someflag: 1

@lynncyrin, I suspect your question in #825 might be related to this, right?

[coilysiren]

Yeah I discovered after my required flags PR that "altsrc" is a completely different code path, so you'd essentially have to implement every feature twice. Once in main, and once in altsrc 😩

I'm, having some negatives feelings about the architecture of altsrc :/ I'm gonna investigate moving some code from altsrc to the main package, which would fix this.

[coilysiren]

I'm making this as a bug since externally it's a bug. The PR to fix this is probably gonna look like a new feature implementation PR though

[costela]

I'm gonna investigate moving some code from altsrc to the main package, which would fix this.

that sounds great! I was wondering myself why using altsrc feels so clunky 😉

[coilysiren]

After thinking on this a bunch more, I've come to the conclusion that I'm definitely going to fix this by adding all of the altsrc features into the main package, and removing altsrc entirely. My current idea is that that'll happen in v3, I'm tracking that here #833.

So I'm gonna close this, since I'm closing all of the "altsrc doesn't work like the main package" issues. Apologies if this is rude, but I'm trying to burn down github issues. (I want to make sure I'm reading all of them! which means I've gotta close a bunch)

[costela]

waiting for v3 sucks, but getting a proper solution in the end sucks a bit less 😉