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

Modify env language to extend bash #5720

Conversation

EricCrosson
Copy link
Contributor

@EricCrosson EricCrosson commented Jan 29, 2023

Additionally, add .envrc to the env-supported file types.

@EricCrosson EricCrosson marked this pull request as ready for review January 29, 2023 16:50
@archseer
Copy link
Member

archseer commented Jan 29, 2023

(Sorry, I posted my comment on the wrong PR)

@pascalkuthe pascalkuthe added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-language-support Area: Support for programming/text languages labels Jan 29, 2023
@archseer
Copy link
Member

What about .env? Would cover .env.staging etc

@pascalkuthe pascalkuthe removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 29, 2023
@EricCrosson
Copy link
Contributor Author

I'm not sure @archseer, .env is listed as a file-type under the env language here

I sniped .envrc because it covers the majority of direnv use-cases and didn't conflict with any other settings, as far as I can tell

@archseer
Copy link
Member

.env has the same usecase as .envrc. https://direnv.net/man/direnv.toml.1.html#codeloaddotenvcode

Both of these should be under the same filetype. We could use the bash grammar for all env types and drop the tree-sitter-env grammar (I'd still use a distinct language definition though).

@EricCrosson
Copy link
Contributor Author

I'm not very familiar with this code -- after reading https://docs.helix-editor.com/master/languages.html I'm still not quite sure what changes you're suggesting.

If I delete this line and add .env to bash.file-types, is that what you're suggesting?

@the-mikedavis
Copy link
Member

You can remove this block

helix/languages.toml

Lines 2016 to 2018 in 482cc22

[[grammar]]
name = "env"
source = { git = "https://github.com/seshotake/tree-sitter-env", rev = "e6c6bb1e7b51d481cba463fe949f083cf22d81f7" }

and add a line grammar = "bash" to the [[language]] entry for env. Then you could change the runtime/queries/env/*.scm to only say ; inherits: bash. This will make env a separate language that re-uses the bash tree-sitter parser and highlights.

Additionally, add `.envrc` to the `env`-supported file types.
@EricCrosson EricCrosson force-pushed the use-bash-language-for-envrc-files branch from f7ab450 to 3e670ec Compare January 29, 2023 23:59
@EricCrosson EricCrosson changed the title Use bash language for envrc files Modify env language to extend bash Jan 29, 2023
@EricCrosson
Copy link
Contributor Author

Thanks for the assist @the-mikedavis 🤠

I followed your instructions and moved .envrc from the bash file-types to the env file-types. How does it look now?

@pascalkuthe pascalkuthe added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 30, 2023
@pascalkuthe
Copy link
Member

This LGTM, I think the testsuite failure is just a GA problem and unrelated to the actual PR. I can't rerun workflows but I am pretty sure that a rerunning CI would fix the pipeline

@pascalkuthe pascalkuthe added the E-easy Call for participation: Experience needed to fix: Easy / not much label Jan 30, 2023
@the-mikedavis the-mikedavis merged commit 447909e into helix-editor:master Jan 30, 2023
@EricCrosson EricCrosson deleted the use-bash-language-for-envrc-files branch January 31, 2023 00:47
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 E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants