-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Better Support for ReadOnly message on editors #13414
Conversation
6b1614f
to
3163217
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why, but there's a regression: Running the Toggle File System Readonly
command and opening a new file opens it as writable, whereas it should be opened as readonly.
examples/api-samples/src/browser/file-system/sample-file-system-capabilities.ts
Outdated
Show resolved
Hide resolved
@msujew, can you approve this PR as #13414 (comment) was resolved with 76ac7d9 please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good to me 👍
Also implements VS Code api for readOnly messages on FileSystemProvider fixes eclipse-theia#13353 contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <[email protected]>
76ac7d9
to
fb077fa
Compare
What it does
This Pull Request continues the work initiated with #13403 and it adds the support of the introduced API in VS Code as described in #13353. This API addition is the workspace.registerFileSystemProvider() with readonly option being also potentially a Markdown String and not only a boolean.
Also, it completes the file system API sample with an action to add and remove a read-only message.
Finally, it improves toggling the readOnly value for a given editor by retrieving the real status of the file, and not only the status of the file system provider. The file can be readonly because of specific permissions for example.
Note: it is currently hard to test the VS Code API as indicated there: #13353 (comment). I used the FS provider vscode sample, but I had same issue. I do not understand why URI has to be transformed to relative and absolute. Removing the lines as indicated in the comment #8167 (comment) and avoiding some absolute translation works for me, but there are probably reasons why this was done initially.
How to test
The behavior is similar to the one on VS Code: the readOnlyMessage has priority over the capability.
Follow-ups
Check the API thanks to this extension when the workspace update is working with other file systems:
zip vsix:
vscode-memfs-0.0.8.zip
src:
vscode-memfs-0.0.8-src.zip
Review checklist
Reminder for reviewers