-
Notifications
You must be signed in to change notification settings - Fork 140
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
Support groupmark #1093
Support groupmark #1093
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1093 +/- ##
=======================================
Coverage 90.40% 90.40%
=======================================
Files 9 9
Lines 2293 2294 +1
=======================================
+ Hits 2073 2074 +1
Misses 220 220
☔ View full report in Codecov by Sentry. |
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.
This looks great @LilithHafner! Can I request one more change? We have the reading.md file where we try to spell out in detail all the various keyword args that are supported. Could you add an entry there for groupmark
and if you're feeling ambitious, link to an example in the example.md file?
That's the kind of request that leads to well-documented packages. Thank you. |
After this PR, I am seeing a compile error in conjunction with Parsers 2.4.2, so I think Parsers compat needs to be bumped to 2.5.
FWIW, it seems like the PR exposed a bug where the |
That is unfortunate—I imagine the best fix is the stop mixing up those arguments rather than restricting compat. Unfortunately, I do not have the bandwidth to debug that rn. Perhaps @quinnj is willing to look into it? |
Good catch @JackDunnNZ; mind making a PR to bump Parsers compat up to 2.5? |
Closes #626. These features don't implement themselves, even when seven separate people comment asking for them.
See also JuliaData/Parsers.jl#168. This PR fixes none of that.
This is my first time working with this codebase and its style and I may have missed something. I'd appreciate a careful review.