Explorer: copy() of dirty file reverts the source if dirty (fix #89217)

This commit is contained in:
Benjamin Pasero 2020-01-24 11:37:10 +01:00
parent 4ba2070818
commit 10cbb8533f
3 changed files with 45 additions and 15 deletions

View file

@ -192,12 +192,12 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
// find all models that related to either source or target (can be many if resource is a folder) // find all models that related to either source or target (can be many if resource is a folder)
const sourceModels: ITextFileEditorModel[] = []; const sourceModels: ITextFileEditorModel[] = [];
const conflictingModels: ITextFileEditorModel[] = []; const targetModels: ITextFileEditorModel[] = [];
for (const model of this.getFileModels()) { for (const model of this.getFileModels()) {
const resource = model.resource; const resource = model.resource;
if (isEqualOrParent(resource, target, false /* do not ignorecase, see https://github.com/Microsoft/vscode/issues/56384 */)) { if (isEqualOrParent(resource, target, false /* do not ignorecase, see https://github.com/Microsoft/vscode/issues/56384 */)) {
conflictingModels.push(model); targetModels.push(model);
} }
if (isEqualOrParent(resource, source)) { if (isEqualOrParent(resource, source)) {
@ -232,10 +232,11 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
modelsToRestore.push(modelToRestore); modelsToRestore.push(modelToRestore);
} }
// in order to move and copy, we need to soft revert all dirty models, // handle dirty models depending on the operation:
// both from the source as well as the target if any // - move: revert both source and target (if any)
const dirtyModels = [...sourceModels, ...conflictingModels].filter(model => model.isDirty()); // - copy: revert target (if any)
await this.doRevertFiles(dirtyModels.map(dirtyModel => dirtyModel.resource), { soft: true }); const dirtyModelsToRevert = (move ? [...sourceModels, ...targetModels] : [...targetModels]).filter(model => model.isDirty());
await this.doRevertFiles(dirtyModelsToRevert.map(dirtyModel => dirtyModel.resource), { soft: true });
// now we can rename the source to target via file operation // now we can rename the source to target via file operation
let stat: IFileStatWithMetadata; let stat: IFileStatWithMetadata;
@ -248,7 +249,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
} catch (error) { } catch (error) {
// in case of any error, ensure to set dirty flag back // in case of any error, ensure to set dirty flag back
dirtyModels.forEach(dirtyModel => dirtyModel.makeDirty()); dirtyModelsToRevert.forEach(dirtyModel => dirtyModel.makeDirty());
throw error; throw error;
} }

View file

@ -5,7 +5,7 @@
import * as assert from 'assert'; import * as assert from 'assert';
import { URI } from 'vs/base/common/uri'; import { URI } from 'vs/base/common/uri';
import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle'; import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle';
import { workbenchInstantiationService, TestLifecycleService, TestTextFileService, TestContextService, TestFileService, TestElectronService, TestFilesConfigurationService, TestFileDialogService } from 'vs/workbench/test/workbenchTestServices'; import { workbenchInstantiationService, TestLifecycleService, TestContextService, TestFileService, TestElectronService, TestFilesConfigurationService, TestFileDialogService, TestTextFileService } from 'vs/workbench/test/workbenchTestServices';
import { toResource } from 'vs/base/test/common/utils'; import { toResource } from 'vs/base/test/common/utils';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel'; import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel';
@ -126,6 +126,19 @@ suite('Files - TextFileService', () => {
assert.ok(!accessor.textFileService.isDirty(model.resource)); assert.ok(!accessor.textFileService.isDirty(model.resource));
}); });
test('create', async function () {
model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8', undefined);
(<TextFileEditorModelManager>accessor.textFileService.files).add(model.resource, model);
await model.load();
model!.textEditorModel!.setValue('foo');
assert.ok(accessor.textFileService.isDirty(model.resource));
await accessor.textFileService.create(model.resource, 'Foo');
assert.ok(!accessor.textFileService.isDirty(model.resource));
});
test('delete - dirty file', async function () { test('delete - dirty file', async function () {
model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8', undefined); model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8', undefined);
(<TextFileEditorModelManager>accessor.textFileService.files).add(model.resource, model); (<TextFileEditorModelManager>accessor.textFileService.files).add(model.resource, model);
@ -139,14 +152,22 @@ suite('Files - TextFileService', () => {
}); });
test('move - dirty file', async function () { test('move - dirty file', async function () {
await testMove(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt')); await testMoveOrCopy(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'), true);
}); });
test('move - dirty file (target exists and is dirty)', async function () { 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); await testMoveOrCopy(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'), true, true);
}); });
async function testMove(source: URI, target: URI, targetDirty?: boolean): Promise<void> { test('copy - dirty file', async function () {
await testMoveOrCopy(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'), false);
});
test('copy - dirty file (target exists and is dirty)', async function () {
await testMoveOrCopy(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'), false, true);
});
async function testMoveOrCopy(source: URI, target: URI, move: boolean, targetDirty?: boolean): Promise<void> {
let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, source, 'utf8', undefined); let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, source, 'utf8', undefined);
let targetModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, target, 'utf8', undefined); let targetModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, target, 'utf8', undefined);
(<TextFileEditorModelManager>accessor.textFileService.files).add(sourceModel.resource, sourceModel); (<TextFileEditorModelManager>accessor.textFileService.files).add(sourceModel.resource, sourceModel);
@ -162,11 +183,19 @@ suite('Files - TextFileService', () => {
assert.ok(accessor.textFileService.isDirty(targetModel.resource)); assert.ok(accessor.textFileService.isDirty(targetModel.resource));
} }
await accessor.textFileService.move(sourceModel.resource, targetModel.resource, true); if (move) {
await accessor.textFileService.move(sourceModel.resource, targetModel.resource, true);
} else {
await accessor.textFileService.copy(sourceModel.resource, targetModel.resource, true);
}
assert.equal(targetModel.textEditorModel!.getValue(), 'foo'); assert.equal(targetModel.textEditorModel!.getValue(), 'foo');
assert.ok(!accessor.textFileService.isDirty(sourceModel.resource)); if (move) {
assert.ok(!accessor.textFileService.isDirty(sourceModel.resource));
} else {
assert.ok(accessor.textFileService.isDirty(sourceModel.resource));
}
assert.ok(accessor.textFileService.isDirty(targetModel.resource)); assert.ok(accessor.textFileService.isDirty(targetModel.resource));
sourceModel.dispose(); sourceModel.dispose();

View file

@ -1111,11 +1111,11 @@ export class TestFileService implements IFileService {
} }
copy(_source: URI, _target: URI, _overwrite?: boolean): Promise<IFileStatWithMetadata> { copy(_source: URI, _target: URI, _overwrite?: boolean): Promise<IFileStatWithMetadata> {
throw new Error('not implemented'); return Promise.resolve(null!);
} }
createFile(_resource: URI, _content?: VSBuffer | VSBufferReadable, _options?: ICreateFileOptions): Promise<IFileStatWithMetadata> { createFile(_resource: URI, _content?: VSBuffer | VSBufferReadable, _options?: ICreateFileOptions): Promise<IFileStatWithMetadata> {
throw new Error('not implemented'); return Promise.resolve(null!);
} }
createFolder(_resource: URI): Promise<IFileStatWithMetadata> { createFolder(_resource: URI): Promise<IFileStatWithMetadata> {