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

quick_xml::de::from_reader requires a BufRead instead of just an std::io::Read #716

Open
phdavis1027 opened this issue Feb 23, 2024 · 2 comments

Comments

@phdavis1027
Copy link
Contributor

phdavis1027 commented Feb 23, 2024

I'm curious why this is the case. Does the implementation require buffering? If not, why not users worry about whether the read uses buffering? Similar thoughts as #499 here. Unless there's particular hesitance, would the maintainers be open to a PR allowing implementations which use unbuffered read?

EDIT: For the purposes of forthrightness I'll go ahead and explain the usecase that caused me to think about this: I have a trait Protocol whose implementers I would like to be stateless (i.e., the structs underlying implementers are just empty types to get to the functions). So I can't give implementers a buffer to manage, and I don't want to allocate extra memory every time I serialize a protocol message. Conversely, I don't want my API to assume consumers are using buffered reading, which may not be their preference for any number of reasons, and so I don't want the buffer to come as an argument to any Protocol functions.

@phdavis1027 phdavis1027 changed the title quick_xml::de::from_reader requires a BufReader instead of just an std::io::Read quick_xml::de::from_reader requires a BufRead instead of just an std::io::Read Feb 23, 2024
@Mingun
Copy link
Collaborator

Mingun commented Feb 24, 2024

Yes, if you look at the code, parsing uses BufRead::fill_buf. Because quick-xml returns tokens with contents of tags it should somewhere store content until the full tag will be recognized. If the first read returns <ab we need to wait while c> would be read so we can recognize the <abc> tag.

Instead of doing buffering itself quick-xml only requires bufferisation support from source which is what the idea of traits. If you think, that this is unnecessary limitation (maybe it is!) please make a PR!

@phdavis1027
Copy link
Contributor Author

Upon thinking about it, this might be best. For any text-based serialization format like UTF-8 buffering probably is more efficient. That said, it's not inconceivable others might have weird usescases like mine where they might not want to be tied to BufRead. I am currently not at a place in my project where I feel able to invest in that PR, but I might revisit it over the summer when I might have more time on my hands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants