Skip to content

Commit

Permalink
Introduce Conflicts view (microsoft#161116)
Browse files Browse the repository at this point in the history
* Introduice Conflicts view
- supports showing conflicts across profiles

* fix tests
  • Loading branch information
sandy081 committed Sep 16, 2022
1 parent 236e476 commit c126d94
Show file tree
Hide file tree
Showing 13 changed files with 311 additions and 190 deletions.
4 changes: 2 additions & 2 deletions src/vs/platform/userDataSync/common/keybindingsSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export class KeybindingsSynchroniser extends AbstractJsonFileSynchroniser implem
}

const previewResult: IMergeResult = {
content: mergedContent,
content: hasConflicts ? lastSyncContent : mergedContent,
localChange: hasLocalChanged ? fileContent ? Change.Modified : Change.Added : Change.None,
remoteChange: hasRemoteChanged ? Change.Modified : Change.None,
hasConflicts
Expand All @@ -146,7 +146,7 @@ export class KeybindingsSynchroniser extends AbstractJsonFileSynchroniser implem
fileContent,

baseResource: this.baseResource,
baseContent: lastSyncContent !== null ? lastSyncContent : localContent,
baseContent: lastSyncContent,

localResource: this.localResource,
localContent,
Expand Down
8 changes: 5 additions & 3 deletions src/vs/platform/userDataSync/common/settingsSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,19 +116,21 @@ export class SettingsSynchroniser extends AbstractJsonFileSynchroniser implement
hasRemoteChanged = true;
}

const localContent = fileContent ? fileContent.value.toString() : null;
const baseContent = lastSettingsSyncContent?.settings ?? null;

const previewResult = {
content: mergedContent,
content: hasConflicts ? baseContent : mergedContent,
localChange: hasLocalChanged ? Change.Modified : Change.None,
remoteChange: hasRemoteChanged ? Change.Modified : Change.None,
hasConflicts
};

const localContent = fileContent ? fileContent.value.toString() : null;
return [{
fileContent,

baseResource: this.baseResource,
baseContent: lastSettingsSyncContent ? lastSettingsSyncContent.settings : localContent,
baseContent,

localResource: this.localResource,
localContent,
Expand Down
14 changes: 7 additions & 7 deletions src/vs/platform/userDataSync/common/snippetsSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
resourcePreviews.set(key, {
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
baseContent: baseSnippets[key] ?? localContent,
baseContent: baseSnippets[key] ?? null,
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
fileContent: localFileContent[key],
localContent,
Expand All @@ -231,7 +231,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
resourcePreviews.set(key, {
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
baseContent: baseSnippets[key] ?? localContent,
baseContent: baseSnippets[key] ?? null,
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
fileContent: localFileContent[key],
localContent,
Expand All @@ -256,7 +256,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
resourcePreviews.set(key, {
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
baseContent: baseSnippets[key] ?? localContent,
baseContent: baseSnippets[key] ?? null,
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
fileContent: localFileContent[key],
localContent,
Expand All @@ -281,7 +281,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
resourcePreviews.set(key, {
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
baseContent: baseSnippets[key] ?? localContent,
baseContent: baseSnippets[key] ?? null,
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
fileContent: localFileContent[key],
localContent,
Expand Down Expand Up @@ -322,15 +322,15 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
/* Snippets with conflicts */
for (const key of snippetsMergeResult.conflicts) {
const previewResult: IMergeResult = {
content: localFileContent[key] ? localFileContent[key].value.toString() : null,
content: baseSnippets[key] ?? null,
hasConflicts: true,
localChange: localFileContent[key] ? Change.Modified : Change.Added,
remoteChange: remoteSnippets[key] ? Change.Modified : Change.Added
};
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
resourcePreviews.set(key, {
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
baseContent: baseSnippets[key] ?? localContent,
baseContent: baseSnippets[key] ?? null,
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
fileContent: localFileContent[key] || null,
localContent,
Expand All @@ -356,7 +356,7 @@ export class SnippetsSynchroniser extends AbstractSynchroniser implements IUserD
const localContent = localFileContent[key] ? localFileContent[key].value.toString() : null;
resourcePreviews.set(key, {
baseResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'base' }),
baseContent: baseSnippets[key] ?? localContent,
baseContent: baseSnippets[key] ?? null,
localResource: this.extUri.joinPath(this.syncPreviewFolder, key).with({ scheme: USER_DATA_SYNC_SCHEME, authority: 'local' }),
fileContent: localFileContent[key] || null,
localContent,
Expand Down
4 changes: 2 additions & 2 deletions src/vs/platform/userDataSync/common/tasksSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export class TasksSynchroniser extends AbstractFileSynchroniser implements IUser
}

const previewResult: IMergeResult = {
content,
content: hasConflicts ? lastSyncContent : content,
localChange: hasLocalChanged ? fileContent ? Change.Modified : Change.Added : Change.None,
remoteChange: hasRemoteChanged ? Change.Modified : Change.None,
hasConflicts
Expand All @@ -110,7 +110,7 @@ export class TasksSynchroniser extends AbstractFileSynchroniser implements IUser
fileContent,

baseResource: this.baseResource,
baseContent: lastSyncContent !== null ? lastSyncContent : localContent,
baseContent: lastSyncContent,

localResource: this.localResource,
localContent,
Expand Down
2 changes: 1 addition & 1 deletion src/vs/platform/userDataSync/common/userDataSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export type IUserDataResourceManifest = Record<ServerResource, string>;

export interface IUserDataCollectionManifest {
[collectionId: string]: {
readonly latest: IUserDataResourceManifest;
readonly latest?: IUserDataResourceManifest;
};
}

Expand Down
11 changes: 6 additions & 5 deletions src/vs/platform/userDataSync/common/userDataSyncService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { toErrorMessage } from 'vs/base/common/errorMessage';
import { Emitter, Event } from 'vs/base/common/event';
import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { isEqual } from 'vs/base/common/resources';
import { isUndefined } from 'vs/base/common/types';
import { isUndefined, isUndefinedOrNull } from 'vs/base/common/types';
import { URI } from 'vs/base/common/uri';
import { generateUuid } from 'vs/base/common/uuid';
import { IHeaders } from 'vs/base/parts/request/common/request';
Expand Down Expand Up @@ -225,7 +225,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
}
return undefined;
});
if (!isUndefined(result)) {
if (!isUndefinedOrNull(result)) {
return result;
}
}
Expand Down Expand Up @@ -414,7 +414,8 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
}
}

private setConflicts(conflicts: IUserDataSyncResourceConflicts[]): void {
private updateConflicts(): void {
const conflicts = this.getActiveProfileSynchronizers().map(synchronizer => synchronizer.conflicts).flat();
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)))) {
this._conflicts = conflicts;
this._onDidChangeConflicts.fire(conflicts);
Expand All @@ -435,7 +436,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
const disposables = new DisposableStore();
const profileSynchronizer = disposables.add(this.instantiationService.createInstance(ProfileSynchronizer, profile, syncProfile?.collection));
disposables.add(profileSynchronizer.onDidChangeStatus(e => this.setStatus(e)));
disposables.add(profileSynchronizer.onDidChangeConflicts(conflicts => this.setConflicts(conflicts)));
disposables.add(profileSynchronizer.onDidChangeConflicts(conflicts => this.updateConflicts()));
disposables.add(profileSynchronizer.onDidChangeLocal(e => this._onDidChangeLocal.fire(e)));
this.activeProfileSynchronizers.set(profile.id, activeProfileSynchronizer = [profileSynchronizer, disposables]);
}
Expand Down Expand Up @@ -939,7 +940,7 @@ class ProfileSynchronizer extends Disposable {
}

try {
const resourceManifest: IUserDataResourceManifest | null = (this.collection ? manifest?.collections?.[this.collection].latest : manifest?.latest) ?? null;
const resourceManifest: IUserDataResourceManifest | null = (this.collection ? manifest?.collections?.[this.collection]?.latest : manifest?.latest) ?? null;
await synchroniser.sync(resourceManifest, syncHeaders);
} catch (e) {
const userDataSyncError = UserDataSyncError.toUserDataSyncError(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,10 +498,7 @@ suite('SettingsSync - Auto', () => {

const fileService = client.instantiationService.get(IFileService);
const mergeContent = (await fileService.readFile(testObject.conflicts.conflicts[0].previewResource)).value.toString();
assert.deepStrictEqual(JSON.parse(mergeContent), {
'b': 1,
'settingsSync.ignoredSettings': ['a']
});
assert.strictEqual(mergeContent, '');
});

test('sync profile settings', async () => {
Expand Down
8 changes: 4 additions & 4 deletions src/vs/platform/userDataSync/test/common/tasksSync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,21 +280,21 @@ suite('TasksSync', () => {
fileService.writeFile(tasksResource, VSBuffer.fromString(content));
await testObject.sync(await client.getResourceManifest());

const previewContent = (await fileService.readFile(testObject.conflicts.conflicts[0].previewResource)).value.toString();
assert.deepStrictEqual(testObject.status, SyncStatus.HasConflicts);
assert.deepStrictEqual(testObject.conflicts.conflicts.length, 1);
assert.deepStrictEqual(testObject.conflicts.conflicts[0].mergeState, MergeState.Conflict);
assert.deepStrictEqual(testObject.conflicts.conflicts[0].localChange, Change.Modified);
assert.deepStrictEqual(testObject.conflicts.conflicts[0].remoteChange, Change.Modified);
assert.deepStrictEqual((await fileService.readFile(testObject.conflicts.conflicts[0].previewResource)).value.toString(), content);

await testObject.accept(testObject.conflicts.conflicts[0].previewResource);
await testObject.apply(false);
assert.deepStrictEqual(testObject.status, SyncStatus.Idle);
const lastSyncUserData = await testObject.getLastSyncUserData();
const remoteUserData = await testObject.getRemoteUserData(null);
assert.strictEqual(getTasksContentFromSyncContent(lastSyncUserData!.syncData!.content!, client.instantiationService.get(ILogService)), content);
assert.strictEqual(getTasksContentFromSyncContent(remoteUserData!.syncData!.content!, client.instantiationService.get(ILogService)), content);
assert.strictEqual((await fileService.readFile(tasksResource)).value.toString(), content);
assert.strictEqual(getTasksContentFromSyncContent(lastSyncUserData!.syncData!.content!, client.instantiationService.get(ILogService)), previewContent);
assert.strictEqual(getTasksContentFromSyncContent(remoteUserData!.syncData!.content!, client.instantiationService.get(ILogService)), previewContent);
assert.strictEqual((await fileService.readFile(tasksResource)).value.toString(), previewContent);
});

test('when tasks file has moved forward locally and remotely - accept modified preview', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,12 +249,12 @@ export class UserDataSyncTestServer implements IRequestService {
if (this.collectionCounter) {
collection = {};
for (let collectionId = 1; collectionId <= this.collectionCounter; collectionId++) {
const latest: Record<ServerResource, string> = Object.create({});
const collectionData = this.collections.get(`${collectionId}`);
if (collectionData) {
const latest: Record<ServerResource, string> = Object.create({});
collectionData.forEach((value, key) => latest[key] = value.ref);
collection[`${collectionId}`] = { latest };
}
collection[`${collectionId}`] = { latest };
}
}
const manifest = { session: this.session, latest, collection };
Expand Down
Loading

0 comments on commit c126d94

Please sign in to comment.