maxmustermann/vscode
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

fix #77735

Browse source
This commit is contained in:
Benjamin Pasero 2019-08-12 08:13:10 +02:00
parent 5117a8e61f
commit 5099e36e3d
8 changed files with 238 additions and 85 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

90
extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts
View file

@ -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<void> {
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<void> {
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);
}
}
});

4
extensions/vscode-api-tests/src/utils.ts
View file

@ -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));
}

16
src/vs/workbench/services/textfile/common/textFileEditorModel.ts
View file

@ -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;
}

16
src/vs/workbench/services/textfile/common/textFileEditorModelManager.ts
View file

@ -74,11 +74,11 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
return this._onModelsReverted;
}
private mapResourceToDisposeListener: ResourceMap<IDisposable>;
private mapResourceToStateChangeListener: ResourceMap<IDisposable>;
private mapResourceToModelContentChangeListener: ResourceMap<IDisposable>;
private mapResourceToModel: ResourceMap<ITextFileEditorModel>;
private mapResourceToPendingModelLoaders: ResourceMap<Promise<ITextFileEditorModel>>;
private mapResourceToDisposeListener = new ResourceMap<IDisposable>();
private mapResourceToStateChangeListener = new ResourceMap<IDisposable>();
private mapResourceToModelContentChangeListener = new ResourceMap<IDisposable>();
private mapResourceToModel = new ResourceMap<ITextFileEditorModel>();
private mapResourceToPendingModelLoaders = new ResourceMap<Promise<ITextFileEditorModel>>();
constructor(
@ILifecycleService private readonly lifecycleService: ILifecycleService,
@ -86,12 +86,6 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
) {
super();
this.mapResourceToModel = new ResourceMap<ITextFileEditorModel>();
this.mapResourceToDisposeListener = new ResourceMap<IDisposable>();
this.mapResourceToStateChangeListener = new ResourceMap<IDisposable>();
this.mapResourceToModelContentChangeListener = new ResourceMap<IDisposable>();
this.mapResourceToPendingModelLoaders = new ResourceMap<Promise<ITextFileEditorModel>>();
this.registerListeners();
}

141
src/vs/workbench/services/textfile/common/textFileService.ts
View file

@ -443,9 +443,94 @@ export abstract class TextFileService extends Disposable implements ITextFileSer
}
async move(source: URI, target: URI, overwrite?: boolean): Promise<IFileStatWithMetadata> {
// 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<void> {
const waitForPromises: Promise<unknown>[] = [];
// 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()));

4
src/vs/workbench/services/textfile/common/textfiles.ts
View file

@ -470,7 +470,9 @@ export interface ITextFileEditorModel extends ITextEditorModel, IEncodingSupport
hasBackup(): boolean;
isDirty(): boolean;
isDirty(): this is IResolvedTextFileEditorModel;
makeDirty(): void;
isResolved(): this is IResolvedTextFileEditorModel;

27
src/vs/workbench/services/textfile/test/textFileEditorModel.test.ts
View file

@ -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);

25
src/vs/workbench/services/textfile/test/textFileService.test.ts
View file

@ -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<void> {
let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, source, 'utf8', undefined);
let targetModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, target, 'utf8', undefined);
(<TextFileEditorModelManager>accessor.textFileService.models).add(sourceModel.getResource(), sourceModel);
(<TextFileEditorModelManager>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', () => {