From c32eadba8d146211d255dae5c3efe677bc6d5960 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 9 Jul 2026 22:56:44 -0400 Subject: [PATCH] feat(edit): comment/uncomment toggle on M-; (Arc 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New builtin/runtime/comment.lua: `edit.toggle-comment` comments or uncomments the current line — or every line the region touches — using the language's line prefix from the public, user-extensible `pmacs.comment.strings` table (Q#CT3; block comments deferred). Language detection reuses lsp.lua's grammar+filetypes chain, now exported as `pmacs.lsp.active_buffer_language()` (the only lsp.lua touch — one assignment). Semantics (Q#CT4): uncomment iff every non-blank line already starts (after its indentation) with the prefix, stripping the prefix plus one padding space; otherwise comment, inserting `prefix .. " "` at the minimum indentation of the span's non-blank lines (Emacs comment-region alignment). Blank lines are skipped in both directions and don't feed the min-indent; an all-blank span is a status no-op. Mixed spans comment — the double prefix round-trips, preserving inner commented-out code. The whole toggle is ONE buf:replace (Q#CT5): one undo step (no undo grouping exists — N per-line edits would need N undos), one CRDT op, and one effective-edit verification with the killring intercept discipline (pcall'd; a rejection reports rather than throws; any post-intercept deviation reports and skips the cursor fix-up). No-region M-; is Emacs `comment-line`, not `comment-dwim`: toggle, then move to the next line so repeated M-; walks a block (named deviation; DWIM's append-at-EOL can come later under its own name). Region toggles clear the selection and land at the span start. The command boundary substrate provides chain-break and after-edit for free (Q#CT6) — asserted anyway. Tests (comment_toggle_acceptance, 14): rust/lua/python prefixes and exact round-trips; cursor-next-line incl. the no-trailing-newline clamp; region min-indent alignment + blank-line skip + selection clear; mixed-span round-trip; region ending at column 0 excludes that line; unknown-language and pathless-scratch no-ops; ONE undo restores a multi-line toggle; rejecting/transforming intercepts (cursor fix-up skipped); after-edit exactly once on both keybound and M-x paths; C-k, M-;, C-k breaks the kill chain. Fixture editors empty pmacs.lsp.config so .rs/.py files never spawn real servers. Gates: fmt; workspace clippy -D warnings; lib 1500; crdt 1672; comment 14; killring 30; cua 5; completion 9; autosave 29; m4 100 (--skip basedpyright); GPU 58 (PMACS_REQUIRE_GPU=1); full workspace sweep clean; git diff --check clean. Framing: docs/comment-toggle-framing.md. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MtRqijWecEzTjPt1B4Nrt5 --- builtin/runtime/comment.lua | 208 ++++++++++++++++ builtin/runtime/lsp.lua | 3 + src/editor.rs | 6 + tests/comment_toggle_acceptance.rs | 368 +++++++++++++++++++++++++++++ 4 files changed, 585 insertions(+) create mode 100644 builtin/runtime/comment.lua create mode 100644 tests/comment_toggle_acceptance.rs diff --git a/builtin/runtime/comment.lua b/builtin/runtime/comment.lua new file mode 100644 index 0000000..7ee91e8 --- /dev/null +++ b/builtin/runtime/comment.lua @@ -0,0 +1,208 @@ +-- comment.lua --- language-aware comment/uncomment (Arc 2). +-- +-- `M-;` (`edit.toggle-comment`) comments or uncomments the current +-- line — or every line the region touches — using the language's line +-- prefix from the public `pmacs.comment.strings` table. Semantics +-- (Q#CT4): uncomment iff every non-blank line already starts (after +-- its indentation) with the prefix; otherwise comment, inserting +-- `prefix .. " "` at the minimum indentation of the span's non-blank +-- lines (Emacs comment-region alignment). Blank lines are skipped in +-- both directions. The whole toggle is ONE `buf:replace` (Q#CT5): one +-- undo step, one CRDT op, one effective-edit verification. +-- +-- Named deviation (Q#CT2): the no-region case is Emacs `comment-line` +-- (toggle, then move to the next line so repeated `M-;` walks a +-- block), not `comment-dwim`'s append-comment-at-EOL. +-- +-- Framing: docs/comment-toggle-framing.md. + +pmacs.comment = pmacs.comment or {} + +local ed = pmacs.editor + +-- Language → line-comment prefix (Q#CT3). Public and user-extensible, +-- like `pmacs.lsp.filetypes`: `pmacs.comment.strings.mylang = ";;"`. +-- Block comments are a named deferral. +pmacs.comment.strings = { + rust = "//", + c = "//", + cpp = "//", + go = "//", + zig = "//", + javascript = "//", + typescript = "//", + javascriptreact = "//", + typescriptreact = "//", + lua = "--", + python = "#", + bash = "#", + sh = "#", + toml = "#", + yaml = "#", +} + +-- Start of the line containing `pos`: chunked backward scan for the +-- last newline strictly before it (same chunk discipline as +-- killring's forward scan — giant lines stay safe). +local function line_start_before(buf, pos) + local p = pos + while p > 0 do + local from = math.max(0, p - 4096) + local chunk = buf:slice(from, p) + local nl = chunk:match("()\n[^\n]*$") + if nl then return from + nl end + p = from + end + return 0 +end + +-- Byte offset of the first newline at or after `pos`, or `len`. +local function line_end_at(buf, pos, len) + local p = pos + while p < len do + local chunk_to = math.min(p + 4096, len) + local chunk = buf:slice(p, chunk_to) + local nl = chunk:find("\n", 1, true) + if nl then return p + nl - 1 end + p = chunk_to + end + return len +end + +-- Split span text (no trailing newline) into lines, preserving empties. +local function split_lines(text) + local lines = {} + local i = 1 + while true do + local nl = text:find("\n", i, true) + if not nl then + table.insert(lines, text:sub(i)) + break + end + table.insert(lines, text:sub(i, nl - 1)) + i = nl + 1 + end + return lines +end + +local function is_blank(line) + return line:match("^%s*$") ~= nil +end + +-- Leading indentation in BYTES. `[ \t]` rather than `%s` so a CR on a +-- CRLF line never counts as indent. +local function indent_of(line) + return line:match("^[ \t]*") +end + +-- edit.toggle-comment body. +function pmacs.comment.toggle() + local buf = pmacs.window.buffer() + if not buf then + ed.set_status("no buffer") + return false + end + local lang = pmacs.lsp.active_buffer_language() + local prefix = lang and pmacs.comment.strings[lang] + if not prefix then + ed.set_status("no comment syntax known for " .. (lang or "this buffer")) + return false + end + + local len = buf:len() + local region = ed.region() + local has_region = region ~= nil and region["end"] > region.start + local span_first, span_last_end + if has_region then + span_first = line_start_before(buf, region.start) + -- The last line the region TOUCHES: a region ending at column 0 + -- stops at the previous line (Emacs comment-region), hence end-1. + span_last_end = line_end_at(buf, region["end"] - 1, len) + else + local cursor = ed.cursor() + span_first = line_start_before(buf, cursor) + span_last_end = line_end_at(buf, cursor, len) + end + + local lines = split_lines(buf:slice(span_first, span_last_end)) + + -- Classify (Q#CT4): uncomment iff every non-blank line is commented; + -- blank lines neither count nor contribute to the min indent. + local any_nonblank = false + local all_commented = true + local min_indent = nil + for _, line in ipairs(lines) do + if not is_blank(line) then + any_nonblank = true + local ind = indent_of(line) + if line:sub(#ind + 1, #ind + #prefix) ~= prefix then + all_commented = false + end + if min_indent == nil or #ind < min_indent then min_indent = #ind end + end + end + if not any_nonblank then + ed.set_status("nothing to comment") + return false + end + + for i, line in ipairs(lines) do + if not is_blank(line) then + if all_commented then + local ind = indent_of(line) + local rest = line:sub(#ind + 1 + #prefix) + if rest:sub(1, 1) == " " then rest = rest:sub(2) end + lines[i] = ind .. rest + else + lines[i] = line:sub(1, min_indent) + .. prefix + .. " " + .. line:sub(min_indent + 1) + end + end + end + local new_text = table.concat(lines, "\n") + + -- One replace = one undo step, one CRDT op (Q#CT5). Same intercept + -- discipline as killring: a rejection reports rather than throws, + -- and any deviation of the EFFECTIVE edit from the request means an + -- intercept rewrote it — the interceptor's result stands and the + -- cursor fix-up is skipped (a moved span makes it meaningless). + local ok, estart, estop, einserted = pcall(function() + return buf:replace(span_first, span_last_end, new_text) + end) + if not ok then + ed.set_status("comment toggle rejected by buffer intercept") + return false + end + if estart ~= span_first or estop ~= span_last_end or einserted ~= #new_text then + ed.set_status("comment toggle altered by buffer intercept") + return false + end + + if has_region then + -- CUA convention after a region op: selection off, cursor at the + -- span start (Q#CT2). + ed.clear_selection() + ed.goto_byte(span_first) + else + -- comment-line behavior: move to the next line so repeated M-; + -- walks down the block. The byte after the rewritten span is the + -- old trailing newline iff one existed. + local new_end = span_first + #new_text + if new_end < buf:len() then + ed.goto_byte(new_end + 1) + else + ed.goto_byte(new_end) + end + end + return true +end + +pmacs.command.define { + name = "edit.toggle-comment", + description = "Comment or uncomment the current line or selected lines.", + fn = function() pmacs.comment.toggle() end, +} + +pmacs.keymap.bind { scope = "global", sequence = "M-;", command = "edit.toggle-comment" } diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index 70b520e..b4f8474 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -344,6 +344,9 @@ local function active_buffer_language() local ext = path:match("%.([%w_]+)$") return ext and pmacs.lsp.filetypes[ext] or nil 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 -- Directory component of a path, or nil if it has none. local function dir_of(path) diff --git a/src/editor.rs b/src/editor.rs index b9bdf03..98ae44b 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -333,6 +333,12 @@ impl EditorState { include_str!("../builtin/runtime/killring.lua"), ) .expect("load killring builtin chunk"); + lua_host + .eval( + Some("@pmacs/builtin/runtime/comment.lua"), + include_str!("../builtin/runtime/comment.lua"), + ) + .expect("load comment builtin chunk"); // T M7.11 bundled-package bootstrap. Through M7.10 the REPL // was loaded directly via `eval(include_str!(...))`; the // M7.11 deliverable migrates it to the package system so it diff --git a/tests/comment_toggle_acceptance.rs b/tests/comment_toggle_acceptance.rs new file mode 100644 index 0000000..0e00f21 --- /dev/null +++ b/tests/comment_toggle_acceptance.rs @@ -0,0 +1,368 @@ +//! Comment-toggle acceptance (Arc 2, docs/comment-toggle-framing.md). +//! +//! Dispatch-driven: `M-;` through `dispatch_key`, `M-x` through the +//! real minibuffer. Buffers are file-backed (language detection needs +//! a path); each editor gets a private tempdir `StateDir` and an +//! emptied `pmacs.lsp.config` so opening `.rs`/`.py` fixtures never +//! spawns a real language server. + +use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; +use pmacs::editor::EditorState; +use pmacs::lua_bindings::StateDir; +use pmacs::protocol::FrontendId; +use std::path::PathBuf; +use std::sync::atomic::{AtomicUsize, Ordering}; + +fn fresh_state_dir() -> PathBuf { + static SEQ: AtomicUsize = AtomicUsize::new(0); + let dir = std::env::temp_dir().join(format!( + "pmacs-comment-{}-{}", + std::process::id(), + SEQ.fetch_add(1, Ordering::Relaxed) + )); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +fn editor(state_dir: &std::path::Path) -> EditorState { + let s = EditorState::new(); + s.lua_host.lua().remove_app_data::(); + s.lua_host + .lua() + .set_app_data(StateDir(state_dir.to_path_buf())); + // Language DETECTION must work (filetypes/grammars); server + // SPAWNING must not (rust/python have default configs). + exec(&s, "pmacs.lsp.config = {}"); + s +} + +fn write_file(dir: &std::path::Path, name: &str, body: &str) -> String { + let p = dir.join(name); + std::fs::write(&p, body).unwrap(); + p.display().to_string() +} + +fn key(code: KeyCode, mods: KeyModifiers) -> KeyEvent { + KeyEvent { + code, + modifiers: mods, + kind: KeyEventKind::Press, + state: KeyEventState::NONE, + } +} + +fn ctrl(s: &mut EditorState, c: char) { + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char(c), KeyModifiers::CONTROL), + ); +} + +fn alt(s: &mut EditorState, c: char) { + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Char(c), KeyModifiers::ALT)); +} + +fn press(s: &mut EditorState, code: KeyCode) { + s.dispatch_key(FrontendId::LOCAL, key(code, KeyModifiers::NONE)); +} + +fn type_str(s: &mut EditorState, text: &str) { + for ch in text.chars() { + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char(ch), KeyModifiers::NONE), + ); + } +} + +fn m_x(s: &mut EditorState, name: &str) { + alt(s, 'x'); + type_str(s, name); + press(s, KeyCode::Enter); +} + +fn exec(s: &EditorState, src: &str) { + s.lua_host.lua().load(src.to_string()).exec().unwrap(); +} + +fn eval(s: &EditorState, src: &str) -> T { + s.lua_host.lua().load(src.to_string()).eval().unwrap() +} + +fn buffer_text(s: &EditorState) -> String { + let b: mlua::String = eval( + s, + "local b = pmacs.window.buffer(); return b:slice(0, b:len())", + ); + String::from_utf8_lossy(&b.as_bytes()).into_owned() +} + +fn cursor(s: &EditorState) -> i64 { + eval(s, "return pmacs.editor.cursor()") +} + +fn status(s: &EditorState) -> String { + s.core.borrow().status.clone() +} + +/// Fresh editor visiting `name` (created in the state tempdir) with +/// `body` on disk, cursor at 0. +fn editor_visiting(name: &str, body: &str) -> EditorState { + let dir = fresh_state_dir(); + let s = editor(&dir); + let f = write_file(&dir, name, body); + exec(&s, &format!("pmacs.buffer.find_or_open({f:?})")); + exec(&s, "pmacs.editor.goto_byte(0)"); + s +} + +// --------------------------------------------------------------------------- +// Single-line toggle (comment-line behavior, Q#CT2) +// --------------------------------------------------------------------------- + +#[test] +fn rust_line_toggle_round_trips_and_cursor_walks_to_the_next_line() { + let mut s = editor_visiting("t.rs", "fn main() {\n let x = 1;\n}\n"); + exec(&s, "pmacs.editor.goto_byte(16)"); // inside " let x = 1;" + alt(&mut s, ';'); + assert_eq!(buffer_text(&s), "fn main() {\n // let x = 1;\n}\n"); + assert_eq!(cursor(&s), 30, "cursor moved to the next line's start"); + // Toggle back from anywhere in the commented line: exact round + // trip, including the padding space. + exec(&s, "pmacs.editor.goto_byte(14)"); + alt(&mut s, ';'); + assert_eq!(buffer_text(&s), "fn main() {\n let x = 1;\n}\n"); + assert_eq!(cursor(&s), 27); +} + +#[test] +fn lua_buffer_gets_the_dash_dash_prefix() { + let mut s = editor_visiting("t.lua", "local x = 1\nreturn x\n"); + alt(&mut s, ';'); + assert_eq!(buffer_text(&s), "-- local x = 1\nreturn x\n"); + assert_eq!(cursor(&s), 15); +} + +#[test] +fn last_line_without_newline_toggles_and_clamps_the_cursor() { + let mut s = editor_visiting("e.py", "x = 1"); + alt(&mut s, ';'); + assert_eq!(buffer_text(&s), "# x = 1"); + assert_eq!(cursor(&s), 7, "no next line: cursor clamps to buffer end"); + alt(&mut s, ';'); + assert_eq!(buffer_text(&s), "x = 1"); +} + +#[test] +fn a_blank_line_is_a_noop_with_a_status() { + let mut s = editor_visiting("b.py", "\n \n"); + alt(&mut s, ';'); + assert!( + status(&s).contains("nothing to comment"), + "got: {:?}", + status(&s) + ); + assert_eq!(buffer_text(&s), "\n \n"); +} + +// --------------------------------------------------------------------------- +// Region toggles (Q#CT4) +// --------------------------------------------------------------------------- + +#[test] +fn region_comments_at_min_indent_skips_blanks_and_clears_the_selection() { + let mut s = editor_visiting("t.py", " two\nzero\n\n four\n"); + exec( + &s, + "pmacs.editor.begin_selection(0); pmacs.editor.goto_byte(20)", + ); + alt(&mut s, ';'); + // Min indent across non-blank lines is 0 (line "zero"), so every + // prefix lands at column 0; the blank line is untouched. + assert_eq!(buffer_text(&s), "# two\n# zero\n\n# four\n"); + let region_active: bool = eval(&s, "return pmacs.editor.region() ~= nil"); + assert!(!region_active, "selection clears after a region toggle"); + assert_eq!(cursor(&s), 0, "cursor lands at the span start"); +} + +#[test] +fn mixed_region_comments_preserving_inner_prefixes_and_round_trips() { + let mut s = editor_visiting("t2.py", "# a\nb\n"); + exec( + &s, + "pmacs.editor.begin_selection(0); pmacs.editor.goto_byte(5)", + ); + alt(&mut s, ';'); + // Mixed span COMMENTS (Q#CT4): the already-commented line gets a + // second prefix, preserving the inner commented-out code. + assert_eq!(buffer_text(&s), "# # a\n# b\n"); + exec( + &s, + "pmacs.editor.begin_selection(0); pmacs.editor.goto_byte(9)", + ); + alt(&mut s, ';'); + // Now every line is commented → uncomment strips the outer layer. + assert_eq!(buffer_text(&s), "# a\nb\n", "double-prefix round-trips"); +} + +#[test] +fn region_ending_at_column_zero_excludes_that_line() { + let mut s = editor_visiting("c.py", "one\ntwo\n"); + exec( + &s, + "pmacs.editor.begin_selection(0); pmacs.editor.goto_byte(4)", + ); + alt(&mut s, ';'); + assert_eq!( + buffer_text(&s), + "# one\ntwo\n", + "a region stopping at a line's column 0 does not touch that line" + ); +} + +// --------------------------------------------------------------------------- +// Unknown language (Q#CT3) +// --------------------------------------------------------------------------- + +#[test] +fn unknown_language_reports_and_edits_nothing() { + let mut s = editor_visiting("t.txt", "hello\n"); + alt(&mut s, ';'); + assert!( + status(&s).contains("no comment syntax known"), + "got: {:?}", + status(&s) + ); + assert_eq!(buffer_text(&s), "hello\n"); +} + +#[test] +fn pathless_scratch_buffer_reports_and_edits_nothing() { + let dir = fresh_state_dir(); + let mut s = editor(&dir); + type_str(&mut s, "hello"); + exec(&s, "pmacs.editor.goto_byte(0)"); + alt(&mut s, ';'); + assert!( + status(&s).contains("no comment syntax known"), + "got: {:?}", + status(&s) + ); + assert_eq!(buffer_text(&s), "hello"); +} + +// --------------------------------------------------------------------------- +// One edit, one undo step (Q#CT5) +// --------------------------------------------------------------------------- + +#[test] +fn a_multi_line_toggle_is_one_undo_step() { + let mut s = editor_visiting("u.py", "a = 1\nb = 2\n"); + exec( + &s, + "pmacs.editor.begin_selection(0); pmacs.editor.goto_byte(11)", + ); + alt(&mut s, ';'); + assert_eq!(buffer_text(&s), "# a = 1\n# b = 2\n"); + ctrl(&mut s, '/'); // buffer.undo, exactly once + assert_eq!( + buffer_text(&s), + "a = 1\nb = 2\n", + "one undo restores the whole multi-line toggle" + ); +} + +// --------------------------------------------------------------------------- +// Intercept discipline (Q#CT5) +// --------------------------------------------------------------------------- + +#[test] +fn rejecting_intercept_reports_without_throwing() { + let mut s = editor_visiting("i.py", "a\nb\n"); + exec( + &s, + r#" + _G.reject_once = true + pmacs.buffer.add_intercept(pmacs.window.buffer(), function(_op) + if _G.reject_once then + _G.reject_once = false + error("rejected by test intercept") + end + return nil + end) + "#, + ); + alt(&mut s, ';'); // rejected: reported, nothing changed, no throw + assert!(status(&s).contains("rejected"), "got: {:?}", status(&s)); + assert_eq!(buffer_text(&s), "a\nb\n"); + assert_eq!(cursor(&s), 0, "no cursor fix-up on a rejected toggle"); + alt(&mut s, ';'); // allowed again: the command still works + assert_eq!(buffer_text(&s), "# a\nb\n"); +} + +#[test] +fn transforming_intercept_is_reported_and_skips_the_cursor_fixup() { + let mut s = editor_visiting("j.py", "ab\ncd\n"); + // Enlarges every replace's end by one byte — the effective edit + // deviates from the request, so the toggle must report and leave + // the cursor alone (the span it would fix up toward moved). + exec( + &s, + r#" + pmacs.buffer.add_intercept(pmacs.window.buffer(), function(op) + if op.kind == "replace" then + return { + kind = "replace", + start = op.start, + ["end"] = op["end"] + 1, + bytes = op.bytes, + } + end + return nil + end) + "#, + ); + alt(&mut s, ';'); + assert!(status(&s).contains("altered"), "got: {:?}", status(&s)); + // The interceptor's result stands (accepted post-hoc semantics): + // it swallowed the newline after "ab". + assert_eq!(buffer_text(&s), "# abcd\n"); + assert_eq!(cursor(&s), 0, "cursor fix-up skipped"); +} + +// --------------------------------------------------------------------------- +// Substrate plumbing (Q#CT6) +// --------------------------------------------------------------------------- + +#[test] +fn after_edit_fires_exactly_once_per_toggle_keybound_and_m_x() { + let mut s = editor_visiting("h.py", "a\nb\n"); + exec( + &s, + "_G.ae = 0; pmacs.hook.add('buffer.after-edit', function() _G.ae = _G.ae + 1 end)", + ); + alt(&mut s, ';'); // keybound path + let n: i64 = eval(&s, "return _G.ae"); + assert_eq!(n, 1, "keybound toggle fires after-edit once"); + // Cursor walked to line 2; M-x path must fire it too (via + // invoke_interactive + with_after_edit_check), and the minibuffer + // typing itself must not inflate the count. + m_x(&mut s, "edit.toggle-comment"); + assert_eq!(buffer_text(&s), "# a\n# b\n"); + let n: i64 = eval(&s, "return _G.ae"); + assert_eq!(n, 2, "M-x toggle fires after-edit exactly once more"); +} + +#[test] +fn toggle_between_kills_breaks_the_kill_chain() { + let mut s = editor_visiting("k.rs", "one\ntwo\nthree\n"); + ctrl(&mut s, 'k'); // kills "one"; line now blank, cursor 0 + alt(&mut s, ';'); // no-op on the blank line, but the command ROTATES + ctrl(&mut s, 'k'); // kills "\n" — must push fresh, not append + let ring: Vec = eval(&s, "return pmacs.killring.list()"); + assert_eq!( + ring, + vec!["\n", "one"], + "C-k, M-;, C-k yields two ring entries (chain broken)" + ); +}