From ec800a375c93596062a1ec616f2b58c28bea2d9a Mon Sep 17 00:00:00 2001 From: atul-upadhyay-7 Date: Fri, 19 Jun 2026 22:03:49 +0530 Subject: [PATCH] fix(core): include full color value in getStyleLine hash to prevent collision The diff renderer uses getStyleLine to detect style-only changes. The previous hash only used the first character of the color type string (e.g. 'a' for 'ansi256'), so two colors of the same type but different codes produced the same hash. This silently skipped color updates. Add hashColor helper that fingerprints the complete color value (type + code/name/r,g,b/hex) so different colors always produce distinct hashes. Closes #1659 --- packages/core/src/terminal/Screen.test.ts | 85 +++++++++++++++++++++++ packages/core/src/terminal/Screen.ts | 32 ++++++++- 2 files changed, 114 insertions(+), 3 deletions(-) diff --git a/packages/core/src/terminal/Screen.test.ts b/packages/core/src/terminal/Screen.test.ts index 782688bc..dd451731 100644 --- a/packages/core/src/terminal/Screen.test.ts +++ b/packages/core/src/terminal/Screen.test.ts @@ -133,6 +133,91 @@ describe('Screen', () => { }); }); +describe('getStyleLine', () => { + it('returns empty string for out-of-bounds row', () => { + const screen = new Screen(5, 3); + expect(screen.getStyleLine(-1)).toBe(''); + expect(screen.getStyleLine(3)).toBe(''); + }); + + it('different ansi256 codes produce different hashes', () => { + const screen = new Screen(5, 1); + screen.setCell(0, 0, { char: 'A', fg: { type: 'ansi256', code: 1 } }); + const hash1 = screen.getStyleLine(0); + + screen.setCell(0, 0, { char: 'A', fg: { type: 'ansi256', code: 9 } }); + const hash2 = screen.getStyleLine(0); + + expect(hash1).not.toBe(hash2); + }); + + it('different named colors produce different hashes', () => { + const screen = new Screen(5, 1); + screen.setCell(0, 0, { char: 'A', fg: { type: 'named', name: 'red' } }); + const hash1 = screen.getStyleLine(0); + + screen.setCell(0, 0, { char: 'A', fg: { type: 'named', name: 'blue' } }); + const hash2 = screen.getStyleLine(0); + + expect(hash1).not.toBe(hash2); + }); + + it('different rgb values produce different hashes', () => { + const screen = new Screen(5, 1); + screen.setCell(0, 0, { char: 'A', fg: { type: 'rgb', r: 255, g: 0, b: 0 } }); + const hash1 = screen.getStyleLine(0); + + screen.setCell(0, 0, { char: 'A', fg: { type: 'rgb', r: 0, g: 255, b: 0 } }); + const hash2 = screen.getStyleLine(0); + + expect(hash1).not.toBe(hash2); + }); + + it('different hex values produce different hashes', () => { + const screen = new Screen(5, 1); + screen.setCell(0, 0, { char: 'A', fg: { type: 'hex', hex: '#ff0000' } }); + const hash1 = screen.getStyleLine(0); + + screen.setCell(0, 0, { char: 'A', fg: { type: 'hex', hex: '#00ff00' } }); + const hash2 = screen.getStyleLine(0); + + expect(hash1).not.toBe(hash2); + }); + + it('same color values produce the same hash', () => { + const screen = new Screen(5, 1); + screen.setCell(0, 0, { char: 'A', fg: { type: 'ansi256', code: 42 } }); + const hash1 = screen.getStyleLine(0); + + screen.setCell(0, 0, { char: 'A', fg: { type: 'ansi256', code: 42 } }); + const hash2 = screen.getStyleLine(0); + + expect(hash1).toBe(hash2); + }); + + it('style flags still affect the hash', () => { + const screen = new Screen(5, 1); + screen.setCell(0, 0, { char: 'A', bold: false }); + const hash1 = screen.getStyleLine(0); + + screen.setCell(0, 0, { char: 'A', bold: true }); + const hash2 = screen.getStyleLine(0); + + expect(hash1).not.toBe(hash2); + }); + + it('background color changes are detected', () => { + const screen = new Screen(5, 1); + screen.setCell(0, 0, { char: 'A', bg: { type: 'ansi256', code: 1 } }); + const hash1 = screen.getStyleLine(0); + + screen.setCell(0, 0, { char: 'A', bg: { type: 'ansi256', code: 9 } }); + const hash2 = screen.getStyleLine(0); + + expect(hash1).not.toBe(hash2); + }); +}); + describe('cellsEqual', () => { it('returns true for identical cells', () => { const a = emptyCell(); diff --git a/packages/core/src/terminal/Screen.ts b/packages/core/src/terminal/Screen.ts index 1e33bcab..9dce7ed1 100644 --- a/packages/core/src/terminal/Screen.ts +++ b/packages/core/src/terminal/Screen.ts @@ -7,6 +7,32 @@ import { stringWidth, segmenter } from '../utils/unicode.js'; import { stripAnsiControl } from '../utils/ansi.js'; import { caps } from './env-caps.js'; +/** + * Produce a numeric fingerprint from a Color value. + * The hash incorporates both the color type and its specific value + * (code, name, r/g/b, or hex) so that two colors of the same type + * but different values produce distinct hashes. + */ +function hashColor(c: Color): number { + switch (c.type) { + case 'none': + return 0; + case 'named': + return c.name.charCodeAt(0) * 31 + c.name.charCodeAt(c.name.length - 1); + case 'ansi256': + return c.code * 7 + 1; + case 'rgb': + return (c.r << 16) | (c.g << 8) | c.b; + case 'hex': { + let h = 0; + for (let i = 0; i < c.hex.length; i++) { + h = ((h << 5) - h + c.hex.charCodeAt(i)) | 0; + } + return h; + } + } +} + const EMPTY_COLOR: Color = Object.freeze({ type: 'none' } as const); /** @@ -194,8 +220,8 @@ export class Screen { let hash = 0; for (const cell of this.back[row]) { if (cell.width === 0) continue; - const fg = cell.fg.type; - const bg = cell.bg.type; + const fgVal = hashColor(cell.fg); + const bgVal = hashColor(cell.bg); const bits = (cell.bold ? 1 : 0) | (cell.italic ? 2 : 0) | @@ -203,7 +229,7 @@ export class Screen { (cell.dim ? 8 : 0) | (cell.strikethrough ? 16 : 0) | (cell.inverse ? 32 : 0); - const seed = fg.charCodeAt(0) * 65536 + bg.charCodeAt(0) * 4096 + bits; + const seed = fgVal * 65536 + bgVal * 4096 + bits; hash = ((hash << 7) - hash + seed) | 0; if (cell.link) { for (let i = 0; i < cell.link.length; i++)