dense-analysis / ale

Check syntax in Vim/Neovim asynchronously and fix files, with Language Server Protocol (LSP) support
BSD 2-Clause "Simplified" License
13.51k stars 1.43k forks source link

Add support for gjs template linting using embertemplate lint #4653

Closed SamSaffron closed 7 months ago

SamSaffron commented 10 months ago

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

w0rp commented 10 months ago

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 commented 10 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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:

SamSaffron commented 9 months ago

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 commented 9 months ago

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

image

SamSaffron commented 8 months ago

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

SamSaffron commented 7 months ago

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

hsanson commented 7 months ago

@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 commented 7 months ago

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

passing on local now @w0rp

SamSaffron commented 7 months ago

thanks heaps for merging @hsanson