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

Using quick-xml #68

Closed
madig opened this issue Jul 24, 2021 · 6 comments
Closed

Using quick-xml #68

madig opened this issue Jul 24, 2021 · 6 comments
Milestone

Comments

@madig
Copy link
Contributor

madig commented Jul 24, 2021

I see in the 1.0 checklist that you'd like to use quick-xml for the XML part. Me too. What specifically would need to be done?

@ebarnard
Copy link
Owner

quick_xml::Reader requires B: BufRead instead of just B: Read.

I was planning to wait until specialisation was stable so that we can wrap our R: Read in a BufReader if not R: BufRead. Not sure if there's a hacky way to implement this now.

@ebarnard ebarnard added this to the Release 1.4 milestone Sep 17, 2022
@ozgunozerk
Copy link

Any updates on this one? If you need any help, I can volunteer

@ebarnard
Copy link
Owner

ebarnard commented Oct 2, 2022

I've added this to V1.4 as a nice-to-have. I'm not sure if I'll have time to work on it or if @pr2502 has time to finish #78, and would be very happy to accept a PR switching to quick-xml.

It doesn't look like min_specialization is going to hit stable any time soon so we'll have to make do without.

A way forward would be to add plist::from_buf_reader_xml<R: BufRead>(...) to complement the existing plist::from_reader_xml<R: Read>(...) for those who care about performance, or already have a type implementing BufRead and know they're going to be reading an XML plist.

Then, when specialization becomes stable, we can specialise for BufRead on the R: Read parameter of plist::from_reader_xml and plist::from_reader and deprecate plist::from_buf_reader_xml.

@ozgunozerk
Copy link

Thanks for this introduction. I'd like to work on this after I finish my current workload. It seems like I will have the time (most probably, but I cannot promise).
👍

@jcgruenhage
Copy link
Contributor

I've wanted to look into a plist file I couldn't parse again, and before looking deeper into #52, I decided to look into the quick-xml topic again. #88 now contains an up-to-date port to quick-xml, mostly based on @pr2502's work, but with the remaining mentioned issues fixed up and it being rebased on master. The test suite now fully passes again, so if some people could try it out, that'd be great.

@ebarnard
Copy link
Owner

Closed by #88. Thank you again @jcgruenhage for your work on this.

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

No branches or pull requests

4 participants