From 434bbbde983bb1d2f054dd5d6b2ad4ff05df2771 Mon Sep 17 00:00:00 2001 From: isidor Date: Thu, 17 Dec 2020 20:01:00 +0100 Subject: [PATCH 1/8] workingCopyService: take options alongside each argument --- .../mainThreadFileSystemEventService.ts | 8 +- .../contrib/bulkEdit/browser/bulkFileEdits.ts | 6 +- .../textfile/browser/textFileService.ts | 2 +- .../workingCopyFileOperationParticipant.ts | 7 +- .../common/workingCopyFileService.ts | 90 ++++++++++++------- .../browser/workingCopyFileService.test.ts | 25 +++--- .../browser/api/mainThreadEditors.test.ts | 16 ++-- .../test/common/workbenchTestServices.ts | 8 +- 8 files changed, 96 insertions(+), 66 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts b/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts index cb72e3adc86..b7706f0dc96 100644 --- a/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts +++ b/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts @@ -10,7 +10,7 @@ import { ExtHostContext, FileSystemEvents, IExtHostContext } from '../common/ext import { localize } from 'vs/nls'; import { Extensions, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry'; import { Registry } from 'vs/platform/registry/common/platform'; -import { IWorkingCopyFileOperationParticipant, IWorkingCopyFileService, SourceTargetPair } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; +import { IWorkingCopyFileOperationParticipant, IWorkingCopyFileService, SourceTargetPairWithOptions } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; import { reviveWorkspaceEditDto2 } from 'vs/workbench/api/browser/mainThreadEditors'; import { IBulkEditService } from 'vs/editor/browser/services/bulkEditService'; import { IProgressService, ProgressLocation } from 'vs/platform/progress/common/progress'; @@ -74,8 +74,8 @@ export class MainThreadFileSystemEventService { const fileOperationParticipant = new class implements IWorkingCopyFileOperationParticipant { - async participate(files: SourceTargetPair[], operation: FileOperation, undoRedoGroupId: number | undefined, isUndoing: boolean | undefined, timeout: number, token: CancellationToken) { - if (isUndoing) { + async participate(files: SourceTargetPairWithOptions[], operation: FileOperation, timeout: number, token: CancellationToken) { + if (files.some(f => f.options?.isUndoing)) { return; } @@ -169,6 +169,8 @@ export class MainThreadFileSystemEventService { logService.info('[onWill-handler] applying additional workspace edit from extensions', data.extensionNames); + // TODO@isidor undo redo group is per resource, this is false generalization + const undoRedoGroupId = files.length > 0 ? files[0].options?.undoRedoGroupId : undefined; await bulkEditService.apply( reviveWorkspaceEditDto2(data.edit), { undoRedoGroupId, showPreview } diff --git a/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts b/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts index f4ac30ca099..fc9abffd80b 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts @@ -57,7 +57,7 @@ class RenameOperation implements IFileOperation { return new Noop(); // not overwriting, but ignoring, and the target file exists } - await this._workingCopyFileService.move([{ source: this.oldUri, target: this.newUri }], { overwrite: this.options.overwrite, ...this.undoRedoInfo }, token); + await this._workingCopyFileService.move([{ file: { source: this.oldUri, target: this.newUri }, options: { overwrite: this.options.overwrite, ...this.undoRedoInfo } }], token); return new RenameOperation(this.oldUri, this.newUri, this.options, { isUndoing: true }, this._workingCopyFileService, this._fileService); } @@ -93,7 +93,7 @@ class CopyOperation implements IFileOperation { return new Noop(); // not overwriting, but ignoring, and the target file exists } - const stat = await this._workingCopyFileService.copy([{ source: this.oldUri, target: this.newUri }], { overwrite: this.options.overwrite, ...this.undoRedoInfo }, token); + const stat = await this._workingCopyFileService.copy([{ file: { source: this.oldUri, target: this.newUri }, options: { overwrite: this.options.overwrite, ...this.undoRedoInfo } }], token); const folder = this.options.folder || (stat.length === 1 && stat[0].isDirectory); return this._instaService.createInstance(DeleteOperation, this.newUri, { recursive: true, folder, ...this.options }, { isUndoing: true }, false); } @@ -175,7 +175,7 @@ class DeleteOperation implements IFileOperation { } const useTrash = !this.options.skipTrashBin && this._fileService.hasCapability(this.oldUri, FileSystemProviderCapabilities.Trash) && this._configurationService.getValue('files.enableTrash'); - await this._workingCopyFileService.delete([this.oldUri], { useTrash, recursive: this.options.recursive, ...this.undoRedoInfo }, token); + await this._workingCopyFileService.delete([{ resource: this.oldUri, options: { useTrash, recursive: this.options.recursive, ...this.undoRedoInfo } }], token); if (typeof this.options.maxSize === 'number' && fileContent && (fileContent?.size > this.options.maxSize)) { return new Noop(); diff --git a/src/vs/workbench/services/textfile/browser/textFileService.ts b/src/vs/workbench/services/textfile/browser/textFileService.ts index 4ecca900e14..13ddf1513c7 100644 --- a/src/vs/workbench/services/textfile/browser/textFileService.ts +++ b/src/vs/workbench/services/textfile/browser/textFileService.ts @@ -243,7 +243,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex // However, this will only work if the source exists // and is not orphaned, so we need to check that too. if (this.fileService.canHandleResource(source) && this.uriIdentityService.extUri.isEqual(source, target) && (await this.fileService.exists(source))) { - await this.workingCopyFileService.move([{ source, target }]); + await this.workingCopyFileService.move([{ file: { source, target } }]); return this.save(target, options); } diff --git a/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts b/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts index 31fda6475fe..d2a912b8352 100644 --- a/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts +++ b/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts @@ -6,8 +6,7 @@ import { CancellationToken } from 'vs/base/common/cancellation'; import { ILogService } from 'vs/platform/log/common/log'; import { IDisposable, Disposable, toDisposable } from 'vs/base/common/lifecycle'; -import { IWorkingCopyFileOperationParticipant } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; -import { URI } from 'vs/base/common/uri'; +import { IWorkingCopyFileOperationParticipant, SourceTargetPairWithOptions } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; import { FileOperation } from 'vs/platform/files/common/files'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { LinkedList } from 'vs/base/common/linkedList'; @@ -28,7 +27,7 @@ export class WorkingCopyFileOperationParticipant extends Disposable { return toDisposable(() => remove()); } - async participate(files: { source?: URI, target: URI }[], operation: FileOperation, undoRedoGroupId: number | undefined, isUndoing: boolean | undefined, token: CancellationToken | undefined): Promise { + async participate(files: SourceTargetPairWithOptions[], operation: FileOperation, token: CancellationToken | undefined): Promise { const timeout = this.configurationService.getValue('files.participants.timeout'); if (timeout <= 0) { return; // disabled @@ -37,7 +36,7 @@ export class WorkingCopyFileOperationParticipant extends Disposable { // For each participant for (const participant of this.participants) { try { - await participant.participate(files, operation, undoRedoGroupId, isUndoing, timeout, token ?? CancellationToken.None); + await participant.participate(files, operation, timeout, token ?? CancellationToken.None); } catch (err) { this.logService.warn(err); } diff --git a/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts b/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts index 922c722741a..54a91d5d2f6 100644 --- a/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts +++ b/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts @@ -31,6 +31,13 @@ export interface SourceTargetPair { readonly target: URI } +export interface SourceTargetPairWithOptions extends SourceTargetPair { + options?: { + undoRedoGroupId?: number; + isUndoing?: boolean + } +} + export interface WorkingCopyFileEvent extends IWaitUntil { /** @@ -57,15 +64,34 @@ export interface IWorkingCopyFileOperationParticipant { * change the working copies before they are being saved to disk. */ participate( - files: SourceTargetPair[], + files: SourceTargetPairWithOptions[], operation: FileOperation, - undoRedoGroupId: number | undefined, - isUndoing: boolean | undefined, timeout: number, token: CancellationToken ): Promise; } +export interface IDeleteOperation { + resource: URI; + options?: { + useTrash?: boolean; + recursive?: boolean; + undoRedoGroupId?: number; + isUndoing?: boolean + }; +} + +export interface IMoveOperation { + file: Required; + options?: { + overwrite?: boolean; + undoRedoGroupId?: number; + isUndoing?: boolean + } +} + +export interface ICopyOperation extends IMoveOperation { } + /** * Returns the working copies for a given resource. */ @@ -150,7 +176,7 @@ export interface IWorkingCopyFileService { * Working copy owners can listen to the `onWillRunWorkingCopyFileOperation` and * `onDidRunWorkingCopyFileOperation` events to participate. */ - move(files: Required[], options?: { overwrite?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise; + move(operations: IMoveOperation[], token?: CancellationToken): Promise; /** * Will copy working copies matching the provided resources and corresponding children @@ -159,7 +185,7 @@ export interface IWorkingCopyFileService { * Working copy owners can listen to the `onWillRunWorkingCopyFileOperation` and * `onDidRunWorkingCopyFileOperation` events to participate. */ - copy(files: Required[], options?: { overwrite?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise; + copy(operations: ICopyOperation[], token?: CancellationToken): Promise; /** * Will delete working copies matching the provided resources and children @@ -168,7 +194,7 @@ export interface IWorkingCopyFileService { * Working copy owners can listen to the `onWillRunWorkingCopyFileOperation` and * `onDidRunWorkingCopyFileOperation` events to participate. */ - delete(resources: URI[], options?: { useTrash?: boolean, recursive?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise; + delete(operations: IDeleteOperation[], token?: CancellationToken): Promise; //#endregion @@ -256,7 +282,7 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi } // file operation participant - await this.runFileOperationParticipants([{ target: resource }], FileOperation.CREATE, options?.undoRedoGroupId, options?.isUndoing, token); + await this.runFileOperationParticipants([{ target: resource, options: { isUndoing: options?.isUndoing, undoRedoGroupId: options?.undoRedoGroupId } }], FileOperation.CREATE, token); // before events const event = { correlationId: this.correlationIds++, operation: FileOperation.CREATE, files: [{ target: resource }] }; @@ -284,36 +310,38 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi return stat; } - async move(files: Required[], options?: { overwrite?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise { - return this.doMoveOrCopy(files, true, options, token); + async move(operations: IMoveOperation[], token?: CancellationToken): Promise { + return this.doMoveOrCopy(operations, true, token); } - async copy(files: Required[], options?: { overwrite?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise { - return this.doMoveOrCopy(files, false, options, token); + async copy(operations: ICopyOperation[], token?: CancellationToken): Promise { + return this.doMoveOrCopy(operations, false, token); } - private async doMoveOrCopy(files: Required[], move: boolean, options?: { overwrite?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise { - const overwrite = options?.overwrite; + private async doMoveOrCopy(operations: IMoveOperation[] | ICopyOperation[], move: boolean, token?: CancellationToken): Promise { const stats: IFileStatWithMetadata[] = []; // validate move/copy operation before starting - for (const { source, target } of files) { - const validateMoveOrCopy = await (move ? this.fileService.canMove(source, target, overwrite) : this.fileService.canCopy(source, target, overwrite)); + for (const operation of operations) { + const { source, target } = operation.file; + const validateMoveOrCopy = await (move ? this.fileService.canMove(source, target, operation.options?.overwrite) : this.fileService.canCopy(source, target, operation.options?.overwrite)); if (validateMoveOrCopy instanceof Error) { throw validateMoveOrCopy; } } // file operation participant - await this.runFileOperationParticipants(files, move ? FileOperation.MOVE : FileOperation.COPY, options?.undoRedoGroupId, options?.isUndoing, token); + const files = operations.map(o => o.file); + // TODO@isidor should the options be passed here + await this.runFileOperationParticipants(files, move ? FileOperation.MOVE : FileOperation.COPY, token); // before event const event = { correlationId: this.correlationIds++, operation: move ? FileOperation.MOVE : FileOperation.COPY, files }; await this._onWillRunWorkingCopyFileOperation.fireAsync(event, CancellationToken.None); try { - for (const { source, target } of files) { - + for (const operation of operations) { + const { source, target } = operation.file; // if source and target are not equal, handle dirty working copies // depending on the operation: // - move: revert both source and target (if any) @@ -325,9 +353,9 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi // now we can rename the source to target via file operation if (move) { - stats.push(await this.fileService.move(source, target, overwrite)); + stats.push(await this.fileService.move(source, target, operation.options?.overwrite)); } else { - stats.push(await this.fileService.copy(source, target, overwrite)); + stats.push(await this.fileService.copy(source, target, operation.options?.overwrite)); } } } catch (error) { @@ -344,19 +372,19 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi return stats; } - async delete(resources: URI[], options?: { useTrash?: boolean, recursive?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise { + async delete(operations: IDeleteOperation[], token?: CancellationToken): Promise { // validate delete operation before starting - for (const resource of resources) { - const validateDelete = await this.fileService.canDelete(resource, options); + for (const operation of operations) { + const validateDelete = await this.fileService.canDelete(operation.resource, operation.options); if (validateDelete instanceof Error) { throw validateDelete; } } // file operation participant - const files = resources.map(target => ({ target })); - await this.runFileOperationParticipants(files, FileOperation.DELETE, options?.undoRedoGroupId, options?.isUndoing, token); + const files = operations.map(operation => ({ target: operation.resource, options: operation.options })); + await this.runFileOperationParticipants(files, FileOperation.DELETE, token); // before events const event = { correlationId: this.correlationIds++, operation: FileOperation.DELETE, files }; @@ -365,15 +393,15 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi // check for any existing dirty working copies for the resource // and do a soft revert before deleting to be able to close // any opened editor with these working copies - for (const resource of resources) { - const dirtyWorkingCopies = this.getDirty(resource); + for (const operation of operations) { + const dirtyWorkingCopies = this.getDirty(operation.resource); await Promise.all(dirtyWorkingCopies.map(dirtyWorkingCopy => dirtyWorkingCopy.revert({ soft: true }))); } // now actually delete from disk try { - for (const resource of resources) { - await this.fileService.del(resource, options); + for (const operation of operations) { + await this.fileService.del(operation.resource, operation.options); } } catch (error) { @@ -398,8 +426,8 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi return this.fileOperationParticipants.addFileOperationParticipant(participant); } - private runFileOperationParticipants(files: SourceTargetPair[], operation: FileOperation, undoRedoGroupId: number | undefined, isUndoing: boolean | undefined, token: CancellationToken | undefined): Promise { - return this.fileOperationParticipants.participate(files, operation, undoRedoGroupId, isUndoing, token); + private runFileOperationParticipants(files: SourceTargetPairWithOptions[], operation: FileOperation, token: CancellationToken | undefined): Promise { + return this.fileOperationParticipants.participate(files, operation, token); } //#endregion diff --git a/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts b/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts index 7c0a7688690..238b49c2c5a 100644 --- a/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts +++ b/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts @@ -13,6 +13,7 @@ import { URI } from 'vs/base/common/uri'; import { FileOperation } from 'vs/platform/files/common/files'; import { TestWorkingCopy } from 'vs/workbench/test/common/workbenchTestServices'; import { VSBuffer } from 'vs/base/common/buffer'; +import { ICopyOperation } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; suite('WorkingCopyFileService', () => { @@ -52,7 +53,7 @@ suite('WorkingCopyFileService', () => { let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8', undefined); (accessor.textFileService.files).add(sourceModel.resource, sourceModel); - const eventCounter = await testEventsMoveOrCopy([{ source: sourceModel.resource, target: sourceModel.resource }], true); + const eventCounter = await testEventsMoveOrCopy([{ file: { source: sourceModel.resource, target: sourceModel.resource }, options: { overwrite: true } }], true); sourceModel.dispose(); assert.equal(eventCounter, 3); @@ -67,8 +68,8 @@ suite('WorkingCopyFileService', () => { (accessor.textFileService.files).add(targetModel2.resource, targetModel2); const eventCounter = await testEventsMoveOrCopy([ - { source: sourceModel1.resource, target: sourceModel1.resource }, - { source: sourceModel2.resource, target: targetModel2.resource } + { file: { source: sourceModel1.resource, target: sourceModel1.resource }, options: { overwrite: true } }, + { file: { source: sourceModel2.resource, target: targetModel2.resource }, options: { overwrite: true } } ], true); sourceModel1.dispose(); @@ -96,7 +97,7 @@ suite('WorkingCopyFileService', () => { let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8', undefined); (accessor.textFileService.files).add(sourceModel.resource, sourceModel); - const eventCounter = await testEventsMoveOrCopy([{ source: sourceModel.resource, target: sourceModel.resource }]); + const eventCounter = await testEventsMoveOrCopy([{ file: { source: sourceModel.resource, target: sourceModel.resource }, options: { overwrite: true } }]); sourceModel.dispose(); assert.equal(eventCounter, 3); @@ -111,8 +112,8 @@ suite('WorkingCopyFileService', () => { (accessor.textFileService.files).add(targetModel2.resource, targetModel2); const eventCounter = await testEventsMoveOrCopy([ - { source: sourceModel1.resource, target: sourceModel1.resource }, - { source: sourceModel2.resource, target: targetModel2.resource } + { file: { source: sourceModel1.resource, target: sourceModel1.resource }, options: { overwrite: true } }, + { file: { source: sourceModel2.resource, target: targetModel2.resource }, options: { overwrite: true } } ]); sourceModel1.dispose(); @@ -232,7 +233,7 @@ suite('WorkingCopyFileService', () => { listener2.dispose(); }); - async function testEventsMoveOrCopy(files: { source: URI, target: URI }[], move?: boolean): Promise { + async function testEventsMoveOrCopy(files: ICopyOperation[], move?: boolean): Promise { let eventCounter = 0; const participant = accessor.workingCopyFileService.addFileOperationParticipant({ @@ -250,9 +251,9 @@ suite('WorkingCopyFileService', () => { }); if (move) { - await accessor.workingCopyFileService.move(files, { overwrite: true }); + await accessor.workingCopyFileService.move(files); } else { - await accessor.workingCopyFileService.copy(files, { overwrite: true }); + await accessor.workingCopyFileService.copy(files); } participant.dispose(); @@ -330,9 +331,9 @@ suite('WorkingCopyFileService', () => { }); if (move) { - await accessor.workingCopyFileService.move(models.map(model => ({ source: model.sourceModel.resource, target: model.targetModel.resource })), { overwrite: true }); + await accessor.workingCopyFileService.move(models.map(model => ({ file: { source: model.sourceModel.resource, target: model.targetModel.resource }, options: { overwrite: true } }))); } else { - await accessor.workingCopyFileService.copy(models.map(model => ({ source: model.sourceModel.resource, target: model.targetModel.resource })), { overwrite: true }); + await accessor.workingCopyFileService.copy(models.map(model => ({ file: { source: model.sourceModel.resource, target: model.targetModel.resource }, options: { overwrite: true } }))); } for (let i = 0; i < models.length; i++) { @@ -406,7 +407,7 @@ suite('WorkingCopyFileService', () => { eventCounter++; }); - await accessor.workingCopyFileService.delete(models.map(m => m.resource)); + await accessor.workingCopyFileService.delete(models.map(m => ({ resource: m.resource }))); for (const model of models) { assert.ok(!accessor.workingCopyService.isDirty(model.resource)); model.dispose(); diff --git a/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts b/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts index b413ec9fa5d..8ce5268be39 100644 --- a/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts +++ b/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts @@ -40,7 +40,7 @@ import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; import { ILabelService } from 'vs/platform/label/common/label'; -import { IWorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; +import { IWorkingCopyFileService, IMoveOperation, IDeleteOperation, ICopyOperation } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; import { UndoRedoService } from 'vs/platform/undoRedo/common/undoRedoService'; import { TestDialogService } from 'vs/platform/dialogs/test/common/testDialogService'; import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; @@ -107,19 +107,19 @@ suite('MainThreadEditors', () => { createdResources.add(resource); return Promise.resolve(Object.create(null)); } - move(files: { source: URI, target: URI }[]) { - const { source, target } = files[0]; + move(operations: IMoveOperation[]) { + const { source, target } = operations[0].file; movedResources.set(source, target); return Promise.resolve(Object.create(null)); } - copy(files: { source: URI, target: URI }[]) { - const { source, target } = files[0]; + copy(operations: ICopyOperation[]) { + const { source, target } = operations[0].file; copiedResources.set(source, target); return Promise.resolve(Object.create(null)); } - delete(resources: URI[]) { - for (const resource of resources) { - deletedResources.add(resource); + delete(operations: IDeleteOperation[]) { + for (const operation of operations) { + deletedResources.add(operation.resource); } return Promise.resolve(undefined); } diff --git a/src/vs/workbench/test/common/workbenchTestServices.ts b/src/vs/workbench/test/common/workbenchTestServices.ts index 08210ef22ab..63502cf7697 100644 --- a/src/vs/workbench/test/common/workbenchTestServices.ts +++ b/src/vs/workbench/test/common/workbenchTestServices.ts @@ -16,7 +16,7 @@ import { isLinux, isMacintosh } from 'vs/base/common/platform'; import { InMemoryStorageService, WillSaveStateReason } from 'vs/platform/storage/common/storage'; import { WorkingCopyService, IWorkingCopy, IWorkingCopyBackup, WorkingCopyCapabilities } from 'vs/workbench/services/workingCopy/common/workingCopyService'; import { NullExtensionService } from 'vs/workbench/services/extensions/common/extensions'; -import { IWorkingCopyFileService, IWorkingCopyFileOperationParticipant, WorkingCopyFileEvent } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; +import { IWorkingCopyFileService, IWorkingCopyFileOperationParticipant, WorkingCopyFileEvent, IDeleteOperation, ICopyOperation, IMoveOperation } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; import { IDisposable, Disposable } from 'vs/base/common/lifecycle'; import { IFileStatWithMetadata } from 'vs/platform/files/common/files'; import { VSBuffer, VSBufferReadable, VSBufferReadableStream } from 'vs/base/common/buffer'; @@ -196,7 +196,7 @@ export class TestWorkingCopyFileService implements IWorkingCopyFileService { addFileOperationParticipant(participant: IWorkingCopyFileOperationParticipant): IDisposable { return Disposable.None; } - async delete(resources: URI[], options?: { useTrash?: boolean | undefined; recursive?: boolean | undefined; } | undefined): Promise { } + async delete(operations: IDeleteOperation[], token?: CancellationToken): Promise { } registerWorkingCopyProvider(provider: (resourceOrFolder: URI) => IWorkingCopy[]): IDisposable { return Disposable.None; } @@ -205,9 +205,9 @@ export class TestWorkingCopyFileService implements IWorkingCopyFileService { create(resource: URI, contents?: VSBuffer | VSBufferReadable | VSBufferReadableStream, options?: { overwrite?: boolean | undefined; } | undefined): Promise { throw new Error('Method not implemented.'); } createFolder(resource: URI): Promise { throw new Error('Method not implemented.'); } - move(files: { source: URI; target: URI; }[], options?: { overwrite?: boolean }): Promise { throw new Error('Method not implemented.'); } + move(operations: IMoveOperation[]): Promise { throw new Error('Method not implemented.'); } - copy(files: { source: URI; target: URI; }[], options?: { overwrite?: boolean }): Promise { throw new Error('Method not implemented.'); } + copy(operations: ICopyOperation[], token?: CancellationToken): Promise { throw new Error('Method not implemented.'); } } export function mock(): Ctor { From 5d6f7a65d38bd14a2c80094cc0531fec07f15d9e Mon Sep 17 00:00:00 2001 From: isidor Date: Fri, 18 Dec 2020 11:13:32 +0100 Subject: [PATCH 2/8] Use IFileOperationUndoRedoInfo --- .../mainThreadFileSystemEventService.ts | 10 +-- .../contrib/bulkEdit/browser/bulkFileEdits.ts | 13 +--- .../workingCopyFileOperationParticipant.ts | 6 +- .../common/workingCopyFileService.ts | 74 ++++++++----------- .../browser/workingCopyFileService.test.ts | 12 +-- .../test/common/workbenchTestServices.ts | 8 +- 6 files changed, 52 insertions(+), 71 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts b/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts index b7706f0dc96..7bf3cb81904 100644 --- a/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts +++ b/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts @@ -10,7 +10,7 @@ import { ExtHostContext, FileSystemEvents, IExtHostContext } from '../common/ext import { localize } from 'vs/nls'; import { Extensions, IConfigurationRegistry } from 'vs/platform/configuration/common/configurationRegistry'; import { Registry } from 'vs/platform/registry/common/platform'; -import { IWorkingCopyFileOperationParticipant, IWorkingCopyFileService, SourceTargetPairWithOptions } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; +import { IWorkingCopyFileOperationParticipant, IWorkingCopyFileService, SourceTargetPair, IFileOperationUndoRedoInfo } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; import { reviveWorkspaceEditDto2 } from 'vs/workbench/api/browser/mainThreadEditors'; import { IBulkEditService } from 'vs/editor/browser/services/bulkEditService'; import { IProgressService, ProgressLocation } from 'vs/platform/progress/common/progress'; @@ -74,8 +74,8 @@ export class MainThreadFileSystemEventService { const fileOperationParticipant = new class implements IWorkingCopyFileOperationParticipant { - async participate(files: SourceTargetPairWithOptions[], operation: FileOperation, timeout: number, token: CancellationToken) { - if (files.some(f => f.options?.isUndoing)) { + async participate(files: SourceTargetPair[], operation: FileOperation, undoInfo: IFileOperationUndoRedoInfo, timeout: number, token: CancellationToken) { + if (undoInfo.isUndoing) { return; } @@ -169,11 +169,9 @@ export class MainThreadFileSystemEventService { logService.info('[onWill-handler] applying additional workspace edit from extensions', data.extensionNames); - // TODO@isidor undo redo group is per resource, this is false generalization - const undoRedoGroupId = files.length > 0 ? files[0].options?.undoRedoGroupId : undefined; await bulkEditService.apply( reviveWorkspaceEditDto2(data.edit), - { undoRedoGroupId, showPreview } + { undoRedoGroupId: undoInfo.undoRedoGroupId, showPreview } ); } diff --git a/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts b/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts index fc9abffd80b..04ea382ea8a 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts @@ -8,7 +8,7 @@ import { WorkspaceFileEditOptions } from 'vs/editor/common/modes'; import { IFileService, FileSystemProviderCapabilities, IFileContent } from 'vs/platform/files/common/files'; import { IProgress } from 'vs/platform/progress/common/progress'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { IWorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; +import { IWorkingCopyFileService, IFileOperationUndoRedoInfo } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; import { IWorkspaceUndoRedoElement, UndoRedoElementType, IUndoRedoService, UndoRedoGroup, UndoRedoSource } from 'vs/platform/undoRedo/common/undoRedo'; import { URI } from 'vs/base/common/uri'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; @@ -18,11 +18,6 @@ import { ResourceFileEdit } from 'vs/editor/browser/services/bulkEditService'; import * as resources from 'vs/base/common/resources'; import { CancellationToken } from 'vs/base/common/cancellation'; -interface IFileOperationUndoRedoInfo { - undoRedoGroupId?: number; - isUndoing?: boolean; -} - interface IFileOperation { uris: URI[]; perform(token: CancellationToken): Promise; @@ -57,7 +52,7 @@ class RenameOperation implements IFileOperation { return new Noop(); // not overwriting, but ignoring, and the target file exists } - await this._workingCopyFileService.move([{ file: { source: this.oldUri, target: this.newUri }, options: { overwrite: this.options.overwrite, ...this.undoRedoInfo } }], token); + await this._workingCopyFileService.move([{ file: { source: this.oldUri, target: this.newUri }, overwrite: this.options.overwrite }], this.undoRedoInfo, token); return new RenameOperation(this.oldUri, this.newUri, this.options, { isUndoing: true }, this._workingCopyFileService, this._fileService); } @@ -93,7 +88,7 @@ class CopyOperation implements IFileOperation { return new Noop(); // not overwriting, but ignoring, and the target file exists } - const stat = await this._workingCopyFileService.copy([{ file: { source: this.oldUri, target: this.newUri }, options: { overwrite: this.options.overwrite, ...this.undoRedoInfo } }], token); + const stat = await this._workingCopyFileService.copy([{ file: { source: this.oldUri, target: this.newUri }, overwrite: this.options.overwrite }], this.undoRedoInfo, token); const folder = this.options.folder || (stat.length === 1 && stat[0].isDirectory); return this._instaService.createInstance(DeleteOperation, this.newUri, { recursive: true, folder, ...this.options }, { isUndoing: true }, false); } @@ -175,7 +170,7 @@ class DeleteOperation implements IFileOperation { } const useTrash = !this.options.skipTrashBin && this._fileService.hasCapability(this.oldUri, FileSystemProviderCapabilities.Trash) && this._configurationService.getValue('files.enableTrash'); - await this._workingCopyFileService.delete([{ resource: this.oldUri, options: { useTrash, recursive: this.options.recursive, ...this.undoRedoInfo } }], token); + await this._workingCopyFileService.delete([{ resource: this.oldUri, useTrash, recursive: this.options.recursive }], this.undoRedoInfo, token); if (typeof this.options.maxSize === 'number' && fileContent && (fileContent?.size > this.options.maxSize)) { return new Noop(); diff --git a/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts b/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts index d2a912b8352..87d5005df8a 100644 --- a/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts +++ b/src/vs/workbench/services/workingCopy/common/workingCopyFileOperationParticipant.ts @@ -6,7 +6,7 @@ import { CancellationToken } from 'vs/base/common/cancellation'; import { ILogService } from 'vs/platform/log/common/log'; import { IDisposable, Disposable, toDisposable } from 'vs/base/common/lifecycle'; -import { IWorkingCopyFileOperationParticipant, SourceTargetPairWithOptions } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; +import { IWorkingCopyFileOperationParticipant, SourceTargetPair, IFileOperationUndoRedoInfo } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; import { FileOperation } from 'vs/platform/files/common/files'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { LinkedList } from 'vs/base/common/linkedList'; @@ -27,7 +27,7 @@ export class WorkingCopyFileOperationParticipant extends Disposable { return toDisposable(() => remove()); } - async participate(files: SourceTargetPairWithOptions[], operation: FileOperation, token: CancellationToken | undefined): Promise { + async participate(files: SourceTargetPair[], operation: FileOperation, undoInfo: IFileOperationUndoRedoInfo | undefined, token: CancellationToken | undefined): Promise { const timeout = this.configurationService.getValue('files.participants.timeout'); if (timeout <= 0) { return; // disabled @@ -36,7 +36,7 @@ export class WorkingCopyFileOperationParticipant extends Disposable { // For each participant for (const participant of this.participants) { try { - await participant.participate(files, operation, timeout, token ?? CancellationToken.None); + await participant.participate(files, operation, undoInfo, timeout, token ?? CancellationToken.None); } catch (err) { this.logService.warn(err); } diff --git a/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts b/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts index 54a91d5d2f6..c5fb0ee11c8 100644 --- a/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts +++ b/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts @@ -31,11 +31,9 @@ export interface SourceTargetPair { readonly target: URI } -export interface SourceTargetPairWithOptions extends SourceTargetPair { - options?: { - undoRedoGroupId?: number; - isUndoing?: boolean - } +export interface IFileOperationUndoRedoInfo { + undoRedoGroupId?: number; + isUndoing?: boolean } export interface WorkingCopyFileEvent extends IWaitUntil { @@ -64,8 +62,9 @@ export interface IWorkingCopyFileOperationParticipant { * change the working copies before they are being saved to disk. */ participate( - files: SourceTargetPairWithOptions[], + files: SourceTargetPair[], operation: FileOperation, + undoInfo: IFileOperationUndoRedoInfo | undefined, timeout: number, token: CancellationToken ): Promise; @@ -73,21 +72,13 @@ export interface IWorkingCopyFileOperationParticipant { export interface IDeleteOperation { resource: URI; - options?: { - useTrash?: boolean; - recursive?: boolean; - undoRedoGroupId?: number; - isUndoing?: boolean - }; + useTrash?: boolean; + recursive?: boolean; } export interface IMoveOperation { file: Required; - options?: { - overwrite?: boolean; - undoRedoGroupId?: number; - isUndoing?: boolean - } + overwrite?: boolean; } export interface ICopyOperation extends IMoveOperation { } @@ -167,7 +158,7 @@ export interface IWorkingCopyFileService { * Note: events will only be emitted for the provided resource, but not any * parent folders that are being created as part of the operation. */ - createFolder(resource: URI, options?: { undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise; + createFolder(resource: URI, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; /** * Will move working copies matching the provided resources and corresponding children @@ -176,7 +167,7 @@ export interface IWorkingCopyFileService { * Working copy owners can listen to the `onWillRunWorkingCopyFileOperation` and * `onDidRunWorkingCopyFileOperation` events to participate. */ - move(operations: IMoveOperation[], token?: CancellationToken): Promise; + move(operations: IMoveOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; /** * Will copy working copies matching the provided resources and corresponding children @@ -185,7 +176,7 @@ export interface IWorkingCopyFileService { * Working copy owners can listen to the `onWillRunWorkingCopyFileOperation` and * `onDidRunWorkingCopyFileOperation` events to participate. */ - copy(operations: ICopyOperation[], token?: CancellationToken): Promise; + copy(operations: ICopyOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; /** * Will delete working copies matching the provided resources and children @@ -194,7 +185,7 @@ export interface IWorkingCopyFileService { * Working copy owners can listen to the `onWillRunWorkingCopyFileOperation` and * `onDidRunWorkingCopyFileOperation` events to participate. */ - delete(operations: IDeleteOperation[], token?: CancellationToken): Promise; + delete(operations: IDeleteOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; //#endregion @@ -282,7 +273,7 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi } // file operation participant - await this.runFileOperationParticipants([{ target: resource, options: { isUndoing: options?.isUndoing, undoRedoGroupId: options?.undoRedoGroupId } }], FileOperation.CREATE, token); + await this.runFileOperationParticipants([{ target: resource }], FileOperation.CREATE, options, token); // before events const event = { correlationId: this.correlationIds++, operation: FileOperation.CREATE, files: [{ target: resource }] }; @@ -310,21 +301,20 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi return stat; } - async move(operations: IMoveOperation[], token?: CancellationToken): Promise { - return this.doMoveOrCopy(operations, true, token); + async move(operations: IMoveOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { + return this.doMoveOrCopy(operations, true, undoInfo, token); } - async copy(operations: ICopyOperation[], token?: CancellationToken): Promise { - return this.doMoveOrCopy(operations, false, token); + async copy(operations: ICopyOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { + return this.doMoveOrCopy(operations, false, undoInfo, token); } - private async doMoveOrCopy(operations: IMoveOperation[] | ICopyOperation[], move: boolean, token?: CancellationToken): Promise { + private async doMoveOrCopy(operations: IMoveOperation[] | ICopyOperation[], move: boolean, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { const stats: IFileStatWithMetadata[] = []; // validate move/copy operation before starting - for (const operation of operations) { - const { source, target } = operation.file; - const validateMoveOrCopy = await (move ? this.fileService.canMove(source, target, operation.options?.overwrite) : this.fileService.canCopy(source, target, operation.options?.overwrite)); + for (const { file: { source, target }, overwrite } of operations) { + const validateMoveOrCopy = await (move ? this.fileService.canMove(source, target, overwrite) : this.fileService.canCopy(source, target, overwrite)); if (validateMoveOrCopy instanceof Error) { throw validateMoveOrCopy; } @@ -332,16 +322,14 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi // file operation participant const files = operations.map(o => o.file); - // TODO@isidor should the options be passed here - await this.runFileOperationParticipants(files, move ? FileOperation.MOVE : FileOperation.COPY, token); + await this.runFileOperationParticipants(files, move ? FileOperation.MOVE : FileOperation.COPY, undoInfo, token); // before event const event = { correlationId: this.correlationIds++, operation: move ? FileOperation.MOVE : FileOperation.COPY, files }; await this._onWillRunWorkingCopyFileOperation.fireAsync(event, CancellationToken.None); try { - for (const operation of operations) { - const { source, target } = operation.file; + for (const { file: { source, target }, overwrite } of operations) { // if source and target are not equal, handle dirty working copies // depending on the operation: // - move: revert both source and target (if any) @@ -353,9 +341,9 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi // now we can rename the source to target via file operation if (move) { - stats.push(await this.fileService.move(source, target, operation.options?.overwrite)); + stats.push(await this.fileService.move(source, target, overwrite)); } else { - stats.push(await this.fileService.copy(source, target, operation.options?.overwrite)); + stats.push(await this.fileService.copy(source, target, overwrite)); } } } catch (error) { @@ -372,19 +360,19 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi return stats; } - async delete(operations: IDeleteOperation[], token?: CancellationToken): Promise { + async delete(operations: IDeleteOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { // validate delete operation before starting for (const operation of operations) { - const validateDelete = await this.fileService.canDelete(operation.resource, operation.options); + const validateDelete = await this.fileService.canDelete(operation.resource, { recursive: operation.recursive, useTrash: operation.useTrash }); if (validateDelete instanceof Error) { throw validateDelete; } } // file operation participant - const files = operations.map(operation => ({ target: operation.resource, options: operation.options })); - await this.runFileOperationParticipants(files, FileOperation.DELETE, token); + const files = operations.map(operation => ({ target: operation.resource })); + await this.runFileOperationParticipants(files, FileOperation.DELETE, undoInfo, token); // before events const event = { correlationId: this.correlationIds++, operation: FileOperation.DELETE, files }; @@ -401,7 +389,7 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi // now actually delete from disk try { for (const operation of operations) { - await this.fileService.del(operation.resource, operation.options); + await this.fileService.del(operation.resource, { recursive: operation.recursive, useTrash: operation.useTrash }); } } catch (error) { @@ -426,8 +414,8 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi return this.fileOperationParticipants.addFileOperationParticipant(participant); } - private runFileOperationParticipants(files: SourceTargetPairWithOptions[], operation: FileOperation, token: CancellationToken | undefined): Promise { - return this.fileOperationParticipants.participate(files, operation, token); + private runFileOperationParticipants(files: SourceTargetPair[], operation: FileOperation, undoInfo: IFileOperationUndoRedoInfo | undefined, token: CancellationToken | undefined): Promise { + return this.fileOperationParticipants.participate(files, operation, undoInfo, token); } //#endregion diff --git a/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts b/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts index 238b49c2c5a..78eecbaa6f3 100644 --- a/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts +++ b/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts @@ -53,7 +53,7 @@ suite('WorkingCopyFileService', () => { let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8', undefined); (accessor.textFileService.files).add(sourceModel.resource, sourceModel); - const eventCounter = await testEventsMoveOrCopy([{ file: { source: sourceModel.resource, target: sourceModel.resource }, options: { overwrite: true } }], true); + const eventCounter = await testEventsMoveOrCopy([{ file: { source: sourceModel.resource, target: sourceModel.resource }, overwrite: true }], true); sourceModel.dispose(); assert.equal(eventCounter, 3); @@ -68,8 +68,8 @@ suite('WorkingCopyFileService', () => { (accessor.textFileService.files).add(targetModel2.resource, targetModel2); const eventCounter = await testEventsMoveOrCopy([ - { file: { source: sourceModel1.resource, target: sourceModel1.resource }, options: { overwrite: true } }, - { file: { source: sourceModel2.resource, target: targetModel2.resource }, options: { overwrite: true } } + { file: { source: sourceModel1.resource, target: sourceModel1.resource }, overwrite: true }, + { file: { source: sourceModel2.resource, target: targetModel2.resource }, overwrite: true } ], true); sourceModel1.dispose(); @@ -97,7 +97,7 @@ suite('WorkingCopyFileService', () => { let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8', undefined); (accessor.textFileService.files).add(sourceModel.resource, sourceModel); - const eventCounter = await testEventsMoveOrCopy([{ file: { source: sourceModel.resource, target: sourceModel.resource }, options: { overwrite: true } }]); + const eventCounter = await testEventsMoveOrCopy([{ file: { source: sourceModel.resource, target: sourceModel.resource }, overwrite: true }]); sourceModel.dispose(); assert.equal(eventCounter, 3); @@ -112,8 +112,8 @@ suite('WorkingCopyFileService', () => { (accessor.textFileService.files).add(targetModel2.resource, targetModel2); const eventCounter = await testEventsMoveOrCopy([ - { file: { source: sourceModel1.resource, target: sourceModel1.resource }, options: { overwrite: true } }, - { file: { source: sourceModel2.resource, target: targetModel2.resource }, options: { overwrite: true } } + { file: { source: sourceModel1.resource, target: sourceModel1.resource }, overwrite: true }, + { file: { source: sourceModel2.resource, target: targetModel2.resource }, overwrite: true } ]); sourceModel1.dispose(); diff --git a/src/vs/workbench/test/common/workbenchTestServices.ts b/src/vs/workbench/test/common/workbenchTestServices.ts index 63502cf7697..79c02bedf6b 100644 --- a/src/vs/workbench/test/common/workbenchTestServices.ts +++ b/src/vs/workbench/test/common/workbenchTestServices.ts @@ -16,7 +16,7 @@ import { isLinux, isMacintosh } from 'vs/base/common/platform'; import { InMemoryStorageService, WillSaveStateReason } from 'vs/platform/storage/common/storage'; import { WorkingCopyService, IWorkingCopy, IWorkingCopyBackup, WorkingCopyCapabilities } from 'vs/workbench/services/workingCopy/common/workingCopyService'; import { NullExtensionService } from 'vs/workbench/services/extensions/common/extensions'; -import { IWorkingCopyFileService, IWorkingCopyFileOperationParticipant, WorkingCopyFileEvent, IDeleteOperation, ICopyOperation, IMoveOperation } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; +import { IWorkingCopyFileService, IWorkingCopyFileOperationParticipant, WorkingCopyFileEvent, IDeleteOperation, ICopyOperation, IMoveOperation, IFileOperationUndoRedoInfo } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; import { IDisposable, Disposable } from 'vs/base/common/lifecycle'; import { IFileStatWithMetadata } from 'vs/platform/files/common/files'; import { VSBuffer, VSBufferReadable, VSBufferReadableStream } from 'vs/base/common/buffer'; @@ -196,7 +196,7 @@ export class TestWorkingCopyFileService implements IWorkingCopyFileService { addFileOperationParticipant(participant: IWorkingCopyFileOperationParticipant): IDisposable { return Disposable.None; } - async delete(operations: IDeleteOperation[], token?: CancellationToken): Promise { } + async delete(operations: IDeleteOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { } registerWorkingCopyProvider(provider: (resourceOrFolder: URI) => IWorkingCopy[]): IDisposable { return Disposable.None; } @@ -205,9 +205,9 @@ export class TestWorkingCopyFileService implements IWorkingCopyFileService { create(resource: URI, contents?: VSBuffer | VSBufferReadable | VSBufferReadableStream, options?: { overwrite?: boolean | undefined; } | undefined): Promise { throw new Error('Method not implemented.'); } createFolder(resource: URI): Promise { throw new Error('Method not implemented.'); } - move(operations: IMoveOperation[]): Promise { throw new Error('Method not implemented.'); } + move(operations: IMoveOperation[], undoInfo?: IFileOperationUndoRedoInfo): Promise { throw new Error('Method not implemented.'); } - copy(operations: ICopyOperation[], token?: CancellationToken): Promise { throw new Error('Method not implemented.'); } + copy(operations: ICopyOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { throw new Error('Method not implemented.'); } } export function mock(): Ctor { From 780ae7676088104fcc9f9dc795b03e243c0e05ba Mon Sep 17 00:00:00 2001 From: isidor Date: Fri, 18 Dec 2020 11:39:26 +0100 Subject: [PATCH 3/8] workingCopyService: create and createFolder also use IOperation interfaces --- .../contrib/bulkEdit/browser/bulkFileEdits.ts | 4 +- .../textfile/browser/textFileService.ts | 3 +- .../common/workingCopyFileService.ts | 41 +++++++++++++------ .../browser/workingCopyFileService.test.ts | 4 +- .../browser/api/mainThreadEditors.test.ts | 8 ++-- .../test/common/workbenchTestServices.ts | 7 ++-- 6 files changed, 43 insertions(+), 24 deletions(-) diff --git a/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts b/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts index 04ea382ea8a..73a7234fd28 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts @@ -120,9 +120,9 @@ class CreateOperation implements IFileOperation { return new Noop(); // not overwriting, but ignoring, and the target file exists } if (this.options.folder) { - await this._workingCopyFileService.createFolder(this.newUri, { ...this.undoRedoInfo }, token); + await this._workingCopyFileService.createFolder([{ resource: this.newUri }], this.undoRedoInfo, token); } else { - await this._workingCopyFileService.create(this.newUri, this.contents, { overwrite: this.options.overwrite, ...this.undoRedoInfo }, token); + await this._workingCopyFileService.create([{ resource: this.newUri, contents: this.contents, overwrite: this.options.overwrite }], this.undoRedoInfo, token); } return this._instaService.createInstance(DeleteOperation, this.newUri, this.options, { isUndoing: true }, !this.options.folder && !this.contents); } diff --git a/src/vs/workbench/services/textfile/browser/textFileService.ts b/src/vs/workbench/services/textfile/browser/textFileService.ts index 13ddf1513c7..208a0cda90b 100644 --- a/src/vs/workbench/services/textfile/browser/textFileService.ts +++ b/src/vs/workbench/services/textfile/browser/textFileService.ts @@ -151,7 +151,8 @@ export abstract class AbstractTextFileService extends Disposable implements ITex async create(resource: URI, value?: string | ITextSnapshot, options?: ICreateFileOptions): Promise { const readable = await this.getEncodedReadable(resource, value); - return this.workingCopyFileService.create(resource, readable, options); + const result = await this.workingCopyFileService.create([{ resource, contents: readable, overwrite: options?.overwrite }]); + return result[0]; } async write(resource: URI, value: string | ITextSnapshot, options?: IWriteTextFileOptions): Promise { diff --git a/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts b/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts index c5fb0ee11c8..1c3d2055b1a 100644 --- a/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts +++ b/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts @@ -32,7 +32,15 @@ export interface SourceTargetPair { } export interface IFileOperationUndoRedoInfo { + + /** + * Id of the undo group that the file operation belongs to. + */ undoRedoGroupId?: number; + + /** + * Flag indicates if the operation is an undo. + */ isUndoing?: boolean } @@ -70,6 +78,15 @@ export interface IWorkingCopyFileOperationParticipant { ): Promise; } +export interface ICreateOperation { + resource: URI; + overwrite?: boolean; +} + +export interface ICreateFileOperation extends ICreateOperation { + contents?: VSBuffer | VSBufferReadable | VSBufferReadableStream, +} + export interface IDeleteOperation { resource: URI; useTrash?: boolean; @@ -147,7 +164,7 @@ export interface IWorkingCopyFileService { * Working copy owners can listen to the `onWillRunWorkingCopyFileOperation` and * `onDidRunWorkingCopyFileOperation` events to participate. */ - create(resource: URI, contents?: VSBuffer | VSBufferReadable | VSBufferReadableStream, options?: { overwrite?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise; + create(operations: ICreateFileOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; /** * Will create a folder and any parent folder that needs to be created. @@ -158,7 +175,7 @@ export interface IWorkingCopyFileService { * Note: events will only be emitted for the provided resource, but not any * parent folders that are being created as part of the operation. */ - createFolder(resource: URI, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; + createFolder(operations: ICreateOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; /** * Will move working copies matching the provided resources and corresponding children @@ -254,38 +271,38 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi //#region File operations - create(resource: URI, contents?: VSBuffer | VSBufferReadable | VSBufferReadableStream, options?: { overwrite?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise { - return this.doCreateFileOrFolder(resource, true, contents, options, token); + create(operations: ICreateFileOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { + return Promise.all(operations.map(o => this.doCreateFileOrFolder(o, true, undoInfo, token))); } - createFolder(resource: URI, options?: { overwrite?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise { - return this.doCreateFileOrFolder(resource, false, undefined, options, token); + createFolder(operations: ICreateOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { + return Promise.all(operations.map(o => this.doCreateFileOrFolder(o, false, undoInfo, token))); } - async doCreateFileOrFolder(resource: URI, isFile: boolean, contents?: VSBuffer | VSBufferReadable | VSBufferReadableStream, options?: { overwrite?: boolean, undoRedoGroupId?: number, isUndoing?: boolean }, token?: CancellationToken): Promise { + async doCreateFileOrFolder(operation: ICreateFileOperation | ICreateOperation, isFile: boolean, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { // validate create operation before starting if (isFile) { - const validateCreate = await this.fileService.canCreateFile(resource, options); + const validateCreate = await this.fileService.canCreateFile(operation.resource, { overwrite: operation.overwrite }); if (validateCreate instanceof Error) { throw validateCreate; } } // file operation participant - await this.runFileOperationParticipants([{ target: resource }], FileOperation.CREATE, options, token); + await this.runFileOperationParticipants([{ target: operation.resource }], FileOperation.CREATE, undoInfo, token); // before events - const event = { correlationId: this.correlationIds++, operation: FileOperation.CREATE, files: [{ target: resource }] }; + const event = { correlationId: this.correlationIds++, operation: FileOperation.CREATE, files: [{ target: operation.resource }] }; await this._onWillRunWorkingCopyFileOperation.fireAsync(event, CancellationToken.None); // now actually create on disk let stat: IFileStatWithMetadata; try { if (isFile) { - stat = await this.fileService.createFile(resource, contents, { overwrite: options?.overwrite }); + stat = await this.fileService.createFile(operation.resource, (operation as ICreateFileOperation).contents, { overwrite: operation.overwrite }); } else { - stat = await this.fileService.createFolder(resource); + stat = await this.fileService.createFolder(operation.resource); } } catch (error) { diff --git a/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts b/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts index 78eecbaa6f3..3ea04364e32 100644 --- a/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts +++ b/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts @@ -224,7 +224,7 @@ suite('WorkingCopyFileService', () => { eventCounter++; }); - await accessor.workingCopyFileService.createFolder(resource); + await accessor.workingCopyFileService.createFolder([{ resource }]); assert.equal(eventCounter, 3); @@ -459,7 +459,7 @@ suite('WorkingCopyFileService', () => { eventCounter++; }); - await accessor.workingCopyFileService.create(resource, contents); + await accessor.workingCopyFileService.create([{ resource, contents }]); assert.ok(!accessor.workingCopyService.isDirty(model.resource)); model.dispose(); diff --git a/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts b/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts index 8ce5268be39..cf370439d42 100644 --- a/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts +++ b/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts @@ -40,7 +40,7 @@ import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; import { ILabelService } from 'vs/platform/label/common/label'; -import { IWorkingCopyFileService, IMoveOperation, IDeleteOperation, ICopyOperation } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; +import { IWorkingCopyFileService, IMoveOperation, IDeleteOperation, ICopyOperation, ICreateFileOperation } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; import { UndoRedoService } from 'vs/platform/undoRedo/common/undoRedoService'; import { TestDialogService } from 'vs/platform/dialogs/test/common/testDialogService'; import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; @@ -103,8 +103,10 @@ suite('MainThreadEditors', () => { }); services.set(IWorkingCopyFileService, new class extends mock() { onDidRunWorkingCopyFileOperation = Event.None; - create(resource: URI) { - createdResources.add(resource); + create(operations: ICreateFileOperation[]) { + for (const operation of operations) { + createdResources.add(operation.resource); + } return Promise.resolve(Object.create(null)); } move(operations: IMoveOperation[]) { diff --git a/src/vs/workbench/test/common/workbenchTestServices.ts b/src/vs/workbench/test/common/workbenchTestServices.ts index 79c02bedf6b..6a00cd3ef4d 100644 --- a/src/vs/workbench/test/common/workbenchTestServices.ts +++ b/src/vs/workbench/test/common/workbenchTestServices.ts @@ -16,10 +16,9 @@ import { isLinux, isMacintosh } from 'vs/base/common/platform'; import { InMemoryStorageService, WillSaveStateReason } from 'vs/platform/storage/common/storage'; import { WorkingCopyService, IWorkingCopy, IWorkingCopyBackup, WorkingCopyCapabilities } from 'vs/workbench/services/workingCopy/common/workingCopyService'; import { NullExtensionService } from 'vs/workbench/services/extensions/common/extensions'; -import { IWorkingCopyFileService, IWorkingCopyFileOperationParticipant, WorkingCopyFileEvent, IDeleteOperation, ICopyOperation, IMoveOperation, IFileOperationUndoRedoInfo } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; +import { IWorkingCopyFileService, IWorkingCopyFileOperationParticipant, WorkingCopyFileEvent, IDeleteOperation, ICopyOperation, IMoveOperation, IFileOperationUndoRedoInfo, ICreateFileOperation, ICreateOperation } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; import { IDisposable, Disposable } from 'vs/base/common/lifecycle'; import { IFileStatWithMetadata } from 'vs/platform/files/common/files'; -import { VSBuffer, VSBufferReadable, VSBufferReadableStream } from 'vs/base/common/buffer'; import { ISaveOptions, IRevertOptions } from 'vs/workbench/common/editor'; import { CancellationToken } from 'vs/base/common/cancellation'; @@ -202,8 +201,8 @@ export class TestWorkingCopyFileService implements IWorkingCopyFileService { getDirty(resource: URI): IWorkingCopy[] { return []; } - create(resource: URI, contents?: VSBuffer | VSBufferReadable | VSBufferReadableStream, options?: { overwrite?: boolean | undefined; } | undefined): Promise { throw new Error('Method not implemented.'); } - createFolder(resource: URI): Promise { throw new Error('Method not implemented.'); } + create(operations: ICreateFileOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { throw new Error('Method not implemented.'); } + createFolder(operations: ICreateOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { throw new Error('Method not implemented.'); } move(operations: IMoveOperation[], undoInfo?: IFileOperationUndoRedoInfo): Promise { throw new Error('Method not implemented.'); } From 9c30a0e98cc3c3fc337e04a63e2165514573ed8f Mon Sep 17 00:00:00 2001 From: isidor Date: Fri, 18 Dec 2020 11:41:14 +0100 Subject: [PATCH 4/8] null guard --- .../api/browser/mainThreadFileSystemEventService.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts b/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts index 7bf3cb81904..c03943332e4 100644 --- a/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts +++ b/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts @@ -74,8 +74,8 @@ export class MainThreadFileSystemEventService { const fileOperationParticipant = new class implements IWorkingCopyFileOperationParticipant { - async participate(files: SourceTargetPair[], operation: FileOperation, undoInfo: IFileOperationUndoRedoInfo, timeout: number, token: CancellationToken) { - if (undoInfo.isUndoing) { + async participate(files: SourceTargetPair[], operation: FileOperation, undoInfo: IFileOperationUndoRedoInfo | undefined, timeout: number, token: CancellationToken) { + if (undoInfo?.isUndoing) { return; } @@ -171,7 +171,7 @@ export class MainThreadFileSystemEventService { await bulkEditService.apply( reviveWorkspaceEditDto2(data.edit), - { undoRedoGroupId: undoInfo.undoRedoGroupId, showPreview } + { undoRedoGroupId: undoInfo?.undoRedoGroupId, showPreview } ); } From c78bd5bd78337a33b596d1166bbff3aa52bebe22 Mon Sep 17 00:00:00 2001 From: isidor Date: Fri, 18 Dec 2020 14:40:57 +0100 Subject: [PATCH 5/8] createFile and createFolder only allow single operation until there are more use cases --- .../contrib/bulkEdit/browser/bulkFileEdits.ts | 4 ++-- .../services/textfile/browser/textFileService.ts | 3 +-- .../workingCopy/common/workingCopyFileService.ts | 12 ++++++------ .../test/browser/workingCopyFileService.test.ts | 4 ++-- .../test/browser/api/mainThreadEditors.test.ts | 6 ++---- .../workbench/test/common/workbenchTestServices.ts | 4 ++-- 6 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts b/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts index 73a7234fd28..077f9f8d5c0 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts @@ -120,9 +120,9 @@ class CreateOperation implements IFileOperation { return new Noop(); // not overwriting, but ignoring, and the target file exists } if (this.options.folder) { - await this._workingCopyFileService.createFolder([{ resource: this.newUri }], this.undoRedoInfo, token); + await this._workingCopyFileService.createFolder({ resource: this.newUri }, this.undoRedoInfo, token); } else { - await this._workingCopyFileService.create([{ resource: this.newUri, contents: this.contents, overwrite: this.options.overwrite }], this.undoRedoInfo, token); + await this._workingCopyFileService.create({ resource: this.newUri, contents: this.contents, overwrite: this.options.overwrite }, this.undoRedoInfo, token); } return this._instaService.createInstance(DeleteOperation, this.newUri, this.options, { isUndoing: true }, !this.options.folder && !this.contents); } diff --git a/src/vs/workbench/services/textfile/browser/textFileService.ts b/src/vs/workbench/services/textfile/browser/textFileService.ts index 208a0cda90b..615fa0ca4bb 100644 --- a/src/vs/workbench/services/textfile/browser/textFileService.ts +++ b/src/vs/workbench/services/textfile/browser/textFileService.ts @@ -151,8 +151,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex async create(resource: URI, value?: string | ITextSnapshot, options?: ICreateFileOptions): Promise { const readable = await this.getEncodedReadable(resource, value); - const result = await this.workingCopyFileService.create([{ resource, contents: readable, overwrite: options?.overwrite }]); - return result[0]; + return await this.workingCopyFileService.create({ resource, contents: readable, overwrite: options?.overwrite }); } async write(resource: URI, value: string | ITextSnapshot, options?: IWriteTextFileOptions): Promise { diff --git a/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts b/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts index 1c3d2055b1a..c54bc352be7 100644 --- a/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts +++ b/src/vs/workbench/services/workingCopy/common/workingCopyFileService.ts @@ -164,7 +164,7 @@ export interface IWorkingCopyFileService { * Working copy owners can listen to the `onWillRunWorkingCopyFileOperation` and * `onDidRunWorkingCopyFileOperation` events to participate. */ - create(operations: ICreateFileOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; + create(operation: ICreateFileOperation, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; /** * Will create a folder and any parent folder that needs to be created. @@ -175,7 +175,7 @@ export interface IWorkingCopyFileService { * Note: events will only be emitted for the provided resource, but not any * parent folders that are being created as part of the operation. */ - createFolder(operations: ICreateOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; + createFolder(operation: ICreateOperation, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise; /** * Will move working copies matching the provided resources and corresponding children @@ -271,12 +271,12 @@ export class WorkingCopyFileService extends Disposable implements IWorkingCopyFi //#region File operations - create(operations: ICreateFileOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { - return Promise.all(operations.map(o => this.doCreateFileOrFolder(o, true, undoInfo, token))); + create(operation: ICreateFileOperation, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { + return this.doCreateFileOrFolder(operation, true, undoInfo, token); } - createFolder(operations: ICreateOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { - return Promise.all(operations.map(o => this.doCreateFileOrFolder(o, false, undoInfo, token))); + createFolder(operation: ICreateOperation, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { + return this.doCreateFileOrFolder(operation, false, undoInfo, token); } async doCreateFileOrFolder(operation: ICreateFileOperation | ICreateOperation, isFile: boolean, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { diff --git a/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts b/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts index 3ea04364e32..cb1562c837d 100644 --- a/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts +++ b/src/vs/workbench/services/workingCopy/test/browser/workingCopyFileService.test.ts @@ -224,7 +224,7 @@ suite('WorkingCopyFileService', () => { eventCounter++; }); - await accessor.workingCopyFileService.createFolder([{ resource }]); + await accessor.workingCopyFileService.createFolder({ resource }); assert.equal(eventCounter, 3); @@ -459,7 +459,7 @@ suite('WorkingCopyFileService', () => { eventCounter++; }); - await accessor.workingCopyFileService.create([{ resource, contents }]); + await accessor.workingCopyFileService.create({ resource, contents }); assert.ok(!accessor.workingCopyService.isDirty(model.resource)); model.dispose(); diff --git a/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts b/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts index cf370439d42..ffb4e0e6f61 100644 --- a/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts +++ b/src/vs/workbench/test/browser/api/mainThreadEditors.test.ts @@ -103,10 +103,8 @@ suite('MainThreadEditors', () => { }); services.set(IWorkingCopyFileService, new class extends mock() { onDidRunWorkingCopyFileOperation = Event.None; - create(operations: ICreateFileOperation[]) { - for (const operation of operations) { - createdResources.add(operation.resource); - } + create(operation: ICreateFileOperation) { + createdResources.add(operation.resource); return Promise.resolve(Object.create(null)); } move(operations: IMoveOperation[]) { diff --git a/src/vs/workbench/test/common/workbenchTestServices.ts b/src/vs/workbench/test/common/workbenchTestServices.ts index 6a00cd3ef4d..1cd6d63130f 100644 --- a/src/vs/workbench/test/common/workbenchTestServices.ts +++ b/src/vs/workbench/test/common/workbenchTestServices.ts @@ -201,8 +201,8 @@ export class TestWorkingCopyFileService implements IWorkingCopyFileService { getDirty(resource: URI): IWorkingCopy[] { return []; } - create(operations: ICreateFileOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { throw new Error('Method not implemented.'); } - createFolder(operations: ICreateOperation[], undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { throw new Error('Method not implemented.'); } + create(operation: ICreateFileOperation, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { throw new Error('Method not implemented.'); } + createFolder(operation: ICreateOperation, undoInfo?: IFileOperationUndoRedoInfo, token?: CancellationToken): Promise { throw new Error('Method not implemented.'); } move(operations: IMoveOperation[], undoInfo?: IFileOperationUndoRedoInfo): Promise { throw new Error('Method not implemented.'); } From 3bff49d246801371822ed2d735a24a2ff8082f52 Mon Sep 17 00:00:00 2001 From: Alexandru Dima Date: Fri, 18 Dec 2020 14:43:07 +0100 Subject: [PATCH 6/8] More tweaks to Windows cache exclusions --- .github/workflows/ci.yml | 59 ++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4c0ec3964a5..f62e84d5e84 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,8 +28,8 @@ jobs: uses: actions/cache@v2 with: path: "**/node_modules" - key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - restore-keys: ${{ runner.os }}-cacheNodeModules4- + key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + restore-keys: ${{ runner.os }}-cacheNodeModules5- - name: Get yarn cache directory path id: yarnCacheDirPath if: ${{ steps.cacheNodeModules.outputs.cache-hit != 'true' }} @@ -71,8 +71,8 @@ jobs: # uses: actions/cache@v2 # with: # path: '**/node_modules' - # key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - # restore-keys: ${{ runner.os }}-cacheNodeModules4- + # key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + # restore-keys: ${{ runner.os }}-cacheNodeModules5- # - name: Get yarn cache directory path # id: yarnCacheDirPath # if: ${{ steps.cacheNodeModules.outputs.cache-hit != 'true' }} @@ -125,8 +125,8 @@ jobs: # uses: actions/cache@v2 # with: # path: '**/node_modules' - # key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - # restore-keys: ${{ runner.os }}-cacheNodeModules4- + # key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + # restore-keys: ${{ runner.os }}-cacheNodeModules5- # - name: Get yarn cache directory path # id: yarnCacheDirPath # if: ${{ steps.cacheNodeModules.outputs.cache-hit != 'true' }} @@ -184,8 +184,8 @@ jobs: # uses: actions/cache@v2 # with: # path: '**/node_modules' - # key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - # restore-keys: ${{ runner.os }}-cacheNodeModules4- + # key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + # restore-keys: ${{ runner.os }}-cacheNodeModules5- # - name: Restore compiled core code # id: cacheCompiledCoreCode @@ -246,8 +246,8 @@ jobs: # uses: actions/cache@v2 # with: # path: '**/node_modules' - # key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - # restore-keys: ${{ runner.os }}-cacheNodeModules4- + # key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + # restore-keys: ${{ runner.os }}-cacheNodeModules5- # - name: Restore compiled core code # id: cacheCompiledCoreCode @@ -291,8 +291,8 @@ jobs: # uses: actions/cache@v2 # with: # path: '**/node_modules' - # key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - # restore-keys: ${{ runner.os }}-cacheNodeModules4- + # key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + # restore-keys: ${{ runner.os }}-cacheNodeModules5- # - name: Get yarn cache directory path # id: yarnCacheDirPath # if: ${{ steps.cacheNodeModules.outputs.cache-hit != 'true' }} @@ -329,8 +329,8 @@ jobs: # uses: actions/cache@v2 # with: # path: '**/node_modules' - # key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - # restore-keys: ${{ runner.os }}-cacheNodeModules4- + # key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + # restore-keys: ${{ runner.os }}-cacheNodeModules5- # - name: Restore compiled core code # id: cacheCompiledCoreCode @@ -381,8 +381,8 @@ jobs: # uses: actions/cache@v2 # with: # path: '**/node_modules' - # key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - # restore-keys: ${{ runner.os }}-cacheNodeModules4- + # key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + # restore-keys: ${{ runner.os }}-cacheNodeModules5- # - name: Restore compiled core code # id: cacheCompiledCoreCode @@ -427,8 +427,8 @@ jobs: # uses: actions/cache@v2 # with: # path: '**/node_modules' - # key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - # restore-keys: ${{ runner.os }}-cacheNodeModules4- + # key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + # restore-keys: ${{ runner.os }}-cacheNodeModules5- # - name: Restore compiled core code # id: cacheCompiledCoreCode @@ -484,8 +484,8 @@ jobs: uses: actions/cache@v2 with: path: "**/node_modules" - key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - restore-keys: ${{ runner.os }}-cacheNodeModules4- + key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + restore-keys: ${{ runner.os }}-cacheNodeModules5- - name: Get yarn cache directory path id: yarnCacheDirPath if: ${{ steps.cacheNodeModules.outputs.cache-hit != 'true' }} @@ -542,11 +542,12 @@ jobs: with: path: | **/node_modules - !**/Release/*.pdb - !**/Release/*.ilk - !**/Release/obj/** - key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - restore-keys: ${{ runner.os }}-cacheNodeModules4- + !**/Release/**/*.pdb + !**/Release/**/*.ilk + !**/Release/**/*.obj + !**/Release/**/*.tlog + key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + restore-keys: ${{ runner.os }}-cacheNodeModules5- - name: Get yarn cache directory path id: yarnCacheDirPath if: ${{ steps.cacheNodeModules.outputs.cache-hit != 'true' }} @@ -594,8 +595,8 @@ jobs: uses: actions/cache@v2 with: path: "**/node_modules" - key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - restore-keys: ${{ runner.os }}-cacheNodeModules4- + key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + restore-keys: ${{ runner.os }}-cacheNodeModules5- - name: Get yarn cache directory path id: yarnCacheDirPath if: ${{ steps.cacheNodeModules.outputs.cache-hit != 'true' }} @@ -643,8 +644,8 @@ jobs: uses: actions/cache@v2 with: path: "**/node_modules" - key: ${{ runner.os }}-cacheNodeModules4-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} - restore-keys: ${{ runner.os }}-cacheNodeModules4- + key: ${{ runner.os }}-cacheNodeModules5-${{ hashFiles('.yarnrc', 'remote/.yarnrc', '**/yarn.lock', '!**/node_modules/**/yarn.lock', '!**/.*/**/yarn.lock') }} + restore-keys: ${{ runner.os }}-cacheNodeModules5- - name: Execute yarn if: ${{ steps.cacheNodeModules.outputs.cache-hit != 'true' }} env: From 2c4988b76396998de55e1810fe92c8988c068aed Mon Sep 17 00:00:00 2001 From: Alexandru Dima Date: Fri, 18 Dec 2020 15:10:40 +0100 Subject: [PATCH 7/8] Fixes #112301: Wrapped lines contain the wrapping whitespace in the line content --- .../editor/browser/controller/mouseTarget.ts | 5 ++- .../common/controller/cursorMoveOperations.ts | 11 +++---- .../test/browser/controller/cursor.test.ts | 31 +++++++++++++++++++ 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/vs/editor/browser/controller/mouseTarget.ts b/src/vs/editor/browser/controller/mouseTarget.ts index 88815bb8eb0..d880c1a322f 100644 --- a/src/vs/editor/browser/controller/mouseTarget.ts +++ b/src/vs/editor/browser/controller/mouseTarget.ts @@ -1014,12 +1014,11 @@ export class MouseTargetFactory { } private static _snapToSoftTabBoundary(position: Position, viewModel: IViewModel): Position { - const minColumn = viewModel.getLineMinColumn(position.lineNumber); const lineContent = viewModel.getLineContent(position.lineNumber); const { tabSize } = viewModel.getTextModelOptions(); - const newPosition = AtomicTabMoveOperations.atomicPosition(lineContent, position.column - minColumn, tabSize, Direction.Nearest); + const newPosition = AtomicTabMoveOperations.atomicPosition(lineContent, position.column - 1, tabSize, Direction.Nearest); if (newPosition !== -1) { - return new Position(position.lineNumber, newPosition + minColumn); + return new Position(position.lineNumber, newPosition + 1); } return position; } diff --git a/src/vs/editor/common/controller/cursorMoveOperations.ts b/src/vs/editor/common/controller/cursorMoveOperations.ts index 9229c2e3584..ac505bdcdb5 100644 --- a/src/vs/editor/common/controller/cursorMoveOperations.ts +++ b/src/vs/editor/common/controller/cursorMoveOperations.ts @@ -39,11 +39,11 @@ export class MoveOperations { public static leftPositionAtomicSoftTabs(model: ICursorSimpleModel, lineNumber: number, column: number, tabSize: number): Position { const minColumn = model.getLineMinColumn(lineNumber); const lineContent = model.getLineContent(lineNumber); - const newPosition = AtomicTabMoveOperations.atomicPosition(lineContent, column - minColumn, tabSize, Direction.Left); - if (newPosition === -1) { + const newPosition = AtomicTabMoveOperations.atomicPosition(lineContent, column - 1, tabSize, Direction.Left); + if (newPosition === -1 || newPosition + 1 < minColumn) { return this.leftPosition(model, lineNumber, column); } - return new Position(lineNumber, minColumn + newPosition); + return new Position(lineNumber, newPosition + 1); } public static left(config: CursorConfiguration, model: ICursorSimpleModel, lineNumber: number, column: number): CursorPosition { @@ -81,13 +81,12 @@ export class MoveOperations { } public static rightPositionAtomicSoftTabs(model: ICursorSimpleModel, lineNumber: number, column: number, tabSize: number, indentSize: number): Position { - const minColumn = model.getLineMinColumn(lineNumber); const lineContent = model.getLineContent(lineNumber); - const newPosition = AtomicTabMoveOperations.atomicPosition(lineContent, column - minColumn, tabSize, Direction.Right); + const newPosition = AtomicTabMoveOperations.atomicPosition(lineContent, column - 1, tabSize, Direction.Right); if (newPosition === -1) { return this.rightPosition(model, lineNumber, column); } - return new Position(lineNumber, minColumn + newPosition); + return new Position(lineNumber, newPosition + 1); } public static right(config: CursorConfiguration, model: ICursorSimpleModel, lineNumber: number, column: number): CursorPosition { diff --git a/src/vs/editor/test/browser/controller/cursor.test.ts b/src/vs/editor/test/browser/controller/cursor.test.ts index 9dc368f6998..4fc05dacf97 100644 --- a/src/vs/editor/test/browser/controller/cursor.test.ts +++ b/src/vs/editor/test/browser/controller/cursor.test.ts @@ -2349,6 +2349,37 @@ suite('Editor Controller - Regression tests', () => { }); }); + test('issue #112301: new stickyTabStops feature interferes with word wrap', () => { + withTestCodeEditor([ + [ + 'function hello() {', + ' console.log(`this is a long console message`)', + '}', + ].join('\n') + ], { wordWrap: 'wordWrapColumn', wordWrapColumn: 32, stickyTabStops: true }, (editor, viewModel) => { + viewModel.setSelections('test', [ + new Selection(2, 31, 2, 31) + ]); + moveRight(editor, viewModel, false); + assertCursor(viewModel, new Position(2, 32)); + + moveRight(editor, viewModel, false); + assertCursor(viewModel, new Position(2, 33)); + + moveRight(editor, viewModel, false); + assertCursor(viewModel, new Position(2, 34)); + + moveLeft(editor, viewModel, false); + assertCursor(viewModel, new Position(2, 33)); + + moveLeft(editor, viewModel, false); + assertCursor(viewModel, new Position(2, 32)); + + moveLeft(editor, viewModel, false); + assertCursor(viewModel, new Position(2, 31)); + }); + }); + test('issue #44805: Should not be able to undo in readonly editor', () => { let model = createTextModel( [ From 14cb2089dc2ec1f3be6eed21a66b2e1023c6d472 Mon Sep 17 00:00:00 2001 From: Alexandru Dima Date: Fri, 18 Dec 2020 15:51:24 +0100 Subject: [PATCH 8/8] update distro --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 92be75cbad4..bae7cf63202 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "code-oss-dev", "version": "1.53.0", - "distro": "c89ced323fffc6ffd5caa6e9bb6662b8e26b5fb4", + "distro": "252d98105e15a5ba22421ff5aef8d1aa7c3b69fa", "author": { "name": "Microsoft Corporation" },