From 11c2ef633537f0c0f2bd457fa4be9f2c32ff2a60 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Fri, 15 Jun 2018 17:48:59 +0200 Subject: [PATCH] fix #42640 --- .../files/electron-browser/fileActions.ts | 2 +- .../electron-browser/bulkEditService.ts | 9 ++-- .../textfile/common/textFileService.ts | 52 +++++++++++++++++++ .../services/textfile/common/textfiles.ts | 24 ++++++--- .../textfile/test/textFileService.test.ts | 39 ++++++++++++++ .../api/mainThreadEditors.test.ts | 17 +++--- 6 files changed, 123 insertions(+), 20 deletions(-) diff --git a/src/vs/workbench/parts/files/electron-browser/fileActions.ts b/src/vs/workbench/parts/files/electron-browser/fileActions.ts index 725875e9e81..40cd32a481d 100644 --- a/src/vs/workbench/parts/files/electron-browser/fileActions.ts +++ b/src/vs/workbench/parts/files/electron-browser/fileActions.ts @@ -308,7 +308,7 @@ class RenameFileAction extends BaseRenameAction { renamed = this.element.parent.resource.with({ path: targetPath }); } - // Otherwise, a parent of the dirty resource got moved, so we have to reparent more complicated. Example: + // 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)) }); } diff --git a/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts b/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts index 13c9cf74de2..cfa25c08037 100644 --- a/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts +++ b/src/vs/workbench/services/bulkEdit/electron-browser/bulkEditService.ts @@ -25,6 +25,7 @@ import { IProgress, IProgressRunner, emptyProgressRunner } from 'vs/platform/pro import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { IEnvironmentService } from 'vs/platform/environment/common/environment'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; +import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles'; abstract class Recording { @@ -269,6 +270,7 @@ export class BulkEdit { progress: IProgressRunner, @ITextModelService private readonly _textModelService: ITextModelService, @IFileService private readonly _fileService: IFileService, + @ITextFileService private readonly _textFileService: ITextFileService, @IEnvironmentService private readonly _environmentService: IEnvironmentService, @IWorkspaceContextService private readonly _contextService: IWorkspaceContextService ) { @@ -345,9 +347,9 @@ export class BulkEdit { progress.report(undefined); if (edit.newUri && edit.oldUri) { - await this._fileService.moveFile(edit.oldUri, edit.newUri, false); + await this._textFileService.move(edit.oldUri, edit.newUri, false); } else if (!edit.newUri && edit.oldUri) { - await this._fileService.del(edit.oldUri, true); + await this._textFileService.delete(edit.oldUri, true); } else if (edit.newUri && !edit.oldUri) { await this._fileService.createFile(edit.newUri, undefined, { overwrite: false }); } @@ -387,6 +389,7 @@ export class BulkEditService implements IBulkEditService { @IEditorService private readonly _editorService: IEditorService, @ITextModelService private readonly _textModelService: ITextModelService, @IFileService private readonly _fileService: IFileService, + @ITextFileService private readonly _textFileService: ITextFileService, @IEnvironmentService private readonly _environmentService: IEnvironmentService, @IWorkspaceContextService private readonly _contextService: IWorkspaceContextService ) { @@ -419,7 +422,7 @@ export class BulkEditService implements IBulkEditService { } } - const bulkEdit = new BulkEdit(options.editor, options.progress, this._textModelService, this._fileService, this._environmentService, this._contextService); + const bulkEdit = new BulkEdit(options.editor, options.progress, this._textModelService, this._fileService, this._textFileService, this._environmentService, this._contextService); bulkEdit.add(edits); return bulkEdit.perform().then(selection => { return { diff --git a/src/vs/workbench/services/textfile/common/textFileService.ts b/src/vs/workbench/services/textfile/common/textFileService.ts index 4c86dfd430c..f413f4b9210 100644 --- a/src/vs/workbench/services/textfile/common/textFileService.ts +++ b/src/vs/workbench/services/textfile/common/textFileService.ts @@ -701,6 +701,58 @@ 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)); + } + + public move(source: URI, target: URI, overwrite?: boolean): TPromise { + + // Handle target model if existing + 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 }); + } + + 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)); + } else { + handleSourceModelPromise = TPromise.as(false); + } + + return handleSourceModelPromise.then(dirty => { + + // Soft revert the source file + return this.revert(source, { 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; + }, error => { + return this.backupFileService.discardResourceBackup(target).then(() => TPromise.wrapError(error)); + }); + }); + }); + }); + } + public getAutoSaveMode(): AutoSaveMode { if (this.configuredAutoSaveOnFocusChange) { return AutoSaveMode.ON_FOCUS_CHANGE; diff --git a/src/vs/workbench/services/textfile/common/textfiles.ts b/src/vs/workbench/services/textfile/common/textfiles.ts index 7bcde710cea..c5c6f3164dd 100644 --- a/src/vs/workbench/services/textfile/common/textfiles.ts +++ b/src/vs/workbench/services/textfile/common/textfiles.ts @@ -237,13 +237,16 @@ export interface ITextFileEditorModel extends ITextEditorModel, IEncodingSupport export interface ITextFileService extends IDisposable { _serviceBrand: any; - onAutoSaveConfigurationChange: Event; - onFilesAssociationChange: Event; + + readonly onAutoSaveConfigurationChange: Event; + readonly onFilesAssociationChange: Event; + + readonly isHotExitEnabled: boolean; /** * Access to the manager of text file editor models providing further methods to work with them. */ - models: ITextFileEditorModelManager; + readonly models: ITextFileEditorModelManager; /** * Resolve the contents of a file identified by the resource. @@ -307,6 +310,16 @@ export interface ITextFileService extends IDisposable { */ revertAll(resources?: URI[], options?: IRevertOptions): TPromise; + /** + * Delete a file. If the file is dirty, it will get reverted and then deleted from disk. + */ + delete(resource: URI, useTrash?: boolean): TPromise; + + /** + * Move a file. If the file is dirty, its contents will be preserved and restored. + */ + move(source: URI, target: URI, overwrite?: boolean): TPromise; + /** * Brings up the confirm dialog to either save, don't save or cancel. * @@ -324,9 +337,4 @@ export interface ITextFileService extends IDisposable { * Convinient fast access to the raw configured auto save settings. */ getAutoSaveConfiguration(): IAutoSaveConfiguration; - - /** - * Convinient fast access to the hot exit file setting. - */ - isHotExitEnabled: boolean; } diff --git a/src/vs/workbench/services/textfile/test/textFileService.test.ts b/src/vs/workbench/services/textfile/test/textFileService.test.ts index e6d16134bfd..216f1742075 100644 --- a/src/vs/workbench/services/textfile/test/textFileService.test.ts +++ b/src/vs/workbench/services/textfile/test/textFileService.test.ts @@ -252,6 +252,45 @@ suite('Files - TextFileService', () => { }); }); + test('delete - dirty file', function () { + model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8'); + (accessor.textFileService.models).add(model.getResource(), model); + + const service = accessor.textFileService; + + return model.load().then(() => { + model.textEditorModel.setValue('foo'); + + assert.ok(service.isDirty(model.getResource())); + + return service.delete(model.getResource()).then(() => { + assert.ok(!service.isDirty(model.getResource())); + }); + }); + }); + + test('move - dirty file', function () { + let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8'); + let targetModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file_target.txt'), 'utf8'); + (accessor.textFileService.models).add(sourceModel.getResource(), sourceModel); + (accessor.textFileService.models).add(targetModel.getResource(), targetModel); + + const service = accessor.textFileService; + + return sourceModel.load().then(() => { + sourceModel.textEditorModel.setValue('foo'); + + assert.ok(service.isDirty(sourceModel.getResource())); + + return service.move(sourceModel.getResource(), targetModel.getResource(), true).then(() => { + assert.ok(!service.isDirty(sourceModel.getResource())); + + sourceModel.dispose(); + targetModel.dispose(); + }); + }); + }); + suite('Hot Exit', () => { suite('"onExit" setting', () => { test('should hot exit on non-Mac (reason: CLOSE, windows: single, workspace)', function () { diff --git a/src/vs/workbench/test/electron-browser/api/mainThreadEditors.test.ts b/src/vs/workbench/test/electron-browser/api/mainThreadEditors.test.ts index c9d29d87348..25d71aee4d8 100644 --- a/src/vs/workbench/test/electron-browser/api/mainThreadEditors.test.ts +++ b/src/vs/workbench/test/electron-browser/api/mainThreadEditors.test.ts @@ -48,22 +48,23 @@ suite('MainThreadEditors', () => { deletedResources.clear(); const fileService = new class extends TestFileService { - async moveFile(from: URI, target: URI): TPromise { - movedResources.set(from, target); - return createMockFileStat(target); - } async createFile(uri: URI): TPromise { createdResources.add(uri); return createMockFileStat(uri); } - async del(uri: URI): TPromise { - deletedResources.add(uri); - } }; const textFileService = new class extends mock() { isDirty() { return false; } + delete(resource: URI) { + deletedResources.add(resource); + return TPromise.as(void 0); + } + move(source: URI, target: URI) { + movedResources.set(source, target); + return TPromise.as(void 0); + } models = { onModelSaved: Event.None, onModelReverted: Event.None, @@ -73,7 +74,7 @@ suite('MainThreadEditors', () => { const workbenchEditorService = new TestEditorService(); const editorGroupService = new TestEditorGroupsService(); - const bulkEditService = new BulkEditService(modelService, new TestEditorService(), null, fileService, TestEnvironmentService, new TestContextService()); + const bulkEditService = new BulkEditService(modelService, new TestEditorService(), null, fileService, textFileService, TestEnvironmentService, new TestContextService()); const rpcProtocol = new TestRPCProtocol(); rpcProtocol.set(ExtHostContext.ExtHostDocuments, new class extends mock() { -- GitLab