implemente better merging strategy

src/vs/workbench/services/keybinding/common/keybindingsMerge.ts

@@ -4,179 +4,269 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as objects from 'vs/base/common/objects';
-import { parse, findNodeAtLocation, parseTree } from 'vs/base/common/json';
+import { parse } from 'vs/base/common/json';
 import { values, keys } from 'vs/base/common/map';
-import { IUserFriendlyKeybinding } from 'vs/platform/keybinding/common/keybinding';
+import { IUserFriendlyKeybinding, IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
 import { firstIndex as findFirstIndex, equals } from 'vs/base/common/arrays';
 import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
 import * as contentUtil from 'vs/platform/userDataSync/common/content';
 import { IKeybindingsMergeService } from 'vs/platform/userDataSync/common/userDataSync';
 
+interface ICompareResult {
+	added: Set<string>;
+	removed: Set<string>;
+	updated: Set<string>;
+}
+
+interface IMergeResult {
+	added: Set<string>;
+	removed: Set<string>;
+	updated: Set<string>;
+	conflicts: Set<string>;
+}
+
 export class KeybindingsMergeService implements IKeybindingsMergeService {
 
 	_serviceBrand: undefined;
 
+	constructor(
+		@IKeybindingService private readonly keybindingsService: IKeybindingService
+	) { }
+
 	public async merge(localContent: string, remoteContent: string, baseContent: string | null): Promise<{ mergeContent: string, hasChanges: boolean, hasConflicts: boolean }> {
 		const local = <IUserFriendlyKeybinding[]>parse(localContent);
 		const remote = <IUserFriendlyKeybinding[]>parse(remoteContent);
 		const base = baseContent ? <IUserFriendlyKeybinding[]>parse(baseContent) : null;
 
-		const byCommand = (keybindings: IUserFriendlyKeybinding[]) => {
-			const map: Map<string, IUserFriendlyKeybinding[]> = new Map<string, IUserFriendlyKeybinding[]>();
-			for (const keybinding of keybindings) {
-				const command = keybinding.command[0] === '-' ? keybinding.command.substring(1) : keybinding.command;
-				let value = map.get(command);
+		const normalize = (keybinding: IUserFriendlyKeybinding): IUserFriendlyKeybinding => ({
+			...keybinding,
+			...{
+				key: this.keybindingsService.resolveUserBinding(keybinding.key).map(part => part.getUserSettingsLabel()).join(' ')
+			}
+		});
+
+		const normalizedLocal = local.map(keybinding => normalize(keybinding));
+		const normalizedRemote = remote.map(keybinding => normalize(keybinding));
+		const normalizedBase = base ? base.map(keybinding => normalize(keybinding)) : null;
+
+		const byKeybinding = (keybindings: IUserFriendlyKeybinding[], normalized: IUserFriendlyKeybinding[]) => {
+			const map: Map<string, { keybinding: IUserFriendlyKeybinding, normalized: IUserFriendlyKeybinding }[]> = new Map<string, { keybinding: IUserFriendlyKeybinding, normalized: IUserFriendlyKeybinding }[]>();
+			for (let index = 0; index < normalized.length; index++) {
+				let value = map.get(normalized[index].key);
 				if (!value) {
 					value = [];
-					map.set(command, value);
+					map.set(normalized[index].key, value);
 				}
-				value.push(keybinding);
+				value.push({ keybinding: keybindings[index], normalized: normalized[index] });
 			}
 			return map;
 		};
 
-		const localByCommand = byCommand(local);
-		const remoteByCommand = byCommand(remote);
-		const baseByCommand = base ? byCommand(base) : null;
+		const localByKeybinding = byKeybinding(local, normalizedLocal);
+		const remoteByKeybinding = byKeybinding(remote, normalizedRemote);
+		const baseByKeybinding = base ? byKeybinding(base, normalizedBase!) : null;
 
-		const localToRemote = this.compare(localByCommand, remoteByCommand);
-		if (localToRemote.added.size === 0 && localToRemote.removed.size === 0 && localToRemote.updated.size === 0) {
+		const localToRemoteByKeybinding = this.compareByKeybinding(localByKeybinding, remoteByKeybinding);
+		if (localToRemoteByKeybinding.added.size === 0 && localToRemoteByKeybinding.removed.size === 0 && localToRemoteByKeybinding.updated.size === 0) {
 			// No changes found between local and remote.
 			return { mergeContent: localContent, hasChanges: false, hasConflicts: false };
 		}
 
-		const conflictCommands: Set<string> = new Set<string>();
-		const baseToLocal = baseByCommand ? this.compare(baseByCommand, localByCommand) : { added: keys(localByCommand).reduce((r, k) => { r.add(k); return r; }, new Set<string>()), removed: new Set<string>(), updated: new Set<string>() };
-		const baseToRemote = baseByCommand ? this.compare(baseByCommand, remoteByCommand) : { added: keys(remoteByCommand).reduce((r, k) => { r.add(k); return r; }, new Set<string>()), removed: new Set<string>(), updated: new Set<string>() };
+		const baseToLocalByKeybinding = baseByKeybinding ? this.compareByKeybinding(baseByKeybinding, localByKeybinding) : { added: keys(localByKeybinding).reduce((r, k) => { r.add(k); return r; }, new Set<string>()), removed: new Set<string>(), updated: new Set<string>() };
+		if (baseToLocalByKeybinding.added.size === 0 && baseToLocalByKeybinding.removed.size === 0 && baseToLocalByKeybinding.updated.size === 0) {
+			// Remote has moved forward and local has not. Return remote
+			return { mergeContent: remoteContent, hasChanges: true, hasConflicts: false };
+		}
+
+		const baseToRemoteByKeybinding = baseByKeybinding ? this.compareByKeybinding(baseByKeybinding, remoteByKeybinding) : { added: keys(remoteByKeybinding).reduce((r, k) => { r.add(k); return r; }, new Set<string>()), removed: new Set<string>(), updated: new Set<string>() };
+		if (baseToRemoteByKeybinding.added.size === 0 && baseToRemoteByKeybinding.removed.size === 0 && baseToRemoteByKeybinding.updated.size === 0) {
+			// Local has moved forward and remote has not. Return local.
+			return { mergeContent: localContent, hasChanges: true, hasConflicts: false };
+		}
+
+		// Both local and remote has moved forward.
+		const byCommand = (keybindings: IUserFriendlyKeybinding[], normalized: IUserFriendlyKeybinding[]) => {
+			const map: Map<string, { keybinding: IUserFriendlyKeybinding, normalized: IUserFriendlyKeybinding }[]> = new Map<string, { keybinding: IUserFriendlyKeybinding, normalized: IUserFriendlyKeybinding }[]>();
+			for (let index = 0; index < normalized.length; index++) {
+				const command = normalized[index].command[0] === '-' ? normalized[index].command.substring(1) : normalized[index].command;
+				let value = map.get(command);
+				if (!value) {
+					value = [];
+					map.set(command, value);
+				}
+				value.push({ keybinding: keybindings[index], normalized: normalized[index] });
+			}
+			return map;
+		};
+		const localByCommand = byCommand(local, normalizedLocal);
+		const remoteByCommand = byCommand(remote, normalizedRemote);
+		const baseByCommand = base ? byCommand(base, normalizedBase!) : null;
+		const localToRemoteByCommand = this.compareByCommand(localByCommand, remoteByCommand);
+		const baseToLocalByCommand = baseByCommand ? this.compareByCommand(baseByCommand, localByCommand) : { added: keys(localByCommand).reduce((r, k) => { r.add(k); return r; }, new Set<string>()), removed: new Set<string>(), updated: new Set<string>() };
+		const baseToRemoteByCommand = baseByCommand ? this.compareByCommand(baseByCommand, remoteByCommand) : { added: keys(remoteByCommand).reduce((r, k) => { r.add(k); return r; }, new Set<string>()), removed: new Set<string>(), updated: new Set<string>() };
+
+		const commandsMergeResult = this.computeMergeResult(localToRemoteByCommand, baseToLocalByCommand, baseToRemoteByCommand);
+		const keybindingsMergeResult = this.computeMergeResult(localToRemoteByKeybinding, baseToLocalByKeybinding, baseToRemoteByKeybinding);
 		const eol = contentUtil.getEol(localContent);
 		let mergeContent = localContent;
 
-		// Removed commands in Local
-		for (const command of values(baseToLocal.removed)) {
+		// Removed commands in Remote
+		for (const command of values(commandsMergeResult.removed)) {
+			if (commandsMergeResult.conflicts.has(command)) {
+				continue;
+			}
+			mergeContent = this.removeKeybindings(mergeContent, eol, command);
+		}
+
+		// Added commands in remote
+		for (const command of values(commandsMergeResult.added)) {
+			if (commandsMergeResult.conflicts.has(command)) {
+				continue;
+			}
+			const keybindings = remoteByCommand.get(command)!;
+			if (keybindings.some(({ normalized }) => keybindingsMergeResult.conflicts.has(normalized.key))) {
+				continue;
+			}
+			mergeContent = this.addKeybindings(mergeContent, eol, keybindings.map(({ keybinding }) => keybinding));
+		}
+
+		// Updated commands in Remote
+		for (const command of values(commandsMergeResult.updated)) {
+			if (commandsMergeResult.conflicts.has(command)) {
+				continue;
+			}
+			const keybindings = remoteByCommand.get(command)!;
+			if (keybindings.some(({ normalized }) => keybindingsMergeResult.conflicts.has(normalized.key))) {
+				continue;
+			}
+			mergeContent = this.updateKeybindings(mergeContent, eol, command, keybindings.map(({ keybinding }) => keybinding));
+		}
+
+		const hasConflicts = commandsMergeResult.conflicts.size > 0 || keybindingsMergeResult.conflicts.size > 0;
+		if (hasConflicts) {
+			mergeContent = `<<<<<<< local${eol}`
+				+ mergeContent
+				+ `${eol}=======${eol}`
+				+ remoteContent
+				+ `${eol}>>>>>>> remote`;
+		}
+
+		return { mergeContent, hasChanges: true, hasConflicts };
+	}
+
+	private computeMergeResult(localToRemote: ICompareResult, baseToLocal: ICompareResult, baseToRemote: ICompareResult): IMergeResult {
+		const added: Set<string> = new Set<string>();
+		const removed: Set<string> = new Set<string>();
+		const updated: Set<string> = new Set<string>();
+		const conflicts: Set<string> = new Set<string>();
+
+		// Removed keys in Local
+		for (const key of values(baseToLocal.removed)) {
 			// Got updated in remote
-			if (baseToRemote.updated.has(command)) {
-				conflictCommands.add(command);
+			if (baseToRemote.updated.has(key)) {
+				conflicts.add(key);
 			}
 		}
 
-		// Removed commands in Remote
-		for (const command of values(baseToRemote.removed)) {
-			if (conflictCommands.has(command)) {
+		// Removed keys in Remote
+		for (const key of values(baseToRemote.removed)) {
+			if (conflicts.has(key)) {
 				continue;
 			}
 			// Got updated in local
-			if (baseToLocal.updated.has(command)) {
-				conflictCommands.add(command);
+			if (baseToLocal.updated.has(key)) {
+				conflicts.add(key);
 			} else {
-				// remove the command
-				mergeContent = this.removeKeybindings(mergeContent, eol, command);
+				// remove the key
+				removed.add(key);
 			}
 		}
 
-		// Added commands in Local
-		for (const command of values(baseToLocal.added)) {
-			if (conflictCommands.has(command)) {
+		// Added keys in Local
+		for (const key of values(baseToLocal.added)) {
+			if (conflicts.has(key)) {
 				continue;
 			}
 			// Got added in remote
-			if (baseToRemote.added.has(command)) {
+			if (baseToRemote.added.has(key)) {
 				// Has different value
-				if (localToRemote.updated.has(command)) {
-					conflictCommands.add(command);
+				if (localToRemote.updated.has(key)) {
+					conflicts.add(key);
 				}
 			}
 		}
 
-		// Added commands in remote
-		for (const command of values(baseToRemote.added)) {
-			if (conflictCommands.has(command)) {
+		// Added keys in remote
+		for (const key of values(baseToRemote.added)) {
+			if (conflicts.has(key)) {
 				continue;
 			}
 			// Got added in local
-			if (baseToLocal.added.has(command)) {
+			if (baseToLocal.added.has(key)) {
 				// Has different value
-				if (localToRemote.updated.has(command)) {
-					conflictCommands.add(command);
+				if (localToRemote.updated.has(key)) {
+					conflicts.add(key);
 				}
 			} else {
-				mergeContent = this.addKeybinding(mergeContent, eol, command, remoteByCommand.get(command)!);
+				added.add(key);
 			}
 		}
 
-		// Updated commands in Local
-		for (const command of values(baseToLocal.updated)) {
-			if (conflictCommands.has(command)) {
+		// Updated keys in Local
+		for (const key of values(baseToLocal.updated)) {
+			if (conflicts.has(key)) {
 				continue;
 			}
 			// Got updated in remote
-			if (baseToRemote.updated.has(command)) {
+			if (baseToRemote.updated.has(key)) {
 				// Has different value
-				if (localToRemote.updated.has(command)) {
-					conflictCommands.add(command);
+				if (localToRemote.updated.has(key)) {
+					conflicts.add(key);
 				}
 			}
 		}
 
-		// Updated commands in Remote
-		for (const command of values(baseToRemote.updated)) {
-			if (conflictCommands.has(command)) {
+		// Updated keys in Remote
+		for (const key of values(baseToRemote.updated)) {
+			if (conflicts.has(key)) {
 				continue;
 			}
 			// Got updated in local
-			if (baseToLocal.updated.has(command)) {
+			if (baseToLocal.updated.has(key)) {
 				// Has different value
-				if (localToRemote.updated.has(command)) {
-					conflictCommands.add(command);
+				if (localToRemote.updated.has(key)) {
+					conflicts.add(key);
 				}
 			} else {
-				// update the command
-				mergeContent = this.updateKeybinding(mergeContent, eol, command, remoteByCommand.get(command)!);
+				// updated key
+				updated.add(key);
 			}
 		}
+		return { added, removed, updated, conflicts };
+	}
 
-		const conflicts: { command: string, local: IUserFriendlyKeybinding[] | undefined, remote: IUserFriendlyKeybinding[] | undefined, firstIndex: number }[] = [];
-
-		for (const command of values(conflictCommands)) {
-			const local = localByCommand.get(command);
-			const remote = remoteByCommand.get(command);
-			mergeContent = this.updateKeybinding(mergeContent, eol, command, [...local || [], ...remote || []]);
-			conflicts.push({ command, local, remote, firstIndex: -1 });
-		}
+	private compareByKeybinding(from: Map<string, { keybinding: IUserFriendlyKeybinding, normalized: IUserFriendlyKeybinding }[]>, to: Map<string, { keybinding: IUserFriendlyKeybinding, normalized: IUserFriendlyKeybinding }[]>): ICompareResult {
+		const fromKeys = keys(from);
+		const toKeys = keys(to);
+		const added = toKeys.filter(key => fromKeys.indexOf(key) === -1).reduce((r, key) => { r.add(key); return r; }, new Set<string>());
+		const removed = fromKeys.filter(key => toKeys.indexOf(key) === -1).reduce((r, key) => { r.add(key); return r; }, new Set<string>());
+		const updated: Set<string> = new Set<string>();
 
-		const allKeybindings = <IUserFriendlyKeybinding[]>parse(mergeContent);
-		for (const conflict of conflicts) {
-			conflict.firstIndex = findFirstIndex(allKeybindings, keybinding => keybinding.command === conflict.command || keybinding.command === `-${conflict.command}`);
-		}
-		// Sort reverse so that conflicts content is added from last
-		conflicts.sort((a, b) => b.firstIndex - a.firstIndex);
-
-		const tree = parseTree(mergeContent);
-		for (const { firstIndex, local, remote } of conflicts) {
-			const firstNode = findNodeAtLocation(tree, [firstIndex])!;
-			const startLocalOffset = contentUtil.getLineStartOffset(mergeContent, eol, firstNode.offset) - eol.length;
-			let endLocalOffset = startLocalOffset;
-			if (local) {
-				const lastLocalValueNode = findNodeAtLocation(tree, [firstIndex + local.length - 1])!;
-				endLocalOffset = contentUtil.getLineEndOffset(mergeContent, eol, lastLocalValueNode.offset + lastLocalValueNode.length);
+		for (const key of fromKeys) {
+			if (removed.has(key)) {
+				continue;
 			}
-			let remoteOffset = endLocalOffset;
-			if (remote) {
-				const lastRemoteValueNode = findNodeAtLocation(tree, [firstIndex + (local ? local.length : 0) + remote.length - 1])!;
-				remoteOffset = contentUtil.getLineEndOffset(mergeContent, eol, lastRemoteValueNode.offset + lastRemoteValueNode.length);
+			const value1: IUserFriendlyKeybinding[] = from.get(key)!.map(({ normalized }) => normalized);
+			const value2: IUserFriendlyKeybinding[] = to.get(key)!.map(({ normalized }) => normalized);
+			if (!equals(value1, value2, (a, b) => this.isSameKeybinding(a, b))) {
+				updated.add(key);
 			}
-			mergeContent = mergeContent.substring(0, startLocalOffset)
-				+ `${eol}<<<<<<< local`
-				+ mergeContent.substring(startLocalOffset, endLocalOffset)
-				+ `${eol}=======`
-				+ mergeContent.substring(endLocalOffset, remoteOffset)
-				+ `${eol}>>>>>>> remote`
-				+ mergeContent.substring(remoteOffset);
 		}
 
-		return { mergeContent, hasChanges: true, hasConflicts: conflicts.length > 0 };
+		return { added, removed, updated };
 	}
 
-	private compare(from: Map<string, IUserFriendlyKeybinding[]>, to: Map<string, IUserFriendlyKeybinding[]>): { added: Set<string>, removed: Set<string>, updated: Set<string> } {
+	private compareByCommand(from: Map<string, { keybinding: IUserFriendlyKeybinding, normalized: IUserFriendlyKeybinding }[]>, to: Map<string, { keybinding: IUserFriendlyKeybinding, normalized: IUserFriendlyKeybinding }[]>): ICompareResult {
 		const fromKeys = keys(from);
 		const toKeys = keys(to);
 		const added = toKeys.filter(key => fromKeys.indexOf(key) === -1).reduce((r, key) => { r.add(key); return r; }, new Set<string>());
@@ -187,9 +277,9 @@ export class KeybindingsMergeService implements IKeybindingsMergeService {
 			if (removed.has(key)) {
 				continue;
 			}
-			const value1: IUserFriendlyKeybinding[] = from.get(key)!;
-			const value2: IUserFriendlyKeybinding[] = to.get(key)!;
-			if (!this.areSameKeybindings(value1, value2)) {
+			const value1: IUserFriendlyKeybinding[] = from.get(key)!.map(({ normalized }) => normalized);
+			const value2: IUserFriendlyKeybinding[] = to.get(key)!.map(({ normalized }) => normalized);
+			if (!this.areSameKeybindingsWithSameCommand(value1, value2)) {
 				updated.add(key);
 			}
 		}
@@ -197,7 +287,7 @@ export class KeybindingsMergeService implements IKeybindingsMergeService {
 		return { added, removed, updated };
 	}
 
-	private areSameKeybindings(value1: IUserFriendlyKeybinding[], value2: IUserFriendlyKeybinding[]): boolean {
+	private areSameKeybindingsWithSameCommand(value1: IUserFriendlyKeybinding[], value2: IUserFriendlyKeybinding[]): boolean {
 		// Compare entries adding keybindings
 		if (!equals(value1.filter(({ command }) => command[0] !== '-'), value2.filter(({ command }) => command[0] !== '-'), (a, b) => this.isSameKeybinding(a, b))) {
 			return false;
@@ -230,7 +320,7 @@ export class KeybindingsMergeService implements IKeybindingsMergeService {
 		return true;
 	}
 
-	private addKeybinding(content: string, eol: string, command: string, keybindings: IUserFriendlyKeybinding[]): string {
+	private addKeybindings(content: string, eol: string, keybindings: IUserFriendlyKeybinding[]): string {
 		for (const keybinding of keybindings) {
 			content = contentUtil.edit(content, eol, [-1], keybinding);
 		}
@@ -247,7 +337,7 @@ export class KeybindingsMergeService implements IKeybindingsMergeService {
 		return content;
 	}
 
-	private updateKeybinding(content: string, eol: string, command: string, keybindings: IUserFriendlyKeybinding[]): string {
+	private updateKeybindings(content: string, eol: string, command: string, keybindings: IUserFriendlyKeybinding[]): string {
 		const allKeybindings = <IUserFriendlyKeybinding[]>parse(content);
 		const location = findFirstIndex(allKeybindings, keybinding => keybinding.command === command || keybinding.command === `-${command}`);
 		// Remove all entries with this command

src/vs/workbench/services/keybinding/test/electron-browser/keybindingsMerge.test.ts

@@ -4,12 +4,23 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as assert from 'assert';
-import { workbenchInstantiationService } from 'vs/workbench/test/workbenchTestServices';
+import { OS } from 'vs/base/common/platform';
 import { KeybindingsMergeService } from 'vs/workbench/services/keybinding/common/keybindingsMerge';
+import { MockKeybindingService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
+import { ResolvedKeybinding, ChordKeybinding, SimpleKeybinding } from 'vs/base/common/keyCodes';
+import { KeybindingParser } from 'vs/base/common/keybindingParser';
+import { USLayoutResolvedKeybinding } from 'vs/platform/keybinding/common/usLayoutResolvedKeybinding';
 
 let testObject: KeybindingsMergeService;
 
-suiteSetup(() => testObject = workbenchInstantiationService().createInstance(KeybindingsMergeService));
+suiteSetup(() => {
+	testObject = new KeybindingsMergeService(new class extends MockKeybindingService {
+		resolveUserBinding(userBinding: string): ResolvedKeybinding[] {
+			const parts = <SimpleKeybinding[]>KeybindingParser.parseUserBinding(userBinding);
+			return [new USLayoutResolvedKeybinding(new ChordKeybinding(parts), OS)];
+		}
+	});
+});
 
 suite('KeybindingsMerge - No Conflicts', () => {
 
@@ -31,6 +42,21 @@ suite('KeybindingsMerge - No Conflicts', () => {
 		assert.equal(actual.mergeContent, localContent);
 	});
 
+	test('merge when local and remote has entries in different order', async () => {
+		const localContent = stringify([
+			{ key: 'alt+d', command: 'a', when: 'editorTextFocus && !editorReadonly' },
+			{ key: 'alt+a', command: 'a', when: 'editorTextFocus' }
+		]);
+		const remoteContent = stringify([
+			{ key: 'alt+a', command: 'a', when: 'editorTextFocus' },
+			{ key: 'alt+d', command: 'a', when: 'editorTextFocus && !editorReadonly' }
+		]);
+		const actual = await testObject.merge(localContent, remoteContent, null);
+		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([
 			{ key: 'alt+c', command: 'a', when: 'editorTextFocus && !editorReadonly' },
@@ -209,16 +235,10 @@ suite('KeybindingsMerge - No Conflicts', () => {
 			{ key: 'alt+d', command: 'b' },
 			{ key: 'cmd+d', command: 'a' },
 		]);
-		const expected = stringify([
-			{ key: 'shift+c', command: 'c' },
-			{ key: 'alt+c', command: 'a', when: 'editorTextFocus && !editorReadonly' },
-			{ key: 'cmd+d', command: 'a' },
-			{ key: 'alt+d', command: 'b' },
-		]);
 		const actual = await testObject.merge(localContent, remoteContent, localContent);
 		assert.ok(actual.hasChanges);
 		assert.ok(!actual.hasConflicts);
-		assert.equal(actual.mergeContent, expected);
+		assert.equal(actual.mergeContent, remoteContent);
 	});
 
 	test('merge when remote has moved forwareded with multiple changes and local stays with base', async () => {
@@ -238,19 +258,10 @@ suite('KeybindingsMerge - No Conflicts', () => {
 			{ key: 'cmd+d', command: 'c', when: 'context1' },
 			{ key: 'cmd+c', command: '-c' },
 		]);
-		const expected = stringify([
-			{ key: 'alt+d', command: 'a', when: 'editorTextFocus && !editorReadonly' },
-			{ key: 'alt+d', command: '-a' },
-			{ key: 'cmd+e', command: 'd' },
-			{ key: 'cmd+d', command: 'c', when: 'context1' },
-			{ key: 'cmd+c', command: '-c' },
-			{ key: 'alt+f', command: 'f' },
-			{ key: 'alt+d', command: '-f' },
-		]);
 		const actual = await testObject.merge(localContent, remoteContent, localContent);
 		assert.ok(actual.hasChanges);
 		assert.ok(!actual.hasConflicts);
-		assert.equal(actual.mergeContent, expected);
+		assert.equal(actual.mergeContent, remoteContent);
 	});
 
 	test('merge when a new entry is added to local', async () => {
@@ -397,47 +408,6 @@ suite('KeybindingsMerge - No Conflicts', () => {
 		assert.equal(actual.mergeContent, expected);
 	});
 
-	test('merge when local and remote has moved forwareded with no conflicts', async () => {
-		const baseContent = stringify([
-			{ key: 'alt+d', command: 'a', when: 'editorTextFocus && !editorReadonly' },
-			{ key: 'alt+c', command: '-a' },
-			{ key: 'cmd+e', command: 'd' },
-			{ key: 'alt+a', command: 'f' },
-			{ key: 'alt+d', command: '-f' },
-			{ key: 'cmd+d', command: 'c', when: 'context1' },
-			{ key: 'cmd+c', command: '-c' },
-		]);
-		const localContent = stringify([
-			{ key: 'alt+d', command: '-f' },
-			{ key: 'cmd+e', command: 'd' },
-			{ key: 'cmd+c', command: '-c' },
-			{ key: 'cmd+d', command: 'c', when: 'context1' },
-			{ key: 'alt+a', command: 'f' },
-			{ key: 'alt+e', command: 'e' },
-		]);
-		const remoteContent = stringify([
-			{ key: 'alt+a', command: 'f' },
-			{ key: 'cmd+c', command: '-c' },
-			{ key: 'cmd+d', command: 'd' },
-			{ key: 'alt+d', command: '-f' },
-			{ key: 'alt+c', command: 'c', when: 'context1' },
-			{ key: 'alt+g', command: 'g', when: 'context2' },
-		]);
-		const expected = stringify([
-			{ key: 'alt+d', command: '-f' },
-			{ key: 'cmd+d', command: 'd' },
-			{ key: 'cmd+c', command: '-c' },
-			{ key: 'alt+c', command: 'c', when: 'context1' },
-			{ key: 'alt+a', command: 'f' },
-			{ key: 'alt+e', command: 'e' },
-			{ key: 'alt+g', command: 'g', when: 'context2' },
-		]);
-		const actual = await testObject.merge(localContent, remoteContent, baseContent);
-		assert.ok(actual.hasChanges);
-		assert.ok(!actual.hasConflicts);
-		assert.equal(actual.mergeContent, expected);
-	});
-
 });
 
 suite('KeybindingsMerge - Conflicts', () => {
@@ -449,21 +419,23 @@ suite('KeybindingsMerge - Conflicts', () => {
 		assert.ok(actual.hasChanges);
 		assert.ok(actual.hasConflicts);
 		assert.equal(actual.mergeContent,
-			`[
-<<<<<<< local
+			`<<<<<<< local
+[
 	{
 		"key": "alt+d",
 		"command": "a",
 		"when": "editorTextFocus && !editorReadonly"
-	},
+	}
+]
 =======
+[
 	{
 		"key": "alt+c",
 		"command": "a",
 		"when": "editorTextFocus && !editorReadonly"
 	}
->>>>>>> remote
-]`);
+]
+>>>>>>> remote`);
 	});
 
 	test('merge when local and remote with different keybinding', async () => {
@@ -479,8 +451,8 @@ suite('KeybindingsMerge - Conflicts', () => {
 		assert.ok(actual.hasChanges);
 		assert.ok(actual.hasConflicts);
 		assert.equal(actual.mergeContent,
-			`[
-<<<<<<< local
+			`<<<<<<< local
+[
 	{
 		"key": "alt+d",
 		"command": "a",
@@ -490,8 +462,10 @@ suite('KeybindingsMerge - Conflicts', () => {
 		"key": "alt+a",
 		"command": "-a",
 		"when": "editorTextFocus && !editorReadonly"
-	},
+	}
+]
 =======
+[
 	{
 		"key": "alt+c",
 		"command": "a",
@@ -502,48 +476,8 @@ suite('KeybindingsMerge - Conflicts', () => {
 		"command": "-a",
 		"when": "editorTextFocus && !editorReadonly"
 	}
->>>>>>> remote
-]`);
-	});
-
-	test('merge when local and remote has entries in different order', async () => {
-		const localContent = stringify([
-			{ key: 'alt+d', command: 'a', when: 'editorTextFocus && !editorReadonly' },
-			{ key: 'alt+a', command: 'a', when: 'editorTextFocus' }
-		]);
-		const remoteContent = stringify([
-			{ key: 'alt+a', command: 'a', when: 'editorTextFocus' },
-			{ key: 'alt+d', command: 'a', when: 'editorTextFocus && !editorReadonly' }
-		]);
-		const actual = await testObject.merge(localContent, remoteContent, null);
-		assert.ok(actual.hasChanges);
-		assert.ok(actual.hasConflicts);
-		assert.equal(actual.mergeContent,
-			`[
-<<<<<<< local
-	{
-		"key": "alt+d",
-		"command": "a",
-		"when": "editorTextFocus && !editorReadonly"
-	},
-	{
-		"key": "alt+a",
-		"command": "a",
-		"when": "editorTextFocus"
-	},
-=======
-	{
-		"key": "alt+a",
-		"command": "a",
-		"when": "editorTextFocus"
-	},
-	{
-		"key": "alt+d",
-		"command": "a",
-		"when": "editorTextFocus && !editorReadonly"
-	}
->>>>>>> remote
-]`);
+]
+>>>>>>> remote`);
 	});
 
 	test('merge when the entry is removed in local but updated in remote', async () => {
@@ -554,16 +488,17 @@ suite('KeybindingsMerge - Conflicts', () => {
 		assert.ok(actual.hasChanges);
 		assert.ok(actual.hasConflicts);
 		assert.equal(actual.mergeContent,
-			`[
-<<<<<<< local
+			`<<<<<<< local
+[]
 =======
+[
 	{
 		"key": "alt+c",
 		"command": "a",
 		"when": "editorTextFocus && !editorReadonly"
 	}
->>>>>>> remote
-]`);
+]
+>>>>>>> remote`);
 	});
 
 	test('merge when the entry is removed in local but updated in remote and a new entry is added in local', async () => {
@@ -574,20 +509,22 @@ suite('KeybindingsMerge - Conflicts', () => {
 		assert.ok(actual.hasChanges);
 		assert.ok(actual.hasConflicts);
 		assert.equal(actual.mergeContent,
-			`[
+			`<<<<<<< local
+[
 	{
 		"key": "alt+b",
 		"command": "b"
-	},
-<<<<<<< local
+	}
+]
 =======
+[
 	{
 		"key": "alt+c",
 		"command": "a",
 		"when": "editorTextFocus && !editorReadonly"
 	}
->>>>>>> remote
-]`);
+]
+>>>>>>> remote`);
 	});
 
 	test('merge when the entry is removed in remote but updated in local', async () => {
@@ -598,16 +535,17 @@ suite('KeybindingsMerge - Conflicts', () => {
 		assert.ok(actual.hasChanges);
 		assert.ok(actual.hasConflicts);
 		assert.equal(actual.mergeContent,
-			`[
-<<<<<<< local
+			`<<<<<<< local
+[
 	{
 		"key": "alt+c",
 		"command": "a",
 		"when": "editorTextFocus && !editorReadonly"
 	}
+]
 =======
->>>>>>> remote
-]`);
+[]
+>>>>>>> remote`);
 	});
 
 	test('merge when the entry is removed in remote but updated in local and a new entry is added in remote', async () => {
@@ -618,20 +556,121 @@ suite('KeybindingsMerge - Conflicts', () => {
 		assert.ok(actual.hasChanges);
 		assert.ok(actual.hasConflicts);
 		assert.equal(actual.mergeContent,
-			`[
-<<<<<<< local
+			`<<<<<<< local
+[
 	{
 		"key": "alt+c",
 		"command": "a",
 		"when": "editorTextFocus && !editorReadonly"
 	},
+	{
+		"key": "alt+b",
+		"command": "b"
+	}
+]
 =======
->>>>>>> remote
+[
 	{
 		"key": "alt+b",
 		"command": "b"
 	}
-]`);
+]
+>>>>>>> remote`);
+	});
+
+	test('merge when local and remote has moved forwareded with conflicts', async () => {
+		const baseContent = stringify([
+			{ key: 'alt+d', command: 'a', when: 'editorTextFocus && !editorReadonly' },
+			{ key: 'alt+c', command: '-a' },
+			{ key: 'cmd+e', command: 'd' },
+			{ key: 'alt+a', command: 'f' },
+			{ key: 'alt+d', command: '-f' },
+			{ key: 'cmd+d', command: 'c', when: 'context1' },
+			{ key: 'cmd+c', command: '-c' },
+		]);
+		const localContent = stringify([
+			{ key: 'alt+d', command: '-f' },
+			{ key: 'cmd+e', command: 'd' },
+			{ key: 'cmd+c', command: '-c' },
+			{ key: 'cmd+d', command: 'c', when: 'context1' },
+			{ key: 'alt+a', command: 'f' },
+			{ key: 'alt+e', command: 'e' },
+		]);
+		const remoteContent = stringify([
+			{ key: 'alt+a', command: 'f' },
+			{ key: 'cmd+c', command: '-c' },
+			{ key: 'cmd+d', command: 'd' },
+			{ key: 'alt+d', command: '-f' },
+			{ key: 'alt+c', command: 'c', when: 'context1' },
+			{ key: 'alt+g', command: 'g', when: 'context2' },
+		]);
+		const actual = await testObject.merge(localContent, remoteContent, baseContent);
+		assert.ok(actual.hasChanges);
+		assert.ok(actual.hasConflicts);
+		assert.equal(actual.mergeContent,
+			`<<<<<<< local
+[
+	{
+		"key": "alt+d",
+		"command": "-f"
+	},
+	{
+		"key": "cmd+d",
+		"command": "d"
+	},
+	{
+		"key": "cmd+c",
+		"command": "-c"
+	},
+	{
+		"key": "cmd+d",
+		"command": "c",
+		"when": "context1"
+	},
+	{
+		"key": "alt+a",
+		"command": "f"
+	},
+	{
+		"key": "alt+e",
+		"command": "e"
+	},
+	{
+		"key": "alt+g",
+		"command": "g",
+		"when": "context2"
+	}
+]
+=======
+[
+	{
+		"key": "alt+a",
+		"command": "f"
+	},
+	{
+		"key": "cmd+c",
+		"command": "-c"
+	},
+	{
+		"key": "cmd+d",
+		"command": "d"
+	},
+	{
+		"key": "alt+d",
+		"command": "-f"
+	},
+	{
+		"key": "alt+c",
+		"command": "c",
+		"when": "context1"
+	},
+	{
+		"key": "alt+g",
+		"command": "g",
+		"when": "context2"
+	}
+]
+>>>>>>> remote`);
 	});
 
 });