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

Add impl ErrorConvert for ContextError #500

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

Lexicality
Copy link
Contributor

Quick fix for #498

I didn't touch TreeError because I couldn't see a particularly easy way of converting input types on it other than rewriting the entire tree which seemed a bit over the top.

I added the InputError -> ContextError conversion because I could rather than because I needed it, I can remove it if you don't want it.

@Lexicality
Copy link
Contributor Author

Huh, weird. It wouldn't build on my machine either but I assumed I had something configured wrong. 🤔

@Lexicality Lexicality force-pushed the errorcontext-bits-compatability branch from e5fcf9b to fc65414 Compare April 7, 2024 19:02
Lexicality added a commit to Lexicality/libmburs that referenced this pull request Apr 7, 2024
I've got a PR open for Winnow (winnow-rs/winnow#500) that should
hopefully render this unnecessary once it's merged & released, but until
then this lets me use context with bit parsers (and simplifies
invocation for `bits::bits`!)
@epage
Copy link
Collaborator

epage commented Apr 9, 2024

I added the InputError -> ContextError conversion because I could rather than because I needed it, I can remove it if you don't want it.

Let's remove that for now as its unrelated to the problem at hand and it would be good to understand the use case for why someone is doing it.

@Lexicality Lexicality force-pushed the errorcontext-bits-compatability branch from fc65414 to 40a0a15 Compare April 11, 2024 08:43
@Lexicality
Copy link
Contributor Author

I added the InputError -> ContextError conversion because I could rather than because I needed it, I can remove it if you don't want it.

Let's remove that for now as its unrelated to the problem at hand and it would be good to understand the use case for why someone is doing it.

done

@coveralls
Copy link

Pull Request Test Coverage Report for Build 8643968260

Details

  • 0 of 2 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 41.817%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/error.rs 0 2 0.0%
Totals Coverage Status
Change from base Build 8541132511: 0.005%
Covered Lines: 1275
Relevant Lines: 3049

💛 - Coveralls

Copy link

codecov bot commented Apr 11, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 41.81%. Comparing base (8512ab7) to head (40a0a15).

Files Patch % Lines
src/error.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #500   +/-   ##
=======================================
  Coverage   41.81%   41.81%           
=======================================
  Files          18       18           
  Lines        3047     3049    +2     
=======================================
+ Hits         1274     1275    +1     
- Misses       1773     1774    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@epage epage merged commit 92b008e into winnow-rs:main Apr 11, 2024
16 of 17 checks passed
@Lexicality Lexicality deleted the errorcontext-bits-compatability branch April 11, 2024 14:16
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.

3 participants