'(read-only)' suffix no longer added to tab labels of files served by readonly filesystem

[gjsjohnmurray]

Issue Type: Bug

Looks like the work in 54c3db1 by @isidorn maybe resulted in the '(read-only)' suffix no longer getting added to the tab label of a file opened from a readonly filesystem. This used to work correctly in 1.38, though only after an additional click (see #69651).

The issue also exists in the latest Insiders.

I will prepare a branch of the MemFS sample to repro the issue.

VS Code version: Code 1.39.2 (6ab5985, 2019-10-15T15:35:18.241Z)
OS version: Windows_NT x64 10.0.17134

[vscodebot]

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

• Files served by readonly filesystem initially lack the '(read-only)' suffix on tab labels (#69651)

[gjsjohnmurray]

Branch at https://github.com/gjsjohnmurray/vscode-extension-samples/tree/show-82964 simply amends the MemFS sample so it registers as a readonly FileSystemProvider. The files created by first running MemFS: Setup Workspace followed by MemFS: Create Files behave correctly as readonly, but ever since 1.39 their tabs no longer show the '(read-only)' suffix. Nor does that suffix appear in the window titlebar.

[isidorn]

The issue here is as I have already mentioned in an older issue:
What is missing it seems is the textFileService should have an event which fires when the model becomes readonly. fyi @bpasero
For example when the lastResolvedFileStat is set here we could check if the readonly state changes and if yes we could fire an event.
@bpasero would be open for a PR for this?

[bpasero]

@isidorn yeah makes sense to me. we would probably not be able to detect this change unless the tab gets loaded again because there is no file event when this happens.

[gjsjohnmurray]

I'm not clear why it worked OK before 1.39. Even now it's not (yet) possible to set an individual file as read-only. All we can do is register a FileSystemProvider with readonly: true in its options.

[bpasero]

Isi would maybe know under which condition this can happen.

[isidorn]

It was working because we were caching the title less agressivly.
Now we cache it properly and drop the cache only when there is an actual change.

[gjsjohnmurray]

@isidorn I hope you agree that we at least need to initialize the cached value correctly. If a file is opened from a readonly FileSystemProvider it is correctly treated as readonly in VSCode. But since this change we have lost the indicator text. Surely a regression, no?

[bpasero]

@isidorn I think this is unrelated to the file model, but rather we might need to have another event that causes all file editor inputs to loose their caches when a file system provider becomes available? The way I see it:

• user has many files opened
• files are from readonly file system
• user reloads window
• we show all the files very early as part of restoring the workbench
• after a while the file system provider is resolved
• now we should redraw labels if the provider e.g. is readonly

The model layer is too late because most tabs that visually appear in the editor are not loaded until you click them.

[isidorn]

This makes sense.
I have pushed a change to drop all cache when a new file system provider is registered
@gjsjohnmurray can you please try vsocde insiders from Wednesday and let us know if the issue gets fixed for you. Thank you

[gjsjohnmurray]

@isidorn no, that hasn't fixed the issue for me. I just tested with the following using the MemFS example branch previously notified:

Version: 1.40.0-insider (user setup)
Commit: f5d3ffa6a0d655c87e1eb0e1e90773df58f7ff25
Date: 2019-10-23T05:25:50.857Z
Electron: 6.0.12
Chrome: 76.0.3809.146
Node.js: 12.4.0
V8: 7.6.303.31-electron.0
OS: Windows_NT x64 10.0.17134

[isidorn]

Thanks for letting us know, let me reopen and setup your reproducable steps

[isidorn]

1. I can reproduce this
2. It is a timing issue. When the title bar part asks for the title the textFileService is not yet aware of the model here

I am open for ideas. Maybe we need a new event when a new model is added to the textFileService.models that I should drop the cache. But that would not be enough, since on new model add the tabs title control would need to be refershed.

[bpasero]

@isidorn @gjsjohnmurray this should no longer be a problem in latest because for a couple of days now editor inputs have a isReadonly method that works even when no model is loaded yet by asking the file system provider for its capability:

vscode/src/vs/workbench/contrib/files/common/editors/fileEditorInput.ts

Line 229 in 4934a6f

return model?.isReadonly() || this.fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly);

Can you verify?

PS: I still wonder if we need to catch the case of a file system provider suddenly becoming readonly or not.

[gjsjohnmurray]

@bpasero verified using my https://github.com/gjsjohnmurray/vscode-extension-samples/tree/show-82964 branch. Thanks!

Version: 1.41.0-insider (user setup)
Commit: 4934a6f
Date: 2019-11-20T05:25:39.619Z
Electron: 6.1.4
Chrome: 76.0.3809.146
Node.js: 12.4.0
V8: 7.6.303.31-electron.0
OS: Windows_NT x64 10.0.17763

[isidorn]

Thanks for verifiying, adding verified label

[gjsjohnmurray]

PS: I still wonder if we need to catch the case of a file system provider suddenly becoming readonly or not.

@bpasero I'm unclear what you mean here. IIRC a FileSystemProvider can only declare itself readonly when it registers. I guess if it registered with readOnly: false there'd be nothing stopping it from later making all of its files report themselves as readonly.

[bpasero]

@gjsjohnmurray yeah nevermind, internally we allow changes to capabilities but I think that is just not surfaced as extension API.