Consider XML version when parse XML using Deserializer#938
Consider XML version when parse XML using Deserializer#938Mingun merged 4 commits intotafia:masterfrom
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #938 +/- ##
==========================================
+ Coverage 55.00% 55.08% +0.08%
==========================================
Files 44 44
Lines 16816 16911 +95
==========================================
+ Hits 9249 9316 +67
- Misses 7567 7595 +28
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
| Event::Text(e) => { | ||
| black_box(e.xml_content()?); | ||
| black_box(e.xml10_content()?); |
There was a problem hiding this comment.
Why bother with this when xml_content() is what you would generally expect users to be using more frequently?
This makes it explicit, but I don't see why making it explicit matters in this case, given (as the commit message states) all documents declare being version 1.0
There was a problem hiding this comment.
Generally I expect that people will read Event::Decl, store .xml_version() from it and use that version in .xml_content(). For that xml_content() was changed in the next commit to accept XmlVersion.
There was a problem hiding this comment.
In my mind that still means we would want to benchmark .xml_content(XmlVersion::1_0), because that is how we expect it to be used. Especially for "macro" benchmarks.
I understand the practical difference is basically nonexistent, it's more philosophical. And just a suggestion. Not going to block over it.
dralley
left a comment
dralley
left a comment
There was a problem hiding this comment.
Apart from my question, this looks good
In all places XML documents has version 1.0, so explicitly request content in that version
failures:
xml_version::unknown
xml_version::v1_0_explicit
xml_version::v1_0_implicit
Fixes all errors
3 participants
XML 1.0 and XML 1.1 has different rules for end-of-line normalization. Previously quick-xml used XML 1.1 rules, while it should rely on XML declaration.
After this PR XML declaration will be dictate the mode.
This PR introduces
XmlVersionenumeration and makes breaking changes forxml_version()methods: now they accept the version.Because in practice there very highly unlikely will be versions other that 1.0 and 1.1, all other version strings produces new
IllFormedError::UnknownVersionerror.