feat(language_server/vscode): support multi workspace folder setups#10515
feat(language_server/vscode): support multi workspace folder setups#10515
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
CodSpeed Instrumentation Performance ReportMerging #10515 will not alter performanceComparing Summary
|
63a5aff to
028d74c
Compare
ef3d0b1 to
94f0d9d
Compare
4668bd3 to
0b6b1f1
Compare
94f0d9d to
327d41f
Compare
0b6b1f1 to
55d5a6c
Compare
327d41f to
e9e176a
Compare
55d5a6c to
a851e3b
Compare
a851e3b to
f151e2e
Compare
284836b to
52bfc01
Compare
8ffa8d6 to
30437da
Compare
52bfc01 to
d9a77d9
Compare
05ed2c1 to
6c24b23
Compare
|
Thanks for testing it out. EDIT: it is fixed in main and this PR is restacked |
6c24b23 to
74d4a12
Compare
74d4a12 to
08c162b
Compare
08c162b to
b7cdaa9
Compare
b7cdaa9 to
01cc2e4
Compare
|
Re-tested within IntelliJ today and didn't notice any issues. Both nested config and specific config path support seems to be working. I don't think I'll be able to review the actual code changes here, but general usage seemed fine for me. I did find that IntelliJ does have some support for the workspace concept, but I'm not sure if that means it supports LSP workspaces. https://blog.jetbrains.com/idea/2024/08/workspaces-in-intellij-idea/ |
|
@camc314 can you merge after testing this PR manually? |
|
@Sysix , mind clarifying how this is meant to work? I think i'm just doing something wront while testing though. I tested on https://github.com/oxc-project/oxc-intellij-plugin/tree/main/sandbox, added a This is my {
"folders": [
{
"path": "custom-config"
},
{
"path": "nested-configs"
}
]
}
|
I tested my changes with the following changes: In {
"oxc.path.server": "/home/sysix/dev/oxc/editors/vscode/target/debug/oxc_language_server",
"oxc.trace.server": "verbose"
}I am also updating the docs to clarify how to use the debug version: oxc-project/website#357 Then add / remove folders to the workspace with the explorer: When you're in single folder mode and adding a folder, VSCode will restart the window on my side. Every workspace folder can be configured by its own |
| let Ok(configs) = self.client.configuration(config_items).await else { | ||
| debug!("failed to get configuration"); | ||
| // return none for each workspace folder | ||
| return vec![None; length]; | ||
| }; |
There was a problem hiding this comment.
| let Ok(configs) = self.client.configuration(config_items).await else { | |
| debug!("failed to get configuration"); | |
| // return none for each workspace folder | |
| return vec![None; length]; | |
| }; | |
| let configs = match self.client.configuration(config_items).await { | |
| Ok(config) => config, | |
| Err(e) => { | |
| debug!("failed to get configuration {e:?}"); | |
| return vec![None; length]; | |
| } | |
| }; |
i think it would be ideal that the error is logged here?
|
thanks - been debugging this trying to work out whats going on, I keep on seeing the following, when running this (i changed the code slightly to add more logging, but the lsp gets the workspace folders, but fails to get configuration - error server not initiaizlized The error comes from tower code: |
|
Thanks for this info ❤️ I looked into the My Plan: I guess we need to move The configuration This was mostly notes for me to get my problems / ideas structured. @camc314 and every other feel free to give some feedback. Will start tomorrow and some new PRs to get the goal ❤️ |
|
that sounds great, thank you for working on this! The code for this change looked good to me, just the issue with the client request during startup. It's up to you if you would like to close this PR/create a new one |
Follow up from #10515 Testsetup can be found here: #10515 (comment)
Follow up from #10515 Testsetup can be found here: #10515 (comment)
Follow up from oxc-project/oxc#10515 Testsetup can be found here: oxc-project/oxc#10515 (comment)






Server Side Changes
As a Server, we need to make sure we follow the LSP specifications. So can not make changes in VSCode and implement them in the language server.
initialize
Check if a custom configuration is passed with
initialization_options. Not every Client will send a configuration and expect that the server will request them withworkspace/configuration.We still uses
root_urias a fallback for older clients which do not support workspace folders.Every use case tries to have a good fallback when something is wrong.
Change Configuration
The Client is sending a Custom Configuration to the Client. Not every Client will send it. So we request for the specific workspace configuration and update them.
Workspace Folder Change
This is new and should be self explained.
Client (VSCode Side Changes)
Changed Configuration scopes
This should reflect what the server will support.
onConfigChangeWe are now making sure that we check for every possible configuration. Custom User Configuration we will ignore.
Testing
Testing will be done with #10648.
We need to change the test structure to different between multi root setups and normal setups.
Not every test should be executed twice.
I tried to test every use case with VSCode possible and only found one problem.
When we add a workspace folder with an already defined
oxc.configPath, we want to restart the server, but theonConfigChangeevent will already fireworkspace/didChangeConfigurationrequest. This will result into a small error window, but still works perfectly.We should avoid restarting the server when we need to create new file watchers. There are some workarounds that the server needs to support first. This is a task for later :)