From 0e6bc508d2ff7147be22cd011f54a3d4f368ce83 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 18 Jun 2018 11:44:12 +0200 Subject: [PATCH] fix #42640 --- .../files/electron-browser/fileActions.ts | 43 +------- .../electron-browser/views/explorerViewer.ts | 99 +++++-------------- .../textfile/common/textFileService.ts | 71 ++++++++----- 3 files changed, 75 insertions(+), 138 deletions(-) diff --git a/src/vs/workbench/parts/files/electron-browser/fileActions.ts b/src/vs/workbench/parts/files/electron-browser/fileActions.ts index 405be9a3a7c..9c1a8580d78 100644 --- a/src/vs/workbench/parts/files/electron-browser/fileActions.ts +++ b/src/vs/workbench/parts/files/electron-browser/fileActions.ts @@ -36,7 +36,6 @@ import { IQuickOpenService } from 'vs/platform/quickOpen/common/quickOpen'; import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet'; import { IInstantiationService, ServicesAccessor, IConstructorSignature2 } from 'vs/platform/instantiation/common/instantiation'; import { ITextModel } from 'vs/editor/common/model'; -import { IBackupFileService } from 'vs/workbench/services/backup/common/backup'; import { IWindowService } from 'vs/platform/windows/common/windows'; import { COPY_PATH_COMMAND_ID, REVEAL_IN_EXPLORER_COMMAND_ID, SAVE_ALL_COMMAND_ID, SAVE_ALL_LABEL, SAVE_ALL_IN_GROUP_COMMAND_ID } from 'vs/workbench/parts/files/electron-browser/fileCommands'; import { ITextModelService, ITextModelContentProvider } from 'vs/editor/common/services/resolverService'; @@ -289,8 +288,7 @@ class RenameFileAction extends BaseRenameAction { element: ExplorerItem, @IFileService fileService: IFileService, @INotificationService notificationService: INotificationService, - @ITextFileService textFileService: ITextFileService, - @IBackupFileService private backupFileService: IBackupFileService + @ITextFileService textFileService: ITextFileService ) { super(RenameFileAction.ID, nls.localize('rename', "Rename"), element, fileService, notificationService, textFileService); @@ -298,43 +296,10 @@ class RenameFileAction extends BaseRenameAction { } public runAction(newName: string): TPromise { - const dirty = this.textFileService.getDirty().filter(d => resources.isEqualOrParent(d, this.element.resource, !isLinux /* ignorecase */)); - const dirtyRenamed: URI[] = []; - return TPromise.join(dirty.map(d => { - let renamed: URI; - - // If the dirty file itself got moved, just reparent it to the target folder - const targetPath = paths.join(this.element.parent.resource.path, newName); - if (this.element.resource.toString() === d.toString()) { - renamed = this.element.parent.resource.with({ path: targetPath }); - } - - // Otherwise, a parent of the dirty resource got moved, so we have to reparent more complicated - else { - renamed = this.element.parent.resource.with({ path: paths.join(targetPath, d.path.substr(this.element.resource.path.length + 1)) }); - } - - dirtyRenamed.push(renamed); + const parentResource = this.element.parent.resource; + const targetResource = parentResource.with({ path: paths.join(parentResource.path, newName) }); - const model = this.textFileService.models.get(d); - - return this.backupFileService.backupResource(renamed, model.createSnapshot(), model.getVersionId()); - })) - - // 2. soft revert all dirty since we have backed up their contents - .then(() => this.textFileService.revertAll(dirty, { soft: true /* do not attempt to load content from disk */ })) - - // 3.) run the rename operation - .then(() => this.fileService.rename(this.element.resource, newName).then(null, (error: Error) => { - return TPromise.join(dirtyRenamed.map(d => this.backupFileService.discardResourceBackup(d))).then(() => { - this.onErrorWithRetry(error, () => this.runAction(newName)); - }); - })) - - // 4.) resolve those that were dirty to load their previous dirty contents from disk - .then(() => { - return TPromise.join(dirtyRenamed.map(t => this.textFileService.models.loadOrCreate(t))); - }); + return this.textFileService.move(this.element.resource, targetResource); } } diff --git a/src/vs/workbench/parts/files/electron-browser/views/explorerViewer.ts b/src/vs/workbench/parts/files/electron-browser/views/explorerViewer.ts index 2ca075113d8..28964ea3d89 100644 --- a/src/vs/workbench/parts/files/electron-browser/views/explorerViewer.ts +++ b/src/vs/workbench/parts/files/electron-browser/views/explorerViewer.ts @@ -56,7 +56,6 @@ import { IDialogService, IConfirmationResult, IConfirmation, getConfirmMessage } import { INotificationService } from 'vs/platform/notification/common/notification'; import { IEditorService, SIDE_GROUP, ACTIVE_GROUP } from 'vs/workbench/services/editor/common/editorService'; import { fillInContextMenuActions } from 'vs/platform/actions/browser/menuItemActionItem'; -import { IBackupFileService } from 'vs/workbench/services/backup/common/backup'; export class FileDataSource implements IDataSource { constructor( @@ -759,7 +758,6 @@ export class FileDragAndDrop extends SimpleFileResourceDragAndDrop { @IConfigurationService private configurationService: IConfigurationService, @IInstantiationService instantiationService: IInstantiationService, @ITextFileService private textFileService: ITextFileService, - @IBackupFileService private backupFileService: IBackupFileService, @IWindowService private windowService: IWindowService, @IWorkspaceEditingService private workspaceEditingService: IWorkspaceEditingService ) { @@ -1033,91 +1031,48 @@ export class FileDragAndDrop extends SimpleFileResourceDragAndDrop { } private doHandleExplorerDrop(tree: ITree, source: ExplorerItem, target: ExplorerItem | Model, isCopy: boolean): TPromise { + if (!(target instanceof ExplorerItem)) { + return TPromise.as(void 0); + } + return tree.expand(target).then(() => { + // Reuse duplicate action if user copies if (isCopy) { return this.instantiationService.createInstance(DuplicateFileAction, tree, source, target).run(); } - const dirtyMoved: URI[] = []; + // Otherwise move + const targetResource = target.resource.with({ path: paths.join(target.resource.path, source.name) }); - // Success: load all files that are dirty again to restore their dirty contents - // Error: discard any backups created during the process - const onSuccess = () => TPromise.join(dirtyMoved.map(t => this.textFileService.models.loadOrCreate(t))); - const onError = (error?: Error, showError?: boolean) => { - if (showError) { - this.notificationService.error(error); - } + return this.textFileService.move(source.resource, targetResource).then(null, error => { - return TPromise.join(dirtyMoved.map(d => this.backupFileService.discardResourceBackup(d))); - }; - if (!(target instanceof ExplorerItem)) { - return TPromise.as(void 0); - } + // Conflict + if ((error).fileOperationResult === FileOperationResult.FILE_MOVE_CONFLICT) { + const confirm: IConfirmation = { + message: nls.localize('confirmOverwriteMessage', "'{0}' already exists in the destination folder. Do you want to replace it?", source.name), + detail: nls.localize('irreversible', "This action is irreversible!"), + primaryButton: nls.localize({ key: 'replaceButtonLabel', comment: ['&& denotes a mnemonic'] }, "&&Replace"), + type: 'warning' + }; - // 1. check for dirty files that are being moved and backup to new target - const dirty = this.textFileService.getDirty().filter(d => resources.isEqualOrParent(d, source.resource, !isLinux /* ignorecase */)); - return TPromise.join(dirty.map(d => { - let moved: URI; + // Move with overwrite if the user confirms + return this.dialogService.confirm(confirm).then(res => { + if (res.confirmed) { + return this.textFileService.move(source.resource, targetResource, true /* overwrite */).then(null, error => this.notificationService.error(error)); + } - // If the dirty file itself got moved, just reparent it to the target folder - if (source.resource.toString() === d.toString()) { - moved = target.resource.with({ path: paths.join(target.resource.path, source.name) }); + return void 0; + }); } - // Otherwise, a parent of the dirty resource got moved, so we have to reparent more complicated. Example: + // Any other error else { - moved = target.resource.with({ path: paths.join(target.resource.path, d.path.substr(source.parent.resource.path.length + 1)) }); + this.notificationService.error(error); } - dirtyMoved.push(moved); - - const model = this.textFileService.models.get(d); - - return this.backupFileService.backupResource(moved, model.createSnapshot(), model.getVersionId()); - })) - - // 2. soft revert all dirty since we have backed up their contents - .then(() => this.textFileService.revertAll(dirty, { soft: true /* do not attempt to load content from disk */ })) - - // 3.) run the move operation - .then(() => { - const targetResource = target.resource.with({ path: paths.join(target.resource.path, source.name) }); - - return this.fileService.moveFile(source.resource, targetResource).then(null, error => { - - // Conflict - if ((error).fileOperationResult === FileOperationResult.FILE_MOVE_CONFLICT) { - const confirm: IConfirmation = { - message: nls.localize('confirmOverwriteMessage', "'{0}' already exists in the destination folder. Do you want to replace it?", source.name), - detail: nls.localize('irreversible', "This action is irreversible!"), - primaryButton: nls.localize({ key: 'replaceButtonLabel', comment: ['&& denotes a mnemonic'] }, "&&Replace"), - type: 'warning' - }; - - // Move with overwrite if the user confirms - return this.dialogService.confirm(confirm).then(res => { - if (res.confirmed) { - const targetDirty = this.textFileService.getDirty().filter(d => resources.isEqualOrParent(d, targetResource, !isLinux /* ignorecase */)); - - // Make sure to revert all dirty in target first to be able to overwrite properly - return this.textFileService.revertAll(targetDirty, { soft: true /* do not attempt to load content from disk */ }).then(() => { - - // Then continue to do the move operation - return this.fileService.moveFile(source.resource, targetResource, true).then(onSuccess, error => onError(error, true)); - }); - } - - return onError(); - }); - } - - return onError(error, true); - }); - }) - - // 4.) resolve those that were dirty to load their previous dirty contents from disk - .then(onSuccess, onError); + return void 0; + }); }, errors.onUnexpectedError); } } diff --git a/src/vs/workbench/services/textfile/common/textFileService.ts b/src/vs/workbench/services/textfile/common/textFileService.ts index f413f4b9210..3e1b6a3f3a3 100644 --- a/src/vs/workbench/services/textfile/common/textFileService.ts +++ b/src/vs/workbench/services/textfile/common/textFileService.ts @@ -33,6 +33,7 @@ import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/c import { createTextBufferFactoryFromSnapshot } from 'vs/editor/common/model/textModel'; import { IModelService } from 'vs/editor/common/services/modelService'; import { INotificationService } from 'vs/platform/notification/common/notification'; +import { isEqualOrParent, isEqual } from 'vs/base/common/resources'; export interface IBackupResult { didBackup: boolean; @@ -702,51 +703,67 @@ export abstract class TextFileService implements ITextFileService { } public delete(resource: URI, useTrash?: boolean): TPromise { - return this.revert(resource, { soft: true }).then(() => this.fileService.del(resource, useTrash)); + const dirtyFiles = this.getDirty().filter(dirty => isEqualOrParent(dirty, resource, !platform.isLinux /* ignorecase */)); + + return this.revertAll(dirtyFiles, { soft: true }).then(() => this.fileService.del(resource, useTrash)); } public move(source: URI, target: URI, overwrite?: boolean): TPromise { - // Handle target model if existing + // Handle target models if existing (if target URI is a folder, this can be multiple) let handleTargetModelPromise: TPromise = TPromise.as(void 0); - const targetModel = this.models.get(target); - if (targetModel) { - if (!overwrite) { - return TPromise.wrapError(new Error('Cannot move file because target file exists and we are not overwriting')); - } - - // Soft revert the target file since we overwrite - handleTargetModelPromise = this.revert(target, { soft: true }); + const dirtyTargetModels = this.getDirtyFileModels().filter(model => isEqualOrParent(model.getResource(), target, !platform.isLinux /* ignorecase */)); + if (dirtyTargetModels.length) { + handleTargetModelPromise = this.revertAll(dirtyTargetModels.map(targetModel => targetModel.getResource()), { soft: true }); } return handleTargetModelPromise.then(() => { - // Handle source model if existing - let handleSourceModelPromise: TPromise; - const sourceModel = this.models.get(source); - if (sourceModel && sourceModel.isDirty()) { - // Backup to target if the source is dirty - handleSourceModelPromise = this.backupFileService.backupResource(target, sourceModel.createSnapshot(), sourceModel.getVersionId()).then((() => true)); + // Handle dirty source models if existing (if source URI is a folder, this can be multiple) + let handleDirtySourceModels: TPromise; + const dirtySourceModels = this.getDirtyFileModels().filter(model => isEqualOrParent(model.getResource(), source, !platform.isLinux /* ignorecase */)); + const dirtyTargetModels: URI[] = []; + if (dirtySourceModels.length) { + handleDirtySourceModels = TPromise.join(dirtySourceModels.map(sourceModel => { + const sourceModelResource = sourceModel.getResource(); + let targetModelResource: URI; + + // If the source is the actual model, just use target as new resource + if (isEqual(sourceModelResource, source, !platform.isLinux /* ignorecase */)) { + targetModelResource = target; + } + + // Otherwise a parent folder of the source is being moved, so we need + // to compute the target resource based on that + else { + targetModelResource = sourceModelResource.with({ path: paths.join(target.path, sourceModelResource.path.substr(source.path.length + 1)) }); + } + + // Remember as dirty target model to load after the operation + dirtyTargetModels.push(targetModelResource); + + // Backup dirty source model to the target resource it will become later + return this.backupFileService.backupResource(targetModelResource, sourceModel.createSnapshot(), sourceModel.getVersionId()); + })); } else { - handleSourceModelPromise = TPromise.as(false); + handleDirtySourceModels = TPromise.as(void 0); } - return handleSourceModelPromise.then(dirty => { + return handleDirtySourceModels.then(() => { - // Soft revert the source file - return this.revert(source, { soft: true }).then(() => { + // Soft revert the dirty source files if any + return this.revertAll(dirtySourceModels.map(dirtySourceModel => dirtySourceModel.getResource()), { soft: true }).then(() => { // Rename to target return this.fileService.moveFile(source, target, overwrite).then(() => { - // Load if we were dirty before - if (dirty) { - return this.models.loadOrCreate(target).then(() => void 0); - } - - return void 0; + // Load models that were dirty before + return TPromise.join(dirtyTargetModels.map(dirtyTargetModel => this.models.loadOrCreate(dirtyTargetModel))).then(() => void 0); }, error => { - return this.backupFileService.discardResourceBackup(target).then(() => TPromise.wrapError(error)); + + // In case of an error, discard any dirty target backups that were made + return TPromise.join(dirtyTargetModels.map(dirtyTargetModel => this.backupFileService.discardResourceBackup(dirtyTargetModel))) + .then(() => TPromise.wrapError(error)); }); }); }); -- GitLab