-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Contextful validity errors, null-byte specs fix #9
base: main
Are you sure you want to change the base?
Contextful validity errors, null-byte specs fix #9
Conversation
Codecov Report
@@ Coverage Diff @@
## main #9 +/- ##
==========================================
- Coverage 66.01% 59.13% -6.89%
==========================================
Files 1 1
Lines 103 115 +12
==========================================
Hits 68 68
- Misses 27 36 +9
- Partials 8 11 +3
Continue to review full report at Codecov.
|
20a958c
to
b5ec11a
Compare
Based on GitHub Actions benchmarks, It would be nice to have some tests for invalid inputs, maybe even checking the position (although that might be overkill). |
b5ec11a
to
124dee0
Compare
type InvalidRuneError struct {
Rune rune
Position int
} |
I think this change would almost exclusively be used for tests, so I'm not too sure. I was originally thinking parsing the error string for the reported position and checking if it matches what is expected (hence me calling it overkill). |
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.
I think we should add the position to InvalidRuneError
and add tests for invalid inputs, also checking the position to ensure it matches.
Do you have a set of reference invalid inputs? |
A few examples:
|
This pull request adds meaningful errors while decoding as well as proper null-byte encoding.
Benchmarks
New is the pull request and old is
upstream/main
. Test performed on a laptop with an Intel Core i5-8250U.The performance loss in
DecodeFrom
is probablyscanner.Text()
having to allocate a new string for each chunk. The speed is still fairly decent, though.