Multi root: files are allowed as folder?

[bpasero]

I am surprised to see that I can add a literal file path into my workspace config and have it appear as a node in the explorer:

This is only asking for trouble I would say, e.g we try to install a file watcher on the file assuming it is a folder.

I think we should ignore such an entry from the workspace if we detect it is a file and not a folder.

//cc @isidorn

[sandy081]

I never validate the added path/uri in the workspace - neither for existence nor file type.
@isidorn May be this validation shall also go to explorer?

[bpasero]

While I think the explorer should not show the file as folder as it happens today, I think some kind of validation needs to happen on the level of the workspace service. Otherwise e.g. extensions are seeing the file as workspace folder even though it is a file.

Not sure how to best fix this to be honest. I would not want to block the startup to verify the file type because it could be a remote URI that takes long to resolve...

[isidorn]

@sandy081 yes I can do the validation in the Explorer. But yeah idealy the validation would happen before on the worskapce service level. If that is not possible, let me know and I will add validation on the explorer level as a workaround.

[sandy081]

I can do the validation after initialisation so that it does not block the start up. So validation removes the file from workspace folders. In case of non existing folders we validate while rendering. Does not this cause inconsistency? Is not it better for user to see it in the explorer and let the user to remove it instead of silently hiding?

[isidorn]

Having a non existing folder can be caused by not manually editiing the workspace json file. User nicely adds a folder, and after a couple of days deletes it and then reopen the workspace.
Having a file there can only be done by manualy editing the workspace setting file. Thus I think it will happen much more rarely. Thus I do not care that much if we guide the user to fix this.
So just filtering out on the first level is perfectly fine imho

[sandy081]

Makes sense. Added folder validation to workspace service.

@bpasero I still see following errors while resolving invalid paths

Error: Unable to read file '/Users/sandy081/work/vscode/package.json/.vscode/launch.json' (Unknown (FileSystemError): Error: ENOTDIR: not a directory, open '/Users/sandy081/work/vscode/package.json/.vscode/launch.json')

Currently file service is returning FILE_OTHER_ERROR . Is it possible to get a specific error?

[bpasero]

@sandy081 I added FileOperationResult.FILE_NOT_DIRECTORY just now.

How would this work now, are you validating the folder being an actual folder async and then remove it from your list of workspaces but keep it in the workspace file?

[bpasero]

@sandy081 looks like this error code does not work on Windows at least, Windows is throwing ENOTFOUND when trying to resolve a file that has a file as parent.

[sandy081]

How would this work now, are you validating the folder being an actual folder async and then remove it from your list of workspaces but keep it in the workspace file?

Yes. I only remove it in memory and keep it in workspace file. And regarding validation, I am not blocking the initialisation.

vscode/src/vs/workbench/services/configuration/browser/configurationService.ts

Lines 391 to 392 in 9cf403a

	// Not waiting on this validation to unblock start up
	this.validateWorkspaceFoldersAndReload();

I will use both FileOperationResult.FILE_NOT_DIRECTORY and FileOperationResult.FILE_NOT_FOUND

[bpasero]

Great, thanks for the fast turnaround.