diff --git a/context/code/protocol.md b/context/code/protocol.md index 0e695880..abb34291 100644 --- a/context/code/protocol.md +++ b/context/code/protocol.md @@ -2489,14 +2489,17 @@ dev-dependency here. ### Choices the ADR did not make, made here -- **`minibuffer.*` inherits a `minibuffer` base face, not `mode.text-entry`.** - The ADR says the minibuffer faces "inherit `mode.text-entry` directly". But - the mode indicator and the minibuffer paint opposite halves of the cell — the - indicator is Black ink on an accent background, the minibuffer is accent ink - on `Indexed(235)` — so inheriting one from the other makes every attribute an - override and the chain decorative. The `minibuffer` face carries only what - the strip has in common (its background and bold), and encodes nothing about - green and yellow, which is the property the ADR's sentence was protecting. +- **`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`. diff --git a/context/test-evidence/protocol.md b/context/test-evidence/protocol.md index c44cd45b..882f49ab 100644 --- a/context/test-evidence/protocol.md +++ b/context/test-evidence/protocol.md @@ -1283,3 +1283,19 @@ 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. diff --git a/crates/improvise-protocol/src/face.rs b/crates/improvise-protocol/src/face.rs index e2ecdbf4..5173b575 100644 --- a/crates/improvise-protocol/src/face.rs +++ b/crates/improvise-protocol/src/face.rs @@ -276,7 +276,6 @@ pub enum Face { ModeOverlay, ModeTransient, ModeTileSelect, - Minibuffer, MinibufferOf(TextEntry), } @@ -290,7 +289,6 @@ impl Face { out.extend(TextEntry::ALL.map(Face::ModeTextEntryOf)); out.extend(Panel::ALL.map(Face::ModePanel)); out.extend([Face::ModeOverlay, Face::ModeTransient, Face::ModeTileSelect]); - out.push(Face::Minibuffer); out.extend(TextEntry::ALL.map(Face::MinibufferOf)); out } @@ -299,7 +297,7 @@ impl Face { pub fn parent(self) -> Option { match self { Face::Default => None, - Face::Dim | Face::Mode | Face::Minibuffer => Some(Face::Default), + 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 @@ -308,8 +306,7 @@ impl Face { | Face::ModeOverlay | Face::ModeTransient | Face::ModeTileSelect => Some(Face::Mode), - Face::ModeTextEntryOf(_) => Some(Face::ModeTextEntry), - Face::MinibufferOf(_) => Some(Face::Minibuffer), + Face::ModeTextEntryOf(_) | Face::MinibufferOf(_) => Some(Face::ModeTextEntry), } } @@ -328,7 +325,6 @@ impl Face { Face::ModeOverlay => "mode.overlay".into(), Face::ModeTransient => "mode.transient".into(), Face::ModeTileSelect => "mode.tile-select".into(), - Face::Minibuffer => "minibuffer".into(), Face::MinibufferOf(entry) => format!("minibuffer.{}", entry.name()), } } @@ -450,6 +446,12 @@ impl Theme { 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))), @@ -485,42 +487,16 @@ impl Theme { FaceSpec::bg(named(Yellow)), ), (Face::ModeTileSelect, FaceSpec::bg(named(Magenta))), - ( - Face::Minibuffer, - FaceSpec { - background: Some(Colour::Indexed(235)), - weight: Some(Weight::Bold), - ..FaceSpec::default() - }, - ), - ( - Face::MinibufferOf(TextEntry::Edit), - FaceSpec::fg(named(Green)), - ), - ( - Face::MinibufferOf(TextEntry::ItemAdd), - FaceSpec::fg(named(Green)), - ), + (Face::MinibufferOf(TextEntry::Edit), minibuffer(Green)), + (Face::MinibufferOf(TextEntry::ItemAdd), minibuffer(Green)), ( Face::MinibufferOf(TextEntry::CategoryAdd), - FaceSpec::fg(named(Green)), - ), - ( - Face::MinibufferOf(TextEntry::Formula), - FaceSpec::fg(named(Cyan)), - ), - ( - Face::MinibufferOf(TextEntry::Command), - FaceSpec::fg(named(Yellow)), - ), - ( - Face::MinibufferOf(TextEntry::Export), - FaceSpec::fg(named(Yellow)), - ), - ( - Face::MinibufferOf(TextEntry::Save), - FaceSpec::fg(named(Yellow)), + 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(), @@ -581,6 +557,22 @@ mod tests { } } + /// 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() { diff --git a/src/draw.rs b/src/draw.rs index 6005c190..af5a86e7 100644 --- a/src/draw.rs +++ b/src/draw.rs @@ -450,7 +450,7 @@ fn draw_bottom_bar(f: &mut Frame, area: Rect, app: &App) { let text = minibuffer_line(&mb.prompt, buf, &app.view_state.status_msg, area.width); f.render_widget( Paragraph::new(text).style(face_style( - Face::of_minibuffer(&app.view_state.mode).unwrap_or(Face::Minibuffer), + Face::of_minibuffer(&app.view_state.mode).unwrap_or(Face::ModeTextEntry), )), area, );