Workspace folder changes are not sent to the server if they occur during initialisation

[DanTup]

I don't have an easy repro for this because it involves setting up a server, but I've noticed that if you add/remove workspace folders while the initialisation is happening, the workspace events never make it to the server, so the server ends up with the original workspace list that was transmitted in the initialize request, even though the client modified them prior to getting a response.

I think probably the client should queue up any workspace folder changes during initialization and transmit them after the initialized notification. It may also be worth calling this out in the LSP spec since it seems like something that could easily be implementing wrong elsewhere.

[tsmaeder]

We are hitting this case when automatically cloning java projects at startup and adding them to the workspace.
I think the problem lies in client.ts#2804 and following.
The client sends "initialize", but does not yet have listeners set up for forwarding of workspace folders changed events. That happens on line 2812.
Folder changes might happen after sending initialize but before registering the feature. I think it would be necessary to send "didChangeWorkspaceFolders" if workspace folders changed between those two lines.

[tsmaeder]

@dbaeumer with a bit of guidance, we could invest some time in whipping up a PR.

[tsmaeder]

I'm working on a PR.

[dbaeumer]

@tsmaeder thanks for looking into this. Here is how I would tackle this:

Options one:

• before calling initialize add a workspace folder listener that captures all changes and buffers them
• when the feature gets registered check for buffer events and replay them

Option two:

• remember which folders are sent during initialize
• when the feature gets registered check the folder again and compare against the captured once. If different craft a change event and send it.

Since I think this will not happen frequently I would go for the second options.

[tsmaeder]

I've been working on option 2. The hard part is testing.

[tsmaeder]

Particularly becuase updateWorkspaceFolders does not return a promise.

[tsmaeder]

@dbaeumer I am trying to write a unit test using the vscode API (in the test suite in client-tests). However, try as I might, "updateWorkspaceFolders" never seems to have any effects. What am I missing?

[tsmaeder]

I'm missing that this is turned off explicitly when in tests. Grmpfl! Hmm...any suggestions on how to write a test for this?

[tsmaeder]

@dbaeumer I refactored the feature to be able to test the notification behaviour without using the plugin API and wrote tests using that test interface.

[dbaeumer]

@tsmaeder sorry for the late response, but I needed to do some work around LSIF. I will look into your PR beginning of next week.

[dbaeumer]

@tsmaeder reviewed the PR.

[dbaeumer]

This got addressed by @tsmaeder

[tsmaeder]

@dbaeumer so is this in the latest release? I didn't see it in the changelog.

[dbaeumer]

It is in currently in next which we plan to make official beginning of next week.

[dbaeumer]

Version number is 6.0.0-next.*