-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[ls] implement multiple workspace folders in MonacoWorkspace #7182
Conversation
I do not understand this explicit |
I see. They're from here: https://github.com/microsoft/vscode/blob/master/src/vs/vscode.d.ts#L8432-L8452. |
6b63efd
to
f39b687
Compare
Sorry for the sloppy initial commit. I've changed the |
f39b687
to
04ccad6
Compare
I addressed your comments and re-implemented the workspaceFolder init/updated |
@JanKoehnlein the build is failing, for some reasons it cannot find monaco namespace anymore |
I had this sporadically here, too. I guess we should not add a dependency to |
04ccad6
to
ecedb65
Compare
Thanks for the updates, @JanKoehnlein. Once the build is green I am going to verify it with one or two VS Code LS extensions. |
It throws an error at startup:
You get the value before you set it:
Set:
|
Fixes #6108 Signed-off-by: Jan Koehnlein <jan.koehnlein@typefox.io>
ecedb65
to
4985878
Compare
Wow. Looks like I should get more sleep. Initialization of the property with an empty array was missing. |
From the LSP specs:
If I open Theia without a workspace and then open a single file (e.g.: |
@kittaakos Docs for VS Code APIs: https://github.com/microsoft/vscode/blob/c975df3b59ac221f70d525498ae6ff81bce77a10/src/vs/vscode.d.ts#L8515-L8519 Although It says the same 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tried it with the TS LS, and the vscode-go
extension in the browser. I added then removed a few workspaces, even ones with whitespace in their names. It worked as expected: I had diagnostics and content assist support in my newly added workspaces. Thank you for the path patch 👍
Thanks a lot for the reviews! |
Fixes #6108
Signed-off-by: Jan Koehnlein jan.koehnlein@typefox.io
What it does
This PR adds support of multiple workspace root folders to Monaco workspace.
Language servers will be notified with a
didChangeWorkspaceFolders
notification whenever the user adds or removes a workspace folder.How to test
Add/remove workspace folders in Theia and watch extensions with language servers get notified.
Reminder for reviewers