address review comments

eclipse-theia · Feb 26, 2024 · 76ac7d9 · 76ac7d9
1 parent 3163217
commit 76ac7d9
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 19 deletions.
diff --git a/examples/api-samples/src/browser/file-system/sample-file-system-capabilities.ts b/examples/api-samples/src/browser/file-system/sample-file-system-capabilities.ts
@@ -18,7 +18,7 @@ import { CommandContribution, CommandRegistry } from '@theia/core';
 import { inject, injectable, interfaces } from '@theia/core/shared/inversify';
 import { RemoteFileSystemProvider } from '@theia/filesystem/lib/common/remote-file-system-provider';
 import { FileSystemProviderCapabilities } from '@theia/filesystem/lib/common/files';
-import {  MarkdownStringImpl } from '@theia/core/lib/common/markdown-rendering';
+import { MarkdownStringImpl } from '@theia/core/lib/common/markdown-rendering';
 
 @injectable()
 export class SampleFileSystemCapabilities implements CommandContribution {

diff --git a/packages/filesystem/src/browser/file-resource.ts b/packages/filesystem/src/browser/file-resource.ts
@@ -106,11 +106,11 @@ export class FileResource implements Resource {
 
     protected async updateReadOnly(): Promise<void> {
         const oldReadOnly = this.options.readOnly;
-        const readOnlyMessage = this.fileService.readOnlyMessage(this.uri);
+        const readOnlyMessage = this.fileService.getReadOnlyMessage(this.uri);
         if (readOnlyMessage) {
             this.options.readOnly = readOnlyMessage;
         } else {
-            this.options.readOnly = await this.fileService.isReadOnly(this.uri) || this.fileService.hasCapability(this.uri, FileSystemProviderCapabilities.Readonly);
+            this.options.readOnly = this.fileService.hasCapability(this.uri, FileSystemProviderCapabilities.Readonly);
         }
         if (this.options.readOnly !== oldReadOnly) {
             this.updateSavingContentChanges();
@@ -345,10 +345,12 @@ export class FileResourceResolver implements ResourceResolver {
             throw new Error('The given uri is a directory: ' + this.labelProvider.getLongName(uri));
         }
 
-        const readOnlyMessage = this.fileService.readOnlyMessage(uri);
+        const readOnlyMessage = this.fileService.getReadOnlyMessage(uri);
+        const isFileSystemReadOnly = this.fileService.hasCapability(uri, FileSystemProviderCapabilities.Readonly);
+        const readOnly = readOnlyMessage ?? (isFileSystemReadOnly ? isFileSystemReadOnly : (stat?.isReadonly ?? false));
 
         return new FileResource(uri, this.fileService, {
-            readOnly: readOnlyMessage ?? stat?.isReadonly ?? false,
+            readOnly: readOnly,
             shouldOverwrite: () => this.shouldOverwrite(uri),
             shouldOpenAsText: error => this.shouldOpenAsText(uri, error)
         });

diff --git a/packages/filesystem/src/browser/file-service.ts b/packages/filesystem/src/browser/file-service.ts
@@ -377,8 +377,8 @@ export class FileService {
         providerDisposables.push(provider.onDidChangeFile(changes => this.onDidFilesChangeEmitter.fire(new FileChangesEvent(changes))));
         providerDisposables.push(provider.onFileWatchError(() => this.handleFileWatchError()));
         providerDisposables.push(provider.onDidChangeCapabilities(() => this.onDidChangeFileSystemProviderCapabilitiesEmitter.fire({ provider, scheme })));
-        if(ReadOnlyMessageFileSystemProvider.is(provider)) {
-            providerDisposables.push(provider.onDidChangeReadOnlyMessage(message => this.onDidChangeFileSystemProviderReadOnlyMessageEmitter.fire( { provider, scheme, message})));
+        if (ReadOnlyMessageFileSystemProvider.is(provider)) {
+            providerDisposables.push(provider.onDidChangeReadOnlyMessage(message => this.onDidChangeFileSystemProviderReadOnlyMessageEmitter.fire({ provider, scheme, message})));
         }
 
         return Disposable.create(() => {
@@ -429,23 +429,14 @@ export class FileService {
         return this.providers.has(resource.scheme);
     }
 
-    readOnlyMessage(resource: URI): MarkdownString | undefined {
+    getReadOnlyMessage(resource: URI): MarkdownString | undefined {
         const provider = this.providers.get(resource.scheme);
-        if(ReadOnlyMessageFileSystemProvider.is(provider)) {
+        if (ReadOnlyMessageFileSystemProvider.is(provider)) {
             return provider.readOnlyMessage;
         }
         return undefined;
     }
 
-    async isReadOnly(resource: URI): Promise<boolean> {
-        const exists = await this.exists(resource);
-        if(exists) {
-            const stat = await this.resolve(resource);
-            return stat?.isReadonly ?? false;
-        }
-        return false;
-    }
-
     /**
      * Tests if the service (i.e the {@link FileSystemProvider} registered for the given uri scheme) provides the given capability.
      * @param resource `URI` of the resource to test.

diff --git a/packages/filesystem/src/common/remote-file-system-provider.ts b/packages/filesystem/src/common/remote-file-system-provider.ts
@@ -413,6 +413,8 @@ export class FileSystemProviderServer implements RemoteFileSystemServer {
     async getReadOnlyMessage(): Promise<MarkdownString | undefined> {
         if (ReadOnlyMessageFileSystemProvider.is(this.provider)) {
             return this.provider.readOnlyMessage;
+        } else {
+            return undefined;
         }
     }
 

diff --git a/packages/monaco/src/browser/monaco-editor-provider.ts b/packages/monaco/src/browser/monaco-editor-provider.ts
@@ -211,7 +211,7 @@ export class MonacoEditorProvider {
         return editor;
     }
 
-    protected updateReadOnlyMessage(options: MonacoEditor.IOptions, readOnly: boolean | MarkdownString ): any {
+    protected updateReadOnlyMessage(options: MonacoEditor.IOptions, readOnly: boolean | MarkdownString ): void {
         options.readOnlyMessage = MarkdownString.is(readOnly) ? readOnly : undefined;
     }