Skip to content
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

bug: Unable to parse some of the modern syntaxes #4

Open
1 of 2 tasks
bajrangCoder opened this issue May 6, 2024 · 5 comments
Open
1 of 2 tasks

bug: Unable to parse some of the modern syntaxes #4

bajrangCoder opened this issue May 6, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@bajrangCoder
Copy link

bajrangCoder commented May 6, 2024

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

Tree-Sitter CLI Version, if relevant (output of tree-sitter --version)

No response

Describe the bug

The current version of Tree-sitter-scss does not fully parse certain syntaxes in SCSS, such as variable interpolation

Examples:

  1. Variable Interpolation:
color: #{$color};

These syntaxes are not parsed correctly by Tree-sitter, leading to potential parsing errors and inconsistencies in syntax highlighting.

Steps To Reproduce/Bad Parse Tree

Just try to parse the above examples

Expected Behavior/Parse Tree

It should parse those syntaxes without any errors

Repro

No response

@bajrangCoder bajrangCoder added the bug Something isn't working label May 6, 2024
@xpe
Copy link

xpe commented May 6, 2024

With the caveat that I'm not very familiar with tree sitter grammars (yet): I would start by looking at

"declaration": {

"declaration": {
      "type": "SEQ",
      "members": [
        {
          "type": "ALIAS",
          "content": {
            "type": "CHOICE",
            "members": [
              {
                "type": "SYMBOL",
                "name": "identifier"
              },
              {
                "type": "SYMBOL",
                "name": "variable"
              },
              {
                "type": "SYMBOL",
                "name": "_concatenated_identifier"
              }
            ]
          },
          "named": true,
          "value": "property_name"
        },
        {
          "type": "STRING",
          "value": ":"
        },
        {
          "type": "SYMBOL",
          "name": "_value"
        },
        {
          "type": "REPEAT",
          "content": {
            "type": "SEQ",
            "members": [
              {
                "type": "CHOICE",
                "members": [
                  {
                    "type": "STRING",
                    "value": ","
                  },
                  {
                    "type": "BLANK"
                  }
                ]
              },
              {
                "type": "SYMBOL",
                "name": "_value"
              }
            ]
          }
        },
        {
          "type": "CHOICE",
          "members": [
            {
              "type": "SYMBOL",
              "name": "important"
            },
            {
              "type": "BLANK"
            }
          ]
        },
        {
          "type": "STRING",
          "value": ";"
        }
      ]
    },

@xpe
Copy link

xpe commented May 6, 2024

@bajrangCoder As I also mention over at bajrangCoder/zed-scss#2 (comment), the following is not valid SCSS:

body {
    @mixin body_long_14;
    color: red;
}

Per Sass documentation: @mixin and @include, it is correct to use @include; for example:

body {
    @include body_long_14;
    color: red;
}

This said, from what I can tell, the current tree-sitter grammar here does not support the latter (correct) example.

@bajrangCoder
Copy link
Author

@bajrangCoder As I also mention over at bajrangCoder/zed-scss#2 (comment), the following is not valid SCSS:

body {
    @mixin body_long_14;
    color: red;
}

Per Sass documentation: @mixin and @include, it is correct to use @include; for example:

body {
    @include body_long_14;
    color: red;
}

This said, from what I can tell, the current tree-sitter grammar here does not support the latter (correct) example.

I'm apologised for my confusion

@bajrangCoder
Copy link
Author

As for variable Interpolation, it's already implemented here

interpolation: $ => seq('#{', $._value, '}'),
but still it's not able to parse , no idea why🤔

@xpe
Copy link

xpe commented May 6, 2024

@bajrangCoder Speaking of the nested @include, without having dug in very much, I suspect the key problem is that the "declaration" section above is that it does not include "include_statement".

Update as of 2024-05-06 9:21 am EDT: ah, now that I've read a bit more, I see now that grammar.js is the key starting point. To concentrate on enabling @include inside a declaration, I'm looking at:

// Declarations

// Declarations

declaration: $ => seq(
  alias(
    choice($.identifier, $.variable, $._concatenated_identifier),
    $.property_name,
  ),
  ':',
  $._value,
  repeat(seq(optional(','), $._value)),
  optional($.important),
  ';',
),

... and I'm not seeing any inclusion of include_statement, which is probably a problem.

I would bet (high confidence) that it will make sense and be practical to extract a new rule that parses one "property: value" pair. Call it property_value_pair perhaps? I'm not sure about the name, so this is only a starting guess.

Then declaration can parse a seq of either include_statement or property_value_pair.

@bajrangCoder Does this make sense to you?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants