-
-
Notifications
You must be signed in to change notification settings - Fork 399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor for Server Initialization Options #1916
Conversation
Just curious... did you consider having a single Volar server (process) and have it start additional processes, if needed, and route requests accordingly? That would be quite convenient for the clients (at least for ST) since this approach of starting multiple servers is not supported very well. Though I see that you are not running typescript server in a separate process so that's probably why you are not doing that. It would all be blocking the main process... |
This is an interesting idea. Maybe there could be a generic routing language server to support multiple servers for any languages. But honestly multiple servers are not the ultimate solution and because it's increase memory usage with TS language service in large project, a better way is optimize IDE request strategy to mitigate the impact of request blocking, usually the biggest trouble is that slow An another major problem is LSP cancel token is useless for TS, because the TS API is synchronous and the cancel token cannot receive updates from the LSP until the TS API execution is complete. If we can solve either of the above, I hope to avoid use multiple servers. |
But I guess that's the consequence of Volar using Typescript API synchronously. It could spin up a separate process for it like typescript-language-server does and communicate through IPC/stdio. Then it wouldn't be blocking the main process. |
I do want to migrate to tsserver, I'm still waiting for microsoft/TypeScript#47600, but I found it is just closed... |
I just found how TS supports cancellation token for tsserver (https://github.com/Microsoft/TypeScript/wiki/Standalone-Server-%28tsserver%29#cancellation, https://github.com/microsoft/TypeScript/blob/663b19fe4a7c4d4ddaa61aedadd28da06acd27b6/src/cancellationToken/cancellationToken.ts#L11-L20), and implement in 2e1108c. Now we're down from 3 servers to 2 servers. We have a new
|
Why does client have to do it rather than the language server creating a temporary, random file? |
Sorry, didn't look at your code carefully and didn't noticed that the client is supposed to write to the file also on file changes. I must say that I don't like pushing this responsibility to the client but I'm guessing it's optional so it should be fine not to implement it. |
This is optional not requirement, at least for now it can only be implemented on the client side.
Yes, here is just to provide a solution, it is up to the client whether to use it or not. Actually typescript-language-server also has this, but you may not use it in client yet. |
typescript-language-server creates the file itself. It doesn't need to come from the client. But it makes sense since it doesn't use typescript API synchronously so it isn't blocked by it. |
Sorry i didn't make it clear, The "blocking" mean in my words is that outdated requests cannot be cancel while executing a synchronous API, thus blocking new incoming requests, not mean blocking the main process. If client do not implement You can reference to VSCode for how it implement and provide cancellationPipeName to tsserver: https://github.com/microsoft/vscode/blob/61f8dee571efac71035000f4c57f1c42ada04e46/extensions/typescript-language-features/src/tsServer/cancellation.electron.ts#L27 |
typescript-language-server does use cancellation tokens for some requests. For example for requesting diagnostics here: https://github.com/typescript-language-server/typescript-language-server/blob/7f69c27eb8cce71d3db006623757a74f93d76dd3/src/lsp-server.ts#L330-L337 And the client doesn't need to do anything because the "cancellationPipeName" is created by the server and is utilized whenever client sends a standard LSP requests (though most requests don't care about canceling old requests currently). Since typescript-language-server is using typescript through the IPC or STDIO, it is not being blocked itself and can write to that cancellation file itself. |
Thanks for the clarification. I can see that typescript-language-server seems is a LSP server version of typescript-language-features. |
Yes, exactly. |
Volar introduced some breaking changes in version 1.0 vuejs/language-tools#1916. Also see neovim/nvim-lspconfig#2181.
@johnsoncodehk With nvim-lspconfig now certain features like goto definition or hover don' work anymore. I assume it has to do with this change.
I have problems to follow this. If I understand right you ignore client capabilities now? Your explanation sounds like you only need extra configuration if you want to turn capabilities off - but that's not what I found. Can you explain a bit more what configuration we need to enable these capabilites again? |
@mikehaertl It should be resolved by 8daa162 |
@johnsoncodehk Cool, thanks. Looking forward to the next release so I can test it. |
@johnsoncodehk I finally compiled Would you consider creating a 1.0.10 release to make this easier to install? There's quite some people affected by this bug and thay all can't update to 1.x due to this problem. |
This PR refactors the
initializationOptions
interface for language server, and make language clients based on the v0.x version no longer available.For language clients that do not support multiple servers, just need to change
typescript.serverPath
,typescript.localizedPath
totypescript.tsdk
.For multiple servers supports, we don't have
languageFeatures
anddocumentFeatures
anymore, and you need to modify language client capabilities of InitializeParams for hint to language server which features should not be active.But server diagnostic is use push model by default and do not care about client capabilities, to disable diagnostic, you can setup
{ serverMode: 2 }
to create language server that only active syntactic features, or config{ diagnosticModel: 0 }
to disable diagnostic for semantic features server.You can refer to:
cc @yaegassy, @sethidden, @tsukkee, @predragnikolic, @rchl, @kabiaa, @jadestrong, @tommasongr, @yoyo930021, @xiaoxin-sky