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 Helm extension #443

Closed
wants to merge 12 commits into from
Closed

Conversation

cabrinha
Copy link
Contributor

@cabrinha cabrinha commented Apr 4, 2024

Support for https://helm.sh

@cla-bot cla-bot bot added the cla-signed label Apr 4, 2024
@maxdeviant maxdeviant changed the title Helm: Adding new extension Add Helm extension Apr 7, 2024
@maxdeviant
Copy link
Member

It looks like there are still some problems when trying to compile the go-template grammar to Wasm:

[2024-04-07T12:09:22Z INFO  extension::extension_builder] compiling go-template parser
Error: failed to compile extension

Caused by:
    0: failed to compile grammar 'go-template'
    1: failed to compile go-template parser with clang: wasm-ld: warning: creating shared libraries, with -shared, is not yet stable
       wasm-ld: error: symbol exported via --export not found: tree_sitter_go-template
       clang: error: linker command failed with exit code 1 (use -v to see invocation)

@maxbrunsfeld
Copy link
Contributor

wasm-ld: error: symbol exported via --export not found: tree_sitter_go-template

The function is probably called go_template (with an underscore, not a dash), so that needs to the name of the grammar. @cabrinha I think that means we need to change the directory name from grammars/go-template to grammars/go_template.

Also, since this is a new extension, it'd be good to be using the latest extension schema and API. Could you copy the structure of the extensions in here? https://github.com/zed-industries/zed/tree/main/extensions

@cabrinha
Copy link
Contributor Author

cabrinha commented Apr 9, 2024

wasm-ld: error: symbol exported via --export not found: tree_sitter_go-template

The function is probably called go_template (with an underscore, not a dash), so that needs to the name of the grammar. @cabrinha I think that means we need to change the directory name from grammars/go-template to grammars/go_template.

Alright, I think I can do that by forking the other repo...

Also, since this is a new extension, it'd be good to be using the latest extension schema and API. Could you copy the structure of the extensions in here? https://github.com/zed-industries/zed/tree/main/extensions

Do I need to create a helm.rs file inside of a src directory in my own helm.zed repo? I'm not sure what to populate helm.rs file with.

@maxbrunsfeld
Copy link
Contributor

That would only be needed if there is a Helm language server.

@cabrinha
Copy link
Contributor Author

cabrinha commented Apr 9, 2024

That would only be needed if there is a Helm language server.

Ok, here are the changes I've made to the extension: https://github.com/hjoshi123/helm.zed/pull/1/files

@cabrinha
Copy link
Contributor Author

cabrinha commented Apr 9, 2024

Thanks for the help so far, let me know if there are any other changes I need to make to get this working.

@maxbrunsfeld
Copy link
Contributor

The Cargo.toml is only needed if you're providing Rust code to configure a language server.

@cabrinha
Copy link
Contributor Author

cabrinha commented Apr 9, 2024

The Cargo.toml is only needed if you're providing Rust code to configure a language server.

Ok, it has been removed.

@cabrinha
Copy link
Contributor Author

cabrinha commented Apr 9, 2024

Updated the submodule and confirmed it uses underscores now.

CI keeps failing, weird.

@maxbrunsfeld
Copy link
Contributor

It looks like the grammar actually is called gotmpl (not go_template): https://github.com/ngalaiko/tree-sitter-go-template/blob/master/grammar.js#L3

@cabrinha
Copy link
Contributor Author

It looks like the grammar actually is called gotmpl (not go_template): https://github.com/ngalaiko/tree-sitter-go-template/blob/master/grammar.js#L3

Looks like it's now failing on: https://github.com/cabrinha/helm.zed/blob/master/languages/gotmpl/injections.scm#L1

@maxbrunsfeld
Copy link
Contributor

Yeah, apparently yaml_command is not a node in the grammar. Where did that injection query come from?

@cabrinha
Copy link
Contributor Author

Not sure, but here is the line: https://github.com/cabrinha/helm.zed/blob/master/languages/gotmpl/injections.scm#L1

Basically, Helm syntax is a combination of yaml and go-template, there is a tree sitter for both, but I don't know the proper way to combine them. I forked this repo: https://github.com/hjoshi123/helm.zed

If there is anything I can do to fix this issue, just let me know.

@cabrinha
Copy link
Contributor Author

cabrinha commented May 15, 2024

Yeah, apparently yaml_command is not a node in the grammar. Where did that injection query come from?

Interesting note in the README from the go-template tree-sitter: Note that yaml is listed under used_by. I've set this to highlight helm templates as Go Templates instead of yaml. To ensure that yaml highlighting is still working, you should set up language injection for gotmpl filetypes.: https://github.com/ngalaiko/tree-sitter-go-template/blob/master/README.md

Does this mean I should change yaml_command to yaml?

I've changed yaml_command to text, so I hope thats the right thing here: https://github.com/ngalaiko/tree-sitter-go-template/blob/master/dialects/helm/src/grammar.json#L24

@cabrinha
Copy link
Contributor Author

Now, I'm wondering if this approach is correct to use the gotmpl grammar, since tree-sitter-go-template has it's own dialect and grammar for helm here: https://github.com/ngalaiko/tree-sitter-go-template/blob/master/dialects/helm/src/grammar.json

@maxbrunsfeld do you think we're still on the right track here?

@mikayla-maki
Copy link

mikayla-maki commented May 16, 2024

Discussed in discord, author has decided to discontinue this effort, that said extension is very close, just needs to improve the association between heml and yaml. If someone else is interested in picking it up and finishing this, we'd be happy to have it :)

@qvalentin
Copy link

That would only be necessary if there was a Helm language server.

Well, there is https://github.com/mrjosh/helm-ls 😄

Perhaps I can provide some information on how helm is currently handled in Neovim:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants