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

report_error refactoring #219

Merged
merged 4 commits into from
Sep 5, 2015
Merged

Conversation

Vinatorul
Copy link
Contributor

This refactor will help me a lot to change get_matches_with to return Result.
Also it is good for same code deletions.
Only I'm not sure with module and enum name, so waiting for your suggestions before merge ;)

@kbknapp
Copy link
Member

kbknapp commented Sep 3, 2015

Looks like a good start! I'd re-name the error enum to something like ClapError (or similar??) though because ArgNames confused me at first. And there are also errors that may not be related to args, or down the road there could be. Then we can also implement std::error::Error

@Vinatorul
Copy link
Contributor Author

there is a big problem with Error trait. I tried to do it, but for now reports using huge bunch of App code, it is unattainable from Display trait, which is must-have if you implement Error
The main problem is that create_usage can't be used without App instance.
I have some ideas how to solve this in non rust ideology, but it is non goal for us, I think.

Hm, I think I have good idea how to solve it. Just generate error message with usage in get_matches_with and handle it in get_matches_from or get_matches_from_safe

@Vinatorul
Copy link
Contributor Author

Resolved

@kbknapp
Copy link
Member

kbknapp commented Sep 5, 2015

Very nice!

kbknapp added a commit that referenced this pull request Sep 5, 2015
@kbknapp kbknapp merged commit 11451b9 into clap-rs:master Sep 5, 2015
@Vinatorul Vinatorul deleted the report_error_refact branch September 5, 2015 17:40
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