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

feat: Support context recognition for injected languages #388

Merged
merged 7 commits into from
Feb 16, 2024

Conversation

kwaszczuk
Copy link
Contributor

Currently, the plugin only recognizes the outermost language of a file, disregarding any injected languages within the file. A notable example are .html files, where JavaScript content within <script> tags is ignored.

The root cause of this behavior lies in the fact that, during context evaluation for a line, only the root language tree is considered. Fortunately, the nvim-treesitter API provides a solution by allowing us to retrieve all language trees associated with a given range (LanguageTree:language_for_range({range})).

This PR proposes a simple yet effective change: iterating over parent nodes of each language tree detected for the range, as opposed to doing so only for the root language tree.

I believe, introduced functionality aims to enhance the plugin's behavior without overcomplicating the source code. It particularly improves compatibility with JavaScript frameworks like Svelte or Vue, where language injection is common, if not inevitable.

To illustrate the impact of this PR, below there is a comparison of the plugin's behavior on test/test.html file before (left side) and after introducing the changes (right side):

asciicast

@kwaszczuk kwaszczuk marked this pull request as ready for review February 5, 2024 22:41
@kwaszczuk
Copy link
Contributor Author

@lewis6991, any chance you could review this pull request anytime soon? I'd like to add Svelte support as well, but without this PR, it doesn't make much sense. Thanks!

@lewis6991
Copy link
Member

Can you rebase and add a functional test to test/ts_context_spec.lua?

@kwaszczuk
Copy link
Contributor Author

Done. I've added one test for the plain HTML without any injections, and two more for JavaScript within <script> tag.

lua/treesitter-context/context.lua Outdated Show resolved Hide resolved
---@param row integer
---@param col integer
---@return LanguageTree[]
local function get_parent_langtrees(bufnr, range)
Copy link
Member

Choose a reason for hiding this comment

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

Does this work properly for nested injections? it looks to me this will only return at most 2 langtrees. The root and the most inner.

Copy link
Contributor Author

@kwaszczuk kwaszczuk Feb 15, 2024

Choose a reason for hiding this comment

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

You are right, I misunderstood language_for_range implementation. I changed the code to manually traverse children langtrees, please take a look.

To test the nested injection, I came up with the markdown test case replacing HTML one. Context both for <html> and <script> displays properly when I run vim manually, but in tests injecting language in Markdown seems to be broken (look at test job for the reference). Do you have any idea what could be the problem here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lewis6991 It took me a while, but I finally get it to work.

Looks like tests are not performing full initialization of nvim-treesitter plugin, but just require'nvim-treesitter.configs'.setup { ... }. Because of that some modules are not initialized, including nvim-treesitter.query_predicates which contains set-lang-from-info-string! directive configuration required by Markdown queries.

My straightforward solution for that is to explicitly initialize the plugin via require'nvim-treesitter'.setup(). I don't think it is the greatest solution, as we end up invoking setup() twice (just on different modules), which seems weird, but I couldn't find any other way that would not induce setup duplication.

@lewis6991 lewis6991 merged commit 0a95d47 into nvim-treesitter:master Feb 16, 2024
2 checks passed
@lewis6991
Copy link
Member

Thanks. Let go with this for now. Can tidy up stuff later if we need to.

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.

2 participants