String DocumentFilter should only match scheme 'file' and 'untitled'

[dbaeumer]

• VSCode Version: 1.10.x

Document filters that are only string based should only match scheme file and untitled. So if an extension registers a feature provider for 'bat' for example the provider should only be invoked for 'file' and 'untitled' scheme and not for example for 'git' scheme.

[dbaeumer]

@egamma FYI.

[jrieken]

@dbaeumer Pushed this to master, please verify with the language servers.

@aeschli You might need to adjust some selectors for html, json, etc that work off the virtual state

[dbaeumer]

@jrieken will do. Thanks!

[chrmarti]

@jrieken This breaks the interactive playground. Adding walkThroughSnippet to the _defaultSchemes in languageSelector.ts appears to fix it. Please advise.

[jrieken]

Please share more details about your problem? Are you registering your document as walkThroughSnippet and what extension do you expect to work with that? The consequence of our change is that those extensions must not opt in and adjust their language selector

[chrmarti]

The embedded editors use models with walkThroughSnippet as their URL scheme. These URLs no longer match the registered languages, they did before. As a consequence editorModelConext.ts deactivates all contexts and actions like Format and Rename are no longer available in the embedded editors, that's a regression.

[jrieken]

Yes, that is the new design. It's not enough for the languages to just register with 'langId' anymore but they must spell out all schemes they should support (file, untitled, walkThroughSnippet ).

[jrieken]

fyi @dbaeumer

[dbaeumer]

The consequences of the old design was that the walkThroughSnippets were honored by a lot of extensions although they can't actually do something meaningful with them. With the new design extension by default participate in file and untitled (what most extensions thought happens anyways) and you must explicitly opt in into more schemes. This is breaking by design.

[chrmarti]

Chatted with @jrieken, he suggests to add scheme: * to the languages participating in the playground, effectively reversing the effect of this change. It would be easier and avoid matching all schemes to handle walkThroughSnippet like untitled. The original change meant to address issues with git and similar, not walkThroughSnippet, if I understand correctly.

[jrieken]

We will revert the changes for March to give us more time for this

[jrieken]

Moving to April for further discussion

[chrmarti]

Another way to look at it is that the Playground document is at odds with the layering, because it comes with languages that are not in the core, but in extensions. Longer term we could move the document out of the core, while keeping the Playground URI scheme and editor as language-agnostic support for playgrounds in the core. That would allow for special-casing the Playground scheme in the core similarly to untitled. /cc @kieferrm

[dbaeumer]

IMO we should not have any playground specific URI knowledge in the core. A Playground should be contributable by an extension without it being necessary to modify the core.

I know that due to limitation we need to, but we should push hard to improve our system to not require thus.

[jrieken]

IMO we should not have any playground specific URI knowledge in the core.

Agree

A Playground should be contributable by an extension without it being necessary to modify the core.

Disagree. The extension API is there to enrich the core development cycle not these scenarios. IMO the playground should use the standalone editor with its language service implementation. It's problematic that playground samples bleed into the extension host and causing all kinds issues, like markers we don't really want.

[jrieken]

This feature request will not be considered in the next 6-12 months roadmap and as such will be closed to keep the number of issues we have to maintain actionable. Thanks for understanding and happy coding!

[jrieken]

Revisiting this for April...

[jrieken]

Idea is to educate because it's hard to make the right (breaking) decision for every extension. As a first step, I have added an info-message that gets logged once for extensions that are under development and that use a lax-selector. Next step is a page with more documentation/information that we can link to.

[jrieken]

@gregvanl Can you please review the text here: https://go.microsoft.com/fwlink/?linkid=872305. We should also find a place in the docs for this.

/cc @egamma