Parser changes for Gecko integration #168
Conversation
| @@ -226,7 +226,7 @@ impl<'a> Tokenizer<'a> { | |||
| input: input, | |||
| position: 0, | |||
| last_known_source_location: Cell::new((SourcePosition(0), | |||
| SourceLocation { line: 1, column: 1 })), | |||
| SourceLocation { line: 0, column: 0 })), | |||
There was a problem hiding this comment.
SourceLocation’s doc-comments needs to up updated accordingly.
| @@ -188,6 +188,11 @@ impl<'i: 't, 't> Parser<'i, 't> { | |||
| } | |||
| } | |||
|
|
|||
| /// Return the current line that is being parsed. | |||
There was a problem hiding this comment.
Newlines are not significant in CSS. Why is the current line relevant? Is this counting on authors not using minification, adding newlines between declarations, and not writing multi-line declarations?
There was a problem hiding this comment.
Gecko's error messages include a sourceLine property which is expected to be the current line being parsed.
|
Related Gecko work is in the second patch of https://bugzilla.mozilla.org/show_bug.cgi?id=1352669. |
|
r+ with the doc-comment updated for line and columns numbers starting at 0 rather than 1. I still think that "current line" is not the right thing to do, but I suppose adding it will help enable these tests and get more test coverage of other things more easily than changing the tests to do something else. So let’s add it. |
|
@bors-servo: r=SimonSapin |
|
📌 Commit c6156c0 has been approved by |
Parser changes for Gecko integration This is a grab bag of changes that were important for getting Stylo tests passing. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-cssparser/168) <!-- Reviewable:end -->
|
☀️ Test successful - status-travis |
|
@jdm It looks like you landed this without the doc-comment change I requested. Could you fix that? CC https://bugzilla.mozilla.org/show_bug.cgi?id=1380890 |
|
Weird; I remember making that change. |
|
I misread the diff, sorry. |
This is a grab bag of changes that were important for getting Stylo tests passing.
This change is