@creekorful hiya, please get buy-in from a maintainer before you make a PR that changes our API surface! it's important that it's well curated.

@creekorful hiya, please get buy-in from a maintainer before you make a PR that changes our API surface! it's important that it's well curated.

Hello! What do you mean by buy-in exactly ?

Hello! What do you mean by buy-in exactly ?

You need the agreement of someone who maintains a package before you make a externally visible change to that package

To repeat my comment in #1082, I don't think a HideHelpOnBeforeError config is the right solution here. I would be interested in discussing any other potential solutions to this problem 👍

Hello back!

A proper solution, for me, would be to let the user decide of the behavior in case of error, instead of printing the error & the help usage.

In the RunAsSubcommand method, the BeforeFunc is processed like this:

if a.Before != nil {
	beforeErr := a.Before(context)
	if beforeErr != nil {
		a.handleExitCoder(context, beforeErr)
		err = beforeErr
		return err
	}
}

which is more cleaner way, since we let the user decide what he wants to do with the error.
(by implementing the ExitErrHandler function)

I think we should unify the way BeforeFunc are handled in RunContext & RunAsSubcommand

What do you think?

@creekorful that makes a whole lot of sense to me. Two features of this strike me as really important:

• The behavior of RunContext and RunAsSubcommand should be as similar as possible.
• Calling ShowAppHelp effectively assumes that any error encountered in Before is a usage error, and that does not strike me as a safe assumption to make.

• Calling ShowAppHelp effectively assumes that any error encountered in Before is a usage error, and that does not strike me as a safe assumption to make.

That's a really good point.

What do you think about about making a PR to unify the behavior as in RunAsSubcommand ?

Curious what @lynncyrin thinks, but I'd be 👍 for that solution.

I'd say feel free to open a PR whenever. For me, the big reason to get buy-in before making a change to the API surface is that spending the effort to curate and submit a code change, only to find out that it wasn't something that the maintainers were interested in the first place, is a not-fun situation on both sides. If the change is only as big as deleting a couple lines of code, ¯\_(ツ)_/¯

Hello there, PR is done.

Thank you

Nice work! ✨