Review usages of `EditorOption.lineHeight`

[aiday-mar]

This iteration it has been made possible to define variable line heights in the editor. More specifically this means now that the EditorOption.lineHeight no longer will truly reflect the line height of a line. This option now defines the default line height which is used when no explicit line height is set. To retrieve the actual line height you may use the editor.getLineHeightForPosition method. As part of this adoption, it would be useful to review the instances of the usage of EditorOption.lineHeight and fontInfo.lineHeight and potentially change these instances to editor.getLineHeightForPosition. Please double-check all the instances of the text lineHeight in the given file. If you feel the assignment is not correct please let me know.

@aiday-mar

• mouseHandler.ts line 714, 728
• mouseTarget.ts line 257
• glyphMargin.ts line 149, 174
• lineNumbers.ts line 51
• minimap.ts line 129
• decorationsOverviewRuler.ts line 51
• overviewRuler.ts line 39, 56, 57
• viewLineOptions.ts line 44
• viewLines.ts line 137, 197
• viewZones.ts line 56, 111, 115
• whitespace.ts line 315
• renderLines.ts line 108
• viewModelImpl.ts line 997
• hoverColorPickerParticipant.ts line 115
• standaloneColorPickerParticipant.ts line 132
• contentHoverWidget.ts line 63, 432, 438
• glyphHoverWidget.ts line 177
• stickyScrollWidget.ts line 445, 489, 523
• stickyScrollWidget.ts line 81, 104, 105
• stickyScroll.test.ts line 177, 230, 329
• exceptionWidget.ts line 73, 113

@hediet

• accessibleDiffViewer.ts line 461
• diffEditorViewZones.ts line 84, 87, 188, 475, 476, 608
• inlineDiffDeletedCodeMargin.ts line 57
• hideUnchangedRegionsFeature.ts line 380, 415
• ghostTextView.ts line 208, 238, 424, 538, 560
• placeholderTextContribution.ts line 67
• conflictActions.ts line 63

@justschen

• lightBulbWidget.ts line 127, 174

@jrieken

• codelensController.ts line 83
• goToErrorWidget.ts line 161, 187
• peekView.ts line 257
• zoneWidget.ts line 364, 382, 395, 545
• chatEditingCodeEditorIntegration.ts line 155, 711
• suggestWidget.ts line 922
• suggestWidgetDetails.ts line 93, 108
• suggestWidgetRenderer.ts line 138
• inlineChatWidget.ts line 605
• inlineChatZoneWidget.ts line 171, 176

@benibenj

• gutterIndicatorView.ts line 200
• inlineEditsCustomView.ts line 140
• inlineEditsInsertionView.ts line 64
• inlineEditsLineReplacementView.ts line 135, 208
• inlineEditsSideBySideView.ts line 292
• inlineEditsWordInsertView.ts line 51
• inlineEditsWordReplacementView.ts line 85

@mjbvz

• inlineProgress.ts line 68, 75
• parameterHintsWidget.ts line 130
• markupCell.ts line 316

@ulugbekna

• renameWidget.ts line 296, 686, 1026, 1033

@connor4312

• disassemblyView.ts line 205, 234, 235, 278, 696
• breakpointEditorContribution.ts line 771, 779
• breakpointWidget.ts line 427
• repl.ts line 392
• codeCoverageDecorations.ts line 145, 179
• testingDecorations.ts line 802, 804, 1370, 1376
• testingOutputPeek.ts line 716, 848

@amunger

• arrow.ts line 457
• notebookEditorWidget line 1743
• notebookOptions line 330, 391
• codeCellViewModel.ts line 246, 258

@DonJayamanne

• diffComponents.ts line 304, 908, 1143, 1878
• diffElementViewModel.ts line 458
• notebookDiffEditor.ts line 173
• notebookDiffList.ts line 47
• notebookMultiDiffEditor line 119
• notebookCellDiffDecorator.ts line 70

@rebornix

• cellComments.ts line 82, 126, 127
• codeCell.ts line 184
• notebookCellList line 985
• cellRenderer.ts line 67
• notebookOverviewRuler line 66, 107

@roblourens

• cellDragRenderer.ts line 43
• codeBlockPart.ts line 377, 389

@bpasero

• editorDictation.ts line 155

@bhavyaus

• walkThroughPart.ts line 328, 345

@alexr00

• commentThreadZoneWidget.ts line 418, 487, 488
• simpleCommentEditor.ts line 157

@lszomoru

• quickDiffWidget.ts line 229
• scmViewPane.ts line 1958

[bpasero]

@aiday-mar is there plans to pass down the current line number down to the widget? In my case for example, I implement the beforeRender function of a IContentWidget:

vscode/src/vs/workbench/contrib/codeEditor/browser/dictation/editorDictation.ts

Lines 154 to 162 in 519df4f

	beforeRender(): IDimension | null {
	const lineHeight = this.editor.getOption(EditorOption.lineHeight);
	const width = this.editor.getLayoutInfo().contentWidth * 0.7;
	this.domNode.style.setProperty('--vscode-editor-dictation-widget-height', `${lineHeight}px`);
	this.domNode.style.setProperty('--vscode-editor-dictation-widget-width', `${width}px`);
	return null;
	}

[aiday-mar]

Hi @bpasero currently there is no such plan, although I could implement that. Could we perhaps get the line height as follows?

const lineHeight = this.editor.getLineHeightForPosition(this.editor.getPosition());

[bpasero]

@aiday-mar since that can be null would this suffice:

const position = this.editor.getPosition();
const lineHeight = position ? this.editor.getLineHeightForPosition(position) : this.editor.getOption(EditorOption.lineHeight);

And is this beforeRender called each time the cursor moves?

[aiday-mar]

Yes I think beforeRender is called when the cursor moves.

beforeRender is called by prepareRender in contentWidget.ts which is called by prepareRender in ViewContentWidgets which is ultimately called by _onRenderScheduled in view.ts, which is called by prepareRender in View, which is called by _scheduleRender in View. This method is called by handleEvents in View and this method is itself called on different ViewEvents which includes the ViewCursorStateChangedEvent.

[DonJayamanne]

@aiday-mar is it possible to turn off the variable line height feature in editors used within the notebook.
I'm assuming it is
The problem is, with notebooks we have multiple editors laid out in a stack.
However the height of the editors are pre-computed to ensure there's not UI janking (this happens a lot in notebook land, diffing, notebook editors, scrolling, etc).

With this feature the height will never be close to accurate as the height can be different variable
Until we get this fixed, is there an option that can be passed into the monaco editor that disables variable heights at least in the notebook land, until we get this fixed.

Fixin in notebooks will not be a trivial job, hence this request.

[lszomoru]

@aiday-mar is it possible to turn off the variable line height?

+1 on this, as I would like to turn off the variable line height for the SCM input.

[aiday-mar]

Hi everyone thank you for the comments. Yes absolutely, you can turn off the variable line heights in an editor while constructing an editor using the IEditorConstructionOptions and setting the setting allowVariableLineHeights to false. There is an example of this in the file diffEditorEditors.ts.

Which reminds me, I forgot to mention that variable line heights are currently disabled in diff editors, so as to not have to align variable line heights in a side-by-side diff @hediet