diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts index 3617b55f41c..738e1406834 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts @@ -5,7 +5,7 @@ import * as assert from 'assert'; import * as vscode from 'vscode'; -import { createRandomFile, deleteFile, closeAllEditors, pathEquals, rndName, disposeAll, testFs } from '../utils'; +import { createRandomFile, deleteFile, closeAllEditors, pathEquals, rndName, disposeAll, testFs, delay } from '../utils'; import { join, posix, basename } from 'path'; import * as fs from 'fs'; @@ -587,13 +587,15 @@ suite('workspace-namespace', () => { }, cancellation.token); }); - test('applyEdit', () => { + test('applyEdit', async () => { + const doc = await vscode.workspace.openTextDocument(vscode.Uri.parse('untitled:' + join(vscode.workspace.rootPath || '', './new2.txt'))); - return vscode.workspace.openTextDocument(vscode.Uri.parse('untitled:' + join(vscode.workspace.rootPath || '', './new2.txt'))).then(doc => { - let edit = new vscode.WorkspaceEdit(); - edit.insert(doc.uri, new vscode.Position(0, 0), new Array(1000).join('Hello World')); - return vscode.workspace.applyEdit(edit); - }); + let edit = new vscode.WorkspaceEdit(); + edit.insert(doc.uri, new vscode.Position(0, 0), new Array(1000).join('Hello World')); + + let success = await vscode.workspace.applyEdit(edit); + assert.equal(success, true); + assert.equal(doc.isDirty, true); }); test('applyEdit should fail when editing deleted resource', async () => { @@ -630,19 +632,31 @@ suite('workspace-namespace', () => { }); test('applyEdit "edit A -> rename A to B -> edit B"', async () => { + await testEditRenameEdit(oldUri => oldUri.with({ path: oldUri.path + 'NEW' })); + }); + + test('applyEdit "edit A -> rename A to B (different case)" -> edit B', async () => { + await testEditRenameEdit(oldUri => oldUri.with({ path: oldUri.path.toUpperCase() })); + }); + + test('applyEdit "edit A -> rename A to B (same case)" -> edit B', async () => { + await testEditRenameEdit(oldUri => oldUri); + }); + + async function testEditRenameEdit(newUriCreator: (oldUri: vscode.Uri) => vscode.Uri): Promise { const oldUri = await createRandomFile(); - const newUri = oldUri.with({ path: oldUri.path + 'NEW' }); + const newUri = newUriCreator(oldUri); const edit = new vscode.WorkspaceEdit(); edit.insert(oldUri, new vscode.Position(0, 0), 'BEFORE'); edit.renameFile(oldUri, newUri); edit.insert(newUri, new vscode.Position(0, 0), 'AFTER'); - let success = await vscode.workspace.applyEdit(edit); - assert.equal(success, true); + assert.ok(await vscode.workspace.applyEdit(edit)); let doc = await vscode.workspace.openTextDocument(newUri); assert.equal(doc.getText(), 'AFTERBEFORE'); - }); + assert.equal(doc.isDirty, true); + } function nameWithUnderscore(uri: vscode.Uri) { return uri.with({ path: posix.join(posix.dirname(uri.path), `_${posix.basename(uri.path)}`) }); @@ -807,7 +821,7 @@ suite('workspace-namespace', () => { assert.ok(await vscode.workspace.applyEdit(we)); }); - test('The api workspace.applyEdit drops the TextEdit if there is a RenameFile later #77735', async function () { + test('WorkspaceEdit: insert & rename multiple', async function () { let [f1, f2, f3] = await Promise.all([createRandomFile(), createRandomFile(), createRandomFile()]); @@ -831,4 +845,56 @@ suite('workspace-namespace', () => { assert.ok(true); } }); + + test('workspace.applyEdit drops the TextEdit if there is a RenameFile later #77735 (with opened editor)', async function () { + await test77735(true); + }); + + test('workspace.applyEdit drops the TextEdit if there is a RenameFile later #77735 (without opened editor)', async function () { + await test77735(false); + }); + + async function test77735(withOpenedEditor: boolean): Promise { + const docUriOriginal = await createRandomFile(); + const docUriMoved = docUriOriginal.with({ path: `${docUriOriginal.path}.moved` }); + + if (withOpenedEditor) { + const document = await vscode.workspace.openTextDocument(docUriOriginal); + await vscode.window.showTextDocument(document); + } else { + await vscode.commands.executeCommand('workbench.action.closeAllEditors'); + } + + for (let i = 0; i < 4; i++) { + let we = new vscode.WorkspaceEdit(); + let oldUri: vscode.Uri; + let newUri: vscode.Uri; + let expected: string; + + if (i % 2 === 0) { + oldUri = docUriOriginal; + newUri = docUriMoved; + we.insert(oldUri, new vscode.Position(0, 0), 'Hello'); + expected = 'Hello'; + } else { + oldUri = docUriMoved; + newUri = docUriOriginal; + we.delete(oldUri, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 5))); + expected = ''; + } + + we.renameFile(oldUri, newUri); + assert.ok(await vscode.workspace.applyEdit(we)); + + const document = await vscode.workspace.openTextDocument(newUri); + assert.equal(document.isDirty, true); + + await document.save(); + assert.equal(document.isDirty, false); + + assert.equal(document.getText(), expected); + + await delay(10); + } + } }); diff --git a/extensions/vscode-api-tests/src/utils.ts b/extensions/vscode-api-tests/src/utils.ts index 38ede848840..969a7cd0051 100644 --- a/extensions/vscode-api-tests/src/utils.ts +++ b/extensions/vscode-api-tests/src/utils.ts @@ -67,3 +67,7 @@ export function conditionalTest(name: string, testCallback: (done: MochaDone) => function isTestTypeActive(): boolean { return !!vscode.extensions.getExtension('vscode-resolver-test'); } + +export function delay(ms: number) { + return new Promise(resolve => setTimeout(resolve, ms)); +} diff --git a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts index 3ece543b7d3..4fa61e19c83 100644 --- a/src/vs/workbench/services/textfile/common/textFileEditorModel.ts +++ b/src/vs/workbench/services/textfile/common/textFileEditorModel.ts @@ -470,7 +470,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil // We also want to trigger auto save if it is enabled to simulate the exact same behaviour // you would get if manually making the model dirty (fixes https://github.com/Microsoft/vscode/issues/16977) if (fromBackup) { - this.makeDirty(); + this.doMakeDirty(); if (this.autoSaveAfterMilliesEnabled) { this.doAutoSave(this.versionId); } @@ -549,7 +549,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil this.logService.trace('onModelContentChanged() - model content changed and marked as dirty', this.resource); // Mark as dirty - this.makeDirty(); + this.doMakeDirty(); // Start auto save process unless we are in conflict resolution mode and unless it is disabled if (this.autoSaveAfterMilliesEnabled) { @@ -564,7 +564,15 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil this.contentChangeEventScheduler.schedule(); } - private makeDirty(): void { + makeDirty(): void { + if (!this.isResolved()) { + return; // only resolved models can be marked dirty + } + + this.doMakeDirty(); + } + + private doMakeDirty(): void { // Track dirty state and version id const wasDirty = this.dirty; @@ -913,7 +921,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil TextFileEditorModel.saveErrorHandler.onSaveError(error, this); } - isDirty(): boolean { + isDirty(): this is IResolvedTextFileEditorModel { return this.dirty; } diff --git a/src/vs/workbench/services/textfile/common/textFileEditorModelManager.ts b/src/vs/workbench/services/textfile/common/textFileEditorModelManager.ts index 13f60f4629b..4de574861a0 100644 --- a/src/vs/workbench/services/textfile/common/textFileEditorModelManager.ts +++ b/src/vs/workbench/services/textfile/common/textFileEditorModelManager.ts @@ -74,11 +74,11 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE return this._onModelsReverted; } - private mapResourceToDisposeListener: ResourceMap; - private mapResourceToStateChangeListener: ResourceMap; - private mapResourceToModelContentChangeListener: ResourceMap; - private mapResourceToModel: ResourceMap; - private mapResourceToPendingModelLoaders: ResourceMap>; + private mapResourceToDisposeListener = new ResourceMap(); + private mapResourceToStateChangeListener = new ResourceMap(); + private mapResourceToModelContentChangeListener = new ResourceMap(); + private mapResourceToModel = new ResourceMap(); + private mapResourceToPendingModelLoaders = new ResourceMap>(); constructor( @ILifecycleService private readonly lifecycleService: ILifecycleService, @@ -86,12 +86,6 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE ) { super(); - this.mapResourceToModel = new ResourceMap(); - this.mapResourceToDisposeListener = new ResourceMap(); - this.mapResourceToStateChangeListener = new ResourceMap(); - this.mapResourceToModelContentChangeListener = new ResourceMap(); - this.mapResourceToPendingModelLoaders = new ResourceMap>(); - this.registerListeners(); } diff --git a/src/vs/workbench/services/textfile/common/textFileService.ts b/src/vs/workbench/services/textfile/common/textFileService.ts index a061414b910..713ef13e395 100644 --- a/src/vs/workbench/services/textfile/common/textFileService.ts +++ b/src/vs/workbench/services/textfile/common/textFileService.ts @@ -443,9 +443,94 @@ export abstract class TextFileService extends Disposable implements ITextFileSer } async move(source: URI, target: URI, overwrite?: boolean): Promise { + + // await onWillMove event joiners + await this.notifyOnWillMove(source, target); + + // find all models that related to either source or target (can be many if resource is a folder) + const sourceModels: ITextFileEditorModel[] = []; + const conflictingModels: ITextFileEditorModel[] = []; + for (const model of this.getFileModels()) { + const resource = model.getResource(); + + if (isEqualOrParent(resource, target, false /* do not ignorecase, see https://github.com/Microsoft/vscode/issues/56384 */)) { + conflictingModels.push(model); + } + + if (isEqualOrParent(resource, source)) { + sourceModels.push(model); + } + } + + // remember each source model to load again after move is done + // with optional content to restore if it was dirty + type ModelToRestore = { resource: URI; snapshot?: ITextSnapshot }; + const modelsToRestore: ModelToRestore[] = []; + for (const sourceModel of sourceModels) { + const sourceModelResource = sourceModel.getResource(); + + // If the source is the actual model, just use target as new resource + let modelToRestoreResource: URI; + if (isEqual(sourceModelResource, source)) { + modelToRestoreResource = target; + } + + // Otherwise a parent folder of the source is being moved, so we need + // to compute the target resource based on that + else { + modelToRestoreResource = joinPath(target, sourceModelResource.path.substr(source.path.length + 1)); + } + + const modelToRestore: ModelToRestore = { resource: modelToRestoreResource }; + if (sourceModel.isDirty()) { + modelToRestore.snapshot = sourceModel.createSnapshot(); + } + + modelsToRestore.push(modelToRestore); + } + + // in order to move, we need to soft revert all dirty models, + // both from the source as well as the target if any + const dirtyModels = [...sourceModels, ...conflictingModels].filter(model => model.isDirty()); + await this.revertAll(dirtyModels.map(dirtyModel => dirtyModel.getResource()), { soft: true }); + + // now we can rename the source to target via file operation + let stat: IFileStatWithMetadata; + try { + stat = await this.fileService.move(source, target, overwrite); + } catch (error) { + + // in case of any error, ensure to set dirty flag back + dirtyModels.forEach(dirtyModel => dirtyModel.makeDirty()); + + throw error; + } + + // finally, restore models that we had loaded previously + await Promise.all(modelsToRestore.map(async modelToRestore => { + + // restore the model, forcing a reload. this is important because + // we know the file has changed on disk after the move and the + // model might have still existed with the previous state. this + // ensures we are not tracking a stale state. + const restoredModel = await this.models.loadOrCreate(modelToRestore.resource, { reload: { async: false } }); + + // restore previous dirty content if any and ensure to mark + // the model as dirty + if (modelToRestore.snapshot && restoredModel.isResolved()) { + this.modelService.updateModel(restoredModel.textEditorModel, createTextBufferFactoryFromSnapshot(modelToRestore.snapshot)); + + restoredModel.makeDirty(); + } + })); + + return stat; + } + + private async notifyOnWillMove(source: URI, target: URI): Promise { const waitForPromises: Promise[] = []; - // Event + // fire event this._onWillMove.fire({ oldResource: source, newResource: target, @@ -458,58 +543,6 @@ export abstract class TextFileService extends Disposable implements ITextFileSer Object.freeze(waitForPromises); await Promise.all(waitForPromises); - - // Handle target models if existing (if target URI is a folder, this can be multiple) - const dirtyTargetModels = this.getDirtyFileModels().filter(model => isEqualOrParent(model.getResource(), target, false /* do not ignorecase, see https://github.com/Microsoft/vscode/issues/56384 */)); - if (dirtyTargetModels.length) { - await this.revertAll(dirtyTargetModels.map(targetModel => targetModel.getResource()), { soft: true }); - } - - // Handle dirty source models if existing (if source URI is a folder, this can be multiple) - const dirtySourceModels = this.getDirtyFileModels().filter(model => isEqualOrParent(model.getResource(), source)); - const dirtyTargetModelUris: URI[] = []; - if (dirtySourceModels.length) { - await Promise.all(dirtySourceModels.map(async 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)) { - 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: joinPath(target, sourceModelResource.path.substr(source.path.length + 1)).path }); - } - - // Remember as dirty target model to load after the operation - dirtyTargetModelUris.push(targetModelResource); - - // Backup dirty source model to the target resource it will become later - await sourceModel.backup(targetModelResource); - })); - } - - // Soft revert the dirty source files if any - await this.revertAll(dirtySourceModels.map(dirtySourceModel => dirtySourceModel.getResource()), { soft: true }); - - // Rename to target - try { - const stat = await this.fileService.move(source, target, overwrite); - - // Load models that were dirty before - await Promise.all(dirtyTargetModelUris.map(dirtyTargetModel => this.models.loadOrCreate(dirtyTargetModel))); - - return stat; - } catch (error) { - - // In case of an error, discard any dirty target backups that were made - await Promise.all(dirtyTargetModelUris.map(dirtyTargetModel => this.backupFileService.discardResourceBackup(dirtyTargetModel))); - - throw error; - } } //#endregion @@ -857,7 +890,7 @@ export abstract class TextFileService extends Disposable implements ITextFileSer return false; } - // take over encoding, mode (only if more specific) and model value from source model + // take over model value, encoding and mode (only if more specific) from source model targetModel.updatePreferredEncoding(sourceModel.getEncoding()); if (sourceModel.isResolved() && targetModel.isResolved()) { this.modelService.updateModel(targetModel.textEditorModel, createTextBufferFactoryFromSnapshot(sourceModel.createSnapshot())); diff --git a/src/vs/workbench/services/textfile/common/textfiles.ts b/src/vs/workbench/services/textfile/common/textfiles.ts index 5438af0f575..02956fd1cdd 100644 --- a/src/vs/workbench/services/textfile/common/textfiles.ts +++ b/src/vs/workbench/services/textfile/common/textfiles.ts @@ -470,7 +470,9 @@ export interface ITextFileEditorModel extends ITextEditorModel, IEncodingSupport hasBackup(): boolean; - isDirty(): boolean; + isDirty(): this is IResolvedTextFileEditorModel; + + makeDirty(): void; isResolved(): this is IResolvedTextFileEditorModel; diff --git a/src/vs/workbench/services/textfile/test/textFileEditorModel.test.ts b/src/vs/workbench/services/textfile/test/textFileEditorModel.test.ts index e22cbe034db..b5609d42d32 100644 --- a/src/vs/workbench/services/textfile/test/textFileEditorModel.test.ts +++ b/src/vs/workbench/services/textfile/test/textFileEditorModel.test.ts @@ -219,6 +219,33 @@ suite('Files - TextFileEditorModel', () => { assert.ok(model.isDirty()); }); + test('Make Dirty', async function () { + let eventCounter = 0; + + const model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/index_async.txt'), 'utf8', undefined); + + model.makeDirty(); + assert.ok(!model.isDirty()); // needs to be resolved + + await model.load(); + model.textEditorModel!.setValue('foo'); + assert.ok(model.isDirty()); + + await model.revert(true /* soft revert */); + assert.ok(!model.isDirty()); + + model.onDidStateChange(e => { + if (e === StateChange.DIRTY) { + eventCounter++; + } + }); + + model.makeDirty(); + assert.ok(model.isDirty()); + assert.equal(eventCounter, 1); + model.dispose(); + }); + test('File not modified error is handled gracefully', async function () { let model: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/index_async.txt'), 'utf8', undefined); diff --git a/src/vs/workbench/services/textfile/test/textFileService.test.ts b/src/vs/workbench/services/textfile/test/textFileService.test.ts index 30079948c80..07676420066 100644 --- a/src/vs/workbench/services/textfile/test/textFileService.test.ts +++ b/src/vs/workbench/services/textfile/test/textFileService.test.ts @@ -272,8 +272,16 @@ suite('Files - TextFileService', () => { }); test('move - dirty file', async function () { - let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8', undefined); - let targetModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file_target.txt'), 'utf8', undefined); + await testMove(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt')); + }); + + test('move - dirty file (target exists and is dirty)', async function () { + await testMove(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'), true); + }); + + async function testMove(source: URI, target: URI, targetDirty?: boolean): Promise { + let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, source, 'utf8', undefined); + let targetModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, target, 'utf8', undefined); (accessor.textFileService.models).add(sourceModel.getResource(), sourceModel); (accessor.textFileService.models).add(targetModel.getResource(), targetModel); @@ -283,11 +291,22 @@ suite('Files - TextFileService', () => { sourceModel.textEditorModel!.setValue('foo'); assert.ok(service.isDirty(sourceModel.getResource())); + if (targetDirty) { + await targetModel.load(); + targetModel.textEditorModel!.setValue('bar'); + assert.ok(service.isDirty(targetModel.getResource())); + } + await service.move(sourceModel.getResource(), targetModel.getResource(), true); + + assert.equal(targetModel.textEditorModel!.getValue(), 'foo'); + assert.ok(!service.isDirty(sourceModel.getResource())); + assert.ok(service.isDirty(targetModel.getResource())); + sourceModel.dispose(); targetModel.dispose(); - }); + } suite('Hot Exit', () => { suite('"onExit" setting', () => {