Skip to content

Commit 10cbb85

Browse files
author
Benjamin Pasero
committed
Explorer: copy() of dirty file reverts the source if dirty (fix microsoft#89217)
1 parent 4ba2070 commit 10cbb85

3 files changed

Lines changed: 45 additions & 15 deletions

File tree

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -192,12 +192,12 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
192192

193193
// find all models that related to either source or target (can be many if resource is a folder)
194194
const sourceModels: ITextFileEditorModel[] = [];
195-
const conflictingModels: ITextFileEditorModel[] = [];
195+
const targetModels: ITextFileEditorModel[] = [];
196196
for (const model of this.getFileModels()) {
197197
const resource = model.resource;
198198

199199
if (isEqualOrParent(resource, target, false /* do not ignorecase, see https://github.com/Microsoft/vscode/issues/56384 */)) {
200-
conflictingModels.push(model);
200+
targetModels.push(model);
201201
}
202202

203203
if (isEqualOrParent(resource, source)) {
@@ -232,10 +232,11 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
232232
modelsToRestore.push(modelToRestore);
233233
}
234234

235-
// in order to move and copy, we need to soft revert all dirty models,
236-
// both from the source as well as the target if any
237-
const dirtyModels = [...sourceModels, ...conflictingModels].filter(model => model.isDirty());
238-
await this.doRevertFiles(dirtyModels.map(dirtyModel => dirtyModel.resource), { soft: true });
235+
// handle dirty models depending on the operation:
236+
// - move: revert both source and target (if any)
237+
// - copy: revert target (if any)
238+
const dirtyModelsToRevert = (move ? [...sourceModels, ...targetModels] : [...targetModels]).filter(model => model.isDirty());
239+
await this.doRevertFiles(dirtyModelsToRevert.map(dirtyModel => dirtyModel.resource), { soft: true });
239240

240241
// now we can rename the source to target via file operation
241242
let stat: IFileStatWithMetadata;
@@ -248,7 +249,7 @@ export abstract class AbstractTextFileService extends Disposable implements ITex
248249
} catch (error) {
249250

250251
// in case of any error, ensure to set dirty flag back
251-
dirtyModels.forEach(dirtyModel => dirtyModel.makeDirty());
252+
dirtyModelsToRevert.forEach(dirtyModel => dirtyModel.makeDirty());
252253

253254
throw error;
254255
}

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

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import * as assert from 'assert';
66
import { URI } from 'vs/base/common/uri';
77
import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle';
8-
import { workbenchInstantiationService, TestLifecycleService, TestTextFileService, TestContextService, TestFileService, TestElectronService, TestFilesConfigurationService, TestFileDialogService } from 'vs/workbench/test/workbenchTestServices';
8+
import { workbenchInstantiationService, TestLifecycleService, TestContextService, TestFileService, TestElectronService, TestFilesConfigurationService, TestFileDialogService, TestTextFileService } from 'vs/workbench/test/workbenchTestServices';
99
import { toResource } from 'vs/base/test/common/utils';
1010
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
1111
import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel';
@@ -126,6 +126,19 @@ suite('Files - TextFileService', () => {
126126
assert.ok(!accessor.textFileService.isDirty(model.resource));
127127
});
128128

129+
test('create', async function () {
130+
model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8', undefined);
131+
(<TextFileEditorModelManager>accessor.textFileService.files).add(model.resource, model);
132+
133+
await model.load();
134+
model!.textEditorModel!.setValue('foo');
135+
assert.ok(accessor.textFileService.isDirty(model.resource));
136+
137+
138+
await accessor.textFileService.create(model.resource, 'Foo');
139+
assert.ok(!accessor.textFileService.isDirty(model.resource));
140+
});
141+
129142
test('delete - dirty file', async function () {
130143
model = instantiationService.createInstance(TextFileEditorModel, toResource.call(this, '/path/file.txt'), 'utf8', undefined);
131144
(<TextFileEditorModelManager>accessor.textFileService.files).add(model.resource, model);
@@ -139,14 +152,22 @@ suite('Files - TextFileService', () => {
139152
});
140153

141154
test('move - dirty file', async function () {
142-
await testMove(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'));
155+
await testMoveOrCopy(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'), true);
143156
});
144157

145158
test('move - dirty file (target exists and is dirty)', async function () {
146-
await testMove(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'), true);
159+
await testMoveOrCopy(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'), true, true);
160+
});
161+
162+
test('copy - dirty file', async function () {
163+
await testMoveOrCopy(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'), false);
147164
});
148165

149-
async function testMove(source: URI, target: URI, targetDirty?: boolean): Promise<void> {
166+
test('copy - dirty file (target exists and is dirty)', async function () {
167+
await testMoveOrCopy(toResource.call(this, '/path/file.txt'), toResource.call(this, '/path/file_target.txt'), false, true);
168+
});
169+
170+
async function testMoveOrCopy(source: URI, target: URI, move: boolean, targetDirty?: boolean): Promise<void> {
150171
let sourceModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, source, 'utf8', undefined);
151172
let targetModel: TextFileEditorModel = instantiationService.createInstance(TextFileEditorModel, target, 'utf8', undefined);
152173
(<TextFileEditorModelManager>accessor.textFileService.files).add(sourceModel.resource, sourceModel);
@@ -162,11 +183,19 @@ suite('Files - TextFileService', () => {
162183
assert.ok(accessor.textFileService.isDirty(targetModel.resource));
163184
}
164185

165-
await accessor.textFileService.move(sourceModel.resource, targetModel.resource, true);
186+
if (move) {
187+
await accessor.textFileService.move(sourceModel.resource, targetModel.resource, true);
188+
} else {
189+
await accessor.textFileService.copy(sourceModel.resource, targetModel.resource, true);
190+
}
166191

167192
assert.equal(targetModel.textEditorModel!.getValue(), 'foo');
168193

169-
assert.ok(!accessor.textFileService.isDirty(sourceModel.resource));
194+
if (move) {
195+
assert.ok(!accessor.textFileService.isDirty(sourceModel.resource));
196+
} else {
197+
assert.ok(accessor.textFileService.isDirty(sourceModel.resource));
198+
}
170199
assert.ok(accessor.textFileService.isDirty(targetModel.resource));
171200

172201
sourceModel.dispose();

‎src/vs/workbench/test/workbenchTestServices.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1111,11 +1111,11 @@ export class TestFileService implements IFileService {
11111111
}
11121112

11131113
copy(_source: URI, _target: URI, _overwrite?: boolean): Promise<IFileStatWithMetadata> {
1114-
throw new Error('not implemented');
1114+
return Promise.resolve(null!);
11151115
}
11161116

11171117
createFile(_resource: URI, _content?: VSBuffer | VSBufferReadable, _options?: ICreateFileOptions): Promise<IFileStatWithMetadata> {
1118-
throw new Error('not implemented');
1118+
return Promise.resolve(null!);
11191119
}
11201120

11211121
createFolder(_resource: URI): Promise<IFileStatWithMetadata> {

0 commit comments

Comments
 (0)