Enable FileSystemProvider to stat a file as readonly

[gjsjohnmurray]

Currently a FileSystemProvider can register as readonly, meaning that every file it provides is opened in a readonly editor. This is nice, but even better if the FileStat it returns from its stat() method had an optional readonly property. Then I think this line could easily treat a true readonly value as a signal to set isReadonly true. @bpasero what do you think?

[bpasero]

I think this is generally a nice idea, but I am worried that it will make file operations very complicated: imagine a folder where just a few files are readonly. Now a fs operation runs that operates on all files in the folder, not just one (e.g. rename of the folder? delete?). Naturally with this we would first have to ask the provider if any of the files that are potentially modified are readonly before even starting the operation, which seems quite crazy to me.

[gjsjohnmurray]

Fair point, but what happens at the moment in such scenarios if the user has set restrictive OS-level file permissions to protect just a few of the files in a folder tree? Does VSCode already run a preflight check on, say, a parent folder delete and decline to start the operation if any of the descendant files isn't deletable?

[bpasero]

@gjsjohnmurray yeah we are not doing anything but rely fully on the OS to do the right thing. If one file in a folder is write protected, the OS may decide that a rename on the parent folder is not possible (have not validated that).

[gjsjohnmurray]

@bpasero I'd also argue that it shouldn't be VSCode's responsibility to recursively check the readonly stat of every file within a folder before asking the FileSystemProvider to delete the folder. The FSP is best placed to know if that's an appropriate check to make. For example my FSP might simply fail the folder delete request immediately because it's a root folder. I certainly don't want you to have wasted time (yours and mine) walking the entire tree and asking me to stat each file for you if I'm always going to reject your subsequent request to delete the root folder.

[bpasero]

@gjsjohnmurray yeah good point 👍

[gjsjohnmurray]

Please can this issue get some love soon?

[gjsjohnmurray]

@jrieken are you open for a PR to implement this? I'm guessing it needs to be treated as proposed API. I'd copy the FileStat interface into vscode.proposed.d.ts, add readonly?: boolean, and wire this up. That has become a bit more complicated since @bpasero did some restructuring in 920d80e but ultimately the new readonly from FileStat would have its effect here.

vscode/src/vs/workbench/services/textfile/common/textFileEditorModel.ts

Line 1031 in 8134d8c

return this.fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly);

[jrieken]

Proposing API is usually the very last step and requires an "internal" API first and I don't think there is one. I believe that would be around here and than needs options for editors, custom editor etc

[gjsjohnmurray]

I have submitted draft PR #111237 for comment and guidance.

[bpasero]

One thing to note is that if we decide to push this, the experience for readonly files will be different from how we treat OS readonly files. We would probably not adopt this for OS local files to preserve this current imho inferior behaviour:

• you can always type in readonly files, even if they are readonly
• you can save readonly files and be asked to overwrite the write protection at that point
• you can still decide to "Save As" to a different file name to avoid readonly'ess

With this solution all you see is a readonly editor.

[bpasero]

Little update to #73122 (comment): with 5a8936a there is a new notion of FileWriteLock errors that a provider can throw and a new unlock write capability. As such, the feature of unlocking write protected files is fully independent from "readonly" notion. As such I think it would be possible to allow for readonly files that turn the editor readonly without impacting the scenario of write protected files.

Timing wise I cannot make any better promises than tackling this in our issue grooming iteration.

[gjsjohnmurray]

I think this shipped in 1.57 so can be closed, right?

https://code.visualstudio.com/updates/v1_57#_enable-file-system-providers-to-declare-a-file-as-readonly

[bpasero]

@gjsjohnmurray it is only proposed API, not finalized yet.

[gjsjohnmurray]

@bpasero sorry, I misread the 1.57 release notes. I often find I miss the point where the "Proposed Extension APIs" section begins. Maybe the text of each subsection of that could helpfully reiterate "proposed API" in future release notes?

[bpasero]

Verification

Test that you can mark a file as readonly without enabling proposed API via the permissions property on a file. #124846 has details how to use it.

@gjsjohnmurray if you are able to verify in current insiders, that would be ideal 👍

[DavyLandman]

I hope this isn't considered an annoying question, but what is the process for an api change like this to graduate out of the insiders/proposed status?

[bpasero]

Not at all, see: https://github.com/microsoft/vscode/wiki/Extension-API-process

[DavyLandman]

@bpasero thanks, I was already clicking through the wiki, but seemed to have missed that.

So without jumping on calls, is there a way I can help move this forward? The change seems quite minimal, and would help 2 extensions I'm developing (both are languages with their own virtual file systems for some parts).

I saw this list, in the wiki:

• Is there a non-trivial usage-sample for this new API?
• Are there multiple (ideally not similar) use-cases that this proposal solves?
• Is the proposal too narrow (not solving enough) or too broad (trying to solve too much)?
• Who would use this new API?

And I'm not completely sure where this should be discussed? Should I raise a new issue, there are already quite a few issues about this topic on file specific read-only status.

[bpasero]

@DavyLandman this API will be available in our upcoming release next week.

[DavyLandman]

Thanks @bpasero!

[rchiodo]

I believe this was verified by #124846?

[gjsjohnmurray]

@gjsjohnmurray if you are able to verify in current insiders, that would be ideal 👍

/verified

First I had to work out how to make fsprovider-sample use the new API because it is no longer in vscode.proposed.d.ts but not yet published in @types/vscode