sublimelsp / LSP-eslint

ESLint support for Sublime LSP plugin
MIT License
36 stars 5 forks source link

Update vscode-eslint to 2.2.6 #54

Closed alecmev closed 2 years ago

alecmev commented 2 years ago

Build using Webpack, so it's closer to upstream. Smoke-tested locally. I updated the build, but I know nothing about LSP, so I don't know if I need to touch anything else, and where to look for things to touch/add/remove. Please do jump in!

alecmev commented 2 years ago

Please don't merge yet, I'm in the process of switching from NpmClientHandler to GenericClientHandler, since node_modules isn't needed anymore, and the caching neither.

predragnikolic commented 2 years ago

The switching to GenericClientHandler can be a separate PR.🙂

alecmev commented 2 years ago

That works too! :wink:

rchl commented 2 years ago

Actually the best is the third option - just set skip_npm_install class property on NpmClientHandler - https://sublimelsp.github.io/lsp_utils/client_handlers.html#lsp_utils.NpmClientHandler.skip_npm_install

alecmev commented 2 years ago

Thought about that, but it will still look for package.json, which isn't needed anymore, and create a new cache entry for each Node version, if I understand it right. I aim to have just language-server.js in the root of the project and nothing else.

rchl commented 2 years ago

There is a purpose in having package.json present. That way the ST package knows when to update the source files that are stored in package storage directory (as those are not used directly from the compressed package files). Otherwise the eslint server source files would not get updated on updating the package. Or alternatively would update on every package update, even when not needed.

alecmev commented 2 years ago

I might be misunderstanding how LSP works! From what I can see, the server "binary" is shipped with this plugin, and it has no dependencies (Webpack bundles it all up), and this plugin is tightly-coupled to the server, so there's no reason to do anything if a different Node version is used, as long as it's supported, and there are no other variables. Or is there more to it?

Edit:

as those are not used directly from the compressed package files

Does this mean that the server has to reside outside of the package? I don't know much about how ST package distribution works, are they really zipped? On my system they're just folders.

Edit:

Okay, I found Installed Packages... Sorry, I understand now! skip_npm_install it is, not worth a refactor, thanks!

rchl commented 2 years ago

Yes, ST packages typically ship as zip files (with .sublime-package extension). This is something that package can opt-out of but we are not doing that most of the time. The server files need to be copied outside of the package so that the node process can access those.

alecmev commented 2 years ago

The reason I went down this path was to simplify server development, so I could ln -s the out directory into the project, but I'll just do that in the cache instead.

Thanks for the explanation!