Editors - pathsToEditors should ignore folders

[lszomoru]

Ignore folders when creating the editor inputs.

[bpasero]

I am a bit worried that doing the stat everytime now, even when we now the file exists could slow down startup. This is on the critical path of the perf machine, so we should probably avoid the file service use unless we need it.

[lszomoru]

@bpasero, I think that is a fair concern, though what other option do we have to detect that the path is a folder without making a stat call? Could we enhance the editor service in order to handle this scenario? Happy to chat about our options. Thanks!

[bpasero]

Whoever is setting the exists property to true could maybe rather indicate the type of the path? We seem to be doing a stat call somewhere else to figure this out so we can make it provide the needed information.

[lszomoru]

@bpasero, what about adding file/folder information to IPathData?
By doing so, we would have to call stat if that information is missing.

[bpasero]

Yeah, something like that is what I had in mind. It would probably be sufficient to just indicate if its a file or folder.

[joaomoreno]

I am a bit worried that doing the stat everytime now, even when we now the file exists could slow down startup. This is on the critical path of the perf machine, so we should probably avoid the file service use unless we need it.

Perf-wise, what's the difference between fileService.exists and fileService.resolve? In a Node.JS based file system provider, for example, those two are the same: exists is built on top of resolve. And in fact...

https://github.com/microsoft/vscode/blob/bccd9b904b24c4b778753c93669c5f21dc1e1c7d/src/vs/platform/files/common/fileService.ts/#L297-L308

The code already calls fileService.exist. Switching that with fileService.resolve won't make it any slower.

[lszomoru]

@joaomoreno, I believe that the concern was that originally fileService.exists was only called in case that path.exists was not set. To understand whether the path is a file/folder we would have had to call fileService.resolve every single time independent whether path.exists was set or not. The latest iteration of the PR, only calls fileService.resolve if path.exists or path.isFile is not set.

[joaomoreno]

To understand whether the path is a file/folder we would have had to call fileService.resolve every single time independent whether path.exists was set or not.

For your intended purposes, you're always passing path.exists: true, so I'd keep the current semantics: if path.exists: true then resolve is called, otherwise we just assume it exists and is a file and go ahead with it.

[lszomoru]

As far as I can tell, I could be wrong, path.exists is not being set when opening a file through vscode.dev.

[joaomoreno]

I see now. Sad!

[bpasero]

@lszomoru please see my 💄 change that I just pushed. I only changed to have the FileType instead of a boolean.