Since one possibly hijacks their "Before" by using altsrc we should have another statement run after the context is loaded but after Before

[johnwyles]

As you can see in a few examples floating around the web the Before method is sort of hijacked by the use of altsrc in loading YAML or TOML:

• V2: Making altsrc flag optional in Before: #793
• altsrc newYamlSourceFromFlagFunc example not working. #570

This leaves other initiation you may want to perform that perhaps is some preparation for your entire command and subsequent subcommands (for example setting up generic logging or setting some values all subcommands will need). If you are not loading configuration using altsrc then Before is just fine for your use case. However in the event you do want to load configuration from YAML or TOML you are left with your Before held hostage with no option to perform any other Before actions. Thus I am proposing a post-Flags and post-Before directived I've dubbed Prepare.

The following code from how you would do this currently is pretty standard in your searches around the documentation and the web for examples of loading configuration. Note that the Before method is "hostage" (e.g. it can no longer do anything else) to this process of loading configuration leaving you little option to perform any other Before actions you may want to take:

With the YAML file file.yml:

test: 10

In previous test.go:

package main

import (
        "fmt"
        "os"

        "gopkg.in/urfave/cli.v1"
        "gopkg.in/urfave/cli.v1/altsrc"
)

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

        flags := []cli.Flag{
                altsrc.NewIntFlag(cli.IntFlag{Name: "test"}),
                cli.StringFlag{Name: "load"},
        }

        app.Action = func(c *cli.Context) error {
                fmt.Println("action")
                fmt.Println(c.Int("test"))
                return nil
        }

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

        app.Flags = flags

        app.Run(os.Args)
}

Output:

$ go run test.go --load file.yaml
action
10

Instead I have proposed the use of a Prepare directive that will run after Before so you can still load configuration and then do something with everything loaded. Take the above example modified with this new Prepare directive:

The file test.go:

package main

import (
        "fmt"
        "os"

        "gopkg.in/urfave/cli.v1"
        "gopkg.in/urfave/cli.v1/altsrc"
)

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

        flags := []cli.Flag{
                altsrc.NewIntFlag(cli.IntFlag{Name: "test"}),
                cli.StringFlag{Name: "load"},
        }

        app.Action = func(c *cli.Context) error {
                fmt.Println("action")
                fmt.Println(c.Int("test"))
                return nil
        }

        app.Prepare = func(c *cli.Context) error {
                fmt.Println("prepare")
                fmt.Println(c.Int("test"))
                return nil
        }

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

        app.Flags = flags

        app.Run(os.Args)
}

Output:

$ go run test.go --load file.yaml
prepare
10
action
10

[johnwyles]

@jszwedko @meatballhat @michaeljs1990 would you guys like to take a look over things and have a discussion and review the PR's with me - I am currently having to use this to get Before back since it is hijacked by altsrc.InitInputSourceWithContext(...) - next up for me is making a config file entirely optional if you don't specify one, as it is without the code mentioned in #793 Before you would be forced to supply one. Separate issue entirely but highlights the hoops I am having to jump through with altsrc - I love the feature but I think with these two improvements or some discussion we all have to arrive at a better approach would be an ideal great addition.

[michaeljs1990]

Sounds good to me, this weekend I'll take some time out to sit down and grok the above.

[johnwyles]

@michaeljs1990 I should mention that there are two separate PR's for master (v1) #802 and v2#803 - I am not sure if master => v2 is actively how things are merged up or not so let me know and I can put together the PR from urfave/cli:master => urfave/cli:v2 and we can just drop the PR for urfave/cli:v2 #803

[johnwyles]

BUMP Any interest in this? It has been helpful in my personal project to use this as such where the overrides are in decreasing priority:

1. ENVIRONMENT VARIABLE VALUES
2. CONFIG VALUES (if config file is found and defined with Before)
3. FLAG VALUES
4. INTERNAL DEFAULT VALUES

Also once the Before is used in a very quirky way (see: #793) to make the config file optional or not then one can use the Prepare block I have created that will perform Before-like tasks after the config file is loaded but before Run.

[coilysiren]

Hiya, I'm having a bit of a hard time parsing this github issue. I have 2 thoughts:

Can you rewrite the issue to follow some slightly more standard change request patterns? Here's some examples (one I googled, and two by me)

• https://github.com/rstudio/rstudio/wiki/Writing-Good-Feature-Requests
• [Feature Request] Support resolving python versions to some major or minor version pyenv/pyenv#1349
• [Feature Request] Use codegen to create client libraries CircleCI-Archived/api-preview-docs#43

I would also note that none of the current maintainers (to my knowledge) are a fan of altsrc. It's currently my intention to remove altsrc entirely, and more it into the main package (I mention this here #833). If I'm reading this issue correctly and your issue is only with altsrc, then I think it should be closed. More issues describing quirks in altsrc I want to resolve via the previously mentioned removal of it.

[coilysiren]

🕐 issue timeout 🕐

I'm timing out this issue since it hasn't been updated in quite some time. Feel free to comment in support, and I'll re-open it!