diff --git a/.changeset/theme-guard-washout.md b/.changeset/theme-guard-washout.md new file mode 100644 index 000000000..f4c4af32d --- /dev/null +++ b/.changeset/theme-guard-washout.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Stop the theme contrast guards from washing out diff accents: low-contrast sign colors now get the smallest readable adjustment instead of a fixed 45% blend, and word-level diff emphasis is derived to the renderer's own separation floor so the highlight you see is the one the theme defines. diff --git a/src/ui/diff/diffRows.ts b/src/ui/diff/diffRows.ts index 467be3751..315bb597a 100644 --- a/src/ui/diff/diffRows.ts +++ b/src/ui/diff/diffRows.ts @@ -25,7 +25,7 @@ import type { DiffFile, DiffLineMoveKind } from "../../core/changeset/model"; import { blendHex, hexColorDistance } from "../lib/color"; import { measureTextWidth } from "../lib/text"; import { sanitizeTerminalLine } from "../../lib/terminalText"; -import { TRANSPARENT_BACKGROUND, type AppTheme } from "../themes"; +import { MIN_EMPHASIS_SEPARATION, TRANSPARENT_BACKGROUND, type AppTheme } from "../themes"; import { expandDiffTabs } from "./codeColumns"; import type { DiffRow, RenderSpan, SplitLineCell, StackLineCell } from "./diffRowModel"; import { @@ -116,7 +116,6 @@ function tabify(text: string, tabWidth: number, initialColumn = 0) { // into terminal spans. The same highlighted line objects are reused when files remount or when // we build both split and stack rows, so memoize flattened spans by line node + theme/background. const flattenedHighlightedLineCache = new WeakMap>(); -const MIN_WORD_DIFF_BG_DISTANCE = 28; const WORD_DIFF_BLEND_STEP = 0.005; const WORD_DIFF_MAX_BLEND = 0.2; const wordDiffBackgroundCache = new Map>(); @@ -131,7 +130,7 @@ function strengthenWordDiffBg(lineBg: string, signColor: string) { const candidate = blendHex(signColor, lineBg, blendRatio); strongestCandidate = candidate; - if (hexColorDistance(candidate, lineBg) >= MIN_WORD_DIFF_BG_DISTANCE) { + if (hexColorDistance(candidate, lineBg) >= MIN_EMPHASIS_SEPARATION) { return candidate; } } @@ -144,8 +143,8 @@ function isHexThemeColor(color: string) { return /^#[0-9a-f]{6}$/i.test(color); } -/** Resolve one word-diff background without turning transparent surfaces into black blends. */ -function resolveWordDiffHighlightBg(contentBg: string, lineBg: string, signColor: string) { +/** Strengthen custom-theme overrides whose pair sits too close together. */ +export function resolveWordDiffHighlightBg(contentBg: string, lineBg: string, signColor: string) { if (contentBg === TRANSPARENT_BACKGROUND || lineBg === TRANSPARENT_BACKGROUND) { return contentBg; } @@ -154,7 +153,7 @@ function resolveWordDiffHighlightBg(contentBg: string, lineBg: string, signColor return contentBg; } - return hexColorDistance(contentBg, lineBg) >= MIN_WORD_DIFF_BG_DISTANCE + return hexColorDistance(contentBg, lineBg) >= MIN_EMPHASIS_SEPARATION ? contentBg : strengthenWordDiffBg(lineBg, signColor); } diff --git a/src/ui/diff/rowStyle.ts b/src/ui/diff/rowStyle.ts index c95dfb82c..2b6739cec 100644 --- a/src/ui/diff/rowStyle.ts +++ b/src/ui/diff/rowStyle.ts @@ -185,7 +185,7 @@ export function stackCellPalette( }; } -// Word-diff emphasis guarantees 28 (`MIN_WORD_DIFF_BG_DISTANCE` in diffRows.ts), +// Word-diff emphasis guarantees 28 (`MIN_EMPHASIS_SEPARATION` in themes.ts), // but that floor is tuned for subtle tinting inside already-tinted lines. // Extension marks are things the user is looking *for* — search hits, // diagnostics — so they target a substantially higher floor: distances are diff --git a/src/ui/themes.test.ts b/src/ui/themes.test.ts index 13105e456..1aca66063 100644 --- a/src/ui/themes.test.ts +++ b/src/ui/themes.test.ts @@ -1,18 +1,27 @@ import { describe, expect, test } from "bun:test"; import { createTestCustomThemes } from "../../test/helpers/theme-helpers"; import { blendHex, contrastRatio, hexColorDistance } from "./lib/color"; -import { BUNDLED_SHIKI_THEME_IDS } from "../core/theme/catalog"; +import { + BUNDLED_SHIKI_THEME_IDS, + getBundledShikiThemeBackground, + getBundledShikiThemeDiffColors, +} from "../core/theme/catalog"; +import { resolveWordDiffHighlightBg } from "./diff/diffRows"; import { availableThemeIds, availableThemes, DEFAULT_DARK_THEME_ID, DEFAULT_LIGHT_THEME_ID, + MIN_DIFF_SIGN_CONTRAST, + MIN_EMPHASIS_SEPARATION, + readableDiffSign, resolveTheme, TRANSPARENT_BACKGROUND, withTransparentSurfaces, } from "./themes"; const MIN_READABLE_TEXT_CONTRAST = 4.5; +const MAX_RESCUE_HUE_DRIFT = 2; const SYNTAX_ROLES = [ "default", "keyword", @@ -27,6 +36,46 @@ const SYNTAX_ROLES = [ "punctuation", ] as const; +/** Return the HSL hue in degrees for a #rrggbb color, or null when achromatic. */ +function hexHueDegrees(hex: string): number | null { + const r = parseInt(hex.slice(1, 3), 16); + const g = parseInt(hex.slice(3, 5), 16); + const b = parseInt(hex.slice(5, 7), 16); + const max = Math.max(r, g, b); + const min = Math.min(r, g, b); + if (max === min) { + return null; + } + const chroma = max - min; + const segment = + max === r ? ((g - b) / chroma) % 6 : max === g ? (b - r) / chroma + 2 : (r - g) / chroma + 4; + return (segment * 60 + 360) % 360; +} + +/** Return the shortest angular distance between two hues in degrees. */ +function hueDistance(left: number, right: number) { + const delta = Math.abs(left - right) % 360; + return delta > 180 ? 360 - delta : delta; +} + +/** List each bundled theme's catalog diff accents beside the derived theme slots. */ +function bundledDiffSignSlots(themeId: string) { + const background = getBundledShikiThemeBackground(themeId) ?? "#0d1117"; + const diffColors = getBundledShikiThemeDiffColors(themeId); + const theme = resolveTheme(themeId, null); + const slots: Array<{ slot: string; source: string; derived: string }> = []; + if (diffColors?.added) { + slots.push({ slot: "added", source: diffColors.added, derived: theme.addedSignColor }); + } + if (diffColors?.removed) { + slots.push({ slot: "removed", source: diffColors.removed, derived: theme.removedSignColor }); + } + if (diffColors?.modified) { + slots.push({ slot: "modified", source: diffColors.modified, derived: theme.accent }); + } + return { background, slots }; +} + /** Return a compact failure list for semantic theme foreground/background pairs. */ function themeContrastFailures( pairs: Array<{ label: string; foreground: string; background: string; minimum?: number }>, @@ -78,7 +127,7 @@ describe("themes", () => { expect(dark.syntaxColors.default).toBe("#e6edf3"); expect(dark.addedSignColor).toBe("#3fb950"); expect(dark.removedSignColor).toBe("#f85149"); - expect(dark.addedBg).toBe(blendHex("#3fb950", "#0d1117", 0.2)); + expect(dark.addedBg).toBe(blendHex("#3fb950", "#0d1117", 0.18)); expect(dark.removedBg).toBe(blendHex("#f85149", "#0d1117", 0.2)); expect(light.background).toBe("#ffffff"); @@ -231,6 +280,111 @@ describe("themes", () => { } }); + test("keeps catalog diff accents untouched when they already meet the sign contrast floor", () => { + const failures = BUNDLED_SHIKI_THEME_IDS.flatMap((themeId) => { + const { background, slots } = bundledDiffSignSlots(themeId); + return slots.flatMap(({ slot, source, derived }) => { + if (contrastRatio(source, background) < MIN_DIFF_SIGN_CONTRAST) { + return []; + } + return derived === source ? [] : [`${themeId} ${slot}: ${source} rescued to ${derived}`]; + }); + }); + + expect(failures).toEqual([]); + }); + + test("rescued diff signs keep the source accent hue and clear the contrast floor", () => { + const failures = BUNDLED_SHIKI_THEME_IDS.flatMap((themeId) => { + const { background, slots } = bundledDiffSignSlots(themeId); + return slots.flatMap(({ slot, source, derived }) => { + if (contrastRatio(source, background) >= MIN_DIFF_SIGN_CONTRAST) { + return []; + } + const label = `${themeId} ${slot}: ${source} rescued to ${derived}`; + const rescuedContrast = contrastRatio(derived, background); + if (rescuedContrast < MIN_DIFF_SIGN_CONTRAST) { + return [`${label} but contrast is ${rescuedContrast.toFixed(2)}`]; + } + const sourceHue = hexHueDegrees(source); + const derivedHue = hexHueDegrees(derived); + if (sourceHue === null || derivedHue === null) { + // Achromatic accents (e.g. slack-ochin's white removed slot) have no hue to keep. + return []; + } + const drift = hueDistance(sourceHue, derivedHue); + return drift <= MAX_RESCUE_HUE_DRIFT ? [] : [`${label}, hue drifted ${drift.toFixed(1)}°`]; + }); + }); + + expect(failures).toEqual([]); + }); + + test("rescues diff signs with the smallest blend that clears the contrast floor", () => { + const failures = BUNDLED_SHIKI_THEME_IDS.flatMap((themeId) => { + const { background, slots } = bundledDiffSignSlots(themeId); + return slots.flatMap(({ slot, source, derived }) => { + if (contrastRatio(source, background) >= MIN_DIFF_SIGN_CONTRAST) { + return []; + } + const minimalRescues = ["#000000", "#ffffff"].flatMap((anchor) => { + for (let amount = 0.02; amount < 1; amount += 0.02) { + const candidate = blendHex(anchor, source, amount); + if (contrastRatio(candidate, background) >= MIN_DIFF_SIGN_CONTRAST) { + return [candidate]; + } + } + return []; + }); + return minimalRescues.includes(derived) + ? [] + : [ + `${themeId} ${slot}: ${source} rescued to ${derived}, expected a minimal rescue (${minimalRescues.join(", ")})`, + ]; + }); + }); + + expect(failures).toEqual([]); + }); + + test("nudges catppuccin-latte's near-miss green instead of washing it out", () => { + expect(resolveTheme("catppuccin-latte", null).addedSignColor).toBe("#3f9d2a"); + }); + + test("readableDiffSign upholds the contrast floor on mid-luminance backgrounds", () => { + const rescued = readableDiffSign("#b0b0b0", "#aaaaaa"); + expect(contrastRatio(rescued, "#aaaaaa")).toBeGreaterThanOrEqual(MIN_DIFF_SIGN_CONTRAST); + }); + + test("keeps the rendered word-level emphasis separated and readable on every bundled theme", () => { + const failures = BUNDLED_SHIKI_THEME_IDS.flatMap((themeId) => { + const theme = resolveTheme(themeId, null); + return ( + [ + ["added", theme.addedBg, theme.addedContentBg, theme.addedSignColor], + ["removed", theme.removedBg, theme.removedContentBg, theme.removedSignColor], + ] as const + ).flatMap(([slot, rowBackground, contentBackground, signColor]) => { + const rendered = resolveWordDiffHighlightBg(contentBackground, rowBackground, signColor); + const problems: string[] = []; + if (rendered !== contentBackground) { + problems.push(`renderer rewrote ${contentBackground} to ${rendered}`); + } + const separation = hexColorDistance(rowBackground, rendered); + if (separation < MIN_EMPHASIS_SEPARATION) { + problems.push(`separation ${separation} vs ${rowBackground}`); + } + const textContrast = contrastRatio(theme.text, rendered); + if (textContrast + 0.005 < MIN_READABLE_TEXT_CONTRAST) { + problems.push(`text contrast ${textContrast.toFixed(2)} on ${rendered}`); + } + return problems.map((problem) => `${themeId} ${slot}: ${problem}`); + }); + }); + + expect(failures).toEqual([]); + }); + test("layers custom theme overrides on a bundled base", () => { const custom = resolveTheme( "custom", diff --git a/src/ui/themes.ts b/src/ui/themes.ts index 28df368ff..1d24204f3 100644 --- a/src/ui/themes.ts +++ b/src/ui/themes.ts @@ -2,7 +2,7 @@ import type { ThemeMode } from "@opentui/core"; import { LEGACY_CUSTOM_THEME_ID } from "../core/theme/customThemes"; import { resolveSyntaxScopeOverrides } from "../core/theme/legacySyntaxScopes"; import type { NamedCustomThemeConfig } from "../extension-api/types"; -import { blendHex, contrastRatio, relativeLuminance } from "./lib/color"; +import { blendHex, contrastRatio, hexColorDistance, relativeLuminance } from "./lib/color"; import { BUNDLED_SHIKI_THEME_IDS, resolveBundledShikiThemeId, @@ -20,7 +20,8 @@ export const DEFAULT_DARK_THEME_ID = "github-dark-default"; export const DEFAULT_LIGHT_THEME_ID = "github-light-default"; const MIN_GUTTER_CONTRAST = 4.5; -const MIN_DIFF_SIGN_CONTRAST = 3; +export const MIN_DIFF_SIGN_CONTRAST = 3; +export const MIN_EMPHASIS_SEPARATION = 28; const FALLBACK_DIFF_COLORS = { dark: { added: "#5ecc71", removed: "#ff6762", modified: "#69b1ff" }, @@ -48,14 +49,23 @@ function readableDimForeground(preferred: string, background: string) { } /** Return a semantic diff marker color that remains legible on a theme editor surface. */ -function readableDiffSign(preferred: string, background: string) { +export function readableDiffSign(preferred: string, background: string) { if (contrastRatio(preferred, background) >= MIN_DIFF_SIGN_CONTRAST) { return preferred; } - return relativeLuminance(background) > 0.45 - ? blendHex("#000000", preferred, 0.45) - : blendHex("#ffffff", preferred, 0.45); + let anchor = relativeLuminance(background) > 0.45 ? "#000000" : "#ffffff"; + if (contrastRatio(anchor, background) < MIN_DIFF_SIGN_CONTRAST) { + anchor = anchor === "#000000" ? "#ffffff" : "#000000"; + } + for (let amount = 0.02; amount < 1; amount += 0.02) { + const candidate = blendHex(anchor, preferred, amount); + if (contrastRatio(candidate, background) >= MIN_DIFF_SIGN_CONTRAST) { + return candidate; + } + } + + return anchor; } /** Build Hunk's fallback semantic syntax palette for non-Shiki custom highlighting. */ @@ -92,6 +102,29 @@ function readableTintedBackground( return background; } +/** Return the strongest readable row tint that stays visibly apart from the word-emphasis tint. */ +function readableSeparatedRowBackground( + tintColor: string, + background: string, + foreground: string, + preferredAmount: number, + contentBackground: string, +) { + let readableFallback: string | undefined; + for (let amount = preferredAmount; amount >= 0.02; amount -= 0.02) { + const candidate = blendHex(tintColor, background, amount); + if (contrastRatio(foreground, candidate) < MIN_GUTTER_CONTRAST) { + continue; + } + if (hexColorDistance(candidate, contentBackground) >= MIN_EMPHASIS_SEPARATION) { + return candidate; + } + readableFallback ??= candidate; + } + + return readableFallback ?? background; +} + /** Keep semantic status colors readable on sidebar and menu surfaces. */ function readableChromeColor(preferred: string, panel: string, panelAlt: string) { if ( @@ -151,35 +184,37 @@ function buildShikiTheme(themeId: BundledShikiThemeId): AppTheme { diffColors?.modified ?? fallbackDiffColors.modified, editorBackground, ); - const addedBg = readableTintedBackground( + const addedContentBg = readableTintedBackground( addedSignColor, editorBackground, textForeground, - rowTint, + contentTint, ); - const removedBg = readableTintedBackground( + const removedContentBg = readableTintedBackground( removedSignColor, editorBackground, textForeground, - rowTint, + contentTint, ); - const movedBg = readableTintedBackground( - modifiedColor, + const addedBg = readableSeparatedRowBackground( + addedSignColor, editorBackground, textForeground, rowTint, + addedContentBg, ); - const addedContentBg = readableTintedBackground( - addedSignColor, + const removedBg = readableSeparatedRowBackground( + removedSignColor, editorBackground, textForeground, - contentTint, + rowTint, + removedContentBg, ); - const removedContentBg = readableTintedBackground( - removedSignColor, + const movedBg = readableTintedBackground( + modifiedColor, editorBackground, textForeground, - contentTint, + rowTint, ); const accentMuted = readableTintedBackground( modifiedColor,