Run Before actions after setting up subcommand

[fjl]

I'm posting this to explore a possible resolution for #2027.

Let me explain the change to TestFlagAction/mixture. The test shows a behavior difference introduced by this PR: since flag actions will now run after all flags have been set, the test does not see the 'shadowed' flag --f_string=app. I personally think the new behavior is more correct: the flag is shadowed after all.

I can't really think of a scenario in a real app where you would want to set the same flag two times with different values for the root command and the subcommand. So I propose to change the test, locking in the new behavior.

I have also added a test for issue #2027 here.

What type of PR is this?

• bug
• cleanup

What this PR does / why we need it:

This fixes an issue where the Before hook of a parent command would not see flag values set in the subcommand context.

Which issue(s) this PR fixes:

Fixes #2027

[fjl]

Actually, after thinking a bit longer, the behavior change to flag actions may not be acceptable. The existing behavior of flag actions was basically to invoke the action for every instance of the flag, with the value of that instance. invoke the flag at every command level. It can make sense for some applications. The problem is the context parameter.

If flag actions didn't take the context.Context parameter, they could be run before Before actions. This would make sense for applications where the flag action is used to set a global or something.

However, if the context is desired in flag actions, the only option would be collecting flag instances in another slice in Command so their local values can be accessed later when running the actions.

[fjl]

Having spent quite a bit of time trying to restore the behavior of flag actions while keeping the new Before logic, I can now say it's impossible. But working on that has convinced me flag actions are handled kind of incorrectly right now.

At the moment, flag actions for persistent flags are invoked multiple times, basically once per 'command-level', with the value at that level. That doesn't make a lot of sense, given that the ultimate goal of flag parsing, setup etc. is to execute the innermost command action, and the flag values at that level are the ones that should be applied to the program.

So I propose that flag actions of persistent flags should be run exactly once, with the value that will actually be set for the command action. The current behavior does make sense for local flags, so this can be kept.

[fjl]

@dearchap would be nice to get your input on this PR (even though it's not fully done yet). The basic change is just to run Before functions later, when the innermost values of flags have been resolved.

[dearchap]

@fjl i'm AFK and on travel. Will get to it in a day or two. Thank you so much for your patience. I have been following all the comments in this PR closely

[dearchap]

@fjl sorry for the late approval. The changes look good.