Open
Conversation
Contributor
There was a problem hiding this comment.
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
ILineBreaksComputerContextand updates line breaks computers/factories to request bylineNumberand read content/injected text from the context atfinalize()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
createEmptyLineBreakWithPossiblyInjectedTextchecksif (injectedTexts)which will be true for an empty array. With contexts that return[]for “no injected text”, this will incorrectly produce injection metadata (emptyinjectionOptions/injectionOffsets) and return a non-nullModelLineProjectionDataeven 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 setsinjectionOptions/injectionOffsetsto empty arrays instead ofnull, which can break the “no injections” invariant and block optimizations elsewhere. Consider normalizing withcurInjectedTexts?.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 tonull(or switch checks toinjectedText?.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);
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It shows that the new inserted lines span
fromLineNumbertofromLineNumber + cnt - 1.The
toLineNumberis equal tostartLineNumber + insertingLinesCnt.But:
Hence:
In this context,
spliceLineNumber + 1represents 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-placespliceLineNumber + 1— the line number immediately after the last edited line, where newly inserted lines beginThis 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 + 1tells ModelRawLinesInserted where in the document the first of those 3 new lines appears.fromLineNumberrepresents the actual line number in the updated buffer where the newly inserted content can be found.The calculation
newLineCount - lineCount - cnt + spliceLineNumber + 1maps 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
fromLineNumbertells listeners (like the view model) where to actually read the new line content fromSo:
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