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

Exclude only named children without injection.include-children #3129

Merged
merged 2 commits into from
Aug 3, 2022

Conversation

MDeiml
Copy link
Contributor

@MDeiml MDeiml commented Jul 21, 2022

This fixes problems arising in #3108.

The markdown parser needs to exclude children for language injection, but only named ones. This is different from the behavior in tree-sitter-highlight, but to me it seems more sensible.

To give a technical example: Otherwise there is almost no way to write something like

rule: $ => seq('(', repeat(choice($._content, $.excluded)), ')'),

where $._content and the parentheses should be included and $.excluded excluded. This is because there will always be '(' and ')' nodes as a child of rule.

@the-mikedavis
Copy link
Member

If you want a node from the tree to be queryable, it should be named: that's the point of named nodes. I think you should restructure the grammar to expose the nodes you want to capture in queries.

I can't test this at the moment but I suspect this breaks injections for some templating languages or maybe Rust macros? I'll give it a look when I have a chance

@MDeiml
Copy link
Contributor Author

MDeiml commented Jul 26, 2022

If you want a node from the tree to be queryable, it should be named: that's the point of named nodes. I think you should restructure the grammar to expose the nodes you want to capture in queries.

That's my point. I expose inline nodes and block_continuation nodes. I want to capture the former while excluding the latter. I don't want to exclude unnamed nodes that I have less control over when writing the grammar.

Also notice how not excluding unnamed nodes is strictly more expressive. In a tree-sitter grammar an unnamed node can always be turned into a named one with alias(...). I think this is exactly what you meant?

I can't test this at the moment but I suspect this breaks injections for some templating languages or maybe Rust macros? I'll give it a look when I have a chance

I'm gonna have a look myself too. If that's the case and the queries can't easily be changed to work with the new behavior: how do you feel about adding a include-unnamed or exclude-named flag? I'm not sure it's possible to make my grammar work without this.

@MDeiml
Copy link
Contributor Author

MDeiml commented Jul 26, 2022

It seems you're right. Many languages rely on excluding unnamed nodes for injecting the comment language.

@the-mikedavis
Copy link
Member

the-mikedavis commented Jul 26, 2022

A separate property like include-unnamed sounds like a good addition to me 👍

@MDeiml MDeiml force-pushed the exclude-named-children branch from 574fac3 to d32deb1 Compare July 27, 2022 21:34
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, thanks for working on it 👍

@archseer archseer merged commit 6b244e2 into helix-editor:master Aug 3, 2022
@MDeiml MDeiml deleted the exclude-named-children branch August 3, 2022 12:18
thomasskk pushed a commit to thomasskk/helix that referenced this pull request Sep 9, 2022
…-editor#3129)

* Exclude only named children without injection.include-children

* Add injection.include-unnamed-children parameter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants