diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 25fb74f..56ec2d1 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -358,37 +358,90 @@ local function is_utf8_continuation(b) return b >= 0x80 and b < 0xC0 end +-- Codepoint count of `s` (lead bytes only; `s` always holds complete +-- scalars — parser text events never carry a partial sequence). +local function count_codepoints(s) + local n = 0 + for i = 1, #s do + if not is_utf8_continuation(s:byte(i)) then n = n + 1 end + end + return n +end + +-- Byte length of the first `n` codepoints of `s`, clamped to #s. +local function codepoint_prefix_bytes(s, n) + local len = #s + local i = 0 + local seen = 0 + while i < len and seen < n do + i = i + 1 + while i < len and is_utf8_continuation(s:byte(i + 1)) do + i = i + 1 + end + seen = seen + 1 + end + return i +end + -- Append `text` at the tracked output position with overwrite -- semantics (CR progress bars rewrite the current line in place). -- --- UTF-8 safety (PR #113 round-3 finding 1): overwrite ranges are --- byte-counted but must consume WHOLE existing codepoints — the --- range end is aligned forward past continuation bytes — and the --- incoming text event is applied as ONE atomic replace, never split --- (parser text events carry only complete scalars). Splitting either --- side left malformed bytes on the plain rope and made the --- byte-native CRDT edit reject the range, aborting the pump --- mid-batch after its events were already consumed. `out_pos` stays --- on codepoint boundaries by induction: it moves to end-of-write, --- line starts (after \n), or a boundary-aligned backspace target. +-- Overwrites are COLUMN-counted and newline-segmented (PR #113 +-- round-4 finding 1; codepoints approximate columns — double-width +-- and combining characters count as one, the framing's documented +-- stance). Each newline-free segment consumes one existing codepoint +-- per incoming codepoint — `abc\ré` yields `ébc`, never the +-- byte-counted `éc` — and LF is NOT an overwrite column: a newline +-- arriving mid-line drops the cursor to a fresh line and the stale +-- remainder of the current line survives in place (terminal +-- semantics), where the byte-counted overwrite wrote `X\n` INTO the +-- line, splitting it and leaving the remainder as a ghost line the +-- parser saw again at EOF. +-- +-- UTF-8 safety (round-3 finding 1) is per-edit: every segment holds +-- complete scalars (the parser never splits one, and \n is ASCII) +-- and every consumed range covers whole existing codepoints, so the +-- rope is valid UTF-8 after each step and the byte-native CRDT edit +-- never rejects a range. `out_pos` stays on codepoint boundaries by +-- induction: it moves to end-of-segment, line starts (after \n), or +-- a boundary-aligned backspace target. local function emit_text(slot, text) if #text == 0 then return end local buf = slot.buf - local len = buf:len() - local pos = math.min(slot.out_pos, len) - -- The current line's remainder — the only bytes an overwrite may - -- touch (no \n exists past out_pos). - local tail = buf:slice(pos, len) - local overwrite = math.min(#text, #tail) - -- Align the overwrite end forward to a codepoint boundary of the - -- EXISTING content: replacing 1 byte of a 2-byte é must consume - -- both of its bytes (é and X both occupy one terminal column). - while overwrite < #tail and is_utf8_continuation(tail:byte(overwrite + 1)) do - overwrite = overwrite + 1 + local idx = 1 + while idx <= #text do + local len = buf:len() + local pos = math.min(slot.out_pos, len) + if pos >= len then + -- Append fast path: nothing ahead to overwrite, so the whole + -- remainder (newlines included) lands as one edit. + local rest = text:sub(idx) + buf:insert(len, rest, { bypass_intercept = true }) + slot.out_pos = len + #rest + add_style_span(slot, len, len + #rest) + return + end + local nl = text:find("\n", idx, true) + if nl == idx then + -- Newline while mid-line: cursor to a fresh line; the stale + -- remainder stays. The \n is appended past it, never written + -- over it. + buf:insert(len, "\n", { bypass_intercept = true }) + slot.out_pos = len + 1 + idx = idx + 1 + else + local seg = text:sub(idx, (nl or #text + 1) - 1) + -- The current line's remainder — the only bytes an overwrite + -- may touch. No \n exists at or past out_pos: rewinds stay + -- within the final line and \n is only ever appended. + local tail = buf:slice(pos, len) + local ow = codepoint_prefix_bytes(tail, count_codepoints(seg)) + buf:replace(pos, pos + ow, seg, { bypass_intercept = true }) + slot.out_pos = pos + #seg + add_style_span(slot, pos, pos + #seg) + idx = idx + #seg + end end - buf:replace(pos, pos + overwrite, text, { bypass_intercept = true }) - slot.out_pos = pos + #text - add_style_span(slot, pos, pos + #text) end -- Byte offset where the line containing `out_pos` starts. Scanned diff --git a/docs/compile-mode-framing.md b/docs/compile-mode-framing.md index d04ecd9..f093783 100644 --- a/docs/compile-mode-framing.md +++ b/docs/compile-mode-framing.md @@ -1,7 +1,29 @@ # Compile-mode — framing (Arc 5 stage 1, terminal) -**Revision 9 — 2026-07-13. Status: implemented on branch -`compile-mode` (PR #113); revisions 7–9 fold in PR rounds 1–3.** +**Revision 10 — 2026-07-13. Status: implemented on branch +`compile-mode` (PR #113); revisions 7–10 fold in PR rounds 1–4.** + +Revision 10 (PR #113 round 4, findings 1–2): CR rewrites are +COLUMN-counted and newline-segmented, not byte-counted — each +newline-free segment of a text event consumes one existing codepoint +per incoming codepoint (codepoints approximate columns; double-width +and combining characters count as one, the documented stance), and +LF is not an overwrite column: a newline arriving mid-line drops the +cursor to a fresh line and the stale remainder of the current line +survives in place (terminal semantics). Pre-fix, `abcdef\rX\n` wrote +`X\n` over `ab` — splitting the line and leaving `cdef` as a ghost +line the parser saw again at EOF — and `abc\ré` ate two ASCII +columns because é is two bytes (bites: single-batch, split-feed with +a split codepoint, and a CRDT twin — the segmented renderer makes +several byte-native edits per event, each on codepoint boundaries). +The ANSI parser now tracks the style the consumer LAST RECEIVED +(`emitted_style`): SGR changes inside the alternate screen advance +the internal style while their events are suppressed, so an ordinary +`?1049l` exit resynchronizes the effective style, and `finish()` +balances against the emitted style rather than the internal one — a +suppressed SGR reset inside the alt screen no longer strands the +consumer on stale pre-enter color (consumer-mirror units + a Lua +twin that bites). Revision 9 (PR #113 round 3, findings 1–3): the CR/backspace renderer is UTF-8-safe — overwrites consume WHOLE existing @@ -447,9 +469,16 @@ No protocol change, no frontend change. rather than continuing a pre-EOF escape, staying suppressed, or inheriting stale color. The reset is OBSERVABLE (Revision 9): balancing events — `AlternateScreenExit` for an unclosed enter, a - default `SetStyle` for a non-default running style — let - consumers that mirror parser state unwind from the event stream - alone. Compile-mode calls it once at the terminal event, before + default `SetStyle` — let consumers that mirror parser state + unwind from the event stream alone. The balance point is the + style the consumer LAST RECEIVED, not the internal style + (Revision 10, `emitted_style`): SGR changes inside the alternate + screen advance `current_style` while their events are suppressed, + so a suppressed reset would otherwise strand the consumer on + pre-enter color with nothing to compare unequal. The same field + drives an ordinary `?1049l` exit, which resynchronizes the + effective style whenever suppressed SGR changes drifted it. + Compile-mode calls `finish()` once at the terminal event, before finalizing the pending line. ## Decisions @@ -493,15 +522,25 @@ invoke time, so commands/runtime load order stays irrelevant for it. intra-line treatment so progress bars collapse. Alt-screen suppression comes free. One parser, one running style, one output position — coherent because the child delivers **one merged - stream** (Q#CM3). **UTF-8 safety (Revision 9):** overwrite ranges - are byte-counted but consume whole existing codepoints (the range - end aligns forward past continuation bytes) and each text event is - applied as ONE atomic replace, never split; backspace steps to the - previous codepoint boundary. `out_pos` stays on codepoint - boundaries by induction. A split on either side previously left - malformed bytes on the plain rope, and under CRDT made the - byte-native edit reject — aborting the pump after events_take had - already consumed the batch. + stream** (Q#CM3). **Overwrite semantics (Revision 9→10):** + overwrites are COLUMN-counted and newline-segmented — each + newline-free segment of a text event consumes one existing + codepoint per incoming codepoint (codepoints approximate columns; + double-width and combining characters count as one, the documented + stance), LF is never an overwrite column (a mid-line newline + appends past the surviving stale remainder — terminal semantics — + instead of being written INTO the line, which split it and left + the remainder as a ghost line), and backspace steps to the + previous codepoint boundary. Every buffer edit keeps both range + ends on codepoint boundaries — Revision 9's UTF-8 invariant, held + per-segment now rather than by one atomic replace: segments carry + complete scalars, so the rope is valid UTF-8 after every step and + the byte-native CRDT edit never rejects a range. `out_pos` stays + on codepoint boundaries by induction. History: pre-Revision-9 + splits left malformed bytes and aborted the CRDT pump after + events_take had already consumed the batch; pre-Revision-10 the + byte count split lines (`abcdef\rX\n` → ghost `cdef` line) and ate + columns under multibyte overwrites (`abc\ré` → `éc`). - **External-edit resilience (Revision 4–6).** Generated-buffer keys round-trip, so honest frontend optimistic undo is not an escape from the local bindings; accepted replica ops also fire diff --git a/src/ansi.rs b/src/ansi.rs index 5cb50e3..4455fe7 100644 --- a/src/ansi.rs +++ b/src/ansi.rs @@ -287,6 +287,15 @@ pub struct AnsiParser { /// Current SGR state. Mutated by SGR parameters; emitted as /// [`AnsiEvent::SetStyle`] when it changes. current_style: Style, + /// The style the CONSUMER last received via an emitted + /// `SetStyle` event. Outside alternate-screen this always equals + /// `current_style` (every SGR emits immediately); inside, SGR + /// events are suppressed while `current_style` keeps advancing, + /// so the two drift apart — and alternate-screen exit (ordinary + /// or via [`Self::finish`]) must resynchronize the consumer from + /// this field, not from an internal comparison against default + /// (PR #113 round-4 finding 2). + emitted_style: Style, /// Byte count consumed in the current Ignore state. Reset to /// zero on every entry to an Ignore state: this is a per-state /// budget, not a global counter, so each malformed sequence @@ -339,6 +348,7 @@ impl AnsiParser { Self { state: State::Ground, current_style: Style::default(), + emitted_style: Style::default(), ignore_byte_count: 0, text_run: String::new(), utf8_buf: Vec::new(), @@ -352,7 +362,9 @@ impl AnsiParser { /// Reset the parser to ground state. The running style is *not* /// reset --- callers that want a clean style should pair this - /// with their own `SetStyle(Style::default())`. + /// with their own `SetStyle(Style::default())`. Neither is + /// `emitted_style`: a mid-stream reset changes nothing about + /// what the consumer has already been shown. pub fn reset(&mut self) { self.state = State::Ground; self.ignore_byte_count = 0; @@ -409,9 +421,12 @@ impl AnsiParser { /// round-2 finding 4; round-3 finding 2). The reset is /// OBSERVABLE: consumers that mirror parser state from the event /// stream receive balancing events — an `AlternateScreenExit` - /// for an unclosed enter, a default `SetStyle` when the running - /// style was non-default — so they unwind without out-of-band - /// knowledge. Idempotent once drained. First consumer: + /// for an unclosed enter, a default `SetStyle` whenever the + /// style the consumer LAST RECEIVED was non-default. The + /// comparison is against `emitted_style`, not `current_style`: + /// an SGR reset inside the alternate screen leaves the internal + /// style default while the consumer still shows pre-enter color + /// (round-4 finding 2). Idempotent once drained. First consumer: /// compile-mode's terminal-event path (Q#CM4). pub fn finish(&mut self) -> Vec { let mut events = Vec::new(); @@ -430,10 +445,11 @@ impl AnsiParser { self.alt_screen_active = false; events.push(AnsiEvent::AlternateScreenExit); } - if self.current_style != Style::default() { - self.current_style = Style::default(); + if self.emitted_style != Style::default() { events.push(AnsiEvent::SetStyle(Style::default())); } + self.current_style = Style::default(); + self.emitted_style = Style::default(); self.reset(); events } @@ -523,6 +539,7 @@ impl AnsiParser { if self.alt_screen_active { return; } + self.emitted_style = self.current_style; events.push(AnsiEvent::SetStyle(self.current_style)); } @@ -875,6 +892,15 @@ impl AnsiParser { } else if !set && self.alt_screen_active { self.alt_screen_active = false; events.push(AnsiEvent::AlternateScreenExit); + // SGR changes inside the alternate + // screen advanced `current_style` while + // their events were suppressed; the + // consumer still holds the pre-enter + // style. Resynchronize the effective + // style on exit (round-4 finding 2). + if self.current_style != self.emitted_style { + self.emit_set_style(events); + } } } } @@ -1891,4 +1917,82 @@ mod tests { "finish must reset the running style observably" ); } + + /// Consumer mirror for the round-4 alt-screen style-desync + /// tests: alt flag + last received style, driven purely by + /// emitted events. + fn mirror(evs: &[AnsiEvent], alt: &mut bool, style: &mut Style) { + for ev in evs { + match ev { + AnsiEvent::AlternateScreenEnter => *alt = true, + AnsiEvent::AlternateScreenExit => *alt = false, + AnsiEvent::SetStyle(s) => *style = *s, + _ => {} + } + } + } + + #[test] + fn alt_screen_exit_resyncs_suppressed_style_changes() { + // SGR events are suppressed inside the alternate screen + // while `current_style` keeps advancing; an ordinary exit + // must resynchronize the consumer's effective style (PR #113 + // round-4 finding 2). Both drift directions: a reset the + // consumer never saw, and a color it never saw. + let mut p = AnsiParser::new(); + let mut alt = false; + let mut style = Style::default(); + let evs = p.feed(b"\x1b[31mred\x1b[?1049h\x1b[0m\x1b[?1049l"); + mirror(&evs, &mut alt, &mut style); + assert!(!alt, "exit observed"); + assert_eq!( + style, + Style::default(), + "the suppressed SGR reset must reach the consumer on exit" + ); + + let mut p = AnsiParser::new(); + let mut alt = false; + let mut style = Style::default(); + let evs = p.feed(b"\x1b[?1049h\x1b[31m\x1b[?1049lafter"); + mirror(&evs, &mut alt, &mut style); + assert_ne!( + style, + Style::default(), + "a color set inside the alt screen styles post-exit text" + ); + // No drift → no spurious resync event. + let mut p = AnsiParser::new(); + let evs = p.feed(b"\x1b[?1049h\x1b[?1049l"); + assert!( + !evs.iter().any(|e| matches!(e, AnsiEvent::SetStyle(_))), + "style untouched inside alt screen emits no resync" + ); + } + + #[test] + fn finish_emits_default_style_when_reset_was_suppressed() { + // The round-4 finding-2 finish() scenario: consumer shows + // red from before the alt-screen enter; an SGR reset inside + // makes the INTERNAL style default, so a current_style + // comparison sees nothing to balance — but the consumer is + // still red. finish() must compare against what was last + // EMITTED. + let mut p = AnsiParser::new(); + let mut alt = false; + let mut style = Style::default(); + let evs = p.feed(b"\x1b[31mred\x1b[?1049h\x1b[0m"); + mirror(&evs, &mut alt, &mut style); + assert!(alt, "enter observed"); + assert_ne!(style, Style::default(), "consumer is red pre-finish"); + let evs = p.finish(); + mirror(&evs, &mut alt, &mut style); + assert!(!alt, "finish balances the enter"); + assert_eq!( + style, + Style::default(), + "finish must emit the default SetStyle the consumer needs \ + even though the internal style is already default" + ); + } } diff --git a/tests/compile_mode_acceptance.rs b/tests/compile_mode_acceptance.rs index ddec894..2cb689c 100644 --- a/tests/compile_mode_acceptance.rs +++ b/tests/compile_mode_acceptance.rs @@ -2085,3 +2085,130 @@ fn r3f3_spec_fields_are_raw_reads_metatables_not_honored() { ", ); } + +// --------------------------------------------------------------------------- +// PR #113 round 4 — bite tests +// --------------------------------------------------------------------------- + +#[test] +fn r4f1_cr_rewrites_are_column_based_not_byte_based() { + // PR #113 round-4 finding 1: the round-3 renderer preserved + // UTF-8 validity but still counted the overwrite in BYTES. + // `abcdef\rX\n` wrote "X\n" over "ab", splitting the line and + // leaving "cdef" as a ghost line the parser saw again at EOF; + // `abc\ré` overwrote TWO ASCII characters because é is two + // bytes. Overwrites are column-counted (codepoints), and LF is + // not an overwrite column: the newline drops to a fresh line and + // the stale remainder survives in place (terminal semantics). + // CRLF (a CR event followed by a text event starting "\n") is + // the same rule and used to corrupt the same way. + let dir = tempfile::tempdir().unwrap(); + let script = write_script( + dir.path(), + "cols.sh", + concat!( + "printf 'abcdef\\rX\\n'\n", // → Xbcdef + "printf 'abc\\r\\303\\251\\n'\n", // → ébc + "printf 'one\\r\\ntwo\\r\\n'\n", // CRLF → one / two + ), + ); + let mut s = editor(); + compile_and_finish(&mut s, &format!("sh {script}"), dir.path()); + let text = compilation_text(&s); + assert!( + text.contains("\nXbcdef\n\u{e9}bc\none\ntwo\n"), + "shorter rewrites keep the line whole (no ghost line), a \ + multibyte overwrite consumes one COLUMN, and CRLF is a \ + plain line break; buffer:\n{text:?}" + ); + assert!( + text.contains("[compile exited with code 0]"), + "run completes: {text}" + ); + assert!( + errors_buffer(&s).is_empty(), + "no error spam: {}", + errors_buffer(&s) + ); +} + +#[test] +fn r4f1_split_feed_rewrites_and_split_codepoints() { + // The same rewrites with the CR, the overwrite text, and even + // the é's two bytes arriving in SEPARATE pump batches: the + // buffer-scanned line start and the parser's cross-feed UTF-8 + // buffer must compose with the column-based overwrite. + let dir = tempfile::tempdir().unwrap(); + let script = write_script( + dir.path(), + "split.sh", + concat!( + "printf 'abcdef'\n", + "sleep 0.3\n", + "printf '\\rX\\n'\n", + "printf 'abc\\r\\303'\n", // é's lead byte ends the batch + "sleep 0.3\n", + "printf '\\251\\n'\n", + ), + ); + let mut s = editor(); + compile_and_finish(&mut s, &format!("sh {script}"), dir.path()); + let text = compilation_text(&s); + assert!( + text.contains("\nXbcdef\n\u{e9}bc\n"), + "cross-batch rewrites must match the single-batch results; \ + buffer:\n{text:?}" + ); + assert!( + errors_buffer(&s).is_empty(), + "no error spam: {}", + errors_buffer(&s) + ); +} + +#[test] +fn r4f2_alt_screen_style_desync_resynced_on_exit_and_finish() { + // Round-4 finding 2, Lua twin (the in-crate Rust units vanish + // with an ansi.rs swap; this one bites): a consumer mirroring + // style from the event stream must be resynchronized when SGR + // changes were suppressed inside the alternate screen — on + // ordinary exit AND at finish(). The finish() half is the + // internal-comparison trap: the suppressed reset makes the + // PARSER's style default, so only the emitted-style comparison + // sees anything to balance. + let s = editor(); + let (exit_resynced, finish_resynced): (bool, bool) = eval( + &s, + r#" + local function last_style(evs, style) + for _, ev in ipairs(evs) do + if ev.kind == "set_style" then style = ev.style end + end + return style + end + local function is_default(style) + return style.fg == "default" and style.bg == "default" + and not style.bold + end + -- Ordinary exit: red before enter, SGR reset inside + -- (suppressed), explicit CSI ?1049l. + local p = pmacs.ansi.parser() + local evs = p:feed("\27[31mred\27[?1049h\27[0m\27[?1049l") + local a = is_default(last_style(evs, {})) + -- finish(): the same drift, closed by stream end instead. + local q = pmacs.ansi.parser() + local style = last_style(q:feed("\27[31mred\27[?1049h\27[0m"), {}) + local b = is_default(last_style(q:finish(), style)) + return a, b + "#, + ); + assert!( + exit_resynced, + "ordinary alt-screen exit must resync the suppressed SGR reset" + ); + assert!( + finish_resynced, + "finish must emit the default SetStyle the CONSUMER needs even \ + though the internal style is already default" + ); +} diff --git a/tests/compile_mode_crdt_acceptance.rs b/tests/compile_mode_crdt_acceptance.rs index 9c0d106..ba79d49 100644 --- a/tests/compile_mode_crdt_acceptance.rs +++ b/tests/compile_mode_crdt_acceptance.rs @@ -302,3 +302,55 @@ fn r3f1_unicode_cr_backspace_survive_crdt_replication() { "whole-codepoint overwrites replicate as valid UTF-8; got:\n{src_text:?}" ); } + +#[test] +fn r4f1_column_rewrites_replicate_and_converge() { + // PR #113 round-4 finding 1, CRDT twin: the column-based + // renderer makes SEVERAL byte-native edits per text event + // (segment overwrites plus appended newlines instead of one + // atomic replace); every edit must land on codepoint boundaries + // and the full run — including the shorter rewrite whose stale + // remainder survives in place and the multibyte-over-ASCII + // overwrite — must converge byte-identically. Pre-fix the + // byte-counted overwrite replicates the corrupted structure + // (ghost line, eaten column) to both replicas. + let dir = tempfile::tempdir().expect("tempdir"); + let script = dir.path().join("cols.sh"); + std::fs::write( + &script, + "printf 'abcdef\\rX\\n'\nprintf 'abc\\r\\303\\251\\n'\n", + ) + .unwrap(); + let init = format!( + r#" + pmacs.command.define {{ + name = "test.compile-columns", + description = "round-4 column-rewrite fixture trigger", + fn = function() + pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + end, + }} + pmacs.keymap.bind {{ scope = "global", sequence = "C-c 7", command = "test.compile-columns" }} + "#, + script = script.display(), + dir = dir.path().display(), + ); + let daemon = TestDaemon::spawn_with_config(&init); + let mut source = attach_replica(&daemon); + let mut observer = attach_replica(&daemon); + + send_key(&mut source, Key::Char('c'), Modifiers::CTRL); + send_key(&mut source, Key::Char('7'), Modifiers::NONE); + + adopt_next_buffer(&mut source, "source"); + adopt_next_buffer(&mut observer, "observer"); + + let done = |t: &str| t.contains("[compile exited with code 0]"); + let src_text = pump_until_text(&mut source, Duration::from_secs(15), "source run", done); + let obs_text = pump_until_text(&mut observer, Duration::from_secs(15), "observer run", done); + assert_eq!(src_text, obs_text, "byte-identical convergence"); + assert!( + src_text.contains("\nXbcdef\n\u{e9}bc\n"), + "column-based rewrites replicate intact; got:\n{src_text:?}" + ); +}