diff --git a/src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts b/src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts index 96ce984b782..b750a9b003f 100644 --- a/src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts +++ b/src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts @@ -50,11 +50,10 @@ import { IFileService } from 'vs/platform/files/common/files'; import { DiskFileSystemProvider } from 'vs/platform/files/electron-browser/diskFileSystemProvider'; import { Schemas } from 'vs/base/common/network'; import { IProductService } from 'vs/platform/product/common/productService'; -import { IUserDataSyncService, IUserDataSyncStoreService, ISettingsMergeService, registerConfiguration, IUserDataSyncLogService, IUserDataSyncUtilService } from 'vs/platform/userDataSync/common/userDataSync'; +import { IUserDataSyncService, IUserDataSyncStoreService, registerConfiguration, IUserDataSyncLogService, IUserDataSyncUtilService } from 'vs/platform/userDataSync/common/userDataSync'; import { UserDataSyncService } from 'vs/platform/userDataSync/common/userDataSyncService'; import { UserDataSyncStoreService } from 'vs/platform/userDataSync/common/userDataSyncStoreService'; import { UserDataSyncChannel } from 'vs/platform/userDataSync/common/userDataSyncIpc'; -import { SettingsMergeChannelClient } from 'vs/platform/userDataSync/common/settingsSyncIpc'; import { IElectronService } from 'vs/platform/electron/node/electron'; import { LoggerService } from 'vs/platform/log/node/loggerService'; import { UserDataSyncLogService } from 'vs/platform/userDataSync/common/userDataSyncLog'; @@ -186,8 +185,6 @@ async function main(server: Server, initData: ISharedProcessInitData, configurat services.set(ICredentialsService, new SyncDescriptor(KeytarCredentialsService)); services.set(IAuthTokenService, new SyncDescriptor(AuthTokenService)); services.set(IUserDataSyncLogService, new SyncDescriptor(UserDataSyncLogService)); - const settingsMergeChannel = server.getChannel('settingsMerge', activeWindowRouter); - services.set(ISettingsMergeService, new SettingsMergeChannelClient(settingsMergeChannel)); services.set(IUserDataSyncUtilService, new UserDataSyncUtilServiceClient(server.getChannel('userDataSyncUtil', activeWindowRouter))); services.set(IUserDataSyncStoreService, new SyncDescriptor(UserDataSyncStoreService)); services.set(IUserDataSyncService, new SyncDescriptor(UserDataSyncService)); diff --git a/src/vs/platform/userDataSync/common/keybindingsSync.ts b/src/vs/platform/userDataSync/common/keybindingsSync.ts index 269065cd9a0..4735db86782 100644 --- a/src/vs/platform/userDataSync/common/keybindingsSync.ts +++ b/src/vs/platform/userDataSync/common/keybindingsSync.ts @@ -19,6 +19,7 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur import { CancellationToken } from 'vs/base/common/cancellation'; import { OS, OperatingSystem } from 'vs/base/common/platform'; import { isUndefined } from 'vs/base/common/types'; +import { FormattingOptions } from 'vs/base/common/jsonFormatter'; interface ISyncContent { mac?: string; @@ -217,7 +218,7 @@ export class KeybindingsSynchroniser extends Disposable implements ISynchroniser || lastSyncContent !== remoteContent // Remote has forwarded ) { this.logService.trace('Keybindings: Merging remote keybindings with local keybindings...'); - const formattingOptions = await this.userDataSyncUtilService.resolveFormattingOptions(this.environmentService.keybindingsResource); + const formattingOptions = await this.getFormattingOptions(); const result = await merge(localContent, remoteContent, lastSyncContent, formattingOptions, this.userDataSyncUtilService); // Sync only if there are changes if (result.hasChanges) { @@ -243,6 +244,14 @@ export class KeybindingsSynchroniser extends Disposable implements ISynchroniser return { fileContent, remoteUserData, hasLocalChanged, hasRemoteChanged, hasConflicts }; } + private _formattingOptions: Promise | undefined = undefined; + private getFormattingOptions(): Promise { + if (!this._formattingOptions) { + this._formattingOptions = this.userDataSyncUtilService.resolveFormattingOptions(this.environmentService.keybindingsResource); + } + return this._formattingOptions; + } + private async getLocalContent(): Promise { try { return await this.fileService.readFile(this.environmentService.keybindingsResource); diff --git a/src/vs/platform/userDataSync/common/settingsMerge.ts b/src/vs/platform/userDataSync/common/settingsMerge.ts new file mode 100644 index 00000000000..d7224e9f3a0 --- /dev/null +++ b/src/vs/platform/userDataSync/common/settingsMerge.ts @@ -0,0 +1,191 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as objects from 'vs/base/common/objects'; +import { parse, findNodeAtLocation, parseTree, Node } from 'vs/base/common/json'; +import { setProperty } from 'vs/base/common/jsonEdit'; +import { values } from 'vs/base/common/map'; +import { IStringDictionary } from 'vs/base/common/collections'; +import { FormattingOptions } from 'vs/base/common/jsonFormatter'; +import * as contentUtil from 'vs/platform/userDataSync/common/content'; + +export function computeRemoteContent(localContent: string, remoteContent: string, ignoredSettings: string[], formattingOptions: FormattingOptions): string { + if (ignoredSettings.length) { + const remote = parse(remoteContent); + const ignored = ignoredSettings.reduce((set, key) => { set.add(key); return set; }, new Set()); + for (const key of ignoredSettings) { + if (ignored.has(key)) { + localContent = contentUtil.edit(localContent, [key], remote[key], formattingOptions); + } + } + } + return localContent; +} + +export function merge(localContent: string, remoteContent: string, baseContent: string | null, ignoredSettings: string[], formattingOptions: FormattingOptions): { mergeContent: string, hasChanges: boolean, hasConflicts: boolean } { + const local = parse(localContent); + const remote = parse(remoteContent); + const base = baseContent ? parse(baseContent) : null; + const ignored = ignoredSettings.reduce((set, key) => { set.add(key); return set; }, new Set()); + + const localToRemote = compare(local, remote, ignored); + if (localToRemote.added.size === 0 && localToRemote.removed.size === 0 && localToRemote.updated.size === 0) { + // No changes found between local and remote. + return { mergeContent: localContent, hasChanges: false, hasConflicts: false }; + } + + const conflicts: Set = new Set(); + const baseToLocal = base ? compare(base, local, ignored) : { added: Object.keys(local).reduce((r, k) => { r.add(k); return r; }, new Set()), removed: new Set(), updated: new Set() }; + const baseToRemote = base ? compare(base, remote, ignored) : { added: Object.keys(remote).reduce((r, k) => { r.add(k); return r; }, new Set()), removed: new Set(), updated: new Set() }; + let mergeContent = localContent; + + // Removed settings in Local + for (const key of values(baseToLocal.removed)) { + // Got updated in remote + if (baseToRemote.updated.has(key)) { + conflicts.add(key); + } + } + + // Removed settings in Remote + for (const key of values(baseToRemote.removed)) { + if (conflicts.has(key)) { + continue; + } + // Got updated in local + if (baseToLocal.updated.has(key)) { + conflicts.add(key); + } else { + mergeContent = contentUtil.edit(mergeContent, [key], undefined, formattingOptions); + } + } + + // Added settings in Local + for (const key of values(baseToLocal.added)) { + if (conflicts.has(key)) { + continue; + } + // Got added in remote + if (baseToRemote.added.has(key)) { + // Has different value + if (localToRemote.updated.has(key)) { + conflicts.add(key); + } + } + } + + // Added settings in remote + for (const key of values(baseToRemote.added)) { + if (conflicts.has(key)) { + continue; + } + // Got added in local + if (baseToLocal.added.has(key)) { + // Has different value + if (localToRemote.updated.has(key)) { + conflicts.add(key); + } + } else { + mergeContent = contentUtil.edit(mergeContent, [key], remote[key], formattingOptions); + } + } + + // Updated settings in Local + for (const key of values(baseToLocal.updated)) { + if (conflicts.has(key)) { + continue; + } + // Got updated in remote + if (baseToRemote.updated.has(key)) { + // Has different value + if (localToRemote.updated.has(key)) { + conflicts.add(key); + } + } + } + + // Updated settings in Remote + for (const key of values(baseToRemote.updated)) { + if (conflicts.has(key)) { + continue; + } + // Got updated in local + if (baseToLocal.updated.has(key)) { + // Has different value + if (localToRemote.updated.has(key)) { + conflicts.add(key); + } + } else { + mergeContent = contentUtil.edit(mergeContent, [key], remote[key], formattingOptions); + } + } + + if (conflicts.size > 0) { + const conflictNodes: { key: string, node: Node | undefined }[] = []; + const tree = parseTree(mergeContent); + const eol = formattingOptions.eol!; + for (const key of values(conflicts)) { + const node = findNodeAtLocation(tree, [key]); + conflictNodes.push({ key, node }); + } + conflictNodes.sort((a, b) => { + if (a.node && b.node) { + return b.node.offset - a.node.offset; + } + return a.node ? 1 : -1; + }); + const lastNode = tree.children ? tree.children[tree.children.length - 1] : undefined; + for (const { key, node } of conflictNodes) { + const remoteEdit = setProperty(`{${eol}\t${eol}}`, [key], remote[key], { tabSize: 4, insertSpaces: false, eol: eol })[0]; + const remoteContent = remoteEdit ? `${remoteEdit.content.substring(remoteEdit.offset + remoteEdit.length + 1)},${eol}` : ''; + if (node) { + // Updated in Local and Remote with different value + const localStartOffset = contentUtil.getLineStartOffset(mergeContent, eol, node.parent!.offset); + const localEndOffset = contentUtil.getLineEndOffset(mergeContent, eol, node.offset + node.length); + mergeContent = mergeContent.substring(0, localStartOffset) + + `<<<<<<< local${eol}` + + mergeContent.substring(localStartOffset, localEndOffset) + + `${eol}=======${eol}${remoteContent}>>>>>>> remote` + + mergeContent.substring(localEndOffset); + } else { + // Removed in Local, but updated in Remote + if (lastNode) { + const localStartOffset = contentUtil.getLineEndOffset(mergeContent, eol, lastNode.offset + lastNode.length); + mergeContent = mergeContent.substring(0, localStartOffset) + + `${eol}<<<<<<< local${eol}=======${eol}${remoteContent}>>>>>>> remote` + + mergeContent.substring(localStartOffset); + } else { + const localStartOffset = tree.offset + 1; + mergeContent = mergeContent.substring(0, localStartOffset) + + `${eol}<<<<<<< local${eol}=======${eol}${remoteContent}>>>>>>> remote${eol}` + + mergeContent.substring(localStartOffset); + } + } + } + } + + return { mergeContent, hasChanges: true, hasConflicts: conflicts.size > 0 }; +} + +function compare(from: IStringDictionary, to: IStringDictionary, ignored: Set): { added: Set, removed: Set, updated: Set } { + const fromKeys = Object.keys(from).filter(key => !ignored.has(key)); + const toKeys = Object.keys(to).filter(key => !ignored.has(key)); + const added = toKeys.filter(key => fromKeys.indexOf(key) === -1).reduce((r, key) => { r.add(key); return r; }, new Set()); + const removed = fromKeys.filter(key => toKeys.indexOf(key) === -1).reduce((r, key) => { r.add(key); return r; }, new Set()); + const updated: Set = new Set(); + + for (const key of fromKeys) { + if (removed.has(key)) { + continue; + } + const value1 = from[key]; + const value2 = to[key]; + if (!objects.equals(value1, value2)) { + updated.add(key); + } + } + + return { added, removed, updated }; +} diff --git a/src/vs/platform/userDataSync/common/settingsSync.ts b/src/vs/platform/userDataSync/common/settingsSync.ts index 4532f40b9dd..d65b456a539 100644 --- a/src/vs/platform/userDataSync/common/settingsSync.ts +++ b/src/vs/platform/userDataSync/common/settingsSync.ts @@ -5,7 +5,7 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { IFileService, FileSystemProviderErrorCode, FileSystemProviderError, IFileContent } from 'vs/platform/files/common/files'; -import { IUserData, UserDataSyncStoreError, UserDataSyncStoreErrorCode, ISynchroniser, SyncStatus, ISettingsMergeService, IUserDataSyncStoreService, DEFAULT_IGNORED_SETTINGS, IUserDataSyncLogService } from 'vs/platform/userDataSync/common/userDataSync'; +import { IUserData, UserDataSyncStoreError, UserDataSyncStoreErrorCode, ISynchroniser, SyncStatus, IUserDataSyncStoreService, DEFAULT_IGNORED_SETTINGS, IUserDataSyncLogService, IUserDataSyncUtilService } from 'vs/platform/userDataSync/common/userDataSync'; import { VSBuffer } from 'vs/base/common/buffer'; import { parse, ParseError } from 'vs/base/common/json'; import { localize } from 'vs/nls'; @@ -17,6 +17,8 @@ import { joinPath, dirname } from 'vs/base/common/resources'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { startsWith } from 'vs/base/common/strings'; import { CancellationToken } from 'vs/base/common/cancellation'; +import { computeRemoteContent, merge } from 'vs/platform/userDataSync/common/settingsMerge'; +import { FormattingOptions } from 'vs/base/common/jsonFormatter'; interface ISyncPreviewResult { readonly fileContent: IFileContent | null; @@ -46,8 +48,8 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser { @IFileService private readonly fileService: IFileService, @IEnvironmentService private readonly environmentService: IEnvironmentService, @IUserDataSyncStoreService private readonly userDataSyncStoreService: IUserDataSyncStoreService, - @ISettingsMergeService private readonly settingsMergeService: ISettingsMergeService, @IUserDataSyncLogService private readonly logService: IUserDataSyncLogService, + @IUserDataSyncUtilService private readonly userDataSyncUtilService: IUserDataSyncUtilService, @IConfigurationService private readonly configurationService: IConfigurationService, ) { super(); @@ -148,7 +150,8 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser { await this.writeToLocal(content, fileContent); } if (hasRemoteChanged) { - const remoteContent = remoteUserData.content ? await this.settingsMergeService.computeRemoteContent(content, remoteUserData.content, this.getIgnoredSettings(content)) : content; + const formatUtils = await this.getFormattingOptions(); + const remoteContent = remoteUserData.content ? computeRemoteContent(content, remoteUserData.content, this.getIgnoredSettings(content), formatUtils) : content; this.logService.info('Settings: Updating remote settings'); const ref = await this.writeToRemote(remoteContent, remoteUserData.ref); remoteUserData = { ref, content }; @@ -205,7 +208,8 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser { || lastSyncData.content !== remoteContent // Remote has forwarded ) { this.logService.trace('Settings: Merging remote settings with local settings...'); - const result = await this.settingsMergeService.merge(localContent, remoteContent, lastSyncData ? lastSyncData.content : null, this.getIgnoredSettings()); + const formatUtils = await this.getFormattingOptions(); + const result = merge(localContent, remoteContent, lastSyncData ? lastSyncData.content : null, this.getIgnoredSettings(), formatUtils); // Sync only if there are changes if (result.hasChanges) { hasLocalChanged = result.mergeContent !== localContent; @@ -230,6 +234,14 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser { return { fileContent, remoteUserData, hasLocalChanged, hasRemoteChanged, hasConflicts }; } + private _formattingOptions: Promise | undefined = undefined; + private getFormattingOptions(): Promise { + if (!this._formattingOptions) { + this._formattingOptions = this.userDataSyncUtilService.resolveFormattingOptions(this.environmentService.settingsResource); + } + return this._formattingOptions; + } + private getIgnoredSettings(settingsContent?: string): string[] { let value: string[] = []; if (settingsContent) { diff --git a/src/vs/platform/userDataSync/common/settingsSyncIpc.ts b/src/vs/platform/userDataSync/common/settingsSyncIpc.ts deleted file mode 100644 index 3f231e86454..00000000000 --- a/src/vs/platform/userDataSync/common/settingsSyncIpc.ts +++ /dev/null @@ -1,42 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { IChannel, IServerChannel } from 'vs/base/parts/ipc/common/ipc'; -import { Event } from 'vs/base/common/event'; -import { ISettingsMergeService } from 'vs/platform/userDataSync/common/userDataSync'; - -export class SettingsMergeChannel implements IServerChannel { - - constructor(private readonly service: ISettingsMergeService) { } - - listen(_: unknown, event: string): Event { - throw new Error(`Event not found: ${event}`); - } - - call(context: any, command: string, args?: any): Promise { - switch (command) { - case 'merge': return this.service.merge(args[0], args[1], args[2], args[3]); - case 'computeRemoteContent': return this.service.computeRemoteContent(args[0], args[1], args[2]); - } - throw new Error('Invalid call'); - } -} - -export class SettingsMergeChannelClient implements ISettingsMergeService { - - _serviceBrand: undefined; - - constructor(private readonly channel: IChannel) { - } - - merge(localContent: string, remoteContent: string, baseContent: string | null, ignoredSettings: string[]): Promise<{ mergeContent: string, hasChanges: boolean, hasConflicts: boolean }> { - return this.channel.call('merge', [localContent, remoteContent, baseContent, ignoredSettings]); - } - - computeRemoteContent(localContent: string, remoteContent: string, ignoredSettings: string[]): Promise { - return this.channel.call('computeRemoteContent', [localContent, remoteContent, ignoredSettings]); - } - -} diff --git a/src/vs/platform/userDataSync/common/userDataSync.ts b/src/vs/platform/userDataSync/common/userDataSync.ts index bc76b41f9b0..21f92cacb8e 100644 --- a/src/vs/platform/userDataSync/common/userDataSync.ts +++ b/src/vs/platform/userDataSync/common/userDataSync.ts @@ -177,18 +177,6 @@ export interface IUserDataSyncService extends ISynchroniser { removeExtension(identifier: IExtensionIdentifier): Promise; } -export const ISettingsMergeService = createDecorator('ISettingsMergeService'); - -export interface ISettingsMergeService { - - _serviceBrand: undefined; - - merge(localContent: string, remoteContent: string, baseContent: string | null, ignoredSettings: string[]): Promise<{ mergeContent: string, hasChanges: boolean, hasConflicts: boolean }>; - - computeRemoteContent(localContent: string, remoteContent: string, ignoredSettings: string[]): Promise; - -} - export const IUserDataSyncUtilService = createDecorator('IUserDataSyncUtilService'); export interface IUserDataSyncUtilService { diff --git a/src/vs/platform/userDataSync/test/common/settingsMerge.test.ts b/src/vs/platform/userDataSync/test/common/settingsMerge.test.ts new file mode 100644 index 00000000000..911e11c97b7 --- /dev/null +++ b/src/vs/platform/userDataSync/test/common/settingsMerge.test.ts @@ -0,0 +1,547 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { merge, computeRemoteContent } from 'vs/platform/userDataSync/common/settingsMerge'; + +const formattingOptions = { eol: '\n', insertSpaces: false, tabSize: 4 }; + +suite('SettingsMerge - No Conflicts', () => { + + test('merge when local and remote are same with one entry', async () => { + const localContent = stringify({ 'a': 1 }); + const remoteContent = stringify({ 'a': 1 }); + const actual = merge(localContent, remoteContent, null, [], formattingOptions); + assert.ok(!actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('merge when local and remote are same with multiple entries', async () => { + const localContent = stringify({ + 'a': 1, + 'b': 2 + }); + const remoteContent = stringify({ + 'a': 1, + 'b': 2 + }); + const actual = merge(localContent, remoteContent, null, [], formattingOptions); + assert.ok(!actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('merge when local and remote are same with multiple entries in different order', async () => { + const localContent = stringify({ + 'b': 2, + 'a': 1, + }); + const remoteContent = stringify({ + 'a': 1, + 'b': 2 + }); + const actual = merge(localContent, remoteContent, null, [], formattingOptions); + assert.ok(!actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('merge when local and remote are same with different base content', async () => { + const localContent = stringify({ + 'b': 2, + 'a': 1, + }); + const baseContent = stringify({ + 'a': 2, + 'b': 1 + }); + const remoteContent = stringify({ + 'a': 1, + 'b': 2 + }); + const actual = merge(localContent, remoteContent, baseContent, [], formattingOptions); + assert.ok(!actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('merge when a new entry is added to remote', async () => { + const localContent = stringify({ + 'a': 1, + }); + const remoteContent = stringify({ + 'a': 1, + 'b': 2 + }); + const actual = merge(localContent, remoteContent, null, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, remoteContent); + }); + + test('merge when multiple new entries are added to remote', async () => { + const localContent = stringify({ + 'a': 1, + }); + const remoteContent = stringify({ + 'b': 2, + 'a': 1, + 'c': 3, + }); + const expected = stringify({ + 'a': 1, + 'b': 2, + 'c': 3, + }); + const actual = merge(localContent, remoteContent, null, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, expected); + }); + + test('merge when multiple new entries are added to remote from base and local has not changed', async () => { + const localContent = stringify({ + 'a': 1, + }); + const remoteContent = stringify({ + 'b': 2, + 'a': 1, + 'c': 3, + }); + const expected = stringify({ + 'a': 1, + 'b': 2, + 'c': 3, + }); + const actual = merge(localContent, remoteContent, localContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, expected); + }); + + test('merge when an entry is removed from remote from base and local has not changed', async () => { + const localContent = stringify({ + 'a': 1, + 'b': 2, + }); + const remoteContent = stringify({ + 'a': 1, + }); + const actual = merge(localContent, remoteContent, localContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, remoteContent); + }); + + test('merge when all entries are removed from base and local has not changed', async () => { + const localContent = stringify({ + 'a': 1, + }); + const remoteContent = stringify({}); + const actual = merge(localContent, remoteContent, localContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.deepEqual(JSON.parse(actual.mergeContent), {}); + }); + + test('merge when an entry is updated in remote from base and local has not changed', async () => { + const localContent = stringify({ + 'a': 1, + }); + const remoteContent = stringify({ + 'a': 2 + }); + const actual = merge(localContent, remoteContent, localContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, remoteContent); + }); + + test('merge when remote has moved forwareded with multiple changes and local stays with base', async () => { + const localContent = stringify({ + 'a': 1, + }); + const remoteContent = stringify({ + 'a': 2, + 'b': 1, + 'c': 3, + 'd': 4, + }); + const actual = merge(localContent, remoteContent, localContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, remoteContent); + }); + + test('merge when a new entries are added to local', async () => { + const localContent = stringify({ + 'a': 1, + 'b': 2, + 'c': 3, + 'd': 4, + }); + const remoteContent = stringify({ + 'a': 1, + }); + const actual = merge(localContent, remoteContent, null, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('merge when multiple new entries are added to local from base and remote is not changed', async () => { + const localContent = stringify({ + 'a': 2, + 'b': 1, + 'c': 3, + 'd': 4, + }); + const remoteContent = stringify({ + 'a': 1, + }); + const actual = merge(localContent, remoteContent, remoteContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('merge when an entry is removed from local from base and remote has not changed', async () => { + const localContent = stringify({ + 'a': 1, + 'c': 2 + }); + const remoteContent = stringify({ + 'a': 2, + 'b': 1, + 'c': 3, + 'd': 4, + }); + const actual = merge(localContent, remoteContent, remoteContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('merge when an entry is updated in local from base and remote has not changed', async () => { + const localContent = stringify({ + 'a': 1, + 'c': 2 + }); + const remoteContent = stringify({ + 'a': 2, + 'c': 2, + }); + const actual = merge(localContent, remoteContent, remoteContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('merge when local has moved forwarded with multiple changes and remote stays with base', async () => { + const localContent = stringify({ + 'a': 2, + 'b': 1, + 'c': 3, + 'd': 4, + }); + const remoteContent = stringify({ + 'a': 1, + }); + const actual = merge(localContent, remoteContent, remoteContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + +}); + +suite('SettingsMerge - Conflicts', () => { + + test('merge when local and remote with one entry but different value', async () => { + const localContent = stringify({ + 'a': 1 + }); + const remoteContent = stringify({ + 'a': 2 + }); + const actual = merge(localContent, remoteContent, null, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(actual.hasConflicts); + assert.equal(actual.mergeContent, + `{ +<<<<<<< local + "a": 1 +======= + "a": 2, +>>>>>>> remote +}`); + }); + + test('merge when the entry is removed in remote but updated in local and a new entry is added in remote', async () => { + const baseContent = stringify({ + 'a': 1 + }); + const localContent = stringify({ + 'a': 2 + }); + const remoteContent = stringify({ + 'b': 2 + }); + const actual = merge(localContent, remoteContent, baseContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(actual.hasConflicts); + assert.equal(actual.mergeContent, + `{ +<<<<<<< local + "a": 2, +======= +>>>>>>> remote + "b": 2 +}`); + }); + + test('merge with single entry and local is empty', async () => { + const baseContent = stringify({ + 'a': 1 + }); + const localContent = stringify({}); + const remoteContent = stringify({ + 'a': 2 + }); + const actual = merge(localContent, remoteContent, baseContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(actual.hasConflicts); + assert.equal(actual.mergeContent, + `{ +<<<<<<< local +======= + "a": 2, +>>>>>>> remote +}`); + }); + + test('merge when local and remote has moved forwareded with conflicts', async () => { + const baseContent = stringify({ + 'a': 1, + 'b': 2, + 'c': 3, + 'd': 4, + }); + const localContent = stringify({ + 'a': 2, + 'c': 3, + 'd': 5, + 'e': 4, + 'f': 1, + }); + const remoteContent = stringify({ + 'b': 3, + 'c': 3, + 'd': 6, + 'e': 5, + }); + const actual = merge(localContent, remoteContent, baseContent, [], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(actual.hasConflicts); + assert.equal(actual.mergeContent, + `{ +<<<<<<< local + "a": 2, +======= +>>>>>>> remote + "c": 3, +<<<<<<< local + "d": 5, +======= + "d": 6, +>>>>>>> remote +<<<<<<< local + "e": 4, +======= + "e": 5, +>>>>>>> remote + "f": 1 +<<<<<<< local +======= + "b": 3, +>>>>>>> remote +}`); + }); + +}); + +suite('SettingsMerge - Ignored Settings', () => { + + test('ignored setting is not merged when changed in local and remote', async () => { + const localContent = stringify({ 'a': 1 }); + const remoteContent = stringify({ 'a': 2 }); + const actual = merge(localContent, remoteContent, null, ['a'], formattingOptions); + assert.ok(!actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('ignored setting is not merged when changed in local and remote from base', async () => { + const baseContent = stringify({ 'a': 0 }); + const localContent = stringify({ 'a': 1 }); + const remoteContent = stringify({ 'a': 2 }); + const actual = merge(localContent, remoteContent, baseContent, ['a'], formattingOptions); + assert.ok(!actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('ignored setting is not merged when added in remote', async () => { + const localContent = stringify({}); + const remoteContent = stringify({ 'a': 1 }); + const actual = merge(localContent, remoteContent, null, ['a'], formattingOptions); + assert.ok(!actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('ignored setting is not merged when added in remote from base', async () => { + const localContent = stringify({ 'b': 2 }); + const remoteContent = stringify({ 'a': 1, 'b': 2 }); + const actual = merge(localContent, remoteContent, localContent, ['a'], formattingOptions); + assert.ok(!actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('ignored setting is not merged when removed in remote', async () => { + const localContent = stringify({ 'a': 1 }); + const remoteContent = stringify({}); + const actual = merge(localContent, remoteContent, null, ['a'], formattingOptions); + assert.ok(!actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('ignored setting is not merged when removed in remote from base', async () => { + const localContent = stringify({ 'a': 2 }); + const remoteContent = stringify({}); + const actual = merge(localContent, remoteContent, localContent, ['a'], formattingOptions); + assert.ok(!actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, localContent); + }); + + test('ignored setting is not merged with other changes without conflicts', async () => { + const baseContent = stringify({ + 'a': 2, + 'b': 2, + 'c': 3, + 'd': 4, + 'e': 5, + }); + const localContent = stringify({ + 'a': 1, + 'b': 2, + 'c': 3, + }); + const remoteContent = stringify({ + 'a': 3, + 'b': 3, + 'd': 4, + 'e': 6, + }); + const expectedContent = stringify({ + 'a': 1, + 'b': 3, + }); + const actual = merge(localContent, remoteContent, baseContent, ['a', 'e'], formattingOptions); + assert.ok(actual.hasChanges); + assert.ok(!actual.hasConflicts); + assert.equal(actual.mergeContent, expectedContent); + }); + + test('ignored setting is not merged with other changes conflicts', async () => { + const baseContent = stringify({ + 'a': 2, + 'b': 2, + 'c': 3, + 'd': 4, + 'e': 5, + }); + const localContent = stringify({ + 'a': 1, + 'b': 4, + 'c': 3, + 'd': 5, + }); + const remoteContent = stringify({ + 'a': 3, + 'b': 3, + 'e': 6, + }); + const actual = merge(localContent, remoteContent, baseContent, ['a', 'e'], formattingOptions); + //'{\n\t"a": 1,\n\n<<<<<<< local\t"b": 4,\n=======\n\t"b": 3,\n>>>>>>> remote' + //'{\n\t"a": 1,\n<<<<<<< local\n\t"b": 4,\n=======\n\t"b": 3,\n>>>>>>> remote\n<<<<<<< local\n\t"d": 5\n=======\n>>>>>>> remote\n}' + assert.ok(actual.hasChanges); + assert.ok(actual.hasChanges); + assert.ok(actual.hasConflicts); + assert.equal(actual.mergeContent, + `{ + "a": 1, +<<<<<<< local + "b": 4, +======= + "b": 3, +>>>>>>> remote +<<<<<<< local + "d": 5 +======= +>>>>>>> remote +}`); + }); + +}); + +suite('SettingsMerge - Compute Remote Content', () => { + + test('local content is returned when there are no ignored settings', async () => { + const localContent = stringify({ + 'a': 1, + 'b': 2, + 'c': 3, + }); + const remoteContent = stringify({ + 'a': 3, + 'b': 3, + 'd': 4, + 'e': 6, + }); + const actual = computeRemoteContent(localContent, remoteContent, [], formattingOptions); + assert.equal(actual, localContent); + }); + + test('ignored settings are not updated from remote content', async () => { + const localContent = stringify({ + 'a': 1, + 'b': 2, + 'c': 3, + }); + const remoteContent = stringify({ + 'a': 3, + 'b': 3, + 'd': 4, + 'e': 6, + }); + const expected = stringify({ + 'a': 3, + 'b': 2, + 'c': 3, + }); + const actual = computeRemoteContent(localContent, remoteContent, ['a'], formattingOptions); + assert.equal(actual, expected); + }); + +}); + +function stringify(value: any): string { + return JSON.stringify(value, null, '\t'); +} diff --git a/src/vs/workbench/contrib/userDataSync/electron-browser/userDataSync.contribution.ts b/src/vs/workbench/contrib/userDataSync/electron-browser/userDataSync.contribution.ts index 1ff66d24159..db872bdd689 100644 --- a/src/vs/workbench/contrib/userDataSync/electron-browser/userDataSync.contribution.ts +++ b/src/vs/workbench/contrib/userDataSync/electron-browser/userDataSync.contribution.ts @@ -4,21 +4,18 @@ *--------------------------------------------------------------------------------------------*/ import { IWorkbenchContributionsRegistry, Extensions as WorkbenchExtensions, IWorkbenchContribution } from 'vs/workbench/common/contributions'; -import { ISettingsMergeService, IUserDataSyncUtilService } from 'vs/platform/userDataSync/common/userDataSync'; +import { IUserDataSyncUtilService } from 'vs/platform/userDataSync/common/userDataSync'; import { Registry } from 'vs/platform/registry/common/platform'; import { LifecyclePhase } from 'vs/platform/lifecycle/common/lifecycle'; import { ISharedProcessService } from 'vs/platform/ipc/electron-browser/sharedProcessService'; -import { SettingsMergeChannel } from 'vs/platform/userDataSync/common/settingsSyncIpc'; import { UserDataSycnUtilServiceChannel } from 'vs/platform/userDataSync/common/keybindingsSyncIpc'; class UserDataSyncServicesContribution implements IWorkbenchContribution { constructor( - @ISettingsMergeService settingsMergeService: ISettingsMergeService, @IUserDataSyncUtilService userDataSyncUtilService: IUserDataSyncUtilService, @ISharedProcessService sharedProcessService: ISharedProcessService, ) { - sharedProcessService.registerChannel('settingsMerge', new SettingsMergeChannel(settingsMergeService)); sharedProcessService.registerChannel('userDataSyncUtil', new UserDataSycnUtilServiceChannel(userDataSyncUtilService)); } } diff --git a/src/vs/workbench/services/userDataSync/common/settingsMergeService.ts b/src/vs/workbench/services/userDataSync/common/settingsMergeService.ts deleted file mode 100644 index 3e3c652ff1f..00000000000 --- a/src/vs/workbench/services/userDataSync/common/settingsMergeService.ts +++ /dev/null @@ -1,208 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import * as objects from 'vs/base/common/objects'; -import { parse, findNodeAtLocation, parseTree } from 'vs/base/common/json'; -import { EditOperation } from 'vs/editor/common/core/editOperation'; -import { IModeService } from 'vs/editor/common/services/modeService'; -import { ITextModel } from 'vs/editor/common/model'; -import { setProperty } from 'vs/base/common/jsonEdit'; -import { Range } from 'vs/editor/common/core/range'; -import { Selection } from 'vs/editor/common/core/selection'; -import { IModelService } from 'vs/editor/common/services/modelService'; -import { Position } from 'vs/editor/common/core/position'; -import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; -import { ISettingsMergeService } from 'vs/platform/userDataSync/common/userDataSync'; -import { values } from 'vs/base/common/map'; -import { IStringDictionary } from 'vs/base/common/collections'; - -class SettingsMergeService implements ISettingsMergeService { - - _serviceBrand: undefined; - - constructor( - @IModelService private readonly modelService: IModelService, - @IModeService private readonly modeService: IModeService, - ) { } - - async merge(localContent: string, remoteContent: string, baseContent: string | null, ignoredSettings: string[]): Promise<{ mergeContent: string, hasChanges: boolean, hasConflicts: boolean }> { - const local = parse(localContent); - const remote = parse(remoteContent); - const base = baseContent ? parse(baseContent) : null; - const ignored = ignoredSettings.reduce((set, key) => { set.add(key); return set; }, new Set()); - - const localToRemote = this.compare(local, remote, ignored); - if (localToRemote.added.size === 0 && localToRemote.removed.size === 0 && localToRemote.updated.size === 0) { - // No changes found between local and remote. - return { mergeContent: localContent, hasChanges: false, hasConflicts: false }; - } - - const conflicts: Set = new Set(); - const baseToLocal = base ? this.compare(base, local, ignored) : { added: Object.keys(local).reduce((r, k) => { r.add(k); return r; }, new Set()), removed: new Set(), updated: new Set() }; - const baseToRemote = base ? this.compare(base, remote, ignored) : { added: Object.keys(remote).reduce((r, k) => { r.add(k); return r; }, new Set()), removed: new Set(), updated: new Set() }; - const settingsPreviewModel = this.modelService.createModel(localContent, this.modeService.create('jsonc')); - - // Removed settings in Local - for (const key of values(baseToLocal.removed)) { - // Got updated in remote - if (baseToRemote.updated.has(key)) { - conflicts.add(key); - } - } - - // Removed settings in Remote - for (const key of values(baseToRemote.removed)) { - if (conflicts.has(key)) { - continue; - } - // Got updated in local - if (baseToLocal.updated.has(key)) { - conflicts.add(key); - } else { - this.editSetting(settingsPreviewModel, key, undefined); - } - } - - // Added settings in Local - for (const key of values(baseToLocal.added)) { - if (conflicts.has(key)) { - continue; - } - // Got added in remote - if (baseToRemote.added.has(key)) { - // Has different value - if (localToRemote.updated.has(key)) { - conflicts.add(key); - } - } - } - - // Added settings in remote - for (const key of values(baseToRemote.added)) { - if (conflicts.has(key)) { - continue; - } - // Got added in local - if (baseToLocal.added.has(key)) { - // Has different value - if (localToRemote.updated.has(key)) { - conflicts.add(key); - } - } else { - this.editSetting(settingsPreviewModel, key, remote[key]); - } - } - - // Updated settings in Local - for (const key of values(baseToLocal.updated)) { - if (conflicts.has(key)) { - continue; - } - // Got updated in remote - if (baseToRemote.updated.has(key)) { - // Has different value - if (localToRemote.updated.has(key)) { - conflicts.add(key); - } - } - } - - // Updated settings in Remote - for (const key of values(baseToRemote.updated)) { - if (conflicts.has(key)) { - continue; - } - // Got updated in local - if (baseToLocal.updated.has(key)) { - // Has different value - if (localToRemote.updated.has(key)) { - conflicts.add(key); - } - } else { - this.editSetting(settingsPreviewModel, key, remote[key]); - } - } - - for (const key of values(conflicts)) { - const tree = parseTree(settingsPreviewModel.getValue()); - const valueNode = findNodeAtLocation(tree, [key]); - const eol = settingsPreviewModel.getEOL(); - const remoteEdit = setProperty(`{${eol}\t${eol}}`, [key], remote[key], { tabSize: 4, insertSpaces: false, eol: eol })[0]; - const remoteContent = remoteEdit ? `${remoteEdit.content.substring(remoteEdit.offset + remoteEdit.length + 1)},${eol}` : ''; - if (valueNode) { - // Updated in Local and Remote with different value - const keyPosition = settingsPreviewModel.getPositionAt(valueNode.parent!.offset); - const valuePosition = settingsPreviewModel.getPositionAt(valueNode.offset + valueNode.length); - const editOperations = [ - EditOperation.insert(new Position(keyPosition.lineNumber - 1, settingsPreviewModel.getLineMaxColumn(keyPosition.lineNumber - 1)), `${eol}<<<<<<< local`), - EditOperation.insert(new Position(valuePosition.lineNumber, settingsPreviewModel.getLineMaxColumn(valuePosition.lineNumber)), `${eol}=======${eol}${remoteContent}>>>>>>> remote`) - ]; - settingsPreviewModel.pushEditOperations([new Selection(keyPosition.lineNumber, keyPosition.column, keyPosition.lineNumber, keyPosition.column)], editOperations, () => []); - } else { - // Removed in Local, but updated in Remote - const position = new Position(settingsPreviewModel.getLineCount() - 1, settingsPreviewModel.getLineMaxColumn(settingsPreviewModel.getLineCount() - 1)); - const editOperations = [ - EditOperation.insert(position, `${eol}<<<<<<< local${eol}=======${eol}${remoteContent}>>>>>>> remote`) - ]; - settingsPreviewModel.pushEditOperations([new Selection(position.lineNumber, position.column, position.lineNumber, position.column)], editOperations, () => []); - } - } - return { mergeContent: settingsPreviewModel.getValue(), hasChanges: true, hasConflicts: conflicts.size > 0 }; - } - - async computeRemoteContent(localContent: string, remoteContent: string, ignoredSettings: string[]): Promise { - const remote = parse(remoteContent); - const remoteModel = this.modelService.createModel(localContent, this.modeService.create('jsonc')); - const ignored = ignoredSettings.reduce((set, key) => { set.add(key); return set; }, new Set()); - for (const key of ignoredSettings) { - if (ignored.has(key)) { - this.editSetting(remoteModel, key, undefined); - this.editSetting(remoteModel, key, remote[key]); - } - } - return remoteModel.getValue(); - } - - private editSetting(model: ITextModel, key: string, value: any | undefined): void { - const insertSpaces = model.getOptions().insertSpaces; - const tabSize = model.getOptions().tabSize; - const eol = model.getEOL(); - const edit = setProperty(model.getValue(), [key], value, { tabSize, insertSpaces, eol })[0]; - if (edit) { - const startPosition = model.getPositionAt(edit.offset); - const endPosition = model.getPositionAt(edit.offset + edit.length); - const range = new Range(startPosition.lineNumber, startPosition.column, endPosition.lineNumber, endPosition.column); - let currentText = model.getValueInRange(range); - if (edit.content !== currentText) { - const editOperation = currentText ? EditOperation.replace(range, edit.content) : EditOperation.insert(startPosition, edit.content); - model.pushEditOperations([new Selection(startPosition.lineNumber, startPosition.column, startPosition.lineNumber, startPosition.column)], [editOperation], () => []); - } - } - } - - private compare(from: IStringDictionary, to: IStringDictionary, ignored: Set): { added: Set, removed: Set, updated: Set } { - const fromKeys = Object.keys(from).filter(key => !ignored.has(key)); - const toKeys = Object.keys(to).filter(key => !ignored.has(key)); - const added = toKeys.filter(key => fromKeys.indexOf(key) === -1).reduce((r, key) => { r.add(key); return r; }, new Set()); - const removed = fromKeys.filter(key => toKeys.indexOf(key) === -1).reduce((r, key) => { r.add(key); return r; }, new Set()); - const updated: Set = new Set(); - - for (const key of fromKeys) { - if (removed.has(key)) { - continue; - } - const value1 = from[key]; - const value2 = to[key]; - if (!objects.equals(value1, value2)) { - updated.add(key); - } - } - - return { added, removed, updated }; - } - -} - -registerSingleton(ISettingsMergeService, SettingsMergeService); diff --git a/src/vs/workbench/workbench.common.main.ts b/src/vs/workbench/workbench.common.main.ts index 9f9f8a80d7c..c0b70a6d0e9 100644 --- a/src/vs/workbench/workbench.common.main.ts +++ b/src/vs/workbench/workbench.common.main.ts @@ -79,7 +79,6 @@ import 'vs/workbench/services/label/common/labelService'; import 'vs/workbench/services/extensionManagement/common/extensionEnablementService'; import 'vs/workbench/services/notification/common/notificationService'; import 'vs/workbench/services/extensions/common/staticExtensions'; -import 'vs/workbench/services/userDataSync/common/settingsMergeService'; import 'vs/workbench/services/userDataSync/common/userDataSyncUtil'; import 'vs/workbench/services/path/common/remotePathService'; import 'vs/workbench/services/remote/common/remoteExplorerService';