From a0bd4d7f6c14db431753c8fe336893f163d4c2b4 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 10 Jun 2026 17:14:59 -0400 Subject: [PATCH 1/6] mode line: per-buffer diagnostic counts (E:n W:n) M4.6 follow-up piece 1: the mode line's right segment now shows error/warning counts for the window's buffer, computed from the shared diag store at paint time. Counts are suppressed while the URI's diagnostics are stale (mid-edit, pre-publish) so the readout never describes text that no longer exists. Info/hint severities stay off the mode line. Co-Authored-By: Claude Fable 5 --- src/editor.rs | 130 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 128 insertions(+), 2 deletions(-) diff --git a/src/editor.rs b/src/editor.rs index 861af26..2e22edc 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -1207,6 +1207,8 @@ pub fn paint_frame( // Render every window. let registry = core.registry.clone(); let reg = registry.borrow(); + let diag_store = state.lsp_manager.borrow().diag_store(); + let diag_guard = diag_store.lock().expect("diag store mutex poisoned"); for (id, window) in &mut core.windows { let Some(rect) = placements.get(id).copied() else { continue; @@ -1247,6 +1249,7 @@ pub fn paint_frame( window.text_view.line_count(), coord.row as usize, ); + let diags = diag_mode_line_summary(&diag_guard, buf); paint_mode_line( grid, &rect, @@ -1256,8 +1259,10 @@ pub fn paint_frame( coord.row, coord.col, &scroll, + &diags, ); } + drop(diag_guard); drop(reg); paint_status_line(grid, core, &state.lua_host, &state.dispatcher, term_size); @@ -1398,9 +1403,43 @@ fn paint_local_selection( } } +/// Format the mode-line diagnostic readout for a buffer: `"E:2 W:5"` +/// with only the nonzero severities (errors, then warnings; info and +/// hints stay off the mode line). Empty when the buffer has no file +/// path, no diagnostics, or the stored diagnostics are stale — the +/// document was edited since the last `publishDiagnostics`, so the +/// counts would describe text that no longer exists (T M4.6). +fn diag_mode_line_summary( + store: &crate::diag::DiagnosticStore, + buf: &crate::buffer::Buffer, +) -> String { + let Some(path) = buf.file_path() else { + return String::new(); + }; + let uri = crate::lsp::path_to_file_uri(path); + if store.is_stale(&uri) { + return String::new(); + } + let mut errors = 0usize; + let mut warnings = 0usize; + for d in store.for_uri(&uri) { + match d.severity { + crate::diag::DiagnosticSeverity::Error => errors += 1, + crate::diag::DiagnosticSeverity::Warning => warnings += 1, + _ => {} + } + } + match (errors, warnings) { + (0, 0) => String::new(), + (e, 0) => format!("E:{e}"), + (0, w) => format!("W:{w}"), + (e, w) => format!("E:{e} W:{w}"), + } +} + #[allow( clippy::too_many_arguments, - reason = "the mode line packs eight unrelated facts; bundling them into a struct just adds ceremony" + reason = "the mode line packs nine unrelated facts; bundling them into a struct just adds ceremony" )] fn paint_mode_line( grid: &mut crate::cell::CellGrid<'_>, @@ -1411,6 +1450,7 @@ fn paint_mode_line( cursor_row: u32, cursor_col: u32, scroll: &str, + diags: &str, ) { if rect.size.rows == 0 || rect.size.cols == 0 { return; @@ -1419,7 +1459,11 @@ fn paint_mode_line( let marker = if modified { '*' } else { ' ' }; let active_marker = if is_active { '+' } else { '-' }; let left = format!(" {active_marker}{marker} {name} "); - let right = format!(" L{}:C{} {scroll} ", cursor_row + 1, cursor_col + 1); + let right = if diags.is_empty() { + format!(" L{}:C{} {scroll} ", cursor_row + 1, cursor_col + 1) + } else { + format!(" {diags} L{}:C{} {scroll} ", cursor_row + 1, cursor_col + 1) + }; // Fill the row with reverse-video spaces. let mode_style = crate::cell::Style { @@ -5089,6 +5133,88 @@ mod tests { } } + /// Give the active buffer a file path and return its `file://` + /// URI, so diag-store entries can be keyed to it. + fn set_active_buffer_path(s: &EditorState, path: &str) -> String { + let core = s.core.borrow(); + let registry = core.registry.clone(); + let mut reg = registry.borrow_mut(); + let buf = reg.get_mut(core.active_buffer_id()).unwrap(); + buf.set_file_path(Some(std::path::PathBuf::from(path))); + crate::lsp::path_to_file_uri(buf.file_path().unwrap()) + } + + fn diag_with_severity(severity: crate::diag::DiagnosticSeverity) -> crate::diag::Diagnostic { + crate::diag::Diagnostic { + start_line: 0, + start_col: 0, + end_line: 0, + end_col: 1, + severity, + message: "boom".into(), + source: None, + code: None, + } + } + + #[test] + fn render_mode_line_shows_diagnostic_counts() { + use crate::diag::DiagnosticSeverity::{Error, Hint, Warning}; + let s = fresh_with(b"hello\n"); + let uri = set_active_buffer_path(&s, "/tmp/modeline_diag.rs"); + let store = s.lsp_manager.borrow().diag_store(); + store.lock().unwrap().set( + uri, + vec![ + diag_with_severity(Error), + diag_with_severity(Error), + diag_with_severity(Warning), + diag_with_severity(Hint), // hints stay off the mode line + ], + ); + let (cells, stride, _) = render_to_grid(&s, 24, 80); + let mode_text = row_text(&cells, stride, 22, 80); + assert!( + mode_text.contains("E:2 W:1"), + "mode line missing diagnostic counts: {mode_text:?}" + ); + assert!( + !mode_text.contains("H:"), + "hints should not appear on the mode line: {mode_text:?}" + ); + } + + #[test] + fn render_mode_line_hides_diagnostic_counts_while_stale() { + use crate::diag::DiagnosticSeverity::Error; + let s = fresh_with(b"hello\n"); + let uri = set_active_buffer_path(&s, "/tmp/modeline_stale.rs"); + let store = s.lsp_manager.borrow().diag_store(); + { + let mut guard = store.lock().unwrap(); + guard.set(uri.clone(), vec![diag_with_severity(Error)]); + guard.mark_stale(uri); + } + let (cells, stride, _) = render_to_grid(&s, 24, 80); + let mode_text = row_text(&cells, stride, 22, 80); + assert!( + !mode_text.contains("E:"), + "stale diagnostics must not reach the mode line: {mode_text:?}" + ); + } + + #[test] + fn render_mode_line_omits_diagnostic_counts_when_clean() { + let s = fresh_with(b"hello\n"); + let _uri = set_active_buffer_path(&s, "/tmp/modeline_clean.rs"); + let (cells, stride, _) = render_to_grid(&s, 24, 80); + let mode_text = row_text(&cells, stride, 22, 80); + assert!( + !mode_text.contains("E:") && !mode_text.contains("W:"), + "clean buffer must not show diagnostic counts: {mode_text:?}" + ); + } + #[test] fn render_places_cursor_on_active_window() { let mut s = fresh_with(b"abc\n"); From 72799f771a1f543467a8ea2513a4235c96211a44 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 10 Jun 2026 19:16:42 -0400 Subject: [PATCH 2/6] 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