From 72799f771a1f543467a8ea2513a4235c96211a44 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 10 Jun 2026 19:16:42 -0400 Subject: [PATCH] protocol v6: per-severity diagnostic underline colors (SGR 58) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit M4.6 follow-up piece 2. `Style` gains `underline_color: Color` (Default = follow the text color) so a diagnostic squiggle can be red/yellow/cyan/gray without clobbering the syntax color of the text it underlines — exactly why error_style() left its 'red' unwired until now. The wire consequence: Style rides inside Cell / CellDelta / Snapshot / StyleSpans, so this is the protocol's first encoding-breaking change. PROTOCOL_VERSION 5 → 6 and SUPPORTED_PROTOCOL_VERSIONS narrows to [6]: postcard is not self-describing, so no per-session send gate can keep a v5 peer decoding v6 cells — a mismatched pair now fails the handshake with a clean VersionMismatch instead of garbling mid-session. Version policy tests rewritten to pin the new contract. Surface wiring: - diag.rs: per-severity underline_color (indexed 1/3/6/8). - frontend.rs: kitty-style CSI 4:N for Double/Curly/Dotted/Dashed (previously flattened to plain SGR 4) + SGR 58:5/58:2 emission. - ansi.rs: parse SGR 58/59 with the 38/48 extended-color grammar. - overlay.rs merge_styles: non-default-wins, like fg/bg/underline. - lua_bindings.rs: underline_color on Lua style tables. Co-Authored-By: Claude Fable 5 --- pmacs-protocol/src/cell.rs | 5 ++ pmacs-protocol/src/message.rs | 23 +++++- src/ansi.rs | 38 ++++++++++ src/diag.rs | 45 ++++++++++-- src/frontend.rs | 78 ++++++++++++++++++-- src/lua_bindings.rs | 5 +- src/overlay.rs | 45 ++++++++++-- src/protocol.rs | 135 ++++++++++++---------------------- 8 files changed, 265 insertions(+), 109 deletions(-) diff --git a/pmacs-protocol/src/cell.rs b/pmacs-protocol/src/cell.rs index a79c233..2b50199 100644 --- a/pmacs-protocol/src/cell.rs +++ b/pmacs-protocol/src/cell.rs @@ -125,6 +125,11 @@ pub struct Style { pub underline: UnderlineStyle, /// Reverse video. pub reverse: bool, + /// Underline color (SGR 58/59). `Color::Default` means "follow + /// the text color": the underline draws in `fg`. Diagnostics set + /// this per severity so the squiggle color can differ from the + /// syntax-colored text it underlines (T M4.6, protocol v6). + pub underline_color: Color, } /// A non-text attachment carried in a cell (TUI ignores this). diff --git a/pmacs-protocol/src/message.rs b/pmacs-protocol/src/message.rs index df07cb5..7f2963c 100644 --- a/pmacs-protocol/src/message.rs +++ b/pmacs-protocol/src/message.rs @@ -1020,7 +1020,18 @@ pub enum ResourceBody { /// frontend sends `Pointer` only when the instance's /// `Hello.protocol_version >= 5`, because an older instance would /// hard-error decoding the unknown variant. -pub const PROTOCOL_VERSION: u32 = 5; +/// +/// T M4.6 (diagnostic surface): bumped from 5 to 6 for +/// `Style::underline_color`. Unlike every previous bump, this one +/// changes the *encoding of an existing struct* — `Style` rides +/// inside `Cell` / `CellDelta` / `Snapshot` / `StyleSpans`, messages +/// every session receives — so per-session send gating cannot +/// preserve compatibility (postcard is not self-describing; a v5 +/// decoder mis-reads any v6 `Style`). v6 binaries therefore accept +/// only v6 peers: a version-mismatched pair fails the handshake with +/// [`GoodbyeReason::VersionMismatch`] instead of garbling cell +/// traffic mid-session. +pub const PROTOCOL_VERSION: u32 = 6; /// T M10.5: the set of protocol versions a v1.0 binary accepts on /// the wire. v0.1 binaries only accepted `[1]`; v1.0 binaries accept @@ -1044,7 +1055,15 @@ pub const PROTOCOL_VERSION: u32 = 5; /// Mouse framing Q#M1: extended to `[1, 2, 3, 4, 5]`. v5 peers may /// send `FrontendEvent::Pointer`; the frontend-side gate (see /// [`PROTOCOL_VERSION`]) keeps the variant off wires negotiated `< 5`. -pub const SUPPORTED_PROTOCOL_VERSIONS: &[u32] = &[1, 2, 3, 4, 5]; +/// +/// T M4.6: narrowed to `[6]`. `Style::underline_color` changed the +/// postcard encoding of every cell-carrying message, so v6 binaries +/// cannot exchange cell traffic with any earlier wire. The v1–v5 +/// compat ladder (additive variants, per-session filtering) assumed +/// shared-struct encodings never changed; this bump is the first +/// that breaks that assumption, and slice membership is how the +/// handshake communicates it. +pub const SUPPORTED_PROTOCOL_VERSIONS: &[u32] = &[6]; /// T M10.5: predicate for the handshake check. Returns `true` if /// `peer_version` is in [`SUPPORTED_PROTOCOL_VERSIONS`]. diff --git a/src/ansi.rs b/src/ansi.rs index 8ec4d24..37f19fe 100644 --- a/src/ansi.rs +++ b/src/ansi.rs @@ -931,6 +931,15 @@ impl AnsiParser { } } 49 => self.current_style.bg = Color::Default, + // 58/59 (underline color, kitty/mintty extension): + // same extended-color grammar as 38/48 (T M4.6). + 58 => { + if let Some((color, extra)) = parse_extended_color(p, ¶ms[i + 1..]) { + self.current_style.underline_color = color; + consumed_extra = extra; + } + } + 59 => self.current_style.underline_color = Color::Default, 90..=97 => { self.current_style.fg = Color::Indexed(u8::try_from(p.main - 90 + 8).unwrap_or(0)); @@ -1266,6 +1275,35 @@ mod tests { ); } + /// Underline color via SGR 58 (both colon-subparam and semicolon + /// grammars, mirroring 38/48), reset via SGR 59 (T M4.6). + #[test] + fn m4_6_underline_color() { + let cases: &[(&[u8], Color)] = &[ + (b"\x1b[58:5:1m", Color::Indexed(1)), + (b"\x1b[58;5;124m", Color::Indexed(124)), + (b"\x1b[58:2::255:0:0m", Color::Rgb(255, 0, 0)), + ]; + for (input, expected) in cases { + let mut p = AnsiParser::new(); + let evs = p.feed(input); + let styles = collect_styles(&evs); + assert_eq!( + styles.first().map(|s| s.underline_color), + Some(*expected), + "input {input:?} should produce underline color {expected:?}; got {evs:?}" + ); + } + // 59 resets to follow-text-color. + let mut p = AnsiParser::new(); + let evs = p.feed(b"\x1b[58:5:1m\x1b[59m"); + let styles = collect_styles(&evs); + assert_eq!( + styles.last().map(|s| s.underline_color), + Some(Color::Default) + ); + } + // ----------------------------------------------------------------- // Intra-line motion and erase // ----------------------------------------------------------------- diff --git a/src/diag.rs b/src/diag.rs index 3dfe2bf..ed12b31 100644 --- a/src/diag.rs +++ b/src/diag.rs @@ -332,14 +332,14 @@ pub fn make_shared_store() -> SharedDiagStore { const TAB_WIDTH: u32 = 8; /// Style applied to bytes covered by an `Error` diagnostic. Wavy -/// underline in red so it composes with whatever the syntax view -/// painted on top. +/// underline colored via `underline_color` (not `fg`) so the +/// squiggle reads red while the syntax view's text color survives +/// underneath (T M4.6, protocol v6). fn error_style() -> Style { Style { underline: UnderlineStyle::Curly, - // Red. Indexed 1 is portable across 8/16-color terminals. - fg: Color::Default, - bg: Color::Default, + // Indexed 1 (red) is portable across 8/16-color terminals. + underline_color: Color::Indexed(1), ..Style::default() } } @@ -348,6 +348,8 @@ fn error_style() -> Style { fn warning_style() -> Style { Style { underline: UnderlineStyle::Curly, + // Indexed 3: yellow. + underline_color: Color::Indexed(3), ..Style::default() } } @@ -356,6 +358,8 @@ fn warning_style() -> Style { fn info_style() -> Style { Style { underline: UnderlineStyle::Single, + // Indexed 6: cyan. + underline_color: Color::Indexed(6), ..Style::default() } } @@ -364,6 +368,9 @@ fn info_style() -> Style { fn hint_style() -> Style { Style { underline: UnderlineStyle::Dotted, + // Indexed 8: bright black ("gray") — present on 16-color + // terminals, subtle by design for hints. + underline_color: Color::Indexed(8), ..Style::default() } } @@ -791,6 +798,33 @@ mod tests { } } + #[test] + fn severity_styles_color_the_underline_not_the_text() { + // T M4.6: each severity gets a distinct underline color via + // `underline_color` (SGR 58); `fg` stays Default so the + // syntax view's text color survives the merge. + let mut seen = Vec::new(); + for s in [ + DiagnosticSeverity::Error, + DiagnosticSeverity::Warning, + DiagnosticSeverity::Information, + DiagnosticSeverity::Hint, + ] { + let style = style_for(s); + assert_eq!(style.fg, Color::Default, "{s:?} must not set fg"); + assert_ne!( + style.underline_color, + Color::Default, + "{s:?} must color its underline" + ); + assert!( + !seen.contains(&style.underline_color), + "{s:?} reuses another severity's underline color" + ); + seen.push(style.underline_color); + } + } + #[test] fn view_advertises_diagnostic_kind() { // `pmacs.window._overlay_kinds()` introspection (task #23 wire-up, @@ -845,4 +879,5 @@ mod tests { "stale diagnostics must not underline shifted TUI bytes" ); } + } diff --git a/src/frontend.rs b/src/frontend.rs index fc5be2b..6a69ad3 100644 --- a/src/frontend.rs +++ b/src/frontend.rs @@ -634,16 +634,30 @@ fn apply_style(w: &mut W, style: &Style) -> io::Result<()> { } match style.underline { crate::cell::UnderlineStyle::None => {} - crate::cell::UnderlineStyle::Single - | crate::cell::UnderlineStyle::Double - | crate::cell::UnderlineStyle::Curly - | crate::cell::UnderlineStyle::Dotted - | crate::cell::UnderlineStyle::Dashed => { - // crossterm exposes Underlined; richer styles need a manual - // CSI 4:N m emit, which most terminals fall back to plain - // underline on. M2 can wire CSI 4:N m for kitty/iTerm. + crate::cell::UnderlineStyle::Single => { queue!(w, SetAttribute(Attribute::Underlined))?; } + // Kitty-style underline subparameters (CSI 4:N m). Terminals + // that predate the extension treat the whole sequence as + // SGR 4 (plain underline) or ignore the subparameter, so the + // fallback is a straight underline — same as the previous + // flatten-to-`Underlined` behavior (T M4.6). + crate::cell::UnderlineStyle::Double => write!(w, "\x1b[4:2m")?, + crate::cell::UnderlineStyle::Curly => write!(w, "\x1b[4:3m")?, + crate::cell::UnderlineStyle::Dotted => write!(w, "\x1b[4:4m")?, + crate::cell::UnderlineStyle::Dashed => write!(w, "\x1b[4:5m")?, + } + // Underline color (SGR 58, colon subparameter form). Emitted only + // when set and an underline is present: the SGR 0 at the top of + // this function already reset the underline color to + // follow-text-color (SGR 59 state), which is what + // `Color::Default` means. + if style.underline != crate::cell::UnderlineStyle::None { + match style.underline_color { + Color::Default => {} + Color::Rgb(r, g, b) => write!(w, "\x1b[58:2::{r}:{g}:{b}m")?, + Color::Indexed(n) => write!(w, "\x1b[58:5:{n}m")?, + } } if style.reverse { queue!(w, SetAttribute(Attribute::Reverse))?; @@ -771,6 +785,54 @@ mod tests { assert!(s.contains("\x1b[1m"), "missing bold-on in {s:?}"); } + #[test] + fn curly_underline_emits_csi_4_3_and_underline_color() { + // A diagnostic-style cell: curly underline colored red via + // SGR 58 (T M4.6). The text color must NOT be touched. + let span = DiffSpan { + start: CellCoord::new(0, 0), + cells: vec![Cell { + glyph: Glyph::Char('x'), + style: Style { + underline: crate::cell::UnderlineStyle::Curly, + underline_color: crate::cell::Color::Indexed(1), + ..Style::default() + }, + attachment: None, + }], + }; + let mut out = Vec::new(); + emit_span(&mut out, &span).unwrap(); + let s = String::from_utf8_lossy(&out); + assert!(s.contains("\x1b[4:3m"), "missing curly underline in {s:?}"); + assert!( + s.contains("\x1b[58:5:1m"), + "missing underline color in {s:?}" + ); + } + + #[test] + fn default_underline_color_emits_no_sgr_58() { + // A plain single underline with follow-text color: no SGR 58 + // on the wire (the reset at the start of every style apply + // already put the terminal in SGR 59 state). + let span = DiffSpan { + start: CellCoord::new(0, 0), + cells: vec![Cell { + glyph: Glyph::Char('x'), + style: Style { + underline: crate::cell::UnderlineStyle::Single, + ..Style::default() + }, + attachment: None, + }], + }; + let mut out = Vec::new(); + emit_span(&mut out, &span).unwrap(); + let s = String::from_utf8_lossy(&out); + assert!(!s.contains("\x1b[58"), "unexpected SGR 58 in {s:?}"); + } + // ---- status overlay (T M5.8) ----------------------------------------- #[test] diff --git a/src/lua_bindings.rs b/src/lua_bindings.rs index 33afe52..2f82183 100644 --- a/src/lua_bindings.rs +++ b/src/lua_bindings.rs @@ -6248,6 +6248,7 @@ fn lua_to_style(t: &Table) -> mlua::Result