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 support for gjs template linting using embertemplate lint #4653

Merged
merged 3 commits into from
Feb 23, 2024

Conversation

SamSaffron
Copy link
Contributor

No description provided.

@SamSaffron
Copy link
Contributor Author

feel free to close this PR, it is just a proof of concept.

@w0rp
Copy link
Member

w0rp commented Nov 19, 2023

feel free to close this PR, it is just a proof of concept.

You might not be far off a solution. Is there any easy way to detect if a file is an embertemplatelint file? Do other plugins fix the filetypes so the templates have a filetype like foo.js. etc.?

@SamSaffron
Copy link
Contributor Author

I think @chancancode can help here, I think the easy way is only to look at *.gjs files, but from what I can tell I can't put a linter in the javascript.glimmer directory, ALE treats javascript.glimmer and javascript the same?

@w0rp
Copy link
Member

w0rp commented Dec 10, 2023

I think @chancancode can help here, I think the easy way is only to look at *.gjs files, but from what I can tell I can't put a linter in the javascript.glimmer directory, ALE treats javascript.glimmer and javascript the same?

If the files will always have that file extension, you can just check if the file contains the file extension. We should only need to do this if filetype detection isn't reliable already. If the Vim filtetype is javascript.glimmer consistently, you can move the linter to a glimmer directory. ALE finds linters for all filetypes split on ..

@SamSaffron
Copy link
Contributor Author

Interesting... any ideas on how to reuse the implementation? eg have 1 linter rule across both javascript.glimmer and html?

also going to ping @NullVoxPopuli who use Vim in case he has ideas on how to sort this out?

@NullVoxPopuli
Copy link

also going to ping @NullVoxPopuli who use Vim in case he has ideas on how to sort this out?

I've never tried to get template-lint going via ale.
I currently use:

also moves handler to the glimmer directory so it only fires
for gjs files
@SamSaffron SamSaffron changed the title super hacky way to get ember template lint to work on gjs files Add support for gjs template linting using embertemplate lint Jan 4, 2024
@SamSaffron
Copy link
Contributor Author

Hi @w0rp this should be good to merge now, I tested and it is working correctly.

Extra linting only happens on gjs files (which are classified as glimmer).

Since hbs files can not be auto detected as glimmer, we needed a handler to reuse code.

@SamSaffron
Copy link
Contributor Author

FYI this is how it works on hbs / gjs now (new is gjs support which was missing)

image

@SamSaffron
Copy link
Contributor Author

@w0rp sorry for nagging, I get holidays/life get in the way, have you had a chance to have a look?

@SamSaffron
Copy link
Contributor Author

@w0rp I don't think this CI failure is related, PR is working well and us tidy now

@hsanson
Copy link
Contributor

hsanson commented Feb 22, 2024

@SamSaffron the tests fail because this PR changes the name of some functions, specifically ale_linters#handlebars#embertemplatelint#Handle:

 Starting Vader: C:\testplugin\test\handler\test_embertemplatelint_handler.vader
    (1/3) [EXECUTE] The ember-template-lint handler should parse lines correctly
    (1/3) [EXECUTE] (X) Vim(call):E117: Unknown function: ale_linters#handlebars#embertemplatelint#Handle
      > C:\Users\appveyor\AppData\Local\Temp\1\VIA5EBE.tmp, line 39
    (2/3) [EXECUTE] The ember-template-lint handler should handle template parsing error correctly
    (2/3) [EXECUTE] (X) Vim(call):E117: Unknown function: ale_linters#handlebars#embertemplatelint#Handle
      > C:\Users\appveyor\AppData\Local\Temp\1\VID5ED1.tmp, line 24
    (3/3) [EXECUTE] The ember-template-lint handler should handle no lint errors/warnings
    (3/3) [EXECUTE] (X) Vim(call):E117: Unknown function: ale_linters#handlebars#embertemplatelint#Handle
      > C:\Users\appveyor\AppData\Local\Temp\1\VIG5EE3.tmp, line 3
  Success/Total: 0/3
  Starting Vader: C:\testplugin\test\handler\test_erblint_handler.vader

To fix this you need to update the corresponding test file test_embertemplatelint_handler.vader to use the new function names.

@SamSaffron
Copy link
Contributor Author

I see thanks @hsanson , no idea what I am doing, but I think I got this sorted :)

passing on local now @w0rp

Copy link
Contributor

@hsanson hsanson left a comment

Choose a reason for hiding this comment

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

Looking good, thanks.

@hsanson hsanson merged commit 5e8904c into dense-analysis:master Feb 23, 2024
7 checks passed
@SamSaffron
Copy link
Contributor Author

thanks heaps for merging @hsanson

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

Successfully merging this pull request may close these issues.

None yet

4 participants