| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
|
|
|
|
|
|
|
|
Conceptually the tokenizer emits tokens, which are then handled in the
tree construction stage (which this crate doesn't yet implement).
While the tokenizer can operate almost entirely based on its state
(which may be changed via Tokenizer::set_state) and its internal state,
there is the exception of the 'Markup declaration open state'[1], the third
condition of which depends on the "adjusted current node", which in turn
depends on the "stack of open elements" only known to the tree constructor.
In 82898967320f90116bbc686ab7ffc2f61ff456c4 I tried to address this
by adding the adjusted_current_node_present_and_not_in_html_namespace
method to the Emitter trait. What I missed was that adding this method
to the Emitter trait effectively crippled the composability of the API.
You should be able to do the following:
struct TreeConstructor<R, O> {
tokenizer: Tokenizer<R, O, SomeEmitter<O>>,
stack_of_open_elements: Vec<NodeId>,
// ...
}
However this doesn't work if the implementation of SomeEmitter
depends on the stack_of_open_elements field.
This commits remedies this oversight by removing this method and
instead making the Tokenizer yield values of a new Event enum:
enum Event<T> { Token(T), CdataOpen }
Event::CdataOpen signals that the new Tokenizer::handle_cdata_open
method has to be called, which accepts a CdataAction:
enum CdataAction { Cdata, BogusComment }
the variants of which correspond exactly to the possible outcomes
of the third condition of the 'Markup declaration open state'.
Removing this method also has the added benefit that the DefaultEmitter
is now again spec-compliant, which lets us expose it again in the next
commit in good conscience (previously it just hard-coded the method
implementation to return false, which is why I had removed the
DefaultEmitter from the public API in the last release).
[1]: https://html.spec.whatwg.org/multipage/parsing.html#markup-declaration-open-state
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
It doesn't make sense that you're able to construct
a Tokenizer/NaiveParser that you're unable to iterate over.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Display impls should return human-readable strings. After
this commit we're able to introduce a proper Display impl
in the future without that being a breaking change.
|
|
|
|
|
|
Just a bit more succinct. And now rustdoc also no longer
cuts off the names of these Emitter methods in its sidebar.
|
|
|
|
|
|
Note that while making this breaking change, we're also swapping
the parameter order for more consistency so that the reader
parameter always comes last in Emitter methods.
|
|
This spares us two usizes per AttrInternal<Range<usize>>.
So on a 64 bit target where a usize is 8 bytes this spares
us 16 bytes of memory per attribute (if spans are enabled,
... for Token<()> this obviously doesn't change anything).
And the DefaultEmitter now also no longer has to update the
spans on each Emitter::push_attribute_(name|value) call.
The spans are now calculated on demand by the Attribute methods,
which is fine since the assumption is that API users are only
interested in a few specific spans (rather than all spans).
|
|
|
|
Making this change made me realize that adding an
`impl IntoIterator for T` can be a breaking change if
`impl IntoIterator for &T` already exists.
See also the cargo-semver-checks issue[1] I filed about that.
[1]: https://github.com/obi1kenobi/cargo-semver-checks/issues/518
|
|
This has a number of benefits:
* it hides the implementation of the map
* it hides the type used for the map values
(which lets us e.g. change name_span to name_offset while still
being able to provide a convenient `Attribute::name_span` method.)
* it lets us provide convenience impls for the map
such as `FromIterator<(String, String)>`
|
|
This is done separately so that the following commit has a cleaner diff.
|
|
Also more performant since we no longer have to update
the name span on every Emitter::push_tag_name call.
|
|
|
|
Previously the PosTrackingReader always mysteriously subtracted 1
from the current position ... this wasn't sound at all ... the machine
just happens to often call `Tokenizer::unread_char` ... but not always.
E.g. for proper comments it didn't which resulted in their offset and
spans being off-by-one, which is fixed by this commit (see test_spans.rs).
|
|
Emitters should not have access to the reader at all. Also the
current position of the reader, at the time an Emitted method is
called, very much depends on machine implementation details such
as if `Tokenizer::unread_char` is used. Having the Emitter
methods take offsets lets the machine take care of providing
the right offsets, as evidenced by the next commit.
|
|
|
|
|
|
`std::mem::size_of::<Range<NoopOffset>>()` is 0
so there's no need to abstract over Range.
|
|
Previously Span was generic over R just
so that it could provide the method:
fn from_reader(reader: &R) -> Self;
and properly implementing that method again
relied on R implementing the Position trait:
impl<P: Position> Span<P> for Range<usize> { .. }
which was a very roundabout and awkward way of doing things.
It makes much more sense to make the Position trait generic
over the return type of its method (which previously always had
to be usize). Which lets us provide a blanket implementation:
impl<R: Reader> Position<NoopOffset> for R { .. }
|
|
`#![deny(missing_docs)]` makes `cargo test` abort immediately
if any public API member is missing a doc comment ...
which is quite annoying when experimenting with API designs.
Also sometimes refactor commits (such as the very next commit)
introduce new types that are then immediately removed afterwards,
this should be possible without having to add a `/// TODO```
(which contrary to a compiler warning is easy to miss).
|
|
More in line with RFC 344.[1]
[1]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#gettersetter-apis
|
|
|
