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

Use latest tree-sitter-bicep, support bicepparams #11525

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

heaths
Copy link
Contributor

@heaths heaths commented Aug 19, 2024

Updates to the latest tree-sitter-bicep and adds support for .bicepparam
files, which can replace .parameters.json files.

Updates to the latest tree-sitter-bicep and adds support for .bicepparam
files, which can replace .parameters.json files.
@heaths
Copy link
Contributor Author

heaths commented Aug 19, 2024

As recommended in Sjord/tree-sitter-bicep#2 since the current grammar is 3 years old and quite a bit behind. Mainly I wanted to add .bicepparam file support, which also required a couple new keywords.

Based on thread in #11412 I didn't install nixos on this machine so didn't run grammars.nix. I did try running hx -g fetch and hx -g build with and without setting HELIX_RUNTIME to the local repo directory but it didn't update.

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

It looks like the new grammar is basically a rewrite so the queries will need to be redone. The repository has these which we can start from: https://github.com/tree-sitter-grammars/tree-sitter-bicep/blob/master/queries/highlights.scm but need their captures to be adapted to the ones we use: https://docs.helix-editor.com/master/themes.html#syntax-highlighting

@the-mikedavis the-mikedavis added the A-language-support Area: Support for programming/text languages label Aug 19, 2024
@heaths
Copy link
Contributor Author

heaths commented Aug 19, 2024

To test this locally, is there a less costly way than installing nixOS to build the modified grammar? I remember from the previous PR I don't have to install the nixShell at least, but I still need to run grammar.nix? Also seems I could just run the same commands it ends up computing.

@the-mikedavis
Copy link
Member

You can manually test as long as you have a recent enough cargo/rust (I would recommend using rustup for that), git and a C/C++ compiler: cargo run will start up Helix in a debug build and the build process automatically takes care of fetching and building the grammars. You can also use cargo run -- -g fetch and cargo run -- -g build to do those steps manually. grammars.nix is to help with the Nix build: since Nix builds are sandboxed we need Nix to download and build the grammars instead of letting the build process do it automatically. If you aren't using Nix to install Helix then you shouldn't need to interact with any of the *.nix files

@heaths
Copy link
Contributor Author

heaths commented Aug 20, 2024

This may not be ready yet. The highlights should be right, but when I try to test with cargo run -- <file> nothing is highlighted - not even for other grammars. Since the output of git rev-parse --git-dir is watched for changes and cargo rebuilds, it should rebuild the grammars but it's definitely too fast. I'm on macOS Senoma and clang 15 is installed. No errors when building.

I'm working on a Dockerfile based on nixOS so maybe that will help.

@heaths
Copy link
Contributor Author

heaths commented Aug 20, 2024

After installing nixOS and starting with nix-shell --extra-experimental-features flakes --extra-experimental-features nix-command shell.nix, I was able to successfully test both .bicep and .bicepparam files. I notied when nix-shell was initializing from the declared packages that it was downloading all the grammars (sans a few it was erring on), unlike either cargo build (and, yes, my toolset it up to date at 1.80 but your rust-toolchain.toml pins it to 1.74 anyway, which was also installed) or even HELIX_RUNTIME=$PWD/runtime cargo run -- -g fetch where my working directory is the repo.

So this appears ready now.

@heaths
Copy link
Contributor Author

heaths commented Aug 22, 2024

Since the new grammar handles more keywords, would you like me to add more to the corpus as well? There's no regressions, at least.

@the-mikedavis
Copy link
Member

Yeah if the grammar handles new keywords let's add them to the highlights. Otherwise the highlight queries are looking good now 👍

@heaths
Copy link
Contributor Author

heaths commented Sep 5, 2024

Never mind. I was incorrectly remembering the corpus test data was in this repo - not the tree-sitter repos. This PR should be good to go, then. Unless you wanted to wait and see if tree-sitter-grammars/tree-sitter-bicep#32 is merged and sync to that commit.

the-mikedavis
the-mikedavis previously approved these changes Sep 6, 2024
@the-mikedavis the-mikedavis merged commit 843c058 into helix-editor:master Nov 20, 2024
6 checks passed
@heaths heaths deleted the bicepparam branch November 20, 2024 23:05
GladkihEgor pushed a commit to GladkihEgor/helix that referenced this pull request Jan 4, 2025
Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
diucicd pushed a commit to diucicd/helix that referenced this pull request Jan 8, 2025
Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-support Area: Support for programming/text languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants