Skip to content

Commit c126d94

Browse files
authored
Introduce Conflicts view (microsoft#161116)
* Introduice Conflicts view - supports showing conflicts across profiles * fix tests
1 parent 236e476 commit c126d94

File tree

13 files changed

+311
-190
lines changed

13 files changed

+311
-190
lines changed

src/vs/platform/userDataSync/common/keybindingsSync.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ export class KeybindingsSynchroniser extends AbstractJsonFileSynchroniser implem
135135
}
136136

137137
const previewResult: IMergeResult = {
138-
content: mergedContent,
138+
content: hasConflicts ? lastSyncContent : mergedContent,
139139
localChange: hasLocalChanged ? fileContent ? Change.Modified : Change.Added : Change.None,
140140
remoteChange: hasRemoteChanged ? Change.Modified : Change.None,
141141
hasConflicts
@@ -146,7 +146,7 @@ export class KeybindingsSynchroniser extends AbstractJsonFileSynchroniser implem
146146
fileContent,
147147

148148
baseResource: this.baseResource,
149-
baseContent: lastSyncContent !== null ? lastSyncContent : localContent,
149+
baseContent: lastSyncContent,
150150

151151
localResource: this.localResource,
152152
localContent,

src/vs/platform/userDataSync/common/settingsSync.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,19 +116,21 @@ export class SettingsSynchroniser extends AbstractJsonFileSynchroniser implement
116116
hasRemoteChanged = true;
117117
}
118118

119+
const localContent = fileContent ? fileContent.value.toString() : null;
120+
const baseContent = lastSettingsSyncContent?.settings ?? null;
121+
119122
const previewResult = {
120-
content: mergedContent,
123+
content: hasConflicts ? baseContent : mergedContent,
121124
localChange: hasLocalChanged ? Change.Modified : Change.None,
122125
remoteChange: hasRemoteChanged ? Change.Modified : Change.None,
123126
hasConflicts
124127
};
125128

126-
const localContent = fileContent ? fileContent.value.toString() : null;
127129
return [{
128130
fileContent,
129131

130132
baseResource: this.baseResource,
131-
baseContent: lastSettingsSyncContent ? lastSettingsSyncContent.settings : localContent,
133+
baseContent,
132134

133135
localResource: this.localResource,
134136
localContent,

src/vs/platform/userDataSync/common/snippetsSync.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
206206
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
207207
resourcePreviews.set(key, {
208208
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
209-
baseContent: baseSnippets[key] ?? localContent,
209+
baseContent: baseSnippets[key] ?? null,
210210
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
211211
fileContent: localFileContent[key],
212212
localContent,
@@ -231,7 +231,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
231231
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
232232
resourcePreviews.set(key, {
233233
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
234-
baseContent: baseSnippets[key] ?? localContent,
234+
baseContent: baseSnippets[key] ?? null,
235235
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
236236
fileContent: localFileContent[key],
237237
localContent,
@@ -256,7 +256,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
256256
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
257257
resourcePreviews.set(key, {
258258
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
259-
baseContent: baseSnippets[key] ?? localContent,
259+
baseContent: baseSnippets[key] ?? null,
260260
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
261261
fileContent: localFileContent[key],
262262
localContent,
@@ -281,7 +281,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
281281
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
282282
resourcePreviews.set(key, {
283283
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
284-
baseContent: baseSnippets[key] ?? localContent,
284+
baseContent: baseSnippets[key] ?? null,
285285
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
286286
fileContent: localFileContent[key],
287287
localContent,
@@ -322,15 +322,15 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
322322
/* Snippets with conflicts */
323323
for (const key of snippetsMergeResult.conflicts) {
324324
const previewResult: IMergeResult = {
325-
content: localFileContent[key] ? localFileContent[key].value.toString() : null,
325+
content: baseSnippets[key] ?? null,
326326
hasConflicts: true,
327327
localChange: localFileContent[key] ? Change.Modified : Change.Added,
328328
remoteChange: remoteSnippets[key] ? Change.Modified : Change.Added
329329
};
330330
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
331331
resourcePreviews.set(key, {
332332
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
333-
baseContent: baseSnippets[key] ?? localContent,
333+
baseContent: baseSnippets[key] ?? null,
334334
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
335335
fileContent: localFileContent[key] || null,
336336
localContent,
@@ -356,7 +356,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
356356
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
357357
resourcePreviews.set(key, {
358358
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
359-
baseContent: baseSnippets[key] ?? localContent,
359+
baseContent: baseSnippets[key] ?? null,
360360
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
361361
fileContent: localFileContent[key] || null,
362362
localContent,

src/vs/platform/userDataSync/common/tasksSync.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ export class TasksSynchroniser extends AbstractFileSynchroniser implements IUser
9999
}
100100

101101
const previewResult: IMergeResult = {
102-
content,
102+
content: hasConflicts ? lastSyncContent : content,
103103
localChange: hasLocalChanged ? fileContent ? Change.Modified : Change.Added : Change.None,
104104
remoteChange: hasRemoteChanged ? Change.Modified : Change.None,
105105
hasConflicts
@@ -110,7 +110,7 @@ export class TasksSynchroniser extends AbstractFileSynchroniser implements IUser
110110
fileContent,
111111

112112
baseResource: this.baseResource,
113-
baseContent: lastSyncContent !== null ? lastSyncContent : localContent,
113+
baseContent: lastSyncContent,
114114

115115
localResource: this.localResource,
116116
localContent,

src/vs/platform/userDataSync/common/userDataSync.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ export type IUserDataResourceManifest = Record<ServerResource, string>;
155155

156156
export interface IUserDataCollectionManifest {
157157
[collectionId: string]: {
158-
readonly latest: IUserDataResourceManifest;
158+
readonly latest?: IUserDataResourceManifest;
159159
};
160160
}
161161

src/vs/platform/userDataSync/common/userDataSyncService.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { toErrorMessage } from 'vs/base/common/errorMessage';
1010
import { Emitter, Event } from 'vs/base/common/event';
1111
import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
1212
import { isEqual } from 'vs/base/common/resources';
13-
import { isUndefined } from 'vs/base/common/types';
13+
import { isUndefined, isUndefinedOrNull } from 'vs/base/common/types';
1414
import { URI } from 'vs/base/common/uri';
1515
import { generateUuid } from 'vs/base/common/uuid';
1616
import { IHeaders } from 'vs/base/parts/request/common/request';
@@ -225,7 +225,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
225225
}
226226
return undefined;
227227
});
228-
if (!isUndefined(result)) {
228+
if (!isUndefinedOrNull(result)) {
229229
return result;
230230
}
231231
}
@@ -414,7 +414,8 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
414414
}
415415
}
416416

417-
private setConflicts(conflicts: IUserDataSyncResourceConflicts[]): void {
417+
private updateConflicts(): void {
418+
const conflicts = this.getActiveProfileSynchronizers().map(synchronizer => synchronizer.conflicts).flat();
418419
if (!equals(this._conflicts, conflicts, (a, b) => a.profile.id === b.profile.id && a.syncResource === b.syncResource && equals(a.conflicts, b.conflicts, (a, b) => isEqual(a.previewResource, b.previewResource)))) {
419420
this._conflicts = conflicts;
420421
this._onDidChangeConflicts.fire(conflicts);
@@ -435,7 +436,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
435436
const disposables = new DisposableStore();
436437
const profileSynchronizer = disposables.add(this.instantiationService.createInstance(ProfileSynchronizer, profile, syncProfile?.collection));
437438
disposables.add(profileSynchronizer.onDidChangeStatus(e => this.setStatus(e)));
438-
disposables.add(profileSynchronizer.onDidChangeConflicts(conflicts => this.setConflicts(conflicts)));
439+
disposables.add(profileSynchronizer.onDidChangeConflicts(conflicts => this.updateConflicts()));
439440
disposables.add(profileSynchronizer.onDidChangeLocal(e => this._onDidChangeLocal.fire(e)));
440441
this.activeProfileSynchronizers.set(profile.id, activeProfileSynchronizer = [profileSynchronizer, disposables]);
441442
}
@@ -939,7 +940,7 @@ class ProfileSynchronizer extends Disposable {
939940
}
940941

941942
try {
942-
const resourceManifest: IUserDataResourceManifest | null = (this.collection ? manifest?.collections?.[this.collection].latest : manifest?.latest) ?? null;
943+
const resourceManifest: IUserDataResourceManifest | null = (this.collection ? manifest?.collections?.[this.collection]?.latest : manifest?.latest) ?? null;
943944
await synchroniser.sync(resourceManifest, syncHeaders);
944945
} catch (e) {
945946
const userDataSyncError = UserDataSyncError.toUserDataSyncError(e);

src/vs/platform/userDataSync/test/common/settingsSync.test.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -498,10 +498,7 @@ suite('SettingsSync - Auto', () => {
498498

499499
const fileService = client.instantiationService.get(IFileService);
500500
const mergeContent = (await fileService.readFile(testObject.conflicts.conflicts[0].previewResource)).value.toString();
501-
assert.deepStrictEqual(JSON.parse(mergeContent), {
502-
'b': 1,
503-
'settingsSync.ignoredSettings': ['a']
504-
});
501+
assert.strictEqual(mergeContent, '');
505502
});
506503

507504
test('sync profile settings', async () => {

src/vs/platform/userDataSync/test/common/tasksSync.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,21 +280,21 @@ suite('TasksSync', () => {
280280
fileService.writeFile(tasksResource, VSBuffer.fromString(content));
281281
await testObject.sync(await client.getResourceManifest());
282282

283+
const previewContent = (await fileService.readFile(testObject.conflicts.conflicts[0].previewResource)).value.toString();
283284
assert.deepStrictEqual(testObject.status, SyncStatus.HasConflicts);
284285
assert.deepStrictEqual(testObject.conflicts.conflicts.length, 1);
285286
assert.deepStrictEqual(testObject.conflicts.conflicts[0].mergeState, MergeState.Conflict);
286287
assert.deepStrictEqual(testObject.conflicts.conflicts[0].localChange, Change.Modified);
287288
assert.deepStrictEqual(testObject.conflicts.conflicts[0].remoteChange, Change.Modified);
288-
assert.deepStrictEqual((await fileService.readFile(testObject.conflicts.conflicts[0].previewResource)).value.toString(), content);
289289

290290
await testObject.accept(testObject.conflicts.conflicts[0].previewResource);
291291
await testObject.apply(false);
292292
assert.deepStrictEqual(testObject.status, SyncStatus.Idle);
293293
const lastSyncUserData = await testObject.getLastSyncUserData();
294294
const remoteUserData = await testObject.getRemoteUserData(null);
295-
assert.strictEqual(getTasksContentFromSyncContent(lastSyncUserData!.syncData!.content!, client.instantiationService.get(ILogService)), content);
296-
assert.strictEqual(getTasksContentFromSyncContent(remoteUserData!.syncData!.content!, client.instantiationService.get(ILogService)), content);
297-
assert.strictEqual((await fileService.readFile(tasksResource)).value.toString(), content);
295+
assert.strictEqual(getTasksContentFromSyncContent(lastSyncUserData!.syncData!.content!, client.instantiationService.get(ILogService)), previewContent);
296+
assert.strictEqual(getTasksContentFromSyncContent(remoteUserData!.syncData!.content!, client.instantiationService.get(ILogService)), previewContent);
297+
assert.strictEqual((await fileService.readFile(tasksResource)).value.toString(), previewContent);
298298
});
299299

300300
test('when tasks file has moved forward locally and remotely - accept modified preview', async () => {

src/vs/platform/userDataSync/test/common/userDataSyncClient.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,12 +249,12 @@ export class UserDataSyncTestServer implements IRequestService {
249249
if (this.collectionCounter) {
250250
collection = {};
251251
for (let collectionId = 1; collectionId <= this.collectionCounter; collectionId++) {
252-
const latest: Record<ServerResource, string> = Object.create({});
253252
const collectionData = this.collections.get(`${collectionId}`);
254253
if (collectionData) {
254+
const latest: Record<ServerResource, string> = Object.create({});
255255
collectionData.forEach((value, key) => latest[key] = value.ref);
256+
collection[`${collectionId}`] = { latest };
256257
}
257-
collection[`${collectionId}`] = { latest };
258258
}
259259
}
260260
const manifest = { session: this.session, latest, collection };

0 commit comments

Comments
 (0)