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

Bug in sqlx query highlighting for Rust #8608

Open
rmehri01 opened this issue Oct 24, 2023 · 5 comments
Open

Bug in sqlx query highlighting for Rust #8608

rmehri01 opened this issue Oct 24, 2023 · 5 comments
Labels
A-tree-sitter Area: Tree-sitter C-bug Category: This is a bug

Comments

@rmehri01
Copy link
Contributor

Summary

Based on a bisect from the 23.05 release, it seems like #7621 caused the highlighting of the sqlx query macros to stop working:

Before:

image

After:

image

I think this is because the injection query for normal macros overwrites the whole thing, when really we just want the nested query to be SQL and the rest should be Rust:

((macro_invocation
(token_tree) @injection.content)
(#set! injection.language "rust")
(#set! injection.include-children))

There's also a smaller regression that #6793 removed the highlighting for query_unchecked that #6256 introduced but that is fairly easy to fix :)

Reproduction Steps

I tried this:

  1. hx on a Rust file with any sqlx query, such as sqlx::query!("SELECT id FROM posts")

I expected this to happen:

The SQL string should be highlighted properly.

Instead, this happened:

It is just highlighted as a normal Rust string.

Helix log

N/A

Platform

Linux

Terminal Emulator

Kitty

Helix Version

23.05-498-g68c7537d

@rmehri01 rmehri01 added the C-bug Category: This is a bug label Oct 24, 2023
@the-mikedavis the-mikedavis added A-language-support Area: Support for programming/text languages A-tree-sitter Area: Tree-sitter and removed A-language-support Area: Support for programming/text languages labels Oct 24, 2023
@pascalkuthe
Copy link
Member

that is not really a regression but rather an existing bug (that could have lead to crashes before that fix) in the queries. The normal macro injection needs to ignore the sqlx macro (similar for other injections)

@rmehri01 rmehri01 changed the title Regressions in sqlx query highlighting for Rust Bug in sqlx query highlighting for Rust Oct 25, 2023
@rmehri01
Copy link
Contributor Author

Ah okay, sorry about that.

Is there an example of where something nested like this is done in another injection query? I see something kind of similar in vue where by default scripts are javascript and if there is lang= then it is that lang, but I don't really see one with one injection inside another?

To clarify, I think I can do something that gets the normal macro injection to not trigger so that the SQL works but then some other parts don't get properly highlighted like the comma since the macro body isn't Rust:

image

@pascalkuthe
Copy link
Member

pascalkuthe commented Oct 25, 2023

I would need to look qt the specifics but I think you should be able to only inject sql into the first string literal/argument and then injection rust into the rest of them.

It might be possible that the order of injections matters and you could get it to work just by changing the order of the existing injections now that I think about jt (potentially the first injection is preffered) but I am not sure if that works reliably ( a little fuzzy on how query order works there and I didn't intentionally implement any specific precendence)

@the-mikedavis
Copy link
Member

I think swapping the order doesn't fix the precedence here since the sqlx queries are a little more 'specific' (they need to match more of the tree than the general macro ones). I'm not sure if that's something we should change so that the more specific query gets precedence rather than the first one that matches successfully or if we should use negative matches in this case (so the general macro pattern would explicitly not match on sqlx macros)

@CedarMatt
Copy link

Is this still active? I found an interesting addition to the "not highlighting" part of this bug recently. It turns out if you remove the comma between the macro arguments in query_as! the sql highlighting works again.

I know its a syntax error but idk maybe it will help resolve the bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tree-sitter Area: Tree-sitter C-bug Category: This is a bug
Projects
None yet
Development

No branches or pull requests

4 participants