-
-
Notifications
You must be signed in to change notification settings - Fork 43
Description
Did you check existing issues?
- I have read all the tree-sitter docs if it relates to using the parser
- I have searched the existing issues of tree-sitter-css
Tree-Sitter CLI Version, if relevant (output of tree-sitter --version
)
No response
Describe the bug
The problem
Consider this selector:
div {
display: flex;
}
stylesheet (0, 0) – (4, 0)
rule_set (0, 0) – (2, 1)
selectors (0, 0) – (0, 3)
tag_name (0, 0) – (0, 3)
block (0, 4) – (2, 1)
declaration (1, 2) – (1, 16)
property_name (1, 2) – (1, 9)
plain_value (1, 11) – (1, 15)
Now suppose I want to add a new declaration above display: none;
. I insert a line break and start typing:
div {
padding
display: flex;
}
stylesheet (0, 0) – (5, 0)
rule_set (0, 0) – (3, 1)
selectors (0, 0) – (0, 3)
tag_name (0, 0) – (0, 3)
block (0, 4) – (3, 1)
ERROR (1, 2) – (2, 16)
descendant_selector (1, 2) – (2, 9)
tag_name (1, 2) – (1, 9)
tag_name (2, 2) – (2, 9)
ERROR (2, 11) – (2, 15)
tree-sitter-css
does a naive thing here: since descendant combinators can technically tolerate a line break as their whitespace (I think?), it assumes it's finding a selector called padding display
, and doesn't reassess its conclusion when it hits the :
(colon plus space, which rules out a pseudoclass).
If you add a colon, the tree changes:
div {
padding:
display: flex;
}
stylesheet (0, 0) – (5, 0)
rule_set (0, 0) – (3, 1)
selectors (0, 0) – (0, 3)
tag_name (0, 0) – (0, 3)
block (0, 4) – (3, 1)
declaration (1, 2) – (2, 16)
property_name (1, 2) – (1, 9)
plain_value (2, 2) – (2, 10)
plain_value (2, 11) – (2, 15)
Luckily, if you type a ;
, the error goes away (which is maybe too aggressive, considering it's not valid CSS):
div {
padding:;
display: flex;
}
stylesheet (0, 0) – (5, 0)
rule_set (0, 0) – (3, 1)
selectors (0, 0) – (0, 3)
tag_name (0, 0) – (0, 3)
block (0, 4) – (3, 1)
declaration (1, 2) – (1, 11)
property_name (1, 2) – (1, 9)
integer_value (1, 10) – (1, 10)
declaration (2, 2) – (2, 16)
property_name (2, 2) – (2, 9)
plain_value (2, 11) – (2, 15)
In Pulsar, this results in distracting re-highlighting of input, as illustrated below:
tree-sitter-css-inside-block-parsing.mp4
Unfortunately, it applies to every property after the cursor until :
is inserted — they're all incorrectly reinterpreted as tag names.
The fix
In general, I think it's a good goal for incomplete input on one line to generate an ERROR
node but to somehow recover before the valid input on the next line. Easier said than done, of course.
It used to be easier to rule out this scenario, but CSS nesting makes it so that you really could have valid CSS like the following:
div {
padding display {
display: flex;
}
}
It's implausible in this specific case, but I don't think the solution is to try to whitelist tag names or property names (we'd always be playing catch-up).
Maybe this is a job for conflicts
somehow? Conceptually, it seems like the best way to proceed is on two parallel paths. If {
is encountered first, the “interpret as a selector” path wins. If ;
is encountered first (or something else that rules out a selector, like :
, then the “interpret as an incomplete property-value pair” path wins.
Steps To Reproduce/Bad Parse Tree
stylesheet (0, 0) – (5, 0)
rule_set (0, 0) – (3, 1)
selectors (0, 0) – (0, 3)
tag_name (0, 0) – (0, 3)
block (0, 4) – (3, 1)
ERROR (1, 2) – (2, 16)
descendant_selector (1, 2) – (2, 9)
tag_name (1, 2) – (1, 9)
tag_name (2, 2) – (2, 9)
ERROR (2, 11) – (2, 15)
Expected Behavior/Parse Tree
stylesheet (0, 0) – (5, 0)
rule_set (0, 0) – (3, 1)
selectors (0, 0) – (0, 3)
tag_name (0, 0) – (0, 3)
block (0, 4) – (3, 1)
ERROR (1, 2) – (1, 9)
identifier (1, 2) – (2, 9)
declaration (2, 2) – (2, 16)
property_name (2, 2) – (2, 9)
plain_value (2, 11) – (2, 15)
Repro
div {
padding
display: flex;
}