Skip to content

Commit 18ed234

Browse files
committed
Refactor existing diff for merge conflicts
- Remove MergeDiffModel - Delete PlainTextMergeDiff - Add unit test for base content
1 parent fe36150 commit 18ed234

File tree

5 files changed

+114
-302
lines changed

5 files changed

+114
-302
lines changed

src/commandsAndMenu.tsx

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,8 @@ import { PromiseDelegate } from '@lumino/coreutils';
2121
import { Message } from '@lumino/messaging';
2222
import { ContextMenu, Menu, Panel } from '@lumino/widgets';
2323
import * as React from 'react';
24-
import { DiffModel, MergeDiffModel } from './components/diff/model';
24+
import { DiffModel } from './components/diff/model';
2525
import { createPlainTextDiff } from './components/diff/PlainTextDiff';
26-
import { createPlainTextMergeDiff } from './components/diff/PlainTextMergeDiff';
2726
import { CONTEXT_COMMANDS } from './components/FileList';
2827
import { AUTH_ERROR_MESSAGES, requestAPI } from './git';
2928
import { logger } from './logger';
@@ -435,8 +434,7 @@ export function addCommands(
435434
};
436435

437436
const buildDiffWidget =
438-
getDiffProvider(model.filename) ??
439-
(isText && model.base ? createPlainTextMergeDiff : createPlainTextDiff);
437+
getDiffProvider(model.filename) ?? (isText && createPlainTextDiff);
440438

441439
if (buildDiffWidget) {
442440
const id = `diff-${model.filename}-${model.reference.label}-${model.challenger.label}`;
@@ -651,10 +649,7 @@ export function addCommands(
651649
};
652650

653651
// Create the diff widget
654-
const model =
655-
status === 'unmerged'
656-
? new MergeDiffModel<string>(props)
657-
: new DiffModel<string>(props);
652+
const model = new DiffModel<string>(props);
658653

659654
const widget = await commands.execute(CommandIDs.gitShowDiff, {
660655
model,

src/components/diff/PlainTextDiff.ts

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export class PlainTextDiff extends Widget implements Git.Diff.IDiffWidget {
3030
super({
3131
node: PlainTextDiff.createNode(
3232
model.reference.label,
33+
model.base?.label,
3334
model.challenger.label
3435
)
3536
});
@@ -41,11 +42,13 @@ export class PlainTextDiff extends Widget implements Git.Diff.IDiffWidget {
4142
// Load file content early
4243
Promise.all([
4344
this._model.reference.content(),
44-
this._model.challenger.content()
45+
this._model.challenger.content(),
46+
this._model.base?.content()
4547
])
46-
.then(([reference, challenger]) => {
48+
.then(([reference, challenger, base]) => {
4749
this._reference = reference;
4850
this._challenger = challenger;
51+
this._base = base;
4952

5053
getReady.resolve();
5154
})
@@ -70,7 +73,7 @@ export class PlainTextDiff extends Widget implements Git.Diff.IDiffWidget {
7073
this.ready
7174
.then(() => {
7275
if (this._challenger !== null && this._reference !== null) {
73-
this.createDiffView(this._challenger, this._reference);
76+
this.createDiffView(this._challenger, this._reference, this._base);
7477
}
7578
})
7679
.catch(reason => {
@@ -105,9 +108,16 @@ export class PlainTextDiff extends Widget implements Git.Diff.IDiffWidget {
105108
this._challenger = await this._model.challenger.content();
106109
}
107110

108-
this.createDiffView(this._challenger, this._reference);
111+
// Request base content only if base was provided in the model
112+
this._base = await this._model.base?.content();
113+
114+
this.createDiffView(this._challenger, this._reference, this._base);
109115
this._challenger = null;
110116
this._reference = null;
117+
118+
// Set to null only if base was provided in the model
119+
// else leave as undefined
120+
this._base = !!this._model.base && null;
111121
} catch (reason) {
112122
this.showError(reason);
113123
}
@@ -116,17 +126,15 @@ export class PlainTextDiff extends Widget implements Git.Diff.IDiffWidget {
116126
/**
117127
* Create wrapper node
118128
*/
119-
protected static createNode(
120-
baseLabel: string,
121-
remoteLabel: string
122-
): HTMLElement {
129+
protected static createNode(...labels: string[]): HTMLElement {
123130
const head = document.createElement('div');
124131
head.className = 'jp-git-diff-root';
125132
head.innerHTML = `
126133
<div class="jp-git-diff-banner">
127-
<span>${baseLabel}</span>
128-
<span class="jp-spacer"></span>
129-
<span>${remoteLabel}</span>
134+
${labels
135+
.filter(label => !!label)
136+
.map(label => `<span>${label}</span>`)
137+
.join('<span class="jp-spacer"></span>')}
130138
</div>
131139
<div class="jp-git-PlainText-diff"></div>`;
132140
return head;
@@ -137,19 +145,40 @@ export class PlainTextDiff extends Widget implements Git.Diff.IDiffWidget {
137145
*/
138146
protected async createDiffView(
139147
challengerContent: string,
140-
referenceContent: string
148+
referenceContent: string,
149+
baseContent?: string
141150
): Promise<void> {
142151
if (!this._mergeView) {
152+
// Empty base content ("") can be an edge case.
153+
const isMergeConflict = baseContent !== undefined;
154+
143155
const mode =
144156
Mode.findByFileName(this._model.filename) ||
145157
Mode.findBest(this._model.filename);
146158

147-
this._mergeView = mergeView(this._container, {
159+
let options: Parameters<typeof mergeView>[1] = {
148160
value: challengerContent,
149161
orig: referenceContent,
150162
mode: mode.mime,
151163
...this.getDefaultOptions()
152-
}) as MergeView.MergeViewEditor;
164+
};
165+
166+
// Show three-way diff on merge conflict
167+
if (isMergeConflict) {
168+
options = {
169+
...options,
170+
value: baseContent,
171+
origRight: referenceContent,
172+
origLeft: challengerContent,
173+
readOnly: true,
174+
revertButtons: false
175+
};
176+
}
177+
178+
this._mergeView = mergeView(
179+
this._container,
180+
options
181+
) as MergeView.MergeViewEditor;
153182
}
154183

155184
return Promise.resolve();
@@ -192,4 +221,5 @@ export class PlainTextDiff extends Widget implements Git.Diff.IDiffWidget {
192221

193222
private _reference: string | null = null;
194223
private _challenger: string | null = null;
224+
private _base?: string | null;
195225
}

src/components/diff/PlainTextMergeDiff.ts

Lines changed: 0 additions & 216 deletions
This file was deleted.

0 commit comments

Comments
 (0)