sublimelsp / LSP-angular

Convenience plugin for Angular Language Service
MIT License
9 stars 2 forks source link

The HTML language ID #4

Closed rwols closed 4 years ago

rwols commented 4 years ago

I see that the languageId is set to html, but the selector is different from regular html. That is, the selector here is text.html.ngx but other server setups use text.html.basic. If we want to abstract away these document selectors they should all agree for every VSCode language ID.

Could you try changing the languageId to html-ngx and see if this Angular server still works as expected?

rwols commented 4 years ago

It's a server that only works for one languageId so I doubt it even looks at the languageId that we send in textDocument/didOpen

rchl commented 4 years ago

Just a quick search in the server's code returns this: https://github.com/angular/vscode-ng-language-service/blob/546870c3a486424200aecd1b8255f68ff1dbb415/server/src/session.ts#L26-L29

So it does support multiple languageId's and we should also add typescript here.

rchl commented 4 years ago

Looking at the logic where it compares languageId, it seems like it only explicitly checks for typescript and if it gets something else, it won't really use that value for anything.

https://github.com/angular/vscode-ng-language-service/blob/546870c3a486424200aecd1b8255f68ff1dbb415/server/src/session.ts#L253

So changing to html-ngx should work but I wouldn't personally recommended that as it's not official ID it supports and things could break in the future.

rwols commented 4 years ago

I thought {"languageId": "html", "pattern": "**/*.component.html?"} would perhaps be necessary, given that that's the file extension of this HTML Ngx syntax. But the VSCode client just starts this server for any html file? I'm not well-versed in typescript though https://github.com/angular/vscode-ng-language-service/blob/546870c3a486424200aecd1b8255f68ff1dbb415/client/src/extension.ts#L28-L33

rwols commented 4 years ago

The problem with having different Sublime text selectors for the same language ID is that the matchers are different (obviously), but that causes the language ID stored in the view.settings() to be ambiguous and dependent on the particular server config (see https://github.com/sublimelsp/LSP/issues/1052#issuecomment-630904320)

ghost commented 4 years ago

Could you try changing the languageId to html-ngx and see if this Angular server still works as expected?

I'll check on the weekends

So it does support multiple languageId's and we should also add typescript here.

Initially, I added typescript, but from time to time LSP-angular was replacing LSP-typescript capabilities, I'll check this too.

rwols commented 4 years ago

but from time to time LSP-angular was replacing LSP-typescript capabilities, I'll check this too.

What you can do in that case (provided that theia's typescript-language-server is better than angular's), is you can set an empty feature_selector so that its score is always 1, which is less than 16 (= score of typescript's langserver). This will select the typescript langserver in case there's multiple providers (e.g. for completionProvider).

In code:

{
    "languageId": "typescript",
    "document_selector": "source.ts",
    "feature_selector": ""  // prefer other typescript langservers over this one
}
ghost commented 4 years ago

@rwols, hello :)

I thought {"languageId": "html", "pattern": "*/.component.html?"} would perhaps be necessary

I did some investigation, and I can say that VSCode client starts this server for any HTML file, but only if a valid tsconfig.json file is present in the workspace folder.

I've changed the language id to html-ngx and I believe everything is working.

"feature_selector": "" // prefer other typescript langservers over this one

It seems that the empty feature_selector is not working, because LSP-angular still overrides LSP-typescript. Will do PR to LSP-typescript, it works like a charm with the correct config.

rwols commented 4 years ago

Hmm, I agree with rchl in that the language ID should stay html as things can break in the future. Apologies for not communicating that clearly enough.