From 99bf0fa0eba8305de925d26424facaee21771858 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 22:09:01 -0500 Subject: [PATCH 1/8] docs: spec the legible-colour floor and the trigger route that keeps a line Two reported defects with one cause. Measured every colour the client can paint against the plane it lands on: only `grey` of the sixteen picker names clears 3:1 on both the dark and light themes, and the freeze bar's accent is 1.27:1 on a focused dark pane -- which is the reported "purple against a blue background". Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- ...gible-colour-and-trigger-routing-design.md | 188 ++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md diff --git a/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md b/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md new file mode 100644 index 0000000..442f20b --- /dev/null +++ b/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md @@ -0,0 +1,188 @@ +# Legible colour, and a route that means "leave it where it is" + +**Date:** 2026-08-11 +**Status:** approved + +Two reported defects that turn out to share a cause. + +> "we are using unreadable colors by default against our backgrounds. Such as Freeze +> being purple against a blue background." + +> "we failed to solve the issue of Triggers — we should be able to Highlight text and +> send it to the pane where we found it. That way, players can highlight character name +> text." + +The second is mostly the first: a highlight rule *does* fire, and the colour it fires in +is invisible. What is genuinely missing on the trigger side is smaller than it looked, +and is stated in §5. + +## The measurement + +Contrast ratios (WCAG relative luminance) of each colour against the pane plane it is +actually painted on — the focused pane surface, which is the brightest plane a dark theme +can wear and therefore the worst case for a foreground being read on it. + +The F2 highlight picker's palette: + +| name | dark | light | | name | dark | light | +|---|---|---|---|---|---|---| +| purple | **1.27** | 8.43 | | gold | 8.55 | **1.26** | +| blue | **1.40** | 7.69 | | yellow | 11.16 | **1.04** | +| black | **1.75** | 18.79 | | white | 11.99 | **1.12** | +| green | **2.33** | 4.60 | | cyan | 9.56 | **1.12** | +| teal | **2.51** | 4.27 | | pink | 7.79 | **1.38** | +| red | **3.00** | 3.58 | | lime | 8.74 | **1.23** | +| magenta | 3.82 | **2.81** | | orange | 6.07 | **1.77** | +| silver | 6.59 | **1.63** | | grey | 3.04 | 3.53 | + +Six of sixteen fail on dark, nine of sixteen on light, and **only `grey` clears 3:1 on +both**. That is the finding the whole design turns on: a palette of fixed hexes cannot +serve two themes, so the resolution has to happen at the moment of painting, against the +plane the text lands on — not at the moment of picking. + +The client's own chrome, same measurement: + +| what | value | dark | light | +|---|---|---|---| +| freeze / away / restore bar accent | `ResolveIndex(5)` = `#800080` | **1.27** | 8.43 | +| rail accent, drop zones, ⌃P chip | `#00f5b7` | 11.13 | **1.42** | +| draft pen | `#ffd700` | 11.30 | **1.26** | +| prefix panel, client warnings | `#e5c07b` | 9.18 | **1.73** | + +The reported Freeze defect is the first row. The rest of the table says the Light theme's +chrome has never been readable. + +And the server's own text, on the default dark theme's focused pane: ANSI 4 blue +**1.34**, ANSI 1 red **1.09**, ANSI 0 black **1.75**, ANSI 5 magenta **1.27**. MU\* servers +emit these constantly. + +## 1 — One rule, applied where a colour meets its plane + +New `SharpMUTerm.Core.Text.Contrast`. Pure, no UI dependency: + +- `RelativeLuminance(Rgb)` and `Ratio(Rgb, Rgb)` — WCAG 2.x, the definition every number + in this document was produced with. +- `Legible(Rgb foreground, Rgb plane, double floor)` — the foreground blended toward white + when `plane` is dark and toward black when it is light, by the **smallest** amount that + clears `floor`. Returns the foreground unchanged when it already clears it. + +Two properties this has to have, and one it cannot. + +**Direction is the plane's, not the colour's.** One function serves all three themes +because it asks the plane which way "away" is. A dark plane always lifts, a light plane +always darkens; there is no theme in which the answer is ambiguous, because a pane plane is +never mid-grey. + +**Hue survives until headroom runs out, and then it desaturates.** Blending toward white +raises luminance monotonically and reaches any target without clipping a channel — which +scaling upward cannot do, and which is the same reasoning `WorkspacePalette.AtLuma` is +already written on. It has to desaturate eventually: pure `#0000ff` has a relative +luminance of 0.0722 and so tops out at 1.88:1 against a dark pane *at full blue*. A rule +that preserved hue absolutely would leave that colour unreadable, which is the defect. + +**It is not reversible.** `Legible` is a projection: two foregrounds that differ only below +the floor come out closer together than they went in. That is the cost of the floor and it +is accepted — the alternative is the current behaviour, where they are equally invisible. + +**The floor is 3.0:1**, WCAG AA for large text and UI components. Deliberately not 4.5: at +4.5 the server's bright-black de-emphasis and the `dim` attribute stop meaning anything — +the client would flatten every deliberate act of de-emphasis a game makes. + +## 2 — What the server sends + +`MarkupFormatter.StyleTag` passes its resolved foreground through `Legible` before writing +the hex, measured against: + +- the span's **own background** when it has one (a highlight's background, or a `reverse` + swap) — that plane is known exactly, and it is the one the text lands on; +- otherwise the **pane plane**, because a span with a default background emits none and + takes whatever it is drawn on. + +**The plane handed to the formatter is per theme, not per pane.** A pane's actual plane +varies with the character's tint and with focus, and re-resolving per pane would mean +re-formatting a whole buffer on every focus move — the expensive path CLAUDE.md reserves for +one deliberate keystroke. Instead the lift targets the **extreme of the band** in the plane's +own direction: on a dark theme the brightest plane a pane can wear (untinted, focused), on a +light theme the darkest. Clearing the floor there clears it on every other plane, because +once the foreground is past the background's luminance the ratio is monotone in the +background — so one reference plane per theme is not an approximation, it is the worst case. + +Gated by a new F7 preference, `keep text legible`, **default on**. Off emits today's exact +bytes: a user who wants their game's own palette untouched, or who has a theme where the +floor fights their taste, turns it off and nothing else changes. + +## 3 — What the client paints itself + +The chrome literals in the table above are not rescued by §2 — they are the client's own +colours and should be *derived* correctly rather than repaired at the last moment. They +become named inks on `WorkspacePalette`, each resolved from the active theme and then held +to the same floor against the plane it actually lands on: + +| ink | replaces | plane it is measured against | +|---|---|---| +| `Marker` | `SharpMUTermApp.FrozenAccentHex()` (`ResolveIndex(5)`) | the pane band | +| `Accent` | `RailRenderer.DefaultAccent`, `PaneDropRenderer.ZoneColor` | `Backdrop` | +| `Notice` | `PrefixPanel` / `ClientMessageRenderer` `#e5c07b` | `Backdrop` | +| `Draft` | `RailRenderer` `#ffd700` | `Backdrop` | + +`ScreenPalette` is **not** touched. Those constants sit on the settings screens' own fixed +dark backdrop, which the theme does not move — measuring them against a theme plane would be +measuring them against a plane they are never painted on. + +## 4 — The picker + +The palette's names stay. What changes is that the F2 swatch is painted through the same +lift, so the picker shows the colour the pane will show. A name is then a *hue* the theme +resolves, which is what the pane-tint work already established as this codebase's way of +naming a colour that has to survive a theme change. + +## 5 — Triggers + +Four changes. No schema change and no migration: `TriggerActions.SpawnTarget` keeps its +type and its null. + +**`route` gains an explicit `(none)`.** The rule adds no destination; the line follows +whatever the other matched rules decided, and lands in the session's main window if nobody +routed it. This is exactly what `SpawnTarget = null` has always meant — F2 labelled it +`main`, and that label is why "highlight it and leave it where it was" looked like something +the screen could not express. New rules default to it. + +**`main` becomes a real destination** — the matching session's own main window. Reserved +name; no window is titled `main` today (a character's session window is titled after the +character, and `main` is only the rail's *label* for it), so nothing collides. A config that +already says `SpawnTarget = "main"` currently conjures a capture pane called "main"; after +this it reaches the window whoever wrote it meant. + +**Gag suppresses the default delivery only.** Explicit destinations survive it. That is +already true of spawn panes — `route: Chat` + gag has always meant "only in Chat" — and it +becomes true of `main`, so `route: main` + gag keeps the line in the main window instead of +deleting it. This is the answer to "gag means only where I routed it". + +**Destinations are deduplicated.** Two rules naming one pane deliver one line. Today +`TriggerEngine` appends to a bare list and `WorldSession` raises `SpawnLine` per entry, so +a highlight rule pointed at the same pane as its capture rule delivers the line twice. + +What is deliberately *not* changed: a highlight rule does not need a route to reach the pane +a capture rule sent the line to. There is one line and one set of destinations, and every +matched rule's highlight is on it — which is what the user's own framing asked for ("it +should still follow the original route, as long as it does not change where it routes to"). + +## Testing + +**Core.** +- `ContrastTests` — the floor is reached; the ratio is at least the floor and not + wastefully above it; hue is held while headroom lasts; both directions; already-legible + colours come back byte-identical; idempotent. +- `LegiblePaletteTests` — a table over all 16 picker names × ANSI 0–15 × three themes × + the whole plane band (untinted and all six tints, focused and not), asserting the floor. + This is the test that would have caught the reported defect, and it is the one that keeps + a future theme from reintroducing it. +- Trigger routing: `(none)` adds nothing; `main` delivers to the main window; gag + + explicit route keeps the line; two rules on one target deliver once. + +**Tui.** +- `WorkspacePaletteTests` — every named ink clears the floor on every theme, against the + plane it is documented as landing on. +- Snapshots: `freeze`, `away`, `highlight`, and a Light-theme frame, with a decoded-grid + assertion on the freeze bar's painted cells rather than on the markup string — the bug + was in what reached the screen. From f518efa53198b31f59129fd9b540c71f93ca7d8d Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 22:23:26 -0500 Subject: [PATCH 2/8] feat(colour): a legibility floor where a colour meets the plane it lands on The reported defect is one cell of a grid. The freeze bar took its accent from the theme's index 5 and painted it on the pane: #800080 on a #36363d focused pane is 1.27:1, very nearly the same colour twice. Measured across the whole grid, six of the F2 picker's sixteen names fail 3:1 on the dark theme, nine fail on the light one, and only `grey` clears both -- a palette of fixed hexes cannot serve two themes, so the resolution has to happen where the colour meets its plane rather than where it is chosen. Core.Text.Contrast is that rule: WCAG luminance and ratio, plus Legible(), which moves a foreground the smallest distance that clears the floor and leaves anything already legible byte-identical. Direction is the plane's, not the colour's -- a dark plane lifts and a light plane darkens -- which is what lets one function serve all three themes. Hue survives while there is headroom and then desaturates, because pure #0000ff tops out at 1.88:1 on a dark pane at full blue and a rule that held hue absolutely would leave the commonest unreadable colour in MU* output unreadable. The floor is 3.0:1 and deliberately not 4.5: a game's own de-emphasis is spoken in exactly the colours a 4.5 floor would erase. MarkupFormatter applies it to every foreground it paints, against the span's own background when it has one and the theme's reading plane when it does not. That plane is per *theme*, not per pane -- the extreme of the fourteen a pane can wear, which is the worst case rather than an approximation of one -- so a focus change never re-formats a buffer. F7's `keep text legible` (default on) switches it off to exactly the previous bytes. The client's own hexes become ChromeInk, derived from the theme and held to the same floor. That fixes a second defect nobody had reported: on the Light theme the accent was 1.42:1, the draft pen 1.26:1 and the notice 1.73:1, so its chrome has never been readable -- and no snapshot showed it because every frame in the gallery renders Dark. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- .../Configuration/PreferenceSettings.cs | 23 +++ src/SharpMUTerm.Core/Text/Contrast.cs | 140 +++++++++++++ src/SharpMUTerm.Tui/ChromeInk.cs | 66 ++++++ src/SharpMUTerm.Tui/MarkupFormatter.cs | 26 +++ src/SharpMUTerm.Tui/OptionsScreenRenderer.cs | 8 + src/SharpMUTerm.Tui/PaneDropRenderer.cs | 19 +- src/SharpMUTerm.Tui/PrefixPanel.cs | 20 +- src/SharpMUTerm.Tui/RailRenderer.cs | 44 ++-- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 62 ++++-- src/SharpMUTerm.Tui/TriggersScreenRenderer.cs | 45 +++- src/SharpMUTerm.Tui/TriggersScreenView.cs | 7 +- src/SharpMUTerm.Tui/WorkspacePalette.cs | 92 +++++++++ tests/SharpMUTerm.Core.Tests/ContrastTests.cs | 159 +++++++++++++++ .../LegiblePaletteTests.cs | 192 ++++++++++++++++++ .../MarkupFormatterTests.cs | 74 ++++++- .../PaneDropRendererTests.cs | 4 +- .../ScreenCursorTests.cs | 6 +- .../SharpMUTerm.Tui.Tests/ScreenModelTests.cs | 29 ++- 18 files changed, 939 insertions(+), 77 deletions(-) create mode 100644 src/SharpMUTerm.Core/Text/Contrast.cs create mode 100644 src/SharpMUTerm.Tui/ChromeInk.cs create mode 100644 tests/SharpMUTerm.Core.Tests/ContrastTests.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/LegiblePaletteTests.cs diff --git a/src/SharpMUTerm.Core/Configuration/PreferenceSettings.cs b/src/SharpMUTerm.Core/Configuration/PreferenceSettings.cs index be90c08..6aad4a5 100644 --- a/src/SharpMUTerm.Core/Configuration/PreferenceSettings.cs +++ b/src/SharpMUTerm.Core/Configuration/PreferenceSettings.cs @@ -32,6 +32,29 @@ public sealed class TextSettings /// Honour the blink attribute rather than dropping it. public bool AllowBlink { get; set; } + /// + /// Hold every foreground this client paints to against the plane + /// it lands on — moving it the smallest distance that clears the floor, and leaving anything already + /// legible byte-identical. + /// + /// On by default, because what it replaces is not a stylistic state. On the default dark theme's + /// focused pane the colours a MU* server sends constantly measure blue 1.34:1, red + /// 1.09:1, black 1.75:1 and magenta 1.27:1 — very nearly the surface painted + /// twice. Games are written for black terminals; this one is not black. + /// + /// + /// It is a render-time decision, which is what lets it repaint history: the stored colour is + /// untouched and only the markup a control is fed changes. That places it with the timestamp gutter + /// rather than with and , which are + /// ingest-time and reach only the next line. + /// + /// + /// Off emits exactly the bytes this client emitted before the floor existed — for a reader who wants + /// their game's own palette untouched, or a theme where the floor fights their taste. + /// + /// + public bool KeepTextLegible { get; set; } = true; + /// /// How many spaces a tab in server output is drawn as. Zero removes tabs entirely. /// diff --git a/src/SharpMUTerm.Core/Text/Contrast.cs b/src/SharpMUTerm.Core/Text/Contrast.cs new file mode 100644 index 0000000..513ea40 --- /dev/null +++ b/src/SharpMUTerm.Core/Text/Contrast.cs @@ -0,0 +1,140 @@ +namespace SharpMUTerm.Core.Text; + +/// +/// The legibility floor: how far apart two colours are, and how to move a foreground the smallest +/// distance that makes it readable on the plane it is about to be painted on. +/// +/// It exists because a colour is only legible relative to something, and this client had no +/// place that knew both halves at once. The freeze bar took its accent from ANSI 5 and painted it on +/// the pane surface: #800080 on #36363d is 1.27:1, which is very nearly the same +/// colour twice. The F2 highlight picker had the same shape one layer over — of its sixteen named +/// colours, only grey clears 3:1 on both the dark and the light theme, six fail on dark and +/// nine on light. A palette of fixed hexes cannot serve two themes, so the resolution has to happen +/// where the colour meets its plane rather than where it is chosen. +/// +/// +/// Pure and UI-agnostic on purpose: the same numbers decide a markup hex in the TUI, an ink on the +/// workspace palette, and anything a log renderer wants later. +/// +/// +public static class Contrast +{ + /// + /// The ratio a foreground must clear against its plane: 3.0:1, WCAG AA for large text and + /// user-interface components. + /// + /// Deliberately not 4.5. A MU* server's own de-emphasis is spoken in exactly the colours a 4.5 + /// floor would erase — bright black for asides, dim for a status line nobody is meant to + /// read twice — and a client that lifted those to body-text contrast would be flattening every + /// deliberate act of de-emphasis a game makes. Three is the point where a colour stops being + /// invisible, which is the complaint; four and a half is the point where it stops being quiet, + /// which is not. + /// + /// + public const double Floor = 3.0; + + /// + /// WCAG relative luminance — the sRGB channels linearised and weighted. This is the definition + /// every contrast number in this codebase's design notes was produced with; it is not the + /// BT.601 luma 's other consumers use (see WorkspacePalette.Luma), and the + /// two are not interchangeable: that one is linear in the channels, which is a property the tint + /// arithmetic leans on, and this one is not, which is why it can express a *ratio* a reader + /// perceives. + /// + public static double RelativeLuminance(Rgb rgb) => + (0.2126 * Linearise(rgb.R)) + (0.7152 * Linearise(rgb.G)) + (0.0722 * Linearise(rgb.B)); + + private static double Linearise(byte channel) + { + var v = channel / 255.0; + return v <= 0.03928 ? v / 12.92 : Math.Pow((v + 0.055) / 1.055, 2.4); + } + + /// + /// How far apart two colours read, from 1:1 (identical) to 21:1 (black on white). Symmetric — + /// it says nothing about which one is the text. + /// + public static double Ratio(Rgb a, Rgb b) + { + var la = RelativeLuminance(a); + var lb = RelativeLuminance(b); + return (Math.Max(la, lb) + 0.05) / (Math.Min(la, lb) + 0.05); + } + + /// + /// moved the smallest distance that clears + /// against , or returned unchanged when it already does. + /// + /// The direction is the plane's, not the colour's. A dark plane always lifts and a light + /// plane always darkens, which is what lets one function serve every theme: a pane plane is never + /// mid-grey, so there is no theme in which "away from the background" is ambiguous. The pivot is + /// the plane's own luminance against mid-scale. + /// + /// + /// Hue survives while there is headroom, and then it desaturates. Blending toward white (or + /// black) is monotone in luminance and can reach any target without clipping a channel, which + /// scaling cannot — the same reasoning WorkspacePalette.AtLuma is written on. It has to give + /// hue up eventually: pure #0000ff has a relative luminance of 0.0722 and so tops out at + /// 1.88:1 on a dark pane at full blue, so a rule that held hue absolutely would leave the + /// commonest unreadable colour in MU* output unreadable. What is kept is the channel order, so a + /// lifted colour is still recognisably the one the server sent. + /// + /// + /// It is a projection and not a bijection. Two foregrounds differing only below the floor + /// come out closer together than they went in. That is the price of the floor, and the alternative + /// is the present behaviour, in which they are equally invisible. + /// + /// + /// Best effort. A floor that is arithmetically unreachable (21:1 against a mid-grey) yields + /// the furthest colour in the chosen direction rather than an exception: the caller is a render + /// path fed from the telnet read loop, and there is nothing useful it could do with a throw. + /// + /// + public static Rgb Legible(Rgb foreground, Rgb plane, double floor = Floor) + { + if (Ratio(foreground, plane) >= floor) + { + return foreground; + } + + // Mid-scale in *luminance*, not in bytes: 0.18 is the relative luminance of the sRGB mid-grey + // a reader would call "half way", and comparing bytes instead would call #808080 (0.216) dark. + var target = RelativeLuminance(plane) < 0.18 + ? new Rgb(0xff, 0xff, 0xff) + : new Rgb(0x00, 0x00, 0x00); + + // Binary search on the blend rather than a closed form: the luminance curve is piecewise and + // the blend is in gamma space, so there is no inverse worth writing. Monotone in t, so twelve + // steps land within a quarter of a byte — finer than the channel the answer is rounded to. + var lo = 0.0; + var hi = 1.0; + for (var i = 0; i < 12; i++) + { + var mid = (lo + hi) / 2; + if (Ratio(Mix(foreground, target, mid), plane) >= floor) + { + hi = mid; + } + else + { + lo = mid; + } + } + + var best = Mix(foreground, target, hi); + + // The search converges on the *threshold*; rounding to bytes can land a hair under it, and a + // caller asserting the floor would then see it missed by a thousandth. Step to the end rather + // than iterate: the remaining distance is at most a quarter of a byte, so this is the last + // blend or the extreme itself. + return Ratio(best, plane) >= floor ? best : target; + } + + /// Linear blend, of the way from to . + private static Rgb Mix(Rgb from, Rgb to, double t) => new( + Channel(from.R + ((to.R - from.R) * t)), + Channel(from.G + ((to.G - from.G) * t)), + Channel(from.B + ((to.B - from.B) * t))); + + private static byte Channel(double value) => (byte)Math.Clamp(Math.Round(value), 0, 255); +} diff --git a/src/SharpMUTerm.Tui/ChromeInk.cs b/src/SharpMUTerm.Tui/ChromeInk.cs new file mode 100644 index 0000000..1f78d2b --- /dev/null +++ b/src/SharpMUTerm.Tui/ChromeInk.cs @@ -0,0 +1,66 @@ +using SharpMUTerm.Core.Text; + +namespace SharpMUTerm.Tui; + +/// +/// The four colours this client paints in its own voice on the workspace — as +/// #rrggbb strings, ready to interpolate into markup. They are produced by +/// , which holds each one to +/// against the plane it lands on. +/// +/// Why they are not constants any more. They were, and each was picked against a dark theme +/// and then painted on whatever plane the workspace happened to have. Measured against the plane they +/// actually land on: the boundary bars' accent (ResolveIndex(5), #800080) is +/// 1.27:1 on a focused dark pane — the reported defect — while on the Light theme the accent +/// #00f5b7 is 1.42:1, the draft pen #ffd700 is 1.26:1 and the notice +/// #e5c07b is 1.73:1. The Light theme's chrome has never been readable, and no snapshot +/// showed it because every frame in the gallery renders the dark theme. +/// +/// +/// What is kept from the old hexes is the hue. These are still the app's teal, its amber and +/// its gold; the theme decides how light they have to be to be seen. That is the same division the +/// pane tints already run on — a name (here, a base hue) is the durable thing, and the brightness is +/// the theme's answer, because a hex picked against one theme becomes a hole in the next. +/// +/// +/// is deliberately not built this way. Those colours sit on the +/// settings screens' own fixed backdrop, which no theme moves, so measuring them against a theme plane +/// would be measuring them against a plane they are never painted on. +/// +/// +/// +/// The app's teal: the rail's fallback accent, a live drop zone, the ⌃P chip, the logging indicator. +/// +/// +/// The amber a client-voiced label is drawn in — MOVE, DRAG, the scrollback segment, the +/// ⌃B strip, a which-key entry's chord. +/// +/// The gold pen marking a rail row whose window holds an unsent draft. +/// +/// The boundary bars' accent — ▲ FROZEN, the away bar, the restore bar. It is the one of the +/// four that is drawn from the theme's palette (index 5) rather than from a base hue here, +/// because a theme that overrides the base sixteen has an opinion about its own violet. +/// +internal readonly record struct ChromeInk(string Accent, string Notice, string Draft, string Marker) +{ + /// The app's teal accent, before a theme has said how light it needs to be. + internal static readonly Rgb BaseAccent = new(0x00, 0xf5, 0xb7); + + /// The amber of a client-voiced label, ditto. + internal static readonly Rgb BaseNotice = new(0xe5, 0xc0, 0x7b); + + /// The unsent-draft pen's gold, ditto. + internal static readonly Rgb BaseDraft = new(0xff, 0xd7, 0x00); + + /// + /// The base hues with no plane applied — what the client painted before any of this was measured. + /// It is what a pure renderer falls back to when no ink is handed in, which keeps those renderers + /// callable from a unit test that has no theme and is not asking a question about colour. Nothing + /// in the running app uses it: SharpMUTermApp resolves a real one from the active theme. + /// + internal static ChromeInk Default { get; } = new( + BaseAccent.ToHex(), + BaseNotice.ToHex(), + BaseDraft.ToHex(), + AnsiPalette.ToRgb(5).ToHex()); +} diff --git a/src/SharpMUTerm.Tui/MarkupFormatter.cs b/src/SharpMUTerm.Tui/MarkupFormatter.cs index 0f9d7b3..6cdb46f 100644 --- a/src/SharpMUTerm.Tui/MarkupFormatter.cs +++ b/src/SharpMUTerm.Tui/MarkupFormatter.cs @@ -24,6 +24,14 @@ internal sealed class MarkupFormatter(Theme theme, TextSettings? text = null) private readonly Theme _theme = theme; private readonly TextSettings _text = text ?? new TextSettings(); + /// + /// The plane a span carrying no background of its own is read on — computed once per formatter, + /// because it is a property of the theme and not of any one pane. See + /// for why one plane covers all fourteen a pane can + /// wear, and why resolving it per pane would cost a whole-buffer re-format on every focus move. + /// + private readonly Rgb _plane = WorkspacePalette.ReadingPlane(theme); + /// Renders a whole line to a single markup string, with no timestamp gutter. public string ToMarkup(StyledLine line) { @@ -136,6 +144,24 @@ private void AppendSpan(StringBuilder sb, StyledSpan span) (fg, bg) = (bg, fg); } + // The legibility floor, applied at the one point in this app that knows both the colour and the + // plane it is about to be painted on. A span that carries a background is measured against *it* + // — that plane is known exactly, and a highlight's own pair must be judged as a pair; one with + // no background emits none (see below) and takes the pane it is drawn on, so it is measured + // against the theme's reading plane. + // + // What this is for: on the default dark theme's focused pane, the ANSI colours a MU* server + // sends constantly are blue 1.34:1, red 1.09:1, black 1.75:1 and magenta 1.27:1 — text that is + // very nearly the surface twice. It is a *floor* and not a scheme: a colour already clearing + // Contrast.Floor comes back byte-identical, which is most of what any game sends. + // + // Off restores the previous bytes exactly, for a reader who wants their game's palette + // untouched or a theme where the floor fights their taste. + if (_text.KeepTextLegible) + { + fg = Contrast.Legible(fg, style.Background.Kind == TerminalColorKind.Default && !reverse ? _plane : bg); + } + var tokens = new List(6); if (style.HasAttribute(TextAttributes.Bold)) { diff --git a/src/SharpMUTerm.Tui/OptionsScreenRenderer.cs b/src/SharpMUTerm.Tui/OptionsScreenRenderer.cs index b376f82..e0ce775 100644 --- a/src/SharpMUTerm.Tui/OptionsScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/OptionsScreenRenderer.cs @@ -261,6 +261,14 @@ internal static OptionsScreen TextAnsiScreen(TextSettings? text = null) new("├ COLOUR", null, null), new("strip incoming ANSI colour", null, settings.StripIncomingColour, null, ScreenToggle.Bind(() => settings.StripIncomingColour, v => settings.StripIncomingColour = v)), + + // Directly under the row above, because the two are answers to one question — what to do + // about a colour this terminal cannot show well — and they are the ends of its range: that + // one discards every colour the server sent, this one keeps them all and moves only the few + // that cannot be read. Unticking it is not "no policy"; it is the policy this client had + // before anything was measured. + new("keep text legible", null, settings.KeepTextLegible, null, + ScreenToggle.Bind(() => settings.KeepTextLegible, v => settings.KeepTextLegible = v)), new("allow blink", null, settings.AllowBlink, null, ScreenToggle.Bind(() => settings.AllowBlink, v => settings.AllowBlink = v)), new("underline hyperlinks", null, settings.UnderlineHyperlinks, null, diff --git a/src/SharpMUTerm.Tui/PaneDropRenderer.cs b/src/SharpMUTerm.Tui/PaneDropRenderer.cs index e67a9b0..f98c51e 100644 --- a/src/SharpMUTerm.Tui/PaneDropRenderer.cs +++ b/src/SharpMUTerm.Tui/PaneDropRenderer.cs @@ -11,8 +11,12 @@ namespace SharpMUTerm.Tui; /// internal static class PaneDropRenderer { - /// The highlight for the live drop zone (the app's teal accent). - internal const string ZoneColor = "#00f5b7"; + /// + /// The highlight for the live drop zone — the app's teal accent, for the theme the caller is drawing + /// in. It is a parameter and no longer a constant because this lands in a pane: measured + /// against the plane it is painted on, the old literal is 1.42:1 on the Light theme. + /// + private static string Zone(ChromeInk? ink) => (ink ?? ChromeInk.Default).Accent; /// /// Renders one pane of the drag preview to markup rows of @@ -26,8 +30,10 @@ internal static List Render( int height, bool hovered, Edge? edge, - double edgeFraction = DropZones.DefaultEdgeFraction) + double edgeFraction = DropZones.DefaultEdgeFraction, + ChromeInk? ink = null) { + var zoneColor = Zone(ink); width = Math.Max(1, width); height = Math.Max(1, height); @@ -38,7 +44,7 @@ internal static List Render( for (var row = 0; row < height; row++) { var characters = row == textRow ? text : new string(' ', width); - lines.Add(RenderRow(characters, row, width, height, hovered, edge, edgeFraction)); + lines.Add(RenderRow(characters, row, width, height, hovered, edge, edgeFraction, zoneColor)); } return lines; @@ -88,7 +94,8 @@ private static string RenderRow( int height, bool hovered, Edge? edge, - double edgeFraction) + double edgeFraction, + string zoneColor) { var builder = new StringBuilder(); var open = false; @@ -104,7 +111,7 @@ private static string RenderRow( builder.Append("[/]"); } - builder.Append(zone ? $"[black on {ZoneColor}]" : "[dim]"); + builder.Append(zone ? $"[black on {zoneColor}]" : "[dim]"); open = true; highlighted = zone; } diff --git a/src/SharpMUTerm.Tui/PrefixPanel.cs b/src/SharpMUTerm.Tui/PrefixPanel.cs index 95c0ba4..7664b4d 100644 --- a/src/SharpMUTerm.Tui/PrefixPanel.cs +++ b/src/SharpMUTerm.Tui/PrefixPanel.cs @@ -1,3 +1,4 @@ +using System.Globalization; using static SharpMUTerm.Tui.MarkupText; using static SharpMUTerm.Tui.ScreenPalette; @@ -210,26 +211,33 @@ internal static int MaxWidth(IReadOnlyList lines) => /// narrow is exactly the one where the panel does the explaining a moment later. /// /// - internal static string Strip(int available) + /// + /// is the client's own voice for the active theme. It is a parameter rather + /// than a constant because this string lands on the status line — a themed plane — while + /// everything else this class draws lands on the prefix overlay's own fixed backdrop. Null means the + /// unmeasured base hues, which is what a unit test with no theme wants. + /// + internal static string Strip(int available, ChromeInk? ink = null) { + var notice = (ink ?? ChromeInk.Default).Notice; foreach (var (plain, markup) in Forms) { if (plain.Length <= available) { - return markup; + return string.Format(CultureInfo.InvariantCulture, markup, notice); } } - return Forms[^1].Markup; + return string.Format(CultureInfo.InvariantCulture, Forms[^1].Markup, notice); } /// The strip's spellings, widest first, each as the plain text to measure and the markup to emit. private static readonly (string Plain, string Markup)[] Forms = { ($"⌃B — awaiting {StripKeys} · Esc cancels", - $"[#e5c07b]⌃B — awaiting[/] [dim]{StripKeys} · Esc cancels[/]"), + $"[{{0}}]⌃B — awaiting[/] [dim]{StripKeys} · Esc cancels[/]"), ($"⌃B {StripKeys} Esc", - $"[#e5c07b]⌃B[/] [dim]{StripKeys} Esc[/]"), - ("⌃B — Esc cancels", "[#e5c07b]⌃B[/] [dim]Esc cancels[/]"), + $"[{{0}}]⌃B[/] [dim]{StripKeys} Esc[/]"), + ("⌃B — Esc cancels", "[{0}]⌃B[/] [dim]Esc cancels[/]"), }; } diff --git a/src/SharpMUTerm.Tui/RailRenderer.cs b/src/SharpMUTerm.Tui/RailRenderer.cs index 268b55b..1ca3420 100644 --- a/src/SharpMUTerm.Tui/RailRenderer.cs +++ b/src/SharpMUTerm.Tui/RailRenderer.cs @@ -22,7 +22,6 @@ namespace SharpMUTerm.Tui; /// internal static class RailRenderer { - private const string DefaultAccent = "#00f5b7"; /// The rail's rows, as projects them. /// @@ -31,9 +30,17 @@ internal static class RailRenderer /// (SharpMUTermApp.RailWidth), so any name past the clamp — a web page's title is the easy one — /// would run onto a second line. A wrapped rail row is the thing the report was about. /// - public static List Render(IReadOnlyList rows, int maxWidth = int.MaxValue) + /// + /// The client's own voice for the active theme. A parameter rather than the constants it replaced, + /// because these land on the : measured against the plane they + /// are painted on, the old accent is 1.42:1 and the old draft pen 1.26:1 on the Light theme. Null + /// means the unmeasured base hues, which is what a unit test with no theme wants. + /// + public static List Render( + IReadOnlyList rows, int maxWidth = int.MaxValue, ChromeInk? ink = null) { ArgumentNullException.ThrowIfNull(rows); + var voice = ink ?? ChromeInk.Default; // Whether the chord column is drawn, decided once for the whole rail and **per row kind**. // @@ -54,20 +61,20 @@ public static List Render(IReadOnlyList rows, int maxWidth = in var lines = new List(rows.Count); foreach (var row in rows) { - lines.Add(Fit(row, maxWidth, r => RenderRow(r, reserveWindow, reserveCharacter))); + lines.Add(Fit(row, maxWidth, r => RenderRow(r, reserveWindow, reserveCharacter, voice))); } return lines; } - private static string RenderRow(RailRow row, bool reserveWindow, bool reserveCharacter) => row.Kind switch + private static string RenderRow(RailRow row, bool reserveWindow, bool reserveCharacter, ChromeInk ink) => row.Kind switch { RailRowKind.Header => $"[dim]┌ {Glyphs.Connections} CONNECTIONS[/]", - RailRowKind.World => Link(row, $"[{Accent(row)}]▚[/] [bold]{Escape(row.Label)}[/]"), + RailRowKind.World => Link(row, $"[{Accent(row, ink)}]▚[/] [bold]{Escape(row.Label)}[/]"), RailRowKind.Host => $"{Indent(row)}[dim]{Escape(row.Label)}[/]", RailRowKind.Empty => $"{Indent(row)}{Link(row, $"[dim]{Escape(row.Label)}[/]")}", - RailRowKind.Character => Character(row, reserveCharacter), - RailRowKind.Window => Window(row, reserveWindow), + RailRowKind.Character => Character(row, reserveCharacter, ink), + RailRowKind.Window => Window(row, reserveWindow, ink), _ => Escape(row.Label), }; @@ -108,15 +115,16 @@ private static string Fit(RailRow row, int maxWidth, Func rende /// is the only handle a collapsed rail offers, so if it did not switch character the strip would be /// decoration. (It was decoration, and this comment said otherwise, until the rail was wired up.) /// - public static List RenderCollapsed(IReadOnlyList rows) + public static List RenderCollapsed(IReadOnlyList rows, ChromeInk? ink = null) { + var voice = ink ?? ChromeInk.Default; var lines = new List(); foreach (var row in rows) { switch (row.Kind) { case RailRowKind.World: - lines.Add(Link(row, $"[{Accent(row)}]▚[/]")); + lines.Add(Link(row, $"[{Accent(row, voice)}]▚[/]")); break; case RailRowKind.Character: var initial = row.Label.Length > 0 @@ -128,7 +136,7 @@ public static List RenderCollapsed(IReadOnlyList rows) // Reserved here too. The collapsed strip is clamped to 4–10 cells, so it moves less — // but it moves, and a strip that widens when a background world says something is the // same reflow as the expanded rail's, on a rail chosen for taking no space. - lines.Add(Link(row, $"[{Accent(row)}]{dot}[/]{name}{UnreadField(row.Unread)}")); + lines.Add(Link(row, $"[{Accent(row, voice)}]{dot}[/]{name}{UnreadField(row.Unread)}")); break; } } @@ -156,13 +164,13 @@ public static List RenderCollapsed(IReadOnlyList rows) /// goes to it. /// /// - private static string Character(RailRow row, bool reserve) + private static string Character(RailRow row, bool reserve, ChromeInk ink) { var marker = row.Active ? "[bold]▸[/]" : " "; var dot = row.Connected ? "●" : "○"; var name = row.Active ? $"[bold]{Escape(row.Label)}[/]" : Escape(row.Label); return $"{Indent(row)}{ChordField(row.Chord, reserve)}" - + Link(row, $"{marker} [{Accent(row)}]{dot}[/] {name}{UnreadField(row.Unread)}"); + + Link(row, $"{marker} [{Accent(row, ink)}]{dot}[/] {name}{UnreadField(row.Unread)}"); } /// @@ -224,8 +232,8 @@ private static string ChordField(string? chord, bool reserve) private const int UnreadFieldWidth = UnreadBadge.FieldWidth; /// The pen, or the same width in blanks. See . - private static string Unsent(bool unsent) => - unsent ? $" [#ffd700]{Glyphs.Draft}[/]" : new string(' ', UnsentFieldWidth); + private static string Unsent(bool unsent, ChromeInk ink) => + unsent ? $" [{ink.Draft}]{Glyphs.Draft}[/]" : new string(' ', UnsentFieldWidth); /// /// An unread count in a fixed-width field, right-aligned, blank at zero. Reserved for the same reason @@ -270,12 +278,12 @@ private static string UnreadField(int unread) => /// the two-meanings-in-one-column mistake again. /// /// - private static string Window(RailRow row, bool reserve) + private static string Window(RailRow row, bool reserve, ChromeInk ink) { var name = Escape(row.Label); var closed = row.Closed ? " [dim]closed[/]" : string.Empty; return $"{Indent(row)}{ChordField(row.Closed ? null : row.Chord, reserve)}" - + Link(row, $"[dim]▪[/] {name}{Unsent(row.Unsent)}{UnreadField(row.Unread)}{closed}"); + + Link(row, $"[dim]▪[/] {name}{Unsent(row.Unsent, ink)}{UnreadField(row.Unread)}{closed}"); } /// @@ -291,8 +299,8 @@ private static string Link(RailRow row, string content) => private static string Indent(RailRow row) => new(' ', row.Indent * 2); - private static string Accent(RailRow row) => + private static string Accent(RailRow row, ChromeInk ink) => row.Accent.Kind == TerminalColorKind.Rgb ? $"#{row.Accent.R:x2}{row.Accent.G:x2}{row.Accent.B:x2}" - : DefaultAccent; + : ink.Accent; } diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index d788e32..20ac1ac 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -46,6 +46,14 @@ internal sealed class SharpMUTermApp : IAsyncDisposable private readonly SessionManager _sessions = new(); private readonly TerminalCapabilities _capabilities; private readonly Theme _theme; + + /// + /// The colours this client paints in its own voice, resolved from and held to + /// the legibility floor against the plane they land on — see . They were + /// hexes written into the markup at each use site, each picked against a dark theme and then painted + /// on whatever plane the workspace happened to have. + /// + private readonly ChromeInk _ink; private readonly MarkupFormatter _formatter; private readonly Workspace _workspace; private readonly Dictionary _panes = new(StringComparer.Ordinal); @@ -569,6 +577,7 @@ public SharpMUTermApp( _messages = _diagnostics.Messages; _sessions.Logger = _diagnostics.For("SharpMUTerm.Session"); _theme = ResolveTheme(config); + _ink = WorkspacePalette.Chrome(_theme); _formatter = new MarkupFormatter(_theme, config.Text); _drafts = new DraftStore(() => config.Input.KeepDrafts); _secondBars = new InputBarVisibility(() => config.Input.SecondBar); @@ -4920,7 +4929,7 @@ private string ScrollbackStatus() // segment is the same width whatever it says. var below = Math.Max(1, panel.TotalContentHeight - panel.ViewportHeight - panel.VerticalScrollOffset); var distance = UnreadBadge.Format(below).PadLeft(UnreadBadge.FieldWidth); - return $"[#e5c07b]{Glyphs.Scrollback} scrollback[/] [dim]{distance} · ⌃End live[/]"; + return $"[{_ink.Notice}]{Glyphs.Scrollback} scrollback[/] [dim]{distance} · ⌃End live[/]"; } /// @@ -5806,7 +5815,8 @@ private ScreenBinding TriggersScreen() RouteTargets(), _system.DesktopDimensions.Width, session.Focus(), - _system.DesktopDimensions.Height)); + _system.DesktopDimensions.Height, + _theme)); } /// Opens the F3 Aliases screen: the alias list, then the alias's toggles. @@ -6101,10 +6111,16 @@ private static TerminalColor AccentFor(WorldDefinition world, int index) => } /// Renders a as a #rrggbb markup colour. - private static string AccentHex(TerminalColor accent) => + /// + /// A world's own accent as a markup hex, or the app's when it has none. Held to the legibility floor + /// against the plane the status line and the rail are painted on — a world's accent is a colour a + /// user picked in F5, quite possibly against a different theme from the one they are reading in, and + /// this is the only place it meets the plane it lands on. + /// + private string AccentHex(TerminalColor accent) => accent.Kind == TerminalColorKind.Rgb - ? $"#{accent.R:x2}{accent.G:x2}{accent.B:x2}" - : "#00f5b7"; + ? Contrast.Legible(new Rgb(accent.R, accent.G, accent.B), WorkspacePalette.ChromePlane(_theme)).ToHex() + : _ink.Accent; /// /// Projects live config + workspace state into rail rows: each world (with an accent), its @@ -7838,8 +7854,8 @@ private List RenderRailLines() { var rows = BuildRail(); return _railCollapsed - ? RailRenderer.RenderCollapsed(rows) - : RailRenderer.Render(rows, RailMaxWidth - RailMargin); + ? RailRenderer.RenderCollapsed(rows, _ink) + : RailRenderer.Render(rows, RailMaxWidth - RailMargin, _ink); } /// The narrowest the expanded sidebar goes, so a sparse rail still reads as a column. @@ -8159,11 +8175,17 @@ private MarkupControl FrozenContentFor(string windowId) } /// The frozen-split chrome colour (design token #c678dd / ANSI 5), resolved through the theme. - private string FrozenAccentHex() - { - var rgb = _theme.ResolveIndex(5); - return $"#{rgb.R:x2}{rgb.G:x2}{rgb.B:x2}"; - } + /// + /// The accent the three boundary bars — ▲ FROZEN, the away bar, the restore bar — are drawn in. + /// + /// It is the theme's own index 5 held to the legibility floor against the pane it lands on, + /// and the floor is the whole of a reported defect. Raw, on the default dark theme, that index is + /// #800080 against a #36363d focused pane: 1.27:1, which is a bar the reader was + /// told about and could not see. Keeping the theme's index rather than a hue of our own is what lets + /// a theme that overrides the base sixteen (Solarized does) contribute its own violet. + /// + /// + private string FrozenAccentHex() => _ink.Marker; /// Rebuilds the pane area from the model and swaps it into the live window. private void RebuildPaneArea() @@ -9195,7 +9217,7 @@ private void ExitMoveMode(bool commit) private string MovePromptMarkup() { var name = _moveWindowId is { } id && _workspace.FindWindow(id) is { } w ? Escape(w.Title) : "window"; - return $"[#e5c07b]MOVE[/] [bold]{name}[/] [dim]→[/] [#00f5b7]{DropLabel(_moveTargetPaneId, _moveEdge)}[/]" + return $"[{_ink.Notice}]MOVE[/] [bold]{name}[/] [dim]→[/] [{_ink.Accent}]{DropLabel(_moveTargetPaneId, _moveEdge)}[/]" + " [dim]1–9 pane · ←↑↓→ edge · ⏎ commit · Esc cancel[/]"; } @@ -9498,7 +9520,7 @@ private void EndDrag() private string DragPromptMarkup(string? windowId, string? targetPaneId, Edge? edge) { var name = windowId is { } id && _workspace.FindWindow(id) is { } window ? Escape(window.Title) : "window"; - return $"[#e5c07b]DRAG[/] [bold]{name}[/] [dim]→[/] [{PaneDropRenderer.ZoneColor}]{DropLabel(targetPaneId, edge)}[/]" + return $"[{_ink.Notice}]DRAG[/] [bold]{name}[/] [dim]→[/] [{_ink.Accent}]{DropLabel(targetPaneId, edge)}[/]" + " [dim]release to drop · Esc cancel[/]"; } @@ -9537,7 +9559,7 @@ private void OnUiThread(Action action) private IWindowControl BuildMovePane(PaneNode pane, int ordinal) { var selected = pane.Id == _moveTargetPaneId; - var color = selected ? "#00f5b7" : "#e5c07b"; + var color = selected ? _ink.Accent : _ink.Notice; var lines = new List { string.Empty, string.Empty }; lines.Add($" [bold {color}]▛▀▀▜[/]"); lines.Add($" [bold {color}]▌ {ordinal} ▐[/]"); @@ -9545,7 +9567,7 @@ private IWindowControl BuildMovePane(PaneNode pane, int ordinal) lines.Add(string.Empty); if (selected) { - lines.Add($" [{PaneDropRenderer.ZoneColor}]{DropLabel(pane.Id, _moveEdge)}[/]"); + lines.Add($" [{_ink.Accent}]{DropLabel(pane.Id, _moveEdge)}[/]"); lines.Add(string.Empty); } @@ -10193,7 +10215,7 @@ private void Notice(string text, MessageSeverity severity = MessageSeverity.Warn var body = severity == MessageSeverity.Error ? $"[{ScreenPalette.Warn}]{Escape(text)}[/]" : $"[dim]{Escape(text)}[/]"; - var markup = key is null ? body : $"[#e5c07b]{Escape(key)}[/] {body}"; + var markup = key is null ? body : $"[{_ink.Notice}]{Escape(key)}[/] {body}"; _notice = markup; PaintStatus(markup); @@ -10311,7 +10333,7 @@ private string HeaderMarkup() if (_prefixArmed) { var room = HeaderWidth() - MarkupWidth(leftBar) - 2; - return $"{leftBar} {PrefixPanel.Strip(room)}"; + return $"{leftBar} {PrefixPanel.Strip(room, _ink)}"; } // Both halves count characters — see ConnectedCharacters for why that is the unit and what it used @@ -10330,7 +10352,7 @@ private string HeaderMarkup() var logFormat = HeaderLogFormat(); var log = logFormat == LogFormat.None ? $"[dim]{Glyphs.Log} LOG off[/]" - : $"[#00f5b7]{Glyphs.Log}[/] [dim]LOG {logFormat.ToString().ToLowerInvariant()}[/]"; + : $"[{_ink.Accent}]{Glyphs.Log}[/] [dim]LOG {logFormat.ToString().ToLowerInvariant()}[/]"; // No graphics readout here. Which protocol the probe settled on is decided once at startup and // never changes, so a permanent cell of chrome spends the row's scarcest resource on a fact that // cannot become news — and it was already said twice elsewhere: the session prints @@ -10437,7 +10459,7 @@ private string[] FocusHints() private string StatusBarMarkup(string character, string state) { - var accent = ActiveWorld() is { } world ? AccentHex(world.Accent) : "#00f5b7"; + var accent = ActiveWorld() is { } world ? AccentHex(world.Accent) : _ink.Accent; var left = $"[{accent}]●[/] [bold]{Escape(character)}[/] [dim]{Escape(state)}[/]"; var right = new List(); diff --git a/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs b/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs index f5f5c25..db7ca3f 100644 --- a/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs @@ -1,3 +1,4 @@ +using SharpMUTerm.Core.Theming; using SharpMUTerm.Core.Automation; using SharpMUTerm.Core.Configuration; using SharpMUTerm.Core.Text; @@ -522,7 +523,8 @@ internal static List EditorColumn( IReadOnlyList routeTargets, ScreenFocus? focus = null, int width = ColumnWidth, - int height = 0) + int height = 0, + Theme? theme = null) { ArgumentNullException.ThrowIfNull(sets); ArgumentNullException.ThrowIfNull(routeTargets); @@ -537,7 +539,8 @@ internal static List EditorColumn( cursor, selectedTrigger, width, - height) + height, + theme is null ? null : WorkspacePalette.ReadingPlane(theme)) : new List(); } @@ -615,7 +618,8 @@ private static List BuildEditor( ScreenFocus cursor, int index, int width = ColumnWidth, - int height = 0) + int height = 0, + Rgb? plane = null) { var name = cursor.EditOn(0, index, NameField); var set = cursor.EditOn(0, index, SetField); @@ -681,8 +685,8 @@ private static List BuildEditor( // caption only knew about colours it flatly lied about a bold-only rule. lines.Add(string.Empty); lines.Add(Heading("highlight", foreground ?? background ?? attributes, HighlightCaption(fg, bg, attrs))); - lines.Add(HighlightRow("fg", fg, foreground)); - lines.Add(HighlightRow("bg", bg, background)); + lines.Add(HighlightRow("fg", fg, foreground, plane)); + lines.Add(HighlightRow("bg", bg, background, plane)); lines.Add(AttributeRow(attrs, attributes)); lines.AddRange(AttributeLegend(attributes?.Text ?? ScreenField.FormatFlags(attrs))); @@ -763,14 +767,41 @@ private static string HighlightCaption( /// buffer and caret here. An unset colour gets a hollow swatch rather than none at all, so the row /// is visibly a place a colour goes. /// - private static string HighlightRow(string label, TerminalColor? colour, ScreenFieldEdit? edit) + /// + /// is the pane plane this colour will actually be painted on, when the + /// caller knows the theme. The swatch is drawn in the colour as the pane will show it — + /// through the same legibility floor MarkupFormatter applies — because a picker that shows + /// one colour while the output shows another is a picker that lies, and this screen has exactly one + /// job. Null keeps the raw colour, which is what a unit test with no theme wants. + /// + private static string HighlightRow( + string label, TerminalColor? colour, ScreenFieldEdit? edit, Rgb? plane) { - var swatch = colour is { } set ? $"[{ScreenColours.Hex(set, Accent)}]████[/]" : $"[{Rule}]░░░░[/]"; + var swatch = colour is { } set ? $"[{Painted(set, plane)}]████[/]" : $"[{Rule}]░░░░[/]"; var name = ScreenColours.Format(colour); var display = colour is null ? $"[dim]{name}[/]" : $"[{Value}]{Escape(name)}[/]"; return $" {swatch} {PadVisible(label, HighlightLabelWidth)} {ScreenChrome.Field(display, edit)}"; } + /// + /// A highlight colour's markup hex as the output pane will paint it: the colour resolved, then held + /// to against . + /// + private static string Painted(TerminalColor colour, Rgb? plane) + { + var hex = ScreenColours.Hex(colour, Accent); + if (plane is not { } surface || !hex.StartsWith('#')) + { + return hex; + } + + var rgb = new Rgb( + Convert.ToByte(hex.Substring(1, 2), 16), + Convert.ToByte(hex.Substring(3, 2), 16), + Convert.ToByte(hex.Substring(5, 2), 16)); + return Contrast.Legible(rgb, surface).ToHex(); + } + /// /// The attributes value, drawn in the same swatch label value shape as the two colours it /// sits under — one field well, because it is one setting, however many booleans it packs. Its diff --git a/src/SharpMUTerm.Tui/TriggersScreenView.cs b/src/SharpMUTerm.Tui/TriggersScreenView.cs index 90cb703..14e3871 100644 --- a/src/SharpMUTerm.Tui/TriggersScreenView.cs +++ b/src/SharpMUTerm.Tui/TriggersScreenView.cs @@ -1,3 +1,4 @@ +using SharpMUTerm.Core.Theming; using SharpMUTerm.Core.Configuration; using SharpConsoleUI; using SharpConsoleUI.Builders; @@ -21,7 +22,8 @@ public static IWindowControl Build( IReadOnlyList routeTargets, int width, ScreenFocus? focus = null, - int height = 0) + int height = 0, + Theme? theme = null) { var header = ScreenChrome.Band( TriggersScreenRenderer.HeaderLine( @@ -41,7 +43,8 @@ public static IWindowControl Build( var body = ScreenChrome.Rows(height); var left = TriggersScreenRenderer.RulesColumn(sets, selectedTrigger, focus, rules); var right = TriggersScreenRenderer.EditorColumn( - sets, selectedTrigger, routeTargets, focus, width <= 0 ? rules : width - rules - ScreenChrome.ColumnDivider, body); + sets, selectedTrigger, routeTargets, focus, + width <= 0 ? rules : width - rules - ScreenChrome.ColumnDivider, body, theme); var rulesCol = ScreenChrome.Stretch(new MarkupControl(ScreenChrome.Window(left, body))); var editorCol = ScreenChrome.Stretch(new MarkupControl( diff --git a/src/SharpMUTerm.Tui/WorkspacePalette.cs b/src/SharpMUTerm.Tui/WorkspacePalette.cs index d315e4d..59a83c9 100644 --- a/src/SharpMUTerm.Tui/WorkspacePalette.cs +++ b/src/SharpMUTerm.Tui/WorkspacePalette.cs @@ -361,6 +361,98 @@ internal static Rgb IdleInk(Theme theme) /// The one-cell hairline a split draws between two panes, and beside the rail. internal static Rgb Rule(Theme theme) => Mix(Surface(theme), theme.Border, RuleLift); + /// + /// Every plane a pane's output can be painted on: the untinted surface and all six tints, each + /// focused and not. Fourteen colours, and what matters about them is that they form a band + /// — the whole set sits on one side of mid-scale, because every one of them is one theme background + /// put through a darkening and a brightening. + /// + private static IEnumerable PanePlanes(Theme theme) + { + foreach (var tint in Enum.GetValues()) + { + var plane = Tint(theme, tint); + yield return plane; + yield return Focus(plane); + } + } + + /// + /// The one plane a foreground has to clear for it to be legible on every pane — the extreme + /// of in the direction a foreground on this theme is moved. + /// + /// It is the worst case rather than an approximation of one. A lift pushes a foreground away + /// from the band; once it is past the band the contrast ratio is monotone in the background's + /// luminance, so the plane hardest to clear is the one furthest in the direction of travel — the + /// brightest on a dark theme, the darkest on a light one. Clearing it clears all fourteen. + /// + /// + /// It is per theme, and that is the whole reason it exists. A pane's actual plane depends on + /// its character's tint and on whether it holds focus, and resolving a colour against that + /// would mean re-formatting a whole buffer on every focus move — the expensive whole-buffer path + /// this codebase reserves for one deliberate keystroke. One plane per theme means a lifted colour is + /// decided once, when the line is formatted, and never revisited. + /// + /// + internal static Rgb ReadingPlane(Theme theme) + { + ArgumentNullException.ThrowIfNull(theme); + return Extreme(PanePlanes(theme), Surface(theme)); + } + + /// + /// The same worst case for the colours the client paints in its own voice, which land on + /// the — the status line, the rail — as well as on panes. It is + /// 's band with the backdrop added, so one ink is legible wherever the + /// chrome puts it: a status-line segment and a pane overlay must not need two different teals. + /// + internal static Rgb ChromePlane(Theme theme) + { + ArgumentNullException.ThrowIfNull(theme); + return Extreme(PanePlanes(theme).Append(Backdrop(theme)), Surface(theme)); + } + + /// + /// The member of hardest for a foreground to clear, given that the + /// direction of travel is decided by : brightest when the theme is dark, + /// darkest when it is light. + /// + private static Rgb Extreme(IEnumerable planes, Rgb reference) => + Contrast.RelativeLuminance(reference) < LightPlaneLuminance + ? planes.MaxBy(Contrast.RelativeLuminance) + : planes.MinBy(Contrast.RelativeLuminance); + + /// + /// Mid-scale in relative luminance — 0.18, the sRGB middle grey. A theme whose planes sit below it + /// is dark and its text is lifted; above it and the text is darkened. Measured in luminance and not + /// in bytes because #808080 looks like half way when written down and is not: its relative + /// luminance is 0.216, and a byte pivot would call it dark and push text toward it. + /// + private const double LightPlaneLuminance = 0.18; + + /// + /// The colours this client paints in its own voice, resolved against the theme and held to + /// on the plane they land on. See for what each + /// one is for, and what it measured before it was measured against anything. + /// + internal static ChromeInk Chrome(Theme theme) + { + ArgumentNullException.ThrowIfNull(theme); + + var plane = ChromePlane(theme); + + // The marker's hue is the theme's own index 5, so a theme that overrides the base palette + // (Solarized does) contributes its violet rather than xterm's. What is *not* the theme's to + // decide is whether that colour can be read: on the default dark theme index 5 is #800080 + // against a #36363d pane, which is 1.27:1 — the reported "freeze is purple on a blue + // background", and very nearly the same colour twice. + return new ChromeInk( + Contrast.Legible(ChromeInk.BaseAccent, plane).ToHex(), + Contrast.Legible(ChromeInk.BaseNotice, plane).ToHex(), + Contrast.Legible(ChromeInk.BaseDraft, plane).ToHex(), + Contrast.Legible(theme.ResolveIndex(5), plane).ToHex()); + } + /// Linear blend of two colours, of the way from to . private static Rgb Mix(Rgb from, Rgb to, double t) => new( Channel(from.R + ((to.R - from.R) * t)), diff --git a/tests/SharpMUTerm.Core.Tests/ContrastTests.cs b/tests/SharpMUTerm.Core.Tests/ContrastTests.cs new file mode 100644 index 0000000..c19da04 --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/ContrastTests.cs @@ -0,0 +1,159 @@ +using SharpMUTerm.Core.Text; + +namespace SharpMUTerm.Core.Tests; + +/// +/// The legibility floor: is what stops this client painting a colour +/// nobody can read on the plane it lands on. The reported defect these pin is the freeze bar's +/// #800080 on a focused dark pane — 1.27:1, which is very nearly the same colour twice. +/// +public class ContrastTests +{ + /// The brightest plane a pane can wear on the default dark theme — §2's worst case. + private static readonly Rgb DarkPane = new(0x36, 0x36, 0x3d); + + private static readonly Rgb LightPane = new(0xf2, 0xf2, 0xf5); + + private const double Floor = Contrast.Floor; + + [Test] + public async Task WhiteOnBlackIsTheDefinitionsOwnMaximum() + { + // 21:1 is the ratio the WCAG definition tops out at; getting it exactly is the cheapest + // evidence that the luminance curve here is the standard one and not an approximation of it. + var ratio = Contrast.Ratio(new Rgb(0xff, 0xff, 0xff), new Rgb(0, 0, 0)); + + await Assert.That(ratio).IsBetween(20.99, 21.01); + } + + [Test] + public async Task ARatioIsTheSameWhicheverWayRoundItIsAsked() + { + var a = new Rgb(0x12, 0x34, 0x56); + var b = new Rgb(0xcd, 0xef, 0x01); + + await Assert.That(Contrast.Ratio(a, b)).IsEqualTo(Contrast.Ratio(b, a)).Within(1e-9); + } + + [Test] + public async Task TheFreezeBarsOwnColourIsBelowTheFloorAndComesBackAboveIt() + { + // The reported defect, as a number: ANSI 5 on the plane the bar is actually drawn on. + var magenta = AnsiPalette.ToRgb(5); + await Assert.That(Contrast.Ratio(magenta, DarkPane)).IsLessThan(1.3); + + var lifted = Contrast.Legible(magenta, DarkPane, Floor); + + await Assert.That(Contrast.Ratio(lifted, DarkPane)).IsGreaterThanOrEqualTo(Floor); + } + + [Test] + public async Task AColourThatAlreadyClearsTheFloorIsHandedBackUntouched() + { + // Not "close enough": byte-identical, so turning the floor on cannot quietly restyle text that + // was already fine — which is most of what a game sends. + var gold = new Rgb(0xff, 0xd7, 0x00); + + await Assert.That(Contrast.Legible(gold, DarkPane, Floor)).IsEqualTo(gold); + } + + [Test] + public async Task LiftingIsIdempotent() + { + var once = Contrast.Legible(new Rgb(0x00, 0x00, 0x80), DarkPane, Floor); + var twice = Contrast.Legible(once, DarkPane, Floor); + + await Assert.That(twice).IsEqualTo(once); + } + + [Test] + public async Task ADarkPlaneLiftsAndALightPlaneDarkens() + { + // The direction is the plane's, not the colour's — one function for every theme. + var navy = new Rgb(0x00, 0x00, 0x80); + var gold = new Rgb(0xff, 0xd7, 0x00); + + var onDark = Contrast.Legible(navy, DarkPane, Floor); + var onLight = Contrast.Legible(gold, LightPane, Floor); + + await Assert.That(Contrast.RelativeLuminance(onDark)) + .IsGreaterThan(Contrast.RelativeLuminance(navy)); + await Assert.That(Contrast.RelativeLuminance(onLight)) + .IsLessThan(Contrast.RelativeLuminance(gold)); + } + + [Test] + public async Task TheLiftStopsAtTheFloorRatherThanRunningToWhite() + { + // A floor, not a wash: a colour lifted past what it needed has thrown away the hue it was + // carrying for no gain. Half a point of slack is the search's own resolution. + var lifted = Contrast.Legible(new Rgb(0x80, 0x00, 0x80), DarkPane, Floor); + var ratio = Contrast.Ratio(lifted, DarkPane); + + await Assert.That(ratio).IsGreaterThanOrEqualTo(Floor); + await Assert.That(ratio).IsLessThan(Floor + 0.5); + } + + [Test] + public async Task HueSurvivesTheLiftWhileThereIsHeadroomForIt() + { + // Blending toward white keeps the channel *order*, which is what makes a lifted colour still + // recognisably the colour the server sent. Red stays reddest; blue stays bluest. + var maroon = new Rgb(0x80, 0x00, 0x00); + var lifted = Contrast.Legible(maroon, DarkPane, Floor); + + await Assert.That(lifted.R).IsGreaterThan(lifted.G); + await Assert.That(lifted.R).IsGreaterThan(lifted.B); + } + + [Test] + public async Task APureBlueDesaturatesBecauseHueAloneCannotReachTheFloor() + { + // #0000ff has a relative luminance of 0.0722 and so tops out at 1.88:1 on a dark pane at full + // blue. A rule that held hue absolutely would leave it unreadable, which is the defect. It has + // to bring the other channels up — and this is the test that says so out loud, so nobody + // "fixes" the desaturation later. + var blue = new Rgb(0x00, 0x00, 0xff); + + var lifted = Contrast.Legible(blue, DarkPane, Floor); + + await Assert.That(Contrast.Ratio(lifted, DarkPane)).IsGreaterThanOrEqualTo(Floor); + await Assert.That(lifted.B).IsGreaterThan(lifted.R); + await Assert.That(lifted.R).IsGreaterThan((byte)0); + } + + [Test] + public async Task BlackOnBlackReachesTheFloorToo() + { + // The degenerate case: no hue to preserve at all, so the lift is pure greying-up. It has to + // terminate rather than divide by a zero luminance. + var lifted = Contrast.Legible(new Rgb(0, 0, 0), new Rgb(0, 0, 0), Floor); + + await Assert.That(Contrast.Ratio(lifted, new Rgb(0, 0, 0))).IsGreaterThanOrEqualTo(Floor); + } + + [Test] + public async Task AFloorThatCannotBeReachedGivesTheFurthestColourRatherThanThrowing() + { + // 21:1 against anything but pure black is arithmetically impossible. The contract is best + // effort, because the caller is a render path fed from the telnet read loop and there is + // nothing useful it could do with a throw. + var plane = new Rgb(0x40, 0x40, 0x40); + + var lifted = Contrast.Legible(new Rgb(0x20, 0x20, 0x20), plane, 21.0); + + await Assert.That(lifted).IsEqualTo(new Rgb(0xff, 0xff, 0xff)); + } + + [Test] + public async Task ThePivotIsLuminanceAndNotTheByteValue() + { + // #808080 looks like "half way" written in bytes and is *not*: its relative luminance is 0.216, + // comfortably above the 0.18 mid-scale, so it is a light plane and a foreground on it darkens. + // Pivoting on the bytes instead would call it dark and lift text *toward* it. + var lifted = Contrast.Legible(new Rgb(0x7a, 0x7a, 0x7a), new Rgb(0x80, 0x80, 0x80), Floor); + + await Assert.That(Contrast.RelativeLuminance(lifted)) + .IsLessThan(Contrast.RelativeLuminance(new Rgb(0x7a, 0x7a, 0x7a))); + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/LegiblePaletteTests.cs b/tests/SharpMUTerm.Tui.Tests/LegiblePaletteTests.cs new file mode 100644 index 0000000..1770c14 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/LegiblePaletteTests.cs @@ -0,0 +1,192 @@ +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Text; +using SharpMUTerm.Core.Theming; +using SharpMUTerm.Tui; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// The floor, asserted over the whole cross product it has to hold on: every colour this client can +/// paint × every theme × every plane a pane can wear. +/// +/// This is the test that would have caught the reported defect, and the reason it is a table +/// rather than a case per bug is that the bugs were never individually interesting. The freeze bar's +/// purple was one cell of a grid in which six of sixteen picker names fail on the dark theme, nine of +/// sixteen fail on the light one, and only grey clears 3:1 on both — a palette of fixed hexes +/// cannot serve two themes, and nothing short of a table says so. +/// +/// +public class LegiblePaletteTests +{ + private static IEnumerable Themes() => + ThemeLibrary.Names.Select(ThemeLibrary.Get); + + /// + /// Every plane a pane can be painted on: untinted and all six character tints, focused and not. + /// Rebuilt here from the public surface rather than reached into, so this test measures what the + /// workspace actually paints. + /// + private static IEnumerable<(string Name, Rgb Plane)> Planes(Theme theme) + { + foreach (var tint in Enum.GetValues()) + { + var plane = WorkspacePalette.Tint(theme, tint); + yield return ($"{tint}", plane); + yield return ($"{tint}+focus", WorkspacePalette.Focus(plane)); + } + + yield return ("backdrop", WorkspacePalette.Backdrop(theme)); + } + + [Test] + public async Task EveryPickerColourIsLegibleOnEveryPlaneOfEveryTheme() + { + var failures = new List(); + + foreach (var theme in Themes()) + { + var plane = WorkspacePalette.ReadingPlane(theme); + foreach (var name in ScreenColours.Palette.Where(n => n != ScreenColours.None)) + { + if (!WebColors.TryParse(name, out var colour)) + { + failures.Add($"{name} does not resolve"); + continue; + } + + var painted = Contrast.Legible(new Rgb(colour.R, colour.G, colour.B), plane); + + // Panes only. A highlight colours a line of a world's output and a world's output is + // never drawn on the backdrop, so measuring it there would hold the picker to a plane it + // cannot land on — and on the Light theme the backdrop is darker than every pane, so it + // is the one that would fail. + foreach (var (planeName, actual) in Planes(theme).Where(p => p.Name != "backdrop")) + { + var ratio = Contrast.Ratio(painted, actual); + if (ratio < Contrast.Floor) + { + failures.Add($"{theme.Name}/{name} on {planeName}: {ratio:0.00}"); + } + } + } + } + + await Assert.That(failures).IsEmpty(); + } + + [Test] + public async Task EveryBasePaletteIndexIsLegibleOnEveryPlaneOfEveryTheme() + { + // 0–15: what a MU* server actually sends. The six-cube and the greyscale ramp go through the + // same lift and are not enumerated — 256 × 3 × 15 assertions would buy one more decimal place + // of the same fact, and these sixteen are the ones a game reaches for. + var failures = new List(); + + foreach (var theme in Themes()) + { + var reading = WorkspacePalette.ReadingPlane(theme); + for (var index = 0; index < 16; index++) + { + var painted = Contrast.Legible(theme.ResolveIndex(index), reading); + foreach (var (planeName, plane) in Planes(theme).Where(p => p.Name != "backdrop")) + { + var ratio = Contrast.Ratio(painted, plane); + if (ratio < Contrast.Floor) + { + failures.Add($"{theme.Name}/idx:{index} on {planeName}: {ratio:0.00}"); + } + } + } + } + + await Assert.That(failures).IsEmpty(); + } + + [Test] + public async Task EveryChromeInkIsLegibleWhereverTheChromePutsIt() + { + // Including the backdrop, which is what separates ChromePlane from ReadingPlane: these four are + // painted on the status line and the rail as well as into panes, and one ink has to be readable + // in both places or the client needs two teals. + var failures = new List(); + + foreach (var theme in Themes()) + { + var ink = WorkspacePalette.Chrome(theme); + foreach (var (label, hex) in new[] + { + ("accent", ink.Accent), ("notice", ink.Notice), + ("draft", ink.Draft), ("marker", ink.Marker), + }) + { + var colour = ParseHex(hex); + foreach (var (planeName, plane) in Planes(theme)) + { + var ratio = Contrast.Ratio(colour, plane); + if (ratio < Contrast.Floor) + { + failures.Add($"{theme.Name}/{label} on {planeName}: {ratio:0.00}"); + } + } + } + } + + await Assert.That(failures).IsEmpty(); + } + + [Test] + public async Task TheReportedFreezeBarColourFailedThisTestBeforeItWasFixed() + { + // The defect as it was reported, kept as its own case so the number stays in the repository: the + // bar took its accent straight from the theme's index 5, and on the default dark theme's focused + // pane that is #800080 on #36363d. + var theme = ThemeLibrary.Dark(); + var focusedPane = WorkspacePalette.Focus(WorkspacePalette.Surface(theme)); + + await Assert.That(Contrast.Ratio(theme.ResolveIndex(5), focusedPane)).IsLessThan(1.3); + await Assert.That(Contrast.Ratio(ParseHex(WorkspacePalette.Chrome(theme).Marker), focusedPane)) + .IsGreaterThanOrEqualTo(Contrast.Floor); + } + + [Test] + public async Task TheLightThemesChromeWasUnreadableToo() + { + // Not a rider on the freeze fix but the same defect on the other side: every one of the client's + // own hexes was picked against a dark theme, and no snapshot showed it because every frame in + // the gallery renders Dark. + var theme = ThemeLibrary.Get("Light"); + var plane = WorkspacePalette.ChromePlane(theme); + + await Assert.That(Contrast.Ratio(ChromeInk.BaseAccent, plane)).IsLessThan(Contrast.Floor); + await Assert.That(Contrast.Ratio(ChromeInk.BaseDraft, plane)).IsLessThan(Contrast.Floor); + await Assert.That(Contrast.Ratio(ChromeInk.BaseNotice, plane)).IsLessThan(Contrast.Floor); + + var ink = WorkspacePalette.Chrome(theme); + await Assert.That(Contrast.Ratio(ParseHex(ink.Accent), plane)).IsGreaterThanOrEqualTo(Contrast.Floor); + await Assert.That(Contrast.Ratio(ParseHex(ink.Draft), plane)).IsGreaterThanOrEqualTo(Contrast.Floor); + await Assert.That(Contrast.Ratio(ParseHex(ink.Notice), plane)).IsGreaterThanOrEqualTo(Contrast.Floor); + } + + [Test] + public async Task TheReadingPlaneIsTheWorstOfTheBandAndNotAMemberPickedByName() + { + // The whole argument for one plane per theme is that clearing *it* clears all fourteen. If the + // extreme were mis-chosen the table tests above would still pass on most cells and fail on the + // one that matters, so the property is asserted directly. + foreach (var theme in Themes()) + { + var reading = WorkspacePalette.ReadingPlane(theme); + var luminance = Contrast.RelativeLuminance(reading); + var band = Planes(theme).Where(p => p.Name != "backdrop") + .Select(p => Contrast.RelativeLuminance(p.Plane)).ToList(); + + // Dark theme: the brightest plane. Light theme: the darkest. Either way, an extreme. + await Assert.That(luminance == band.Max() || luminance == band.Min()).IsTrue(); + } + } + + private static Rgb ParseHex(string hex) => new( + Convert.ToByte(hex.Substring(1, 2), 16), + Convert.ToByte(hex.Substring(3, 2), 16), + Convert.ToByte(hex.Substring(5, 2), 16)); +} diff --git a/tests/SharpMUTerm.Tui.Tests/MarkupFormatterTests.cs b/tests/SharpMUTerm.Tui.Tests/MarkupFormatterTests.cs index c656469..d373a09 100644 --- a/tests/SharpMUTerm.Tui.Tests/MarkupFormatterTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/MarkupFormatterTests.cs @@ -73,8 +73,11 @@ public async Task NullOrEmptyTimestamp_AddsNoGutter() [Test] public async Task BoldItalic_EmitsAttributeTokens() { + // Gold rather than red: pure #ff0000 measures 2.998:1 against this theme's reading plane, a + // hair under the floor, so it comes out lifted by a single step — correct, and a distraction in + // a test about attribute tokens. TheLegibilityFloor* tests below are where that is the subject. var style = new TextStyle( - TerminalColor.FromRgb(255, 0, 0), + TerminalColor.FromRgb(0xff, 0xd7, 0x00), TerminalColor.Default, TextAttributes.Bold | TextAttributes.Italic); var line = StyledLine.FromText("x", style); @@ -83,7 +86,74 @@ public async Task BoldItalic_EmitsAttributeTokens() await Assert.That(markup).Contains("bold"); await Assert.That(markup).Contains("italic"); - await Assert.That(markup).Contains("#ff0000"); + await Assert.That(markup).Contains("#ffd700"); + } + + [Test] + public async Task TheLegibilityFloorLiftsAColourThatCannotBeReadOnThePane() + { + // ANSI 4 on the default dark theme is #000080 against a #36363d focused pane: 1.34:1, which is + // most of what "unreadable colours against our backgrounds" was about. MU* servers send it + // constantly, because they are written for black terminals and this one is not black. + var line = StyledLine.FromText("x", new TextStyle( + TerminalColor.FromIndex(4), TerminalColor.Default, TextAttributes.None)); + + var markup = new MarkupFormatter(ThemeLibrary.Dark()).ToMarkup(line); + + await Assert.That(markup).DoesNotContain("#000080"); + await Assert.That(Contrast.Ratio( + Parse(markup), WorkspacePalette.ReadingPlane(ThemeLibrary.Dark()))) + .IsGreaterThanOrEqualTo(Contrast.Floor); + } + + [Test] + public async Task TheLegibilityFloorLeavesAColourThatAlreadyReadsAlone() + { + // Byte-identical, not merely close: the floor must not restyle text that was already fine, which + // is most of what any game sends. + var line = StyledLine.FromText("x", new TextStyle( + TerminalColor.FromRgb(0xff, 0xd7, 0x00), TerminalColor.Default, TextAttributes.None)); + + await Assert.That(new MarkupFormatter(ThemeLibrary.Dark()).ToMarkup(line)).Contains("#ffd700"); + } + + [Test] + public async Task AHighlightIsMeasuredAgainstItsOwnBackgroundAndNotThePane() + { + // A span carrying a background is painted on *that*, so the pane it happens to be in says + // nothing about whether it can be read. Dark blue on white is 14.3:1 and must survive untouched; + // measured against the pane instead it would be lifted to something unreadable on its own band. + var line = StyledLine.FromText("x", new TextStyle( + TerminalColor.FromRgb(0x00, 0x00, 0x80), + TerminalColor.FromRgb(0xff, 0xff, 0xff), + TextAttributes.None)); + + var markup = new MarkupFormatter(ThemeLibrary.Dark()).ToMarkup(line); + + await Assert.That(markup).Contains("#000080 on #ffffff"); + } + + [Test] + public async Task TheFloorCanBeSwitchedOffAndThenTheBytesAreExactlyWhatTheyWere() + { + var line = StyledLine.FromText("x", new TextStyle( + TerminalColor.FromIndex(4), TerminalColor.Default, TextAttributes.None)); + + var markup = new MarkupFormatter(ThemeLibrary.Dark(), new TextSettings { KeepTextLegible = false }) + .ToMarkup(line); + + await Assert.That(markup).Contains("#000080"); + } + + /// The first #rrggbb in a markup string, as a colour. + private static Rgb Parse(string markup) + { + var at = markup.IndexOf('#', StringComparison.Ordinal); + var hex = markup.Substring(at + 1, 6); + return new Rgb( + Convert.ToByte(hex[..2], 16), + Convert.ToByte(hex[2..4], 16), + Convert.ToByte(hex[4..], 16)); } [Test] diff --git a/tests/SharpMUTerm.Tui.Tests/PaneDropRendererTests.cs b/tests/SharpMUTerm.Tui.Tests/PaneDropRendererTests.cs index 0785810..5883d1c 100644 --- a/tests/SharpMUTerm.Tui.Tests/PaneDropRendererTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/PaneDropRendererTests.cs @@ -49,8 +49,8 @@ public async Task OnlyAHoveredPaneIsHighlighted() { var idle = string.Concat(PaneDropRenderer.Render("main", "x", 40, 20, hovered: false, edge: Edge.Left)); - await Assert.That(idle).DoesNotContain(PaneDropRenderer.ZoneColor); - await Assert.That(string.Concat(Hovered(Edge.Left))).Contains(PaneDropRenderer.ZoneColor); + await Assert.That(idle).DoesNotContain(ChromeInk.Default.Accent); + await Assert.That(string.Concat(Hovered(Edge.Left))).Contains(ChromeInk.Default.Accent); } [Test] diff --git a/tests/SharpMUTerm.Tui.Tests/ScreenCursorTests.cs b/tests/SharpMUTerm.Tui.Tests/ScreenCursorTests.cs index 1d0132d..b0cb1f4 100644 --- a/tests/SharpMUTerm.Tui.Tests/ScreenCursorTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/ScreenCursorTests.cs @@ -112,11 +112,11 @@ public async Task Keypad_BarsTheFocusedBinding() [Test] public async Task Options_BarsTheFocusedOptionAndNeverASectionHeader() { - // Navigable row 5 is "emoji substitution" — the three section headers and the two spacers are - // skipped, four colour rows come first, and "tab width (spaces)" sits at 4 between them and this + // Navigable row 6 is "emoji substitution" — the three section headers and the two spacers are + // skipped, five colour rows come first, and "tab width (spaces)" sits at 5 between them and this // one. var screen = OptionsScreenRenderer.TextAnsiScreen(); - var lines = OptionsScreenRenderer.BodyColumn(screen.Rows, new ScreenFocus(0, 5)); + var lines = OptionsScreenRenderer.BodyColumn(screen.Rows, new ScreenFocus(0, 6)); await Assert.That(Barred(lines)).IsEqualTo(1); await Assert.That(lines.Single(l => l.Contains(Bar, StringComparison.Ordinal))).Contains("emoji substitution"); diff --git a/tests/SharpMUTerm.Tui.Tests/ScreenModelTests.cs b/tests/SharpMUTerm.Tui.Tests/ScreenModelTests.cs index 008436d..7fa983f 100644 --- a/tests/SharpMUTerm.Tui.Tests/ScreenModelTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/ScreenModelTests.cs @@ -364,17 +364,18 @@ public async Task Options_NavigableRowsSkipSectionHeadersAndSpacers() var screen = OptionsScreenRenderer.TextAnsiScreen(); var model = OptionsScreenRenderer.Model(screen); - // 14 display rows: 4 section headers + 3 spacers + 7 options. It was 7/4 before WHITESPACE and + // 15 display rows: 4 section headers + 3 spacers + 8 options. It was 7/4 before WHITESPACE and // its "tab width (spaces)" row, which brought a header and a spacer with it, 10/5 before - // "detect links in output" joined the COLOUR section, and 11/6 before ACTIVITY and its - // "activity bar holds for (seconds)" row brought a header and a spacer of their own. - await Assert.That(screen.Rows.Count).IsEqualTo(14); + // "detect links in output" joined the COLOUR section, 11/6 before ACTIVITY and its + // "activity bar holds for (seconds)" row brought a header and a spacer of their own, and 14/7 + // before "keep text legible" joined COLOUR beneath the row it is the other half of. + await Assert.That(screen.Rows.Count).IsEqualTo(15); await Assert.That(model.PaneCount).IsEqualTo(1); - await Assert.That(model.Sizes[0]).IsEqualTo(7); + await Assert.That(model.Sizes[0]).IsEqualTo(8); } /// - /// F7 is five checkboxes and one count. The count is tab width (spaces), and it is what puts + /// F7 is six checkboxes and one count. The count is tab width (spaces), and it is what puts /// ⏎ edit back in this screen's header — HasEditableRow was false for as long as every /// row here was a toggle. Asserted here rather than only in the renderer test, because the header /// advertising a key the screen has no use for is exactly the rule these screens are held to. @@ -388,18 +389,24 @@ public async Task Options_TextAnsiRowsWriteBackToTheTextSettings() model.ToggleAt(0, 0)!.Value.Flip(); await Assert.That(text.StripIncomingColour).IsTrue(); - model.ToggleAt(0, 2)!.Value.Flip(); - await Assert.That(text.UnderlineHyperlinks).IsFalse(); + // 1 is "keep text legible", which sits directly under the row above because the two are the ends + // of one question's range — discard every colour the server sent, or keep them and move the few + // that cannot be read. + model.ToggleAt(0, 1)!.Value.Flip(); + await Assert.That(text.KeepTextLegible).IsFalse(); model.ToggleAt(0, 3)!.Value.Flip(); + await Assert.That(text.UnderlineHyperlinks).IsFalse(); + + model.ToggleAt(0, 4)!.Value.Flip(); await Assert.That(text.DetectLinks).IsFalse(); - model.ToggleAt(0, 5)!.Value.Flip(); + model.ToggleAt(0, 6)!.Value.Flip(); await Assert.That(text.EmojiSubstitution).IsFalse(); - // Row 4 is the tab width — a count, so it is a field rather than a toggle, and it is what makes + // Row 5 is the tab width — a count, so it is a field rather than a toggle, and it is what makes // this screen carry an editable row at all. - await Assert.That(model.ToggleAt(0, 4)).IsNull(); + await Assert.That(model.ToggleAt(0, 5)).IsNull(); await Assert.That(model.HasEditableRow).IsTrue(); } From 1a8a1793c3c380a4f6e7a585e3d5d47660f1481b Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 22:41:55 -0500 Subject: [PATCH 3/8] feat(triggers): a route that means "leave it where it is", and one that means main Four changes, no schema change and no migration. `route` gains an explicit `(none)`: the rule adds no destination and the line follows whatever the other matched rules decided. That is what SpawnTarget=null has always meant -- F2 labelled it `main`, which reads as a destination, so "highlight it and leave it where it was" looked like something the screen could not express. It is the default for a new rule. `main` becomes a real destination: the matching session's own window. It earns a reserved word rather than being spelt as the window's title because one trigger set is shared by every character that lists it, and a title can only name one of them. Gag suppresses the default delivery only. Explicit destinations survive it -- already true of a spawn pane, now true of main -- so `route: main` plus gag keeps the line where before it deleted it. Destinations are deduplicated. They were not, and the session raises one event per entry, so a highlight rule pointed at the same pane as its capture rule delivered every line twice. Also folds in five colour defects the frame audit found after the fact -- the trigger left-rule, the header chip, a world's accent on the rail, the unread badge on a tab, and the command line's ink on Solarized's armed band -- and adds FrameContrastTests, which walks every emitted SGR pair over 24 views x 3 themes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- ...gible-colour-and-trigger-routing-design.md | 26 +++ src/SharpMUTerm.Core/Automation/Trigger.cs | 33 ++- .../Automation/TriggerEngine.cs | 45 +++- src/SharpMUTerm.Core/Session/WorldSession.cs | 8 +- src/SharpMUTerm.Tui/ChromeInk.cs | 24 +- src/SharpMUTerm.Tui/MarkupFormatter.cs | 7 +- src/SharpMUTerm.Tui/Program.cs | 14 ++ src/SharpMUTerm.Tui/RailRenderer.cs | 18 +- src/SharpMUTerm.Tui/SharpMUTermApp.cs | 94 ++++++-- src/SharpMUTerm.Tui/TabTitles.cs | 7 +- src/SharpMUTerm.Tui/TriggersScreenRenderer.cs | 29 ++- src/SharpMUTerm.Tui/UnreadBadge.cs | 9 +- src/SharpMUTerm.Tui/WorkspacePalette.cs | 26 ++- .../TriggerRouteMainTests.cs | 169 ++++++++++++++ .../FrameContrastTests.cs | 192 +++++++++++++++ .../ScreenChoiceListTests.cs | 2 +- .../TabActivityIndicatorTests.cs | 22 +- tests/SharpMUTerm.Tui.Tests/TabTitlesTests.cs | 10 +- .../TriggerKeepItHereTests.cs | 218 ++++++++++++++++++ .../TriggersScreenEditingTests.cs | 25 +- 20 files changed, 903 insertions(+), 75 deletions(-) create mode 100644 tests/SharpMUTerm.Core.Tests/TriggerRouteMainTests.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs create mode 100644 tests/SharpMUTerm.Tui.Tests/TriggerKeepItHereTests.cs diff --git a/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md b/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md index 442f20b..048c1e8 100644 --- a/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md +++ b/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md @@ -167,6 +167,32 @@ a capture rule sent the line to. There is one line and one set of destinations, matched rule's highlight is on it — which is what the user's own framing asked for ("it should still follow the original route, as long as it does not change where it routes to"). +## What the frame audit found that reading the source did not + +The design above was implemented and then the *paint* was measured — every emitted SGR pair, over 24 +views × 3 themes. Five more offenders, each with a plausible-looking call site: + +| what | measured | why the source looked fine | +|---|---|---| +| the trigger left-rule `▌` | 1.42 (Light) | `MarkupFormatter` resolved it a dozen lines above the floor it applies to every other foreground | +| the header ribbon's chip | 1.53 (Light) | a fixed `#3f4859` — a *dark* chip whatever the theme, so the world accent on it was resolved for the wrong plane | +| a world's own accent on the rail | 1.03 (Light) | only the *fallback* accent had been derived; a row carrying its own RGB went through raw | +| the unread badge on a tab | 1.42 (Light) | `UnreadBadge.Tint` was a `const` pointing at `ScreenPalette.Accent` | +| the command line's ink | 2.43 (Solarized) | the band was derived and the ink on it was not — the least readable text in the client, on the theme most often chosen for comfort | + +`FrameContrastTests` is that audit as a test. It exempts three things, each for its own reason and not +because it was failing: the powerline wedges and box-drawing rules (fill boundaries and dividers, not +text), the solid blocks (F2's swatch is a colour *sample*, shown as the pane will paint it), and the +framework's `[dim]`. The half blocks are deliberately **not** exempt — `▌` is the trigger rule and the +focus marker, and one of them was a real defect this found. + +**One thing is outside the floor's reach and is named rather than hidden.** SharpConsoleUI resolves the +`[dim]` tag to a fixed `#808080` of its own, through no option we hold: 4.01:1 on the default dark theme +and **2.52:1** on Solarized Dark's focused pane. Reaching it means giving up `[dim]` across every +renderer in favour of an explicit floor-checked grey — a sweep, for a near miss on one theme. The +exemption is a named predicate with the number in it, so whoever does that sweep can delete it and watch +the test still pass. + ## Testing **Core.** diff --git a/src/SharpMUTerm.Core/Automation/Trigger.cs b/src/SharpMUTerm.Core/Automation/Trigger.cs index d11a44b..ce69e22 100644 --- a/src/SharpMUTerm.Core/Automation/Trigger.cs +++ b/src/SharpMUTerm.Core/Automation/Trigger.cs @@ -45,13 +45,38 @@ public sealed class TriggerActions public string? SendResponse { get; set; } /// - /// Route the line to a named spawn window instead of the main output; null routes to the main - /// window. Settable so the F2 screen's route-to list can re-point a rule live — the engine reads - /// it per match and keeps no routing table of its own, so a change - /// applies to the next line. + /// Where this rule delivers the line, or null when it delivers it nowhere of its own — see + /// for the three states this field has. + /// + /// Settable so the F2 screen's route-to list can re-point a rule live: the engine reads it per match + /// and keeps no routing table of its own, so a change applies to the + /// next line. Capture groups are expanded (Channel $1), which is what lets one rule feed a + /// pane per channel. + /// /// public string? SpawnTarget { get; set; } + /// + /// The reserved target naming the session's own main window — the window the line was going + /// to anyway. + /// + /// It is a destination, and null is the absence of one. Those are different things and the + /// distinction is the whole of a reported defect. A null target means the rule adds nothing and the + /// line goes wherever the other matched rules send it — which is what a highlight rule wants, and + /// what "highlight it and leave it where it was" asks for. This target means the rule delivers + /// there, which matters because suppresses only the default delivery: a + /// gagging rule routed here keeps the line in the main window, where before it deleted it, since + /// main was a label on the absence of a route rather than a route. + /// + /// + /// Reserved, and safely so. No window is titled main — a character's session window is + /// titled after the character, and main is only the rail's label for it — so this collides + /// with nothing. A configuration that already said main conjured a capture pane by that name; + /// it now reaches the window whoever wrote it meant. + /// + /// + public const string MainWindow = "main"; + /// /// Invoke this named script callback (resolved by the scripting layer). Null or empty means the /// rule calls nothing. Settable so the F2 screen can point a rule at a callback live; the engine diff --git a/src/SharpMUTerm.Core/Automation/TriggerEngine.cs b/src/SharpMUTerm.Core/Automation/TriggerEngine.cs index 8a97313..e3cdde1 100644 --- a/src/SharpMUTerm.Core/Automation/TriggerEngine.cs +++ b/src/SharpMUTerm.Core/Automation/TriggerEngine.cs @@ -15,7 +15,8 @@ public TriggerResult( IReadOnlyList responses, IReadOnlyList spawnTargets, IReadOnlyList scriptInvocations, - IReadOnlyList matched) + IReadOnlyList matched, + bool routeMain = false) { Line = line; Suppress = suppress; @@ -23,14 +24,36 @@ public TriggerResult( SpawnTargets = spawnTargets; ScriptInvocations = scriptInvocations; Matched = matched; + RouteMain = routeMain; } /// The (possibly highlighted/rewritten) line to display. public StyledLine Line { get; } - /// True if the line should be gagged (not displayed in the main window). + /// + /// True if a matched rule gagged the line — it is not delivered to the destination it would have + /// reached on its own. + /// + /// It suppresses the default delivery and nothing else. Every destination a rule + /// asked for survives it, which has always been true of a spawn pane (route: Chat + /// plus gag has always meant "only in Chat") and is now true of as well. + /// That is what "gag means only where I routed it" amounts to. + /// + /// public bool Suppress { get; } + /// + /// True when a matched rule asked for the session's own main window by name + /// () — a destination, and therefore not cancelled by + /// . + /// + /// It is kept apart from rather than being an entry in it because the + /// main window is not a capture pane and must never be looked up as one: routing to a spawn called + /// main is exactly what this replaces. + /// + /// + public bool RouteMain { get; } + /// Commands to send back to the server, in order. public IReadOnlyList Responses { get; } @@ -217,6 +240,7 @@ public TriggerResult Process(StyledLine line) var current = line; var suppress = false; + var routeMain = false; List? responses = null; List? spawns = null; List? scripts = null; @@ -288,7 +312,19 @@ actions.HighlightBackground is not null || if (!string.IsNullOrEmpty(actions.SpawnTarget) && ResolveSpawnTarget(actions.SpawnTarget, match) is { } target) { - (spawns ??= new List()).Add(target); + if (string.Equals(target, TriggerActions.MainWindow, StringComparison.OrdinalIgnoreCase)) + { + // A reserved word a user types into a field, so it is matched without regard to case. + routeMain = true; + } + else if (!(spawns ??= new List()).Contains(target, StringComparer.Ordinal)) + { + // Deduplicated, because a destination is a place and not an event: two rules naming + // one pane deliver one line. They delivered two — the list was bare and the session + // raises one event per entry — so a highlight rule pointed at the same pane as its + // capture rule doubled every line it touched. + spawns.Add(target); + } } if (!string.IsNullOrEmpty(actions.ScriptCallback)) @@ -321,7 +357,8 @@ actions.HighlightBackground is not null || (IReadOnlyList?)responses ?? Array.Empty(), (IReadOnlyList?)spawns ?? Array.Empty(), (IReadOnlyList?)scripts ?? Array.Empty(), - (IReadOnlyList?)matched ?? Array.Empty()); + (IReadOnlyList?)matched ?? Array.Empty(), + routeMain); } /// diff --git a/src/SharpMUTerm.Core/Session/WorldSession.cs b/src/SharpMUTerm.Core/Session/WorldSession.cs index 95f35dc..1904622 100644 --- a/src/SharpMUTerm.Core/Session/WorldSession.cs +++ b/src/SharpMUTerm.Core/Session/WorldSession.cs @@ -381,7 +381,13 @@ private void ProcessOutputLine(StyledLine line) _ = SendRawAsync(response); } - if (!result.Suppress) + // A gag suppresses the *default* delivery — the line arriving in this session's own window + // because nothing routed it anywhere. It does not cancel a destination a rule asked for: that + // has always been true of the spawn loop above, which runs whatever the gag says, and it is now + // true of the main window too. So `route: main` plus `gag` keeps the line here and nowhere else, + // where before it deleted the line — because `main` was the F2 screen's label for the *absence* + // of a route rather than a route. + if (result.RouteMain || !result.Suppress) { Print(shown); } diff --git a/src/SharpMUTerm.Tui/ChromeInk.cs b/src/SharpMUTerm.Tui/ChromeInk.cs index 1f78d2b..12896df 100644 --- a/src/SharpMUTerm.Tui/ChromeInk.cs +++ b/src/SharpMUTerm.Tui/ChromeInk.cs @@ -36,12 +36,18 @@ namespace SharpMUTerm.Tui; /// ⌃B strip, a which-key entry's chord. /// /// The gold pen marking a rail row whose window holds an unsent draft. +/// +/// The plane these were measured against — carried so a renderer can hold a colour it is handed +/// to the same floor. A world's own accent is the case: it is a colour a user picked in F5, quite +/// possibly against a different theme from the one they are reading in, and the rail and the header are +/// where it meets a plane. +/// /// /// The boundary bars' accent — ▲ FROZEN, the away bar, the restore bar. It is the one of the /// four that is drawn from the theme's palette (index 5) rather than from a base hue here, /// because a theme that overrides the base sixteen has an opinion about its own violet. /// -internal readonly record struct ChromeInk(string Accent, string Notice, string Draft, string Marker) +internal readonly record struct ChromeInk(string Accent, string Notice, string Draft, string Marker, Rgb Plane) { /// The app's teal accent, before a theme has said how light it needs to be. internal static readonly Rgb BaseAccent = new(0x00, 0xf5, 0xb7); @@ -62,5 +68,19 @@ internal readonly record struct ChromeInk(string Accent, string Notice, string D BaseAccent.ToHex(), BaseNotice.ToHex(), BaseDraft.ToHex(), - AnsiPalette.ToRgb(5).ToHex()); + AnsiPalette.ToRgb(5).ToHex(), + new Rgb(0x22, 0x22, 0x26)); + + /// + /// A colour this client was handed — a world's accent, a rail row's — held to the floor + /// against , as a markup hex. + /// + internal string Lift(Rgb colour) => Contrast.Legible(colour, Plane).ToHex(); + + /// + /// Text that has to be read on a known fill rather than on the plane: the ink on an accent chip, the + /// glyph on the header's character segment. Measured against , because that + /// is what it lands on and the plane says nothing about it. + /// + internal static string On(Rgb ink, Rgb fill) => Contrast.Legible(ink, fill).ToHex(); } diff --git a/src/SharpMUTerm.Tui/MarkupFormatter.cs b/src/SharpMUTerm.Tui/MarkupFormatter.cs index 6cdb46f..15de3bc 100644 --- a/src/SharpMUTerm.Tui/MarkupFormatter.cs +++ b/src/SharpMUTerm.Tui/MarkupFormatter.cs @@ -68,9 +68,14 @@ private string ToMarkupCore(StyledLine line) var sb = new StringBuilder(); // A trigger-highlighted line gets a 2-col left rule in the trigger's colour (design output view). + // Through the floor like every other foreground here: it lands on the pane, and it is the one + // mark saying a rule fired at all — the demo's own teal measured 1.42:1 on the Light theme. if (line.RuleColor is { } rule) { - sb.Append('[').Append(Hex(_theme.Resolve(rule, isBackground: false))).Append("]▌[/] "); + var ink = _theme.Resolve(rule, isBackground: false); + sb.Append('[') + .Append(Hex(_text.KeepTextLegible ? Contrast.Legible(ink, _plane) : ink)) + .Append("]▌[/] "); } foreach (var span in line.Spans) diff --git a/src/SharpMUTerm.Tui/Program.cs b/src/SharpMUTerm.Tui/Program.cs index cd14d08..4b00e07 100644 --- a/src/SharpMUTerm.Tui/Program.cs +++ b/src/SharpMUTerm.Tui/Program.cs @@ -1,3 +1,4 @@ +using SharpMUTerm.Core.Theming; using Microsoft.Extensions.Logging; using SharpMUTerm.Core.Configuration; using SharpMUTerm.Core.Telnet.Mssp; @@ -47,6 +48,19 @@ private static int Main(string[] args) // No save action: a snapshot renders, it does not edit. The settings screens persist each // committed change now, and a --view that drives keys into a field would otherwise write the // demo worlds straight over the real configuration. + // --theme names a built-in flavour for this render only. It exists because the client's own + // chrome is derived from the theme and held to a legibility floor against it, and every frame + // in the gallery renders Dark — which is exactly how the Light theme's accent (1.42:1), draft + // pen (1.26:1) and notice (1.73:1) stayed unreadable without anybody seeing them. + if (GetOption(args, "--theme") is { } themeName) + { + // Both, because ResolveTheme treats an inline Theme whose Name disagrees with ThemeName + // as a customised one and prefers it — so setting the name alone would select the built-in + // and then be overruled by the default Dark still sitting in Theme. + config.ThemeName = themeName; + config.Theme = ThemeLibrary.Get(themeName); + } + var (width, height) = ParseSize(args); var app = new SharpMUTermApp(config, capabilities, new HeadlessConsoleDriver(width, height)); var frame = app.RenderSnapshot(GetOption(args, "--view")); diff --git a/src/SharpMUTerm.Tui/RailRenderer.cs b/src/SharpMUTerm.Tui/RailRenderer.cs index 1ca3420..5c99503 100644 --- a/src/SharpMUTerm.Tui/RailRenderer.cs +++ b/src/SharpMUTerm.Tui/RailRenderer.cs @@ -136,7 +136,7 @@ public static List RenderCollapsed(IReadOnlyList rows, ChromeIn // Reserved here too. The collapsed strip is clamped to 4–10 cells, so it moves less — // but it moves, and a strip that widens when a background world says something is the // same reflow as the expanded rail's, on a rail chosen for taking no space. - lines.Add(Link(row, $"[{Accent(row, voice)}]{dot}[/]{name}{UnreadField(row.Unread)}")); + lines.Add(Link(row, $"[{Accent(row, voice)}]{dot}[/]{name}{UnreadField(row.Unread, voice)}")); break; } } @@ -170,7 +170,7 @@ private static string Character(RailRow row, bool reserve, ChromeInk ink) var dot = row.Connected ? "●" : "○"; var name = row.Active ? $"[bold]{Escape(row.Label)}[/]" : Escape(row.Label); return $"{Indent(row)}{ChordField(row.Chord, reserve)}" - + Link(row, $"{marker} [{Accent(row, ink)}]{dot}[/] {name}{UnreadField(row.Unread)}"); + + Link(row, $"{marker} [{Accent(row, ink)}]{dot}[/] {name}{UnreadField(row.Unread, ink)}"); } /// @@ -247,10 +247,10 @@ private static string Unsent(bool unsent, ChromeInk ink) => /// from as well, so the sidebar and the strip cannot come to say different things about one count. /// /// - private static string UnreadField(int unread) => + private static string UnreadField(int unread, ChromeInk ink) => unread <= 0 ? new string(' ', UnreadFieldWidth) - : $"[{UnreadBadge.Tint}]{UnreadBadge.Format(unread).PadLeft(UnreadFieldWidth)}[/]"; + : $"[{UnreadBadge.TintFor(ink)}]{UnreadBadge.Format(unread).PadLeft(UnreadFieldWidth)}[/]"; /// /// A window row: how you get to it, then what it is, then its badges. @@ -283,7 +283,7 @@ private static string Window(RailRow row, bool reserve, ChromeInk ink) var name = Escape(row.Label); var closed = row.Closed ? " [dim]closed[/]" : string.Empty; return $"{Indent(row)}{ChordField(row.Closed ? null : row.Chord, reserve)}" - + Link(row, $"[dim]▪[/] {name}{Unsent(row.Unsent, ink)}{UnreadField(row.Unread)}{closed}"); + + Link(row, $"[dim]▪[/] {name}{Unsent(row.Unsent, ink)}{UnreadField(row.Unread, ink)}{closed}"); } /// @@ -299,8 +299,14 @@ private static string Link(RailRow row, string content) => private static string Indent(RailRow row) => new(' ', row.Indent * 2); + /// + /// A row's own accent as a markup hex, or the client's when it has none — either way held to the + /// legibility floor against the plane the sidebar is drawn on. A world's accent is a colour a user + /// picked in F5 and this is where it meets a plane: unlifted, the demo's own #ff9f1c measures + /// 1.03:1 on the Light theme's backdrop. + /// private static string Accent(RailRow row, ChromeInk ink) => row.Accent.Kind == TerminalColorKind.Rgb - ? $"#{row.Accent.R:x2}{row.Accent.G:x2}{row.Accent.B:x2}" + ? ink.Lift(new Rgb(row.Accent.R, row.Accent.G, row.Accent.B)) : ink.Accent; } diff --git a/src/SharpMUTerm.Tui/SharpMUTermApp.cs b/src/SharpMUTerm.Tui/SharpMUTermApp.cs index 20ac1ac..19d723c 100644 --- a/src/SharpMUTerm.Tui/SharpMUTermApp.cs +++ b/src/SharpMUTerm.Tui/SharpMUTermApp.cs @@ -3930,8 +3930,6 @@ private void SetUpBar(InputBarControl bar, InputBar kind) // bar being rebuilt. Setting them here anyway means a bar is never unpainted between construction // and the first chrome refresh. PaintInputBands(PaneTint.None); - bar.TextColor = ToColor(_theme.Resolve(TerminalColor.Default, isBackground: false)); - bar.IdleTextColor = ToColor(WorkspacePalette.IdleInk(_theme)); bar.HasSibling = () => _second.Visible; bar.Entered += text => OnCommandEntered(kind, text); bar.Changed += text => OnInputChanged(kind, text); @@ -4993,10 +4991,21 @@ private void UpdateInputChrome() private void PaintInputBands(PaneTint tint) { _inputBands = (WorkspacePalette.ArmedBand(_theme, tint), WorkspacePalette.IdleBand(_theme, tint)); + + // The ink follows the band. A tint moves a band's hue and not its luminance, so this is the same + // answer for every character — but it is *not* the same answer for every theme, and that is what + // this is for: Solarized's foreground on its own armed band measured 2.43:1, which made the + // command line the least readable text in the client on the theme most often chosen for comfort. + var armedInk = ToColor(Contrast.Legible( + _theme.Resolve(TerminalColor.Default, isBackground: false), _inputBands.Armed)); + var idleInk = ToColor(Contrast.Legible(WorkspacePalette.IdleInk(_theme), _inputBands.Idle)); + foreach (var bar in new[] { _input, _second }) { bar.BandColor = ToColor(_inputBands.Armed); bar.IdleBandColor = ToColor(_inputBands.Idle); + bar.TextColor = armedInk; + bar.IdleTextColor = idleInk; } } @@ -6110,18 +6119,39 @@ private static TerminalColor AccentFor(WorldDefinition world, int index) => return null; } - /// Renders a as a #rrggbb markup colour. /// - /// A world's own accent as a markup hex, or the app's when it has none. Held to the legibility floor - /// against the plane the status line and the rail are painted on — a world's accent is a colour a - /// user picked in F5, quite possibly against a different theme from the one they are reading in, and - /// this is the only place it meets the plane it lands on. + /// A world's own accent as text: a markup hex held to the legibility floor against the plane + /// the status line and the rail are painted on. A world's accent is a colour a user picked in F5, + /// quite possibly against a different theme from the one they are reading in, and this is where it + /// meets a plane. Falls back to the client's own accent when the world has none. /// private string AccentHex(TerminalColor accent) => + accent.Kind == TerminalColorKind.Rgb ? _ink.Lift(new Rgb(accent.R, accent.G, accent.B)) : _ink.Accent; + + /// + /// A world's accent as a fill — the header ribbon's segments. Deliberately not lifted: + /// a fill is identity, the same reasoning the pane tints are built on, and holding one to a floor + /// against a plane it covers rather than sits on would flatten the ribbon's own hues to a row of + /// near-identical pastels. What has to clear the floor is the text on it, which is . + /// + private string FillHex(TerminalColor accent) => accent.Kind == TerminalColorKind.Rgb - ? Contrast.Legible(new Rgb(accent.R, accent.G, accent.B), WorkspacePalette.ChromePlane(_theme)).ToHex() + ? new Rgb(accent.R, accent.G, accent.B).ToHex() : _ink.Accent; + /// + /// Text on a known fill, held to the floor against that fill rather than against any plane — + /// the ribbon's segment labels and the ● that wears the world's colour on the character chip. + /// + private static string Ink(string inkHex, string fillHex) => + ChromeInk.On(ParseHex(inkHex), ParseHex(fillHex)); + + /// A #rrggbb markup colour back as a colour. + private static Rgb ParseHex(string hex) => new( + Convert.ToByte(hex.Substring(1, 2), 16), + Convert.ToByte(hex.Substring(3, 2), 16), + Convert.ToByte(hex.Substring(5, 2), 16)); + /// /// Projects live config + workspace state into rail rows: each world (with an accent), its /// characters (connected dot, active marker, the chord that goes to them), and — under the active @@ -8017,12 +8047,20 @@ private bool IsFocusedPane(string paneId) => /// private void PaintTabChips(TabControl tabs, bool focused, Rgb plane) { - var activeBg = ToColor(focused ? WorkspacePalette.ArmedBand(_theme) : plane); - var activeFg = ToColor(focused - ? _theme.Resolve(TerminalColor.Default, isBackground: false) - : WorkspacePalette.IdleInk(_theme)); - var restBg = ToColor(focused ? WorkspacePalette.Focus(plane) : plane); - var restFg = ToColor(WorkspacePalette.IdleInk(_theme)); + // Each ink is held to the floor against the chip it actually lands on, and the two chips are + // different planes. Solarized is the theme that showed why: its foreground on its own armed band + // measured 2.43:1, so the active tab and the command line — the two places this pair is used — + // were the least readable text in the client on the theme most likely to be chosen for comfort. + var activeBgRgb = focused ? WorkspacePalette.ArmedBand(_theme) : plane; + var restBgRgb = focused ? WorkspacePalette.Focus(plane) : plane; + var activeBg = ToColor(activeBgRgb); + var activeFg = ToColor(Contrast.Legible( + focused + ? _theme.Resolve(TerminalColor.Default, isBackground: false) + : WorkspacePalette.IdleInk(_theme), + activeBgRgb)); + var restBg = ToColor(restBgRgb); + var restFg = ToColor(Contrast.Legible(WorkspacePalette.IdleInk(_theme), restBgRgb)); tabs.ActiveFocusedBackgroundColor = activeBg; tabs.ActiveUnfocusedBackgroundColor = activeBg; @@ -8051,7 +8089,7 @@ private IWindowControl BuildPaneTabs(PaneNode pane) // so a glyph is the only per-pane cue the strip can carry, and it is the shape half of the // focus signal — it reads on a monochrome terminal, where a luminance step does not. builder.AddTab( - TabTitles.For(window, ActiveCharacterKey(), focused && pane.ActiveTab == windowId), + TabTitles.For(window, ActiveCharacterKey(), focused && pane.ActiveTab == windowId, _ink), BuildTabContent(pane, windowId, window)); ids.Add(windowId); } @@ -9872,7 +9910,7 @@ private void RefreshTabTitles() if (page.Tag is string id && _workspace.FindWindow(id) is { } window) { page.Title = TabTitles.For( - window, focusedCharacter, IsFocusedPane(paneId) && activeTab == id); + window, focusedCharacter, IsFocusedPane(paneId) && activeTab == id, _ink); // The × follows the active tab, so keep it in step with every title refresh. page.IsClosable = CanCloseTab(id, activeTab); } @@ -10298,25 +10336,37 @@ private string HeaderMarkup() var caret = _palette is { IsOpen: true } ? "▾" : Glyphs.Menu; var dark = Hex(_theme.Resolve(TerminalColor.Default, isBackground: true)); var headerBg = Hex(_theme.StatusBackground); - var chip = "#3f4859"; // dim chrome the character segment sits on + // The dim chrome the character segment sits on. Derived rather than the literal #3f4859 it was: + // the header is painted on the theme's own chrome band, and a fixed dark chip under a *light* + // theme is a plane nothing else on the row is measured against — which is how the world accent + // came to be drawn on it at 1.53:1. + var chipRgb = WorkspacePalette.HeaderChip(_theme); + var chip = chipRgb.ToHex(); // Build the ribbon by hand so only the brand "button" is a link (wrapping the whole bar makes // the driver's link highlight repaint every segment and flatten the flowing colours). - var brandBg = AccentHex(AccentPalette[2]); // violet + // A segment's accent is a *fill* and is not lifted: it is identity, the same reasoning the pane + // tints are built on, and lifting a fill would flatten the ribbon's own hues. What has to be + // legible is the text *on* it, so each segment's ink is measured against the fill it lands on. + var brandBg = FillHex(AccentPalette[2]); // violet var sb = new System.Text.StringBuilder(); - sb.Append($"[link={MenuScheme}toggle][bold {dark} on {brandBg}] {caret} muterm [/][/]"); + sb.Append($"[link={MenuScheme}toggle][bold {Ink(dark, brandBg)} on {brandBg}] {caret} muterm [/][/]"); var tail = brandBg; if (ActiveWorld() is { } active) { - var worldAccent = AccentHex(active.Accent); + var worldAccent = FillHex(active.Accent); sb.Append($"[{tail} on {worldAccent}]{Glyphs.PowerRight}[/]"); - sb.Append($"[bold {dark} on {worldAccent}] {Escape(active.World.Name)} [/]"); + sb.Append($"[bold {Ink(dark, worldAccent)} on {worldAccent}] {Escape(active.World.Name)} [/]"); tail = worldAccent; if (active.Character is { } name) { sb.Append($"[{tail} on {chip}]{Glyphs.PowerRight}[/]"); - sb.Append($"[{worldAccent} on {chip}] ● {Escape(name)} [/]"); + + // The ● wears the world's colour and is *text* here rather than a fill, so it is the one + // place on this row the accent has to clear the floor — against the chip, which is the + // plane it actually lands on. + sb.Append($"[{Ink(worldAccent, chip)} on {chip}] ● {Escape(name)} [/]"); tail = chip; } } diff --git a/src/SharpMUTerm.Tui/TabTitles.cs b/src/SharpMUTerm.Tui/TabTitles.cs index dadf851..d7df90e 100644 --- a/src/SharpMUTerm.Tui/TabTitles.cs +++ b/src/SharpMUTerm.Tui/TabTitles.cs @@ -44,7 +44,10 @@ internal static class TabTitles /// reader's eye starts at, and it is on the active tab only, so a pane never shows two. /// public static string For( - WorkspaceWindow window, string? focusedCharacterKey = null, bool focusedPane = false) + WorkspaceWindow window, + string? focusedCharacterKey = null, + bool focusedPane = false, + ChromeInk? ink = null) { ArgumentNullException.ThrowIfNull(window); @@ -77,7 +80,7 @@ public static string For( // is the focus marker and the ✎ / ⌁ behind it are other facts, and a signal that recoloured them // would be claiming they had changed too. Zero cells — see the remarks on this class. var named = owner + MarkupText.Escape(window.Title) + unread; - var body = window.Unread > 0 ? $"[{UnreadBadge.Tint}]{named}[/]" : named; + var body = window.Unread > 0 ? $"[{UnreadBadge.TintFor(ink)}]{named}[/]" : named; return focus + body + pen + cross; } diff --git a/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs b/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs index db7ca3f..ddc289f 100644 --- a/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs @@ -42,10 +42,25 @@ internal static class TriggersScreenRenderer internal const int MinEditorWidth = 48; /// - /// What the route list calls "no spawn window" — a rule with a null SpawnTarget goes to the - /// main output. It is a real choice in the radio group, not the absence of one. + /// What the route list calls a rule that delivers nowhere of its own — a null + /// SpawnTarget. The line goes wherever the other matched rules send it, and to this session's + /// own window if none of them sends it anywhere. + /// + /// It is the default for a new rule, and it is what a highlight rule wants: recolour the match and + /// leave the line where it was. This choice used to be spelt main, which read as a + /// destination and was not one — so "highlight it and leave it where it was" looked like something + /// this screen could not express, and a rule that gagged while routed to main deleted the + /// line instead of keeping it there. + /// + /// + internal const string NoRoute = "(none)"; + + /// + /// The session's own main window, as a real destination — . + /// Distinct from in exactly the way a place is distinct from no place: a gag + /// leaves this delivery standing and cancels the other. /// - internal const string MainWindow = "main"; + internal const string MainWindow = TriggerActions.MainWindow; /// /// The rule row's field ordinals, in the order ⇥ steps through them. The name leads, as it does on @@ -117,7 +132,7 @@ internal static class TriggersScreenRenderer private const string NewTriggerPattern = "text to match"; /// The window a rule routes to, as the route field reads and writes it. - private static string Route(Trigger trigger) => trigger.Actions.SpawnTarget ?? MainWindow; + private static string Route(Trigger trigger) => trigger.Actions.SpawnTarget ?? NoRoute; /// /// The destinations offered as ↑↓ suggestions on the route field: the main output, every window a @@ -135,7 +150,9 @@ internal static IReadOnlyList Routes(Trigger trigger, IReadOnlyList { MainWindow }; + // Both of the two that are always offered, in the order a reader meets them: the commonest + // answer first, then the one that is a place. + var routes = new List { NoRoute, MainWindow }; foreach (var target in (routeTargets ?? Array.Empty()).Append(Route(trigger))) { if (!string.IsNullOrEmpty(target) && !routes.Contains(target, StringComparer.Ordinal)) @@ -286,7 +303,7 @@ internal static ScreenModel Model( ScreenField.WindowName( "route", () => Route(entry.Trigger), - v => entry.Trigger.Actions.SpawnTarget = v == MainWindow ? null : v.Trim(), + v => entry.Trigger.Actions.SpawnTarget = v == NoRoute ? null : v.Trim(), Routes(entry.Trigger, routeTargets)), ScreenField.Colour( "highlight fg", diff --git a/src/SharpMUTerm.Tui/UnreadBadge.cs b/src/SharpMUTerm.Tui/UnreadBadge.cs index 1f2da74..1e00c29 100644 --- a/src/SharpMUTerm.Tui/UnreadBadge.cs +++ b/src/SharpMUTerm.Tui/UnreadBadge.cs @@ -31,7 +31,14 @@ internal static class UnreadBadge /// focused, unread, both or neither, and each of the four states reads distinctly. /// /// - internal const string Tint = ScreenPalette.Accent; + /// + /// It is the client's own accent for the active theme rather than a constant, because both surfaces + /// it appears on move with the theme: a rail badge lands on the backdrop and a tab badge lands in a + /// pane. As a fixed #00f5b7 it measured 1.41:1 on the Light theme's backdrop and 1.42:1 on its + /// focused pane — a count the reader was shown and could not read. + /// + /// + internal static string TintFor(ChromeInk? ink) => (ink ?? ChromeInk.Default).Accent; /// A count as it is written, capped at . internal static string Format(int unread) => diff --git a/src/SharpMUTerm.Tui/WorkspacePalette.cs b/src/SharpMUTerm.Tui/WorkspacePalette.cs index 59a83c9..ede121d 100644 --- a/src/SharpMUTerm.Tui/WorkspacePalette.cs +++ b/src/SharpMUTerm.Tui/WorkspacePalette.cs @@ -361,6 +361,29 @@ internal static Rgb IdleInk(Theme theme) /// The one-cell hairline a split draws between two panes, and beside the rail. internal static Rgb Rule(Theme theme) => Mix(Surface(theme), theme.Border, RuleLift); + /// + /// The dim chrome the header ribbon's character segment sits on — the theme's chrome band lifted + /// toward its own foreground, so the segment reads as a distinct chip against the band it ends on. + /// + /// Derived rather than the fixed #3f4859 it was. That literal was picked against a dark theme + /// and is a dark chip whatever the theme, so under Light the world's accent — text on this + /// chip — was drawn at 1.53:1 while everything around it had been resolved for a light plane. + /// + /// + internal static Rgb HeaderChip(Theme theme) + { + ArgumentNullException.ThrowIfNull(theme); + return Mix(theme.StatusBackground, theme.Foreground, ChipLift); + } + + /// + /// How far the header chip is lifted off the chrome band toward the theme's ink. Enough that the + /// wedge between the two is visible as a shape — the segment boundary is the only thing that says + /// where one part of the ribbon ends — and no further, because this is a background for a name and + /// not a highlight. + /// + private const double ChipLift = 0.22; + /// /// Every plane a pane's output can be painted on: the untinted surface and all six tints, each /// focused and not. Fourteen colours, and what matters about them is that they form a band @@ -450,7 +473,8 @@ internal static ChromeInk Chrome(Theme theme) Contrast.Legible(ChromeInk.BaseAccent, plane).ToHex(), Contrast.Legible(ChromeInk.BaseNotice, plane).ToHex(), Contrast.Legible(ChromeInk.BaseDraft, plane).ToHex(), - Contrast.Legible(theme.ResolveIndex(5), plane).ToHex()); + Contrast.Legible(theme.ResolveIndex(5), plane).ToHex(), + plane); } /// Linear blend of two colours, of the way from to . diff --git a/tests/SharpMUTerm.Core.Tests/TriggerRouteMainTests.cs b/tests/SharpMUTerm.Core.Tests/TriggerRouteMainTests.cs new file mode 100644 index 0000000..26f3fd9 --- /dev/null +++ b/tests/SharpMUTerm.Core.Tests/TriggerRouteMainTests.cs @@ -0,0 +1,169 @@ +using SharpMUTerm.Core.Automation; +using SharpMUTerm.Core.Text; + +namespace SharpMUTerm.Core.Tests; + +/// +/// The three things a rule's route can say, and the one it could not say before: nothing. +/// +/// A null has always meant "this rule adds no destination — +/// the line goes wherever the other matched rules send it". The F2 screen labelled that main, +/// which made it look like a destination, and left the reader with no way to express either half of +/// what they wanted: "highlight it and leave it where it was" looked impossible, and a gagging rule +/// aimed at main deleted the line rather than keeping it there. +/// +/// +public class TriggerRouteMainTests +{ + private static TriggerEngine EngineWith(params Trigger[] triggers) + { + var engine = new TriggerEngine(); + engine.ReplaceConfigured(triggers); + return engine; + } + + private static Trigger Rule(string pattern, Action configure) + { + var trigger = new Trigger { Name = pattern, Pattern = pattern }; + configure(trigger.Actions); + return trigger; + } + + private static StyledLine Line(string text) => StyledLine.FromText(text, TextStyle.Default); + + [Test] + public async Task ARuleWithNoRouteAddsNoDestination() + { + var engine = EngineWith(Rule("Ann", a => a.HighlightForeground = TerminalColor.FromRgb(255, 215, 0))); + + var result = engine.Process(Line("Ann waves at you")); + + await Assert.That(result.SpawnTargets).IsEmpty(); + await Assert.That(result.RouteMain).IsFalse(); + await Assert.That(result.Suppress).IsFalse(); + } + + [Test] + public async Task AHighlightRuleDoesNotMoveALineAnotherRuleRouted() + { + // The shape the report was about: a capture rule owns where the line goes, and a highlight rule + // added afterwards recolours it without changing that. There is one line and one set of + // destinations, and every matched rule's highlight is on it. + var engine = EngineWith( + Rule("^", a => { a.SpawnTarget = "Chat"; a.Gag = true; }), + Rule("Ann", a => a.HighlightForeground = TerminalColor.FromRgb(255, 215, 0))); + + var result = engine.Process(Line(" Ann says hi")); + + await Assert.That(result.SpawnTargets).IsEquivalentTo(new[] { "Chat" }); + await Assert.That(result.Suppress).IsTrue(); + await Assert.That(result.Line.Spans.Any(s => s.Style.Foreground.Kind == TerminalColorKind.Rgb)).IsTrue(); + } + + [Test] + public async Task RoutingToMainIsADestinationAndNotTheAbsenceOfOne() + { + var engine = EngineWith(Rule("Ann", a => a.SpawnTarget = TriggerActions.MainWindow)); + + var result = engine.Process(Line("Ann waves at you")); + + await Assert.That(result.RouteMain).IsTrue(); + + // It is *not* a spawn: nothing may go looking for a capture pane called "main", which is what + // this target used to conjure. + await Assert.That(result.SpawnTargets).IsEmpty(); + } + + [Test] + public async Task AGaggingRuleRoutedToMainKeepsTheLineThere() + { + // The defect: gag suppresses the *default* delivery, and `main` was not a route, so gag + main + // deleted the line outright. Explicit destinations survive a gag — which has always been true of + // a spawn pane, and is the whole meaning of "only where I routed it". + var engine = EngineWith(Rule("Ann", a => + { + a.SpawnTarget = TriggerActions.MainWindow; + a.Gag = true; + })); + + var result = engine.Process(Line("Ann waves at you")); + + await Assert.That(result.Suppress).IsTrue(); + await Assert.That(result.RouteMain).IsTrue(); + } + + [Test] + public async Task TheMainTargetIsRecognisedWhateverItsCasing() + { + // It is a reserved word the user types into a field, not an identifier. + var engine = EngineWith(Rule("Ann", a => a.SpawnTarget = "Main")); + + await Assert.That(engine.Process(Line("Ann waves")).RouteMain).IsTrue(); + } + + [Test] + public async Task AWindowGenuinelyCalledMainIsStillUnreachableAsASpawn() + { + // Stated so the reservation is a decision rather than an accident: `main` names the session's own + // window and can name nothing else. No window is titled `main` today — a character's session + // window is titled after the character, and `main` is only the rail's label for it — so nothing + // is lost, and a config that already said `main` now reaches what whoever wrote it meant. + var engine = EngineWith(Rule("Ann", a => a.SpawnTarget = "main")); + + var result = engine.Process(Line("Ann waves")); + + await Assert.That(result.SpawnTargets).IsEmpty(); + } + + [Test] + public async Task TwoRulesNamingOnePaneDeliverOneLine() + { + // They delivered two: the engine appended to a bare list and the session raised one event per + // entry, so a highlight rule pointed at the same pane as its capture rule doubled every line. + var engine = EngineWith( + Rule("^", a => a.SpawnTarget = "Chat"), + Rule("Ann", a => a.SpawnTarget = "Chat")); + + var result = engine.Process(Line(" Ann says hi")); + + await Assert.That(result.SpawnTargets).IsEquivalentTo(new[] { "Chat" }); + } + + [Test] + public async Task TwoRulesNamingTwoPanesStillDeliverToBoth() + { + // The dedup is by destination and not "one route per line": a line genuinely captured by two + // channels belongs in both panes. + var engine = EngineWith( + Rule("^", a => a.SpawnTarget = "Chat"), + Rule("Ann", a => a.SpawnTarget = "Mentions")); + + var result = engine.Process(Line(" Ann says hi")); + + await Assert.That(result.SpawnTargets).IsEquivalentTo(new[] { "Chat", "Mentions" }); + } + + [Test] + public async Task MainAndASpawnAreNotEachOthersDuplicates() + { + var engine = EngineWith( + Rule("^", a => a.SpawnTarget = "Chat"), + Rule("Ann", a => a.SpawnTarget = TriggerActions.MainWindow)); + + var result = engine.Process(Line(" Ann says hi")); + + await Assert.That(result.SpawnTargets).IsEquivalentTo(new[] { "Chat" }); + await Assert.That(result.RouteMain).IsTrue(); + } + + [Test] + public async Task ARouteTemplateResolvingToMainRoutesToTheMainWindow() + { + // Routes expand capture groups, so `main` can arrive from the server's own text. That is fine + // *because* the main window is the session's own — the destination a line was going to anyway — + // and it is the one target a resolved name may reach without a user having opened it. + var engine = EngineWith(Rule(@"^\[(\w+)\]", a => a.SpawnTarget = "$1")); + + await Assert.That(engine.Process(Line("[main] hello")).RouteMain).IsTrue(); + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs b/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs new file mode 100644 index 0000000..a9966c1 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs @@ -0,0 +1,192 @@ +using System.Text; +using System.Text.RegularExpressions; +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Text; +using SharpMUTerm.Core.Theming; +using SharpMUTerm.Graphics; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// Every cell of every frame, on every theme. The unit tests hold each colour to the floor where +/// it is produced; this holds the paint to it, which is a different claim and the one that +/// found most of the defects. +/// +/// A colour is only legible relative to the plane it lands on, and there is exactly one place both facts +/// are true at once: the emitted SGR. Walking it back into (foreground, background) pairs found what +/// reading the source did not — the trigger left-rule, the header ribbon's chip, a world's own accent on +/// the rail, the unread badge on a tab, and the command line's ink on the Solarized armed band. Every +/// one of those had a plausible-looking call site. +/// +/// +/// +/// Serialised with the other end-to-end suites: constructing the app and rendering a frame both touch +/// the process-global console streams. +/// +[NotInParallel] +public class FrameContrastTests +{ + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + /// + /// The views worth walking: one of every kind of surface the client paints — panes, the rail, the + /// header, the command line, an overlay, a settings screen, and each of the boundary bars. + /// + private static readonly string[] Views = + [ + "", "freeze", "away", "highlight", "scrollback", "links", "connections", "tint", "tint-input", + "characters", "compose", "mssp", "web", "spawn", "split", "menu", "quit", "worlds", "triggers", + "logging", "startup", "history", "prefix-panel", "keypad", + ]; + + public static IEnumerable<(string Theme, string View)> Cases() => + from theme in ThemeLibrary.Names + from view in Views + select (theme, view); + + [Test] + [MethodDataSource(nameof(Cases))] + public async Task NoPaintedTextIsBelowTheLegibilityFloor((string Theme, string View) test) + { + var frame = Render(test.Theme, test.View); + + var failures = Pairs(frame) + .Select(pair => (pair.Key.Fg, pair.Key.Bg, pair.Value, Ratio: Contrast.Ratio(pair.Key.Fg, pair.Key.Bg))) + .Where(t => t.Ratio < Contrast.Floor) + .Where(t => !IsFrameworkDim(t.Fg)) + .OrderBy(t => t.Ratio) + .Select(t => $"{t.Ratio:0.00} {t.Fg.ToHex()} on {t.Bg.ToHex()} ({t.Value} cells)") + .ToList(); + + await Assert.That(failures).IsEmpty() + .Because($"--theme \"{test.Theme}\" --view {(test.View.Length == 0 ? "(default)" : test.View)}"); + } + + /// + /// The one exemption, and it is the framework's rather than ours. SharpConsoleUI resolves the + /// [dim] markup tag to a fixed #808080 of its own — it is an internal local function in + /// the markup renderer, reachable through no option we hold — so a dim rule measures 4.01:1 on the + /// default dark theme and 2.52:1 on Solarized Dark's focused pane. + /// + /// It is exempted rather than papered over. Reaching it means giving up [dim] everywhere in + /// favour of an explicit floor-checked grey, which is a sweep across every renderer in the app for a + /// near miss on one theme; the honest state is a named exemption and a number, so that anybody who + /// does that sweep can delete this and watch the test still pass. + /// + /// + private static bool IsFrameworkDim(Rgb fg) => fg == new Rgb(0x80, 0x80, 0x80); + + /// + /// A glyph that is a fill rather than text, and to which a text floor therefore does not + /// apply. Three kinds, and each is exempt for its own reason rather than because it was failing: + /// + /// the powerline wedges — a fill boundary: the glyph is by construction the previous + /// ribbon segment's colour drawn on the next segment's, so its "contrast" is the distance between + /// two identities; + /// box-drawing rules and the settings screens' hairlines — a divider's job is to be found, not + /// read, and WorkspacePalette.Rule already says in as many words that a rule at full contrast + /// "reads fine once and shouts at four panes"; + /// the solid and shaded blocks, which in this app are colour samples — F2's highlight + /// swatch is the picked colour shown as the pane will paint it, so holding it to a floor against the + /// settings backdrop would be measuring it against a plane it is deliberately not for. + /// + /// The half blocks are not exempt, and that is the line: ▌ is the trigger left-rule and + /// the focused-pane marker — marks that carry a fact and have to be seen — and one of them was a real + /// defect this test caught. + /// + private static bool IsFill(char ch) => + ch is '' or '' + || ch is >= '\u2500' and <= '\u257f' + || ch is '█' or '░' or '▒' or '▓'; + + private static string Render(string themeName, string view) + { + Console.SetIn(TextReader.Null); + var config = DemoScene.Build(); + config.ThemeName = themeName; + config.Theme = ThemeLibrary.Get(themeName); + + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(140, 36)); + return app.RenderSnapshot(view.Length == 0 ? null : view); + } + + private static readonly Regex Sgr = new(@"\x1b\[([0-9;]*)m", RegexOptions.Compiled); + + private static readonly Regex Csi = new(@"\x1b\[[0-9;?]*[A-Za-z]", RegexOptions.Compiled); + + /// + /// Every (foreground, background) pair the frame actually paints a glyph in, with a cell count. + /// Spaces are skipped — a space has no foreground to read — and so is everything + /// names. + /// + private static Dictionary<(Rgb Fg, Rgb Bg), int> Pairs(string frame) + { + var pairs = new Dictionary<(Rgb, Rgb), int>(); + Rgb? fg = null; + Rgb? bg = null; + + for (var i = 0; i < frame.Length;) + { + var sgr = Sgr.Match(frame, i); + if (sgr.Success && sgr.Index == i) + { + Apply(sgr.Groups[1].Value, ref fg, ref bg); + i = sgr.Index + sgr.Length; + continue; + } + + var csi = Csi.Match(frame, i); + if (csi.Success && csi.Index == i) + { + i = csi.Index + csi.Length; + continue; + } + + var ch = frame[i++]; + if (ch is ' ' or '\n' or '\r' || IsFill(ch) || fg is not { } ink || bg is not { } plane) + { + continue; + } + + pairs[(ink, plane)] = pairs.GetValueOrDefault((ink, plane)) + 1; + } + + return pairs; + } + + private static void Apply(string parameters, ref Rgb? fg, ref Rgb? bg) + { + var codes = parameters.Split(';') + .Where(p => p.Length > 0) + .Select(int.Parse) + .ToList(); + + for (var i = 0; i < codes.Count;) + { + if (codes[i] == 0) + { + fg = bg = null; + i++; + } + else if (codes[i] is 38 or 48 && i + 4 < codes.Count && codes[i + 1] == 2) + { + var colour = new Rgb((byte)codes[i + 2], (byte)codes[i + 3], (byte)codes[i + 4]); + if (codes[i] == 38) + { + fg = colour; + } + else + { + bg = colour; + } + + i += 5; + } + else + { + i++; + } + } + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/ScreenChoiceListTests.cs b/tests/SharpMUTerm.Tui.Tests/ScreenChoiceListTests.cs index 5f96278..1b553a4 100644 --- a/tests/SharpMUTerm.Tui.Tests/ScreenChoiceListTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/ScreenChoiceListTests.cs @@ -272,7 +272,7 @@ public async Task F2DrawsTheRouteListUnderTheRouteField() var sets = Sets(); var editor = TriggersScreenRenderer.EditorColumn(sets, 0, Routes[1..], OpenTheRoute(sets).Focus()); - await Assert.That(Entries(editor)).IsEquivalentTo(new[] { "main", "Chat", "pages", "trade" }); + await Assert.That(Entries(editor)).IsEquivalentTo(new[] { "(none)", "main", "Chat", "pages", "trade" }); await Assert.That(Marked(editor)).IsEqualTo("Chat"); await Assert.That(Caption(editor)).Contains(ScreenChrome.OpenChoicesCaption); } diff --git a/tests/SharpMUTerm.Tui.Tests/TabActivityIndicatorTests.cs b/tests/SharpMUTerm.Tui.Tests/TabActivityIndicatorTests.cs index 1f4d652..3ea04c8 100644 --- a/tests/SharpMUTerm.Tui.Tests/TabActivityIndicatorTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/TabActivityIndicatorTests.cs @@ -141,13 +141,13 @@ public async Task TheSidebarAndTheTabPrintTheSameCount(int lines) // The rail's own rows carry the same badge, right-aligned in the field it reserves for one. var railRows = app.RailLines - .Where(l => Regex.IsMatch(l, $@"\[{Regex.Escape(UnreadBadge.Tint)}\]\s*\d+\+?\[/\]")) + .Where(l => Regex.IsMatch(l, $@"\[{Regex.Escape(UnreadBadge.TintFor(null))}\]\s*\d+\+?\[/\]")) .ToList(); await Assert.That(railRows).IsNotEmpty(); foreach (var railRow in railRows) { await Assert.That(railRow) - .Contains($"[{UnreadBadge.Tint}]{badge.PadLeft(UnreadBadge.FieldWidth)}[/]"); + .Contains($"[{UnreadBadge.TintFor(null)}]{badge.PadLeft(UnreadBadge.FieldWidth)}[/]"); } // Once the cap has bitten, the uncapped number is on neither surface. This is the assertion that @@ -212,16 +212,16 @@ public async Task AFocusedTabWithUnreadShowsBothCuesAndTheyAreDifferentChannels( // wide margin — so it reads whichever pane the tab is in. foreach (var plane in planes) { - await Assert.That(plane).IsNotEqualTo(UnreadBadge.Tint); - await Assert.That(Contrast(UnreadBadge.Tint, plane)).IsGreaterThan(4.5); + await Assert.That(plane).IsNotEqualTo(UnreadBadge.TintFor(null)); + await Assert.That(Contrast(UnreadBadge.TintFor(null), plane)).IsGreaterThan(4.5); } // The tinted cells are all drawn on one background — the focused chip's — which is what makes the // point: same cells, two channels, and the background is doing the focus half on its own. var chip = tinted.Select(c => c.Background).Distinct().ToList(); await Assert.That(chip.Count).IsEqualTo(1); - await Assert.That(chip[0]).IsNotEqualTo(UnreadBadge.Tint); - await Assert.That(Contrast(UnreadBadge.Tint, chip[0]!)).IsGreaterThan(4.5); + await Assert.That(chip[0]).IsNotEqualTo(UnreadBadge.TintFor(null)); + await Assert.That(Contrast(UnreadBadge.TintFor(null), chip[0]!)).IsGreaterThan(4.5); } /// @@ -238,7 +238,7 @@ public async Task TheFocusMarkerKeepsItsOwnColourWhileTheTabIsUnread() var cells = Cells(app.RenderWholeFrame()); var marker = cells.Values.Single(c => c.Row == StripRow(app) && c.Char == Glyphs.FocusedPane[0]); - await Assert.That(marker.Foreground).IsNotEqualTo(UnreadBadge.Tint); + await Assert.That(marker.Foreground).IsNotEqualTo(UnreadBadge.TintFor(null)); } // --- the NAWS trap ---------------------------------------------------------------------------- @@ -394,7 +394,7 @@ public async Task PickingTheTabOfAScrolledBackWindowClearsNeitherBadgeAndTheTail await Assert.That(app.UnreadOf(Main)).IsEqualTo(0); await Assert.That(MainTabLabel(app)).DoesNotContain("("); - await Assert.That(MainTabLabel(app)).DoesNotContain(UnreadBadge.Tint); + await Assert.That(MainTabLabel(app)).DoesNotContain(UnreadBadge.TintFor(null)); await Assert.That(RailShowsABadge(app)).IsFalse(); } @@ -405,7 +405,7 @@ public async Task PickingTheTabOfAScrolledBackWindowClearsNeitherBadgeAndTheTail /// private static bool RailShowsABadge(SharpMUTermApp app) => app.RailLines.Any(l => Regex.IsMatch( - l, $@"\[{Regex.Escape(UnreadBadge.Tint)}\]\s*\d+\+?\[/\]")); + l, $@"\[{Regex.Escape(UnreadBadge.TintFor(null))}\]\s*\d+\+?\[/\]")); // --- frame decoding --------------------------------------------------------------------------- @@ -485,7 +485,7 @@ private static bool RailShowsABadge(SharpMUTermApp app) => /// /// The tinted cells on one row, in column order and within that pane's own columns. /// - /// Scoped rather than swept, because is the app accent and the + /// Scoped rather than swept, because is the app accent and the /// chrome already uses it elsewhere — the header's connected ●, the rail's world spine ▚ and its status /// dots are all painted in it. That is fine where it is: those are fixed decorations in fixed places, /// and inside a tab strip nothing else is accent-coloured, so on this row the tint is unambiguous. But @@ -499,7 +499,7 @@ private static List Tinted( return cells.Values .Where(c => c.Row == row && c.Column >= rect.X && c.Column < rect.X + rect.Width - && c.Foreground == UnreadBadge.Tint) + && c.Foreground == UnreadBadge.TintFor(null)) .OrderBy(c => c.Column) .ToList(); } diff --git a/tests/SharpMUTerm.Tui.Tests/TabTitlesTests.cs b/tests/SharpMUTerm.Tui.Tests/TabTitlesTests.cs index 078c090..1094998 100644 --- a/tests/SharpMUTerm.Tui.Tests/TabTitlesTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/TabTitlesTests.cs @@ -36,7 +36,7 @@ public async Task Unread_AppendsACountBadge() var ws = new Workspace(); ws.RouteSpawn("Chat"); var chat = ws.RouteSpawn("Chat"); // two background routes → unread 2 - await Assert.That(TabTitles.For(chat)).IsEqualTo($"[{UnreadBadge.Tint}]Chat (2)[/]"); + await Assert.That(TabTitles.For(chat)).IsEqualTo($"[{UnreadBadge.TintFor(null)}]Chat (2)[/]"); } /// @@ -52,7 +52,7 @@ public async Task Unread_IsCappedTheWayTheSidebarCapsIt(int unread, string badge { var window = Background("Mannaz", unread); await Assert.That(window.Unread).IsEqualTo(unread); - await Assert.That(TabTitles.For(window)).IsEqualTo($"[{UnreadBadge.Tint}]Mannaz ({badge})[/]"); + await Assert.That(TabTitles.For(window)).IsEqualTo($"[{UnreadBadge.TintFor(null)}]Mannaz ({badge})[/]"); } /// @@ -66,7 +66,7 @@ public async Task TheFocusMarkerStaysOutsideTheActivityTint() var window = Background("Mannaz", 36); var label = TabTitles.For(window, focusedPane: true); - await Assert.That(label).IsEqualTo($"{Glyphs.FocusedPane} [{UnreadBadge.Tint}]Mannaz (36)[/]"); + await Assert.That(label).IsEqualTo($"{Glyphs.FocusedPane} [{UnreadBadge.TintFor(null)}]Mannaz (36)[/]"); await Assert.That(label.StartsWith(Glyphs.FocusedPane, StringComparison.Ordinal)).IsTrue(); } @@ -101,7 +101,7 @@ public async Task UnreadAndUnsent_ShowBoth() var ws = new Workspace(); var chat = ws.RouteSpawn("Chat"); // unread 1, background ws.SetUnsentInput(chat.Id, true); - await Assert.That(TabTitles.For(chat)).IsEqualTo($"[{UnreadBadge.Tint}]Chat (1)[/] {Glyphs.Draft}"); + await Assert.That(TabTitles.For(chat)).IsEqualTo($"[{UnreadBadge.TintFor(null)}]Chat (1)[/] {Glyphs.Draft}"); } [Test] @@ -130,7 +130,7 @@ public async Task ChildWindow_OwnerPrefixPrecedesBadges() chat.OwnerLabel = "Corvid"; ws.SetUnsentInput(chat.Id, true); await Assert.That(TabTitles.For(chat)) - .IsEqualTo($"[{UnreadBadge.Tint}]Corvid - Chat (1)[/] {Glyphs.Draft}"); + .IsEqualTo($"[{UnreadBadge.TintFor(null)}]Corvid - Chat (1)[/] {Glyphs.Draft}"); } [Test] diff --git a/tests/SharpMUTerm.Tui.Tests/TriggerKeepItHereTests.cs b/tests/SharpMUTerm.Tui.Tests/TriggerKeepItHereTests.cs new file mode 100644 index 0000000..085dfb4 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/TriggerKeepItHereTests.cs @@ -0,0 +1,218 @@ +using SharpConsoleUI.Drivers; +using SharpMUTerm.Core.Automation; +using SharpMUTerm.Core.Commands; +using SharpMUTerm.Core.Configuration; +using SharpMUTerm.Core.Text; +using SharpMUTerm.Core.Workspaces; +using SharpMUTerm.Graphics; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// "Highlight text and send it to the pane where we found it" — the reported gap, end to end. +/// +/// Two things it turned out to need, and one it did not. It did not need a highlight rule to +/// carry a route: there is one line and one set of destinations, so a rule that only recolours already +/// reaches every pane the line was going to. What it needed was a way to say that — the F2 +/// route field spelt "delivers nowhere of its own" as main, which reads as a destination — and a +/// main that really is one, so a shared trigger set can name each character's own window without +/// knowing that character's name. +/// +/// +/// +/// Serialised with the other end-to-end suites: constructing the app and rendering a frame both touch +/// the process-global console streams. +/// +[NotInParallel] +public class TriggerKeepItHereTests +{ + private const int Width = 160; + private const int Height = 40; + + private const string Ann = "Convergence.Ann"; + private const string Bob = "Convergence.Bob"; + + private static readonly TerminalCapabilities Headless = + new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); + + /// The gold a highlight rule paints with — comfortably over the legibility floor, so what + /// reaches the pane is the colour that was picked rather than a lift of it. + private const string Gold = "#ffd700"; + + [Test] + public async Task AHighlightRuleReachesTheCapturePaneAnotherRuleSentTheLineTo() + { + // The user's own framing: the second rule "should still follow the original route, as long as it + // does not change where it routes to". The capture rule owns the destination; the highlight rule + // adds no route and recolours the line that goes there. + var app = await Two(Config( + Capture("^", "Chat"), + Highlight("Ann"))); + + Receive(app, AnnWire, " Ann says hello\n"); + + // The highlight splits the line into spans, so the assertion is on the fragments and the colour + // rather than on the sentence — a pane holds markup, and the whole point of the rule is that the + // sentence is no longer one run. + var chat = string.Join("\n", app.PaneLines(Workspace.SpawnWindowId(Ann, "Chat"))); + await Assert.That(chat).Contains($"[{Gold}]Ann[/]"); + await Assert.That(chat).Contains("says hello"); + } + + [Test] + public async Task AHighlightRuleDoesNotOpenAPaneOrCopyTheLineAnywhere() + { + // The half that would be easy to break by "fixing" the above: a rule with no route must add no + // destination at all, or every highlight rule would sprout a pane. + var app = await Two(Config(Highlight("Ann"))); + + Receive(app, AnnWire, "Ann waves at you\n"); + + await Assert.That(app.WindowIds().Any(id => id.StartsWith(Workspace.SpawnPrefix, StringComparison.Ordinal))) + .IsFalse(); + + var main = string.Join("\n", app.PaneLines(MainWindowOf(app, Ann))); + await Assert.That(main).Contains($"[{Gold}]Ann[/]"); + await Assert.That(main).Contains("waves at you"); + } + + [Test] + public async Task RoutingToMainKeepsAGaggedLineInTheOwnersOwnWindow() + { + // `route: main` is a destination and a gag suppresses only the *default* delivery, so the line + // stays. Before, `main` was the label on a null route, and gag deleted the line outright. + var app = await Two(Config(new Trigger + { + Name = "Public", + Pattern = "^ (.+)$", + Actions = new TriggerActions { SpawnTarget = TriggerActions.MainWindow, Gag = true }, + })); + + Receive(app, AnnWire, " Ann says hello\n"); + + await Assert.That(string.Join("\n", app.PaneLines(MainWindowOf(app, Ann)))).Contains("hello"); + } + + [Test] + public async Task MainIsEachCharactersOwnWindowAndNotWhicheverOneMatchedFirst() + { + // The reason `main` earns a reserved word rather than being spelt as the window's title: one + // trigger set is shared by every character that lists it, and a title can only name one of them. + // Both characters run this rule and each keeps their own line. + var app = await Two(Config(new Trigger + { + Name = "Public", + Pattern = "^ (.+)$", + Actions = new TriggerActions { SpawnTarget = TriggerActions.MainWindow, Gag = true }, + })); + + Receive(app, AnnWire, " Ann says first\n"); + Receive(app, BobWire, " Bob says second\n"); + + var annPane = string.Join("\n", app.PaneLines(MainWindowOf(app, Ann))); + var bobPane = string.Join("\n", app.PaneLines(MainWindowOf(app, Bob))); + + await Assert.That(annPane).Contains("first"); + await Assert.That(annPane).DoesNotContain("second"); + await Assert.That(bobPane).Contains("second"); + await Assert.That(bobPane).DoesNotContain("first"); + } + + [Test] + public async Task TwoRulesNamingOnePaneDeliverOneLineToIt() + { + // They delivered two, and the pane showed the line twice. + var app = await Two(Config( + Capture("^", "Chat"), + new Trigger + { + Name = "Mention", + Pattern = "Ann", + Actions = new TriggerActions { SpawnTarget = "Chat" }, + })); + + Receive(app, AnnWire, " Ann says hello\n"); + + var lines = app.PaneLines(Workspace.SpawnWindowId(Ann, "Chat")); + await Assert.That(lines.Count(l => l.Contains("Ann says hello", StringComparison.Ordinal))).IsEqualTo(1); + } + + // ---- Harness ------------------------------------------------------------------------------ + + private RecordingTelnetSession AnnWire { get; set; } = new(); + + private RecordingTelnetSession BobWire { get; set; } = new(); + + private static string MainWindowOf(SharpMUTermApp app, string sessionKey) => + app.WindowIds().Single(id => + app.WindowOwnerOf(id) == sessionKey && !id.StartsWith(Workspace.SpawnPrefix, StringComparison.Ordinal)); + + private static Trigger Capture(string pattern, string target) => new() + { + Name = target, + Pattern = pattern, + Actions = new TriggerActions { SpawnTarget = target, Gag = true }, + }; + + private static Trigger Highlight(string pattern) => new() + { + Name = pattern, + Pattern = pattern, + Actions = new TriggerActions { HighlightForeground = TerminalColor.FromRgb(0xff, 0xd7, 0x00) }, + }; + + private static AppConfiguration Config(params Trigger[] triggers) + { + var config = new AppConfiguration(); + var set = new TriggerSet { Name = "Comms" }; + foreach (var trigger in triggers) + { + set.Triggers.Add(trigger); + } + + config.TriggerSets.Add(set); + config.Worlds.Add(new WorldDefinition + { + Name = "Convergence", + Host = "convergence.example.org", + Port = 4201, + Characters = + { + new CharacterDefinition { Name = "Ann", Logging = new LoggingSettings(), TriggerSets = { "Comms" } }, + new CharacterDefinition { Name = "Bob", Logging = new LoggingSettings(), TriggerSets = { "Comms" } }, + }, + }); + + return config; + } + + private async Task Two(AppConfiguration config) + { + Console.SetIn(TextReader.Null); + AnnWire = new RecordingTelnetSession(); + BobWire = new RecordingTelnetSession(); + + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(Width, Height)); + await Open(app, Ann, AnnWire); + await Open(app, Bob, BobWire); + app.RenderNextFrame(); + return app; + } + + private static async Task Open(SharpMUTermApp app, string sessionKey, RecordingTelnetSession wire) + { + app.TelnetFactory = _ => wire; + if (!app.DispatchCommand(CommandIds.Character(sessionKey))) + { + throw new InvalidOperationException($"the app would not switch to {sessionKey}"); + } + + await app.FindSession(sessionKey)!.ConnectAsync(); + } + + private static void Receive(SharpMUTermApp app, RecordingTelnetSession wire, string text) + { + wire.Receive(text); + app.RenderNextFrame(); + } +} diff --git a/tests/SharpMUTerm.Tui.Tests/TriggersScreenEditingTests.cs b/tests/SharpMUTerm.Tui.Tests/TriggersScreenEditingTests.cs index 420650f..821e366 100644 --- a/tests/SharpMUTerm.Tui.Tests/TriggersScreenEditingTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/TriggersScreenEditingTests.cs @@ -83,31 +83,38 @@ public async Task TheRouteGroupOffersMainEveryKnownWindowAndTheRulesOwnTarget() { var sets = Sets(); + // Two are always offered and they are different things: "(none)" is a rule that delivers + // nowhere of its own, "main" is the session's own window as a destination. var known = TriggersScreenRenderer.Model(sets, 0, Targets).FieldAt(0, 0, TriggersScreenRenderer.RouteField)!.Value.Choices; - await Assert.That(known).IsEquivalentTo(new[] { "main", "Chat", "pages", "trade" }); + await Assert.That(known).IsEquivalentTo(new[] { "(none)", "main", "Chat", "pages", "trade" }); // A rule pointed at a window the workspace has no record of still offers — and keeps — its own // value, rather than being refused by its own field. var unknown = TriggersScreenRenderer.Model(sets, 0).FieldAt(0, 0, TriggersScreenRenderer.RouteField)!.Value; - await Assert.That(unknown.Choices).IsEquivalentTo(new[] { "main", "Chat" }); + await Assert.That(unknown.Choices).IsEquivalentTo(new[] { "(none)", "main", "Chat" }); await Assert.That(unknown.Validate("Chat")).IsNull(); } /// - /// main is how a rule stops routing anywhere: it stores null rather than the literal word. The - /// undo puts it back half went with the screen-wide revert — a committed route is confirmed work - /// and is kept, and only deletions are reviewed on the way out. + /// (none) is how a rule stops routing anywhere: it stores null rather than the literal word. + /// main stores the word, because it is a destination — the two were one choice, spelt + /// main, and the conflation is what made a gagging rule aimed at the main window delete the + /// line. The undo puts it back half went with the screen-wide revert — a committed route is + /// confirmed work and is kept, and only deletions are reviewed on the way out. /// [Test] - public async Task ChoosingMainClearsTheSpawnTarget() + public async Task ChoosingNoRouteClearsTheSpawnTargetAndChoosingMainDoesNot() { var sets = Sets(); var trigger = sets[0].Triggers[0]; var edits = new ScreenEdits(); - edits.Apply(TriggersScreenRenderer.Model(sets, 0, Targets).FieldAt(0, 0, TriggersScreenRenderer.RouteField)!.Value, "main"); + edits.Apply(TriggersScreenRenderer.Model(sets, 0, Targets).FieldAt(0, 0, TriggersScreenRenderer.RouteField)!.Value, "(none)"); await Assert.That(trigger.Actions.SpawnTarget).IsNull(); + edits.Apply(TriggersScreenRenderer.Model(sets, 0, Targets).FieldAt(0, 0, TriggersScreenRenderer.RouteField)!.Value, "main"); + await Assert.That(trigger.Actions.SpawnTarget).IsEqualTo(TriggerActions.MainWindow); + edits.Apply(TriggersScreenRenderer.Model(sets, 0, Targets).FieldAt(0, 0, TriggersScreenRenderer.RouteField)!.Value, "trade"); await Assert.That(trigger.Actions.SpawnTarget).IsEqualTo("trade"); @@ -216,7 +223,7 @@ public async Task UpAndDownStepTheKnownWindows_AndTheDrawnRouteFollowsTheBuffer( session.Handle(Key(ConsoleKey.Enter)); await Assert.That(trigger.Actions.SpawnTarget).IsEqualTo("pages"); - // Wrapping backwards off "main" lands on the last window, not on nothing. + // Wrapping backwards off the first entry lands on the last window, not on nothing. session.Handle(Key(ConsoleKey.Enter)); session.Handle(Key(ConsoleKey.Tab)); session.Handle(Key(ConsoleKey.Tab)); @@ -224,6 +231,8 @@ public async Task UpAndDownStepTheKnownWindows_AndTheDrawnRouteFollowsTheBuffer( session.Handle(Key(ConsoleKey.UpArrow)); await Assert.That(session.Focus().Edit!.Value.Text).IsEqualTo("main"); session.Handle(Key(ConsoleKey.UpArrow)); + await Assert.That(session.Focus().Edit!.Value.Text).IsEqualTo("(none)"); + session.Handle(Key(ConsoleKey.UpArrow)); await Assert.That(session.Focus().Edit!.Value.Text).IsEqualTo("trade"); } From 34b31b61d97802689276bef288aa1fc62a3a6101 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 22:42:58 -0500 Subject: [PATCH 4/8] docs: record the legibility floor, the route that keeps a line, and --theme Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- CLAUDE.md | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index dab94d1..31ca561 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -372,6 +372,73 @@ fallbacks) for inline images/maps. and never add a "probably a URL" fallback for an untagged payload. The window id is what stops a link clicked in a background pane sending to whichever character is focused. +- **No colour this client paints is left below a legibility floor, and the floor is applied where the + colour meets the plane it lands on** (`Contrast`, Core; `ChromeInk` + `WorkspacePalette.ReadingPlane`, + Tui; F7 ▸ `keep text legible`, default on). The reported defect — "Freeze being purple against a blue + background" — was one cell of a grid: the bar took its accent from the theme's index 5 and painted it + on the pane, and `#800080` on a `#36363d` focused pane is **1.27:1**. Measured across the grid, **six of + the F2 picker's sixteen names fail 3:1 on the dark theme, nine fail on the light one, and only `grey` + clears both** — which is the finding the whole design turns on: *a palette of fixed hexes cannot serve + two themes*, so the resolution has to happen at the moment of painting rather than the moment of + picking. + - **`Contrast.Legible(fg, plane)` moves a foreground the smallest distance that clears the floor and + returns anything already legible byte-identical.** Direction is the **plane's**, not the colour's — a + dark plane lifts, a light plane darkens — which is what lets one function serve all three themes; the + pivot is relative luminance against 0.18, because `#808080` looks like half way written down and is + 0.216. Hue survives while there is headroom and then **desaturates**, and it has to: pure `#0000ff` + has a relative luminance of 0.0722 and tops out at 1.88:1 on a dark pane *at full blue*, so a rule + holding hue absolutely would leave the commonest unreadable colour in MU\* output unreadable. + - **The floor is 3.0:1 and deliberately not 4.5.** A game's own de-emphasis is spoken in exactly the + colours a 4.5 floor would erase — bright black for asides, `dim` for a status line nobody is meant to + read twice. Three is where a colour stops being invisible, which is the complaint; four and a half is + where it stops being quiet, which is not. + - **The plane handed to `MarkupFormatter` is per *theme*, not per pane.** A pane's plane depends on its + character's tint and on focus, and resolving against *that* would re-format a whole buffer on every + focus move — the expensive path this file reserves for one deliberate keystroke. `ReadingPlane` is the + **extreme of the fourteen** a pane can wear, in the direction of travel, and that is the worst case + rather than an approximation of one: once the foreground is past the background's luminance the ratio + is monotone in the background, so clearing the extreme clears all fourteen. + - **A span carrying a background is measured against *it*.** That plane is known exactly and a + highlight's pair must be judged as a pair; a span with the default background emits none and takes + the pane, so it is measured against the reading plane. `reverse` swaps first. + - **`ScreenPalette` is deliberately not built this way.** Those constants sit on the settings screens' + own fixed backdrop, which no theme moves; measuring them against a theme plane would be measuring + them against a plane they are never painted on. `ChromeInk` is the other four — `Accent`, `Notice`, + `Draft`, `Marker` — and it carries **the plane it was resolved against**, so a renderer can hold a + colour it was *handed* (a world's accent) to the same floor. + - **A fill is not text.** A ribbon segment's accent, a pane tint and F2's swatch are identity or + sample, and lifting them would flatten the thing they exist to say; what has to clear the floor is + the *ink on* them, measured against the fill. `ChromeInk.On` is that, and the header ribbon is where + both rules appear side by side. + - **The audit that found most of this is a test** (`FrameContrastTests`): every emitted SGR pair over + 24 views × 3 themes. Reading the source found four of the offenders; the paint found nine. It exempts + the powerline wedges and box-drawing rules (fill boundaries and dividers), the solid blocks (a swatch + is a colour sample shown as the *pane* will paint it), and the framework's `[dim]`. **The half blocks + are not exempt** — `▌` is the trigger left-rule and the focus marker, and one of them was a real + defect this caught. + - **One thing is outside the floor's reach and is named rather than hidden.** SharpConsoleUI resolves + `[dim]` to a fixed `#808080` through no option we hold: 4.01:1 on Dark, **2.52:1** on Solarized Dark's + focused pane. Reaching it means giving up `[dim]` across every renderer for an explicit floor-checked + grey — a sweep, for a near miss on one theme. The exemption is a named predicate with the number in + it, so whoever does that sweep can delete it and watch the test pass. +- **A trigger's `route` says three things, and the third is "nothing"** (`TriggerActions.MainWindow`, + Core; F2's `route` list). `SpawnTarget = null` has always meant *this rule adds no destination — the + line follows whatever the other matched rules decided*, which is exactly what a highlight rule wants. + F2 labelled it `main`, so it read as a destination, and "highlight it and leave it where it was" looked + like something the screen could not express. It is `(none)` now, and it is what a new rule defaults to. + - **`main` is a real destination** — the matching session's own window. It earns a **reserved word** + rather than being spelt as the window's title because one trigger set is shared by every character + that lists it and a title can only name one of them; nothing collides, since a character's session + window is titled after the character and `main` is only the rail's label for it. + - **Gag suppresses the *default* delivery and nothing else.** Every destination a rule asked for + survives it — always true of a spawn pane (`route: Chat` + gag has always meant "only in Chat"), and + now true of `main`, so `route: main` + gag keeps the line where it used to delete it. + - **Destinations are deduplicated.** They were not, and `WorldSession` raises one `SpawnLine` per + entry, so a highlight rule pointed at the same pane as its capture rule delivered every line twice. + - **A highlight rule needs no route to reach the pane a capture rule sent the line to.** There is one + line and one set of destinations, and every matched rule's highlight is on it. Do not "fix" that into + a per-rule delivery. + ## Building and testing - **.NET 10 SDK**: install via `apt-get install -y dotnet-sdk-10.0` (the Microsoft CDN is often @@ -399,6 +466,14 @@ dotnet run -c Release --project src/SharpMUTerm.Tui --no-build -- \ python3 tools/ansi_frame_to_image.py frame.ansi frame.html # or .svg ``` +- **`--theme ` renders a frame in a built-in flavour** (`Dark` / `Light` / `Solarized Dark`). It + exists because the client's chrome is derived from the theme and held to a legibility floor against it, + and **every frame in the gallery renders Dark** — which is exactly how the Light theme's accent + (1.42:1), draft pen (1.26:1) and notice (1.73:1) stayed unreadable without anybody ever seeing them. + It sets `ThemeName` *and* `Theme`: `ResolveTheme` treats an inline theme whose name disagrees with + `ThemeName` as a *customised* one and prefers it, so setting the name alone would be overruled by the + Dark theme still sitting in `Theme`. + - **`-c Release` on the build, and it is not a formality.** A bare `dotnet build` produces *Debug*, `--no-build` runs the *Release* output, and nothing warns you: the snapshot renders happily from a binary that predates your change, so a new view comes out byte-identical to the default frame and a From 66e590d9a7329417a2ce6f6065332a3ec97e4842 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Tue, 11 Aug 2026 22:45:03 -0500 Subject: [PATCH 5/8] fix(triggers): the rule list reads the route from one place, not two RuleRow had its own `?? "main"` beside the route field's. After the field learned that a null target is *no destination* rather than the main window, the list would have gone on calling it `main` -- the two surfaces disagreeing about one rule, which is the shape of the confusion the rename exists to remove. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- src/SharpMUTerm.Tui/TriggersScreenRenderer.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs b/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs index ddc289f..0919e78 100644 --- a/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs +++ b/src/SharpMUTerm.Tui/TriggersScreenRenderer.cs @@ -580,7 +580,11 @@ private static string RuleRow(int index, int selectedTrigger, Trigger trigger) { var marker = index == selectedTrigger ? "[bold]▸[/]" : " "; var box = trigger.Enabled ? $"[{Accent}]✓[/]" : "[dim]·[/]"; - var target = trigger.Actions.SpawnTarget ?? "main"; + // Through Route, not a second reading of the same field. This row had its own `?? "main"`, so + // after the route field learned that a null target is *no destination* rather than the main + // window, the list would have gone on calling it `main` — the two surfaces disagreeing about one + // rule, which is the shape of the confusion the rename exists to remove. + var target = Route(trigger); return $"{marker} {box} [bold]{Escape(trigger.Name)}[/] [dim]{Escape(trigger.Pattern)}[/] [dim]→ {Escape(target)}[/]"; } From 6012640a0624928726f9f783089816d4f8276978 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Wed, 12 Aug 2026 10:42:28 -0500 Subject: [PATCH 6/8] review: drop the quadratic frame scan, pin the route trim, fix MD028 Three of CodeRabbit's five on PR #31. FrameContrastTests.Pairs asked both regexes at every character. Regex.Match searches *forward*, so each plain character re-scanned the same upcoming escape from a later start -- quadratic in the length of every unstyled run, and a frame is mostly padding. It now only asks at an escape; the 72-case suite goes 1.54s -> 1.32s, and the cost stops scaling with frame size. The route trim: the finding was that `v == NoRoute` compares before `v.Trim()`, so a padded "(none)" would be stored as a capture pane by that name. It is not -- ScreenField.WindowName's Set is `value => set(value.Trim())`, so the lambda is handed an already-trimmed value and the `v.Trim()` in it is redundant. Pinned rather than changed, since the two halves of that live in different files and nothing else held them together. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- ...gible-colour-and-trigger-routing-design.md | 2 +- .../FrameContrastTests.cs | 32 +++++++++++++++---- .../TriggersScreenEditingTests.cs | 15 ++++++++- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md b/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md index 048c1e8..c0aaec5 100644 --- a/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md +++ b/docs/superpowers/specs/2026-08-11-legible-colour-and-trigger-routing-design.md @@ -7,7 +7,7 @@ Two reported defects that turn out to share a cause. > "we are using unreadable colors by default against our backgrounds. Such as Freeze > being purple against a blue background." - +> > "we failed to solve the issue of Triggers — we should be able to Highlight text and > send it to the pane where we found it. That way, players can highlight character name > text." diff --git a/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs b/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs index a9966c1..c110ff2 100644 --- a/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs @@ -128,6 +128,16 @@ private static string Render(string themeName, string view) for (var i = 0; i < frame.Length;) { + // Only ask the regexes at an escape. `Regex.Match(input, startat)` searches *forward* to the + // next match anywhere in the rest of the string, so calling it at every plain character + // re-scans the same upcoming sequence from progressively later positions — quadratic in the + // length of each unstyled run, and a frame is mostly padding spaces. + if (frame[i] != '\u001b') + { + Count(pairs, frame[i++], fg, bg); + continue; + } + var sgr = Sgr.Match(frame, i); if (sgr.Success && sgr.Index == i) { @@ -143,18 +153,26 @@ private static string Render(string themeName, string view) continue; } - var ch = frame[i++]; - if (ch is ' ' or '\n' or '\r' || IsFill(ch) || fg is not { } ink || bg is not { } plane) - { - continue; - } - - pairs[(ink, plane)] = pairs.GetValueOrDefault((ink, plane)) + 1; + // An escape this walker does not recognise: consume the byte rather than the sequence, which + // is the same thing the loop did before and is safe because both patterns are anchored to a + // real CSI introducer. + Count(pairs, frame[i++], fg, bg); } return pairs; } + /// Records one painted cell, skipping the ones a text floor does not apply to. + private static void Count(Dictionary<(Rgb, Rgb), int> pairs, char ch, Rgb? fg, Rgb? bg) + { + if (ch is ' ' or '\n' or '\r' || IsFill(ch) || fg is not { } ink || bg is not { } plane) + { + return; + } + + pairs[(ink, plane)] = pairs.GetValueOrDefault((ink, plane)) + 1; + } + private static void Apply(string parameters, ref Rgb? fg, ref Rgb? bg) { var codes = parameters.Split(';') diff --git a/tests/SharpMUTerm.Tui.Tests/TriggersScreenEditingTests.cs b/tests/SharpMUTerm.Tui.Tests/TriggersScreenEditingTests.cs index 821e366..2de9c5f 100644 --- a/tests/SharpMUTerm.Tui.Tests/TriggersScreenEditingTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/TriggersScreenEditingTests.cs @@ -118,8 +118,21 @@ public async Task ChoosingNoRouteClearsTheSpawnTargetAndChoosingMainDoesNot() edits.Apply(TriggersScreenRenderer.Model(sets, 0, Targets).FieldAt(0, 0, TriggersScreenRenderer.RouteField)!.Value, "trade"); await Assert.That(trigger.Actions.SpawnTarget).IsEqualTo("trade"); + // Padded, because the two halves of that decision live in different files: the comparison here + // is against the bare label, and the trimming is ScreenField.WindowName's (its Set is + // `value => set(value.Trim())`). A reviewer read this line alone and concluded a padded + // "(none)" would be stored as a capture pane by that name; it is not, and this is what keeps + // that true if the field ever stops trimming for us. + edits.Apply(TriggersScreenRenderer.Model(sets, 0, Targets).FieldAt(0, 0, TriggersScreenRenderer.RouteField)!.Value, " (none) "); + await Assert.That(trigger.Actions.SpawnTarget).IsNull(); + + // And the same for a real destination: what is stored is the name, never the padding, or two + // rules aimed at one pane would open two. + edits.Apply(TriggersScreenRenderer.Model(sets, 0, Targets).FieldAt(0, 0, TriggersScreenRenderer.RouteField)!.Value, " Chat "); + await Assert.That(trigger.Actions.SpawnTarget).IsEqualTo("Chat"); + edits.Revert(); - await Assert.That(trigger.Actions.SpawnTarget).IsEqualTo("trade"); // kept as committed + await Assert.That(trigger.Actions.SpawnTarget).IsEqualTo("Chat"); // the last commit, kept } /// From c5b8f04795ecd7cac2a06dfa4ca7c5c2d79a52d1 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Wed, 12 Aug 2026 13:01:40 -0500 Subject: [PATCH 7/8] review: audit the frame's grid, not its escape stream CodeRabbit's outside-diff finding, and it was right about the premise: a frame is cursor-addressed, so a walker that reads it linearly counts every glyph the driver *wrote* rather than the ones left on screen. The literal suggestion (use FrameGrid.Decode) is not implementable -- Decode keeps glyphs and drops colour, and Backgrounds keeps backgrounds and drops foregrounds, so neither can answer a contrast question. FrameGrid gains a colour-aware `Cells` instead, and FrameContrastTests drops its own walker for it. That also stops this suite being the third copy of a parser FrameGrid's own remarks warn about ("a suite going quietly green on a frame it has misread"). Measured before changing: on all 72 genuine per-view frames the two walks agree on the pair set exactly. The linear walk was a superset of the screen, so it could have raised a false alarm but never missed an offender -- which is the safe direction for an audit, and is why this is a tidy-up rather than a fix. Also adds the guard that matters when swapping walkers: a walker handed the wrong dimensions decodes nothing, and an audit over nothing passes. Counted in cells rather than distinct pairs -- a settings screen is painted from the fixed ScreenPalette and is legitimately down to four pairs (F1's composer), while the thinnest real frame still paints 201 glyphs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- .../FrameContrastTests.cs | 121 +++++----------- tests/SharpMUTerm.Tui.Tests/FrameGrid.cs | 129 ++++++++++++++++++ 2 files changed, 164 insertions(+), 86 deletions(-) diff --git a/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs b/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs index c110ff2..ece0c9c 100644 --- a/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs +++ b/tests/SharpMUTerm.Tui.Tests/FrameContrastTests.cs @@ -1,5 +1,3 @@ -using System.Text; -using System.Text.RegularExpressions; using SharpConsoleUI.Drivers; using SharpMUTerm.Core.Text; using SharpMUTerm.Core.Theming; @@ -26,6 +24,12 @@ namespace SharpMUTerm.Tui.Tests; [NotInParallel] public class FrameContrastTests { + /// The frame the audit renders. Shared with , which decodes a grid of + /// exactly this size — a walker told the wrong dimensions drops cells off the edge silently. + private const int Width = 140; + + private const int Height = 36; + private static readonly TerminalCapabilities Headless = new(GraphicsProtocol.None, supportsTrueColor: true, supportsKittyGraphics: false, supportsSixel: false); @@ -50,8 +54,22 @@ from view in Views public async Task NoPaintedTextIsBelowTheLegibilityFloor((string Theme, string View) test) { var frame = Render(test.Theme, test.View); - - var failures = Pairs(frame) + var pairs = Pairs(frame); + + // A walker handed the wrong dimensions, or one that stopped recognising an escape, decodes + // nothing — and an audit over nothing passes every assertion below it. That is the failure + // FrameGrid's own remarks warn about ("quietly green on a frame it has misread"), and it is + // precisely what a floor assertion cannot notice, so it is asserted separately. + // + // Counted in *cells* rather than in distinct pairs: a settings screen is painted from the fixed + // ScreenPalette and is legitimately down to four colour pairs (F1's composer), while the + // thinnest real frame here still paints 201 glyphs. A pair-count floor would be policing how + // colourful a view is, which is not what this is for. + var painted = pairs.Values.Sum(); + await Assert.That(painted).IsGreaterThanOrEqualTo(100) + .Because($"the frame decoded to {painted} painted cells, too few to be a real one"); + + var failures = pairs .Select(pair => (pair.Key.Fg, pair.Key.Bg, pair.Value, Ratio: Contrast.Ratio(pair.Key.Fg, pair.Key.Bg))) .Where(t => t.Ratio < Contrast.Floor) .Where(t => !IsFrameworkDim(t.Fg)) @@ -107,104 +125,35 @@ private static string Render(string themeName, string view) config.ThemeName = themeName; config.Theme = ThemeLibrary.Get(themeName); - var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(140, 36)); + var app = new SharpMUTermApp(config, Headless, new HeadlessConsoleDriver(Width, Height)); return app.RenderSnapshot(view.Length == 0 ? null : view); } - private static readonly Regex Sgr = new(@"\x1b\[([0-9;]*)m", RegexOptions.Compiled); - - private static readonly Regex Csi = new(@"\x1b\[[0-9;?]*[A-Za-z]", RegexOptions.Compiled); - /// - /// Every (foreground, background) pair the frame actually paints a glyph in, with a cell count. - /// Spaces are skipped — a space has no foreground to read — and so is everything - /// names. + /// Every (foreground, background) pair the frame paints a glyph in, with a cell count — read off the + /// grid () rather than the escape stream, because the frame is + /// cursor-addressed and a cell that was painted and then overwritten is not on screen. The two agree + /// exactly on all 72 frames here; asking the grid is what makes that a fact rather than a hope. + /// + /// Spaces are skipped — a space has no foreground to read — and so is everything + /// names. + /// /// private static Dictionary<(Rgb Fg, Rgb Bg), int> Pairs(string frame) { var pairs = new Dictionary<(Rgb, Rgb), int>(); - Rgb? fg = null; - Rgb? bg = null; - for (var i = 0; i < frame.Length;) + foreach (var cell in FrameGrid.Cells(frame, Width, Height).Values) { - // Only ask the regexes at an escape. `Regex.Match(input, startat)` searches *forward* to the - // next match anywhere in the rest of the string, so calling it at every plain character - // re-scans the same upcoming sequence from progressively later positions — quadratic in the - // length of each unstyled run, and a frame is mostly padding spaces. - if (frame[i] != '\u001b') + if (cell.Glyph is ' ' or '\n' or '\r' || IsFill(cell.Glyph) + || cell.Foreground is not { } ink || cell.Background is not { } plane) { - Count(pairs, frame[i++], fg, bg); continue; } - var sgr = Sgr.Match(frame, i); - if (sgr.Success && sgr.Index == i) - { - Apply(sgr.Groups[1].Value, ref fg, ref bg); - i = sgr.Index + sgr.Length; - continue; - } - - var csi = Csi.Match(frame, i); - if (csi.Success && csi.Index == i) - { - i = csi.Index + csi.Length; - continue; - } - - // An escape this walker does not recognise: consume the byte rather than the sequence, which - // is the same thing the loop did before and is safe because both patterns are anchored to a - // real CSI introducer. - Count(pairs, frame[i++], fg, bg); + pairs[(ink, plane)] = pairs.GetValueOrDefault((ink, plane)) + 1; } return pairs; } - - /// Records one painted cell, skipping the ones a text floor does not apply to. - private static void Count(Dictionary<(Rgb, Rgb), int> pairs, char ch, Rgb? fg, Rgb? bg) - { - if (ch is ' ' or '\n' or '\r' || IsFill(ch) || fg is not { } ink || bg is not { } plane) - { - return; - } - - pairs[(ink, plane)] = pairs.GetValueOrDefault((ink, plane)) + 1; - } - - private static void Apply(string parameters, ref Rgb? fg, ref Rgb? bg) - { - var codes = parameters.Split(';') - .Where(p => p.Length > 0) - .Select(int.Parse) - .ToList(); - - for (var i = 0; i < codes.Count;) - { - if (codes[i] == 0) - { - fg = bg = null; - i++; - } - else if (codes[i] is 38 or 48 && i + 4 < codes.Count && codes[i + 1] == 2) - { - var colour = new Rgb((byte)codes[i + 2], (byte)codes[i + 3], (byte)codes[i + 4]); - if (codes[i] == 38) - { - fg = colour; - } - else - { - bg = colour; - } - - i += 5; - } - else - { - i++; - } - } - } } diff --git a/tests/SharpMUTerm.Tui.Tests/FrameGrid.cs b/tests/SharpMUTerm.Tui.Tests/FrameGrid.cs index 3effe60..22d86d7 100644 --- a/tests/SharpMUTerm.Tui.Tests/FrameGrid.cs +++ b/tests/SharpMUTerm.Tui.Tests/FrameGrid.cs @@ -1,3 +1,6 @@ +using System.Globalization; +using SharpMUTerm.Core.Text; + namespace SharpMUTerm.Tui.Tests; /// @@ -89,6 +92,132 @@ internal static IReadOnlyList Decode(string frame, int width, int height return lines; } + /// One painted cell: the glyph on screen and the two colours it was written in. + internal readonly record struct Cell(char Glyph, Rgb? Foreground, Rgb? Background); + + /// + /// The frame as a {(row, column): cell} grid — the glyph and both its colours, which is + /// what separates this from (glyphs only) and + /// (backgrounds only). + /// + /// It is a grid rather than a stream, and that is the point. The frame is cursor-addressed, so + /// a walker that reads it linearly counts every glyph the driver wrote rather than the ones + /// left on screen; a cell painted and then overwritten would be counted twice, in two + /// different colours. That direction of error is safe for a contrast audit — the stream is a superset + /// of the screen, so it can raise a false alarm but never miss an offender — and on the 72 frames + /// walks the two agree exactly, glyph for glyph. Asking the grid + /// anyway costs nothing and removes the caveat. + /// + /// + /// Only the escape positions are handed to a regex. Regex.Match(input, startat) searches + /// forward to the next match anywhere in the rest of the string, so asking at every character + /// re-scans each upcoming sequence from progressively later starts — quadratic in the length of every + /// unstyled run, and a frame is mostly padding spaces. + /// + /// + internal static Dictionary<(int Row, int Column), Cell> Cells(string frame, int width, int height) + { + ArgumentNullException.ThrowIfNull(frame); + + var cells = new Dictionary<(int, int), Cell>(); + Rgb? foreground = null; + Rgb? background = null; + int row = 0, column = 0, i = 0; + + while (i < frame.Length) + { + if (frame[i] == '\u001b') + { + var sgr = SgrPattern.Match(frame, i); + if (sgr.Success && sgr.Index == i) + { + ApplySgr(sgr.Groups[1].Value, ref foreground, ref background); + i = sgr.Index + sgr.Length; + continue; + } + + var csi = CsiPattern.Match(frame, i); + if (csi.Success && csi.Index == i) + { + if (frame[csi.Index + csi.Length - 1] == 'H') + { + var at = csi.Groups[1].Value.Split(';'); + row = at.Length > 0 && at[0].Length > 0 ? int.Parse(at[0], CultureInfo.InvariantCulture) - 1 : 0; + column = at.Length > 1 && at[1].Length > 0 ? int.Parse(at[1], CultureInfo.InvariantCulture) - 1 : 0; + } + + i = csi.Index + csi.Length; + continue; + } + } + + var ch = frame[i++]; + if (ch == '\n') + { + row++; + column = 0; + continue; + } + + if (ch == '\r') + { + column = 0; + continue; + } + + if (row >= 0 && row < height && column >= 0 && column < width) + { + cells[(row, column)] = new Cell(ch, foreground, background); + } + + column++; + } + + return cells; + } + + private static readonly System.Text.RegularExpressions.Regex SgrPattern = + new(@"\x1b\[([0-9;]*)m", System.Text.RegularExpressions.RegexOptions.Compiled); + + private static readonly System.Text.RegularExpressions.Regex CsiPattern = + new(@"\x1b\[([0-9;?]*)[A-Za-z]", System.Text.RegularExpressions.RegexOptions.Compiled); + + /// Applies one SGR sequence's parameters to the running foreground and background. + private static void ApplySgr(string parameters, ref Rgb? foreground, ref Rgb? background) + { + var codes = parameters.Split(';') + .Where(p => p.Length > 0) + .Select(p => int.Parse(p, CultureInfo.InvariantCulture)) + .ToList(); + + for (var i = 0; i < codes.Count;) + { + if (codes[i] == 0) + { + foreground = background = null; + i++; + } + else if (codes[i] is 38 or 48 && i + 4 < codes.Count && codes[i + 1] == 2) + { + var colour = new Rgb((byte)codes[i + 2], (byte)codes[i + 3], (byte)codes[i + 4]); + if (codes[i] == 38) + { + foreground = colour; + } + else + { + background = colour; + } + + i += 5; + } + else + { + i++; + } + } + } + /// The truecolor background escape a colour is written as, e.g. 48;2;51;57;76. internal static string Sgr(SharpConsoleUI.Color color) => $"48;2;{color.R};{color.G};{color.B}"; From fa0720350a8a382a33c1ef7a9a94ca13050478de Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Wed, 12 Aug 2026 13:17:19 -0500 Subject: [PATCH 8/8] review: one SGR walk, and it handles the resets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit was right that ApplySgr mistracked three legal sequences: `CSI m` (ECMA-48 makes it `CSI 0 m`, and splitting an empty string yields no codes, so the loop ran no body and both colours stayed standing), and `39`/`49`, which return one channel to default. None appears in any frame this driver emits -- 19,681 SGR sequences across the 72 frames, all of them explicit `0;38;2;…` -- so it corrected no live reading. It is fixed because this walker is what every suite asking about painted cells goes through. Chasing it found a live one, and my first fix for it was wrong in the same way. Backgrounds tested for the reset with `parameters.Contains("49")`, which reads the 49 in a truecolor *argument* -- `38;2;49;5;6`, a foreground whose red channel is 49 -- as the reset code and clears a background that sequence never mentions. Splitting on `;` does not see that either: only a walk that consumes `38;2;r;g;b` as one unit can tell an SGR code from a colour argument. So Backgrounds is now a projection of the same walk Cells is, which is what this file's own remarks already argued for ("three copies of a parser can drift into disagreeing about which cells are painted, and the failure that produces is a suite going quietly green on a frame it has misread"). It was three; it is one. FrameGridCellsTests covers the walk directly on hand-written frames. Three of its nine fail against the unfixed parser, verified by reverting it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015nuKnWthnELNkrd86q5KWN --- tests/SharpMUTerm.Tui.Tests/FrameGrid.cs | 109 ++++++++------ .../FrameGridCellsTests.cs | 140 ++++++++++++++++++ 2 files changed, 205 insertions(+), 44 deletions(-) create mode 100644 tests/SharpMUTerm.Tui.Tests/FrameGridCellsTests.cs diff --git a/tests/SharpMUTerm.Tui.Tests/FrameGrid.cs b/tests/SharpMUTerm.Tui.Tests/FrameGrid.cs index 22d86d7..c25e361 100644 --- a/tests/SharpMUTerm.Tui.Tests/FrameGrid.cs +++ b/tests/SharpMUTerm.Tui.Tests/FrameGrid.cs @@ -120,6 +120,21 @@ internal static IReadOnlyList Decode(string frame, int width, int height ArgumentNullException.ThrowIfNull(frame); var cells = new Dictionary<(int, int), Cell>(); + foreach (var (at, cell) in Walk(frame, width, height)) + { + cells[at] = cell; + } + + return cells; + } + + /// + /// The one walk: every glyph the frame writes, in order, with the position and the colours in force + /// when it was written. Both and are projections of + /// it, so the suite cannot come to two views of one frame. + /// + private static IEnumerable<((int Row, int Column) At, Cell Cell)> Walk(string frame, int width, int height) + { Rgb? foreground = null; Rgb? background = null; int row = 0, column = 0, i = 0; @@ -167,13 +182,11 @@ internal static IReadOnlyList Decode(string frame, int width, int height if (row >= 0 && row < height && column >= 0 && column < width) { - cells[(row, column)] = new Cell(ch, foreground, background); + yield return ((row, column), new Cell(ch, foreground, background)); } column++; } - - return cells; } private static readonly System.Text.RegularExpressions.Regex SgrPattern = @@ -182,9 +195,36 @@ internal static IReadOnlyList Decode(string frame, int width, int height private static readonly System.Text.RegularExpressions.Regex CsiPattern = new(@"\x1b\[([0-9;?]*)[A-Za-z]", System.Text.RegularExpressions.RegexOptions.Compiled); - /// Applies one SGR sequence's parameters to the running foreground and background. + /// + /// Applies one SGR sequence's parameters to the running foreground and background. + /// + /// Empty parameters are a reset. ECMA-48 makes CSI m equivalent to CSI 0 m, and + /// a parser that split an empty string into no codes would run no loop body and leave both colours + /// standing — so a glyph after it would be audited in colours it is not wearing. Likewise 39 + /// and 49, which return one channel to the terminal's default without touching the other. + /// + /// + /// None of the three appears in any frame this driver emits — it always writes an explicit + /// 0;38;2;…;48;2;… — so this corrects no live reading. It is here because this walker is the + /// one every suite that asks about painted cells goes through, and a parser that quietly mistracks + /// state on a legal sequence is the "quietly green on a frame it has misread" failure this file + /// warns about, one function down. + /// + /// + /// The indexed forms (38;5;n) are not decoded, and degrade rather than corrupt: the + /// 2 guard below fails, so the code and its arguments fall through as unknowns and the colour + /// stays as it was. This driver emits truecolor only; building the 256-colour cube for a sequence + /// nothing writes would be inventing coverage. + /// + /// private static void ApplySgr(string parameters, ref Rgb? foreground, ref Rgb? background) { + if (parameters.Length == 0) + { + foreground = background = null; + return; + } + var codes = parameters.Split(';') .Where(p => p.Length > 0) .Select(p => int.Parse(p, CultureInfo.InvariantCulture)) @@ -197,6 +237,16 @@ private static void ApplySgr(string parameters, ref Rgb? foreground, ref Rgb? ba foreground = background = null; i++; } + else if (codes[i] == 39) + { + foreground = null; + i++; + } + else if (codes[i] == 49) + { + background = null; + i++; + } else if (codes[i] is 38 or 48 && i + 4 < codes.Count && codes[i + 1] == 2) { var colour = new Rgb((byte)codes[i + 2], (byte)codes[i + 3], (byte)codes[i + 4]); @@ -237,48 +287,19 @@ private static void ApplySgr(string parameters, ref Rgb? foreground, ref Rgb? ba { ArgumentNullException.ThrowIfNull(ansi); + // Derived from Cells rather than walked again. This used to be its own parser, and the two had + // already drifted: it tested for the background reset with `parameters.Contains("49")`, which + // reads the 49 in a truecolor *argument* — `38;2;49;5;6`, a foreground whose red channel is 49 — + // as the reset code, and clears a background that sequence never mentions. Splitting on `;` is + // not enough to see that either; only a walk that consumes `38;2;r;g;b` as one unit can tell an + // SGR code from a colour argument, and Cells already does. + // + // Unbounded, as this always was: callers ask about cells at coordinates the frame chose, so + // there is no width and height to clamp to here. var cells = new Dictionary<(int, int), string?>(); - var current = (string?)null; - var (row, column) = (0, 0); - - foreach (System.Text.RegularExpressions.Match token in - System.Text.RegularExpressions.Regex.Matches(ansi, @"\x1b\[([0-9;]*)([A-Za-z])|([^\x1b\r\n])|(\n)")) + foreach (var (at, cell) in Walk(ansi, int.MaxValue, int.MaxValue)) { - if (token.Groups[4].Success) - { - row++; - column = 0; - continue; - } - - if (token.Groups[3].Success) - { - cells[(row, column)] = current; - column++; - continue; - } - - var parameters = token.Groups[1].Value; - switch (token.Groups[2].Value) - { - case "H": - var at = parameters.Split(';'); - row = at[0].Length > 0 ? int.Parse(at[0]) - 1 : 0; - column = at.Length > 1 && at[1].Length > 0 ? int.Parse(at[1]) - 1 : 0; - break; - case "m": - if (parameters.Length == 0 || parameters == "0" || parameters.Contains("49")) - { - current = null; - } - - if (parameters.Contains("48;2;")) - { - current = parameters[parameters.IndexOf("48;2;", StringComparison.Ordinal)..]; - } - - break; - } + cells[at] = cell.Background is { } bg ? $"48;2;{bg.R};{bg.G};{bg.B}" : null; } return cells; diff --git a/tests/SharpMUTerm.Tui.Tests/FrameGridCellsTests.cs b/tests/SharpMUTerm.Tui.Tests/FrameGridCellsTests.cs new file mode 100644 index 0000000..beed0e9 --- /dev/null +++ b/tests/SharpMUTerm.Tui.Tests/FrameGridCellsTests.cs @@ -0,0 +1,140 @@ +using SharpMUTerm.Core.Text; + +namespace SharpMUTerm.Tui.Tests; + +/// +/// Direct coverage of — the walker every contrast assertion goes +/// through, driven with hand-written frames rather than rendered ones. +/// +/// It is worth its own file because this parser's failure mode is silence. A walker that mistracks +/// colour, or decodes nothing at all, does not throw: it hands back cells nobody painted, or no cells, +/// and an audit over either passes. So the sequences it must get right are asserted here, on frames +/// small enough to read, instead of being inferred from a suite staying green. +/// +/// +/// None of the reset forms below appear in a frame this driver emits — it always writes an +/// explicit 0;38;2;…;48;2;…. They are covered because they are legal, because this walker is +/// shared, and because the cost of finding out otherwise is a suite that has been reading frames wrong +/// for a while without anything going red. +/// +/// +public class FrameGridCellsTests +{ + private const int Width = 20; + private const int Height = 3; + + private static string At(int row, int column) => $"[{row};{column}H"; + + private static string Fg(int r, int g, int b) => $"[38;2;{r};{g};{b}m"; + + private static string Bg(int r, int g, int b) => $"[48;2;{r};{g};{b}m"; + + private static FrameGrid.Cell Cell(string frame, int row, int column) => + FrameGrid.Cells(frame, Width, Height)[(row, column)]; + + [Test] + public async Task AGlyphCarriesTheColoursInForceWhenItWasWritten() + { + var frame = At(1, 1) + Fg(1, 2, 3) + Bg(4, 5, 6) + "x"; + + var cell = Cell(frame, 0, 0); + + await Assert.That(cell.Glyph).IsEqualTo('x'); + await Assert.That(cell.Foreground).IsEqualTo(new Rgb(1, 2, 3)); + await Assert.That(cell.Background).IsEqualTo(new Rgb(4, 5, 6)); + } + + [Test] + public async Task AnEmptySgrIsAReset() + { + // ECMA-48: `CSI m` is `CSI 0 m`. Splitting an empty parameter string yields no codes, so a loop + // over them runs no body — and the colours from the previous span stay standing, which makes + // every glyph after it read as painted in colours it is not wearing. + var frame = At(1, 1) + Fg(1, 2, 3) + Bg(4, 5, 6) + "a" + "" + "b"; + + await Assert.That(Cell(frame, 0, 1).Glyph).IsEqualTo('b'); + await Assert.That(Cell(frame, 0, 1).Foreground).IsNull(); + await Assert.That(Cell(frame, 0, 1).Background).IsNull(); + } + + [Test] + public async Task ThirtyNineClearsTheForegroundAndLeavesTheBackground() + { + var frame = At(1, 1) + Fg(1, 2, 3) + Bg(4, 5, 6) + "a" + "" + "b"; + + var cell = Cell(frame, 0, 1); + + await Assert.That(cell.Foreground).IsNull(); + await Assert.That(cell.Background).IsEqualTo(new Rgb(4, 5, 6)); + } + + [Test] + public async Task FortyNineClearsTheBackgroundAndLeavesTheForeground() + { + var frame = At(1, 1) + Fg(1, 2, 3) + Bg(4, 5, 6) + "a" + "" + "b"; + + var cell = Cell(frame, 0, 1); + + await Assert.That(cell.Foreground).IsEqualTo(new Rgb(1, 2, 3)); + await Assert.That(cell.Background).IsNull(); + } + + [Test] + public async Task AFortyNineInsideATruecolorTripleIsNotAReset() + { + // `38;2;49;5;6` is a *foreground* whose red channel is 49. Backgrounds tested for the reset code + // with `parameters.Contains("49")`, which reads that as "return the background to default" and + // clears a background the sequence never mentioned. It survived unnoticed because the branch + // below it re-set the background whenever the same sequence also carried a `48;2;` — so only a + // foreground-only span could show it, and this driver does not emit one. + var frame = At(1, 1) + Bg(4, 5, 6) + "a" + "" + "b"; + + await Assert.That(FrameGrid.Backgrounds(frame)[(0, 1)]).IsEqualTo("48;2;4;5;6"); + } + + [Test] + public async Task BackgroundsHonoursAStandaloneFortyNine() + { + // The other half of the same line: a real 49 must still clear the background. + var frame = At(1, 1) + Bg(4, 5, 6) + "a" + "" + "b"; + + await Assert.That(FrameGrid.Backgrounds(frame)[(0, 1)]).IsNull(); + } + + [Test] + public async Task ACellPaintedTwiceReportsTheLastColourWrittenToIt() + { + // The property that makes this a *grid* and not a stream, and the whole reason the contrast + // audit goes through it: what is audited is what is left on screen, not everything the driver + // wrote on the way there. + var frame = At(1, 1) + Fg(1, 2, 3) + Bg(4, 5, 6) + "a" + + At(1, 1) + Fg(7, 8, 9) + Bg(10, 11, 12) + "z"; + + var cells = FrameGrid.Cells(frame, Width, Height); + + await Assert.That(cells[(0, 0)].Glyph).IsEqualTo('z'); + await Assert.That(cells[(0, 0)].Foreground).IsEqualTo(new Rgb(7, 8, 9)); + await Assert.That(cells.Count).IsEqualTo(1); + } + + [Test] + public async Task AGlyphPastTheEdgeIsDroppedRatherThanThrowing() + { + var frame = At(1, 1) + Fg(1, 2, 3) + Bg(4, 5, 6) + new string('x', Width + 10); + + var cells = FrameGrid.Cells(frame, Width, Height); + + await Assert.That(cells.Count).IsEqualTo(Width); + } + + [Test] + public async Task AnUndecodedIndexedColourLeavesTheColourAloneRatherThanCorruptingIt() + { + // 38;5;n is legal and this driver never writes it. What matters is that meeting one degrades — + // the code and its arguments fall through as unknowns — rather than being read as a truecolor + // triple and painting a cell in a colour nothing chose. + var frame = At(1, 1) + Fg(1, 2, 3) + Bg(4, 5, 6) + "a" + "" + "b"; + + await Assert.That(Cell(frame, 0, 1).Foreground).IsEqualTo(new Rgb(1, 2, 3)); + } +}