CSS parsing #1270
CSS parsing #1270
Conversation
mrdimidium
force-pushed
the
wp/mrdimidium/css-parsing
branch
from
b0c72ed to
627312b
Compare
karlseguin
left a comment
karlseguin
left a comment
There was a problem hiding this comment.
Since it's still in draft, I figured I'd just comment on the superficial (but still important)
| /// A `<function-token>` | ||
| /// | ||
| /// The value (name) does not include the `(` marker. | ||
| Function: []const u8, |
There was a problem hiding this comment.
We tend to follow Zig's naming guidelines, which has tag names as snake_case.
|
|
||
| // If true, the input has at least `n` bytes left *after* the current one. | ||
| // That is, `Lexer.byte_at(n)` will not panic. | ||
| fn has_at_least(self: *const Tokenizer, n: usize) bool { |
There was a problem hiding this comment.
And function names are camelCase
| // start_pos is at code point boundary, after " or ' | ||
| const start_pos = self.position; | ||
|
|
||
| while (self.is_eof()) { |
There was a problem hiding this comment.
Is this right? I think it shoud be while (!self.is_eof()) { good indication of a missing test.
| }; | ||
|
|
||
| self.advance(2); | ||
| } else if (self.has_at_least(1) and std.ascii.isDigit(self.byte_at(2))) { |
There was a problem hiding this comment.
If we're checking byte_at(2), should this be self.has_at_least(2) ?
|
|
||
| var b = self.next_byte_unchecked(); | ||
| if (b == '-') { | ||
| b = self.next_byte() orelse return false; |
There was a problem hiding this comment.
Isn't b guaranteed to be '-' here?
|
|
||
| fn consume_unquoted_url(self: *Tokenizer) ?Token { | ||
| _ = self; | ||
| @panic("unimplemented"); |
There was a problem hiding this comment.
If this isn't something we plan on implementing now, we should log, not panic.
| // over ASCII bytes (excluding newlines), or UTF-8 sequence | ||
| // leaders (excluding leaders for 4-byte sequences). | ||
| fn advance(self: *Tokenizer, n: usize) void { | ||
| if (builtin.mode == .Debug) { |
There was a problem hiding this comment.
I'm not sure this is super useful...It's not really adding checks in debug for our code, it's adding checks in debug for external code (css). If the assertion fails, what are we supposed to do about it?
| } | ||
|
|
||
| return switch (b) { | ||
| 'a'...'z', 'A'...'Z', '_', 0x0 => true, |
There was a problem hiding this comment.
is 0x0 really valid here (or anywhere else)
No description provided.