ValidateRequiredFlags() and ValidateFlagGroups() should be executed before PersistentRunE() and PreRunE()

[FLAGLORD]

I'm a little confused why ValidateRequiredFlags() and ValidateFlagGroups() are executed after PersistentRunE() and PreRunE() in the current implementation. IMO, PersistentRunE() and PreRunE() can be set and controlled by developers, and all pre-check provided by cobra framework should throw error as soon as possible.

cobra/command.go

Lines 913 to 937 in fd865a4

	for p := c; p != nil; p = p.Parent() {
	if p.PersistentPreRunE != nil {
	if err := p.PersistentPreRunE(c, argWoFlags); err != nil {
	return err
	}
	break
	} else if p.PersistentPreRun != nil {
	p.PersistentPreRun(c, argWoFlags)
	break
	}
	}
	if c.PreRunE != nil {
	if err := c.PreRunE(c, argWoFlags); err != nil {
	return err
	}
	} else if c.PreRun != nil {
	c.PreRun(c, argWoFlags)
	}
	if err := c.ValidateRequiredFlags(); err != nil {
	return err
	}
	if err := c.ValidateFlagGroups(); err != nil {
	return err
	}

[marckhouzam]

Duplicate of #1787 #1521 #700 #655

[FLAGLORD]

Got it. Backward compatibility is really a key point.
Considering that if any error threw by RunE(), the program will print the usage. The easiest way for me to control the print of error or usage is to set SilenceErrors and SilenceUsage to true in PersistentRunE(#340 (comment)). PersistentRunE can be inherited by all subcommands, and cobra can help me validate argument and flags before their executions.
The problem is that ValidateRequiredFlags() and ValidateFlagGroups() executed after would not print any hints as I set SilenceErrors and SilenceUsage. I haven't found a good way. Maybe I should do some checks myself.

[michaelajr]

FWIW It makes sense to me that flag validation would happen a least after PersistentPreRun. That is where we can manipulate the flags if needed prior to validation. I use the PersistentPreRun hook to work around spf13/viper#397.