Skip to content

Commit 5099e36

Browse files
author
Benjamin Pasero
committed
1 parent 5117a8e commit 5099e36

8 files changed

Lines changed: 234 additions & 81 deletions

File tree

‎extensions/vscode-api-tests/src/singlefolder-tests/workspace.test.ts‎

Lines changed: 78 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import * as assert from 'assert';
77
import * as vscode from 'vscode';
8-
import { createRandomFile, deleteFile, closeAllEditors, pathEquals, rndName, disposeAll, testFs } from '../utils';
8+
import { createRandomFile, deleteFile, closeAllEditors, pathEquals, rndName, disposeAll, testFs, delay } from '../utils';
99
import { join, posix, basename } from 'path';
1010
import * as fs from 'fs';
1111

@@ -587,13 +587,15 @@ suite('workspace-namespace', () => {
587587
}, cancellation.token);
588588
});
589589

590-
test('applyEdit', () => {
590+
test('applyEdit', async () => {
591+
const doc = await vscode.workspace.openTextDocument(vscode.Uri.parse('untitled:' + join(vscode.workspace.rootPath || '', './new2.txt')));
591592

592-
return vscode.workspace.openTextDocument(vscode.Uri.parse('untitled:' + join(vscode.workspace.rootPath || '', './new2.txt'))).then(doc => {
593-
let edit = new vscode.WorkspaceEdit();
594-
edit.insert(doc.uri, new vscode.Position(0, 0), new Array(1000).join('Hello World'));
595-
return vscode.workspace.applyEdit(edit);
596-
});
593+
let edit = new vscode.WorkspaceEdit();
594+
edit.insert(doc.uri, new vscode.Position(0, 0), new Array(1000).join('Hello World'));
595+
596+
let success = await vscode.workspace.applyEdit(edit);
597+
assert.equal(success, true);
598+
assert.equal(doc.isDirty, true);
597599
});
598600

599601
test('applyEdit should fail when editing deleted resource', async () => {
@@ -630,19 +632,31 @@ suite('workspace-namespace', () => {
630632
});
631633

632634
test('applyEdit "edit A -> rename A to B -> edit B"', async () => {
635+
await testEditRenameEdit(oldUri => oldUri.with({ path: oldUri.path + 'NEW' }));
636+
});
637+
638+
test('applyEdit "edit A -> rename A to B (different case)" -> edit B', async () => {
639+
await testEditRenameEdit(oldUri => oldUri.with({ path: oldUri.path.toUpperCase() }));
640+
});
641+
642+
test('applyEdit "edit A -> rename A to B (same case)" -> edit B', async () => {
643+
await testEditRenameEdit(oldUri => oldUri);
644+
});
645+
646+
async function testEditRenameEdit(newUriCreator: (oldUri: vscode.Uri) => vscode.Uri): Promise<void> {
633647
const oldUri = await createRandomFile();
634-
const newUri = oldUri.with({ path: oldUri.path + 'NEW' });
648+
const newUri = newUriCreator(oldUri);
635649
const edit = new vscode.WorkspaceEdit();
636650
edit.insert(oldUri, new vscode.Position(0, 0), 'BEFORE');
637651
edit.renameFile(oldUri, newUri);
638652
edit.insert(newUri, new vscode.Position(0, 0), 'AFTER');
639653

640-
let success = await vscode.workspace.applyEdit(edit);
641-
assert.equal(success, true);
654+
assert.ok(await vscode.workspace.applyEdit(edit));
642655

643656
let doc = await vscode.workspace.openTextDocument(newUri);
644657
assert.equal(doc.getText(), 'AFTERBEFORE');
645-
});
658+
assert.equal(doc.isDirty, true);
659+
}
646660

647661
function nameWithUnderscore(uri: vscode.Uri) {
648662
return uri.with({ path: posix.join(posix.dirname(uri.path), `_${posix.basename(uri.path)}`) });
@@ -807,7 +821,7 @@ suite('workspace-namespace', () => {
807821
assert.ok(await vscode.workspace.applyEdit(we));
808822
});
809823

810-
test('The api workspace.applyEdit drops the TextEdit if there is a RenameFile later #77735', async function () {
824+
test('WorkspaceEdit: insert & rename multiple', async function () {
811825

812826
let [f1, f2, f3] = await Promise.all([createRandomFile(), createRandomFile(), createRandomFile()]);
813827

@@ -831,4 +845,56 @@ suite('workspace-namespace', () => {
831845
assert.ok(true);
832846
}
833847
});
848+
849+
test('workspace.applyEdit drops the TextEdit if there is a RenameFile later #77735 (with opened editor)', async function () {
850+
await test77735(true);
851+
});
852+
853+
test('workspace.applyEdit drops the TextEdit if there is a RenameFile later #77735 (without opened editor)', async function () {
854+
await test77735(false);
855+
});
856+
857+
async function test77735(withOpenedEditor: boolean): Promise<void> {
858+
const docUriOriginal = await createRandomFile();
859+
const docUriMoved = docUriOriginal.with({ path: `${docUriOriginal.path}.moved` });
860+
861+
if (withOpenedEditor) {
862+
const document = await vscode.workspace.openTextDocument(docUriOriginal);
863+
await vscode.window.showTextDocument(document);
864+
} else {
865+
await vscode.commands.executeCommand('workbench.action.closeAllEditors');
866+
}
867+
868+
for (let i = 0; i < 4; i++) {
869+
let we = new vscode.WorkspaceEdit();
870+
let oldUri: vscode.Uri;
871+
let newUri: vscode.Uri;
872+
let expected: string;
873+
874+
if (i % 2 === 0) {
875+
oldUri = docUriOriginal;
876+
newUri = docUriMoved;
877+
we.insert(oldUri, new vscode.Position(0, 0), 'Hello');
878+
expected = 'Hello';
879+
} else {
880+
oldUri = docUriMoved;
881+
newUri = docUriOriginal;
882+
we.delete(oldUri, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 5)));
883+
expected = '';
884+
}
885+
886+
we.renameFile(oldUri, newUri);
887+
assert.ok(await vscode.workspace.applyEdit(we));
888+
889+
const document = await vscode.workspace.openTextDocument(newUri);
890+
assert.equal(document.isDirty, true);
891+
892+
await document.save();
893+
assert.equal(document.isDirty, false);
894+
895+
assert.equal(document.getText(), expected);
896+
897+
await delay(10);
898+
}
899+
}
834900
});

‎extensions/vscode-api-tests/src/utils.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,7 @@ export function conditionalTest(name: string, testCallback: (done: MochaDone) =>
6767
function isTestTypeActive(): boolean {
6868
return !!vscode.extensions.getExtension('vscode-resolver-test');
6969
}
70+
71+
export function delay(ms: number) {
72+
return new Promise(resolve => setTimeout(resolve, ms));
73+
}

‎src/vs/workbench/services/textfile/common/textFileEditorModel.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
470470
// We also want to trigger auto save if it is enabled to simulate the exact same behaviour
471471
// you would get if manually making the model dirty (fixes https://github.com/Microsoft/vscode/issues/16977)
472472
if (fromBackup) {
473-
this.makeDirty();
473+
this.doMakeDirty();
474474
if (this.autoSaveAfterMilliesEnabled) {
475475
this.doAutoSave(this.versionId);
476476
}
@@ -549,7 +549,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
549549
this.logService.trace('onModelContentChanged() - model content changed and marked as dirty', this.resource);
550550

551551
// Mark as dirty
552-
this.makeDirty();
552+
this.doMakeDirty();
553553

554554
// Start auto save process unless we are in conflict resolution mode and unless it is disabled
555555
if (this.autoSaveAfterMilliesEnabled) {
@@ -564,7 +564,15 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
564564
this.contentChangeEventScheduler.schedule();
565565
}
566566

567-
private makeDirty(): void {
567+
makeDirty(): void {
568+
if (!this.isResolved()) {
569+
return; // only resolved models can be marked dirty
570+
}
571+
572+
this.doMakeDirty();
573+
}
574+
575+
private doMakeDirty(): void {
568576

569577
// Track dirty state and version id
570578
const wasDirty = this.dirty;
@@ -913,7 +921,7 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
913921
TextFileEditorModel.saveErrorHandler.onSaveError(error, this);
914922
}
915923

916-
isDirty(): boolean {
924+
isDirty(): this is IResolvedTextFileEditorModel {
917925
return this.dirty;
918926
}
919927

‎src/vs/workbench/services/textfile/common/textFileEditorModelManager.ts‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,24 +74,18 @@ export class TextFileEditorModelManager extends Disposable implements ITextFileE
7474
return this._onModelsReverted;
7575
}
7676

77-
private mapResourceToDisposeListener: ResourceMap<IDisposable>;
78-
private mapResourceToStateChangeListener: ResourceMap<IDisposable>;
79-
private mapResourceToModelContentChangeListener: ResourceMap<IDisposable>;
80-
private mapResourceToModel: ResourceMap<ITextFileEditorModel>;
81-
private mapResourceToPendingModelLoaders: ResourceMap<Promise<ITextFileEditorModel>>;
77+
private mapResourceToDisposeListener = new ResourceMap<IDisposable>();
78+
private mapResourceToStateChangeListener = new ResourceMap<IDisposable>();
79+
private mapResourceToModelContentChangeListener = new ResourceMap<IDisposable>();
80+
private mapResourceToModel = new ResourceMap<ITextFileEditorModel>();
81+
private mapResourceToPendingModelLoaders = new ResourceMap<Promise<ITextFileEditorModel>>();
8282

8383
constructor(
8484
@ILifecycleService private readonly lifecycleService: ILifecycleService,
8585
@IInstantiationService private readonly instantiationService: IInstantiationService
8686
) {
8787
super();
8888

89-
this.mapResourceToModel = new ResourceMap<ITextFileEditorModel>();
90-
this.mapResourceToDisposeListener = new ResourceMap<IDisposable>();
91-
this.mapResourceToStateChangeListener = new ResourceMap<IDisposable>();
92-
this.mapResourceToModelContentChangeListener = new ResourceMap<IDisposable>();
93-
this.mapResourceToPendingModelLoaders = new ResourceMap<Promise<ITextFileEditorModel>>();
94-
9589
this.registerListeners();
9690
}
9791

‎src/vs/workbench/services/textfile/common/textFileService.ts‎

Lines changed: 83 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -443,73 +443,106 @@ export abstract class TextFileService extends Disposable implements ITextFileSer
443443
}
444444

445445
async move(source: URI, target: URI, overwrite?: boolean): Promise<IFileStatWithMetadata> {
446-
const waitForPromises: Promise<unknown>[] = [];
447446

448-
// Event
449-
this._onWillMove.fire({
450-
oldResource: source,
451-
newResource: target,
452-
waitUntil(promise: Promise<unknown>) {
453-
waitForPromises.push(promise.then(undefined, errors.onUnexpectedError));
454-
}
455-
});
447+
// await onWillMove event joiners
448+
await this.notifyOnWillMove(source, target);
456449

457-
// prevent async waitUntil-calls
458-
Object.freeze(waitForPromises);
450+
// find all models that related to either source or target (can be many if resource is a folder)
451+
const sourceModels: ITextFileEditorModel[] = [];
452+
const conflictingModels: ITextFileEditorModel[] = [];
453+
for (const model of this.getFileModels()) {
454+
const resource = model.getResource();
459455

460-
await Promise.all(waitForPromises);
456+
if (isEqualOrParent(resource, target, false /* do not ignorecase, see https://github.com/Microsoft/vscode/issues/56384 */)) {
457+
conflictingModels.push(model);
458+
}
461459

462-
// Handle target models if existing (if target URI is a folder, this can be multiple)
463-
const dirtyTargetModels = this.getDirtyFileModels().filter(model => isEqualOrParent(model.getResource(), target, false /* do not ignorecase, see https://github.com/Microsoft/vscode/issues/56384 */));
464-
if (dirtyTargetModels.length) {
465-
await this.revertAll(dirtyTargetModels.map(targetModel => targetModel.getResource()), { soft: true });
460+
if (isEqualOrParent(resource, source)) {
461+
sourceModels.push(model);
462+
}
466463
}
467464

468-
// Handle dirty source models if existing (if source URI is a folder, this can be multiple)
469-
const dirtySourceModels = this.getDirtyFileModels().filter(model => isEqualOrParent(model.getResource(), source));
470-
const dirtyTargetModelUris: URI[] = [];
471-
if (dirtySourceModels.length) {
472-
await Promise.all(dirtySourceModels.map(async sourceModel => {
473-
const sourceModelResource = sourceModel.getResource();
474-
let targetModelResource: URI;
475-
476-
// If the source is the actual model, just use target as new resource
477-
if (isEqual(sourceModelResource, source)) {
478-
targetModelResource = target;
479-
}
465+
// remember each source model to load again after move is done
466+
// with optional content to restore if it was dirty
467+
type ModelToRestore = { resource: URI; snapshot?: ITextSnapshot };
468+
const modelsToRestore: ModelToRestore[] = [];
469+
for (const sourceModel of sourceModels) {
470+
const sourceModelResource = sourceModel.getResource();
471+
472+
// If the source is the actual model, just use target as new resource
473+
let modelToRestoreResource: URI;
474+
if (isEqual(sourceModelResource, source)) {
475+
modelToRestoreResource = target;
476+
}
480477

481-
// Otherwise a parent folder of the source is being moved, so we need
482-
// to compute the target resource based on that
483-
else {
484-
targetModelResource = sourceModelResource.with({ path: joinPath(target, sourceModelResource.path.substr(source.path.length + 1)).path });
485-
}
478+
// Otherwise a parent folder of the source is being moved, so we need
479+
// to compute the target resource based on that
480+
else {
481+
modelToRestoreResource = joinPath(target, sourceModelResource.path.substr(source.path.length + 1));
482+
}
486483

487-
// Remember as dirty target model to load after the operation
488-
dirtyTargetModelUris.push(targetModelResource);
484+
const modelToRestore: ModelToRestore = { resource: modelToRestoreResource };
485+
if (sourceModel.isDirty()) {
486+
modelToRestore.snapshot = sourceModel.createSnapshot();
487+
}
489488

490-
// Backup dirty source model to the target resource it will become later
491-
await sourceModel.backup(targetModelResource);
492-
}));
489+
modelsToRestore.push(modelToRestore);
493490
}
494491

495-
// Soft revert the dirty source files if any
496-
await this.revertAll(dirtySourceModels.map(dirtySourceModel => dirtySourceModel.getResource()), { soft: true });
492+
// in order to move, we need to soft revert all dirty models,
493+
// both from the source as well as the target if any
494+
const dirtyModels = [...sourceModels, ...conflictingModels].filter(model => model.isDirty());
495+
await this.revertAll(dirtyModels.map(dirtyModel => dirtyModel.getResource()), { soft: true });
497496

498-
// Rename to target
497+
// now we can rename the source to target via file operation
498+
let stat: IFileStatWithMetadata;
499499
try {
500-
const stat = await this.fileService.move(source, target, overwrite);
501-
502-
// Load models that were dirty before
503-
await Promise.all(dirtyTargetModelUris.map(dirtyTargetModel => this.models.loadOrCreate(dirtyTargetModel)));
504-
505-
return stat;
500+
stat = await this.fileService.move(source, target, overwrite);
506501
} catch (error) {
507502

508-
// In case of an error, discard any dirty target backups that were made
509-
await Promise.all(dirtyTargetModelUris.map(dirtyTargetModel => this.backupFileService.discardResourceBackup(dirtyTargetModel)));
503+
// in case of any error, ensure to set dirty flag back
504+
dirtyModels.forEach(dirtyModel => dirtyModel.makeDirty());
510505

511506
throw error;
512507
}
508+
509+
// finally, restore models that we had loaded previously
510+
await Promise.all(modelsToRestore.map(async modelToRestore => {
511+
512+
// restore the model, forcing a reload. this is important because
513+
// we know the file has changed on disk after the move and the
514+
// model might have still existed with the previous state. this
515+
// ensures we are not tracking a stale state.
516+
const restoredModel = await this.models.loadOrCreate(modelToRestore.resource, { reload: { async: false } });
517+
518+
// restore previous dirty content if any and ensure to mark
519+
// the model as dirty
520+
if (modelToRestore.snapshot && restoredModel.isResolved()) {
521+
this.modelService.updateModel(restoredModel.textEditorModel, createTextBufferFactoryFromSnapshot(modelToRestore.snapshot));
522+
523+
restoredModel.makeDirty();
524+
}
525+
}));
526+
527+
return stat;
528+
}
529+
530+
private async notifyOnWillMove(source: URI, target: URI): Promise<void> {
531+
const waitForPromises: Promise<unknown>[] = [];
532+
533+
// fire event
534+
this._onWillMove.fire({
535+
oldResource: source,
536+
newResource: target,
537+
waitUntil(promise: Promise<unknown>) {
538+
waitForPromises.push(promise.then(undefined, errors.onUnexpectedError));
539+
}
540+
});
541+
542+
// prevent async waitUntil-calls
543+
Object.freeze(waitForPromises);
544+
545+
await Promise.all(waitForPromises);
513546
}
514547

515548
//#endregion
@@ -857,7 +890,7 @@ export abstract class TextFileService extends Disposable implements ITextFileSer
857890
return false;
858891
}
859892

860-
// take over encoding, mode (only if more specific) and model value from source model
893+
// take over model value, encoding and mode (only if more specific) from source model
861894
targetModel.updatePreferredEncoding(sourceModel.getEncoding());
862895
if (sourceModel.isResolved() && targetModel.isResolved()) {
863896
this.modelService.updateModel(targetModel.textEditorModel, createTextBufferFactoryFromSnapshot(sourceModel.createSnapshot()));

‎src/vs/workbench/services/textfile/common/textfiles.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,9 @@ export interface ITextFileEditorModel extends ITextEditorModel, IEncodingSupport
470470

471471
hasBackup(): boolean;
472472

473-
isDirty(): boolean;
473+
isDirty(): this is IResolvedTextFileEditorModel;
474+
475+
makeDirty(): void;
474476

475477
isResolved(): this is IResolvedTextFileEditorModel;
476478

0 commit comments

Comments
 (0)