Skip to content

Commit 7ff66b3

Browse files
hediethediet
authored
Diff Editor v2 Bugfixes (microsoft#184797)
1 parent 7e1125f commit 7ff66b3

File tree

2 files changed

+71
-58
lines changed

2 files changed

+71
-58
lines changed

‎src/vs/editor/browser/widget/diffEditorWidget2/diffEditorWidget2.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { CodeEditorWidget, ICodeEditorWidgetOptions } from 'vs/editor/browser/wi
2020
import { IDiffCodeEditorWidgetOptions } from 'vs/editor/browser/widget/diffEditorWidget';
2121
import { diffAddDecoration, diffDeleteDecoration, diffFullLineAddDecoration, diffFullLineDeleteDecoration } from 'vs/editor/browser/widget/diffEditorWidget2/decorations';
2222
import { DiffEditorSash } from 'vs/editor/browser/widget/diffEditorWidget2/diffEditorSash';
23-
import { ViewZoneAlignment } from 'vs/editor/browser/widget/diffEditorWidget2/lineAlignment';
23+
import { ViewZoneManager } from 'vs/editor/browser/widget/diffEditorWidget2/lineAlignment';
2424
import { MovedBlocksLinesPart } from 'vs/editor/browser/widget/diffEditorWidget2/movedBlocksLines';
2525
import { OverviewRulerPart } from 'vs/editor/browser/widget/diffEditorWidget2/overviewRulerPart';
2626
import { UnchangedRangesFeature } from 'vs/editor/browser/widget/diffEditorWidget2/unchangedRanges';
@@ -140,7 +140,7 @@ export class DiffEditorWidget2 extends DelegatingEditor implements IDiffEditor {
140140

141141

142142
this._register(new UnchangedRangesFeature(this._originalEditor, this._modifiedEditor, this._diffModel));
143-
this._register(this._instantiationService.createInstance(ViewZoneAlignment, this._originalEditor, this._modifiedEditor, this._diffModel, this._options.map(o => o.renderSideBySide)));
143+
this._register(this._instantiationService.createInstance(ViewZoneManager, this._originalEditor, this._modifiedEditor, this._diffModel, this._options.map(o => o.renderSideBySide)));
144144

145145
this._register(this._instantiationService.createInstance(OverviewRulerPart,
146146
this._originalEditor,
@@ -442,13 +442,21 @@ export class DiffEditorWidget2 extends DelegatingEditor implements IDiffEditor {
442442
return this._originalEditor.hasTextFocus() || this._modifiedEditor.hasTextFocus();
443443
}
444444

445-
override saveViewState(): IDiffEditorViewState | null {
446-
return null;
447-
//throw new Error('Method not implemented.');
445+
public override saveViewState(): IDiffEditorViewState {
446+
const originalViewState = this._originalEditor.saveViewState();
447+
const modifiedViewState = this._modifiedEditor.saveViewState();
448+
return {
449+
original: originalViewState,
450+
modified: modifiedViewState
451+
};
448452
}
449453

450-
override restoreViewState(state: IDiffEditorViewState | null): void {
451-
//throw new Error('Method not implemented.');
454+
public override restoreViewState(s: IDiffEditorViewState): void {
455+
if (s && s.original && s.modified) {
456+
const diffEditorState = s as IDiffEditorViewState;
457+
this._originalEditor.restoreViewState(diffEditorState.original);
458+
this._modifiedEditor.restoreViewState(diffEditorState.modified);
459+
}
452460
}
453461

454462
override getModel(): IDiffEditorModel | null { return this._model.get(); }

‎src/vs/editor/browser/widget/diffEditorWidget2/lineAlignment.ts

Lines changed: 56 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ import { Disposable, DisposableStore } from 'vs/base/common/lifecycle';
88
import { IObservable, derived, observableFromEvent, observableSignalFromEvent, observableValue } from 'vs/base/common/observable';
99
import { autorun, autorunWithStore2 } from 'vs/base/common/observableImpl/autorun';
1010
import { ThemeIcon } from 'vs/base/common/themables';
11+
import { assertIsDefined } from 'vs/base/common/types';
1112
import { applyFontInfo } from 'vs/editor/browser/config/domFontInfo';
1213
import { IViewZone } from 'vs/editor/browser/editorBrowser';
14+
import { StableEditorScrollState } from 'vs/editor/browser/stableEditorScroll';
1315
import { CodeEditorWidget } from 'vs/editor/browser/widget/codeEditorWidget';
1416
import { diffDeleteDecoration, diffRemoveIcon } from 'vs/editor/browser/widget/diffEditorWidget2/decorations';
1517
import { DiffMapping, DiffModel } from 'vs/editor/browser/widget/diffEditorWidget2/diffModel';
@@ -26,14 +28,18 @@ import { InlineDecoration, InlineDecorationType } from 'vs/editor/common/viewMod
2628
import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService';
2729
import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
2830

29-
export class ViewZoneAlignment extends Disposable {
30-
private readonly _origExtraHeight = observableValue('origExtraHeight', 0);
31-
private readonly _modExtraHeight = observableValue('modExtraHeight', 0);
32-
31+
/**
32+
* Ensures both editors have the same height by aligning unchanged lines.
33+
* In inline view mode, inserts viewzones to show deleted code from the original text model in the modified code editor.
34+
* Synchronizes scrolling.
35+
*/
36+
export class ViewZoneManager extends Disposable {
37+
private readonly _originalTopPadding = observableValue('originalTopPadding', 0);
3338
private readonly _originalScrollTop: IObservable<number>;
3439
private readonly _originalScrollOffset = observableValue<number, boolean>('originalScrollOffset', 0);
3540
private readonly _originalScrollOffsetAnimated = animatedObservable(this._originalScrollOffset, this._store);
3641

42+
private readonly _modifiedTopPadding = observableValue('modifiedTopPadding', 0);
3743
private readonly _modifiedScrollTop: IObservable<number>;
3844
private readonly _modifiedScrollOffset = observableValue<number, boolean>('modifiedScrollOffset', 0);
3945
private readonly _modifiedScrollOffsetAnimated = animatedObservable(this._modifiedScrollOffset, this._store);
@@ -50,11 +56,11 @@ export class ViewZoneAlignment extends Disposable {
5056

5157
let isChangingViewZones = false;
5258

53-
const origViewZonesChanged = observableSignalFromEvent(
59+
const originalViewZonesChanged = observableSignalFromEvent(
5460
'origViewZonesChanged',
5561
e => this._originalEditor.onDidChangeViewZones((args) => { if (!isChangingViewZones) { e(args); } })
5662
);
57-
const modViewZonesChanged = observableSignalFromEvent(
63+
const modifiedViewZonesChanged = observableSignalFromEvent(
5864
'modViewZonesChanged',
5965
e => this._modifiedEditor.onDidChangeViewZones((args) => { if (!isChangingViewZones) { e(args); } })
6066
);
@@ -70,24 +76,18 @@ export class ViewZoneAlignment extends Disposable {
7076
const diffModel = this._diffModel.read(reader);
7177
const diff = diffModel?.diff.read(reader);
7278
if (!diffModel || !diff) { return null; }
73-
74-
origViewZonesChanged.read(reader);
75-
modViewZonesChanged.read(reader);
76-
79+
originalViewZonesChanged.read(reader);
80+
modifiedViewZonesChanged.read(reader);
7781
return computeRangeAlignment(this._originalEditor, this._modifiedEditor, diff.mappings, alignmentViewZoneIdsOrig, alignmentViewZoneIdsMod);
7882
});
7983

8084
const alignmentsSyncedMovedText = derived<ILineRangeAlignment[] | null>('alignments', (reader) => {
81-
origViewZonesChanged.read(reader);
82-
modViewZonesChanged.read(reader);
83-
8485
const syncedMovedText = this._diffModel.read(reader)?.syncedMovedTexts.read(reader);
85-
if (!syncedMovedText) {
86-
return null;
87-
}
86+
if (!syncedMovedText) { return null; }
87+
originalViewZonesChanged.read(reader);
88+
modifiedViewZonesChanged.read(reader);
8889
const mappings = syncedMovedText.changes.map(c => new DiffMapping(c));
89-
90-
// TOD dont include alignments outside syncedMovedText
90+
// TODO dont include alignments outside syncedMovedText
9191
return computeRangeAlignment(this._originalEditor, this._modifiedEditor, mappings, alignmentViewZoneIdsOrig, alignmentViewZoneIdsMod);
9292
});
9393

@@ -101,33 +101,33 @@ export class ViewZoneAlignment extends Disposable {
101101
const alignmentViewZones = derived<{ orig: IViewZoneWithZoneId[]; mod: IViewZoneWithZoneId[] }>('alignment viewzones', (reader) => {
102102
alignmentViewZonesDisposables.clear();
103103

104-
const curAlignments = alignments.read(reader) || [];
104+
const alignmentsVal = alignments.read(reader) || [];
105105

106106
const origViewZones: IViewZoneWithZoneId[] = [];
107107
const modViewZones: IViewZoneWithZoneId[] = [];
108108

109-
const curModExtraHeight = this._modExtraHeight.read(reader);
110-
if (curModExtraHeight > 0) {
109+
const modifiedTopPaddingVal = this._modifiedTopPadding.read(reader);
110+
if (modifiedTopPaddingVal > 0) {
111111
modViewZones.push({
112112
afterLineNumber: 0,
113113
domNode: document.createElement('div'),
114-
heightInPx: curModExtraHeight,
114+
heightInPx: modifiedTopPaddingVal,
115115
});
116116
}
117-
const curOrigExtraHeight = this._origExtraHeight.read(reader);
118-
if (curOrigExtraHeight > 0) {
117+
const originalTopPaddingVal = this._originalTopPadding.read(reader);
118+
if (originalTopPaddingVal > 0) {
119119
origViewZones.push({
120120
afterLineNumber: 0,
121121
domNode: document.createElement('div'),
122-
heightInPx: curOrigExtraHeight,
122+
heightInPx: originalTopPaddingVal,
123123
});
124124
}
125125

126126
const renderSideBySide = this._renderSideBySide.read(reader);
127127

128128
const deletedCodeLineBreaksComputer = !renderSideBySide ? this._modifiedEditor._getViewModel()?.createLineBreaksComputer() : undefined;
129129
if (deletedCodeLineBreaksComputer) {
130-
for (const a of curAlignments) {
130+
for (const a of alignmentsVal) {
131131
if (a.diff) {
132132
for (let i = a.originalRange.startLineNumber; i < a.originalRange.endLineNumberExclusive; i++) {
133133
deletedCodeLineBreaksComputer?.addRequest(this._originalEditor.getModel()!.getLineContent(i), null, null);
@@ -147,13 +147,13 @@ export class ViewZoneAlignment extends Disposable {
147147
const mightContainRTL = this._originalEditor.getModel()?.mightContainRTL() ?? false;
148148
const renderOptions = RenderOptions.fromEditor(this._modifiedEditor);
149149

150-
for (const a of curAlignments) {
150+
for (const a of alignmentsVal) {
151151
if (a.diff && !renderSideBySide) {
152152
if (!a.originalRange.isEmpty) {
153153
originalModelTokenizationCompleted.read(reader); // Update view-zones once tokenization completes
154154

155-
const domNode = document.createElement('div');
156-
domNode.classList.add('view-lines', 'line-delete', 'monaco-mouse-cursor-text');
155+
const deletedCodeDomNode = document.createElement('div');
156+
deletedCodeDomNode.classList.add('view-lines', 'line-delete', 'monaco-mouse-cursor-text');
157157
const source = new LineSource(
158158
a.originalRange.mapToLineArray(l => this._originalEditor.getModel()!.tokenization.getLineTokens(l)),
159159
a.originalRange.mapToLineArray(_ => lineBreakData[lineBreakDataIdx++]),
@@ -168,7 +168,7 @@ export class ViewZoneAlignment extends Disposable {
168168
InlineDecorationType.Regular
169169
));
170170
}
171-
const result = renderLines(source, renderOptions, decorations, domNode);
171+
const result = renderLines(source, renderOptions, decorations, deletedCodeDomNode);
172172

173173
const marginDomNode = document.createElement('div');
174174
marginDomNode.className = 'inline-deleted-margin-view-zone';
@@ -183,18 +183,10 @@ export class ViewZoneAlignment extends Disposable {
183183
}
184184
//}
185185

186-
let zoneId: string;
187-
modViewZones.push({
188-
afterLineNumber: a.modifiedRange.startLineNumber - 1,
189-
domNode: domNode,
190-
heightInPx: result.heightInLines * modLineHeight,
191-
minWidthInPx: result.minWidthInPx,
192-
marginDomNode,
193-
setZoneId(id) { zoneId = id; },
194-
});
186+
let zoneId: string | undefined = undefined;
195187
alignmentViewZonesDisposables.add(
196188
new InlineDiffDeletedCodeMargin(
197-
() => zoneId,
189+
() => assertIsDefined(zoneId),
198190
marginDomNode,
199191
this._modifiedEditor,
200192
a.diff,
@@ -207,7 +199,7 @@ export class ViewZoneAlignment extends Disposable {
207199

208200
for (let i = 0; i < result.viewLineCounts.length; i++) {
209201
const count = result.viewLineCounts[i];
210-
// Account for wrapped lines in the (collapsed) original editor (that does not have line wraps).
202+
// Account for wrapped lines in the (collapsed) original editor (which doesn't wrap lines).
211203
if (count > 1) {
212204
origViewZones.push({
213205
afterLineNumber: a.originalRange.startLineNumber + i,
@@ -216,6 +208,15 @@ export class ViewZoneAlignment extends Disposable {
216208
});
217209
}
218210
}
211+
212+
modViewZones.push({
213+
afterLineNumber: a.modifiedRange.startLineNumber - 1,
214+
domNode: deletedCodeDomNode,
215+
heightInPx: result.heightInLines * modLineHeight,
216+
minWidthInPx: result.minWidthInPx,
217+
marginDomNode,
218+
setZoneId(id) { zoneId = id; },
219+
});
219220
}
220221

221222
const marginDomNode = document.createElement('div');
@@ -280,6 +281,8 @@ export class ViewZoneAlignment extends Disposable {
280281
});
281282

282283
this._register(autorunWithStore2('alignment viewzones', (reader) => {
284+
const scrollState = StableEditorScrollState.capture(this._modifiedEditor);
285+
283286
const alignmentViewZones_ = alignmentViewZones.read(reader);
284287
isChangingViewZones = true;
285288
this._originalEditor.changeViewZones((aOrig) => {
@@ -305,6 +308,8 @@ export class ViewZoneAlignment extends Disposable {
305308
}
306309
});
307310
isChangingViewZones = false;
311+
312+
scrollState.restore(this._modifiedEditor);
308313
}));
309314

310315
this._originalScrollTop = observableFromEvent(this._originalEditor.onDidScrollChange, () => this._originalEditor.getScrollTop());
@@ -321,7 +326,7 @@ export class ViewZoneAlignment extends Disposable {
321326
this._register(autorun('update scroll modified', (reader) => {
322327
const newScrollTopModified = this._originalScrollTop.read(reader)
323328
- (this._originalScrollOffsetAnimated.get() - this._modifiedScrollOffsetAnimated.read(reader))
324-
- (this._origExtraHeight.get() - this._modExtraHeight.read(reader));
329+
- (this._originalTopPadding.get() - this._modifiedTopPadding.read(reader));
325330
if (newScrollTopModified !== this._modifiedEditor.getScrollTop()) {
326331
this._modifiedEditor.setScrollTop(newScrollTopModified, ScrollType.Immediate);
327332
}
@@ -330,7 +335,7 @@ export class ViewZoneAlignment extends Disposable {
330335
this._register(autorun('update scroll original', (reader) => {
331336
const newScrollTopOriginal = this._modifiedScrollTop.read(reader)
332337
- (this._modifiedScrollOffsetAnimated.get() - this._originalScrollOffsetAnimated.read(reader))
333-
- (this._modExtraHeight.get() - this._origExtraHeight.read(reader));
338+
- (this._modifiedTopPadding.get() - this._originalTopPadding.read(reader));
334339
if (newScrollTopOriginal !== this._originalEditor.getScrollTop()) {
335340
this._originalEditor.setScrollTop(newScrollTopOriginal, ScrollType.Immediate);
336341
}
@@ -342,21 +347,21 @@ export class ViewZoneAlignment extends Disposable {
342347

343348
let deltaOrigToMod = 0;
344349
if (m) {
345-
const trueTopOriginal = this._originalEditor.getTopForLineNumber(m.lineRangeMapping.originalRange.startLineNumber, true) - this._origExtraHeight.get();
346-
const trueTopModified = this._modifiedEditor.getTopForLineNumber(m.lineRangeMapping.modifiedRange.startLineNumber, true) - this._modExtraHeight.get();
350+
const trueTopOriginal = this._originalEditor.getTopForLineNumber(m.lineRangeMapping.originalRange.startLineNumber, true) - this._originalTopPadding.get();
351+
const trueTopModified = this._modifiedEditor.getTopForLineNumber(m.lineRangeMapping.modifiedRange.startLineNumber, true) - this._modifiedTopPadding.get();
347352
deltaOrigToMod = trueTopModified - trueTopOriginal;
348353
}
349354

350355
if (deltaOrigToMod > 0) {
351-
this._modExtraHeight.set(0, undefined);
352-
this._origExtraHeight.set(deltaOrigToMod, undefined);
356+
this._modifiedTopPadding.set(0, undefined);
357+
this._originalTopPadding.set(deltaOrigToMod, undefined);
353358
} else if (deltaOrigToMod < 0) {
354-
this._modExtraHeight.set(-deltaOrigToMod, undefined);
355-
this._origExtraHeight.set(0, undefined);
359+
this._modifiedTopPadding.set(-deltaOrigToMod, undefined);
360+
this._originalTopPadding.set(0, undefined);
356361
} else {
357362
setTimeout(() => {
358-
this._modExtraHeight.set(0, undefined);
359-
this._origExtraHeight.set(0, undefined);
363+
this._modifiedTopPadding.set(0, undefined);
364+
this._originalTopPadding.set(0, undefined);
360365
}, 400);
361366
}
362367

0 commit comments

Comments
 (0)