Skip to content

Commit be74906

Browse files
authored
fix: rebalance Pierre word highlights in split view (#203)
1 parent 3214793 commit be74906

6 files changed

Lines changed: 199 additions & 39 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ All notable user-visible changes to Hunk are documented in this file.
1010

1111
### Fixed
1212

13+
- Balanced Pierre word-level highlights so split-view inline changes stay visible without overpowering the surrounding diff row.
1314
- Smoothed mouse-wheel review scrolling so small diffs stay precise while sustained wheel gestures still speed up.
1415
- Fixed Shift+mouse-wheel horizontal scrolling so it no longer leaks a one-line vertical scroll in some terminals.
1516

src/ui/components/ui-components.test.tsx

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { act, createRef, useEffect, useState, type ReactNode } from "react";
55
import type { AppBootstrap, DiffFile } from "../../core/types";
66
import { createTestGitAppBootstrap } from "../../../test/helpers/app-bootstrap";
77
import { createTestDiffFile as buildTestDiffFile, lines } from "../../../test/helpers/diff-helpers";
8+
import { hexColorDistance } from "../lib/color";
89
import { resolveTheme } from "../themes";
910
import { measureDiffSectionGeometry } from "../lib/diffSectionGeometry";
1011
import { buildFileSectionLayouts, buildInStreamFileHeaderHeights } from "../lib/fileSectionLayout";
@@ -351,6 +352,44 @@ function frameHasHighlightedMarker(
351352
});
352353
}
353354

355+
/** Convert captured RGBA output back into a #rrggbb color string for contrast assertions. */
356+
function capturedColorToHex(color: { buffer?: ArrayLike<number> } | undefined) {
357+
const buffer = color?.buffer;
358+
if (!buffer || buffer[0] == null || buffer[1] == null || buffer[2] == null) {
359+
return null;
360+
}
361+
362+
const componentToHex = (value: number) =>
363+
Math.max(0, Math.min(255, Math.round(value * 255)))
364+
.toString(16)
365+
.padStart(2, "0");
366+
367+
return `#${componentToHex(buffer[0])}${componentToHex(buffer[1])}${componentToHex(buffer[2])}`;
368+
}
369+
370+
/** Measure the rendered background contrast between one word-diff span and its surrounding line. */
371+
function renderedWordDiffBackgroundDistance(
372+
frame: { lines: Array<{ spans: Array<{ text: string; bg?: { buffer?: ArrayLike<number> } }> }> },
373+
marker: string,
374+
) {
375+
for (const line of frame.lines) {
376+
const spanIndex = line.spans.findIndex((span) => span.text.includes(marker));
377+
if (spanIndex <= 0) {
378+
continue;
379+
}
380+
381+
const wordBg = capturedColorToHex(line.spans[spanIndex]?.bg);
382+
const surroundingBg = capturedColorToHex(line.spans[spanIndex - 1]?.bg);
383+
if (!wordBg || !surroundingBg) {
384+
continue;
385+
}
386+
387+
return hexColorDistance(wordBg, surroundingBg);
388+
}
389+
390+
return null;
391+
}
392+
354393
describe("UI components", () => {
355394
test("SidebarPane renders grouped file rows with indented filenames and right-aligned stats", async () => {
356395
const theme = resolveTheme("midnight", null);
@@ -1901,6 +1940,61 @@ describe("UI components", () => {
19011940
expect(binaryFileFrame).toContain("Binary file skipped");
19021941
});
19031942

1943+
test("PierreDiffView renders word-diff spans with a visibly different background in split view", async () => {
1944+
const file = createTestDiffFile(
1945+
"word-diff",
1946+
"word-diff.ts",
1947+
"export const answer = 41;\nexport const stable = true;\n",
1948+
"export const answer = 42;\nexport const stable = true;\n",
1949+
);
1950+
const theme = resolveTheme("graphite", null);
1951+
const setup = await testRender(
1952+
<PierreDiffView
1953+
file={file}
1954+
layout="split"
1955+
theme={theme}
1956+
width={120}
1957+
selectedHunkIndex={0}
1958+
scrollable={false}
1959+
/>,
1960+
{ width: 124, height: 10 },
1961+
);
1962+
1963+
try {
1964+
let removedBackgroundDistance: number | null = null;
1965+
let addedBackgroundDistance: number | null = null;
1966+
1967+
for (let iteration = 0; iteration < 200; iteration += 1) {
1968+
await act(async () => {
1969+
await setup.renderOnce();
1970+
await Bun.sleep(0);
1971+
await setup.renderOnce();
1972+
await Bun.sleep(0);
1973+
});
1974+
1975+
const frame = setup.captureSpans();
1976+
removedBackgroundDistance = renderedWordDiffBackgroundDistance(frame, "41");
1977+
addedBackgroundDistance = renderedWordDiffBackgroundDistance(frame, "42");
1978+
1979+
if (
1980+
removedBackgroundDistance !== null &&
1981+
addedBackgroundDistance !== null &&
1982+
removedBackgroundDistance > 0 &&
1983+
addedBackgroundDistance > 0
1984+
) {
1985+
break;
1986+
}
1987+
}
1988+
1989+
expect(removedBackgroundDistance).toBeGreaterThanOrEqual(28);
1990+
expect(addedBackgroundDistance).toBeGreaterThanOrEqual(28);
1991+
} finally {
1992+
await act(async () => {
1993+
setup.renderer.destroy();
1994+
});
1995+
}
1996+
});
1997+
19041998
test("PierreDiffView reuses highlighted rows after unmounting and remounting a file section", async () => {
19051999
const file = createTestDiffFile(
19062000
"cache",

src/ui/diff/pierre.test.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,19 @@ describe("Pierre diff rows", () => {
115115
throw new Error("Expected a split-line change row");
116116
}
117117

118-
expect(changedRow.left.spans.some((span) => span.text.includes("41"))).toBe(true);
119-
expect(changedRow.right.spans.some((span) => span.text.includes("42"))).toBe(true);
120-
expect(changedRow.left.spans.some((span) => span.bg === theme.removedContentBg)).toBe(true);
121-
expect(changedRow.right.spans.some((span) => span.bg === theme.addedContentBg)).toBe(true);
118+
const removedWordSpan = changedRow.left.spans.find((span) => span.text.includes("41"));
119+
const addedWordSpan = changedRow.right.spans.find((span) => span.text.includes("42"));
120+
121+
expect(removedWordSpan).toBeDefined();
122+
expect(addedWordSpan).toBeDefined();
123+
expect(removedWordSpan?.bg).toBeDefined();
124+
expect(addedWordSpan?.bg).toBeDefined();
125+
expect(changedRow.left.spans.some((span) => span.text.includes("export") && span.bg)).toBe(
126+
false,
127+
);
128+
expect(changedRow.right.spans.some((span) => span.text.includes("export") && span.bg)).toBe(
129+
false,
130+
);
122131
expect(
123132
changedRow.right.spans.some(
124133
(span) => span.text.includes("export") && typeof span.fg === "string",

src/ui/diff/pierre.ts

Lines changed: 50 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
} from "@pierre/diffs";
88
import { formatHunkHeader } from "../../core/hunkHeader";
99
import type { DiffFile } from "../../core/types";
10+
import { blendHex, hexColorDistance } from "../lib/color";
1011
import type { AppTheme } from "../themes";
1112
import { expandDiffTabs } from "./codeColumns";
1213

@@ -165,6 +166,53 @@ const normalizedColorCache = new Map<string, Map<string, string>>();
165166
// into terminal spans. The same highlighted line objects are reused when files remount or when
166167
// we build both split and stack rows, so memoize flattened spans by line node + theme/background.
167168
const flattenedHighlightedLineCache = new WeakMap<HastNode, Map<string, RenderSpan[]>>();
169+
const MIN_WORD_DIFF_BG_DISTANCE = 28;
170+
const WORD_DIFF_BLEND_STEP = 0.005;
171+
const WORD_DIFF_MAX_BLEND = 0.2;
172+
const wordDiffBackgroundCache = new Map<string, Record<SplitLineCell["kind"], string>>();
173+
174+
/** Blend toward the semantic sign color just enough to hit the minimum visible contrast. */
175+
function strengthenWordDiffBg(lineBg: string, signColor: string) {
176+
let strongestCandidate = lineBg;
177+
const maxSteps = Math.floor(WORD_DIFF_MAX_BLEND / WORD_DIFF_BLEND_STEP);
178+
179+
for (let step = 1; step <= maxSteps; step += 1) {
180+
const blendRatio = step * WORD_DIFF_BLEND_STEP;
181+
const candidate = blendHex(signColor, lineBg, blendRatio);
182+
strongestCandidate = candidate;
183+
184+
if (hexColorDistance(candidate, lineBg) >= MIN_WORD_DIFF_BG_DISTANCE) {
185+
return candidate;
186+
}
187+
}
188+
189+
return strongestCandidate;
190+
}
191+
192+
/** Resolve the inline word-diff background, strengthening theme colors that are too subtle to see. */
193+
function wordDiffHighlightBg(kind: SplitLineCell["kind"], theme: AppTheme) {
194+
let cached = wordDiffBackgroundCache.get(theme.id);
195+
if (!cached) {
196+
const addition =
197+
hexColorDistance(theme.addedContentBg, theme.addedBg) >= MIN_WORD_DIFF_BG_DISTANCE
198+
? theme.addedContentBg
199+
: strengthenWordDiffBg(theme.addedBg, theme.addedSignColor);
200+
const deletion =
201+
hexColorDistance(theme.removedContentBg, theme.removedBg) >= MIN_WORD_DIFF_BG_DISTANCE
202+
? theme.removedContentBg
203+
: strengthenWordDiffBg(theme.removedBg, theme.removedSignColor);
204+
205+
cached = {
206+
addition,
207+
context: theme.contextContentBg,
208+
deletion,
209+
empty: theme.panelAlt,
210+
};
211+
wordDiffBackgroundCache.set(theme.id, cached);
212+
}
213+
214+
return cached[kind];
215+
}
168216

169217
/** Remap Pierre token hues that collide with diff add/remove semantics into theme-safe syntax colors. */
170218
function normalizeHighlightedColor(color: string | undefined, theme: AppTheme) {
@@ -301,15 +349,7 @@ function makeSplitCell(
301349
const fallbackText = cleanDiffLine(rawLine);
302350
spans = fallbackText.length > 0 ? [{ text: fallbackText }] : [];
303351
} else {
304-
spans = flattenHighlightedLine(
305-
highlightedLine,
306-
theme,
307-
kind === "addition"
308-
? theme.addedContentBg
309-
: kind === "deletion"
310-
? theme.removedContentBg
311-
: theme.contextContentBg,
312-
);
352+
spans = flattenHighlightedLine(highlightedLine, theme, wordDiffHighlightBg(kind, theme));
313353

314354
if (spans.length === 0) {
315355
const fallbackText = cleanDiffLine(rawLine);
@@ -341,15 +381,7 @@ function makeStackCell(
341381
const fallbackText = cleanDiffLine(rawLine);
342382
spans = fallbackText.length > 0 ? [{ text: fallbackText }] : [];
343383
} else {
344-
spans = flattenHighlightedLine(
345-
highlightedLine,
346-
theme,
347-
kind === "addition"
348-
? theme.addedContentBg
349-
: kind === "deletion"
350-
? theme.removedContentBg
351-
: theme.contextContentBg,
352-
);
384+
spans = flattenHighlightedLine(highlightedLine, theme, wordDiffHighlightBg(kind, theme));
353385

354386
if (spans.length === 0) {
355387
const fallbackText = cleanDiffLine(rawLine);

src/ui/diff/renderRows.tsx

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
resolveStackCellGeometry,
88
} from "./codeColumns";
99
import type { DiffRow, RenderSpan, SplitLineCell, StackLineCell } from "./pierre";
10+
import { blendHex } from "../lib/color";
1011

1112
/** Clamp a label to one terminal row with an ellipsis. */
1213
export function fitText(text: string, width: number) {
@@ -79,23 +80,6 @@ function sliceSpansWindow(spans: RenderSpan[], offset: number, width: number) {
7980
};
8081
}
8182

82-
/** Parse a hex color string into RGB components. */
83-
function hexToRgb(hex: string) {
84-
const n = parseInt(hex.slice(1), 16);
85-
return { r: (n >> 16) & 0xff, g: (n >> 8) & 0xff, b: n & 0xff };
86-
}
87-
88-
/** Blend a color toward a background at a given ratio (0 = bg, 1 = fg). */
89-
function blendHex(fg: string, bg: string, ratio: number) {
90-
const f = hexToRgb(fg);
91-
const b = hexToRgb(bg);
92-
const mix = (a: number, z: number) => Math.round(z + (a - z) * ratio);
93-
const r = mix(f.r, b.r);
94-
const g = mix(f.g, b.g);
95-
const bl = mix(f.b, b.b);
96-
return `#${((r << 16) | (g << 8) | bl).toString(16).padStart(6, "0")}`;
97-
}
98-
9983
const INACTIVE_RAIL_BLEND = 0.35;
10084

10185
/** Dim a rail color for inactive hunks by blending toward the panel background. */

src/ui/lib/color.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/** One parsed RGB triplet from a #rrggbb hex color. */
2+
interface RgbColor {
3+
r: number;
4+
g: number;
5+
b: number;
6+
}
7+
8+
/** Parse a #rrggbb color into RGB components. Falls back to black for invalid input. */
9+
function hexToRgb(hex: string): RgbColor {
10+
const normalized = /^#?[0-9a-f]{6}$/i.test(hex) ? hex.replace(/^#/, "") : "000000";
11+
const value = parseInt(normalized, 16);
12+
return {
13+
r: (value >> 16) & 0xff,
14+
g: (value >> 8) & 0xff,
15+
b: value & 0xff,
16+
};
17+
}
18+
19+
/** Blend one foreground color toward a background color at a fixed ratio. */
20+
export function blendHex(fg: string, bg: string, ratio: number) {
21+
const foreground = hexToRgb(fg);
22+
const background = hexToRgb(bg);
23+
const mix = (front: number, back: number) =>
24+
Math.max(0, Math.min(255, Math.round(back + (front - back) * ratio)));
25+
26+
return `#${(
27+
(mix(foreground.r, background.r) << 16) |
28+
(mix(foreground.g, background.g) << 8) |
29+
mix(foreground.b, background.b)
30+
)
31+
.toString(16)
32+
.padStart(6, "0")}`;
33+
}
34+
35+
/** Measure how visually separated two #rrggbb colors are using channel deltas. */
36+
export function hexColorDistance(left: string, right: string) {
37+
const a = hexToRgb(left);
38+
const b = hexToRgb(right);
39+
return Math.abs(a.r - b.r) + Math.abs(a.g - b.g) + Math.abs(a.b - b.b);
40+
}

0 commit comments

Comments
 (0)