Skip to content
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

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

rschnekenbu
Copy link
Contributor

@rschnekenbu rschnekenbu commented Feb 22, 2024

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

  1. In a standard workspace, create 2 text files, and make one only writable, the second with only read permissions.
  2. Use the various commands from API sample for File system:
  • Add a File System ReadonlyMessage for readonly
  • Remove File System ReadonlyMessage for readonly
  • Toggle File System Readonly
  1. Check if the lock symbol appears in the title, and also what is displayed in the editor when trying to edit when readonly, for both files. For example, when toggling readonly to false and no readonly message, the file with only read permissions shall still not be editable.

ReadOnlyMessage

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:

Review checklist

Reminder for reviewers

Copy link
Member

@msujew msujew left a 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.

packages/filesystem/src/browser/file-service.ts Outdated Show resolved Hide resolved
packages/filesystem/src/browser/file-service.ts Outdated Show resolved Hide resolved
packages/filesystem/src/common/files.ts Show resolved Hide resolved
packages/monaco/src/browser/monaco-editor-provider.ts Outdated Show resolved Hide resolved
packages/filesystem/src/browser/file-service.ts Outdated Show resolved Hide resolved
packages/filesystem/src/browser/file-resource.ts Outdated Show resolved Hide resolved
@rschnekenbu rschnekenbu requested review from msujew and removed request for tsmaeder February 26, 2024 15:40
@rschnekenbu
Copy link
Contributor Author

@msujew, can you approve this PR as #13414 (comment) was resolved with 76ac7d9 please?
That will be the last remaining task for VS code compat 1.86.

Copy link
Member

@msujew msujew left a 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 <rschnekenburger@eclipsesource.com>
@msujew msujew merged commit b96a84a into eclipse-theia:master Feb 28, 2024
14 checks passed
@jfaltermeier jfaltermeier added this to the 1.47.0 milestone Feb 29, 2024
This was referenced Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[vscode] workspace registerFileSystemProvider options can explain why FS is readonly with 1.86
3 participants