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

Scheme for handling virtual documents #49069

Closed
octref opened this issue May 2, 2018 · 8 comments
Closed

Scheme for handling virtual documents #49069

octref opened this issue May 2, 2018 · 8 comments
Assignees
Labels
api under-discussion Issue is under discussion for relevance, priority, approach
Milestone

Comments

@octref
Copy link
Contributor

octref commented May 2, 2018

In #48777, we attempted to limit HTML/CSS extensions to only handling file and untitled schemes. However, this breaks embedded HTML/CSS completions and other language features in Intelephense. It is reverted for release.

Intelephense creates a virtual HTML document and runs command vscode.executeCompletionItemProvider to get list of completions from HTML language server (which includes CSS/JS embedded in HTML).

We should find a way to let languages mark themselves able to handle virtual documents.

@octref octref added this to the April Recovery 2018 milestone May 2, 2018
@octref octref added the api label May 2, 2018
@jrieken
Copy link
Member

jrieken commented May 3, 2018

We should find a way to let languages mark themselves able to handle virtual documents.

@octref I don't understand that

@jrieken jrieken added the info-needed Issue requires more information from poster label May 3, 2018
@octref
Copy link
Contributor Author

octref commented May 3, 2018

@jrieken I'm suggesting having a virtual scheme in parallel to file and untitled.

@lostintangent
Copy link
Member

lostintangent commented May 3, 2018

@octref What would virtual match? Any document scheme that was defined via the registerTextDocumentContentProvider API? (which presumably accommodates the Intelephense use case?)

But it wouldn't match schemes provides by custom FileSystemProviders? That would be my ideal behavior, since we wouldn't want virtual to automatically match the vsls virtual file system that Live Share registers.

@octref
Copy link
Contributor Author

octref commented May 3, 2018

@lostintangent

What would virtual match? Any document scheme that was defined via the registerTextDocumentContentProvider API?

Yes. From language server's perspectives if they declare themselves as able to handle file, they may not be able to provide some functionalities for untitled files, which might not exist on disk and not have a path. I think it's the same thing for virtually created files.

For example, the Intelephense extension creates virtual html files out of the PHP files on disk, and call VS Code command vscode.executeCompletionItemProvider to get completions. HTML server is now able to handle it, because our HTML file support is single-file (no understanding of dependencies). However as we add more capabilities to HTML, we might need to selectively enable features for untitled or virtual files. There might be LS which don't want to handle untitled and virtual files, and they should be able to say so.

But it wouldn't match schemes provides by custom FileSystemProviders?

Yep files backed by custom FileSystemProviders still have a fs behind it. Just need to make extension author aware to always use that provider instead of Node's fs.

@jrieken
Copy link
Member

jrieken commented May 4, 2018

@jrieken I'm suggesting having a virtual scheme in parallel to file and untitled.

The problem with that is that there can only one provider per scheme (one truth) and I don't understand who that should be. In other words, you cannot be a document provider anymore but we need to invent another indirection.

TBH, this looks like a dependency between Intelephense and the Html/CSS extension (without explicitly being declared). IMO if we want to constrain Html/CSS to certain schemes then we should have a reasonable default and the Html/CSS extension needs to export API that allows to manage those schemes, e.g. Intelephense calls a function that Html/CSS exports

@jrieken jrieken added under-discussion Issue is under discussion for relevance, priority, approach and removed info-needed Issue requires more information from poster labels May 4, 2018
@aeschli
Copy link
Contributor

aeschli commented May 4, 2018

One of the main points for virtual documents is to avoid extension dependencies. That allows users to replace our html/css/json extensions. Also the problem is that many more language extensions that are similar to html/css/json would need such an API.

If we want to store meta information for schemes, that could be added to the file system provider or the document providers. What would that information be? 'If you are a language provider, exclude my resources because I guarantee that someone cares for it'?
If that's what you mean, I think that's what @alexandrudima suggested on the liveshare channel with the 'private' flag on file system providers.
I think the idea there is to say that html/css/json still register without any scheme restriction (equivalent to schema: '*'), but what it means is 'all schemes but the private ones'.

@jrieken
Copy link
Member

jrieken commented May 4, 2018

One of the main points for virtual documents is to avoid extension dependencies. That allows users to replace our html/css/json extensions. Also the problem is that many more language extensions that are similar to html/css/json would need such an API.

I understand, but do we have samples/scenarios for both statements? Otherwise I'd suggest to start simple before bringing out the bigger hammer.

Also when did the virtual document renaissance start? Didn't we conclude that it's best to integrate via libraries because its impossible to simply embed a language, e.g. TypeScript with imports embedded inside my new-super-language?

I think the idea there is to say that html/css/json still register without any scheme restriction (equivalent to schema: '*'), but what it means is 'all schemes but the private ones'.

If we do that then this issue resolves itself, right? Isn't the point of Intelephense creating virtual documents that they are not private to the html/css/json extension?

@aeschli
Copy link
Contributor

aeschli commented May 4, 2018

I understand, but do we have samples/scenarios for both statements? Otherwise I'd suggest to start simple before bringing out the bigger hammer.

We have scenarios and samples. They are all spread in various issues so #47288 is about bringing them together and them to the doc/wiki.
The statement was always that each language is free to choose the way how to deal with embedded languages. Integrate via libraries or use virtual documents. Intelliphense chose the virtual document approach and seem to be happy with it and can live with the limitations.

I suggest to close this issue as we already established that having a well known 'virtual' scheme isn't possible as @jrieken explained in #49069 (comment).

I brought up the 'private' flag as I assume @octref main motivation of the proposal was to find a solution for liveshare. I suggest we better continue the brainstoring and discussion of for a better solution in our liveshare channel. @octref ok to close?

@aeschli aeschli closed this as completed May 4, 2018
@chrmarti chrmarti modified the milestones: April Recovery 2018, Backlog May 7, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

5 participants