Skip to content
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

Current status of didChangeWorkspaceFolders #1810

Closed
musjj opened this issue Jan 7, 2023 · 11 comments
Closed

Current status of didChangeWorkspaceFolders #1810

musjj opened this issue Jan 7, 2023 · 11 comments
Labels
bug Something isn't working Info Needed More information is required

Comments

@musjj
Copy link

musjj commented Jan 7, 2023

This LSP implements a handler for didChangeWorkspaceFolders, but it does not really work correctly (especially on Windows).

The documentation does mention that workspaces cannot be dynamically modified, so I'm not sure why is there a handler for didChangeWorkspaceFolders.

It also seems that the official VSCode extension restarts the server instead of sending didChangeWorkspaceFolders when a new folder is added, which further questions what the handler is there for.

This is a fairly non-standard behavior, as servers are expected to handle didChangeWorkspaceFolders properly if workspace folders are supported. Surprises like this adds hurdles when developing clients for non-VSCode editors.

So, are there any plans for dynamic workspaces in the near feature, or are clients expected to restart the server for the foreseeable future?

@carsakiller carsakiller added the question User has a question label Jan 10, 2023
@sumneko sumneko added bug Something isn't working and removed question User has a question labels Jan 12, 2023
@sumneko
Copy link
Collaborator

sumneko commented Jan 12, 2023

It should be supported since 3.5.1, but I forgot to change the wiki.
Maybe it is broken now, I will check it in next days.

@sumneko
Copy link
Collaborator

sumneko commented Jan 16, 2023

I have tested it in VSCode Windows and it works normally.
The first time you add a new workspace into current workspace, VSCode will force restart all extension since it change single workspace mode to multi-workspace mode.
Once it has runned in multi-workspace mode, you can freely delete or add workspace without restarting.

@sumneko sumneko added the Info Needed More information is required label Jan 16, 2023
@sumneko
Copy link
Collaborator

sumneko commented Jan 16, 2023

file_d%3A_Github_test.log

This my server log for backlog.
Could you please provide your log with --loglevel=debug

@musjj
Copy link
Author

musjj commented Jan 17, 2023

Yes, I know that the VSCode extension restarts the server. That's why the VSCode extension works just fine for me.

Once it has runned in multi-workspace mode, you can freely delete or add workspace without restarting.

Right, I think this is non-standard behaviour. Other LSPs doesn't require the user to do this.
For example with Pyright, you can start the server with a single root, send a didChangeWorkspaceFolders request, and the new workspace folder will be picked up just fine, all without restarting the server at all.

Can we have the same behaviour with lua-language-server?

@sumneko
Copy link
Collaborator

sumneko commented Jan 17, 2023

Could you provide a example RPC log? I will try to build a test case.

@musjj
Copy link
Author

musjj commented Jan 17, 2023

Of lua-language-server? or Pyright?

@sumneko
Copy link
Collaborator

sumneko commented Jan 17, 2023

Of lua-language-server, then I can simulate the client to send these protocols to the server in the test, likes this: https://github.com/sumneko/lua-language-server/blob/master/test/tclient/tests/change-workspace-folder.lua

@musjj
Copy link
Author

musjj commented Jan 17, 2023

Btw, does that test actually work? I'm not completely sure of how it works, but I don't see any restarts in-between the workspace changes.

@sumneko
Copy link
Collaborator

sumneko commented Jan 17, 2023

This feature is designed without restart.
The restart when changing single-workspace mode to multi-workspace is triggered by VSCode itself, I cannot prevent it.

@musjj
Copy link
Author

musjj commented Jan 17, 2023

Here's my log, with --loglevel=trace:
https://gist.github.com/musjj/814f188130b0a0f098c1e7dc43f980c7

@musjj
Copy link
Author

musjj commented Jan 18, 2023

Thank you for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Info Needed More information is required
Projects
None yet
Development

No branches or pull requests

3 participants