[JohnCampionJr]

Adds a theme colour for bold text whose foreground is the default, like Ghostty's bold-color, and one place where the bold rules are resolved. Following review, it also tells the 256-colour palette apart from the sixteen, so bold 38;5;0..7 is no longer brightened. All of it is additive: no existing public member changes signature, value or meaning.

Bold colour

• ThemeOptions.Bold (string?): any form ColorSpec parses. Unset, or a value that doesn't parse, keeps today's behaviour.
• ColorSnapshot.Bold and ColorPalette.Bold (int?, 0xRRGGBB): the parsed colour, or null when unset.
• OSC 5;0 (xterm's colorBD) reports the theme's bold colour until a program sets that slot.
• Live changes: it changes live like the rest of the theme, by assigning a new theme or calling ColorPalette.ApplyTheme.

Setting Bold deliberately does not switch palette brightening on, so each option means one thing. DrawBoldTextInBrightColors already defaults to true, so the default pairing matches Ghostty's.

Resolving a cell's colours

• int ColorSnapshot.ResolveForeground(in AttributeData attributes, bool drawBoldTextInBrightColors)
  • a bold cell with the default foreground takes Bold when the theme sets one;
  • a bold 16-colour index 0-7 moves to 8-15 when drawBoldTextInBrightColors is set;
  • a 256-colour index (38;5;n), even one below 8, and RGB come through as the program named them.
• int? ColorSnapshot.ResolveBackground(in AttributeData attributes) returns null for the default background, otherwise 0xRRGGBB.

Inverse, dim and invisible are left to the host. Before this PR, nothing in the library read DrawBoldTextInBrightColors; each host applied it on its own.

Telling the two palettes apart, without breaking hosts

AttributeData used to store 38;5;n exactly like 3n, so nothing could honour the DrawBoldTextInBrightColors doc ("a 256-palette index the user chose must come through unchanged").

• A flag bit: 38;5;n, 48;5;n and 58;5;n now set a flag in bit 26.
• The int mode accessors mask it off. GetFgColorMode(), GetBgColorMode() and TryGetUnderlineColor return exactly what they did: 0 for any palette colour or the default, 1 for RGB. A host that branches on mode == 0 ? palette : RGB keeps working. Storing 38;5;n as a new mode 2 would have silently broken such hosts.
• Only the new members expose the distinction: AttributeData.GetFgColorKind() and GetBgColorKind() return ColorMode. ColorMode keeps Palette256 = 0 and RGB = 1, and gains Palette16 = 2 and Default = 3.

Other review fixes

• No defensive copies: every pure getter on AttributeData is now readonly, so in AttributeData callers make no defensive copies. Both resolvers read Fg / Bg once. Adding readonly is binary compatible.
• Index past 255 ignored: a palette index above 255 in 38;5, 48;5 or 58;5 (and the colon forms) is ignored, as xterm does, and still consumed so later parameters keep their meaning. Before, it was stored and read as the default colour. The check sits inside the rare ;5; branch.

Kitty placeholders: 8-bit image ids and placement ids

With the two palettes told apart, kitty's Unicode placeholders get the rest of their id encodings:

• Image id: a placeholder cell now takes its image id from a 38;5;n foreground (an 8-bit id) as well as from 38;2 (24-bit). The third diacritic adds the high byte on top of either. 38;5;42 with U+030E gives 42 + (2 << 24), the spec's own example. SGR 30-37 and the default foreground stay colours, so red text doesn't summon image 1.
• Placement id: this was never read before. It now comes from the underline colour (58;5;n or 58;2), is stored on each placeholder run, starts a new rectangle when it changes, and lets a=d,d=i,i=N,p=M reach placeholder tiles.
• High-byte ids that only the third mark completes: these now resolve even when the colour's low bits name no image. That covers the spec's own 38;5;42 + U+030E example, and the usual case for clients that choose random 32-bit ids. Such a cell prints as text until its third mark arrives, then becomes a tile, and the rest of the run inherits the high byte from it (kitty's left-neighbour rule). A cell that never resolves is left exactly as an unknown id was before. Previously the image only appeared if its low 24 bits also happened to name another image.
• Cost: only the U+10EEEE and placeholder-mark paths changed. Ordinary characters pay the same two checks as before.
• A test that couldn't fail: A_palette_colour_is_not_read_as_an_id stored image 3 but wrote SGR 31 (index 1), so it passed either way. It now stores image 1 and is renamed A_sixteen_colour_sgr_is_not_read_as_an_id.

Deliberate, observable changes (bug fixes)

• DECRQSS reports 38;5;n faithfully, even below 16, so the reply replays to the same attributes. Before, it reported 38;5;1 as 31.
• AttributeData for 31 and for 38;5;1 no longer compare equal, as in xterm.js.

Tests and measurements

• Tests: XTerm.NET.Tests has 2459 passed, 1 skipped, 0 failed. New SgrColorModeTests cover every SGR colour form on both sides and pin the old int mode values, and a test pins the getters as readonly. KittyPlaceholderTests has 29 cases, including 8-bit ids with and without the high byte, and placement ids from both underline forms.
• Placeholder mutation check: five mutants, each caught: rejecting Palette256 again, accepting Palette16, accepting the default, dropping the placement id, and continuing a rectangle across placement ids.
• High-byte resolution: four new tests fail on the code before it: the spec example, a 24-bit id only the third mark names, a run inheriting the high byte, and a late-resolving cell keeping the tile its marks named. Five more mutants are each caught.
• Mutation check: seven mutants, each caught by the intended tests:
  • bold 256-colour index brightened;
  • 38;5;n stored without the flag;
  • Get*ColorKind ignores the flag;
  • the int accessor leaks the flag;
  • DECRQSS reports 38;5;1 as 31;
  • the index-past-255 check removed;
  • IsBold not readonly.
• Bench (alternating with the previous head, 12 s runs): truecolor +0.6% and sgr-churn +1.0% on averages, within noise.

🤖 Generated with Claude Code

[tomlm]

Overview

Adds ThemeOptions.Bold (Ghostty-style bold-color for default-foreground bold text), carries it through ColorPalette / ColorSnapshot, adds ColorSnapshot.ResolveForeground as the one place the bold rules live, and makes OSC 5;0 (colorBD) report the theme bold colour until a program sets that slot. The design is sound: plumbing the colour through every snapshot constructor, the ApplyTheme change check, and the OSC fallback chain are all correct.

Findings

Medium

• ResolveForeground brightens bold 38;5;0..7 to 8-15, which the existing DrawBoldTextInBrightColors doc says must not happen. The new theory row 1;38;5;1 -> 0xFF0000 writes that behaviour into a test. This PR turns a per-host quirk into the library's documented single source of truth, so the two docs now contradict each other inside the library (inline on the test row and on ColorPalette.cs:447).
• ResolveForeground(in AttributeData ...) calls non-readonly methods on an in parameter of a mutable struct, so the C# compiler emits a defensive copy of the 12-byte struct for each of the four calls. That's on the per-cell path the PR documents for hosts (inline on ColorPalette.cs:441).

Notes

• Low, already in the parser: 38;5;n is stored unclamped (InputHandler.cs HandleExtendedColor passes GetParam(index + 2) straight to SetFgColor(n, 0)). CSI 1;38;5;256 m or 1;38;5;300 m stores mode 0 with colour >= 256, and ResolveForeground sends that to the default-foreground branch, so the text is drawn in the theme bold colour. Not bold, it gets Foreground. xterm ignores an out-of-range index. The input is malformed, so this isn't blocking. Clamping or rejecting n > 255 in HandleExtendedColor fixes it at the source.
• Checked and correct:
  • all four ColorSnapshot construction sites carry BoldOrNone (SetColor, ResetAllColors, Set, NewSnapshot), so single-colour changes keep the bold colour (ThemeBold_SurvivesSingleColorChanges covers this).
  • ApplyTheme's changed check includes BoldOrNone, so a theme differing only in Bold raises ColorChanged(All).
  • NoBold = -1 can't collide with a parsed 0xRRGGBB.
  • Indexing is safe: GetFgColor() masks to 25 bits, so colors[color] is only reached for 0..255, and color + 8 stays <= 15.
  • The RGB 0x000100 vs default-marker-256 collision is handled by checking mode first, and the theory test exercises it through real SGR.
  • The OSC 5;0 fallback chain _specialColors[0] ?? Colors.Bold ?? Colors.Foreground resets correctly via OSC 105.
  • ThemeOptions copy constructor carries Bold.
  • The mutation claim in the description holds: dropping IsBold() from the default branch fails ResolveForeground_NotBold_IsUnaffectedByEitherBoldSetting, and dropping it from the palette branch fails the same test via Attr(1).
• Perf gate: nothing on the parse/print path changed. ResolveForeground is not called by the library, so the bench corpora are unaffected.

[tomlm]

Medium: this row writes into a test the behaviour the DrawBoldTextInBrightColors doc forbids.

TerminalOptions.cs:430-434 (unchanged by this PR) says:

It does not mean "lighten the colour": a 24-bit colour a program named exactly has no bright counterpart to select and must come through unchanged, and so must a 256-palette index the user chose.

This row asserts bold 38;5;1 resolves to 0xFF0000 (index 9). That's the brightening the doc rules out, and once the test is green the contradiction is locked in. The PR description calls it a known limitation, but before this PR the library contained no resolver: each host decided. Now ResolveForeground is documented as "the one place the bold rules live, so every host applies them alike", so every host that adopts it inherits the violation.

Fix, either way:

• Honour the doc. Give 38;5;n its own mode, the way xterm.js separates CM_P16 and CM_P256 (mode 2 fits, since SetFgColor already shifts mode into bit 25+). Then brighten only when mode is 0 and colour < 8. Every reader of GetFgColorMode() has to be checked: InputHandler.KittyGraphics.cs:122 tests == 0 to reject palette colours and would need != 1, and InputHandler.Csi.cs:133 (DECRQSS SGR report) would need to emit 38;5;n for mode 2. After that, this row should expect snapshot[1].
• Or change the contract. Amend the DrawBoldTextInBrightColors remarks to say a 256-palette index 0-7 is brightened too, because the cell can't tell it apart. Then keep this row, with a comment pointing at that doc instead of describing it as something the code merely cannot do.

[JohnCampionJr]

Fixed in 5e8bbca: we went with "honour the doc."

• This row now expects snapshot[1] (0xCD0000) for bold 38;5;1.
• New rows pin 38;5;9 → 9 and bold 31 → 9 (ResolveForeground_Bold31Brightens_Bold38_5_1DoesNot).
• Instead of mode 2, 38;5;n sets a flag bit that the int mode accessors mask off, so hosts reading GetFgColorMode() see no change.

[tomlm]

Medium: four defensive struct copies per call on the per-cell path.

attributes is an in parameter of AttributeData, which is a mutable struct (not readonly struct). GetFgColorMode(), GetFgColor() (called twice) and IsBold() aren't marked readonly (AttributeData.cs:84,129,132). C# can't prove a non-readonly method leaves a readonly ref alone, so it copies the 12-byte struct into a temp before each call. The JIT can sometimes remove those copies after inlining, but nothing guarantees it. This is the "struct copies hiding behind ordinary syntax" case from the repo's CLAUDE.md, on the method whose own comment says the bold field is an int "so the per-cell resolve reads one field".

Fix (one of):

• read the public fields directly: var fg = attributes.Fg; if ((fg >> 25) == 1) return fg & 0x1FFFFFF; ... and (attributes.Extended & 1) != 0 for bold (field access through an in ref doesn't copy), or
• mark the pure getters on AttributeData readonly (public readonly bool IsBold() => ..., public readonly int GetFgColor() => ..., GetFgColorMode). That removes the copies here and for every other in/ref readonly caller.

Either way, hoist attributes.GetFgColor() / Fg into a local before the mode check so it's read once.

[JohnCampionJr]

Fixed in 452fdc8. Every pure getter on AttributeData is now readonly: all Is* / Get*, TryGetUnderlineColor, Equals, GetHashCode and the new Get*ColorKind. That removes the copies here and for any other in caller. Both resolvers also read Fg / Bg into a local once.

Adding readonly is binary compatible. AttributeData_PureGetters_AreReadonly pins it, and a mutant that drops readonly from IsBold fails it.

[tomlm]

Medium (same issue as the test row at ColorPaletteTests.cs:652): this condition brightens 38;5;0..7 as well as 30-37.

HandleExtendedColor stores 38;5;n as SetFgColor(n, 0), the same as SGR 3n. So for CSI 1;38;5;2 m, color = 2 passes color < 8 and resolves to colors[10]. The <remarks> above say every colour other than palette 0-7 "comes through as the program named it". TerminalOptions.DrawBoldTextInBrightColors says a 256-palette index the user chose must come through unchanged. Both can't be true of this line. See the comment on the test row for the two ways to resolve it. If the contract changes, update these remarks so they say 38;5;0..7 is brightened too.

[JohnCampionJr]

Fixed in 5e8bbca. ResolveForeground now switches on the colour's kind, and only Palette16 0-7 brightens, so CSI 1;38;5;2 m resolves to colors[2]. The remarks say so explicitly, and so does the DrawBoldTextInBrightColors doc. A mutant that brightens Palette256 too fails two tests.

[JohnCampionJr]

Thanks. All addressed in 5e8bbca and 452fdc8, kept additive so it can ship as a minor version.

• Bold 38;5;0..7: fixed by telling the two palettes apart (see the inline threads). Rather than a new mode 2, which would silently break hosts that branch on GetFgColorMode() == 0, the 256-colour palette is a flag in bit 26 that the int accessors mask off. GetFgColorMode(), GetBgColorMode() and TryGetUnderlineColor return exactly what they did. The precise kind comes from the new GetFgColorKind() / GetBgColorKind(). ColorMode gains Palette16 = 2 and Default = 3; Palette256 = 0 and RGB = 1 are unchanged.
• Defensive copies: the getters are readonly now (see the inline reply).
• Unclamped 38;5;n: n > 255 is now ignored, as xterm does, and still consumed so later parameters keep their meaning. The same goes for 48;5, 58;5 and the colon forms. The check sits inside the ;5; branch.
• The readers you named:
  • KittyGraphics.cs:122 is unchanged and still correct: GetFgColorMode() == 0 holds for both palettes, so neither is taken as an image id.
  • The DECRQSS report now switches on the kind and reports 38;5;n faithfully, so the reply replays to the same attributes.
• Two deliberate, observable fixes: DECRQSS reports 38;5;1 as 38;5;1, not 31; and 31 and 38;5;1 no longer compare equal, as in xterm.js.

XTerm.NET.Tests: 2440 passed, 1 skipped, 0 failed. Seven mutants are each caught by the intended tests. The bench against the previous head is within noise (truecolor +0.6%, sgr-churn +1.0%). I've updated the PR description to match.

[JohnCampionJr]

One addition since your review, in de3a58e, because the 16/256 split makes it possible: kitty's Unicode placeholders now take an 8-bit image id from 38;5;n (with the third diacritic's high byte on top), and the placement id from the underline colour, which wasn't read at all before. SGR 30-37 and the default foreground still aren't ids. Only the U+10EEEE path changed.

The earlier commits are unchanged in content. Their messages were reworded, so the hashes in the replies above now read 5e8bbca and 452fdc8.

The description has a new section on it. XTerm.NET.Tests: 2451 passed, 1 skipped, 0 failed.

[JohnCampionJr]

One more addition, in d24cab7: a pre-existing gap the 8-bit work exposed. A placeholder whose image id needs the third (high-byte) diacritic only showed if its low 24 bits also named some other image, so the spec's own 38;5;42 + U+030E example didn't work, and neither did most random 32-bit ids.

Such a cell now prints as text until its third mark arrives, then becomes a tile. The rest of the run inherits the high byte, and a cell that never resolves is left exactly as before. Ordinary characters pay the same two checks on the print path.

Four new tests fail on the previous head. XTerm.NET.Tests: 2459 passed, 1 skipped, 0 failed.