From b3de9fa6db6a9278e4e16808c137e423f49ac896 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Mon, 1 Jun 2026 20:04:25 -0700 Subject: [PATCH] fix(buffer): clear stale attrs and combined data in BufferLine (#5972) * fix(buffer): bound combined-cell copy in copyCellsFrom When copying a column range, combined-string metadata was copied for every source column >= srcCol with no upper limit. Partial copies could pull _combined entries from outside [srcCol, srcCol + length) into the destination and corrupt cells beyond the copied span. Only copy combined keys within the same range as the cell data loop. Co-authored-by: Cursor * fix(buffer): clear stale _combined entry in BufferLine.setCell setCell updated combined data when IS_COMBINED_MASK was set but left orphaned _combined[index] entries when writing a non-combined cell. Delete the map entry in that case so later combined copies and lookups stay aligned with cell content flags. Co-authored-by: Cursor * fix(buffer): clear stale extended attrs in BufferLine.setCell setCell stored extended attributes when HAS_EXTENDED was set but did not remove orphaned _extendedAttrs[index] when overwriting with a cell without extended background data. Delete the entry in that case, matching the combined-cell cleanup added for setCell earlier. Co-authored-by: Cursor * fix(buffer): clear dest extended attrs in copyCellsFrom When copying a column range, extended attributes were copied from source cells with HAS_EXTENDED but stale _extendedAttrs on the destination were left for cells without extended data. Delete dest entries in that case, matching setCell behavior and the bounded _combined copy loop. Co-authored-by: Cursor * fix(buffer): clear extended attrs in setCellFromCodepoint The input-handler fast path set extended data when HAS_EXTENDED was present but left orphaned _extendedAttrs entries when overwriting with non-extended cells. Delete the map entry in that case, matching setCell and copyCellsFrom. Co-authored-by: Cursor * fix(buffer): clear stale combined data in setCellFromCodepoint When the input handler overwrites a cell that previously held a grapheme cluster (IS_COMBINED_MASK), setCellFromCodepoint updated _data and _extendedAttrs but left this._combined[index] intact. setCell already deletes _combined for non-combined cells; this path did not, so getString and renderers could show the old cluster string after a normal codepoint write. Add delete this._combined[index] at the start of setCellFromCodepoint, matching setCell's non-combined branch and the extended-attrs cleanup already performed in the same method. Co-authored-by: Cursor * perf(buffer): only delete stale combined/extended map entries when present Co-authored-by: Daniel Imms * fix(buffer): treat combined/extended maps as flag-gated caches _flags in _data are authoritative; sparse maps are only read when IS_COMBINED_MASK or HAS_EXTENDED is set. setCellFromCodepoint no longer touches _combined. loadCell clears CellData side fields when flags are off. copyCellsFrom copies map entries per-cell from src flags only. Co-authored-by: Daniel Imms --------- Co-authored-by: Cursor Co-authored-by: Daniel Imms --- src/common/buffer/BufferLine.ts | 61 +++++++++++++++++---------------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/src/common/buffer/BufferLine.ts b/src/common/buffer/BufferLine.ts index 4b6f6e8bb..30b4ed3bb 100644 --- a/src/common/buffer/BufferLine.ts +++ b/src/common/buffer/BufferLine.ts @@ -73,7 +73,9 @@ export interface IBufferLineStringCache { */ export class BufferLine implements IBufferLine { protected _data: Uint32Array; + /** Sparse cache; only read when `IS_COMBINED_MASK` is set in `_data`. */ protected _combined: {[index: number]: string} = {}; + /** Sparse cache; only read when `HAS_EXTENDED` is set in `_data`. */ protected _extendedAttrs: {[index: number]: IExtendedAttrs | undefined} = {}; protected _stringCacheEntryRef: WeakRef | undefined; public length: number; @@ -205,9 +207,15 @@ export class BufferLine implements IBufferLine { cell.bg = this._data[$startIndex + Cell.BG]; if (cell.content & Content.IS_COMBINED_MASK) { cell.combinedData = this._combined[index]; + } else { + cell.combinedData = ''; } if (cell.bg & BgFlags.HAS_EXTENDED) { cell.extended = this._extendedAttrs[index]!; + } else { + // Do not mutate cell.extended in place: it may still reference this line's map entry from a + // prior loadCell into a reused CellData (e.g. $workCell during insert/delete). + cell.extended = DEFAULT_ATTR_DATA.extended.clone(); } return cell; } @@ -459,14 +467,7 @@ export class BufferLine implements IBufferLine { this._data.set(line._data); } this.length = line.length; - this._combined = {}; - for (const el in line._combined) { - this._combined[el] = line._combined[el]; - } - this._extendedAttrs = {}; - for (const el in line._extendedAttrs) { - this._extendedAttrs[el] = line._extendedAttrs[el]; - } + this._copySparseMapsFrom(line); this.isWrapped = line.isWrapped; } @@ -475,12 +476,7 @@ export class BufferLine implements IBufferLine { const newLine = new BufferLine(this._stringCache, 0, undefined, false); newLine._data = new Uint32Array(this._data); newLine.length = this.length; - for (const el in this._combined) { - newLine._combined[el] = this._combined[el]; - } - for (const el in this._extendedAttrs) { - newLine._extendedAttrs[el] = this._extendedAttrs[el]; - } + newLine._copySparseMapsFrom(this); newLine.isWrapped = this.isWrapped; return newLine; } @@ -511,27 +507,14 @@ export class BufferLine implements IBufferLine { for (let i = 0; i < Constants.CELL_INDICIES; i++) { this._data[(destCol + cell) * Constants.CELL_INDICIES + i] = srcData[(srcCol + cell) * Constants.CELL_INDICIES + i]; } - if (srcData[(srcCol + cell) * Constants.CELL_INDICIES + Cell.BG] & BgFlags.HAS_EXTENDED) { - this._extendedAttrs[destCol + cell] = src._extendedAttrs[srcCol + cell]; - } + this._copyCellMapsFrom(src, srcCol + cell, destCol + cell); } } else { for (let cell = 0; cell < length; cell++) { for (let i = 0; i < Constants.CELL_INDICIES; i++) { this._data[(destCol + cell) * Constants.CELL_INDICIES + i] = srcData[(srcCol + cell) * Constants.CELL_INDICIES + i]; } - if (srcData[(srcCol + cell) * Constants.CELL_INDICIES + Cell.BG] & BgFlags.HAS_EXTENDED) { - this._extendedAttrs[destCol + cell] = src._extendedAttrs[srcCol + cell]; - } - } - } - - // Move any combined data over as needed, FIXME: repeat for extended attrs - const srcCombinedKeys = Object.keys(src._combined); - for (let i = 0; i < srcCombinedKeys.length; i++) { - const key = parseInt(srcCombinedKeys[i], 10); - if (key >= srcCol) { - this._combined[key - srcCol + destCol] = src._combined[key]; + this._copyCellMapsFrom(src, srcCol + cell, destCol + cell); } } } @@ -620,4 +603,24 @@ export class BufferLine implements IBufferLine { cacheEntry.isTrimmed = false; } } + + /** Copy sparse map entries for a single cell when `_data` flags require them. */ + private _copyCellMapsFrom(src: BufferLine, srcCol: number, destCol: number): void { + const srcStart = srcCol * Constants.CELL_INDICIES; + if (src._data[srcStart + Cell.CONTENT] & Content.IS_COMBINED_MASK) { + this._combined[destCol] = src._combined[srcCol]; + } + if (src._data[srcStart + Cell.BG] & BgFlags.HAS_EXTENDED) { + this._extendedAttrs[destCol] = src._extendedAttrs[srcCol]; + } + } + + /** Rebuild sparse maps from another line, keyed only by `_data` flags. */ + private _copySparseMapsFrom(line: BufferLine): void { + this._combined = {}; + this._extendedAttrs = {}; + for (let i = 0; i < line.length; i++) { + this._copyCellMapsFrom(line, i, i); + } + } }