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

feat: Improve invalid keybind error messages #4444

Closed
wants to merge 1 commit into from

Conversation

otherJL0
Copy link

@otherJL0 otherJL0 commented Jan 2, 2025

Implementation of Better keybind error messages discussion

@otherJL0
Copy link
Author

otherJL0 commented Jan 2, 2025

This PR is a clean implementation of #3727 after mucking up the git history

Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR. I'm not happy with this approach. I recognize it works, and I recognize the effort, so it does pain me to say this but I'm going to reject this PR. My notes below...

First, I think the errors in general are too generic to add to cli/args.zig. I would recommend re-wrapping them with something like BindingInvalidFormat, BindingInvalidAction, etc.

Still, even with that, it doesn't feel like cli/args should know about these errors. Just spitballing in my head (not sure if its a good approach) but one thing I would try is having a function on T (in parse) that can take an error type E and format it. For example:

pub fn formatError(alloc: Allocator, err: FormatableError) { ... }

And then using hasDecl and some basic Zig reflection to determine if the error matches this error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants