Set editor readonly for files that are readonly on disk
adelphes opened this issue · 20 comments
A small gripe, but one which gets me every time.
I use vscode with some auto-generated source files which are marked on disk as read-only. Because the auto-generated files appear similar to the normal source files in my project, I start to edit them and only discover when I try to save that I'm in the wrong file.
As well as #17621, it would be really useful to have a configurable editor option that prevents any editing of read-only files. Any keystrokes, pasting, etc are simply ignored.
Based on the source control I use and the way I use it, this is currently the biggest hurdle for me in migrating to VS Code.
From my experience, both Visual Studio and Notepad++ just respect file status on disk when it comes to editing. Sublime does not, but it's API supports a set_read_only function on views. VS Code currently does not seem to have either of these.
Ideally, VS Code would have a similar API support for plugins to be able to control view edit status, as well as a settings configurable ability to just respect on-disk file status. I'm personally more concerned with the API aspect, but I see them both as being useful.
The editor has an option readOnly that would need to be exercised by the workbench => removing editor label.
I'd love to see this too. I raised #31410 and based on the responsed implemented a text content provider however it caused a bunch of issues (for ex. Code didn't know that my new scheme was actually files, so paths to breakpoints etc. get messed up) and I've had to revert it. Now my users can edit package sources (equiv of node_modules) which is really not ideal.
I'd love to see this as well. We are using source control using locks. Being able to edit read only files causes only confusion.
Any updates/progress on this? I just ran into it and it's becoming an issue.
@alexandrudima Hi - is there any chance the fix you created in #37496 could be easily applied to this issue. Displaying your "Cannot edit" hint when trying to edit read-only source files would be great way to solve this.
@adelphes I think what's needed is for FileSystemProvider to be able to indicate readonly as part of what it returns from its stat method. I created #73122 to request this. Starting at 1.34 even access to the local filesystem is implemented as a FileSystemProvider, so its stat method would be able to check file permissions (and the readonly file attribute on Windows) and return the appropriate readonly value.
Could this feature also allow your workbench configuration to mark entire folders as read only?
Wait, what? The editor should have an option to prevent accidental editing of read-only files. For people who are stuck in older version control systems, the current behavior leads to easy accidents, such as:
- Accidentally click a key or paste into an editor window that contains a file that is set as read-only on disk.
- Autosave runs, and prompts a dialog in the lower right.
- User (me, or maybe you!), in a rush, absent-mindedly (and incorrectly--we are human and prone to mistakes) clicks "overwrite".
- perforce is now broken hard, because it tracks state via file read-write attributes, and will now fail to update that file. This is a classic, easily overlooked problem that many people have lost much time over, especially in large projects.
If you disagree and feel that this issue is crucial: We are happy to listen and to reconsider.
I disagree and feel that this issue is crucial.
Yes. I think it's time to get this re-opened and into the queue.
At the very least, a real live person from the core team should comment here, please.
While working on #161715, I made a patch that propagates stat.mode to set FilePermission.Readonly so textEditorModel.isReadonly() can dynamically sync with FileSystem.Readonly)
That patch explicitly addresses the issue of "files which are marked on disk as read-only" being non-editable.
(which is technically different from #4783, which is about files that are disk-writeable, but intended to be non-editable)
(although it relies on code from #161716 to set model.isReadOnly() and present the 'lock' icon...)
Duplicate of #4873
I disagree that this is the same as #4873 - reading through the comments makes it clear that there are two different scenarios.
4873 refers to a dynamic option which allows a developer to mark files or editors as read-only.
This issue is focused on the read-only state of files in the file system and automatically preventing editing of those files.
Since this is reopened:
Would you like me to make a standalone PR for the patch mentioned above?
It simply applies the 'readonly' bit returned in the file-stat method to set the Permissions.readOnly flag.
(which is already tested/recognized in [textFileEditorModel.isReadonly()](https://github.com/microsoft/vscode/blob/main/src/vs/workbench/services/textfile/common/textFileEditorModel.ts#:~:text=override%20isReadonly(,%7D)
this.fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly));
I know you have concerns that other Editor/Models may also be interested in the readonly status;
But in this case, that only means other places should be checking FileSystemProviderCapabilities.Readonly
Fixing it now sounds good, yes. There is no need to wait for other, more exotic features that happen to be related.
And I see a much higher value-to-effort ratio for fixing this one, too. This is something that is obvious and required; most other editors already do the right thing for read-only files.
There's the PR (one-line + import)
Ok... diskFileService.test.ts is failing.
I see the problem:
There are two places where vscode tests: (stat.permissions ?? 0) & FilePermission.Readonly
and expect that to be FALSE, relying on the assumption that diskFileSystemProvider does NOT set stat.permissions.
[so setting permissions from the actual file stat() breaks the 'unlock and over-write' test]
That is: throwIfFileIsReadonly() has legacy expectation to NOT throw if the file is, in fact, Readonly. (to allow overwrite)
[TBD if fileService.stat() is allowed to return actual Readonly]
That is a fundamental problem...
Was diskFS.stat() intentionally hacked removed permissions: to enable the unlock/sudo-write feature?
MainThreadFileSystem, EditSessionsFileSystemProvider & DebugMemoryFileSystemProvider have $stat() methods with:
permissions: stat.readonly ? FilePermission.Readonly : undefined,
I can imagine ways to do further hacks to ReadonlyHelper to workaround this,
But wonder if it is preferable to recode so that unlock/sudo-write interacts with Permissions.Readonly differently?
Note: Discussion is here, but the PR is merged with #161716,
since this feature interacts with the 'settings' and 'isReadonly()'
Resolved as follows:
Because throwIfFileIsReadonly() needs to not throw for DiskFileSystemProvider [to allow unlock/sudo]
the legacy/historical solution was to remove 'permissions:' from its reported IStat
As a result, the actual file-stat readonly-ness could never be used to signal non-editibility.
To preserve the 'allow DiskFileSystem to bypass throwIfFileIsReadonly()' feature,
while also including FilePermissions.Readonly, we take the simple approach to mark DiskFileSystemProvider with a special feature/capability/permission: IgnoreReadonly (as always I'll defer to the authors for better naming)
That is: instead of removing the standard, righteous Readonly permission indicator for the DiskFSP feature,
we instead add a special permission indicator for DiskFSP (& others that may offer the unlock/chmod/sudo feature)
This fix is 4 lines:
1 & 2: define the new bit in FilePermissions (in files.ts & vscode.d.ts)
3: set the bit in DiskFileSystemProvider.stat()
4: check the bit in throwIfFileIsReadonly()
This provably gives that same results as before, since the place that relied on a null FilePermission now detects the IgnoreReadonly bit.
This landed in our insiders behind a new setting files.readonlyFromPermissions. You can give our preview releases a try from: https://code.visualstudio.com/insiders/