-
Notifications
You must be signed in to change notification settings - Fork 242
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
Rename reading methods, fix some encoding errors in error-processing paths and tests #412
Conversation
Codecov Report
@@ Coverage Diff @@
## master #412 +/- ##
==========================================
- Coverage 50.03% 49.58% -0.46%
==========================================
Files 22 22
Lines 13870 13967 +97
==========================================
- Hits 6940 6925 -15
- Misses 6930 7042 +112
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
tests/xmlrs_reader_tests.rs
Outdated
from_utf8(a.key.as_ref()).unwrap(), | ||
from_utf8(&*a.unescaped_value().unwrap()).unwrap() | ||
decoder.decode(a.key.as_ref()).unwrap(), | ||
decoder.decode(&*a.unescaped_value().unwrap()).unwrap() |
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 seems like it would be another instance of the incorrect "unescape, then decode" behavior mentioned, yes?
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.
Yeah, you right. Need to fix that too
In the future, something like https://crates.io/crates/buffered-reader might be nice - the internal buffer can dynamically resize which would allow avoiding the copy between the Unfortunately, at the moment, it is LGPL licensed which makes it a bit challenging to depend on. |
Also, Text events was incorrectly unescaped before decoding instead of correct decode, then unescape
…e number when test fail
- read_event => read_event_into - read_to_end => read_to_end_into - read_text => read_text_into - read_event_unbuffered => read_event - read_to_end_unbuffered => read_to_end - read_event_buffered => read_event_impl
…sRef<[u8]>` AsRef is too unsafe because you accidentally could pass wrong parameters Also fixes using incorrect encoding if `read_to_end` family of methods or `read_text` method would not find a corresponding end tag and the reader has a non-UTF-8 encoding
Co-authored-by: Daniel Alley <[email protected]>
This PR includes some of the stuff, improving code here and there, including:
fix incorrect encoding used when handle some errors
remove excess
BufRead
constraints which opens door for theasync
support fo the sameReader
structsome minor improvements in tests
some documentation updates
rename core methods to better reflect their purpose:
read_event
read_event_into
read_to_end
read_to_end_into
read_text
read_text_into
read_event_unbuffered
read_event
read_to_end_unbuffered
read_to_end
(
read_namespaced_event
is not renamed because I plan to remove it in the namespace PR)