Skip to content

Use context for line breaks computation#293749

Open
aiday-mar wants to merge 3 commits intomainfrom
testy-tapir
Open

Use context for line breaks computation#293749
aiday-mar wants to merge 3 commits intomainfrom
testy-tapir

Conversation

@aiday-mar
Copy link
Contributor

@aiday-mar aiday-mar commented Feb 8, 2026

  • We want the new injected text to be processed as soon as it is received, hence we call _onDidChangeInjectedText at the end of the methods that change the decorations.
  • From the old piece of code:
for (let i = 0; i < cnt; i++) {
    const lineNumber = fromLineNumber + i;
    newLines[i] = this.getLineContent(lineNumber);

    injectedTextInEditedRangeQueue.takeWhile(r => r.lineNumber < lineNumber);
    injectedTexts[i] = injectedTextInEditedRangeQueue.takeWhile(r => r.lineNumber === lineNumber);
}

It shows that the new inserted lines span fromLineNumber to fromLineNumber + cnt - 1.

The toLineNumber is equal to startLineNumber + insertingLinesCnt.
But:

const spliceLineNumber = startLineNumber + editingLinesCnt;
const cnt = insertingLinesCnt - editingLinesCnt;

Hence:

toLineNumber = startLineNumber + insertingLinesCnt
    = spliceLineNumber - editingLinesCnt + cnt + editingLinesCnt
    = fromLineNumber - 1 + cnt
    = fromLineNumber + cnt - 1

In this context, spliceLineNumber + 1 represents the first line number where new lines are inserted in the document.

Here's the breakdown:

spliceLineNumber = startLineNumber + editingLinesCnt — the last line that was modified in-place
spliceLineNumber + 1 — the line number immediately after the last edited line, where newly inserted lines begin

This code handles the case when an edit inserts more lines than it deletes editingLinesCnt < insertingLinesCnt. For example, if you paste 5 lines over 2 existing lines:

2 lines are edited in-place (replaced with new content)
3 new lines need to be inserted after those

spliceLineNumber + 1 tells ModelRawLinesInserted where in the document the first of those 3 new lines appears.

fromLineNumber represents the actual line number in the updated buffer where the newly inserted content can be found.

The calculation newLineCount - lineCount - cnt + spliceLineNumber + 1 maps from the "logical" insertion point to the real position in the already-modified buffer.

Here's why this is needed:

By the time this event is emitted, the buffer has already been modified with all edits
When processing multiple edits, earlier changes shift line numbers
fromLineNumber tells listeners (like the view model) where to actually read the new line content from

So:

spliceLineNumber + 1 — the logical/semantic position where lines are inserted
fromLineNumber — the actual buffer position to read the inserted content from
cnt — how many lines were inserted

@aiday-mar aiday-mar requested review from alexdima and Copilot February 8, 2026 11:43
@aiday-mar aiday-mar self-assigned this Feb 8, 2026
@vs-code-engineering vs-code-engineering bot added this to the February 2026 milestone Feb 8, 2026
@aiday-mar aiday-mar marked this pull request as draft February 8, 2026 11:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors editor line wrapping computation to use a model-backed context (line-number based) instead of passing line text/injected text arrays through wrapping requests and raw model events. This aligns line break computation more closely with the live model state and reduces per-change payload/allocations.

Changes:

  • Introduces ILineBreaksComputerContext and updates line breaks computers/factories to request by lineNumber and read content/injected text from the context at finalize() time.
  • Simplifies raw text model change events to carry line-number mappings and counts (vs. full inserted line text arrays / injected text payloads).
  • Updates diff editor view zones and multiple tests to the new APIs and event shapes.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/vs/editor/common/modelLineProjectionData.ts Adds ILineBreaksComputerContext; updates factory/request signatures to be line-number based.
src/vs/editor/common/viewModel/monospaceLineBreaksComputer.ts Switches monospace wrapping computation to read line content/injections via context during finalize().
src/vs/editor/browser/view/domLineBreaksComputer.ts Switches DOM-based wrapping computation to read line content/injections via context during finalize().
src/vs/editor/common/viewModel/viewModelLines.ts Threads optional context into line-break computation and updates projection rebuild to use line-number requests.
src/vs/editor/common/viewModel/viewModelImpl.ts Updates view model’s two-pass change handling to compute line breaks from line numbers (using new mapping fields).
src/vs/editor/common/viewModel.ts Extends IViewModel.createLineBreaksComputer to optionally accept a line-break context.
src/vs/editor/common/textModelEvents.ts Redefines raw change payloads (line mapping + counts; computed toLineNumber getters).
src/vs/editor/common/model/textModel.ts Emits updated raw change events; adds injected-text-change event firing and exposes getLineInjectedText.
src/vs/editor/common/model.ts Adds ITextModel.getLineInjectedText (internal) to support context-based wrapping.
src/vs/editor/browser/widget/diffEditor/components/diffEditorViewZones/diffEditorViewZones.ts Supplies a custom line-break context to compute wrapping for “deleted code” view zones based on the original model.
src/vs/editor/test/common/viewModel/monospaceLineBreaksComputer.test.ts Updates wrapping tests to the new context + line-number request API.
src/vs/editor/test/common/model/modelInjectedText.test.ts Updates injected-text event tests to the new raw change shapes.
src/vs/editor/test/common/model/model.test.ts Updates core model change event tests to new constructors and count-based events.
Comments suppressed due to low confidence (3)

src/vs/editor/browser/view/domLineBreaksComputer.ts:55

  • createEmptyLineBreakWithPossiblyInjectedText checks if (injectedTexts) which will be true for an empty array. With contexts that return [] for “no injected text”, this will incorrectly produce injection metadata (empty injectionOptions/injectionOffsets) and return a non-null ModelLineProjectionData even when there is no injected text. Treat empty arrays as “no injected text” (e.g. injectedTexts?.length).
	function createEmptyLineBreakWithPossiblyInjectedText(lineNumber: number): ModelLineProjectionData | null {
		const injectedTexts = context.getLineInjectedText(lineNumber);
		if (injectedTexts) {
			const lineContent = context.getLineContent(lineNumber);
			const lineText = LineInjectedText.applyInjectedText(lineContent, injectedTexts);

			const injectionOptions = injectedTexts.map(t => t.options);
			const injectionOffsets = injectedTexts.map(text => text.column - 1);

			// creating a `LineBreakData` with an invalid `breakOffsetsVisibleColumn` is OK
			// because `breakOffsetsVisibleColumn` will never be used because it contains injected text
			return new ModelLineProjectionData(injectionOffsets, injectionOptions, [lineText.length], [], 0);
		} else {

src/vs/editor/browser/view/domLineBreaksComputer.ts:183

  • In finalize, curInjectedTexts = context.getLineInjectedText(lineNumber) is treated as truthy/falsey. If a context returns [] for “no injected text”, this sets injectionOptions/injectionOffsets to empty arrays instead of null, which can break the “no injections” invariant and block optimizations elsewhere. Consider normalizing with curInjectedTexts?.length ? ... : null.
		let injectionOptions: InjectedTextOptions[] | null;
		let injectionOffsets: number[] | null;
		const curInjectedTexts = context.getLineInjectedText(lineNumber);
		if (curInjectedTexts) {
			injectionOptions = curInjectedTexts.map(t => t.options);
			injectionOffsets = curInjectedTexts.map(text => text.column - 1);
		} else {
			injectionOptions = null;
			injectionOffsets = null;
		}

src/vs/editor/common/viewModel/monospaceLineBreaksComputer.ts:49

  • injectedText = context.getLineInjectedText(lineNumber) is used in boolean checks (!injectedText) to decide whether to take the incremental fast path. If a context returns an empty array for “no injected text”, the fast path will never be taken. Normalize empty arrays to null (or switch checks to injectedText?.length).
					const lineNumber = lineNumbers[i];
					const injectedText = context.getLineInjectedText(lineNumber);
					const lineText = context.getLineContent(lineNumber);
					const previousLineBreakData = previousBreakingData[i];
					const isLineFeedWrappingEnabled = wrapOnEscapedLineFeeds && lineText.includes('"') && lineText.includes('\\n');
					if (previousLineBreakData && !previousLineBreakData.injectionOptions && !injectedText && !isLineFeedWrappingEnabled) {
						result[i] = createLineBreaksFromPreviousLineBreaks(this.classifier, previousLineBreakData, lineText, tabSize, wrappingColumn, columnsForFullWidthChar, wrappingIndent, wordBreak);
					} else {
						result[i] = createLineBreaks(this.classifier, lineText, injectedText, tabSize, wrappingColumn, columnsForFullWidthChar, wrappingIndent, wordBreak, isLineFeedWrappingEnabled);

@aiday-mar aiday-mar marked this pull request as ready for review February 9, 2026 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments