-
Notifications
You must be signed in to change notification settings - Fork 63
Implement tokenization errors as per spec. #92
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
Conversation
…NTEXT and Script data states.
Adding tests for Tag Name parse errors
Comments parse errors
More comment parse errors
Review fixes2
Add error for duplicate attribute
Character error fixes
Spec PR has been merged. This one is ready for review. |
Ping @gsnedders |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't actually checked they match the spec, because that would take forever, and I'd rather just wait until some implementation notices they don't match.
Apart from the one issue, what's going on with the tree construction tests? Some of the #errors
sections have been partially updated for the tokenizer errors, in addition to the #new-errors
sections? (Personally, I have a slight preference against adding a new section temporarily, given I know some things rely on the specific headings.)
@@ -65,7 +65,6 @@ tokens are: | |||
["EndTag", name] | |||
["Comment", data] | |||
["Character", data] | |||
"ParseError" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should document the new errors
property.
Unfortunately, seems like some error codes were erroneously added to the legacy error section and we missed it on review. I'll do a cleanup. Update: I've figured out a cause of this issue: some error codes that didn't get into the final cut were the same as legacy error codes. During automatic rename they were replaced with new codes.
I thought it would be an unobtrusive way for a transition to the new parse errors. The other options are:
Do you have any other ideas? |
Not really. :) Adding a new section probably makes as much sense as anything else. |
@gsnedders Fixed |
It's not used anymore with changes in html5lib#92.
- Use new initial states in tests according to: html5lib/html5lib-tests#101 - Implement tokenization errors introduced in: whatwg/html#2701 html5lib/html5lib-tests#92
For what it's worth, the code I'm testing, Nokogumbo, outputs each (It also tests that the line numbers match and the column numbers in the test are at least as large as the column numbers Nokogumbo outputs. I can't test for column number equality because Nokogumbo outputs column numbers at a place that would be most useful for humans, not the column where the error was detected.) |
Ready for a review, but it makes sense to merge spec changes first. Note that for now we've decided to move new error codes to a separate section in tree construction stage tests to not mix things up. Once we have a spec for tree construction stage errors, we'll remove old errors and move new errors to
#errors
section.Spec PR: whatwg/html#2701