Update to the latest vscode-css-languageserver - Githubissues

sublimelsp / LSP-css

CSS, SCSS, LESS support for Sublime's LSP plugin

MIT License

37 stars 0 forks source link

Update to the latest vscode-css-languageserver #13

Closed jfcherng closed 4 years ago

jfcherng commented 4 years ago

The server executable has been changed from out/cssServerMain.ts to out/node/cssServerMain.ts in the upstream about 16 days ago.

Other than that, I would also like to introduce another change to this (and other NPM-based LSP-* packages). That is, making this plugin less dependent on the server name (i.e., vscode-css, vscode-html etc...).

Change vscode-css/, vscode-html/ etc... into just language-server/
Change compile-vscode-css-languageserver.sh into just compile-language-server.sh
Mention which language server is used in the README

I think these changes should make it less work to create yet another NPM-based plugin by copy-paste. But I am free to revert this change, the major goal of this PR is to fix the changed upstream server path.

rchl commented 4 years ago

Other than that, I would also like to introduce another change to this (and other NPM-based LSP-* packages). That is, making this plugin less dependent on the server name (i.e., vscode-css, vscode-html etc...).

That sounds good to me.

rchl commented 4 years ago

I kinda would wish to know from which to which commit we are updating now so that I could look through all the commits and check for any changes / potential new settings....

And I'm not saying you should tell me but I'm thinking about a potential solution for that like automatically storing commit hash on updating. Put I think that currently, we don't have that info (when pulling master).

jfcherng commented 4 years ago

I think the build script should also clean up after itself - leave only the compiled version.

Not sure what this means. Delete the downloaded file/dir?

rchl commented 4 years ago

I think the build script should also clean up after itself - leave only the compiled version.

Not sure what this means. Delete the downloaded file/dir?

Yes, that's what I'm thinking.

jfcherng commented 4 years ago

I kinda would wish to know from which to which commit we are updating now so that I could look through all the commits and check for any changes / potential new settings....

And I'm not saying you should tell me but I'm thinking about a potential solution for that like automatically storing commit hash on updating. Put I think that currently, we don't have that info (when pulling master).

The simplest solution maybe move from "download the zipped tarball" to "clone the whole repo and then checkout" (and we don't have to clean up the downloaded repo, which can be reused next time). What do you think?

rchl commented 4 years ago

The simplest solution maybe move from "download the zipped tarball" to "clone the whole repo and then checkout" (and we don't have to clean up the downloaded repo, which can be reused next time). What do you think?

I'm fine with using git but I'd still want to clean up I think. Otherwise, all those files are copied to the ST cache (when developing) and I'd like to avoid that.

jfcherng commented 4 years ago

@rchl Luckily, the commit hash is in the comment of the downloaded .zip file (but not in .tar.gz) and unzip -z file.zip outputs it. See https://github.com/sublimelsp/LSP-css/commit/e71c3ffbab09f5223b8cceb96c3caeebbe1b70cf#diff-f12eb39acb360e835c9399c7b0fbda8aR26-R39

rchl commented 4 years ago

If you are gonna be merging this, please change the summary to say that it's about updating the server to the latest version as that is really the main reason for path "fix".

predragnikolic commented 4 years ago

I tested this PR on Linux, and I got an error:

lsp-css: internal/modules/cjs/loader.js:960
lsp-css:   throw err;
lsp-css:   ^

I will investigate further.

predragnikolic commented 4 years ago

Hmm, I deleted the cache LSP-css folder, and now it works. Edit: It was my mistake that LSP-css didn't initialize.

rwols commented 4 years ago

I also noticed this loader error when trying to update LSP-eslint. IMO it should be investigated because I feel like the upgrade mechanism is broken.

predragnikolic commented 4 years ago

I zipped the broken cache LSP-html folder that throws the error. And I zipped the good cache LSP-html folder (after I first deleted the broken chacke, and let the plugin install everything from scratch)

I probably only need to find the differences between those folders to see what might be the cause.

Broken-Cache-LSP-html.zip Good-Cache-LSP-html.zip

jfcherng commented 4 years ago

The only difference is that there is no out/ at all in the broken one.