Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

bug: Poor error recovery when content is added inside a block #65

Open
2 tasks done
savetheclocktower opened this issue Jan 11, 2025 · 0 comments
Open
2 tasks done
Labels
bug Something isn't working

Comments

@savetheclocktower
Copy link
Contributor

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 conflictssomehow? 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;
}
@savetheclocktower savetheclocktower added the bug Something isn't working label Jan 11, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant