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: failed to parse :host pseudo class #61

Open
2 tasks done
jordimarimon opened this issue Nov 25, 2024 · 7 comments · May be fixed by #62
Open
2 tasks done

bug: failed to parse :host pseudo class #61

jordimarimon opened this issue Nov 25, 2024 · 7 comments · May be fixed by #62
Labels
bug Something isn't working

Comments

@jordimarimon
Copy link

jordimarimon commented Nov 25, 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 of tree-sitter-css

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

No response

Describe the bug

I have installed the latest version of the parser (commit hash: 6a442a3cf461b0ce275339e5afa178693484c927)

When using the :host pseudo class without arguments, I get errors in the parsed tree.

https://developer.mozilla.org/en-US/docs/Web/CSS/:host

Steps To Reproduce/Bad Parse Tree

The following fails to parse:

:host {
    display: block;
}

This is the AST that I get when I use the InspectTree command in neovim:

(stylesheet ; [0, 0] - [3, 0]
  (ERROR ; [0, 0] - [2, 1]
    (class_name) ; [0, 1] - [0, 5]
    (ERROR) ; [1, 4] - [1, 5]
    (class_name) ; [1, 5] - [1, 7]
    (ERROR) ; [1, 7] - [1, 11]
    (ERROR))) ; [1, 13] - [1, 18]

If I write the following:

:host() {
     display: block;
}

It parses correctly.

Expected Behavior/Parse Tree

It's valid to write the :host pseudo class without arguments.

I don't have a lot of knowledge about writing grammars with tree-sitter but I suspect that this line may be the one causing the conflict:

alias($.pseudo_class_with_selector_arguments, $.arguments),

Repro

:host {
    display: block;
}
@jordimarimon jordimarimon added the bug Something isn't working label Nov 25, 2024
@mohamedmansour
Copy link

/cc @amaanq , :host is really critical to css spec!

jordimarimon added a commit to jordimarimon/tree-sitter-css that referenced this issue Dec 7, 2024
jordimarimon added a commit to jordimarimon/tree-sitter-css that referenced this issue Dec 7, 2024
@mohamedmansour
Copy link

Hey @jordimarimon are you going to open a PR with your change jordimarimon@2b7ecb8 ? /cc @amaanq

@jordimarimon
Copy link
Author

Sorry for the late response @mohamedmansour, I didn't create a pull request because I wasn't quite sure that my approach for fixing the issue would be the best.

What I have done is make optional the arguments in some pseudo classes which may be desirable or not depending if it's a good thing for the grammar to accept more broader inputs.

I don't see it as a bad thing as I'm using treesitter inside neovim and in my case is for syntax highlighting, so I prefer the grammar to accept invalid CSS also, as this way when I'm in the middle of writting something, the AST doesn't have errors and the highlighting looks fine.

If it's okay for the maintainers of this grammar what I have done, I can open a pull request!

@mohamedmansour
Copy link

@amaanq CSS :host parameter is more broken than before now, can we merge @jordimarimon PR in? Would love to maybe contribute to this repo as well since treesitter CSS needs better syntax support in Zed @maxbrunsfeld

@mohamedmansour
Copy link

@jordimarimon can you please open the PR? And we can see how it goes

@mohamedmansour
Copy link

@jordimarimon I tested your change and I am getting error, unless I am loading it wrong

image

@mohamedmansour
Copy link

mohamedmansour commented Jan 1, 2025

Ah it does work @jordimarimon I had to run npx tree-sitter generate before npm run install

image

mohamedmansour added a commit to mohamedmansour/tree-sitter-css that referenced this issue Jan 1, 2025
@mohamedmansour mohamedmansour linked a pull request Jan 1, 2025 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants