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

Use standard captures in injection queries #9654

Closed
wants to merge 1 commit into from

Conversation

michaelfortunato
Copy link

@michaelfortunato michaelfortunato commented Mar 21, 2024

Before this commit, zed was using captures and properties for injection queries that did not match the tree sitter documentation This was breaking the Java extension when it came to its injections.scm, and would likely break other tree-sitter extensions as authors are instructed in the tree-sitter docs to use the standard captures and properties for injection queries.

Changed

The following properties and captures used for injections have changed to be the standard ones. In particular

  • @language to @injection.language
  • @content to @injection.content
  • combined to injection.combined
  • language to injection.language

Testing

Unit tests have been updated with the standard captures as well. In particular, cd crates/language && cargo test --lib and cd crates/editor && cargo test --lib both have been updated and pass on my machine.

Release Notes:

  • Use standard captures in injection queries (#9656).

Copy link

cla-bot bot commented Mar 21, 2024

We require contributors to sign our Contributor License Agreement, and we don't have @michaelfortunato on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@michaelfortunato
Copy link
Author

@cla-bot check

Copy link

cla-bot bot commented Mar 21, 2024

We require contributors to sign our Contributor License Agreement, and we don't have @michaelfortunato on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

Copy link

cla-bot bot commented Mar 21, 2024

The cla-bot has been summoned, and re-checked this pull request!

@michaelfortunato
Copy link
Author

@cla-bot check

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Mar 21, 2024
Copy link

cla-bot bot commented Mar 21, 2024

The cla-bot has been summoned, and re-checked this pull request!

@maxdeviant maxdeviant changed the title fix(injection queries): Use standard captures Use standard captures in injection queries Mar 21, 2024
@maxbrunsfeld
Copy link
Collaborator

Yeah, good point @michaelfortunato - no reason to do it differently from the other tree-sitter stuff.

Before this commit, zed was using captures and predicates for injection queries
that did not match the tree sitter [documentation][1]
This was breaking the Java extension when it came to its injections.scm,
and would likely break other tree-sitter extensions as authors are instructed
in the tree-sitter docs to use the standard captures and predicates for
injection queries.

Changed
The following predicates and captures used for injections have changed
to be the standard ones. In particular

- @language to @injection.language
- @content to @injection.content
- combined to injection.combined
- language to injection.language

Testing
Unit tests have been updated with the standard captures as well. In
particular,  `cd crates/language && cargo test --lib` and
`cd crates/editor && cargo test --lib` both have been updated and pass
on my machine.

[1]: https://tree-sitter.github.io/tree-sitter/syntax-highlighting#language-injection
@michaelfortunato
Copy link
Author

Sounds good. I had to resolve a conflict, I think it'll be able to merge now with your approval @maxbrunsfeld. Also https://github.com/zed-extensions/svelte/blob/main/languages/svelte/injections.scm will have to be updated as well.

@michaelfortunato
Copy link
Author

Happy to rebase the PR against main if you still want this change. If not I will close the pull request. Thanks

@maxbrunsfeld
Copy link
Collaborator

@michaelfortunato Sorry for the delay. We now have extensions, so there are fewer languages to update in this repo, but we need to make the change in a backwards compatible way (accepting both capture names). If you are up for doing that backwards compatibility change, then we are still up for merging this.

@mikayla-maki
Copy link
Contributor

As there hasn't been movement on this PR in 3 weeks, I'm going to close it for now. We're still interested in supporting these captures, it should just be backwards compatible with our existing captures :)

github-merge-queue bot pushed a commit that referenced this pull request Jan 7, 2025
Closes #9656. Continuation of #9654, but with the addition of backwards
compatibility for the existing captures.

Release Notes:

- Improved Tree-sitter support with added compatibility for standard
injections captures

---------

Co-authored-by: Finn Evers <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants