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

feat: add support for gjs and gts #9940

Merged

Conversation

c0rydoras
Copy link
Contributor

Add support for glimmer-js and glimmer-ts

gjs

image
image
image

gts

(error due to using linguist sample and missing imports)
image
image
image

runtime/queries/_gjs/highlights.scm Show resolved Hide resolved
Comment on lines +16 to +20
(call_expression
function: ((identifier) @_name
(#eq? @_name "hbs"))
arguments: ((template_string) @glimmer
(#offset! @glimmer 0 1 0 -1)))
Copy link
Member

Choose a reason for hiding this comment

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

We don't support #offset!. This will also need to be updated to use injection.content and injection.language rather than tagging with @glimmer - that's the way nvim-treesitter did injections in the past.

Something like

(call_expression
  function: ((identifier) @_name
             (#eq? @_name "hbs"))
  arguments: ((template_string) @injection.content
              (#set! injection.language "glimmer")))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can probably just be dropped due to RFC 0779

As of [RFC 0779][rfc-0779], we decided on <template> over hbs; see the RFC for the full rationale. The hbs format is still technically supported by this repo for transition purposes for the early adopters who helped us get here, but will be removed at some point in the near future! hbs has been removed -- if you rely on this feature, please use ember-template-imports @ < v4, until migrated to <template>

If anywhere this probably belongs into the javascript/typescript injections instead of gjs

Copy link
Member

Choose a reason for hiding this comment

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

Yeah let's drop this pattern. It won't do anything in its current form anyways because of the @glimmer capture

@the-mikedavis the-mikedavis 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 Mar 20, 2024
the-mikedavis
the-mikedavis previously approved these changes Mar 20, 2024
@the-mikedavis the-mikedavis added 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 Mar 20, 2024
@c0rydoras c0rydoras force-pushed the feat/add-gjs-and-gts-support branch from 9152270 to cc0db28 Compare March 25, 2024 14:17
@c0rydoras c0rydoras force-pushed the feat/add-gjs-and-gts-support branch 2 times, most recently from 3ed6534 to 2c53bd6 Compare April 8, 2024 07:53
@c0rydoras c0rydoras force-pushed the feat/add-gjs-and-gts-support branch from 2c53bd6 to 8623c83 Compare April 11, 2024 07:30
@c0rydoras c0rydoras force-pushed the feat/add-gjs-and-gts-support branch 2 times, most recently from 0b225a5 to ef17c6d Compare April 25, 2024 08:57
@c0rydoras c0rydoras force-pushed the feat/add-gjs-and-gts-support branch from ef17c6d to 50ca271 Compare April 29, 2024 09:49
@David-Else
Copy link
Contributor

David-Else commented Apr 29, 2024

It is great we are finally getting a config for [language-server.vscode-eslint-language-server]! I have not used eslint for ages, but I am interested in making sure Helix support is as generic and unopinionated as possible. I created a discussion about it: #8452

You config seems cool, my only question is should we use experimental = { useFlatConfig = false }?

https://eslint.org/blog/2022/08/new-config-system-part-2/
https://stackoverflow.com/questions/78330131/how-do-i-configure-eslint-flat-config-with-typescript-in-vscode-without-flatco

Is FlatConfig not the current standard? Typescript ESlint seems to use it and describes the old one as legacy: https://typescript-eslint.io/getting-started/

Forgive my ignorance on this topic, I thought I would bring it up but don't really have any answers, only questions. Perhaps @Philipp-M could comment?

@c0rydoras
Copy link
Contributor Author

AFAIK eslint is almost always used within ember.js apps/addons

Ember ships with eslint per default: app blueprint

Using eslint-plugin-ember is the only way i found to get useful diagnostics

For reference, using typescript-language-server looks like this:
image

eslint-language-server itself isn't really that useful for things other than diagnostics and formatting so i still included typescript-language-server for that and omitted the diagnostics. This definitely isn't ideal but the best solution I found.

I just contributed my local config upstream, therefore theres things such as

[language-server.eslint.config]
experimental = { useFlatConfig = false }

If there is something like "per language" LSP configurations it would probably make sense to not configure this for other languages.

@David-Else
Copy link
Contributor

If there is something like "per language" LSP configurations it would probably make sense to not configure this for other languages.

I think each LSP config is universal in Helix.

We should probably use the equivalent of the Neovim config for eslint: https://github.com/neovim/nvim-lspconfig/blob/master/doc/server_configurations.md#eslint . It looks almost the same as the one in this PR, they also have experimental = { useFlatConfig = false }. Regarding the following from Neovim:

{
  codeActionOnSave = {
    enable = false,
    mode = "all"
}, 

My understanding is that codeActionOnSave does not work in Helix at this point, but do we want to explicitly turn it off like Neovim?

@c0rydoras c0rydoras force-pushed the feat/add-gjs-and-gts-support branch from 50ca271 to e6ad12f Compare April 30, 2024 11:23
@c0rydoras c0rydoras force-pushed the feat/add-gjs-and-gts-support branch from e6ad12f to aed7c81 Compare May 3, 2024 18:37
@pascalkuthe pascalkuthe merged commit 295a9a9 into helix-editor:master May 6, 2024
6 checks passed
@c0rydoras c0rydoras deleted the feat/add-gjs-and-gts-support branch May 19, 2024 20:38
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
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 S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants