From 732347151005d40837c0c3d1f5ad57fde739fa76 Mon Sep 17 00:00:00 2001 From: Tim Hutt Date: Wed, 18 Dec 2024 15:52:29 +0000 Subject: [PATCH] Add shortcuts to compare incoming/current with common ancestor Git has a diff style `diff3` or `zdiff3` that shows the common ancestor of two conflicting commits in the conflict. For example: ``` <<<<<<< HEAD int c = 2; ||||||| parent of ed293d1 (Capitalise) int c; ======= int C; >>>>>>> ed293d1 (Capitalise) ``` This is enormously useful when resolving conflicts. However sometimes git is dumb and you end up with conflicts that are difficult to follow. For example suppose you have some `really long old code`, and you write some `new code` to replace it. You rebase your change, and find that someone has made a tiny tiny change *somewhere* in the really long old code. It *might* be relevant - it could be some change that you also need to apply to your new code. But the conflict just looks like this: ``` <<<<<<< HEAD really long old code ||||||| parent of ed293d1 (Capitalise) really Long old code ======= new code >>>>>>> ed293d1 (Capitalise) ``` Not too bad with 4 lines but sometimes it's hundreds, and then you end up visually diffing it, like a cave man. There is an existing `Compare Changes` option, but it only compares `Current` with `Incoming`. This commit adds two more options to compare the common ancestor with `Current` or `Incoming` instead, thus solving the problem once and for all. This was mostly straightforward. The two tricky bits are: 1. Apparently you can have more than one common ancestor. I've never seen that so I just don't show the buttons in that case (or pick the first common ancestor if you use the command palette). 2. What if you don't have `diff3` and still run the new compare commands from the command palette? In that case it falls back to not showing any changes. --- extensions/merge-conflict/package.json | 14 ++++ extensions/merge-conflict/package.nls.json | 2 + .../merge-conflict/src/codelensProvider.ts | 33 +++++++-- .../merge-conflict/src/commandHandler.ts | 70 ++++++++++++++++--- 4 files changed, 105 insertions(+), 14 deletions(-) diff --git a/extensions/merge-conflict/package.json b/extensions/merge-conflict/package.json index de56c9c22cfec..176a193821dc3 100644 --- a/extensions/merge-conflict/package.json +++ b/extensions/merge-conflict/package.json @@ -101,6 +101,20 @@ "original": "Compare Current Conflict", "command": "merge-conflict.compare", "enablement": "!isMergeEditor" + }, + { + "category": "%command.category%", + "title": "%command.compare-ancestor-current%", + "original": "Compare Current Conflict (common ancestor ↔ current)", + "command": "merge-conflict.compare-ancestor-current", + "enablement": "!isMergeEditor" + }, + { + "category": "%command.category%", + "title": "%command.compare-ancestor-incoming%", + "original": "Compare Current Conflict (common ancestor ↔ incoming)", + "command": "merge-conflict.compare-ancestor-incoming", + "enablement": "!isMergeEditor" } ], "menus": { diff --git a/extensions/merge-conflict/package.nls.json b/extensions/merge-conflict/package.nls.json index 3353f40b31649..7ba3c2b58fadd 100644 --- a/extensions/merge-conflict/package.nls.json +++ b/extensions/merge-conflict/package.nls.json @@ -12,6 +12,8 @@ "command.next": "Next Conflict", "command.previous": "Previous Conflict", "command.compare": "Compare Current Conflict", + "command.compare-ancestor-current": "Compare Current Conflict (common ancestor ↔ current)", + "command.compare-ancestor-incoming": "Compare Current Conflict (common ancestor ↔ incoming)", "config.title": "Merge Conflict", "config.autoNavigateNextConflictEnabled": "Whether to automatically navigate to the next merge conflict after resolving a merge conflict.", "config.codeLensEnabled": "Create a CodeLens for merge conflict blocks within editor.", diff --git a/extensions/merge-conflict/src/codelensProvider.ts b/extensions/merge-conflict/src/codelensProvider.ts index 73eeddfb109f2..2a411db95a597 100644 --- a/extensions/merge-conflict/src/codelensProvider.ts +++ b/extensions/merge-conflict/src/codelensProvider.ts @@ -63,25 +63,31 @@ export default class MergeConflictCodeLensProvider implements vscode.CodeLensPro conflicts.forEach(conflict => { const acceptCurrentCommand: vscode.Command = { command: 'merge-conflict.accept.current', - title: vscode.l10n.t("Accept Current Change"), + title: vscode.l10n.t("Accept Current"), arguments: ['known-conflict', conflict] }; const acceptIncomingCommand: vscode.Command = { command: 'merge-conflict.accept.incoming', - title: vscode.l10n.t("Accept Incoming Change"), + title: vscode.l10n.t("Accept Incoming"), arguments: ['known-conflict', conflict] }; const acceptBothCommand: vscode.Command = { command: 'merge-conflict.accept.both', - title: vscode.l10n.t("Accept Both Changes"), + title: vscode.l10n.t("Accept Both"), arguments: ['known-conflict', conflict] }; + // Only allow comparing with the common ancestor if there is a single + // one. This is the common case with diff3/zdiff3. + const hasSingleCommonAncestor = conflict.commonAncestors.length === 1; + const diffCommand: vscode.Command = { command: 'merge-conflict.compare', - title: vscode.l10n.t("Compare Changes"), + title: hasSingleCommonAncestor + ? vscode.l10n.t("Compare Current & Incoming") + : vscode.l10n.t("Compare Changes"), arguments: [conflict] }; @@ -92,6 +98,25 @@ export default class MergeConflictCodeLensProvider implements vscode.CodeLensPro new vscode.CodeLens(range, acceptBothCommand), new vscode.CodeLens(range, diffCommand) ); + + if (hasSingleCommonAncestor) { + const diffBaseCurrentCommand: vscode.Command = { + command: 'merge-conflict.compare-ancestor-current', + title: vscode.l10n.t("Compare Ancestor & Current"), + arguments: [conflict] + }; + + const diffBaseIncomingCommand: vscode.Command = { + command: 'merge-conflict.compare-ancestor-incoming', + title: vscode.l10n.t("Compare Ancestor & Incoming"), + arguments: [conflict] + }; + + items.push( + new vscode.CodeLens(range, diffBaseCurrentCommand), + new vscode.CodeLens(range, diffBaseIncomingCommand), + ); + } }); return items; diff --git a/extensions/merge-conflict/src/commandHandler.ts b/extensions/merge-conflict/src/commandHandler.ts index a2b940b24c601..ada8a7f59c3e4 100644 --- a/extensions/merge-conflict/src/commandHandler.ts +++ b/extensions/merge-conflict/src/commandHandler.ts @@ -36,7 +36,9 @@ export default class CommandHandler implements vscode.Disposable { this.registerTextEditorCommand('merge-conflict.accept.all-both', this.acceptAllBoth), this.registerTextEditorCommand('merge-conflict.next', this.navigateNext), this.registerTextEditorCommand('merge-conflict.previous', this.navigatePrevious), - this.registerTextEditorCommand('merge-conflict.compare', this.compare) + this.registerTextEditorCommand('merge-conflict.compare', this.compareCurrentIncoming), + this.registerTextEditorCommand('merge-conflict.compare-ancestor-current', this.compareAncestorCurrent), + this.registerTextEditorCommand('merge-conflict.compare-ancestor-incoming', this.compareAncestorIncoming), ); } @@ -82,7 +84,14 @@ export default class CommandHandler implements vscode.Disposable { return this.acceptAll(interfaces.CommitType.Both, editor); } - async compare(editor: vscode.TextEditor, conflict: interfaces.IDocumentMergeConflict | null) { + private async compare( + editor: vscode.TextEditor, + conflict: interfaces.IDocumentMergeConflict | null, + leftTitle: string, + leftRegion: (conflict: interfaces.IDocumentMergeConflict) => interfaces.IMergeRegion, + rightTitle: string, + rightRegion: (conflict: interfaces.IDocumentMergeConflict) => interfaces.IMergeRegion, + ) { // No conflict, command executed from command palette if (!conflict) { @@ -104,18 +113,18 @@ export default class CommandHandler implements vscode.Disposable { } const scheme = editor.document.uri.scheme; - let range = conflict.current.content; - const leftRanges = conflicts.map(conflict => [conflict.current.content, conflict.range]); - const rightRanges = conflicts.map(conflict => [conflict.incoming.content, conflict.range]); + const leftRanges = conflicts.map(conflict => [leftRegion(conflict)?.content, conflict.range]); + const rightRanges = conflicts.map(conflict => [rightRegion(conflict)?.content, conflict.range]); const leftUri = editor.document.uri.with({ scheme: ContentProvider.scheme, - query: JSON.stringify({ scheme, range: range, ranges: leftRanges }) + query: JSON.stringify({ scheme, ranges: leftRanges }) }); - - range = conflict.incoming.content; - const rightUri = leftUri.with({ query: JSON.stringify({ scheme, ranges: rightRanges }) }); + const rightUri = editor.document.uri.with({ + scheme: ContentProvider.scheme, + query: JSON.stringify({ scheme, ranges: rightRanges }) + }); let mergeConflictLineOffsets = 0; for (const nextconflict of conflicts) { @@ -132,7 +141,7 @@ export default class CommandHandler implements vscode.Disposable { const docPath = editor.document.uri.path; const fileName = docPath.substring(docPath.lastIndexOf('/') + 1); // avoid NodeJS path to keep browser webpack small - const title = vscode.l10n.t("{0}: Current Changes ↔ Incoming Changes", fileName); + const title = `${fileName}: ${leftTitle} ↔ ${rightTitle}`; const mergeConflictConfig = vscode.workspace.getConfiguration('merge-conflict'); const openToTheSide = mergeConflictConfig.get('diffViewPosition'); const opts: vscode.TextDocumentShowOptions = { @@ -147,6 +156,47 @@ export default class CommandHandler implements vscode.Disposable { await vscode.commands.executeCommand('vscode.diff', leftUri, rightUri, title, opts); } + compareCurrentIncoming(editor: vscode.TextEditor, conflict: interfaces.IDocumentMergeConflict | null) { + return this.compare( + editor, + conflict, + vscode.l10n.t("Current Changes"), + (conflict: interfaces.IDocumentMergeConflict) => conflict.current, + vscode.l10n.t("Incoming Changes"), + (conflict: interfaces.IDocumentMergeConflict) => conflict.incoming, + ); + } + + compareAncestorCurrent(editor: vscode.TextEditor, conflict: interfaces.IDocumentMergeConflict | null) { + return this.compare( + editor, + conflict, + vscode.l10n.t("Common Ancestor"), + // Fall back to not showing any changes if there's no common ancestor. + // This can happen if the conflict file is broken, or they use the + // command palette to run this command on a file without common + // ancestors markers. + (conflict: interfaces.IDocumentMergeConflict) => conflict.commonAncestors[0] || conflict.current, + vscode.l10n.t("Current Changes"), + (conflict: interfaces.IDocumentMergeConflict) => conflict.current, + ); + } + + compareAncestorIncoming(editor: vscode.TextEditor, conflict: interfaces.IDocumentMergeConflict | null) { + return this.compare( + editor, + conflict, + vscode.l10n.t("Common Ancestor"), + // Fall back to not showing any changes if there's no common ancestor. + // This can happen if the conflict file is broken, or they use the + // command palette to run this command on a file without common + // ancestors markers. + (conflict: interfaces.IDocumentMergeConflict) => conflict.commonAncestors[0] || conflict.incoming, + vscode.l10n.t("Incoming Changes"), + (conflict: interfaces.IDocumentMergeConflict) => conflict.incoming, + ); + } + navigateNext(editor: vscode.TextEditor): Promise { return this.navigate(editor, NavigationDirection.Forwards); }