Cli: Implement an interface for all errors, so they can be redefined by callers

[lynncyrin]

Currently we return some error messages to end users, like

• "flag provided but not defined"
• "required flag not set"

But do not provide a way for people to re-define those error messages. We should implement some public interfaces for our errors, and provide documentation on how to implement custom error messages.

Related issues / PRs

• https://github.com/urfave/cli/issues/852
• https://github.com/urfave/cli/pull/656

[anberns]

I'd really like to work on this, but I'll probably need some guidance.

[lynncyrin]

That's fine! We can collaborate a bit, I need to do some learning here too 👩‍🎓

[lynncyrin]

Here's the best post I could find about error interfaces https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully

Here's a PR where I added an interface that essentially what I describe in the title https://github.com/urfave/cli/pull/819/, requiredFlagsErr.

The idea is that you would set Flag.RequiredFlagsErr (capital R now ^^) on a flag, and that interface would get called if there was an error with that flag. Feel free to ignore any complexities with your initial implementation, like the error message when there are errors with multiple flags.

[anberns]

Awesome! Thanks for the info. Starting on it now.

[anberns]

Quick question when you get a chance. Now that the flag_generated.go file is generated with flag-gen instead of the python script, what's the process for incorporating the necessary struct field and function for RequiredFlagsErr? Thanks.

[lynncyrin]

I reviewed that code but I don't specifically know what to point you to. You could ask @asahasrabuddhe, but it may be easier for both if ya'll if you dive into the code and explore it a bit. Or looking at the PR https://github.com/urfave/cli/pull/836

[anberns]

Yeah I spent several hours getting to know the codebase by investigating
this question. Don't worry, I don't reach out for things like this unless I
absolutely can't figure it out. I'll take another look at the PR you
mentioned and contact @asahasrabuddhe if necessary. Thanks!

On Mon, Aug 19, 2019, 5:06 PM lynncyrin [e-mail] wrote:

I reviewed that code but I don't specifically know what to point you to.
You could ask @asahasrabuddhe https://github.com/asahasrabuddhe, but it
may be easier for both if ya'll if you dive into the code and explore it a
bit. Or looking at the PR #836 https://github.com/urfave/cli/pull/836

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/urfave/cli/issues/853?email_source=notifications&email_token=AGEPUNEQSTYQZWC47UCDLPTQFMDODA5CNFSM4IK2QG52YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4UJOSA#issuecomment-522753864,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGEPUNGBD4WSVDA4ZSQHMV3QFMDODANCNFSM4IK2QG5Q
.

[lynncyrin]

Also for context, I just made this issue => https://github.com/urfave/cli/issues/870

[anberns]

Ok cool. I'll just bypass the generation for now.

[lynncyrin]

Dropping this into the status: in review bucket, @urfave/cli please add a 👍 or 👎 to the top post if you're in favor or against this feature being added!

_(I'm counting the existing PR by @anberns as an exception here)_

[lynncyrin]

Closing this pending more buy in from maintainers