diff --git a/.agents/skills/code-review/SKILL.md b/.agents/skills/code-review/SKILL.md index 7b3d29c3..1594097a 100644 --- a/.agents/skills/code-review/SKILL.md +++ b/.agents/skills/code-review/SKILL.md @@ -59,7 +59,8 @@ they mean, rather than testing for the variant **Layering.** `forbidden/2` in `kb.pl` is the authority: `improvise-core` no UI/IO, `improvise-io` no UI/commands, `improvise-command` no ratatui/crossterm. A ratatui type in core is the usual violation — the fix -is a semantic enum in core that the render layer maps (see `AxisColor`). +is a semantic value the render layer maps; for appearance that is a `Face` in +`improvise-protocol`, resolved by `draw.rs::face_style` (ADR 004). **`Option`/`Result`/`Box` at the boundaries.** Wrap at parse/lookup/IO, unwrap early, keep core logic on concrete values. Heap-container plumbing should be diff --git a/context/code/clients.md b/context/code/clients.md index 08f49850..5a359f1f 100644 --- a/context/code/clients.md +++ b/context/code/clients.md @@ -1652,7 +1652,7 @@ through the server's own types; with one definition there is nothing left for them to compare, which is what improvise-xks6 was for. **`rejection_message` is spelled here rather than on `Rejection`** for the -reason `AxisColor` is not a ratatui colour: the protocol carries the *fact*, and +reason a `Face` is not a ratatui colour: the protocol carries the *fact*, and each front end words it. The wording tracks ADR 001's force table, because "try again" and "this cannot be forced" are the two different things a user must be able to tell apart — a `RecordGone` has nothing to write to, and offering force @@ -1877,8 +1877,8 @@ Four smaller decisions that would be re-derived wrongly: `improvise_protocol::read_buffer` so the `"search"` special case keeps its one home (gotcha 11); `None` is "no text-entry mode is open", so the page asks nothing about the mode. The colour stays semantic — `MinibufColor` crosses and - the CSS class is the page's last translation, exactly as `draw.rs::minibuf_fg` - is the TUI's (gotcha 26). It is drawn as a surface of its own rather than *in + the CSS class is the page's last translation. (The TUI no longer reads it: it + draws a face per text-entry mode, and improvise-nn4s.4 moves the page there.) It is drawn as a surface of its own rather than *in place of* `#status` as the TUI's bottom bar does, because `#status` also carries the transport's failures and a repaint must not erase one (gotcha 48). Since improvise-n6t8 it shares `#bottom-dock` with the toast, which stacks @@ -1920,7 +1920,7 @@ home for it. The rows themselves are `improvise_protocol::help::pages()`, the same ones `src/ui/help.rs` draws. `RenderHelpRow` carries a `kind` string (`heading`, `keys`, `accent`, …) which is a *semantic* name the page maps to a -CSS class — the `AxisColor → axis_fg` shape again, and the reason the page can +CSS class — the `Face → draw.rs::face_style` shape again, and the reason the page can style a heading without knowing what makes a line a heading. Where the content comes from and what is derived from the keymap is `context/code/protocol.md` §8. @@ -2353,7 +2353,7 @@ truncation. A browser has no column to fit, so the abbreviation buys nothing and reads to a user as a rendering defect — it was spotted in PR #120's screenshots as "Quarter Pag". `render.rs` now asks `plain_label`, which is the other vocabulary already on `Axis`: the decision stays in the shared layer and only -the *choice* of vocabulary is local, exactly the `AxisColor → ratatui::Color` +the *choice* of vocabulary is local, exactly the `Face → ratatui::Style` shape §5 blesses. `short_label` is untouched, because the TUI's width is load-bearing. @@ -2439,8 +2439,8 @@ So it is now a raised bar: fixed, centred over the grid, larger, bordered in the mode's own colour and shadowed. **Every part of that is paint.** The content is `RenderMinibuffer`, built in `render.rs` from `state.view.mode.minibuffer()` — the same semantic value the TUI reads — and the colour is `MinibufColor` -resolved to a concrete colour exactly as `draw.rs::minibuf_fg` resolves it for -the terminal. Nothing here asks which mode is open, which buffer is being typed +resolved to a concrete colour by `style.css`. (The TUI now draws a face per +text-entry mode instead; improvise-nn4s.4 moves the page onto the same faces.) Nothing here asks which mode is open, which buffer is being typed into, or what the prompt says; a second answer to any of those is the disease design-principles §5 is about. The caret is drawn as a CSS `::after` rather than appended to the text for the same reason: the text is the host's, and the page diff --git a/context/code/core.md b/context/code/core.md index b4d667c1..8bbbe391 100644 --- a/context/code/core.md +++ b/context/code/core.md @@ -359,12 +359,14 @@ browser symptom and silently break drill. `Axis` also owns four presentation vocabularies — `plain_label()`, `glyph()` (which `Display` appends, giving "Row ↕"), `short_label()` (three columns for the tile bar; `None` is "·", **not** a truncation of "None") and `initial()` -(one letter for the view panel; `None` is empty) — plus `color() -> AxisColor`. -`AxisColor` is a *semantic* palette because `improvise-core` must not name a -ratatui type; `draw.rs::axis_fg` does the one mapping, exactly as `minibuf_fg` -does for `MinibufColor` (improvise-rml). The tile bar and the category panel each -carried a copy of the colour table before, with nothing keeping them in -agreement. +(one letter for the view panel; `None` is empty). Its **colour is no longer +here**. It was `color() -> AxisColor` (improvise-rml), a semantic palette that +lived in core only because it was a method on a core type; ADR 004 clause 6 +moved it to `improvise-protocol` as `Face::Axis(axis)`, because a face is UI and +this crate is the one that must have none. The TUI resolves it through +`draw.rs::face_style` (`context/code/tui.md`). The tile bar and the category +panel each carried a copy of the colour table before improvise-rml, with nothing +keeping them in agreement, and they still share one: the face. ### `on_category_added`: a virtual on an axis is a configuration, not a vacancy diff --git a/context/code/protocol.md b/context/code/protocol.md index 044ddb9c..30f4ec7d 100644 --- a/context/code/protocol.md +++ b/context/code/protocol.md @@ -1311,8 +1311,8 @@ re-derived from `kind`, so the two ends cannot disagree about what "Measure rather than re-exported, because this crate must not depend on `improvise-io` (the wizard is an io type, gotcha 30) and a client that draws the wizard must still be able to name the step it is on. `WizardFieldKind` is the semantic -half of a colour decision — the terminal mapping lives in `draw.rs`, exactly -as `AxisColor`/`axis_fg` does (gotcha 26). +half of a colour decision: it indexes the `wizard.field.*` faces (§10), and the +terminal mapping is `draw.rs::face_style`. ### `Projection` carries no multi-widget variant @@ -2374,7 +2374,7 @@ deserializes now carries no English at all. `Hint { keys, command, caption }` is the unit: `keys` come off `Keymaps::default_keymaps()`, `command` is the semantic value a localized or non-terminal front end keys off, and `caption` is the default English — exactly -the `AxisColor`/`draw.rs::axis_fg` shape, with the decision shared and only the +the shape faces now have (`Face` → `draw.rs::face_style`, §10), with the decision shared and only the last translation local (`invariant(semantic_palette_at_the_core_boundary)`). ### What that bought, and what it cost @@ -2462,3 +2462,85 @@ the mode, the same shape `col_groups` uses for "no groups". `web/app.js` paints rows it does not author; `src/ui/help.rs` keeps its ratatui styles and lost ~310 lines of content that is now shared. Neither end restates what the keys are. + +## 10. `face.rs` — appearance named by role (ADR 004) + +ADR 004 (`context/adr/004-faces-and-themes.md`) is the decision; this section +is where its first two migration steps landed and what they settled that the +ADR left open (improvise-nn4s.1, improvise-nn4s.2). + +### What is here + +- `Face`: one variant per role. Where a domain enum already exists the face is + **parameterised by it** — `Face::Axis(Axis)`, `Face::WizardField(WizardFieldKind)`, + `Face::ModePanel(Panel)`, `Face::ModeTextEntryOf(TextEntry)`, + `Face::MinibufferOf(TextEntry)` — so adding an arm to that enum adds a face by + construction (clause 2). `Face::name` spells each one as a theme file keys it + (`axis.row`, `mode.text-entry.command`, `minibuffer.category-add`), and those + names are a compatibility surface once a theme can be downloaded (clause 9). +- `Face::parent`: the inheritance, **fixed in the vocabulary rather than stated + by the theme**. A theme that omits a face must still get the face's parent's + values ("a theme written today keeps working when a face is added tomorrow", + clause 9), and that is only true if the chain does not depend on what the + theme says. +- `Colour` (`Default | Named(AnsiSlot) | Indexed(u8) | Rgb`), `Weight`, `Slant`, + and `FaceSpec`, whose six attributes are all `Option`: `None` inherits, and + `Some(Colour::Default)` states "the surface" and stops inheritance. The two + are different answers and the type keeps them apart. +- `Theme { faces }` and `Theme::resolve(Face) -> ResolvedFace`, which walks the + chain once and returns every attribute concrete. **No front end walks the + chain** (clause 3). `resolve` takes a face and nothing else. +- `Theme::shipped()`: today's terminal rendering, palette-relative throughout + (clause 4). `the_shipped_theme_states_no_absolute_colour` holds that line. +- `Face::of_mode(&AppMode)` and `Face::of_minibuffer(&AppMode)`, both + exhaustive over `AppMode`, so a new mode fails the build here as it used to + in `draw.rs::mode_style` (improvise-zx8t). + +### The JSON shape + +Face names are the keys and a colour is spelled for a person: `"default"`, a +slot name (`"green"`, `"bright-black"`), a bare number for an index (`235`), +`"#rrggbb"` for `Rgb`. A misspelt face or attribute is **refused**, not +ignored (`deny_unknown_fields`, and `Face`'s `Deserialize` rejects an unknown +name): a file the user edits by hand should say where it is wrong rather than +silently draw the default. The faces map is a `BTreeMap` keyed on `Face`'s +derived order, so a re-download after an edit is a reviewable diff (clause 9). +The protocol crate derives and the front ends encode, so `serde_json` stays a +dev-dependency here. + +### Choices the ADR did not make, made here + +- **`minibuffer.*` inherits `mode.text-entry` directly, as the ADR says, and + the shipped theme pays for it.** The mode indicator is black text on an + accent, and the minibuffer is accent text on `Indexed(235)`, bold. So under + this chain every `minibuffer.` in the shipped theme states all three of + its attributes and inherits none of them from its parent. A first version gave + them a `minibuffer` base face instead. ChatGPT Codex caught it on PR #319: a + published parent is compatibility behaviour, so a theme written to the + accepted ADR that restyles `mode.text-entry` would have silently missed every + prompt. `a_theme_that_restyles_text_entry_reaches_every_minibuffer` pins the + accepted chain. Changing it is the user's call and belongs in the ADR, not in + this file. +- **`mode.tile-select` hangs directly off `mode`.** The ADR's decomposition + covers the thirteen DarkGray modes and the text-entry chain, and does not + place `TileSelect`. +- **The mode faces still state `Named(Black)` ink.** Clause 5 wants the twelve + `fg(Black).bg(accent)` sites to become `reverse` over the accent, but that is + not a no-op on a `TestBackend` buffer, and clause 8 names only one exception. + So it waits for improvise-nn4s.6 and a ruling. +- **No slot tables yet.** `Theme` has no `slots`/`dark`/`default_scheme`: they + are the browser's substitute for a terminal palette (clause 4), and the first + consumer is improvise-nn4s.5. Adding them then is a field with a serde + default, not a format break. +- **Only the faces some call site draws with exist.** The ADR's full vocabulary + (`emphasis`, `error`, `selection`, `grid.*`, `panel.*`) arrives with the + call sites that use it, because a face with no caller is a published name + with no evidence behind it. + +### The category-add exception + +`minibuffer.category-add` resolves to green in the shipped theme, where the old +literal was yellow: the user ruled the split an accident (ADR 004 Q1), and it +is clause 8's one declared exception. `AppMode::category_add` now carries +`MinibufColor::Green` too, so the browser, which still reads the wire colour +until improvise-nn4s.4, agrees with the TUI rather than drifting from it. diff --git a/context/code/tui.md b/context/code/tui.md index eeae4517..eff93fb6 100644 --- a/context/code/tui.md +++ b/context/code/tui.md @@ -104,46 +104,49 @@ appear. Three rules: - `SHIFT`/`CONTROL`/`ALT` survive; every other modifier bit (`SUPER`, `HYPER`, `META`) is dropped. -### Semantic-to-terminal colour, and the one that is wrong +### `face_style` — the one place a face becomes a terminal style -This file holds the mapping design-principles §5 names as the shape everything -else should imitate: **the decision lives in the shared layer as a semantic -enum, and only the last translation is local.** +ADR 004 replaced the four per-family maps this section used to describe +(`axis_fg`, `minibuf_fg`, `field_kind_fg`, `mode_style`) with one function: +`face_style(Face) -> Style` resolves the face against the shipped theme +(`Theme::resolve`, in `improvise-protocol`) and translates the result. That is +design-principles §5's shape with nothing left over: **the decision is shared, +and only the last translation is local** — `Colour` → `ratatui::Color` and the +six attributes → `Modifier`. The chain is never walked here (ADR 004 clause 3). +What the vocabulary is and why is `context/code/protocol.md` §10. -- `axis_fg(AxisColor) -> Color` — `Axis::color` is the single place each axis's - accent is decided; this is the single place axis colouring is *rendered* - (improvise-rml, gotcha 26). Before it, the tile bar and the category panel - each carried their own copy of the colour table with nothing keeping them in - agreement. -- `minibuf_fg(MinibufColor) -> Color` — the same shape for the minibuffer. -- `src/ui/import_wizard_ui.rs::field_kind_fg(WizardFieldKind) -> Color` — the - same shape again. `improvise-protocol` must not name a ratatui `Color`. +Three things to know. -`mode_style(&AppMode) -> Style` stays here for the same reason (`Style` and -`Color` are ratatui types and `improvise-command`, where `AppMode` lives, -carries no rendering-framework dependency), and since `improvise-zx8t` it is -**exhaustive**: the thirteen variants that take the default chrome are spelled -out rather than swept up by a `_ =>` wildcard, so adding an `AppMode` fails the -build here until someone chooses its colour. The arithmetic is the check a -prose count does not get: 13 + `Editing`/`RecordsEditing` + `CommandMode` + -`TileSelect` = 17, which is `AppMode`'s variant count. It read *fourteen* until -2026-09-22, and nothing could have caught that — a count written in prose is -checked by no one. +- **`Colour::Default` sets nothing.** It maps to "no `fg`/`bg` on the `Style`", + so the terminal's own colour shows through. That is also why a face with no + stated colour produces `Style::default()` and not a `Reset`. +- **The slot names follow ANSI, not ratatui.** ANSI slot 7 is `White`, which + ratatui calls `Gray`; slot 8 is `BrightBlack`, ratatui's `DarkGray`; slot 15 + is `BrightWhite`, ratatui's `White`. `terminal_colour` is the one place that + translation is written. +- **The shipped theme is resolved once**, into a `OnceLock`, because there is no + theme picker yet (ADR 004 "Deferred"). When one arrives the theme becomes the + TUI's own state rather than a static, and a theme is still never on `App`'s + `ViewState` (clause 7). -It is still not quite the same shape as `axis_fg` and its two siblings, and the -difference is deliberate rather than pending. Those three map a *semantic enum* -the shared layer owns; `mode_style` matches `AppMode` itself, because there is -no `ModeColor`. `improvise-zx8t` offered making one, and it was declined here -for two reasons: the colour is the only thing at stake, and the ticket says to -decide it together with `improvise-k099`, which asks the same question of -`hint_text`/`label` and is not in this unit. Exhaustiveness is what the -repo map's "the compiler names every one you must fill in" promise needed; a -semantic enum is a separate, larger answer and is still open. +**The mode indicator is still exhaustive over `AppMode`**, now in +`Face::of_mode` rather than here: adding a variant fails the build there +(improvise-zx8t's guard, moved with the table). The thirteen DarkGray variants +are no longer one arm: they are the five faces ADR 004 derives +(`mode.resting`, `mode.text-entry`, `mode.panel.*`, `mode.overlay`, +`mode.transient`), which all still inherit the same colours from `mode`. -**The guard is the compiler, not a test.** Nothing asserts the colours, and a -test enumerating the variants would not fail when a fifteenth is added — only -the missing match arm does. Verified by deleting the `AppMode::Help` arm: -`error[E0004]: non-exhaustive patterns: `&AppMode::Help` not covered`. +**No-op evidence.** `face_tests::the_shipped_theme_reproduces_every_migrated_literal` +holds the old literals as a table and asserts the shipped theme draws each one, +which is clause 8's property. The one deliberate difference, the category-add +prompt turning green, is asserted by +`the_category_add_prompt_draws_as_the_item_add_prompt_does` rather than being +left out of the table silently. + +**Still literal**: every other `Color::` in `src/` (the selection sites, the +panels' accents, the grid's headers, help, the welcome popup). They move as +their files are touched (improvise-nn4s.3, improvise-nn4s.6). Do not touch +`src/ui/grid.rs` for this while improvise-felo.6 is in flight. ### The minibuffer shares the bottom row with the status line diff --git a/context/design-principles.md b/context/design-principles.md index e9036e59..3e35f6a2 100644 --- a/context/design-principles.md +++ b/context/design-principles.md @@ -269,8 +269,8 @@ why `doctrine(mirror_the_cmd_line_for_line)` is a rule about *resolvers* — the narrow, unavoidable residue — and never a licence to restate a table. **What legitimately stays per-framework.** Painting, event wiring, and the -mapping from a semantic value to a framework type: `AxisColor → ratatui::Color` -in `draw.rs::axis_fg`, `MinibufColor → minibuf_fg`. That mapping is the shape +mapping from a semantic value to a framework type: a `Face`, resolved by the +shared `Theme`, becomes a `ratatui::Style` in `draw.rs::face_style` (ADR 004). That mapping is the shape to imitate — the *decision* is in the shared layer as a semantic enum, and only the last translation is local. Anything larger than a lookup table is a candidate for the store. diff --git a/context/test-evidence/core.md b/context/test-evidence/core.md index d7d99df7..862cabd0 100644 --- a/context/test-evidence/core.md +++ b/context/test-evidence/core.md @@ -710,11 +710,6 @@ Applying `next` four times is the identity from every starting point and no navigable axis is a fixed point, so no variant is stranded or skipped. Together with the entry above this pins the rotation without enumerating it twice. -### every_axis_gets_its_own_color - -Distinct axes must be distinguishable at a glance; `color()` is the only reason -`AxisColor` exists, so a collision defeats it. - ### a_routing_formula_over_a_group_sums_the_members_answers improvise-4zl7, and the hand-computed half of its evidence. Every number in diff --git a/context/test-evidence/protocol.md b/context/test-evidence/protocol.md index b5583639..a04e0deb 100644 --- a/context/test-evidence/protocol.md +++ b/context/test-evidence/protocol.md @@ -1260,6 +1260,46 @@ the Enter commit. `Enter`), run across the whole workspace: fails **only** this test, with `["Later"]` against `["Other"]`. +### resolution_takes_each_attribute_from_the_nearest_face_that_states_it + +ADR 004 clause 3: inheritance resolves once, in shared code. The fixture states +a different attribute at each of three levels — `default` (foreground White, +italic), `mode` (foreground Black, background Blue), and +`mode.text-entry.save` (background `Colour::Default`, underline) — so the one +resolved face has to take its foreground from the middle, its slant from the +root and its background from itself. `Some(Colour::Default)` is the case that +matters: it is a stated value and must not fall through to `mode`'s Blue, which +the sibling `mode.text-entry.export` (stating nothing) does inherit. + +**Injected fault**: fold the chain so the parent wins. This test fails, and so +does the TUI's `the_shipped_theme_reproduces_every_migrated_literal`, because +`mode.text-entry.edit` then draws its parent's DarkGray. + +### every_axis_gets_its_own_accent_in_the_shipped_theme + +Moved from `improvise-core`'s `every_axis_gets_its_own_color` when `AxisColor` +left core (ADR 004 clause 6). Distinct axes must be distinguishable at a +glance, and the axis faces exist for exactly that, so two axes resolving alike +defeats them. It now asks the shipped theme rather than an enum's arms, which +is the stronger question: two distinct `AxisColor` arms could always have been +mapped to one terminal colour. + +### a_theme_that_restyles_text_entry_reaches_every_minibuffer + +ADR 004 says `minibuffer.*` inherits `mode.text-entry` directly. PR #319's first +head gave the minibuffer faces a `minibuffer` base face instead, and ChatGPT +Codex flagged it (P2): face parents are compatibility behaviour once a theme +can be downloaded, so a theme that restyles `mode.text-entry` must reach every +prompt. The test overrides only `mode.text-entry` with italic, an attribute no +minibuffer face states in the shipped theme, and asserts every +`minibuffer.` resolves italic. A test that sets a colour instead would +be vacuous, because each minibuffer face states its own colours. + +**Failed first**, against the `minibuffer` base parent: `Edit` resolved to +`Normal`. It passes under the accepted chain. The no-op oracle +`the_shipped_theme_reproduces_every_migrated_literal` passed both before and +after the change, which shows the change moved no pixel. + ### a_quarters_end_leaves_its_year_open_on_the_same_level improvise-felo.6: runs inside one category nest by `(level, depth)`. `Direct` diff --git a/context/test-evidence/tui.md b/context/test-evidence/tui.md index 2e80651c..ffa6d66e 100644 --- a/context/test-evidence/tui.md +++ b/context/test-evidence/tui.md @@ -1430,6 +1430,39 @@ with a three-line item and a second item after it. It asserts the first item is drawn whole and the header reads `1–1 of 2`. The rival **the header claims every item shown** fails it. +### the_category_add_prompt_draws_as_the_item_add_prompt_does + +ADR 004 clause 8's one declared exception, decided by the user (Q1 on PR #122: +the green/yellow split between item-add and category-add is an accident). It +draws both prompts through the real `draw_bottom_bar` into a `TestBackend` and +compares the style of the prompt's first cell, after checking that cell really +holds the prompt's `n`. + +**Failed first**, at `gh/main` `b639266b` before any face existed: `left: +...yellow().bg(Indexed(235)).bold()`, `right: ...green()...` — the category-add +prompt was yellow. **Injected fault** after the migration: set the shipped +theme's `minibuffer.category-add` back to `Named(Yellow)`; this test fails and +nothing else in the TUI does, which is the point of naming the exception in a +test of its own rather than leaving it out of the no-op table. + +### the_shipped_theme_reproduces_every_migrated_literal + +ADR 004 clause 8's property: migrating a call site onto a face is a no-op. The +oracle is the literal tables as they stood before the migration — axis +accents, wizard field kinds, `mode_style`'s exhaustive match (kept verbatim as +`mode_bg_before_faces`) and the minibuffer's accent-on-`Indexed(235)`-bold — +and every one is compared with `face_style` of its face. Category-add is left +out on purpose (the entry above). It was first written against the old +functions themselves and passed, then the functions were deleted and their +tables moved into the test. + +**Injected faults**, each failing here by the case name in the message: +`axis.column` changed from Blue to Cyan (fails at `Column`); `face_style` made +to add `BOLD` for `Weight::Normal` instead of `Bold`; `Theme::resolve` folded so +that the parent wins over the child (fails at `Editing`, which draws DarkGray +instead of its own Green); and `Face::of_mode(TileSelect)` returning +`mode.resting` (also caught by `the_status_line_is_drawn_in_the_mode_face`). + ### the_two_front_ends_draw_the_same_nested_hierarchy_on_rows improvise-felo.6: Year > Quarter > Month in one category, under a grouped outer diff --git a/crates/improvise-core/src/view/axis.rs b/crates/improvise-core/src/view/axis.rs index ae55a145..77860d28 100644 --- a/crates/improvise-core/src/view/axis.rs +++ b/crates/improvise-core/src/view/axis.rs @@ -1,6 +1,6 @@ use serde::{Deserialize, Serialize}; -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum Axis { Row, @@ -12,17 +12,6 @@ pub enum Axis { None, } -/// Accent color for an axis. Semantic palette only; `draw.rs::axis_fg` maps -/// each variant to a terminal color, so `Axis` names no ratatui type. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum AxisColor { - Row, - Column, - Page, - Filter, - Inactive, -} - impl Axis { /// The next axis in the Row → Column → Page → None → Row rotation. /// `Filter` is outside it and cycling one is a no-op (improvise-dz8). @@ -48,18 +37,6 @@ impl Axis { !matches!(self, Axis::Filter) } - /// Accent color. The single source for axis coloring: the tile bar and the - /// category panel each carried a copy before (improvise-rml). - pub fn color(self) -> AxisColor { - match self { - Axis::Row => AxisColor::Row, - Axis::Column => AxisColor::Column, - Axis::Page => AxisColor::Page, - Axis::Filter => AxisColor::Filter, - Axis::None => AxisColor::Inactive, - } - } - /// Name with no decoration: "Row", "Col", "Page", "None". pub fn plain_label(self) -> &'static str { match self { @@ -176,15 +153,4 @@ mod tests { assert!(!Axis::Column.filters_data()); assert!(!Axis::None.filters_data()); } - - /// A collision defeats the only reason `AxisColor` exists. - #[test] - fn every_axis_gets_its_own_color() { - let colors = ALL.map(Axis::color); - for (i, a) in colors.iter().enumerate() { - for b in &colors[i + 1..] { - assert_ne!(a, b, "two axes share an accent color"); - } - } - } } diff --git a/crates/improvise-core/src/view/mod.rs b/crates/improvise-core/src/view/mod.rs index c0d8892b..e7466938 100644 --- a/crates/improvise-core/src/view/mod.rs +++ b/crates/improvise-core/src/view/mod.rs @@ -2,7 +2,7 @@ pub mod axis; pub mod layout; pub mod types; -pub use axis::{Axis, AxisColor}; +pub use axis::Axis; pub use layout::{ AxisEntry, GridEval, GridLayout, NEW_CATEGORY_COL, NEW_MEASURE_COL, RecordCol, RecordRow, ViewStep, drill_constraint, drill_snapshot, drill_steps, record_row, records_spine, diff --git a/crates/improvise-protocol/src/face.rs b/crates/improvise-protocol/src/face.rs new file mode 100644 index 00000000..5173b575 --- /dev/null +++ b/crates/improvise-protocol/src/face.rs @@ -0,0 +1,682 @@ +//! Faces: appearance named by role, and a theme that binds roles to values +//! (ADR 004). The reasoning is `context/code/protocol.md` §10. + +use std::collections::BTreeMap; +use std::fmt; + +use serde::de::{self, Deserializer, Visitor}; +use serde::{Deserialize, Serialize, Serializer}; + +use crate::render::WizardFieldKind; +use crate::{AppMode, Panel}; +use improvise_core::view::Axis; + +/// One of the sixteen palette slots: "whatever THIS display calls that slot". +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum AnsiSlot { + Black, + Red, + Green, + Yellow, + Blue, + Magenta, + Cyan, + White, + BrightBlack, + BrightRed, + BrightGreen, + BrightYellow, + BrightBlue, + BrightMagenta, + BrightCyan, + BrightWhite, +} + +impl AnsiSlot { + pub const ALL: [AnsiSlot; 16] = [ + AnsiSlot::Black, + AnsiSlot::Red, + AnsiSlot::Green, + AnsiSlot::Yellow, + AnsiSlot::Blue, + AnsiSlot::Magenta, + AnsiSlot::Cyan, + AnsiSlot::White, + AnsiSlot::BrightBlack, + AnsiSlot::BrightRed, + AnsiSlot::BrightGreen, + AnsiSlot::BrightYellow, + AnsiSlot::BrightBlue, + AnsiSlot::BrightMagenta, + AnsiSlot::BrightCyan, + AnsiSlot::BrightWhite, + ]; + + /// The slot's number in the 256-colour palette, 0–15. + pub fn index(self) -> u8 { + self as u8 + } + + /// The spelling a theme file uses. + pub fn name(self) -> &'static str { + match self { + AnsiSlot::Black => "black", + AnsiSlot::Red => "red", + AnsiSlot::Green => "green", + AnsiSlot::Yellow => "yellow", + AnsiSlot::Blue => "blue", + AnsiSlot::Magenta => "magenta", + AnsiSlot::Cyan => "cyan", + AnsiSlot::White => "white", + AnsiSlot::BrightBlack => "bright-black", + AnsiSlot::BrightRed => "bright-red", + AnsiSlot::BrightGreen => "bright-green", + AnsiSlot::BrightYellow => "bright-yellow", + AnsiSlot::BrightBlue => "bright-blue", + AnsiSlot::BrightMagenta => "bright-magenta", + AnsiSlot::BrightCyan => "bright-cyan", + AnsiSlot::BrightWhite => "bright-white", + } + } + + pub fn from_name(name: &str) -> Option { + Self::ALL.into_iter().find(|slot| slot.name() == name) + } +} + +/// A colour attribute. `Default`, `Named` and `Indexed` are palette-relative; +/// `Rgb` is absolute, and the shipped theme never uses it (ADR 004 clause 4). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Colour { + Default, + Named(AnsiSlot), + Indexed(u8), + Rgb(u8, u8, u8), +} + +impl Colour { + /// True for the one arm that opts a viewer out of their own palette. + pub fn is_absolute(self) -> bool { + match self { + Colour::Rgb(..) => true, + Colour::Default | Colour::Named(_) | Colour::Indexed(_) => false, + } + } +} + +impl Serialize for Colour { + fn serialize(&self, s: S) -> Result { + match *self { + Colour::Default => s.serialize_str("default"), + Colour::Named(slot) => s.serialize_str(slot.name()), + Colour::Indexed(n) => s.serialize_u8(n), + Colour::Rgb(r, g, b) => s.serialize_str(&format!("#{r:02x}{g:02x}{b:02x}")), + } + } +} + +struct ColourVisitor; + +impl Visitor<'_> for ColourVisitor { + type Value = Colour; + + fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str("\"default\", a slot name such as \"green\", 0-255, or \"#rrggbb\"") + } + + fn visit_u64(self, n: u64) -> Result { + u8::try_from(n) + .map(Colour::Indexed) + .map_err(|_| E::custom(format!("colour index {n} is outside 0-255"))) + } + + fn visit_str(self, s: &str) -> Result { + parse_colour(s).ok_or_else(|| E::invalid_value(de::Unexpected::Str(s), &self)) + } +} + +fn parse_colour(s: &str) -> Option { + match s.strip_prefix('#') { + Some(hex) if hex.len() == 6 && hex.bytes().all(|b| b.is_ascii_hexdigit()) => { + let byte = |i: usize| u8::from_str_radix(&hex[i..i + 2], 16).ok(); + Some(Colour::Rgb(byte(0)?, byte(2)?, byte(4)?)) + } + Some(_) => None, + None if s == "default" => Some(Colour::Default), + None => AnsiSlot::from_name(s).map(Colour::Named), + } +} + +impl<'de> Deserialize<'de> for Colour { + fn deserialize>(d: D) -> Result { + d.deserialize_any(ColourVisitor) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum Weight { + Normal, + Bold, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum Slant { + Normal, + Italic, +} + +/// The text-entry modes, one per `MinibufferConfig`-carrying `AppMode` arm +/// except that cell editing is one role over two layouts. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum TextEntry { + Edit, + Formula, + Command, + CategoryAdd, + ItemAdd, + Export, + Save, +} + +impl TextEntry { + pub const ALL: [TextEntry; 7] = [ + TextEntry::Edit, + TextEntry::Formula, + TextEntry::Command, + TextEntry::CategoryAdd, + TextEntry::ItemAdd, + TextEntry::Export, + TextEntry::Save, + ]; + + /// The text-entry role of `mode`, or `None` for a mode that takes no text. + pub fn of(mode: &AppMode) -> Option { + match mode { + AppMode::Editing { .. } | AppMode::RecordsEditing { .. } => Some(TextEntry::Edit), + AppMode::FormulaEdit { .. } => Some(TextEntry::Formula), + AppMode::CommandMode { .. } => Some(TextEntry::Command), + AppMode::CategoryAdd { .. } => Some(TextEntry::CategoryAdd), + AppMode::ItemAdd { .. } => Some(TextEntry::ItemAdd), + AppMode::ExportPrompt { .. } => Some(TextEntry::Export), + AppMode::SavePrompt { .. } => Some(TextEntry::Save), + AppMode::Normal + | AppMode::RecordsNormal + | AppMode::FormulaPanel + | AppMode::CategoryPanel + | AppMode::ViewPanel + | AppMode::TileSelect + | AppMode::ImportWizard + | AppMode::Help + | AppMode::Quit => None, + } + } + + fn name(self) -> &'static str { + match self { + TextEntry::Edit => "edit", + TextEntry::Formula => "formula", + TextEntry::Command => "command", + TextEntry::CategoryAdd => "category-add", + TextEntry::ItemAdd => "item-add", + TextEntry::Export => "export", + TextEntry::Save => "save", + } + } +} + +const AXES: [Axis; 5] = [ + Axis::Row, + Axis::Column, + Axis::Page, + Axis::Filter, + Axis::None, +]; + +const FIELD_KINDS: [WizardFieldKind; 4] = [ + WizardFieldKind::Category, + WizardFieldKind::Measure, + WizardFieldKind::TimeCategory, + WizardFieldKind::Label, +]; + +fn axis_name(axis: Axis) -> &'static str { + match axis { + Axis::Row => "row", + Axis::Column => "column", + Axis::Page => "page", + Axis::Filter => "filter", + Axis::None => "none", + } +} + +fn field_kind_name(kind: WizardFieldKind) -> &'static str { + match kind { + WizardFieldKind::Category => "category", + WizardFieldKind::Measure => "measure", + WizardFieldKind::TimeCategory => "time", + WizardFieldKind::Label => "label", + } +} + +/// A role something is drawn in. Its `name` is a compatibility surface once a +/// theme file carries it (ADR 004 clause 9), so never rename one. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum Face { + Default, + Dim, + Axis(Axis), + WizardField(WizardFieldKind), + Mode, + ModeResting, + ModeTextEntry, + ModeTextEntryOf(TextEntry), + ModePanel(Panel), + ModeOverlay, + ModeTransient, + ModeTileSelect, + MinibufferOf(TextEntry), +} + +impl Face { + /// Every face, parents before children. + pub fn all() -> Vec { + let mut out = vec![Face::Default, Face::Dim]; + out.extend(AXES.map(Face::Axis)); + out.extend(FIELD_KINDS.map(Face::WizardField)); + out.extend([Face::Mode, Face::ModeResting, Face::ModeTextEntry]); + out.extend(TextEntry::ALL.map(Face::ModeTextEntryOf)); + out.extend(Panel::ALL.map(Face::ModePanel)); + out.extend([Face::ModeOverlay, Face::ModeTransient, Face::ModeTileSelect]); + out.extend(TextEntry::ALL.map(Face::MinibufferOf)); + out + } + + /// The face this one inherits every attribute it does not state from. + pub fn parent(self) -> Option { + match self { + Face::Default => None, + Face::Dim | Face::Mode => Some(Face::Default), + Face::Axis(Axis::None) | Face::WizardField(WizardFieldKind::Label) => Some(Face::Dim), + Face::Axis(_) | Face::WizardField(_) => Some(Face::Default), + Face::ModeResting + | Face::ModeTextEntry + | Face::ModePanel(_) + | Face::ModeOverlay + | Face::ModeTransient + | Face::ModeTileSelect => Some(Face::Mode), + Face::ModeTextEntryOf(_) | Face::MinibufferOf(_) => Some(Face::ModeTextEntry), + } + } + + /// The spelling a theme file keys this face by. + pub fn name(self) -> String { + match self { + Face::Default => "default".into(), + Face::Dim => "dim".into(), + Face::Axis(axis) => format!("axis.{}", axis_name(axis)), + Face::WizardField(kind) => format!("wizard.field.{}", field_kind_name(kind)), + Face::Mode => "mode".into(), + Face::ModeResting => "mode.resting".into(), + Face::ModeTextEntry => "mode.text-entry".into(), + Face::ModeTextEntryOf(entry) => format!("mode.text-entry.{}", entry.name()), + Face::ModePanel(panel) => format!("mode.panel.{}", panel.name()), + Face::ModeOverlay => "mode.overlay".into(), + Face::ModeTransient => "mode.transient".into(), + Face::ModeTileSelect => "mode.tile-select".into(), + Face::MinibufferOf(entry) => format!("minibuffer.{}", entry.name()), + } + } + + pub fn from_name(name: &str) -> Option { + Face::all().into_iter().find(|face| face.name() == name) + } + + /// The face the mode indicator is drawn in. + pub fn of_mode(mode: &AppMode) -> Face { + match mode { + AppMode::Normal | AppMode::RecordsNormal => Face::ModeResting, + AppMode::FormulaPanel => Face::ModePanel(Panel::Formula), + AppMode::CategoryPanel => Face::ModePanel(Panel::Category), + AppMode::ViewPanel => Face::ModePanel(Panel::View), + AppMode::ImportWizard | AppMode::Help => Face::ModeOverlay, + AppMode::Quit => Face::ModeTransient, + AppMode::TileSelect => Face::ModeTileSelect, + AppMode::Editing { .. } + | AppMode::RecordsEditing { .. } + | AppMode::FormulaEdit { .. } + | AppMode::CommandMode { .. } + | AppMode::CategoryAdd { .. } + | AppMode::ItemAdd { .. } + | AppMode::ExportPrompt { .. } + | AppMode::SavePrompt { .. } => TextEntry::of(mode) + .map(Face::ModeTextEntryOf) + .unwrap_or(Face::ModeTextEntry), + } + } + + /// The face a text-entry mode's minibuffer is drawn in. + pub fn of_minibuffer(mode: &AppMode) -> Option { + TextEntry::of(mode).map(Face::MinibufferOf) + } +} + +impl Serialize for Face { + fn serialize(&self, s: S) -> Result { + s.serialize_str(&self.name()) + } +} + +impl<'de> Deserialize<'de> for Face { + fn deserialize>(d: D) -> Result { + let name = String::deserialize(d)?; + Face::from_name(&name).ok_or_else(|| de::Error::custom(format!("unknown face {name:?}"))) + } +} + +/// What a theme states about one face. `None` inherits from the parent; +/// `Some(Colour::Default)` states "the surface" and does not inherit. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct FaceSpec { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub foreground: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub background: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub weight: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub slant: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub underline: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reverse: Option, +} + +impl FaceSpec { + fn or(self, parent: FaceSpec) -> FaceSpec { + FaceSpec { + foreground: self.foreground.or(parent.foreground), + background: self.background.or(parent.background), + weight: self.weight.or(parent.weight), + slant: self.slant.or(parent.slant), + underline: self.underline.or(parent.underline), + reverse: self.reverse.or(parent.reverse), + } + } + + fn fg(colour: Colour) -> FaceSpec { + FaceSpec { + foreground: Some(colour), + ..FaceSpec::default() + } + } + + fn bg(colour: Colour) -> FaceSpec { + FaceSpec { + background: Some(colour), + ..FaceSpec::default() + } + } +} + +/// A face with every attribute concrete: what a front end translates. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ResolvedFace { + pub foreground: Colour, + pub background: Colour, + pub weight: Weight, + pub slant: Slant, + pub underline: bool, + pub reverse: bool, +} + +/// Faces bound to values. Client-local: never a `Command`, never in +/// `ViewState`, never projected, never in `.improv` (ADR 004 clause 7). +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Theme { + pub faces: BTreeMap, +} + +impl Theme { + /// The theme improvise ships: today's terminal rendering, palette-relative + /// throughout, so the user's terminal theme still applies. + pub fn shipped() -> Theme { + use AnsiSlot::*; + let named = Colour::Named; + let minibuffer = |accent| FaceSpec { + foreground: Some(named(accent)), + background: Some(Colour::Indexed(235)), + weight: Some(Weight::Bold), + ..FaceSpec::default() + }; + let faces = [ + (Face::Dim, FaceSpec::fg(named(BrightBlack))), + (Face::Axis(Axis::Row), FaceSpec::fg(named(Green))), + (Face::Axis(Axis::Column), FaceSpec::fg(named(Blue))), + (Face::Axis(Axis::Page), FaceSpec::fg(named(Magenta))), + (Face::Axis(Axis::Filter), FaceSpec::fg(named(Yellow))), + ( + Face::WizardField(WizardFieldKind::Category), + FaceSpec::fg(named(Green)), + ), + ( + Face::WizardField(WizardFieldKind::Measure), + FaceSpec::fg(named(Cyan)), + ), + ( + Face::WizardField(WizardFieldKind::TimeCategory), + FaceSpec::fg(named(Magenta)), + ), + ( + Face::Mode, + FaceSpec { + foreground: Some(named(Black)), + background: Some(named(BrightBlack)), + ..FaceSpec::default() + }, + ), + ( + Face::ModeTextEntryOf(TextEntry::Edit), + FaceSpec::bg(named(Green)), + ), + ( + Face::ModeTextEntryOf(TextEntry::Command), + FaceSpec::bg(named(Yellow)), + ), + (Face::ModeTileSelect, FaceSpec::bg(named(Magenta))), + (Face::MinibufferOf(TextEntry::Edit), minibuffer(Green)), + (Face::MinibufferOf(TextEntry::ItemAdd), minibuffer(Green)), + ( + Face::MinibufferOf(TextEntry::CategoryAdd), + minibuffer(Green), + ), + (Face::MinibufferOf(TextEntry::Formula), minibuffer(Cyan)), + (Face::MinibufferOf(TextEntry::Command), minibuffer(Yellow)), + (Face::MinibufferOf(TextEntry::Export), minibuffer(Yellow)), + (Face::MinibufferOf(TextEntry::Save), minibuffer(Yellow)), + ]; + Theme { + faces: faces.into_iter().collect(), + } + } + + /// Walk `face`'s inheritance chain once, so no front end ever does. + pub fn resolve(&self, face: Face) -> ResolvedFace { + let stated = std::iter::successors(Some(face), |f| f.parent()) + .filter_map(|f| self.faces.get(&f).copied()) + .fold(FaceSpec::default(), FaceSpec::or); + ResolvedFace { + foreground: stated.foreground.unwrap_or(Colour::Default), + background: stated.background.unwrap_or(Colour::Default), + weight: stated.weight.unwrap_or(Weight::Normal), + slant: stated.slant.unwrap_or(Slant::Normal), + underline: stated.underline.unwrap_or(false), + reverse: stated.reverse.unwrap_or(false), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn every_face_name_is_distinct_and_reads_back_as_that_face() { + let all = Face::all(); + for face in &all { + assert_eq!(Face::from_name(&face.name()), Some(*face), "{face:?}"); + } + let mut names: Vec = all.iter().map(|f| f.name()).collect(); + names.sort(); + names.dedup(); + assert_eq!(names.len(), all.len(), "two faces share a name"); + } + + #[test] + fn every_parent_is_a_face_listed_before_its_child() { + let all = Face::all(); + for (i, face) in all.iter().enumerate() { + if let Some(parent) = face.parent() { + let at = all.iter().position(|f| *f == parent); + assert!(at.is_some_and(|p| p < i), "{face:?}'s parent {parent:?}"); + } + } + } + + /// ADR 004 clause 4: the shipped theme keeps the user's terminal palette. + #[test] + fn the_shipped_theme_states_no_absolute_colour() { + let theme = Theme::shipped(); + for face in Face::all() { + let resolved = theme.resolve(face); + assert!(!resolved.foreground.is_absolute(), "{}", face.name()); + assert!(!resolved.background.is_absolute(), "{}", face.name()); + } + } + + /// ADR 004: `minibuffer.*` inherits `mode.text-entry` directly, so a theme + /// that restyles text entry reaches every prompt. `context/test-evidence/protocol.md`. + #[test] + fn a_theme_that_restyles_text_entry_reaches_every_minibuffer() { + let mut theme = Theme::shipped(); + let italic = FaceSpec { + slant: Some(Slant::Italic), + ..FaceSpec::default() + }; + theme.faces.insert(Face::ModeTextEntry, italic); + for entry in TextEntry::ALL { + let got = theme.resolve(Face::MinibufferOf(entry)); + assert_eq!(got.slant, Slant::Italic, "{entry:?}"); + } + } + + /// A collision defeats the reason the axis faces exist (improvise-rml). + #[test] + fn every_axis_gets_its_own_accent_in_the_shipped_theme() { + let theme = Theme::shipped(); + let accents = AXES.map(|axis| theme.resolve(Face::Axis(axis))); + for (i, a) in accents.iter().enumerate() { + for b in &accents[i + 1..] { + assert_ne!(a, b, "two axes share an accent"); + } + } + } + + /// A child overrides what it states and inherits what it does not, from + /// the nearest ancestor that states it — `context/test-evidence/protocol.md`. + #[test] + fn resolution_takes_each_attribute_from_the_nearest_face_that_states_it() { + let named = Colour::Named; + let theme = Theme { + faces: [ + ( + Face::Default, + FaceSpec { + foreground: Some(named(AnsiSlot::White)), + slant: Some(Slant::Italic), + ..FaceSpec::default() + }, + ), + ( + Face::Mode, + FaceSpec { + foreground: Some(named(AnsiSlot::Black)), + background: Some(named(AnsiSlot::Blue)), + ..FaceSpec::default() + }, + ), + ( + Face::ModeTextEntryOf(TextEntry::Save), + FaceSpec { + background: Some(Colour::Default), + underline: Some(true), + ..FaceSpec::default() + }, + ), + ] + .into_iter() + .collect(), + }; + let got = theme.resolve(Face::ModeTextEntryOf(TextEntry::Save)); + assert_eq!( + got, + ResolvedFace { + foreground: named(AnsiSlot::Black), + background: Colour::Default, + weight: Weight::Normal, + slant: Slant::Italic, + underline: true, + reverse: false, + } + ); + let sibling = theme.resolve(Face::ModeTextEntryOf(TextEntry::Export)); + assert_eq!(sibling.background, named(AnsiSlot::Blue)); + } + + #[test] + fn a_theme_survives_its_own_json() { + let theme = Theme::shipped(); + let text = serde_json::to_string_pretty(&theme).unwrap(); + assert!(text.contains("\"minibuffer.category-add\""), "{text}"); + assert_eq!(serde_json::from_str::(&text).unwrap(), theme); + } + + #[test] + fn a_colour_is_spelled_for_a_person_editing_the_file() { + let cases = [ + (Colour::Default, "\"default\""), + (Colour::Named(AnsiSlot::BrightBlack), "\"bright-black\""), + (Colour::Indexed(235), "235"), + (Colour::Rgb(0x3f, 0x7a, 0x4d), "\"#3f7a4d\""), + ]; + for (colour, json) in cases { + assert_eq!(serde_json::to_string(&colour).unwrap(), json); + assert_eq!(serde_json::from_str::(json).unwrap(), colour); + } + for bad in ["\"grene\"", "\"#3f7a4\"", "256", "\"#gggggg\""] { + assert!(serde_json::from_str::(bad).is_err(), "{bad}"); + } + } + + #[test] + fn a_misspelt_face_or_attribute_is_refused_rather_than_ignored() { + let face = r#"{"faces":{"axis.rwo":{"foreground":"green"}}}"#; + assert!(serde_json::from_str::(face).is_err()); + let attr = r#"{"faces":{"axis.row":{"forground":"green"}}}"#; + assert!(serde_json::from_str::(attr).is_err()); + } + + #[test] + fn every_text_entry_mode_has_a_text_entry_role_and_no_other_mode_does() { + for mode in AppMode::all() { + assert_eq!( + TextEntry::of(&mode).is_some(), + mode.minibuffer().is_some(), + "{mode:?}" + ); + } + } +} diff --git a/crates/improvise-protocol/src/lib.rs b/crates/improvise-protocol/src/lib.rs index 6e535f44..71dabf12 100644 --- a/crates/improvise-protocol/src/lib.rs +++ b/crates/improvise-protocol/src/lib.rs @@ -6,6 +6,7 @@ use std::collections::HashMap; pub mod basis; pub mod client; +pub mod face; pub mod frame; pub mod help; pub mod key; @@ -23,6 +24,7 @@ mod vocabulary_is_composed; pub use basis::{Basis, BasisCache, Rejection}; pub use client::{ClientState, Sent}; +pub use face::{Face, Theme}; pub use frame::{ClientFrame, ServerFrame}; pub use help::{HelpLine, Hint, hint_line, hints, mode_label}; pub use key::{KeyCode, KeyEvent, KeyModifiers}; @@ -103,7 +105,7 @@ pub struct MinibufferConfig { /// One of the three side panels. Paired with [`AppMode::panel`], which names /// the panel a mode is focused in. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] pub enum Panel { Formula, Category, @@ -394,7 +396,7 @@ impl AppMode { minibuf: MinibufferConfig { buffer_key: "category".into(), prompt: "new category: ".into(), - color: MinibufColor::Yellow, + color: MinibufColor::Green, }, } } diff --git a/crates/improvise-protocol/src/render.rs b/crates/improvise-protocol/src/render.rs index c8621397..265d4b96 100644 --- a/crates/improvise-protocol/src/render.rs +++ b/crates/improvise-protocol/src/render.rs @@ -933,7 +933,7 @@ pub enum WizardStepKind { /// What kind of thing a proposed field is. The semantic half of a colour /// decision; the terminal mapping lives in `draw.rs` (gotcha 26). -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize)] pub enum WizardFieldKind { Category, Measure, diff --git a/crates/improvise-wasm-client/src/render.rs b/crates/improvise-wasm-client/src/render.rs index 1b369862..48d89256 100644 --- a/crates/improvise-wasm-client/src/render.rs +++ b/crates/improvise-wasm-client/src/render.rs @@ -75,8 +75,8 @@ pub struct RenderModel { pub struct RenderMinibuffer { pub prompt: String, pub value: String, - /// Semantic, as `MinibufColor` is: the last translation to a CSS class is - /// the page's, exactly as `draw.rs::minibuf_fg` is the TUI's (gotcha 26). + /// A colour, not a role: the TUI now draws a face per text-entry mode, and + /// improvise-nn4s.4 moves the page onto the same faces (ADR 004). pub color: improvise_protocol::MinibufColor, } diff --git a/src/draw.rs b/src/draw.rs index 158e674f..af5a86e7 100644 --- a/src/draw.rs +++ b/src/draw.rs @@ -1,5 +1,6 @@ use std::io::{self, Stdout}; use std::path::PathBuf; +use std::sync::OnceLock; use std::time::Duration; use anyhow::Result; @@ -18,7 +19,8 @@ use ratatui::{ use crate::command::key; use crate::protocol::EditorSession; -use crate::ui::app::{App, AppMode, MinibufColor}; +use crate::protocol::face::{AnsiSlot, Colour, Face, Slant, Theme, Weight}; +use crate::ui::app::{App, AppMode}; use crate::ui::category_panel::CategoryContent; use crate::ui::formula_panel::FormulaContent; use crate::ui::grid::GridWidget; @@ -29,7 +31,6 @@ use crate::ui::text::truncate; use crate::ui::tile_bar::TileBar; use crate::ui::view_panel::ViewContent; use crate::ui::which_key::WhichKeyWidget; -use crate::view::AxisColor; use improvise_command::ui::project; use unicode_width::UnicodeWidthStr; @@ -165,24 +166,54 @@ fn convert_modifiers(mods: event::KeyModifiers) -> key::KeyModifiers { // ── Drawing ────────────────────────────────────────────────────────────────── -/// The one place `AxisColor` becomes a terminal color (improvise-rml); the -/// accent itself is decided by `Axis::color`. See `context/code/tui.md`. -pub(crate) fn axis_fg(color: AxisColor) -> Color { - match color { - AxisColor::Row => Color::Green, - AxisColor::Column => Color::Blue, - AxisColor::Page => Color::Magenta, - AxisColor::Filter => Color::Yellow, - AxisColor::Inactive => Color::DarkGray, +/// The one place a face becomes a terminal style (ADR 004): the theme resolves +/// it, and only this last translation is the TUI's. `context/code/tui.md`. +pub(crate) fn face_style(face: Face) -> Style { + static SHIPPED: OnceLock = OnceLock::new(); + let face = SHIPPED.get_or_init(Theme::shipped).resolve(face); + let mut style = Style::default(); + if let Some(fg) = terminal_colour(face.foreground) { + style = style.fg(fg); } + if let Some(bg) = terminal_colour(face.background) { + style = style.bg(bg); + } + let modifiers = [ + (face.weight == Weight::Bold, Modifier::BOLD), + (face.slant == Slant::Italic, Modifier::ITALIC), + (face.underline, Modifier::UNDERLINED), + (face.reverse, Modifier::REVERSED), + ]; + modifiers + .into_iter() + .filter(|(on, _)| *on) + .fold(style, |style, (_, m)| style.add_modifier(m)) } -/// Map the semantic minibuffer accent to a concrete terminal color. -fn minibuf_fg(color: MinibufColor) -> Color { - match color { - MinibufColor::Green => Color::Green, - MinibufColor::Cyan => Color::Cyan, - MinibufColor::Yellow => Color::Yellow, +/// `None` is "state nothing", so the terminal's own colour shows through. +fn terminal_colour(colour: Colour) -> Option { + match colour { + Colour::Default => None, + Colour::Named(slot) => Some(match slot { + AnsiSlot::Black => Color::Black, + AnsiSlot::Red => Color::Red, + AnsiSlot::Green => Color::Green, + AnsiSlot::Yellow => Color::Yellow, + AnsiSlot::Blue => Color::Blue, + AnsiSlot::Magenta => Color::Magenta, + AnsiSlot::Cyan => Color::Cyan, + AnsiSlot::White => Color::Gray, + AnsiSlot::BrightBlack => Color::DarkGray, + AnsiSlot::BrightRed => Color::LightRed, + AnsiSlot::BrightGreen => Color::LightGreen, + AnsiSlot::BrightYellow => Color::LightYellow, + AnsiSlot::BrightBlue => Color::LightBlue, + AnsiSlot::BrightMagenta => Color::LightMagenta, + AnsiSlot::BrightCyan => Color::LightCyan, + AnsiSlot::BrightWhite => Color::White, + }), + Colour::Indexed(n) => Some(Color::Indexed(n)), + Colour::Rgb(r, g, b) => Some(Color::Rgb(r, g, b)), } } @@ -231,30 +262,6 @@ fn draw_popup_frame(f: &mut Frame, popup: Rect, title: &str, border_color: Color inner } -/// Terminal styling for the mode indicator. Exhaustive on purpose, so a new -/// `AppMode` fails the build here (improvise-zx8t) — `context/code/tui.md`. -fn mode_style(mode: &AppMode) -> Style { - let bg = match mode { - AppMode::Editing { .. } | AppMode::RecordsEditing { .. } => Color::Green, - AppMode::CommandMode { .. } => Color::Yellow, - AppMode::TileSelect => Color::Magenta, - AppMode::Normal - | AppMode::RecordsNormal - | AppMode::FormulaEdit { .. } - | AppMode::FormulaPanel - | AppMode::CategoryPanel - | AppMode::CategoryAdd { .. } - | AppMode::ItemAdd { .. } - | AppMode::ViewPanel - | AppMode::ImportWizard - | AppMode::ExportPrompt { .. } - | AppMode::SavePrompt { .. } - | AppMode::Help - | AppMode::Quit => Color::DarkGray, - }; - Style::default().fg(Color::Black).bg(bg) -} - fn draw(f: &mut Frame, app: &App) { let size = f.area(); @@ -442,12 +449,9 @@ fn draw_bottom_bar(f: &mut Frame, area: Rect, app: &App) { .unwrap_or(""); let text = minibuffer_line(&mb.prompt, buf, &app.view_state.status_msg, area.width); f.render_widget( - Paragraph::new(text).style( - Style::default() - .fg(minibuf_fg(mb.color)) - .bg(Color::Indexed(235)) - .add_modifier(Modifier::BOLD), - ), + Paragraph::new(text).style(face_style( + Face::of_minibuffer(&app.view_state.mode).unwrap_or(Face::ModeTextEntry), + )), area, ); } else { @@ -484,7 +488,7 @@ fn draw_status(f: &mut Frame, area: Rect, app: &App) { let line = fill_line(left, &view_badge, area.width, Keeps::Left); f.render_widget( - Paragraph::new(line).style(mode_style(&app.view_state.mode)), + Paragraph::new(line).style(face_style(Face::of_mode(&app.view_state.mode))), area, ); } @@ -639,6 +643,43 @@ mod bottom_bar_tests { ); } + /// The first cell of the bottom row, where the prompt's first glyph lands. + fn prompt_cell(app: &App) -> ratatui::buffer::Cell { + let mut terminal = Terminal::new(TestBackend::new(40, 1)).expect("test backend"); + terminal + .draw(|f| draw_bottom_bar(f, Rect::new(0, 0, 40, 1), app)) + .expect("draw"); + terminal.backend().buffer().content()[0].clone() + } + + /// ADR 004 clause 8's one declared exception (the user, Q1): the + /// category-add prompt draws as item-add does. `context/test-evidence/tui.md`. + #[test] + fn the_category_add_prompt_draws_as_the_item_add_prompt_does() { + let mut app = App::new(Workbook::new("T"), None); + app.view_state.mode = AppMode::item_add("Region".into()); + let item = prompt_cell(&app); + app.view_state.mode = AppMode::category_add(); + let category = prompt_cell(&app); + assert_eq!( + category.symbol(), + "n", + "the prompt is not where it was looked for" + ); + assert_eq!(item.fg, ratatui::style::Color::Green); + assert_eq!(category.style(), item.style()); + } + + /// The status line wears the mode's face, not the minibuffer's. + #[test] + fn the_status_line_is_drawn_in_the_mode_face() { + let mut app = App::new(Workbook::new("T"), None); + app.view_state.mode = AppMode::TileSelect; + let cell = prompt_cell(&app); + assert_eq!(cell.bg, ratatui::style::Color::Magenta); + assert_eq!(cell.fg, ratatui::style::Color::Black); + } + /// A refusal that keeps the user in a minibuffer stands while they fix it, and /// goes when they leave (improvise-su4u). `context/test-evidence/tui.md`. #[test] @@ -1187,3 +1228,94 @@ mod content_tests { } } } + +#[cfg(test)] +mod face_tests { + use super::*; + use crate::protocol::face::TextEntry; + use crate::protocol::render::WizardFieldKind; + use crate::view::Axis; + + /// `mode_style`'s table as it stood before faces (improvise-zx8t). + fn mode_bg_before_faces(mode: &AppMode) -> Color { + match mode { + AppMode::Editing { .. } | AppMode::RecordsEditing { .. } => Color::Green, + AppMode::CommandMode { .. } => Color::Yellow, + AppMode::TileSelect => Color::Magenta, + AppMode::Normal + | AppMode::RecordsNormal + | AppMode::FormulaEdit { .. } + | AppMode::FormulaPanel + | AppMode::CategoryPanel + | AppMode::CategoryAdd { .. } + | AppMode::ItemAdd { .. } + | AppMode::ViewPanel + | AppMode::ImportWizard + | AppMode::ExportPrompt { .. } + | AppMode::SavePrompt { .. } + | AppMode::Help + | AppMode::Quit => Color::DarkGray, + } + } + + /// ADR 004 clause 8: the shipped theme draws every migrated site exactly as + /// its literal did. `context/test-evidence/tui.md`. + #[test] + fn the_shipped_theme_reproduces_every_migrated_literal() { + let fg = |c| Style::default().fg(c); + let axes = [ + (Axis::Row, Color::Green), + (Axis::Column, Color::Blue), + (Axis::Page, Color::Magenta), + (Axis::Filter, Color::Yellow), + (Axis::None, Color::DarkGray), + ]; + for (axis, color) in axes { + assert_eq!(face_style(Face::Axis(axis)), fg(color), "{axis:?}"); + } + let kinds = [ + (WizardFieldKind::Category, Color::Green), + (WizardFieldKind::Measure, Color::Cyan), + (WizardFieldKind::TimeCategory, Color::Magenta), + (WizardFieldKind::Label, Color::DarkGray), + ]; + for (kind, color) in kinds { + assert_eq!(face_style(Face::WizardField(kind)), fg(color), "{kind:?}"); + } + for mode in AppMode::all() { + let before = fg(Color::Black).bg(mode_bg_before_faces(&mode)); + assert_eq!(face_style(Face::of_mode(&mode)), before, "{mode:?}"); + } + let prompts = [ + (TextEntry::Edit, Color::Green), + (TextEntry::ItemAdd, Color::Green), + (TextEntry::Formula, Color::Cyan), + (TextEntry::Command, Color::Yellow), + (TextEntry::Export, Color::Yellow), + (TextEntry::Save, Color::Yellow), + ]; + for (entry, color) in prompts { + let before = fg(color) + .bg(Color::Indexed(235)) + .add_modifier(Modifier::BOLD); + assert_eq!(face_style(Face::MinibufferOf(entry)), before, "{entry:?}"); + } + } + + #[test] + fn every_face_reaches_the_terminal_with_every_attribute_it_states() { + let resolved = |face| Theme::shipped().resolve(face); + for face in Face::all() { + let r = resolved(face); + let style = face_style(face); + assert_eq!(style.fg, terminal_colour(r.foreground), "{}", face.name()); + assert_eq!(style.bg, terminal_colour(r.background), "{}", face.name()); + assert_eq!( + style.add_modifier.contains(Modifier::BOLD), + r.weight == Weight::Bold, + "{}", + face.name() + ); + } + } +} diff --git a/src/ui/category_panel.rs b/src/ui/category_panel.rs index 6ee4b2e2..7bfb865e 100644 --- a/src/ui/category_panel.rs +++ b/src/ui/category_panel.rs @@ -6,13 +6,14 @@ use ratatui::{ use unicode_width::UnicodeWidthStr; +use crate::protocol::face::Face; use crate::protocol::{CatTreeEntry, CategoryTreeCache, flatten_cat_tree}; use crate::ui::app::Panel as PanelKind; use crate::ui::panel::PanelContent; use crate::view::Axis; -fn axis_display(axis: Axis) -> (String, Color) { - (axis.to_string(), crate::draw::axis_fg(axis.color())) +fn axis_display(axis: Axis) -> (String, Style) { + (axis.to_string(), crate::draw::face_style(Face::Axis(axis))) } /// The category panel draws one flattened tree and nothing else: the axis @@ -78,7 +79,7 @@ impl PanelContent for CategoryContent { axis, } => { let indicator = if *expanded { "▼" } else { "▶" }; - let (axis_str, axis_color) = axis_display(*axis); + let (axis_str, axis_style) = axis_display(*axis); let name_part = format!("{indicator} {name} ({item_count})"); let axis_part = format!(" [{axis_str}]"); @@ -88,11 +89,7 @@ impl PanelContent for CategoryContent { inner.x + name_part.width() as u16, y, &axis_part, - if is_selected { - base_style - } else { - Style::default().fg(axis_color) - }, + if is_selected { base_style } else { axis_style }, ); } } diff --git a/src/ui/import_wizard_ui.rs b/src/ui/import_wizard_ui.rs index f9d11183..d28f20f0 100644 --- a/src/ui/import_wizard_ui.rs +++ b/src/ui/import_wizard_ui.rs @@ -7,7 +7,8 @@ use ratatui::{ use unicode_width::UnicodeWidthStr; -use crate::protocol::{WizardCache, WizardFieldKind, WizardStepKind}; +use crate::protocol::face::Face; +use crate::protocol::{WizardCache, WizardStepKind}; use crate::ui::text::wrap; /// The import wizard, drawn from a [`WizardCache`] (improvise-764) and from @@ -22,17 +23,6 @@ impl<'a> ImportWizardWidget<'a> { } } -/// Map a proposed field's kind to its accent, as `axis_fg` does for -/// `AxisColor` (gotcha 26). The decision is the protocol's; the color is not. -fn field_kind_fg(kind: WizardFieldKind) -> Color { - match kind { - WizardFieldKind::Category => Color::Green, - WizardFieldKind::Measure => Color::Cyan, - WizardFieldKind::TimeCategory => Color::Magenta, - WizardFieldKind::Label => Color::DarkGray, - } -} - impl<'a> Widget for ImportWizardWidget<'a> { fn render(self, area: Rect, buf: &mut Buffer) { let popup_w = area.width.min(80); @@ -139,7 +129,7 @@ impl<'a> Widget for ImportWizardWidget<'a> { } let is_sel = i == self.wizard.cursor; - let kind_color = field_kind_fg(proposal.kind); + let kind_style = crate::draw::face_style(Face::WizardField(proposal.kind)); let accept_str = if proposal.accepted { "[\u{2713}]" @@ -159,7 +149,7 @@ impl<'a> Widget for ImportWizardWidget<'a> { .bg(Color::Cyan) .add_modifier(Modifier::BOLD) } else if proposal.accepted { - Style::default().fg(kind_color) + kind_style } else { Style::default().fg(Color::DarkGray) }; diff --git a/src/ui/tile_bar.rs b/src/ui/tile_bar.rs index 60fac265..75dc0d27 100644 --- a/src/ui/tile_bar.rs +++ b/src/ui/tile_bar.rs @@ -7,6 +7,7 @@ use ratatui::{ use unicode_width::UnicodeWidthStr; use crate::protocol::TileBarCache; +use crate::protocol::face::Face; use crate::ui::app::AppMode; use crate::view::Axis; @@ -44,8 +45,11 @@ impl<'a> TileBar<'a> { tile_cat_idx, } } - fn axis_display(axis: Axis) -> (&'static str, Color) { - (axis.short_label(), crate::draw::axis_fg(axis.color())) + fn axis_display(axis: Axis) -> (&'static str, Style) { + ( + axis.short_label(), + crate::draw::face_style(Face::Axis(axis)), + ) } } @@ -101,7 +105,7 @@ impl<'a> Widget for TileBar<'a> { break; } - let (_, axis_color) = TileBar::axis_display(tiles[i].axis); + let (_, axis_style) = TileBar::axis_display(tiles[i].axis); let is_selected = selected_cat_idx == Some(i); let style = if is_selected { Style::default() @@ -109,7 +113,7 @@ impl<'a> Widget for TileBar<'a> { .bg(Color::Cyan) .add_modifier(Modifier::BOLD) } else { - Style::default().fg(axis_color) + axis_style }; buf.set_string(x, area.y, &labels[i], style);