-
Notifications
You must be signed in to change notification settings - Fork 7
migrate remaining errors to new model #108
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
Conversation
|
c621b26
to
d40db78
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
🚀 New features to boost your workflow:
|
Damnit, I really regret pushing We can cut a |
Great changes and good catch! I kinda regret pushing the last release in haste - ah well. I don't think we will be touching errors after this release so hopefully folks will forgive a bit of churn. |
Yeah, in a way it's good we took a while so the breaks aren't too close to each other. Although, yes, ideally we'd have bundled this one, but oh well. |
The
EncodingError
andParseIndexError
types are parsing errors with awell defined 'subject', but they weren't previously made into
Diagnostic
implementors, so they worked a bit inconsistently with other error types. This
PR makes it so those errors follow the same conventions and general structure
used by its kindred.
There is one related breaking change:
InvalidCharacterError
previously helda copy of the subject, but in the new model this is a responsibility of its
enriched
Report<InvalidCharacterError>
type. Since some methods directlydepended on access to the subject, they had to be removed. I think this is
justifiable since the new error APIs are already breaking (we just should've
made this change in tandem with the other related breaks).
I don't have any other error-related breaks planned.
This PR also incidentally fixes an incorrect error type returned if the token ended with a
~
(it would be flagged as a slash error, not tilde). I updated tests to cover this.