From ceaeb81386b4994cb08b05d54f59a306ee03f21c Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sun, 12 Jul 2026 15:54:16 +0100 Subject: [PATCH] =?UTF-8?q?fix(edit):=20PR=20#110=20round=202=20=E2=80=94?= =?UTF-8?q?=20UTF-8=20well-formedness,=20source-buffer=20relevance,=20non-?= =?UTF-8?q?table=20sets?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 1 (medium): pair entries validate full UTF-8 well-formedness (Unicode Table 3-7), not just lead-byte length — continuation-byte shape on every trailing byte, overlong encodings (C0/C1, E0 80-9F, F0 80-8F), UTF-16 surrogates (ED A0-BF), and beyond-U+10FFFF (F5+, F4 90+) all disqualify, so "(\xC2x" can no longer inject invalid bytes as a closer. char_at shares the validator and returns the raw byte for malformed buffer content: the predicate treats junk as word-like (no pairing before it), never as EOL. Bite: malformed_utf8_pair_entries_are_rejected (four ill-formed shapes). Finding 2 (low): relevance and reporting resolve against the SOURCE buffer the record names, not whatever buffer a context-switching command left active. New pmacs.lsp.buffer_language(buf) is the parameterized primitive (active_buffer_language delegates), backed by a new buf:path() query on buffer handles. Bites: rust→python `'` now stays silent; python→rust `'` now reports "source context changed". Finding 3 (low): non-table set containers degrade language→default→empty instead of throwing from the after-edit callback on every keystroke. Bites: a string default pairs nothing with a clean *errors* buffer; a junk language entry falls back to the default set. Framing synced to revision 5 (Q#AP2 well-formedness + container degradation + source-buffer resolution, Q#AP3 predicate junk-byte posture, acceptance list). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VoiEyuPjoBhvwACf8HAnLB --- builtin/runtime/lsp.lua | 16 +++- builtin/runtime/pair.lua | 96 ++++++++++++++------- docs/auto-pairing-framing.md | 49 ++++++++--- src/lua_bindings/mod.rs | 14 ++++ tests/auto_pair_acceptance.rs | 153 ++++++++++++++++++++++++++++++++++ 5 files changed, 284 insertions(+), 44 deletions(-) diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index b4f8474..928ee37 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -333,9 +333,9 @@ local function active_buffer_path() return pmacs.editor.file_path() end -local function active_buffer_language() - local path = active_buffer_path() - if not path then return nil end +local function buffer_language(buf) + local ok, path = pcall(function() return buf and buf:path() end) + if not ok or not path then return nil end -- Grammar-backed detection first (keeps rust/.rs etc. exactly as -- before); fall back to the LSP-only filetype map so languages -- with a server but no tree-sitter grammar (Python) still attach. @@ -344,6 +344,16 @@ local function active_buffer_language() local ext = path:match("%.([%w_]+)$") return ext and pmacs.lsp.filetypes[ext] or nil end +-- Public: the per-buffer language chain. Auto-pairing resolves +-- relevance against the buffer its typed-edit record names — which a +-- context-switching command may have left inactive by callback time — +-- so the parameterized form is the primitive and the active-buffer +-- form delegates. +pmacs.lsp.buffer_language = buffer_language + +local function active_buffer_language() + return buffer_language(pmacs.window.buffer()) +end -- Public: the comment-toggle module (and future language-aware Lua) -- reuses this grammar+filetypes chain instead of replicating it. pmacs.lsp.active_buffer_language = active_buffer_language diff --git a/builtin/runtime/pair.lua b/builtin/runtime/pair.lua index 231e23d..1fe0e4e 100644 --- a/builtin/runtime/pair.lua +++ b/builtin/runtime/pair.lua @@ -50,55 +50,85 @@ pmacs.pair.sets = { bash = { "()", "[]", "{}", '""', "''" }, } --- UTF-8 sequence length from a leading byte; nil on a continuation --- byte (not a codepoint boundary). -local function cp_len(b) - if b < 0x80 then return 1 end - if b < 0xC0 then return nil end - if b < 0xE0 then return 2 end - if b < 0xF0 then return 3 end +-- Length of the well-formed UTF-8 sequence starting at `s[i]`, or nil +-- for anything ill-formed (Unicode 15, Table 3-7): continuation-byte +-- shapes are checked on EVERY trailing byte, and the narrowed +-- second-byte ranges exclude overlong encodings (C0/C1 leads, +-- E0 80–9F, F0 80–8F), UTF-16 surrogates (ED A0–BF), and codepoints +-- beyond U+10FFFF (F5+ leads, F4 90+). Length-from-lead-byte alone +-- accepted "(\xC2x" as two "codepoints" (PR #110 round 2, finding 1). +local function utf8_seq_len(s, i) + local b1 = s:byte(i) + if not b1 then return nil end + if b1 < 0x80 then return 1 end + if b1 < 0xC2 or b1 > 0xF4 then return nil end + local b2 = s:byte(i + 1) + if not b2 or b2 < 0x80 or b2 > 0xBF then return nil end + if b1 < 0xE0 then return 2 end + if b1 == 0xE0 and b2 < 0xA0 then return nil end + if b1 == 0xED and b2 > 0x9F then return nil end + if b1 == 0xF0 and b2 < 0x90 then return nil end + if b1 == 0xF4 and b2 > 0x8F then return nil end + local b3 = s:byte(i + 2) + if not b3 or b3 < 0x80 or b3 > 0xBF then return nil end + if b1 < 0xF0 then return 3 end + local b4 = s:byte(i + 3) + if not b4 or b4 < 0x80 or b4 > 0xBF then return nil end return 4 end --- The first full UTF-8 codepoint starting at byte `pos`, as a string, --- or nil at end-of-buffer / on a non-boundary byte. Forward twin of --- lsp.lua's `char_before`; reads at most 4 bytes. +-- The first full UTF-8 codepoint starting at byte `pos`, as a string; +-- nil at end-of-buffer. Bytes that do not begin a well-formed +-- sequence (malformed file content, a truncated sequence at EOF) +-- yield the single raw byte instead: it matches neither whitespace +-- nor any validated closer, so the predicate conservatively treats +-- junk like a word character — never like EOL, which nil would mean. local function char_at(buf, pos) local len = buf:len() if pos >= len then return nil end local to = math.min(pos + 4, len) local ok, s = pcall(function() return buf:slice(pos, to) end) if not ok or type(s) ~= "string" or #s == 0 then return nil end - local n = cp_len(s:byte(1)) - if not n or n > #s then return nil end + local n = utf8_seq_len(s, 1) + if not n or n > #s then return s:sub(1, 1) end return s:sub(1, n) end --- Split a pair entry into (opener, closer): EXACTLY two codepoints, --- no trailing bytes (PR #110 round 1, finding 3 — "()x" must be --- skipped entirely, never honored as `(` → `)x`). nil for malformed --- user additions: skipped, not errors — the hook must never throw --- over a config typo. +-- Split a pair entry into (opener, closer): EXACTLY two well-formed +-- UTF-8 codepoints, no trailing bytes (PR #110 round 1 finding 3 + +-- round 2 finding 1 — "()x" and "(\xC2x" must be skipped entirely, +-- never partially honored). nil for malformed user additions: +-- skipped, not errors — the hook must never throw over a config typo. local function split_pair(s) if type(s) ~= "string" or #s < 2 then return nil end - local n1 = cp_len(s:byte(1)) + local n1 = utf8_seq_len(s, 1) if not n1 or n1 >= #s then return nil end - local n2 = cp_len(s:byte(n1 + 1)) + local n2 = utf8_seq_len(s, n1 + 1) if not n2 or n1 + n2 ~= #s then return nil end return s:sub(1, n1), s:sub(n1 + 1) end --- The active buffer's pair set: language entry if the language is --- known and configured, else `default`. `pmacs.lsp` is looked up --- lazily and nil-guarded — this chunk loads before lsp.lua (Q#AP7), --- and language detection is an LSP-runtime service. -local function active_set() +-- The pair set for `buf`: its language's entry if configured, else +-- `default`. Language resolves against the buffer the typed-edit +-- record names — NOT the currently active buffer, which a +-- context-switching command may have replaced by callback time +-- (PR #110 round 2, finding 2). `pmacs.lsp` is looked up lazily and +-- nil-guarded — this chunk loads before lsp.lua (Q#AP7). Non-table +-- values anywhere (a config typo like `pmacs.pair.sets.default = +-- "()"`) degrade to the default set, then to empty — never a throw +-- from the after-edit callback (round 2, finding 3). +local function set_for(buf) local lang - if pmacs.lsp and pmacs.lsp.active_buffer_language then - local ok, l = pcall(pmacs.lsp.active_buffer_language) + if pmacs.lsp and pmacs.lsp.buffer_language then + local ok, l = pcall(pmacs.lsp.buffer_language, buf) if ok then lang = l end end - return (lang and pmacs.pair.sets[lang]) or pmacs.pair.sets.default + local sets = pmacs.pair.sets + if type(sets) ~= "table" then return {} end + local set = lang and sets[lang] + if type(set) ~= "table" then set = sets.default end + if type(set) ~= "table" then return {} end + return set end -- opener → closer, and the set of closer codepoints. @@ -172,11 +202,15 @@ pmacs.hook.add("buffer.after-edit", function() if not buf then return end -- Relevance first (PR #110 round 1, finding 2): pairing has no - -- interest in characters outside the active set, so a transformed - -- or relocated ordinary `a` must stay silent — the reports below - -- are for pair characters only. + -- interest in characters outside the set, so a transformed or + -- relocated ordinary `a` must stay silent — the reports below are + -- for pair characters only. The set is the SOURCE buffer's (round + -- 2, finding 2): `'` typed in Rust stays silent even when a + -- context-switching command lands in Python, and `'` typed in + -- Python still draws the context-change report when it lands in + -- Rust. local ch = rec.char - local openers, closers = maps_for(active_set()) + local openers, closers = maps_for(set_for(rec.buffer)) if not (openers[ch] or closers[ch]) then return end -- Fail closed on a transformed source self-insert (Q#AP3): the diff --git a/docs/auto-pairing-framing.md b/docs/auto-pairing-framing.md index 3e4a840..78375e2 100644 --- a/docs/auto-pairing-framing.md +++ b/docs/auto-pairing-framing.md @@ -36,6 +36,17 @@ is scoped as best-effort under the active-buffer edit-epoch limit — an equal-revision context switch skips the fan-out and fails closed silently. +Revision 5: PR #110 round 2 — set entries validate full UTF-8 +well-formedness (Table 3-7: continuation bytes, overlong encodings, +surrogates, beyond-U+10FFFF all disqualify; lead-byte length alone +had accepted `"(\xC2x"`), and the predicate treats malformed buffer +bytes as word-like (no pairing before junk), never as EOL; relevance +and reporting resolve against the SOURCE buffer's language via the +new `pmacs.lsp.buffer_language(buf)` / `buf:path()` (a +context-switching command no longer attributes them to the +destination buffer); and non-table set containers degrade +language→default→empty instead of throwing from the callback. + ## Ground truth (as of `7e127ab`) - **Dispatch is keymap-first for printables** — `Char('(')` resolves @@ -209,10 +220,17 @@ full fix deferred with the pre-existing mixed-history problem `pmacs.pair.sets` — the `pmacs.comment.strings` shape: language → array of pair strings, plus a `default` entry used when the language is unknown or has no entry (pairing is useful in scratch buffers). -An entry is EXACTLY two codepoints — opener then closer, multibyte -allowed (`"«»"`); malformed entries (trailing bytes, non-boundary -first byte) are skipped entirely, never partially honored (R4: a -`"()x"` typo must not turn `(` into `()x`): +An entry is EXACTLY two **well-formed** UTF-8 codepoints — opener +then closer, multibyte allowed (`"«»"`); malformed entries are +skipped entirely, never partially honored (R4: a `"()x"` typo must +not turn `(` into `()x`; R5: well-formedness per Unicode Table 3-7, +so truncated sequences, overlong encodings, surrogates, and +beyond-U+10FFFF closers like `"(\xC2x"` also disqualify). A +non-table container anywhere — a typo like `pmacs.pair.sets.default += "()"` — degrades language→default→empty rather than throwing from +the after-edit callback (R5). The set (and the language behind it) +always resolves against the buffer the typed-edit record names, via +`pmacs.lsp.buffer_language(buf)` (R5): - `default = { "()", "[]", "{}", '""' }` — no `'` (prose apostrophes), no backtick. @@ -252,13 +270,16 @@ React when ALL hold: it; a surviving nonempty region means the edit arrived through the TUI's selection-blind optimistic gate (custom chars only), where reacting would pile a closer onto an unconsumed region; -- the record's exact typed codepoint is an opener in the buffer's pair - set (language via `pmacs.lsp.active_buffer_language()`, resolved - **at callback time**, nil-guarded — pair.lua loads before lsp.lua, - Q#AP7). `char_before` is not input provenance; +- the record's exact typed codepoint is an opener in the SOURCE + buffer's pair set (language via `pmacs.lsp.buffer_language(buf)` on + the record's buffer, resolved **at callback time**, nil-guarded — + pair.lua loads before lsp.lua, Q#AP7; R5: the active buffer is the + wrong buffer whenever a context-switching command ran). + `char_before` is not input provenance; - **conservative predicate**: the char at the cursor is EOL, whitespace, or a closing bracket from the pair set — `foo|bar` + - `(` gives `(bar`, never `()bar`; + `(` gives `(bar`, never `()bar`. Malformed bytes at the cursor are + word-like (no pairing before junk), not EOL-like (R5); - for symmetric pairs (quotes), the skip check (Q#AP4) runs first. Reaction: one pcall'd `buf:insert(cursor, closer)`. Outcomes, @@ -464,7 +485,15 @@ facility is the only way a consumed record outlives its fan-out (R4). default set. - Set-entry parsing (R4): a malformed `"()x"` (and an overlong multibyte `"«»x"`) pairs nothing; a valid multibyte `"«»"` pairs - and skips at byte-correct cursors. + and skips at byte-correct cursors. Ill-formed UTF-8 closers (R5) — + truncated `"(\xC2x"`, overlong `"(\xC0\xAF"`, surrogate + `"(\xED\xA0\x80"`, beyond-U+10FFFF `"(\xF5\x80\x80\x80"` — all + pair nothing. Non-table containers (R5): a string `default` pairs + nothing without erroring; a junk language entry falls back to the + default set. +- Source-buffer relevance (R5): `'` typed in Rust with a + context-switching command landing in Python stays silent; the + inverse Python→Rust route still reports "source context changed". - Non-typed provenance, with the callback actually exercised: production `FrontendEvent::Paste("(")` after a prior self-insert leaves a lone pasted opener; `buf:insert("(")` followed by explicit diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index a72840f..01bf0d2 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -1167,6 +1167,20 @@ fn add_query_methods>(methods: &mut M) { with_registry(lua, |r| Ok(resolve(r, this.0)?.name().to_owned())) }); + // Backing file path, or nil for pathless buffers (scratch, + // generated). The per-buffer twin of `pmacs.editor.file_path()`: + // consumers that hold a buffer handle from earlier in a hook + // fan-out (auto-pairing's typed-edit record) must resolve + // language/URIs against THAT buffer, not whatever is active by + // the time their callback runs. + methods.add_method("path", |lua, this, ()| { + with_registry(lua, |r| { + Ok(resolve(r, this.0)? + .file_path() + .map(|p| p.display().to_string())) + }) + }); + methods.add_method("is_modified", |lua, this, ()| { with_registry(lua, |r| Ok(resolve(r, this.0)?.is_modified())) }); diff --git a/tests/auto_pair_acceptance.rs b/tests/auto_pair_acceptance.rs index 1998b04..09fee39 100644 --- a/tests/auto_pair_acceptance.rs +++ b/tests/auto_pair_acceptance.rs @@ -281,6 +281,34 @@ fn malformed_pair_entries_are_skipped_not_partially_honored() { ); } +#[test] +fn malformed_utf8_pair_entries_are_rejected() { + // PR #110 round 2, finding 1: byte-length-from-lead-byte alone is + // not validation. Every ill-formed closer shape from Unicode + // Table 3-7 must disqualify the entry — never land in the buffer. + let cases = [ + // Truncated 2-byte sequence with a trailing ASCII byte: the + // lead byte "promises" 2 bytes, so lead-length parsing counts + // "\xC2x" as one codepoint. + ("string.char(0xC2) .. \"x\"", "truncated sequence"), + // Overlong encoding of `/` (C0 AF). + ("string.char(0xC0, 0xAF)", "overlong encoding"), + // UTF-16 surrogate D800 (ED A0 80). + ("string.char(0xED, 0xA0, 0x80)", "surrogate encoding"), + // Beyond U+10FFFF (F5 80 80 80). + ("string.char(0xF5, 0x80, 0x80, 0x80)", "beyond U+10FFFF"), + ]; + for (closer, what) in cases { + let mut s = editor_with(""); + exec( + &s, + &format!("pmacs.pair.sets.default = {{ \"(\" .. {closer} }}"), + ); + type_str(&mut s, "("); + assert_eq!(buffer_text(&s), "(", "a {what} closer must pair nothing"); + } +} + #[test] fn multibyte_pair_entries_pair_and_skip() { // Two-codepoint entries with multibyte members are valid: guillemets. @@ -294,6 +322,41 @@ fn multibyte_pair_entries_pair_and_skip() { assert_eq!(cursor(&s), 4); } +#[test] +fn non_table_default_set_fails_closed_without_erroring() { + // PR #110 round 2, finding 3: a config typo assigning a STRING + // where the set table belongs must behave as an empty set — not + // throw from the after-edit callback on every keystroke. + let mut s = editor_with(""); + exec(&s, "pmacs.pair.sets.default = \"()\""); + type_str(&mut s, "("); + assert_eq!(buffer_text(&s), "(", "a non-table set pairs nothing"); + let log = s.lua_host.errors_buffer_text(); + assert!( + !log.contains("pair"), + "the pairing callback must not error over a config typo; *errors*:\n{log}" + ); +} + +#[test] +fn non_table_language_set_falls_back_to_default() { + // The language entry being junk falls back to `default` (the + // buffer still deserves pairing), and nothing throws. + let mut s = editor_visiting("a.rs", ""); + exec(&s, "pmacs.pair.sets.rust = 42"); + type_str(&mut s, "("); + assert_eq!( + buffer_text(&s), + "()", + "a junk language entry falls back to the default set" + ); + let log = s.lua_host.errors_buffer_text(); + assert!( + !log.contains("pair"), + "the pairing callback must not error over a config typo; *errors*:\n{log}" + ); +} + #[test] fn scratch_buffer_pairs_the_default_set() { let mut s = editor_with(""); @@ -735,6 +798,96 @@ fn source_context_switch_fails_closed() { assert_eq!(other_text, "z", "the switched-to buffer is untouched"); } +#[test] +fn context_switch_relevance_is_the_source_buffers_rust_to_python_is_silent() { + // PR #110 round 2, finding 2: `'` typed in Rust is not a pair + // char THERE — that the context-switching command lands in a + // Python buffer (where `''` pairs) must not conjure an irrelevant + // "source context changed" report. Relevance and reporting are + // attributed to the buffer the record names. + let dir = fresh_state_dir(); + let mut s = editor(&dir); + let py = write_file(&dir, "b.py", ""); + let rs = write_file(&dir, "a.rs", ""); + exec(&s, &format!("pmacs.buffer.find_or_open({py:?})")); + exec(&s, "_G.py = pmacs.window.buffer()"); + exec(&s, &format!("pmacs.buffer.find_or_open({rs:?})")); + exec(&s, "_G.rs = pmacs.window.buffer()"); + // Revision skew so the fan-out runs after the switch (the + // buffer-aware edit epoch is a named substrate deferral). + type_str(&mut s, "xy"); + exec( + &s, + r#" + pmacs.command.unregister("buffer.self-insert") + pmacs.command.define { + name = "buffer.self-insert", + description = "test override: insert, then switch context", + fn = function(cp) + pmacs.editor.insert_char_over_region(cp) + pmacs.window.switch_buffer(_G.py) + end, + } + "#, + ); + type_str(&mut s, "'"); + assert!( + !status(&s).contains("auto-pair"), + "`'` is outside the SOURCE (rust) set; got: {:?}", + status(&s) + ); + let rs_text: String = eval(&s, "return _G.rs:slice(0, _G.rs:len())"); + assert_eq!( + rs_text, "xy'", + "the quote landed in the rust buffer, no pair" + ); + let py_text: String = eval(&s, "return _G.py:slice(0, _G.py:len())"); + assert_eq!(py_text, "", "the python buffer is untouched"); +} + +#[test] +fn context_switch_relevance_is_the_source_buffers_python_to_rust_reports() { + // The inverse route: `'` typed in Python IS a pair char there, so + // the context-change report must fire even though the destination + // (rust) set would have suppressed it under active-buffer lookup. + let dir = fresh_state_dir(); + let mut s = editor(&dir); + let rs = write_file(&dir, "a.rs", ""); + let py = write_file(&dir, "b.py", ""); + exec(&s, &format!("pmacs.buffer.find_or_open({rs:?})")); + exec(&s, "_G.rs = pmacs.window.buffer()"); + exec(&s, &format!("pmacs.buffer.find_or_open({py:?})")); + exec(&s, "_G.py = pmacs.window.buffer()"); + type_str(&mut s, "xy"); + exec( + &s, + r#" + pmacs.command.unregister("buffer.self-insert") + pmacs.command.define { + name = "buffer.self-insert", + description = "test override: insert, then switch context", + fn = function(cp) + pmacs.editor.insert_char_over_region(cp) + pmacs.window.switch_buffer(_G.rs) + end, + } + "#, + ); + type_str(&mut s, "'"); + assert!( + status(&s).contains("auto-pair skipped: source context changed"), + "`'` is in the SOURCE (python) set, so the report fires; got: {:?}", + status(&s) + ); + let py_text: String = eval(&s, "return _G.py:slice(0, _G.py:len())"); + assert_eq!( + py_text, "xy'", + "the quote landed in the python buffer, no pair" + ); + let rs_text: String = eval(&s, "return _G.rs:slice(0, _G.rs:len())"); + assert_eq!(rs_text, "", "the rust buffer is untouched"); +} + #[test] fn source_context_switch_with_equal_revisions_fails_closed_silently() { // PR #110 round 1, finding 5: the twin of the test above WITHOUT