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

Add blade support #9513

Merged
merged 2 commits into from
Mar 17, 2024
Merged

Conversation

lelgenio
Copy link
Contributor

@lelgenio lelgenio commented Feb 3, 2024

Fixes #8562

Add blade tree-sitter config.

It needs php_only, which is separate from the regular php grammar.

image

@lelgenio
Copy link
Contributor Author

lelgenio commented Feb 3, 2024

I can't figure out how to make blade be the default language for .blade.php files, instead of php 😕

languages.toml Outdated Show resolved Hide resolved
languages.toml Outdated Show resolved Hide resolved
languages.toml Outdated Show resolved Hide resolved
runtime/queries/php_only/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/php_only/highlights.scm Outdated Show resolved Hide resolved
@the-mikedavis the-mikedavis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-language-support Area: Support for programming/text languages labels Feb 3, 2024
@lelgenio lelgenio force-pushed the add-blade-support branch 3 times, most recently from 1f25d5f to d5f2f60 Compare February 3, 2024 23:23
languages.toml Outdated Show resolved Hide resolved
@Agilan989

This comment was marked as spam.

@lelgenio lelgenio force-pushed the add-blade-support branch 2 times, most recently from fce1987 to 0cbd556 Compare February 7, 2024 00:11
@the-mikedavis the-mikedavis added S-waiting-on-pr Status: This is waiting on another PR to be merged first and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 7, 2024
@Agilan989

This comment was marked as off-topic.

@lelgenio lelgenio marked this pull request as ready for review February 27, 2024 23:27
@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-pr Status: This is waiting on another PR to be merged first labels Feb 28, 2024
runtime/queries/php-only/injections.scm Outdated Show resolved Hide resolved
runtime/queries/blade/injections.scm Outdated Show resolved Hide resolved
@the-mikedavis the-mikedavis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from a maintainer. labels Feb 28, 2024
php-only is required enabling php injections like in blade templates
the-mikedavis
the-mikedavis previously approved these changes Feb 28, 2024
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.

Thanks!

@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 28, 2024
@inmanturbo
Copy link

Thanks!

languages.toml Show resolved Hide resolved
Copy link
Contributor

@EmranMR EmranMR Mar 7, 2024

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 the tree-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.

((text) @injection.content
    (#not-has-ancestor? @injection.content "envoy")
    (#set! injection.combined)
    (#set! injection.language php))

; could be bash or zsh
; or whatever tree-sitter grammar you have.
((text) @injection.content
    (#has-ancestor? @injection.content "envoy")
    (#set! injection.combined)
    (#set! injection.language bash))


((php_only) @injection.content
    (#set! injection.language php_only))
((parameter) @injection.content
    (#set! injection.language php_only))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, you're right! @tasks don't work correctly. I wonder if there's an equivalent predicated to #has-ancestor in helix 🤔

image

#9513 (comment)

Copy link
Contributor

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

#has-ancestor? / #not-has-ancestor? is a predicate that only exists in nvim as far as I can tell. For now we can remove that line and always inject php

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))

Copy link
Member

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#981

It 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

Copy link
Contributor

@EmranMR EmranMR Mar 7, 2024

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 in blade 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 parse shell correctly in the blade files due to their limitation/complexity. In all honesty, I was just trying to see how far I can push tree-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 both bash and php 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...

Copy link
Contributor Author

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.

@pascalkuthe pascalkuthe merged commit d99b617 into helix-editor:master Mar 17, 2024
6 checks passed
Desdaemon pushed a commit to Desdaemon/helix that referenced this pull request Mar 26, 2024
* Add php-only language config and queries

php-only is required enabling php injections like in blade templates

* Add blade templates support
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
* Add php-only language config and queries

php-only is required enabling php injections like in blade templates

* Add blade templates support
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
* Add php-only language config and queries

php-only is required enabling php injections like in blade templates

* Add blade templates support
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
* Add php-only language config and queries

php-only is required enabling php injections like in blade templates

* Add blade templates support
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 20, 2024
* Add php-only language config and queries

php-only is required enabling php injections like in blade templates

* Add blade templates support
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
* Add php-only language config and queries

php-only is required enabling php injections like in blade templates

* Add blade templates support
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
* Add php-only language config and queries

php-only is required enabling php injections like in blade templates

* Add blade templates support
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
* Add php-only language config and queries

php-only is required enabling php injections like in blade templates

* Add blade templates support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-support Area: Support for programming/text languages S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Laravel Blade support
6 participants