Split out namespace aware reader to a struct that manage the internal namespaces buffer #437
Conversation
This commit only moves code without significant changes (the only changes is: - corrected imports - add imports to the doc comments which have become inaccessible )
The following commit will not allow to call `reader.decoder()` while reference to ResolveResult is valid, so we'll need to save decoder before the loop and update it when it could be changed
Codecov Report
@@ Coverage Diff @@
## master #437 +/- ##
==========================================
+ Coverage 49.51% 51.50% +1.99%
==========================================
Files 22 25 +3
Lines 13847 13310 -537
==========================================
- Hits 6856 6855 -1
+ Misses 6991 6455 -536
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
src/reader/ns_reader.rs
Outdated
| /// [`End`]: Event::End | ||
| /// [`read_event()`]: Self::read_event | ||
| #[inline] | ||
| pub fn read_event_ns(&mut self) -> Result<(ResolveResult, Event<'i>)> { |
There was a problem hiding this comment.
I'm not sure this naming is an improvment, it feels too brief. Perhaps read_event_with_ns() or read_event_and_ns() would be better.
There was a problem hiding this comment.
I agree, not a better name. May be read_resolved_event?
There was a problem hiding this comment.
Changed to read_resolved_event / read_resolved_event_into
… to it This commit only moves code and do not introduce any improvements
This commit only moves code
`Deserializer` implementation can buffer events and it will not be able to buffer the `resolve` result because it is bound to the `NsReader`
Co-authored-by: Daniel Alley <dalley@redhat.com>
…ll non-namespace-aware mutation methods Because `impl DerefMut<Target=Reader> for NsReader` is removed, all Reader's methods that accept `&mut self` won't be available for the NsReader, including configuration methods. Because we need a way to configure our reader, we need to reimplement necessary methods that accepts `&mut self`. In order to not duplicate the code and the doc of this configure methods, they was put into the macro `configure_methods!` and embed to both readers.
src/reader/mod.rs
Outdated
| @@ -77,6 +77,19 @@ macro_rules! configure_methods { | |||
|
|
|||
| /// Changes whether mismatched closing tag names should be detected. | |||
| /// | |||
| /// Note, that start and end tags [should match literally][spec], they cannot | |||
| /// have different prefixes even if both prefixes resolves to the same namespace. | |||
| /// The | |||
There was a problem hiding this comment.
This should be either
The XML
or
This XML:
or
This:
There was a problem hiding this comment.
Also "both prefixes resolves" -> "both prefixes resolve"
There was a problem hiding this comment.
Done ("The XML") and done!
Co-authored-by: Daniel Alley <dalley@redhat.com>
This is PR for low-level namespaces support in the quick-xml, on which high-level serde support will be implemented.
The goals of this PR is to make a safe interface and leave the option of not paying for namespace support if it is not needed. For that the buffer with readed namespaces now managed by the dedicated
NsReaderreader, which should be used instead ofReaderwhen you need namespaces. Because now buffer is stored inside the reader, the user cannot accidently clear it and get a panic or wrong results later, when it would try to resolve namespace. In addition, it is impossible now to mix read methods with and without namespace awareness, which could lead to hard-to-catch bugs.Originally, I've planned to include the serde part into this PR, but number of commits for support serde grows and I've decided to made a dedicated PR for that. At least, the serde part slightly depends on #434, because those changes will break some removed tests.
I would like to merge that before #425, because:
ReaderintoSliceReaderandBufferedReader#425ReaderintoSliceReaderandBufferedReader#425ReaderintoSliceReaderandBufferedReader#425 next week