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

offset directive in injection queries is not respected #37

Closed
jmbuhr opened this issue May 29, 2023 · 7 comments · Fixed by #39
Closed

offset directive in injection queries is not respected #37

jmbuhr opened this issue May 29, 2023 · 7 comments · Fixed by #39
Labels
bug Something isn't working enhancement New feature or request

Comments

@jmbuhr
Copy link
Owner

jmbuhr commented May 29, 2023

@jmbuhr jmbuhr added the enhancement New feature or request label May 29, 2023
@wouthoekstra
Copy link

Treesitter supports syntax highlighting for nested languages by adding a query to your nvim configuration, by putting it in a queries/LANGUAGE/injections.scm file. Would it be possible to use these same queries?

For example; I made a query to get syntax highlighting for SQL in my Javascript files by creating the following query in ~/.config/nvim/queries/javascript/injections.scm. Because the output variable is '@Sql', treesitter recognizes it as sequel and highlights it as such:

(call_expression
  (member_expression
    object: (identifier) @_object (#eq? @_object "knex")
    property: (property_identifier) @_property (#eq? @_property "raw")
  )

  (arguments
    (template_string) @sql
    (#offset! @sql 0 1 0 -1)
  )
)

@jmbuhr
Copy link
Owner Author

jmbuhr commented May 30, 2023

Indeed, my dream-API would be to get the ranges of injections directly from the treesitter module in neovim. Upon closer searching, I think this is possible.
https://github.com/neovim/neovim/blob/58618d208acd3827c4e86668529edb619bb9b8dd/runtime/lua/vim/treesitter/query.lua#L210 can get the query for all injections and I believe this includes user-defined ones.
(Or rather the cached version: https://github.com/nvim-treesitter/nvim-treesitter/blob/23dfae4db84f71e2ddb56c024845b322035182ee/lua/nvim-treesitter/query.lua#LL106C1-L106C1 , which should also make it faster!)

And from this we should be able to get those captures:
https://github.com/neovim/neovim/blob/58618d208acd3827c4e86668529edb619bb9b8dd/runtime/doc/treesitter.txt#L484-L505

@jmbuhr jmbuhr mentioned this issue Jun 1, 2023
@jmbuhr jmbuhr closed this as completed in #39 Jun 3, 2023
@wouthoekstra
Copy link

wouthoekstra commented Jun 6, 2023

Wow this is great! For me it still does not properly work however, and that might be out of the scope of your package. But the used query injection does not seem to acknowledge the offset I've defined for querying my sql. Which means the backticks in my code are incorrectly parsed as part of the SQL:

For example, the query in my previous comment finds the SQL inside the knex.raw(), but it trips over the backticks:

    const plan = await knex.raw(`
      SELECT plan.name
        FROM subscription
        JOIN plan ON subscription.plan_id = plan.id
       WHERE account_id = :accountId
   `, { accountId: user.accountId }).then(firstOrNull);

This might be more an issue with treesitter, but I am not sure.

jmbuhr added a commit that referenced this issue Jun 6, 2023
@jmbuhr
Copy link
Owner Author

jmbuhr commented Jun 6, 2023

Looks like the offset directive only affects the range reported by the nodes metadata, not the text returned by get_node_text as currently used (

text = ts.get_node_text(node, main_nr, metadata)
).
However, I can't get treesitter to give me the correct range with directives applied, which supposedly was supposed to happen with the way I do it in the commit mentioned above. Any idea?

@jmbuhr jmbuhr reopened this Jun 6, 2023
@jmbuhr jmbuhr changed the title feat: allow different ways of querying code content offset directive in injection queries is not respected Jun 6, 2023
@jmbuhr jmbuhr added the bug Something isn't working label Jun 6, 2023
@wouthoekstra
Copy link

If I read this correctly, someone proposed a change to treesitter to update the injection with the offset: https://github.com/nvim-treesitter/nvim-treesitter/pull/1240/files#diff-81f58ecd0f4978b575136c86619adc8dbc9a0c9e87f87702ddf22615d0ac6c4bR93-R102. But it is already two years old.

@jmbuhr
Copy link
Owner Author

jmbuhr commented Aug 26, 2023

I might be able to revisit this with the upstream changes to injections

@jmbuhr
Copy link
Owner Author

jmbuhr commented Dec 2, 2023

Workaround is implemented via #73. The underlying issue remains, but the most common use cases should be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants