search

bmatcuk / stylelint-lsp

A stylelint Language Server
MIT License
46 stars 4 forks source link

stylelint.lint is run despite stylelint not being configured #19

Closed Gelio closed 3 years ago

Gelio commented 3 years ago

I have stylelint-lsp enabled and running inside neovim (using neovim LSP) whenever working on CSS, JS, TS files. The stylelint-lsp is enabled even if the project does not have stylelint configured. This introduces unnecessary work and errors in my lsp.log, because when running the validate function (which calls stylelint.lint in the end):

https://github.com/Gelio/stylelint-lsp/blob/bd989a5ae55d72701531bed1908c6546df3aa203/src/validate.ts#L192

possible errors are not caught. I have raised a PR (https://github.com/bmatcuk/stylelint-lsp/pull/18) that adds error handling, but we could do better and don't run stylelint's lint function if stylelint is not configured.

One idea I had would be to catch the error that lint rejects with, and if it's an error about the configuration being missing, pause any validate requests until the config file watcher notifies that the config has changed:

https://github.com/Gelio/stylelint-lsp/blob/bd989a5ae55d72701531bed1908c6546df3aa203/src/validate.ts#L274-L282

I could try to help out and implement it, if you'd like :slightly_smiling_face:

bmatcuk commented 3 years ago

Hmm, that's not a bad plan, but there is a little complication: it's possible that an appropriate config may be missing for one file, but exist for another. So, it needs to pause validations on a per-file basis.

I've got an idea - I'll find some time to give it a try in the next couple days.

Gelio commented 3 years ago

Valid point, makes sense šŸ‘ Sure thing, thanks for putting the effort

bmatcuk commented 3 years ago

Hey @Gelio, I just released v1.2.3 that should solve this issue, and #17. Let me know if it works for you!

Gelio commented 3 years ago

Great news, thanks! I'm taking time off till the end of the week, so I'll try it out next weekend, hopefully. Thanks again! I appreciate it šŸ‘

Gelio commented 3 years ago

I gave v1.2.3 a try and #17 is fixed, but we seem to have regressed with https://github.com/bmatcuk/stylelint-lsp/pull/18 šŸ˜„ I inspected the problem since I figured it may be easier for me with neovim LSP already configured, and raised #21 with the fix šŸ™‚

After the problem mentioned in #21 is solved, the overall fix for this issue is working great - the errors about the configuration not being provided are not repeated anymore šŸ™‚ Thanks!

Gelio commented 3 years ago

I confirm that the problem is solved in v1.2.4 šŸŽ‰