-
Notifications
You must be signed in to change notification settings - Fork 238
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
Implement ability to read data directly from the underlying reader #783
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #783 +/- ##
==========================================
- Coverage 61.81% 60.09% -1.72%
==========================================
Files 41 41
Lines 16798 16097 -701
==========================================
- Hits 10384 9674 -710
- Misses 6414 6423 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
src/reader/mod.rs
Outdated
/// # Example | ||
/// | ||
/// This example demonstrates, how it is possible to read embedded binary data. | ||
/// Such XML documents are exist in the wild. |
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 example demonstrates how to read stream raw bytes from an XML document. This could be used to implement streaming read of text, or to read raw binary bytes embedded in an XML document. (Documents with embedded raw bytes are not valid XML, but XML-derived file formats exist where such documents are valid).
src/reader/mod.rs
Outdated
/// // Reading from the stream() advances position | ||
/// let mut inner = reader.stream(); | ||
/// | ||
/// // Read binary data. We somehow should known its size |
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.
We must know its size
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.
Also somewhere (maybe up above, not here), we should mention the two conditions that the user must uphold when using this API and the potential consequences if it's misused.
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.
Mmm... what two conditions? I added a note that read data will not be returned in subsequent Event
s
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.
the user should manually:
- ensure, that it is reading text (binary data)
- stop reading when text (binary data) ends
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.
Basically there are many possible uses of stream()
, but for basically all uses it can be presumed that you probably don't intend to end in the middle of an event or an attribute or something
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.
ensure, that it is reading text (binary data)
Technically speaking, everything which would be read thru this method is a binary data (in particular, it may look as a valid and well-formed XML markup). So that this is not very helpful recommendation
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.
@dralley, what do you think about my last comment? If you think, that we can write something clear do not hesitate to write it -- I decidedly have no thoughts. I mean, that just "when binary data ends" is not clear enough, because this literally can be any condition which is already clear.
Co-authored-by: Daniel Alley <dalley@redhat.com>
Closes #623
This also implements #260, but the user should manually:
So I do not consider that #260 is solved.