Open sxhxliang opened 1 year ago
I had a quick look, and it does not seem to be a simple drop-in replacement as both crates use a slightly different approach. Also XML-related code quickly becomes interdependent, as by nature there is a lot of nested calls.
It seems to represent a significant rework on the crate, which potential side-effect difficult to isolate. To give you an example, migrating comment_extended
and comments_extended
to quick-xml gave me some failing tests due to some issues with comment
and comments
.
I had say it would probably require writing a lot more tests, and maybe introduce a gate feature.
@bokuweb let me know if you think it is worth it.
Hello again, I had a closer look and it seems we could use the serde capabilities of quick-xml to simplify part of the code. It will however probably be a significant/big change that will impact the crate, as it means essentially overhauling the whole serialization/deserialization process.
@bokuweb please let me know if you are open to such a change.
@git-noise Hello. I am highly motivated to switch to quick-xml and to reduce the code, but I am concerned about breaking changes. If the changes can be minimized, I would definitely consider it.
What I can do is showcase the migration gains/trade-offs on a few elements on the reader-side. Some of them seem relatively "isolated" enough that they could be good candidates. I'll put that into a branch on my fork and you can then see if it goes in the right direction or not. We can then maybe decide on /design a more comprehensive action plan?
Would that work?
@bokuweb
Hello,
I migrated some of the code - here https://github.com/git-noise/docx-rs/tree/quick-xml - you can see some initial work done on:
They were picked up a bit randomly because they're fairly independent and could easily be isolated.
For now the reader logic - so essentially deserialization - relies on one additional Trait FromXMLQuickXml
which is the pendant of FromXML
for quick-xml
==== TLDR:
==== In more details - please note that this is based on the current code changes, and it may not hold once we migrate more modules:
Pros:
ElementReader
trait would probably not be needed anymore.Cons:
Hello, @bokuweb, just wanted to check if you had time to review this one
Is your feature request related to a problem? Please describe.
On a particular file, quick-xml is around 50 times faster than xml-rs crate. https://github.com/tafia/quick-xml#performance-1
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
Describe the solution you'd like
A clear and concise description of what you want to happen.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context or screenshots about the feature request here.