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 rust injections for c, c++, html, js, py #1526

Closed
wants to merge 1 commit into from

Conversation

pickfire
Copy link
Contributor

@pickfire pickfire commented Jan 16, 2022

cc @the-mikedavis

Example

use inline_python::python;

fn main() {
    let who = "world";
    let n = 5;
    python! {
        import csv  # first import highlight won't work, not sure why
        for i in range('n):
            print(i, "Hello", 'who)
        print("Goodbye")
    }
}

Comment on lines +32 to +43
((macro_invocation
macro: (scoped_identifier
name: (identifier) @_python (#eq? @_python "python"))
(token_tree) @injection.content)
(#set! injection.language "python")
(#set! injection.include-children))

((macro_invocation
macro: (identifier) @_python (#eq? @_python "python")
(token_tree) @injection.content)
(#set! injection.language "python")
(#set! injection.include-children))
Copy link
Contributor Author

@pickfire pickfire Jan 16, 2022

Choose a reason for hiding this comment

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

Not sure how can I simplify this so I leave it like this for now.

One is inline_python::python! and one is python!. I think this is probably applicable for others.

@the-mikedavis
Copy link
Member

I'm not totally sure what's happening here with the injections within (token_tree). The results are same across helix master, incremental, and tree-sitter-cli: the injection works on individual identifiers under the token_tree, but any python that uses multiple identifiers isn't parsed correctly. For example, in the string of identifiers making up the comment after the import, the "#" is highlighted as a comment but the subsequent nodes are not highlighted as comments. Based on the range of the outermost (token_tree) and the description of injection.include-children in the tree-sitter docs

  • injection.include-children - indicates that the @injection.content node's entire text should be re-parsed, including the text of its child nodes. By default, child nodes' text will be excluded from the injected document.

I would expect that the contents of (token_tree) shouldn't matter, as the entire text of that node should be re-parsed.

I think what's happening here is that some of the rules from the rust grammar are excluding nodes from the token tree that are important to be parsed for python's sake. For example, it looks like the # token is not present under (token_tree).

@archseer do you have thoughts about the behavior here?

@archseer
Copy link
Member

I find most of these a bit niche and not sure if we want these in by default. i.e. https://github.com/search?l=Rust&q=inline_python&type=Code is barely used

@pickfire
Copy link
Contributor Author

I find most of these a bit niche and not sure if we want these in by default. i.e. https://github.com/search?l=Rust&q=inline_python&type=Code is barely used

Maybe have them commented out by default? But I doubt anyone using it wants to dig into the highlight to enable support for it. Will enabling it slows down the tree-sitter highlighting?

@the-mikedavis the-mikedavis added the S-needs-discussion Status: Needs discussion or design. label May 18, 2022
@the-mikedavis
Copy link
Member

the-mikedavis commented Aug 6, 2022

The injections for python in rust may work better with the new (#set injection.include-unnamed-children true) rule from #3129

I haven't tried it out though.

@kirawi kirawi added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Sep 13, 2022
@pickfire
Copy link
Contributor Author

pickfire commented Sep 28, 2022

@the-mikedavis Do you think I should continue to work on this or just close it given that these are rarely used?

@the-mikedavis
Copy link
Member

IMO it's more work than it's worth to make these queries look good but I'm biased since I haven't used Rust in a cross-language way like this before :P

@pickfire
Copy link
Contributor Author

pickfire commented Sep 28, 2022

I only used it a couple of times only and I don't need it anymore so I guess I am just going to close this. Or maybe we can add it commented out.

@pickfire pickfire closed this Sep 28, 2022
@pickfire pickfire deleted the rust-injections branch September 28, 2022 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. S-needs-discussion Status: Needs discussion or design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants