-
Notifications
You must be signed in to change notification settings - Fork 2
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
Better parsing exceptions #299
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #299 +/- ##
==========================================
- Coverage 68.21% 68.08% -0.13%
==========================================
Files 26 26
Lines 3492 3506 +14
Branches 394 398 +4
==========================================
+ Hits 2382 2387 +5
- Misses 716 721 +5
- Partials 394 398 +4 ☔ View full report in Codecov by Sentry. |
I decided to use the opportunity to also suggest a solution for #244. I think the error shortening mechanism is only really relevant for the excessive Most importantly it now also covers warnings: |
Very nice, looking forward to it. |
I lost track of this PR. But it looks like the changes I applied so far are meaningful and complete. Maybe you could have a look, @stschiff. It's time for a Poseidon release with the amazing |
Yes, true, will try to get this finished before I go on vacation, similarly to #304. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, looks great. Love the showParsecErr
trick.
My own preference is not to make definitions like data ErrorLength = CharInf | CharCount Int deriving Show
but instead use Maybe Int
for this, but we had this discussion before, and I guess it comes down to personal coding style. My argument is that something like CharInf
, while seemingly adding semantic interpretability, forces me to not only look up the function definitions where this is used, but also the datatype definition. With Nothing
on the other hand, I can just look up the function definition to understand its semantics. Anyway, totally fine like you have it now.
I have not tested this PR at all, and don't have time before vacation for this, so if you've tested things, then I certainly approve merging this in.
There are some trident error messages that are not well rendered on the command line. This PR is an attempt to improve them. Here's some examples of what I did so far:
-f "<<>"
(or other broken CLI input that goes through parsec)Now:
And with --forgeFile, where the file contains only
<<>
YAML file with broken
orcid
field (or other yml fields)With a broken date in
lastModified
: