[Insiders] Throwing error when a file is edited and saved from a diff…

… view (fix #123756)
microsoft · May 13, 2021 · 23dc403 · 23dc403
1 parent 659175f
commit 23dc403
Show file tree

Hide file tree

Showing 7 changed files with 19 additions and 13 deletions.
diff --git a/src/vs/platform/files/common/fileService.ts b/src/vs/platform/files/common/fileService.ts
@@ -234,7 +234,7 @@ export class FileService extends Disposable implements IFileService {
 			mtime: stat.mtime,
 			ctime: stat.ctime,
 			size: stat.size,
-			readonly: Boolean(stat.permissions ?? 0 & FilePermission.Readonly) ?? Boolean(provider.capabilities & FileSystemProviderCapabilities.Readonly),
+			readonly: Boolean(stat.permissions ?? 0 & FilePermission.Readonly) || Boolean(provider.capabilities & FileSystemProviderCapabilities.Readonly),
 			etag: etag({ mtime: stat.mtime, size: stat.size })
 		};
 

diff --git a/src/vs/workbench/contrib/files/common/explorerModel.ts b/src/vs/workbench/contrib/files/common/explorerModel.ts
@@ -30,7 +30,7 @@ export class ExplorerModel implements IDisposable {
 		fileService: IFileService
 	) {
 		const setRoots = () => this._roots = this.contextService.getWorkspace().folders
-			.map(folder => new ExplorerItem(folder.uri, fileService, undefined, true, false, folder.name));
+			.map(folder => new ExplorerItem(folder.uri, fileService, undefined, true, false, false, folder.name));
 		setRoots();
 
 		this._listener = this.contextService.onDidChangeWorkspaceFolders(() => {
@@ -89,6 +89,7 @@ export class ExplorerItem {
 		private _parent: ExplorerItem | undefined,
 		private _isDirectory?: boolean,
 		private _isSymbolicLink?: boolean,
+		private _readonly?: boolean,
 		private _name: string = basenameOrAuthority(resource),
 		private _mtime?: number,
 		private _unknown = false
@@ -124,7 +125,7 @@ export class ExplorerItem {
 	}
 
 	get isReadonly(): boolean {
-		return this.fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly);
+		return this._readonly || this.fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly);
 	}
 
 	get mtime(): number | undefined {
@@ -179,7 +180,7 @@ export class ExplorerItem {
 	}
 
 	static create(fileService: IFileService, raw: IFileStat, parent: ExplorerItem | undefined, resolveTo?: readonly URI[]): ExplorerItem {
-		const stat = new ExplorerItem(raw.resource, fileService, parent, raw.isDirectory, raw.isSymbolicLink, raw.name, raw.mtime, !raw.isFile && !raw.isDirectory);
+		const stat = new ExplorerItem(raw.resource, fileService, parent, raw.isDirectory, raw.isSymbolicLink, raw.readonly, raw.name, raw.mtime, !raw.isFile && !raw.isDirectory);
 
 		// Recursively add children if present
 		if (stat.isDirectory) {

diff --git a/src/vs/workbench/contrib/files/test/browser/explorerModel.test.ts b/src/vs/workbench/contrib/files/test/browser/explorerModel.test.ts
@@ -14,7 +14,7 @@ import { TestFileService } from 'vs/workbench/test/browser/workbenchTestServices
 
 const fileService = new TestFileService();
 function createStat(this: any, path: string, name: string, isFolder: boolean, hasChildren: boolean, size: number, mtime: number): ExplorerItem {
-	return new ExplorerItem(toResource.call(this, path), fileService, undefined, isFolder, false, name, mtime);
+	return new ExplorerItem(toResource.call(this, path), fileService, undefined, isFolder, false, false, name, mtime);
 }
 
 suite('Files - View Model', function () {
@@ -245,19 +245,19 @@ suite('Files - View Model', function () {
 	});
 
 	test('Merge Local with Disk', function () {
-		const merge1 = new ExplorerItem(URI.file(join('C:\\', '/path/to')), fileService, undefined, true, false, 'to', Date.now());
-		const merge2 = new ExplorerItem(URI.file(join('C:\\', '/path/to')), fileService, undefined, true, false, 'to', Date.now());
+		const merge1 = new ExplorerItem(URI.file(join('C:\\', '/path/to')), fileService, undefined, true, false, false, 'to', Date.now());
+		const merge2 = new ExplorerItem(URI.file(join('C:\\', '/path/to')), fileService, undefined, true, false, false, 'to', Date.now());
 
 		// Merge Properties
 		ExplorerItem.mergeLocalWithDisk(merge2, merge1);
 		assert.strictEqual(merge1.mtime, merge2.mtime);
 
 		// Merge Child when isDirectoryResolved=false is a no-op
-		merge2.addChild(new ExplorerItem(URI.file(join('C:\\', '/path/to/foo.html')), fileService, undefined, true, false, 'foo.html', Date.now()));
+		merge2.addChild(new ExplorerItem(URI.file(join('C:\\', '/path/to/foo.html')), fileService, undefined, true, false, false, 'foo.html', Date.now()));
 		ExplorerItem.mergeLocalWithDisk(merge2, merge1);
 
 		// Merge Child with isDirectoryResolved=true
-		const child = new ExplorerItem(URI.file(join('C:\\', '/path/to/foo.html')), fileService, undefined, true, false, 'foo.html', Date.now());
+		const child = new ExplorerItem(URI.file(join('C:\\', '/path/to/foo.html')), fileService, undefined, true, false, false, 'foo.html', Date.now());
 		merge2.removeChild(child);
 		merge2.addChild(child);
 		(<any>merge2)._isDirectoryResolved = true;

diff --git a/src/vs/workbench/contrib/files/test/browser/explorerView.test.ts b/src/vs/workbench/contrib/files/test/browser/explorerView.test.ts
@@ -19,7 +19,7 @@ const $ = dom.$;
 const fileService = new TestFileService();
 
 function createStat(this: any, path: string, name: string, isFolder: boolean, hasChildren: boolean, size: number, mtime: number, isSymLink = false, isUnknown = false): ExplorerItem {
-	return new ExplorerItem(toResource.call(this, path), fileService, undefined, isFolder, isSymLink, name, mtime, isUnknown);
+	return new ExplorerItem(toResource.call(this, path), fileService, undefined, isFolder, isSymLink, false, name, mtime, isUnknown);
 }
 
 suite('Files - ExplorerView', () => {

diff --git a/src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts b/src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts
@@ -465,7 +465,7 @@ export class SimpleNotebookEditorModel extends EditorModel implements INotebookE
 	}
 
 	isReadonly(): boolean {
-		return this._fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly);
+		return this._workingCopy?.isReadonly() || this._fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly);
 	}
 
 	revert(options?: IRevertOptions): Promise<void> {

diff --git a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts
@@ -1008,7 +1008,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
 	}
 
 	override isReadonly(): boolean {
-		return this.lastResolvedFileStat?.readonly ?? this.fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly);
+		return this.lastResolvedFileStat?.readonly || this.fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly);
 	}
 
 	override dispose(): void {

diff --git a/src/vs/workbench/services/workingCopy/common/fileWorkingCopy.ts b/src/vs/workbench/services/workingCopy/common/fileWorkingCopy.ts
@@ -207,6 +207,11 @@ export interface IFileWorkingCopy<T extends IFileWorkingCopyModel> extends IReso
 	 * Whether we have a resolved model or not.
 	 */
 	isResolved(): this is IResolvedFileWorkingCopy<T>;
+
+	/**
+	 * Whether the file working copy is readonly or not.
+	 */
+	isReadonly(): boolean;
 }
 
 export interface IResolvedFileWorkingCopy<T extends IFileWorkingCopyModel> extends IFileWorkingCopy<T> {
@@ -1235,7 +1240,7 @@ export class FileWorkingCopy<T extends IFileWorkingCopyModel> extends ResourceWo
 	}
 
 	isReadonly(): boolean {
-		return this.lastResolvedFileStat?.readonly ?? this.fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly);
+		return this.lastResolvedFileStat?.readonly || this.fileService.hasCapability(this.resource, FileSystemProviderCapabilities.Readonly);
 	}
 
 	private trace(msg: string): void {