Unsafe canHandleResource usages

[jrieken]

We have RemoteFileService#canHandleResource which was introduced as workaround to be able to restore editors from providers that aren't known (yet) but that were around during the last session: https://github.com//Microsoft/vscode/commit/a23633b5d42dc486a668e49a232957696750c254.

This wasn't designed to be a general purpose API and I am surprised by its popularity. In hindsight the FileEditorInput should have serialised that knowledge about other-schemes and not the file service. I want to make that change but many other references leave me puzzled. This is what's to do

• use canHandleResource in combination with onDidChangeFileSystemProviderRegistrations
• don't rely on this information statically, e.g. expect that scheme come and do and expect that in the beginning there is just the file-scheme

[jrieken]

e.g. the ResourceContextKey seems to be misbehaving

[jrieken]

@bpasero I have pushed b7e8a4f which changes the way in which file editor inputs get deserialised. i have tested things but please review/advise

[bpasero]

@jrieken yeah unhappy, I was assuming this would be a way to statically find out if a resource is supported in the file service or not, but I see how this is not true when remote file system providers come and go.

Also adding @isidorn for some places where we use this e.g. in the resource context key to adjust enablement for commands.

I need to revisit the file editor input serialisation and creation given this, but would like to keep using the createInput method for now because that one goes through a cache (here) so that we never have more than one input for the same file. While I don't think your change would cause much harm, I am a bit nervous with that change before end-game.

[isidorn]

We were focused on grid this milestone, next week I am on vacation, this is not for endgame -> June

[bpasero]

I have pushed a change that ensures a FileEditorInput can be created upon reload even if the file system provider is not loaded yet. I checked all usages of canHandleResource and think the only other unsafe usage is within ResourceContextKey#set() as indicated in #48275 (comment)

@isidorn I leave that one up to you to see what needs to be done

[isidorn]

I decided to remove resource.isFile context key completely. Reasons:

1. most of the cases it was not properly used and a hasResource is more fitting
2. it can be substituted with resource.scheme.isEqualTo('file')
3. users found it confusing as they thought that this context key tells if the resource is not a directory

I will mention in the release notes that this context key is not removed ( I do not believe people are using it that often)

[isidorn]

Went all over github and only two repos use "resourceIsFile" I will give them an issue.

[jrieken]

I still see (early) access to canHandleResource from the history service. It seems to delete entries before the extension host is ready, and therefore before file system providers have a chance to register.

There are also other usages but I they don't look critical... To reproduce, have a workspace with a remote fs entry (e.g MemFS), open a few files from that workspace, set a breakpoint here, and reload.

[jrieken]

@isidorn re
#48275 (comment). There is a difference between having an uri (hasResource) and being able to save/load something with a resource, e.g. webview editors have an uri but they don't support things like load, save, revert etc.

[isidorn]

@jrieken thanks, I have pushed a commit which improves this.
Added a new context isFileSystemResource, which now listenes to new fs registrations so the canHandleResource should always be correct.