From 71882d3c309098c7fbad36d7642652fd44dd97b1 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Fri, 18 Feb 2022 14:49:27 +0100 Subject: [PATCH] Fixes #143183: Improve rendering of overview ruler entries for deleted or modified chunks in the inline diff editor with word wrapping enabled --- .../editor/browser/widget/diffEditorWidget.ts | 50 ++++++++++++++----- .../common/viewModel/overviewZoneManager.ts | 22 ++++++-- .../common/view/overviewZoneManager.test.ts | 24 ++++----- 3 files changed, 68 insertions(+), 28 deletions(-) diff --git a/src/vs/editor/browser/widget/diffEditorWidget.ts b/src/vs/editor/browser/widget/diffEditorWidget.ts index 20a1cde4569..bb95c0f9798 100644 --- a/src/vs/editor/browser/widget/diffEditorWidget.ts +++ b/src/vs/editor/browser/widget/diffEditorWidget.ts @@ -1388,8 +1388,8 @@ abstract class DiffEditorWidgetStyle extends Disposable { const zones = this._getViewZones(lineChanges, originalWhitespaces, modifiedWhitespaces, renderIndicators); // Get decorations & overview ruler zones - const originalDecorations = this._getOriginalEditorDecorations(lineChanges, ignoreTrimWhitespace, renderIndicators); - const modifiedDecorations = this._getModifiedEditorDecorations(lineChanges, ignoreTrimWhitespace, renderIndicators); + const originalDecorations = this._getOriginalEditorDecorations(zones, lineChanges, ignoreTrimWhitespace, renderIndicators); + const modifiedDecorations = this._getModifiedEditorDecorations(zones, lineChanges, ignoreTrimWhitespace, renderIndicators); return { original: { @@ -1406,8 +1406,8 @@ abstract class DiffEditorWidgetStyle extends Disposable { } protected abstract _getViewZones(lineChanges: ILineChange[], originalForeignVZ: IEditorWhitespace[], modifiedForeignVZ: IEditorWhitespace[], renderIndicators: boolean): IEditorsZones; - protected abstract _getOriginalEditorDecorations(lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations; - protected abstract _getModifiedEditorDecorations(lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations; + protected abstract _getOriginalEditorDecorations(zones: IEditorsZones, lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations; + protected abstract _getModifiedEditorDecorations(zones: IEditorsZones, lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations; public abstract setEnableSplitViewResizing(enableSplitViewResizing: boolean): void; public abstract layout(): number; @@ -1547,7 +1547,8 @@ abstract class ViewZonesComputer { count = lineChange.modifiedStartLineNumber - lastModifiedLineNumber; } } else { - count = originalModel.getLineCount() - lastOriginalLineNumber; + // `lastOriginalLineNumber` has not been looked at yet + count = originalModel.getLineCount() - lastOriginalLineNumber + 1; } for (let i = 0; i < count; i++) { @@ -1886,7 +1887,7 @@ class DiffEditorWidgetSideBySide extends DiffEditorWidgetStyle implements IVerti return c.getViewZones(); } - protected _getOriginalEditorDecorations(lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations { + protected _getOriginalEditorDecorations(zones: IEditorsZones, lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations { const originalEditor = this._dataSource.getOriginalEditor(); const overviewZoneColor = String(this._removeColor); @@ -1910,7 +1911,7 @@ class DiffEditorWidgetSideBySide extends DiffEditorWidgetStyle implements IVerti } const viewRange = getViewRange(originalModel, originalViewModel, lineChange.originalStartLineNumber, lineChange.originalEndLineNumber); - result.overviewZones.push(new OverviewRulerZone(viewRange.startLineNumber, viewRange.endLineNumber, overviewZoneColor)); + result.overviewZones.push(new OverviewRulerZone(viewRange.startLineNumber, viewRange.endLineNumber, /*use endLineNumber*/0, overviewZoneColor)); if (lineChange.charChanges) { for (const charChange of lineChange.charChanges) { @@ -1943,7 +1944,7 @@ class DiffEditorWidgetSideBySide extends DiffEditorWidgetStyle implements IVerti return result; } - protected _getModifiedEditorDecorations(lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations { + protected _getModifiedEditorDecorations(zones: IEditorsZones, lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations { const modifiedEditor = this._dataSource.getModifiedEditor(); const overviewZoneColor = String(this._insertColor); @@ -1968,7 +1969,7 @@ class DiffEditorWidgetSideBySide extends DiffEditorWidgetStyle implements IVerti } const viewRange = getViewRange(modifiedModel, modifiedViewModel, lineChange.modifiedStartLineNumber, lineChange.modifiedEndLineNumber); - result.overviewZones.push(new OverviewRulerZone(viewRange.startLineNumber, viewRange.endLineNumber, overviewZoneColor)); + result.overviewZones.push(new OverviewRulerZone(viewRange.startLineNumber, viewRange.endLineNumber,/*use endLineNumber*/0, overviewZoneColor)); if (lineChange.charChanges) { for (const charChange of lineChange.charChanges) { @@ -2069,7 +2070,7 @@ class DiffEditorWidgetInline extends DiffEditorWidgetStyle { return computer.getViewZones(); } - protected _getOriginalEditorDecorations(lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations { + protected _getOriginalEditorDecorations(zones: IEditorsZones, lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations { const overviewZoneColor = String(this._removeColor); const result: IEditorDiffDecorations = { @@ -2080,6 +2081,7 @@ class DiffEditorWidgetInline extends DiffEditorWidgetStyle { const originalEditor = this._dataSource.getOriginalEditor(); const originalModel = originalEditor.getModel()!; const originalViewModel = originalEditor._getViewModel()!; + let zoneIndex = 0; for (const lineChange of lineChanges) { @@ -2090,15 +2092,37 @@ class DiffEditorWidgetInline extends DiffEditorWidgetStyle { options: DECORATIONS.lineDeleteMargin }); + while (zoneIndex < zones.modified.length) { + const zone = zones.modified[zoneIndex]; + if (zone.diff && zone.diff.originalStartLineNumber >= lineChange.originalStartLineNumber) { + break; + } + zoneIndex++; + } + + let zoneHeightInLines = 0; + if (zoneIndex < zones.modified.length) { + const zone = zones.modified[zoneIndex]; + if ( + zone.diff + && zone.diff.originalStartLineNumber === lineChange.originalStartLineNumber + && zone.diff.originalEndLineNumber === lineChange.originalEndLineNumber + && zone.diff.modifiedStartLineNumber === lineChange.modifiedStartLineNumber + && zone.diff.modifiedEndLineNumber === lineChange.modifiedEndLineNumber + ) { + zoneHeightInLines = zone.heightInLines; + } + } + const viewRange = getViewRange(originalModel, originalViewModel, lineChange.originalStartLineNumber, lineChange.originalEndLineNumber); - result.overviewZones.push(new OverviewRulerZone(viewRange.startLineNumber, viewRange.endLineNumber, overviewZoneColor)); + result.overviewZones.push(new OverviewRulerZone(viewRange.startLineNumber, viewRange.endLineNumber, zoneHeightInLines, overviewZoneColor)); } } return result; } - protected _getModifiedEditorDecorations(lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations { + protected _getModifiedEditorDecorations(zones: IEditorsZones, lineChanges: ILineChange[], ignoreTrimWhitespace: boolean, renderIndicators: boolean): IEditorDiffDecorations { const modifiedEditor = this._dataSource.getModifiedEditor(); const overviewZoneColor = String(this._insertColor); @@ -2120,7 +2144,7 @@ class DiffEditorWidgetInline extends DiffEditorWidgetStyle { }); const viewRange = getViewRange(modifiedModel, modifiedViewModel, lineChange.modifiedStartLineNumber, lineChange.modifiedEndLineNumber); - result.overviewZones.push(new OverviewRulerZone(viewRange.startLineNumber, viewRange.endLineNumber, overviewZoneColor)); + result.overviewZones.push(new OverviewRulerZone(viewRange.startLineNumber, viewRange.endLineNumber, /*use endLineNumber*/0, overviewZoneColor)); if (lineChange.charChanges) { for (const charChange of lineChange.charChanges) { diff --git a/src/vs/editor/common/viewModel/overviewZoneManager.ts b/src/vs/editor/common/viewModel/overviewZoneManager.ts index 8cf1b7764dc..bf15ab6dcc6 100644 --- a/src/vs/editor/common/viewModel/overviewZoneManager.ts +++ b/src/vs/editor/common/viewModel/overviewZoneManager.ts @@ -39,6 +39,10 @@ export class OverviewRulerZone { public readonly startLineNumber: number; public readonly endLineNumber: number; + /** + * If set to 0, the height in lines will be determined based on `endLineNumber`. + */ + public readonly heightInLines: number; public readonly color: string; private _colorZone: ColorZone | null; @@ -46,10 +50,12 @@ export class OverviewRulerZone { constructor( startLineNumber: number, endLineNumber: number, + heightInLines: number, color: string ) { this.startLineNumber = startLineNumber; this.endLineNumber = endLineNumber; + this.heightInLines = heightInLines; this.color = color; this._colorZone = null; } @@ -57,7 +63,10 @@ export class OverviewRulerZone { public static compare(a: OverviewRulerZone, b: OverviewRulerZone): number { if (a.color === b.color) { if (a.startLineNumber === b.startLineNumber) { - return a.endLineNumber - b.endLineNumber; + if (a.heightInLines === b.heightInLines) { + return a.endLineNumber - b.endLineNumber; + } + return a.heightInLines - b.heightInLines; } return a.startLineNumber - b.startLineNumber; } @@ -193,8 +202,15 @@ export class OverviewZoneManager { } } - const y1 = Math.floor(heightRatio * (this._getVerticalOffsetForLine(zone.startLineNumber))); - const y2 = Math.floor(heightRatio * (this._getVerticalOffsetForLine(zone.endLineNumber) + lineHeight)); + const offset1 = this._getVerticalOffsetForLine(zone.startLineNumber); + const offset2 = ( + zone.heightInLines === 0 + ? this._getVerticalOffsetForLine(zone.endLineNumber) + lineHeight + : offset1 + zone.heightInLines * lineHeight + ); + + const y1 = Math.floor(heightRatio * offset1); + const y2 = Math.floor(heightRatio * offset2); let ycenter = Math.floor((y1 + y2) / 2); let halfHeight = (y2 - ycenter); diff --git a/src/vs/editor/test/common/view/overviewZoneManager.test.ts b/src/vs/editor/test/common/view/overviewZoneManager.test.ts index 751e7651ed1..e9b17c10147 100644 --- a/src/vs/editor/test/common/view/overviewZoneManager.test.ts +++ b/src/vs/editor/test/common/view/overviewZoneManager.test.ts @@ -19,10 +19,10 @@ suite('Editor View - OverviewZoneManager', () => { manager.setPixelRatio(1); manager.setZones([ - new OverviewRulerZone(1, 1, '1'), - new OverviewRulerZone(10, 10, '2'), - new OverviewRulerZone(30, 31, '3'), - new OverviewRulerZone(50, 50, '4'), + new OverviewRulerZone(1, 1, 0, '1'), + new OverviewRulerZone(10, 10, 0, '2'), + new OverviewRulerZone(30, 31, 0, '3'), + new OverviewRulerZone(50, 50, 0, '4'), ]); // one line = 12, but cap is at 6 @@ -45,10 +45,10 @@ suite('Editor View - OverviewZoneManager', () => { manager.setPixelRatio(1); manager.setZones([ - new OverviewRulerZone(1, 1, '1'), - new OverviewRulerZone(10, 10, '2'), - new OverviewRulerZone(30, 31, '3'), - new OverviewRulerZone(50, 50, '4'), + new OverviewRulerZone(1, 1, 0, '1'), + new OverviewRulerZone(10, 10, 0, '2'), + new OverviewRulerZone(30, 31, 0, '3'), + new OverviewRulerZone(50, 50, 0, '4'), ]); // one line = 6, cap is at 6 @@ -71,10 +71,10 @@ suite('Editor View - OverviewZoneManager', () => { manager.setPixelRatio(2); manager.setZones([ - new OverviewRulerZone(1, 1, '1'), - new OverviewRulerZone(10, 10, '2'), - new OverviewRulerZone(30, 31, '3'), - new OverviewRulerZone(50, 50, '4'), + new OverviewRulerZone(1, 1, 0, '1'), + new OverviewRulerZone(10, 10, 0, '2'), + new OverviewRulerZone(30, 31, 0, '3'), + new OverviewRulerZone(50, 50, 0, '4'), ]); // one line = 6, cap is at 12