Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add blade support #9513
Add blade support #9513
Changes from all commits
3cdf30b
9a7d39d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lelgenio I am afraid, without the
#not-has-ancestor
and#has-ancestor
predicates, the injections will not work with the latest versions of thetree-sitter-blade
(v0.9.0^)
Did you remove them because Helix does not support these predicates? I am fairly new to Helix myself and already hooked big time, but not sure about their
tree-sitter
support.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you're right!
@task
s don't work correctly. I wonder if there's an equivalent predicated to#has-ancestor
in helix 🤔#9513 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I just saw the comment by @the-mikedavis
Just to add, Nova added support for this last year with v11 as well. It is quite handy for extremely unusual template grammars! We could maybe do a "feature request" for these?
However for time being, feel free to safely comment/remove this section as it is redundant :)
((text) @injection.content (#has-ancestor? @injection.content "envoy") (#set! injection.combined) (#set! injection.language bash))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like
#has-ancestor?
is working towards recognizing arbitrary nesting tree-sitter/tree-sitter#981It was introduced in neovim/neovim#23606 and scans from the currently captured node potentially up to the root. I don't think that's particularly expensive but I was imagining that when arbitrary nesting was supported upstream by tree-sitter itself (probably a new syntax in the queries) it would track heritage internally and eliminate the cost of scanning.
I will write up an issue for this when I get a chance but let's not block this PR on it since the design is up for debate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@the-mikedavis Absolutely, by all means! The
bash
injection and envoy is for a very specific and niche use case inblade
and its existence or lack of would not do any harm :). In fact no parser out there, had ever had this feature or been able to parseshell
correctly in theblade
files due to their limitation/complexity. In all honesty, I was just trying to see how far I can pushtree-sitter
with this feature when I was writing the grammar for it haha (tree-sitter never fails to please!).But yea, having that section removed or commented for now, before the merge, is very sensible. Just to avoid unintended behaviour, because in the
injection.scm
bothbash
andphp
were being injected to the same node.And I am totally with you on that one, I wished 'tree-sitter' had some form of standardisation for the predicates...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just updated the PR to no longer include the bash injection, by the way.