exclusive flag does not apply to diagnostics

[daytonellwanger]

An LSP server that registers itself with a document selector that uses the (proposed API) exclusive flag

documentSelector: [{ scheme: 'file', language: 'typescript', exclusive: true }]

will disable all other language services for requests, such as hover. However, other language services can still provide diagnostics, and presumably other notifications.

I'm attaching a simple extension that repros the issue. Upon running the "hello world" command it registers an LSP server that takes exclusive control over the file scheme for the typescript language. So if you open a TypeScript file, you'll initially get language services powered by the TS Server. Once you run the command, you no longer will get TypeScript language services for hovers, for example. However, the TS Server is still providing diagnostics.

exclusiveSample.zip

[jrieken]

Yeah, that's the design of diagnostics which use a push model, not a pull model.

[jrieken]

However, other language services can still provide diagnostics, and presumably other notifications.

It seems like you want to completely disable the TS extension. Is that right? What's on your agenda there might be an easier way

[daytonellwanger]

It seems like you want to completely disable the TS extension. Is that right? What's on your agenda there might be an easier way

Yes, that's right. And I'm certainly interested in exploring other ideas, as the exclusive flag has been an awkward solution, and will, I think, never make it past a proposed API.

For the guest in a Live Share session, it's almost never valid for local language servers to provide language services, as they don't have enough context for most things, and most of the requests are forwarded to the host machine, so local language services often cause duplicates (one local, one remote). Today, we use the exclusive flag to prevent this. We also have a custom scheme (vsls) on the guest for all files, which prevents some language servers from ever activating, although we shouldn't rely on this behavior, as it's reasonable for a language server to activate on all schemes (as it seems TS now does).

So I think another idea worth exploring is having a property on schemes that require language servers explicitly support that scheme to be activated for it.

[jrieken]

So I think another idea worth exploring is having a property on schemes that require language servers explicitly support that scheme to be activated for it.

Unsure how that should help with diagnostics and other pushed things like notifications etc. Should this knowledge be shared with extensions so that they ignore said documents? I think that would be a big ask from authors and also a late adoption.

Temporarily disabling extensions is IMO the best solution because it addresses these problems

• don't call providers two times (we use exclusive for this today)
• prevent computation and display of duplicated diagnostics (no fix today)
• prevent bogus static contributions like the "Restart TS Server" command (no fix today)

My knowledge about LS might be outdated but isn't it that the guest's extension host (usually) restarts when joining a session? I think that would be a good opportunity to tell the extension host to temporarily disable extensions that the host already has.

[daytonellwanger]

Unsure how that should help with diagnostics and other pushed things like notifications etc.

If the language server is never activated, it'll never start pushing notifications. This is similar to your proposal for disabling the extension, except it feels more general. The extension disabling approach will require that Live Share keep a list of extensions that should be disabled on the guest (it's not always the case that the presence of an extension on the host signals it should be disabled on the guest). It's also the case that sometimes we don't do a reload when joining, so extensions that require a reload to be disabled would break this strategy.

Should this knowledge be shared with extensions so that they ignore said documents? I think that would be a big ask from authors and also a late adoption.

Agreed any acceptable solution won't require that other extensions require code changes.

I understand that since diagnostics are push instead of pull, VS Code can't prevent language servers from pushing them. But can't it ignore the ones that get pushed when it sees they're for a scheme that the language server doesn't claim to explicitly support?

[jrieken]

But can't it ignore the ones that get pushed when it sees they're for a scheme that the language server doesn't claim to explicitly support?

Sure - but I wouldn't call that an ideal solution but yet another workaround. Why spend cycles to compute diagnostic if the get discarded anyways. That's why I favouring a "don't even start with this" approach.

If the language server is never activated, it'll never start pushing notifications. This is similar to your proposal for disabling the extension, except it feels more general.

Not sure that I understand what general in this context means. General to language servers but not to arbitrary extensions?

as it's reasonable for a language server to activate on all schemes (as it seems TS now does).

CSS, SASS, and JSON are in this camp ever since and I wonder how that is working in LS? How are duplicated diagnostics prevented with those languages?

[daytonellwanger]

Why spend cycles to compute diagnostic if the get discarded anyways.

Ah, yes, that's a good point.

Not sure that I understand what general in this context means. General to language servers but not to arbitrary extensions?

I meant that we wouldn't need to maintain a list of what extensions to disable for the Live Share guest.

How are duplicated diagnostics prevented with those languages?

I believe there are some languages which explicitly disable themselves when they detect the vsls scheme. I forget whether or not CSS, SASS, and JSON do that.. I don't see any other explanation for us not getting duplicates. This is of course not an ideal solution.

So your current recommendation is to maintain a list of extensions that should be disabled for the Live Share guest and write this to the Live Share guest workspace settings when joining a session?

Do you have any ideas for how we could automatically detect which extensions to disable? As I noted in an earlier comment, it's not simply the list of those that also exist on the host. But maybe there's some similar check to this that's more accurate..

The other issue with disabling extensions is that there will be some class of extensions that provide language services in addition to other behavior, and we wouldn't want to disable the entire extension, just the language services. E.g. the LaTeX Workshop extension is a hover provider, and we'd want to disable that piece, but not its other functionality.

[BrunoGuardia]

Let's reopen the discussion, as this has grown into a general issue, the original design is no longer enough to support the current behavior of multiple extensions.

@daytonellwanger @aletsdelarosa

[daytonellwanger]

@jrieken any suggestions for short-term fixes? After the most recent VS Code release, we have the issue mentioned above where the TS server is running for the Live Share guest and reporting errors that aren't actually errors, and our users were much quicker to notice this than I anticipated. We certainly need to revisit the exclusive flag option we've been using, but I imagine it'll be a month before that lands at the soonest.

I thought your suggestion of disabling the TypeScript extension for the guest would be a quick fix, but it doesn't actually appear like that's possible - VS Code offers no APIs for extensions or settings files for disabling other extensions as far as I can tell.

Is there anything we can do short of shipping a VS Code update that includes modifications to the behavior of the exclusive flag, a new mechanism, or not activating the TypeScript extension for the vsls scheme?

[jrieken]

The exclusive flag is certain maxed out. Unsure about TS tricks. Adding @mjbvz since I am not aware of any changes in the behaviour of TypeScript. From looking at its source I don't see anything new.

[daytonellwanger]

AFAICT, and I wasn't able to track it down to a code change, TS used to not activate for the vsls scheme, which prevented it from running on the guest side and providing diagnostics, but as of the latest release, it does.

So no ideas for how we can prevent the local TS extension from providing diagnostics for the Live Share guest with the current version of VS Code?

[mjbvz]

In 1.47, the JS/TS extension would only activate on files that use the file: and untitled: scheme. However this left many people confused as to why JS/TS intellisense did not work in files from custom file systems. It also meant the extension did not work in serverless cases.

In order to support the serverless cases, I removed this restriction 4 weeks again in e41c195. This means that JS/TS intellisense now activies for every js/ts file however all cross file operations are still gated to file and untitled files

I tested live share to make sure this didn't break IntelliSense must have missed the odd behavior with diagnostics. We do want diagnostics in the serverless case so my proposal is add a special path that disables JS/TS diagnostics on vsls files specifically (we could also look into disabling other language features but those don't seem to be causing problems)

[daytonellwanger]

Thanks Matt! That seems like a good short-term fix.

FWIW, I think that was a reasonable change to make, and it's odd having to special-case the vsls scheme. I'm hoping I can work with Johannes to come up with a better general solution, so TS and other languages that want to activate on all schemes don't need to be Live Share-aware.

[mjbvz]

I just pushed a scoped fix that should disable all language features and diagnostics for files with the vsls scheme: f694b07

I was having trouble getting live share working in the OSS build, so once the next insiders comes out let me know if this did not fix the issue

[jrieken]

Closing this as a workaround is in place. The design and usage of exclusive should be revisited on a large scope because it has limitations in all areas, not just diagnostics