Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement an interface for all errors, so they can be redefined by callers #853

Closed
coilysiren opened this issue Aug 10, 2019 · 11 comments
Closed
Labels
kind/feature describes a code enhancement / feature request status/in-review needs to be reviewed by maintainers before it is accepted

Comments

@coilysiren
Copy link
Member

coilysiren commented Aug 10, 2019

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

@coilysiren coilysiren added kind/feature describes a code enhancement / feature request status/confirmed confirmed to be valid, but work has yet to start help wanted please help if you can! labels Aug 10, 2019
@anberns
Copy link
Contributor

anberns commented Aug 16, 2019

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

@coilysiren
Copy link
Member Author

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

@coilysiren coilysiren added status/claimed someone has claimed this issue and removed help wanted please help if you can! status/confirmed confirmed to be valid, but work has yet to start labels Aug 17, 2019
@coilysiren
Copy link
Member Author

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 #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
Copy link
Contributor

anberns commented Aug 19, 2019

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

@anberns
Copy link
Contributor

anberns commented Aug 19, 2019

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.

@coilysiren
Copy link
Member Author

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 #836

@anberns
Copy link
Contributor

anberns commented Aug 19, 2019 via email

@coilysiren
Copy link
Member Author

Also for context, I just made this issue => #870

@anberns
Copy link
Contributor

anberns commented Aug 20, 2019

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

@coilysiren coilysiren added status/in-review needs to be reviewed by maintainers before it is accepted and removed status/claimed someone has claimed this issue labels Sep 10, 2019
@coilysiren
Copy link
Member Author

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)

@coilysiren
Copy link
Member Author

Closing this pending more buy in from maintainers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature describes a code enhancement / feature request status/in-review needs to be reviewed by maintainers before it is accepted
Projects
None yet
Development

No branches or pull requests

2 participants