From f71055a20656d4d09203bc7c4d6196646dfd8d9d Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 14:54:58 -0400 Subject: [PATCH 1/7] feat(dired): the directory view (Stage 1) Dired is the file surface, not a rider on one: before Stage 0 (#162) pmacs had no way to open a file by path, and browsing is the half a user reaches for when they do not already know the path. Stage 1 ships the view. builtin/runtime/dired.lua: one buffer per directory named by the canonical path (Q#DR2) with an ownership check before any paint (F7); read-only intercept plus round-trip input (Q#DR3); a `dired` major mode carrying mode-scoped keys (Q#DR8) -- RET/f visit, ^ parent, n/p, g revert, q quit, s sort; cursor re-seated by basename across every wholesale repaint (Q#DR9); file visits through `pmacs.window.display_file` and directory descent through dired's own window (Q#DR10); `C-x d` / `C-x C-j`; and `dired.kill-when-opening` through the config registry. Two Rust changes, both narrow: * `read_dir` grows per-entry tolerance behind an opt (Q#DR6). Five per-entry conditions used to fail the entire listing, so a plain refresh of a busy directory could just fail; the module doc's claim that a tolerant wrapper was "the package's job" was false, because the primitive hands Lua one structured error and no partial vec. Per-entry readdir/lstat/readlink failures and non-UTF-8 symlink targets now land in an `errors` channel; parent-level failures and non-UTF-8 *names* stay fatal. The tolerance travels in the settled payload, so the Lua boundary keeps the bare-array shape the frozen M8.2 fixture consumes and never has to look the job back up. The read ops' opts parsing now rejects unknown keys, so a typo'd `tolerant` cannot silently degrade to the fatal contract. * `normalize_buffer_path` is exposed as `pmacs.path.canonicalize` rather than mirrored in Lua. Q#DR2 named exposure the preferred end state; it needs no borrow plumbing, so dired's name-dedup and `display_file`'s `find_buffer_for_path` dedup cannot fork, and the mirror's Stage 2 removal is not owed. tests/dired_acceptance.rs covers framing items 1-16 (22 tests), driven through real key dispatch. Item 17 is the m8_1/m8_2/m8_3 gate. One framing claim is corrected by the substrate: R2-3 expected a dedicated dired panel to carry its dedication across a descent, but `display_buffer` never replaces the buffer in a slot dedicated to another one -- it discards every side-specific parameter and falls back to the document window (Q#BP3 2.iii). Dired does not try to unpin the user's panel; both arms are pinned. --- builtin/runtime/dired.lua | 832 +++++++++++++++++++++ builtin/runtime/fs.lua | 59 +- src/async_runtime.rs | 48 +- src/editor.rs | 11 + src/editor_core.rs | 9 +- src/fs.rs | 304 +++++++- src/lua_bindings/mod.rs | 105 ++- src/workers_buffer.rs | 14 +- tests/dired_acceptance.rs | 1469 +++++++++++++++++++++++++++++++++++++ 9 files changed, 2779 insertions(+), 72 deletions(-) create mode 100644 builtin/runtime/dired.lua create mode 100644 tests/dired_acceptance.rs diff --git a/builtin/runtime/dired.lua b/builtin/runtime/dired.lua new file mode 100644 index 0000000..78ef3b4 --- /dev/null +++ b/builtin/runtime/dired.lua @@ -0,0 +1,832 @@ +-- dired.lua --- the directory view (dired arc Stage 1). +-- +-- Dired is not a convenience rider on an existing file surface: until +-- Stage 0 (`C-x C-f`, #162) there was no way to open a file by path at +-- all, and browsing is the half a user reaches for when they do NOT +-- already know the path. So this is a primary surface, and the one +-- thing it may never do is refuse to render a listing --- hence the +-- per-entry-tolerant `read_dir` opt it drives (Q#DR6), the only Rust +-- this stage needed besides exposing the path normalizer. +-- +-- Framing: docs/dired-framing.md (Q#DR1-DR10). Stage 1 is the view: +-- listing, navigation, sort, revert, quit. Marks and operations are +-- Stage 2; the editable wdired layer is Stage 3. +-- +-- Public surface: +-- +-- pmacs.dired.open(path [, opts]) -- awaits; run inside pmacs.async +-- opts.display = "current" | "panel" (Q#BP11b, default "current") +-- opts.select_name = "" -- seat the cursor on it +-- +-- M-x dired / C-x d -- prompt for a directory +-- M-x dired-jump / C-x C-j -- dired on this file's directory +-- +-- In a dired buffer (mode-scoped keys, Q#DR8): +-- RET, f visit (directory -> descend, file -> display_file) +-- ^ parent directory +-- n / p move by line ( / too) +-- g revert (re-read, preserving the cursor's entry) +-- q quit (restore the previous buffer, or window.quit in a panel) +-- s cycle sort mode (name -> mtime -> size) +-- +-- Three structural decisions worth knowing before editing this file: +-- +-- 1. ONE BUFFER PER DIRECTORY, named `*dired:*` +-- (Q#DR2). Navigation *opens the target's buffer*; it never mutates +-- the current one. That is Emacs behavior, and it is also the only +-- way to keep the name honest --- there is no +-- `pmacs.buffer.set_name`, so the M8.2 fixture's in-place repaint +-- leaves a buffer named after a directory it no longer shows. +-- +-- 2. THE CANONICAL FORM IS THE CORE'S, not a copy of it +-- (`pmacs.path.canonicalize` is `normalize_buffer_path` itself). +-- Dired's name-dedup and `display_file`'s `find_buffer_for_path` +-- dedup have to agree; two implementations that disagree on `//tmp` +-- or a `..` at root would mint two buffers for one directory with no +-- error anywhere. +-- +-- 3. EVERY LISTING IS ASYNC. `pmacs.fs.read_dir` is worker-dispatched, +-- so each command spawns a coroutine and the work after the first +-- `:await()` resumes on a later tick --- outside interactive +-- dispatch. Two consequences: errors must be `pcall`ed and reported +-- here (an uncaught raise inside `pmacs.async` goes to *errors*, not +-- the status line), and `pmacs.window.*` calls made after the await +-- act for the *ambient* active frontend, since interactive origin +-- does not survive the tick boundary. + +-- Emacs 28's dired-kill-when-opening-new-dired-buffer, as a setting +-- rather than a hardcoded policy: buffer-per-directory accumulates +-- buffers when walking a deep tree, and Emacs users differ on whether +-- that is a feature. +pmacs.config.define { + name = "dired.kill-when-opening", + description = "Kill the dired buffer being left when descending or ascending.", + type = "boolean", + default = false, + mutability = "live", +} + +-- --------------------------------------------------------------------------- +-- Layout +-- --------------------------------------------------------------------------- +-- +-- The mark column is column 0 (Q#DR4), so every other column sits two +-- bytes right of the M8.2 fixture's offsets. Stage 1 always renders it +-- blank: filling it in is Stage 2's job, but reserving it now means +-- Stage 2 does not have to move every column, and Stage 3's +-- column-classifying intercept can be written against constants that +-- did not shift under it. Offsets are computed from the widths for the +-- same reason --- the fixture hardcoded `NAME_START = 39` and paid for +-- it in every wdired test. + +local MARK_BYTES = 2 +local KIND_BYTES = 1 +local PERMS_BYTES = 9 +local SIZE_BYTES = 10 +local MTIME_BYTES = 16 + +local MARK_START = 0 +local KIND_START = MARK_START + MARK_BYTES -- 2 +local PERMS_START = KIND_START + KIND_BYTES -- 3 +local PERMS_END = PERMS_START + PERMS_BYTES -- 12 (exclusive) +local SIZE_START = PERMS_END + 1 -- 13 +local MTIME_START = SIZE_START + SIZE_BYTES + 1 -- 24 +local NAME_START = MTIME_START + MTIME_BYTES + 1 -- 41 + +local BLANK_MARK = string.rep(" ", MARK_BYTES) + +local SORT_MODES = { "name", "mtime", "size" } + +-- --------------------------------------------------------------------------- +-- Per-buffer state +-- --------------------------------------------------------------------------- +-- +-- handles: array of { buf, path, entries, errors, sort_mode, prev }. +-- +-- Keyed by linear scan over `BufferIdLua.__eq` rather than by table +-- key: two BufferIdLua values for the same buffer are distinct +-- userdata, so a `handles[buf]` lookup would miss. The scan is over a +-- handful of dired buffers. Dead buffers are compacted out first, so a +-- command in a removed dired buffer sees "not in dired" rather than +-- operating on dead state (the M8.2 fixture's `find_handle` lesson). + +local handles = {} + +local function live_handles() + local live = {} + for _, h in ipairs(handles) do + local ok, valid = pcall(h.buf.is_valid, h.buf) + if ok and valid then live[#live + 1] = h end + end + handles = live + return live +end + +local function handle_for_buffer(buf) + if buf == nil then return nil end + for _, h in ipairs(live_handles()) do + if h.buf == buf then return h end + end + return nil +end + +local function handle_for_path(path) + for _, h in ipairs(live_handles()) do + if h.path == path then return h end + end + return nil +end + +local function active_handle() + return handle_for_buffer(pmacs.window.buffer()) +end + +-- --------------------------------------------------------------------------- +-- Paths and names +-- --------------------------------------------------------------------------- + +local canonicalize = pmacs.path.canonicalize + +local function join_path(dir, name) + if dir:sub(-1) == "/" then return dir .. name end + return dir .. "/" .. name +end + +-- Parent of a canonical directory, through the same normalizer: `..` +-- against the root folds away, so `/` is its own parent and no separate +-- root special case can drift out of agreement with the canonical form. +local function parent_path(path) + return canonicalize(join_path(path, "..")) +end + +local function basename(path) + return path:match("([^/]+)/*$") +end + +local function dirname(path) + local dir = path:match("^(.*)/[^/]*$") + if dir == nil then return nil end + if dir == "" then return "/" end + return dir +end + +local function buffer_name(path) + return "*dired:" .. path .. "*" +end + +local function buffer_named(name) + for _, id in ipairs(pmacs.buffer.list()) do + local ok, described = pcall(pmacs.describe.buffer, id) + if ok and described and described.name == name then return id end + end + return nil +end + +-- The directory a prompt or a jump should start from: the active +-- buffer's own directory, else the process cwd (which the normalizer +-- yields for a bare "." because it absolutizes against it). +local function current_directory() + local buf = pmacs.window.buffer() + if buf ~= nil then + local ok, path = pcall(function() return buf:path() end) + if ok and path then + local dir = dirname(path) + if dir then return canonicalize(dir) end + end + local h = handle_for_buffer(buf) + if h then return h.path end + end + return canonicalize(".") +end + +-- --------------------------------------------------------------------------- +-- Failure reporting +-- --------------------------------------------------------------------------- + +-- `Handle:await()` raises structured tables (R45), so `tostring` on a +-- failure yields "table: 0x...". Every user-visible dired failure goes +-- through here. +local function failure_message(err) + if type(err) == "table" then + return tostring(err.message or err.tag or "error") + end + return tostring(err) +end + +local function report(where, err) + pmacs.editor.set_status(where .. ": " .. failure_message(err)) +end + +-- --------------------------------------------------------------------------- +-- Rendering +-- --------------------------------------------------------------------------- + +-- `rwxr-xr-x`, without the leading kind char (rendered separately so a +-- symlink shows `l` and a directory `d`). Arithmetic rather than bit +-- ops: this file has to run on LuaJIT (5.1) as well as Lua 5.4. +local function fmt_perms(mode) + local function tri(bits) + local r = (bits >= 4) and "r" or "-" + local w = ((bits % 4) >= 2) and "w" or "-" + local x = ((bits % 2) >= 1) and "x" or "-" + return r .. w .. x + end + return tri(math.floor(mode / 64) % 8) + .. tri(math.floor(mode / 8) % 8) + .. tri(mode % 8) +end + +local function kind_char(kind) + if kind == "dir" then return "d" + elseif kind == "symlink" then return "l" + elseif kind == "file" then return "-" + else return "?" -- device, fifo, socket + end +end + +local function fmt_size(n) + return string.format("%" .. SIZE_BYTES .. "d", n) +end + +local function fmt_mtime(secs) + -- Explicit format string, so the width is fixed and the result does + -- not move with LC_TIME. A pre-epoch mtime is legal and `os.date`'s + -- behavior on a negative time is platform-dependent, so a + -- non-conforming result degrades to a fixed-width placeholder rather + -- than shifting every column right of it. + local ok, formatted = pcall(os.date, "%Y-%m-%d %H:%M", secs) + if ok and type(formatted) == "string" and #formatted == MTIME_BYTES then + return formatted + end + return string.rep("?", MTIME_BYTES) +end + +-- POSIX permits any byte but `/` and NUL in a filename, including `\n`. +-- Rendering one verbatim would break the one-line-per-entry invariant +-- that cursor-line -> entry resolution rests on (and that Stage 3's +-- intercept will rest on harder), so control bytes are escaped. The +-- backslash goes first, which is what makes the encoding invertible --- +-- Stage 3 needs the exact inverse so a no-op commit cannot fire a +-- spurious rename. Carried over from the M8.2 fixture as decided +-- design, not re-litigated. +local function escape_displayable(s) + if s == nil then return "" end + s = s:gsub("\\", "\\\\") + s = s:gsub("\n", "\\n") + s = s:gsub("\r", "\\r") + s = s:gsub("\t", "\\t") + -- NUL is deliberately absent from the class: the kernel forbids it in + -- a filename, so the fixture's `%z` (removed from Lua 5.2's pattern + -- syntax) was covering a case that cannot occur. + s = s:gsub("[\1-\8\11\12\14-\31]", function(ch) + return string.format("\\x%02X", string.byte(ch)) + end) + return s +end + +local function render_entry(entry) + local target = "" + if entry.symlink_target then + target = " -> " .. escape_displayable(entry.symlink_target) + elseif entry.kind == "symlink" then + -- A tolerant listing keeps a symlink whose target could not be + -- represented (non-UTF-8) or read; say so rather than rendering a + -- bare `l` line that looks like a complete entry. + target = " -> ?" + end + return string.format( + "%s%s%s %s %s %s%s", + BLANK_MARK, kind_char(entry.kind), fmt_perms(entry.mode), + fmt_size(entry.size), fmt_mtime(entry.mtime), + escape_displayable(entry.name), target) +end + +-- Header (line 0) + one line per entry + the unreadable-count footer. +-- The footer exists because a tolerant listing that silently dropped +-- entries is worse than one that failed: the user has to know the view +-- is incomplete (and Stage 3's wdired refuses to open on one). +local function render_text(handle) + local lines = { handle.path .. ":" } + for _, entry in ipairs(handle.entries) do + lines[#lines + 1] = render_entry(entry) + end + local unreadable = #handle.errors + if unreadable > 0 then + lines[#lines + 1] = string.format("%d entries unreadable", unreadable) + end + return table.concat(lines, "\n") +end + +-- Dired's own writes are the only ones that reach the buffer: the +-- read-only intercept rejects everything else, and this bypasses it. +local function paint(handle) + local text = render_text(handle) + handle.buf:replace(0, handle.buf:len(), text, { bypass_intercept = true }) +end + +-- --------------------------------------------------------------------------- +-- Cursor +-- --------------------------------------------------------------------------- +-- +-- Entry i renders on line i (line 0 is the header), so the entry under +-- the cursor is `entries[cursor_line()]`. + +local function entry_at_cursor(handle) + local line = pmacs.editor.cursor_line() + if line < 1 then return nil end + return handle.entries[line], line +end + +local function index_of_name(handle, name) + if name == nil then return nil end + for i, entry in ipairs(handle.entries) do + if entry.name == name then return i end + end + return nil +end + +-- Re-seat by BASENAME (Q#DR9), falling back to the nearest surviving +-- line. Every repaint is wholesale, so without this a revert, a sort, +-- or any Stage 2 operation would drop the cursor to the header. +local function seat_cursor(handle, name, fallback_line) + local count = #handle.entries + if count == 0 then + pmacs.editor.move_to_line(0) + return + end + local target = index_of_name(handle, name) + if target == nil then + target = math.max(1, math.min(fallback_line or 1, count)) + end + pmacs.editor.move_to_line(target) +end + +-- --------------------------------------------------------------------------- +-- Sorting +-- --------------------------------------------------------------------------- + +local function sort_entries(entries, mode) + if mode == "name" then + table.sort(entries, function(a, b) return a.name < b.name end) + elseif mode == "mtime" then + -- Newest first, name as a stable tiebreak so a directory of + -- same-second files renders deterministically. + table.sort(entries, function(a, b) + if a.mtime ~= b.mtime then return a.mtime > b.mtime end + return a.name < b.name + end) + elseif mode == "size" then + table.sort(entries, function(a, b) + if a.size ~= b.size then return a.size > b.size end + return a.name < b.name + end) + else + error("dired: unknown sort mode: " .. tostring(mode)) + end +end + +local function next_sort_mode(mode) + for i, candidate in ipairs(SORT_MODES) do + if candidate == mode then + return SORT_MODES[(i % #SORT_MODES) + 1] + end + end + return SORT_MODES[1] +end + +-- --------------------------------------------------------------------------- +-- Reading +-- --------------------------------------------------------------------------- + +-- Read and sort one directory without touching editor state, so a +-- failure happens before any side effect is committed (acceptance 15). +-- Must run inside `pmacs.async`. +-- +-- Always tolerant (Q#DR6): a plain refresh of a busy directory must not +-- fail because one child was unlinked between `readdir` and `lstat`. +-- Parent-level failures and non-UTF-8 *names* still raise. +local function read_listing(path, sort_mode) + local listing = pmacs.fs.read_dir(path, { tolerant = true }):await() + local entries = listing.entries + sort_entries(entries, sort_mode) + return entries, listing.errors +end + +-- --------------------------------------------------------------------------- +-- Buffer ownership +-- --------------------------------------------------------------------------- + +local READ_ONLY_LIMIT = 99 + +-- `pmacs.buffer.create` takes any caller-chosen name, so a foreign +-- buffer may already be called `*dired:/tmp*`. Painting into it through +-- `bypass_intercept` would clobber a user's data, so found-by-name is +-- NOT adoption: ownership means "this buffer is in dired's own handle +-- table" (F7). +-- +-- That is deliberately narrower than the framing's "in the handle table +-- OR major_mode == dired": a foreign buffer that also carries the mode +-- is precisely the case the check exists to refuse, and a builtin's +-- handle table cannot be lost the way a reloadable package's can. +local function claim_handle(path) + local existing = handle_for_path(path) + if existing then return existing end + + local name = buffer_name(path) + if buffer_named(name) then + local unique = nil + for i = 2, READ_ONLY_LIMIT do + local candidate = string.format("%s<%d>", name, i) + if buffer_named(candidate) == nil then + unique = candidate + break + end + end + if unique == nil then + error(string.format("dired: %s is taken and no free variant remains", name)) + end + name = unique + end + + local buf = pmacs.buffer.create(name) + -- Read-only by the listview idiom (Q#DR3): every non-bypass edit is + -- rejected, and the intercept lives as long as the buffer. + pmacs.buffer.add_intercept(buf, function() + error(name .. " is read-only") + end) + -- Q#DR3/Q#P6: while this buffer is active a semantic frontend must + -- round-trip keys, or optimistic apply would swallow the single-key + -- bindings (`g` would insert a `g` into a CRDT mirror instead of + -- reverting) and bypass the intercept entirely. + pmacs.buffer.set_round_trip_input(buf, true) + -- Q#DR8: the mode is what carries the keymap, and dired is #129's + -- first consumer of mode-scoped keys outside language detection. + pmacs.buffer.set_major_mode(buf, "dired") + + local handle = { + buf = buf, + path = path, + entries = {}, + errors = {}, + sort_mode = SORT_MODES[1], + prev = nil, + } + handles[#handles + 1] = handle + return handle +end + +-- --------------------------------------------------------------------------- +-- Display +-- --------------------------------------------------------------------------- + +local function drop_handle(handle) + for i, candidate in ipairs(handles) do + if candidate == handle then + table.remove(handles, i) + return + end + end +end + +-- Kill the dired buffer being left, when the user asked for it. +-- Deliberately after the new buffer is displayed: `pmacs.buffer.kill` +-- redirects windows showing the doomed buffer, and doing that first +-- would fight the display we are about to perform. +local function kill_departed(departed, arriving) + if departed == nil or departed == arriving then return end + if not pmacs.config.get("dired.kill-when-opening") then return end + local ok, err = pcall(pmacs.buffer.kill, departed.buf) + if ok then + drop_handle(departed) + else + -- A buffer that could not be killed keeps its handle: dropping it + -- would leave a live dired buffer no command recognizes. + report("dired", err) + end +end + +-- Where a dired buffer goes. +-- +-- A fresh `dired` takes the standard adopter opt (Q#BP11b): omitted or +-- "current" is the raw switch every other adopter defaults to in +-- Stages 1-2, "panel" is the bottom side window. +-- +-- Navigation (`departed ~= nil`) instead reuses the window dired +-- already occupies, which is the opposite routing from a file visit and +-- deliberately so (Q#DR10): the next directory is the same kind of +-- thing as the current one and belongs in the same slot, while a file +-- is not a dired buffer and belongs in the document area. +local function display(handle, opts, departed) + local side = nil + if departed ~= nil then + -- Dired's own window, not the request's: walking a tree in a side + -- window keeps the side window. + local params = pmacs.window.params() + side = params and params.side + elseif opts and opts.display == "panel" then + side = "bottom" + end + if side ~= nil then + -- A side slot DEDICATED to another buffer refuses the replacement + -- and this falls back to the document window (Q#BP3 2.iii). That is + -- both the substrate's documented policy and Emacs's, so dired does + -- not try to unpin the user's panel. + pmacs.window.display(handle.buf, { side = side, select = true }) + else + pmacs.window.switch_buffer(handle.buf) + end +end + +-- --------------------------------------------------------------------------- +-- Public: open a directory +-- --------------------------------------------------------------------------- + +pmacs.dired = pmacs.dired or {} + +local OPEN_OPTS = { display = true, select_name = true } + +-- Open `path`'s dired buffer, replacing `departed` (a handle) in the +-- window it occupies when this is a navigation rather than a fresh +-- open. Awaits, so it must run inside `pmacs.async`; raises on a read +-- failure, having changed nothing. Returns the buffer. +local function open_directory(path, opts, departed) + if type(path) ~= "string" then + error("pmacs.dired.open: path must be a string, got " .. type(path)) + end + opts = opts or {} + -- Validated up front, before the read and before any buffer exists, + -- so a bad opt leaves nothing to roll back (the + -- `parse_adopter_placement` discipline). + for key in pairs(opts) do + if not OPEN_OPTS[key] then + error(string.format("pmacs.dired.open: unknown opts key %q", tostring(key))) + end + end + local wanted = opts.display + if wanted ~= nil and wanted ~= "current" and wanted ~= "panel" then + error(string.format('pmacs.dired.open: unknown display %q (expected "current" or "panel")', + tostring(wanted))) + end + local canonical = canonicalize(path) + + -- Read first: a failure must leave no buffer, no window change, and + -- no handle behind. + local sort_mode = (handle_for_path(canonical) or {}).sort_mode or SORT_MODES[1] + local entries, errors = read_listing(canonical, sort_mode) + + local handle = claim_handle(canonical) + handle.entries = entries + handle.errors = errors + handle.sort_mode = sort_mode + + -- `q` returns to the buffer you came from, never to another dired + -- buffer (which would trap `q` walking back down the tree); on a + -- descent the arriving buffer inherits the departing one's origin. + if departed ~= nil then + handle.prev = departed.prev + else + local active = pmacs.window.buffer() + if active ~= nil and handle_for_buffer(active) == nil then + handle.prev = active + end + end + + paint(handle) + display(handle, opts, departed) + -- Seating happens after the display: `switch_buffer` zeroes the + -- window cursor, so an earlier seat would be discarded. + seat_cursor(handle, opts.select_name, 1) + kill_departed(departed, handle) + return handle.buf +end + +function pmacs.dired.open(path, opts) + return open_directory(path, opts, nil) +end + +-- Every interactive entry point funnels through here: spawn the +-- coroutine the await needs, and turn a failure into a status message +-- rather than an uncaught raise inside `pmacs.async` (which would land +-- in *errors* and leave the user with a silent no-op). +local function open_async(path, opts, departed, where) + pmacs.async(function() + local ok, err = pcall(open_directory, path, opts, departed) + if not ok then report(where or "dired", err) end + end) +end + +-- --------------------------------------------------------------------------- +-- Commands +-- --------------------------------------------------------------------------- + +pmacs.command.define { + name = "dired", + description = "Open a directory listing (dired).", + fn = function() + local root = current_directory() + -- No completion source, deliberately. `source = "files"` would make + -- RET-on-empty open whatever sorts first (the minibuffer selects + -- candidate 0 whenever the list is non-empty, and a selected + -- candidate shadows typed text --- S0-1/S0-4), and RET-on-the- + -- default-directory is exactly the gesture `C-x d` exists for. The + -- field is prefilled instead, which is Emacs's own shape here. + pmacs.minibuffer.read { + prompt = "Dired: ", + initial = root, + history = "dired", + on_accept = function(value) + if value == nil or value == "" then return end + open_async(value, nil, nil, "dired") + end, + } + end, +} + +pmacs.command.define { + name = "dired-jump", + description = "Open dired on the current file's directory, cursor on that file.", + fn = function() + local buf = pmacs.window.buffer() + local path = nil + if buf ~= nil then + local ok, value = pcall(function() return buf:path() end) + if ok then path = value end + end + if path == nil then + pmacs.editor.set_status("dired-jump: this buffer has no file") + return + end + local dir = dirname(path) + if dir == nil then + pmacs.editor.set_status("dired-jump: cannot find the directory of " .. path) + return + end + open_async(dir, { select_name = basename(path) }, nil, "dired-jump") + end, +} + +pmacs.command.define { + name = "dired.visit", + description = "Visit the entry under the cursor (descend a directory, open a file).", + fn = function() + local handle = active_handle() + if handle == nil then return end + local entry = entry_at_cursor(handle) + -- The header and the unreadable-count footer are not entries. + if entry == nil then return end + local target = join_path(handle.path, entry.name) + if entry.kind == "dir" then + open_async(target, nil, handle, "dired") + return + end + if entry.kind == "symlink" then + -- `read_dir`/`stat` are lstat-based, so the only way to learn + -- whether a link points at a directory is to try to list it. A + -- symlinked directory is an ordinary thing to walk into, and the + -- probe costs one syscall on symlink lines only. + pmacs.async(function() + local ok = pcall(function() + return pmacs.fs.read_dir(target, { tolerant = true }):await() + end) + if ok then + local descended, err = pcall(open_directory, target, nil, handle) + if not descended then report("dired", err) end + return + end + local visited, err = pcall(pmacs.window.display_file, target, { select = true }) + if not visited then report("dired", err) end + end) + return + end + -- Q#DR10: `display_file`, never `find_or_open`, which switches the + -- active window in both branches before firing hooks --- in a + -- panel-displayed dired that would replace the panel with the + -- visited file, i.e. the panel swallows itself. + local ok, err = pcall(pmacs.window.display_file, target, { select = true }) + if not ok then report("dired", err) end + end, +} + +pmacs.command.define { + name = "dired.parent", + description = "Open the parent directory.", + fn = function() + local handle = active_handle() + if handle == nil then return end + local parent = parent_path(handle.path) + if parent == handle.path then + pmacs.editor.set_status("dired: already at the filesystem root") + return + end + -- Seat on the directory we came from, the way Emacs's `^` does. + open_async(parent, { select_name = basename(handle.path) }, handle, "dired") + end, +} + +pmacs.command.define { + name = "dired.revert", + description = "Re-read the directory, keeping the cursor on its entry.", + fn = function() + local handle = active_handle() + if handle == nil then return end + local entry, line = entry_at_cursor(handle) + local name = entry and entry.name + pmacs.async(function() + local ok, entries, errors = pcall(read_listing, handle.path, handle.sort_mode) + if not ok then + -- On failure `entries` carries the raised value, not a listing. + report("dired", entries) + return + end + if not handle.buf:is_valid() then return end + handle.entries = entries + handle.errors = errors + paint(handle) + seat_cursor(handle, name, line) + end) + end, +} + +pmacs.command.define { + name = "dired.sort-cycle", + description = "Cycle the sort mode: name -> mtime -> size.", + fn = function() + local handle = active_handle() + if handle == nil then return end + local entry, line = entry_at_cursor(handle) + local name = entry and entry.name + -- A pure reorder of the entries already in hand: sort is a display + -- decision, not a reason to re-read the directory. + handle.sort_mode = next_sort_mode(handle.sort_mode) + sort_entries(handle.entries, handle.sort_mode) + paint(handle) + seat_cursor(handle, name, line) + pmacs.editor.set_status("dired: sorted by " .. handle.sort_mode) + end, +} + +pmacs.command.define { + name = "dired.quit", + description = "Leave dired, restoring the previous buffer.", + fn = function() + local handle = active_handle() + if handle == nil then return end + -- Q#BP11b, matching `listview.quit`: `q` keeps its name and its + -- user-visible behavior, delegating to `window.quit` only when + -- dired really is in a side window. + local params = pmacs.window.params() + if params and params.side and params.quit_action then + pmacs.window.quit() + return + end + local target = handle.prev + if not (target and target:is_valid()) then + target = buffer_named("*scratch*") or pmacs.buffer.create("*scratch*") + end + pmacs.window.switch_buffer(target) + end, +} + +-- --------------------------------------------------------------------------- +-- Keys +-- --------------------------------------------------------------------------- + +-- Global: both sequences are unbound repo-wide, and both are the Emacs +-- defaults. +pmacs.keymap.bind { scope = "global", sequence = "C-x d", command = "dired" } +pmacs.keymap.bind { scope = "global", sequence = "C-x C-j", command = "dired-jump" } + +-- In-buffer keys are MODE-scoped (Q#DR8), bound once here rather than +-- per buffer: a second dired buffer needs no `keymap.bind` of its own, +-- and Stage 3's wdired swap changes the whole keymap with the mode +-- instead of unbinding key by key. +local function bind(sequence, command) + pmacs.keymap.bind { scope = "mode", mode = "dired", sequence = sequence, command = command } +end + +bind("RET", "dired.visit") +bind("f", "dired.visit") +bind("^", "dired.parent") +bind("n", "cursor.down") +bind("", "cursor.down") +bind("p", "cursor.up") +bind("", "cursor.up") +bind("g", "dired.revert") +bind("q", "dired.quit") +bind("s", "dired.sort-cycle") + +-- --------------------------------------------------------------------------- +-- Test seam +-- --------------------------------------------------------------------------- +-- +-- The layout constants, so acceptance can assert column positions +-- without hardcoding the numbers this file computes. +pmacs.dired._layout = { + MARK_START = MARK_START, + KIND_START = KIND_START, + PERMS_START = PERMS_START, + PERMS_END = PERMS_END, + SIZE_START = SIZE_START, + MTIME_START = MTIME_START, + NAME_START = NAME_START, +} diff --git a/builtin/runtime/fs.lua b/builtin/runtime/fs.lua index 02ca064..49c006e 100644 --- a/builtin/runtime/fs.lua +++ b/builtin/runtime/fs.lua @@ -12,7 +12,10 @@ -- `symlink_target` is present only on symlink entries. -- `opts` may contain `supersede = ""` to chain into the M3 -- supersede semantics (a later read_dir under the same key --- cancels the earlier one). +-- cancels the earlier one), and `tolerant = true` to swap the +-- all-or-nothing listing for `{ entries = ..., errors = ... }` +-- (see fs.read_dir's own comment below). Any other key is an +-- error rather than being silently ignored. -- -- Order: entries are returned in *filesystem iteration order*, -- which is whatever the kernel's `readdir` syscall returns. On @@ -69,24 +72,61 @@ end local fs = {} --- Shared opts.supersede extractor; raises on misshapen opts. -local function supersede_key(opts, where) - if opts == nil then return nil end +-- Shared read-op opts parser; raises on misshapen opts. +-- +-- Unknown keys are REJECTED, not ignored. The earlier version read +-- `opts.supersede` and silently dropped everything else, which means a +-- typo'd `tolerant` would degrade to the fatal contract with no signal +-- at all --- exactly the failure the tolerant opt exists to prevent +-- (dired framing §8, minor c). `allowed` is the per-op whitelist. +local function read_opts(opts, where, allowed) + if opts == nil then return nil, false end if type(opts) ~= "table" then error(where .. ": opts must be a table or nil, got " .. type(opts)) end - local k = opts.supersede - if k ~= nil and type(k) ~= "string" then + for key in pairs(opts) do + if not allowed[key] then + local names = {} + for name in pairs(allowed) do names[#names + 1] = name end + table.sort(names) + error(string.format("%s: unknown opts key %q (expected one of: %s)", + where, tostring(key), table.concat(names, ", "))) + end + end + local key = opts.supersede + if key ~= nil and type(key) ~= "string" then error(where .. ": opts.supersede must be a string") end - return k + local tolerant = opts.tolerant + if tolerant ~= nil and type(tolerant) ~= "boolean" then + error(where .. ": opts.tolerant must be a boolean") + end + return key, tolerant == true end +local READ_DIR_OPTS = { supersede = true, tolerant = true } +local STAT_OPTS = { supersede = true } + +-- Two result shapes, chosen by `opts.tolerant` (dired Q#DR6): +-- +-- read_dir(path) -> { , ... } +-- read_dir(path, { tolerant = true }) -> { entries = { , ... }, +-- errors = { { name = ...?, +-- message = ... }, ... } } +-- +-- The bare array is the M8.1 contract and stays exactly as it was, so +-- an existing consumer (the frozen M8.2 dired fixture consumes it with +-- `ipairs`) is unaffected. Under the opt, a per-entry `readdir` / +-- `lstat` / `readlink` failure and a non-UTF-8 symlink *target* become +-- `errors` rows instead of failing the whole listing; a failure on the +-- parent directory, and a non-UTF-8 entry *name*, stay fatal. An +-- `errors` row has no `name` when the entry never materialized. function fs.read_dir(path, opts) if type(path) ~= "string" then error("pmacs.fs.read_dir: path must be a string, got " .. type(path)) end - local id = async_mod._dispatch_fs_read_dir(path, supersede_key(opts, "pmacs.fs.read_dir")) + local key, tolerant = read_opts(opts, "pmacs.fs.read_dir", READ_DIR_OPTS) + local id = async_mod._dispatch_fs_read_dir(path, key, tolerant) return build_handle(id) end @@ -94,7 +134,8 @@ function fs.stat(path, opts) if type(path) ~= "string" then error("pmacs.fs.stat: path must be a string, got " .. type(path)) end - local id = async_mod._dispatch_fs_stat(path, supersede_key(opts, "pmacs.fs.stat")) + local key = read_opts(opts, "pmacs.fs.stat", STAT_OPTS) + local id = async_mod._dispatch_fs_stat(path, key) return build_handle(id) end diff --git a/src/async_runtime.rs b/src/async_runtime.rs index 6bc8292..493d993 100644 --- a/src/async_runtime.rs +++ b/src/async_runtime.rs @@ -71,8 +71,8 @@ use crossbeam::channel as cb_channel; use serde::{Deserialize, Serialize}; use crate::fs::{ - FsDirEntry, FsError, chmod_blocking, read_dir_blocking, remove_blocking, rename_blocking, - stat_blocking, + FsDirEntry, FsDirListing, FsError, ReadDirTolerance, chmod_blocking, read_dir_blocking, + remove_blocking, rename_blocking, stat_blocking, }; use crate::message_bus::{BusEnd, MessageBus, SchemaRegistry}; use crate::syntax::{self as syntax_mod, ParseRequest, ParseTreeBundle}; @@ -220,9 +220,10 @@ enum ReplyKind { /// T M4.1. Parse { duration_ms: u64 }, /// `dispatch_fs_read_dir` completed; payload is the directory - /// listing. The Vec is `Serialize` so it crosses the bus - /// directly --- no side handoff like parse trees need. T M8.1. - ReadDir(Vec), + /// listing. The listing is `Serialize` so it crosses the bus + /// directly --- no side handoff like parse trees need. T M8.1; its + /// per-entry error channel is dired Q#DR6. + ReadDir(FsDirListing), /// `dispatch_fs_stat` completed; payload is the per-path /// metadata. T M8.1. Stat(FsDirEntry), @@ -266,10 +267,11 @@ pub enum JobResult { duration_ms: u64, }, /// `dispatch_fs_read_dir` produced a directory listing. The - /// Lua boundary in [`crate::lua_bindings`] turns the Vec into a - /// per-entry table when `_take_result` consumes the result. - /// T M8.1. - ReadDir(Vec), + /// Lua boundary in [`crate::lua_bindings`] turns the entries into + /// per-entry tables when `_take_result` consumes the result, and + /// keys the result *shape* on whether the listing carries a + /// per-entry error channel. T M8.1 / dired Q#DR6. + ReadDir(FsDirListing), /// `dispatch_fs_stat` produced metadata for a single path. The /// Lua boundary turns the [`FsDirEntry`] into the same table /// shape `read_dir` entries use. T M8.1. @@ -832,11 +834,21 @@ impl AsyncRuntime { /// `lstat`-style metadata. Polls cancel every batch of /// entries; supersede follows the same rule as the other /// dispatchers. T M8.1. - pub fn dispatch_fs_read_dir(&self, path: PathBuf, supersede: Option<&str>) -> JobId { + /// + /// `tolerance` selects the per-entry contract (dired Q#DR6): + /// [`ReadDirTolerance::Fatal`] is the original all-or-nothing + /// listing, [`ReadDirTolerance::PerEntry`] carries per-entry + /// failures alongside the entries that survived. + pub fn dispatch_fs_read_dir( + &self, + path: PathBuf, + tolerance: ReadDirTolerance, + supersede: Option<&str>, + ) -> JobId { let (id, cancel) = self.allocate(JobKind::FsReadDir, supersede, None); let bus = self.workers.clone(); self.pool.dispatch(move |_pool| { - let kind = run_fs_read_dir(&cancel, &path); + let kind = run_fs_read_dir(&cancel, &path, tolerance); let _ = bus.send(ASYNC_REPLY_TOPIC, &WorkerReply { job_id: id, kind }); }); id @@ -1038,8 +1050,8 @@ impl AsyncRuntime { ReplyKind::Parse { duration_ms } => { PendingState::Complete(JobResult::Parse { duration_ms }) } - ReplyKind::ReadDir(entries) => { - PendingState::Complete(JobResult::ReadDir(entries)) + ReplyKind::ReadDir(listing) => { + PendingState::Complete(JobResult::ReadDir(listing)) } ReplyKind::Stat(entry) => PendingState::Complete(JobResult::Stat(entry)), ReplyKind::Json(v) => PendingState::Complete(JobResult::Json(v)), @@ -1295,9 +1307,13 @@ fn run_sleep(cancel: &CancellationToken, total: Duration) -> ReplyKind { /// [`FsError::Cancelled`] becomes [`ReplyKind::Cancelled`]; /// [`FsError::Io`] becomes [`ReplyKind::Error`] with the /// human-readable message attached. -fn run_fs_read_dir(cancel: &CancellationToken, path: &Path) -> ReplyKind { - match read_dir_blocking(path, cancel) { - Ok(entries) => ReplyKind::ReadDir(entries), +fn run_fs_read_dir( + cancel: &CancellationToken, + path: &Path, + tolerance: ReadDirTolerance, +) -> ReplyKind { + match read_dir_blocking(path, cancel, tolerance) { + Ok(listing) => ReplyKind::ReadDir(listing), Err(FsError::Cancelled) => ReplyKind::Cancelled, Err(e @ (FsError::Io { .. } | FsError::NonUtf8Path { .. })) => { ReplyKind::Error(e.to_string()) diff --git a/src/editor.rs b/src/editor.rs index 79f1225..5951f01 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -525,6 +525,17 @@ impl EditorState { include_str!("../builtin/runtime/window.lua"), ) .expect("load window builtin chunk"); + // Dired Stage 1: the directory view. Loaded AFTER window.lua, + // whose `window.panel-height` setting a `display = "panel"` + // listing resolves, and after the pre-runtime tables it drives + // (`pmacs.config` / `command` / `keymap` / `buffer` / `editor` / + // `minibuffer` / `path`, plus `pmacs.fs` from fs.lua above). + lua_host + .eval( + Some("@pmacs/builtin/runtime/dired.lua"), + include_str!("../builtin/runtime/dired.lua"), + ) + .expect("load dired builtin chunk"); // Compile-mode (Arc 5 stage 1, Q#CM1) — ORDERING CONTRACT: // compile.lua must load AFTER lsp.lua. It takes over // `M-g n` / `M-g p` for the unified error dispatchers, and diff --git a/src/editor_core.rs b/src/editor_core.rs index cfb2bcb..89432cc 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -4787,7 +4787,14 @@ fn backward_word(buf: &Buffer, mut pos: Position) -> Position { /// path's on-disk identity. Every step is best-effort — if `$HOME` /// or the cwd is unavailable the path is returned as far as it could /// be resolved rather than panicking. -fn normalize_buffer_path(path: PathBuf) -> PathBuf { +/// +/// Public because dired needs the *same* canonical form the buffer +/// registry keys on (Q#DR2): its buffer-per-directory naming and +/// `find_buffer_for_path`'s dedup have to agree, and a Lua-side mirror +/// of this function would be a second implementation of a canonical +/// form — the tab-width-constants class in miniature. `pmacs.path +/// .canonicalize` is this function, not a copy of it. +pub fn normalize_buffer_path(path: PathBuf) -> PathBuf { let path = expand_tilde(path); let abs = if path.is_absolute() { path diff --git a/src/fs.rs b/src/fs.rs index 0b20a6d..1767234 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -116,6 +116,59 @@ impl FsEntryKind { } } +/// Per-entry tolerance for [`read_dir_blocking`] (dired Q#DR6). +/// +/// The M8.1 primitive was all-or-nothing: five per-entry conditions +/// failed the *entire* listing, which makes a plain refresh of a busy +/// directory (`/tmp`, a build tree) fail outright. The module doc used +/// to say a per-entry-tolerant wrapper was "the package's job" --- it +/// cannot be: the primitive hands Lua one structured error and no +/// partial vec, so there is nothing to be tolerant *with*. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ReadDirTolerance { + /// Any per-entry failure fails the whole listing. The original + /// M8.1 contract, and still the default at every Lua call site + /// that does not opt in. + Fatal, + /// Per-entry failures are recorded in [`FsDirListing::errors`] and + /// enumeration continues. A failure on the *parent* `read_dir` + /// stays fatal (a directory you cannot open has no partial + /// answer), and so does a non-UTF-8 entry **name** --- see + /// [`FsError::NonUtf8Path`]. + PerEntry, +} + +/// One per-entry failure recorded by a tolerant [`read_dir_blocking`]. +/// +/// `name` is optional because a per-entry `readdir` *iterator* error +/// has no filename to report: the entry never materialized, and the +/// underlying error is about the parent directory. Every other arm has +/// an entry in hand and names it. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct FsDirEntryError { + /// Basename of the entry that failed, when one is known. + pub name: Option, + /// Rendered failure, already formatted for display. + pub message: String, +} + +/// What [`read_dir_blocking`] returns: the entries it could read, plus +/// the per-entry failures when the caller asked to tolerate them. +/// +/// `errors` is `None` under [`ReadDirTolerance::Fatal`] and `Some` +/// (possibly empty) under [`ReadDirTolerance::PerEntry`]. The +/// distinction is load-bearing at the Lua boundary: it is what selects +/// the bare-array result shape the M8.1 surface promises from the +/// `{ entries = …, errors = … }` shape the tolerant opt returns, so the +/// conversion never has to look the job back up. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct FsDirListing { + /// One entry per readable child, in filesystem iteration order. + pub entries: Vec, + /// Per-entry failures; `None` in [`ReadDirTolerance::Fatal`] mode. + pub errors: Option>, +} + /// Errors produced by [`read_dir_blocking`] / [`stat_blocking`] / /// [`rename_blocking`] / [`chmod_blocking`] / [`remove_blocking`]. /// @@ -192,50 +245,97 @@ pub enum FsError { /// `to_string_lossy` would have mangled dired/wdired round-trips). /// /// Errors on the *parent* `read_dir` call surface as -/// [`FsError::Io`]. Errors on individual entries (a single broken -/// symlink, a permission-denied stat) currently propagate the same -/// way --- the cleanest behavior at this primitive layer is "fail -/// fast and let the caller decide whether a partial listing is -/// acceptable"; dired-class will likely want a per-entry-tolerant -/// wrapper but that's the package's job, not the primitive's. +/// [`FsError::Io`] regardless of `tolerance`: a directory you cannot +/// open has no partial answer. +/// +/// Errors on individual entries (a permission-denied `lstat`, a child +/// unlinked between `readdir` and `lstat`, a `readlink` failure, a +/// non-UTF-8 symlink target) are governed by `tolerance`. Under +/// [`ReadDirTolerance::Fatal`] they fail the whole listing, which is +/// the M8.1 contract every existing caller relies on; under +/// [`ReadDirTolerance::PerEntry`] they land in +/// [`FsDirListing::errors`] and enumeration continues (dired Q#DR6). +/// +/// A non-UTF-8 entry **name** is fatal in both modes. That is not a +/// listing problem but a path-representation one: [`FsDirEntry::name`] +/// is a `String` and every `pmacs.fs` op takes a `String` path, so a +/// tolerantly-rendered non-UTF-8 name would be a name the caller could +/// not pass back through `rename`. Byte-preserving paths are the named +/// deferral (see [`FsError::NonUtf8Path`]). A non-UTF-8 *target* +/// differs in kind --- the entry's own name is fine and nothing needs +/// to round-trip the target --- so it joins the per-entry channel. pub fn read_dir_blocking( path: &Path, cancel: &CancellationToken, -) -> Result, FsError> { + tolerance: ReadDirTolerance, +) -> Result { let iter = std::fs::read_dir(path).map_err(|source| FsError::Io { path: path.display().to_string(), source, })?; let mut out: Vec = Vec::new(); + let mut errors: Option> = + matches!(tolerance, ReadDirTolerance::PerEntry).then(Vec::new); let parent_str = path.display().to_string(); for (i, entry_result) in iter.enumerate() { if i % READDIR_CANCEL_POLL_EVERY == 0 && cancel.is_cancelled() { return Err(FsError::Cancelled); } - let entry = entry_result.map_err(|source| FsError::Io { - path: parent_str.clone(), - source, - })?; + let entry = match entry_result { + Ok(entry) => entry, + Err(source) => { + // R2-2: the entry never materialized, so there is no + // name to report and the error names the parent. + record_entry_error( + &mut errors, + None, + FsError::Io { + path: parent_str.clone(), + source, + }, + )?; + continue; + } + }; let entry_path = entry.path(); - let metadata = std::fs::symlink_metadata(&entry_path).map_err(|source| FsError::Io { - path: entry_path.display().to_string(), - source, - })?; - let kind = classify(&metadata); - let symlink_target = if matches!(kind, FsEntryKind::Symlink) { - match std::fs::read_link(&entry_path) { - Ok(t) => Some(path_to_utf8_string(t.as_os_str(), &parent_str)?), - Err(source) => { - return Err(FsError::Io { + // Resolved first so a later per-entry failure can name it. + let name = path_to_utf8_string(&entry.file_name(), &parent_str)?; + let metadata = match std::fs::symlink_metadata(&entry_path) { + Ok(metadata) => metadata, + Err(source) => { + record_entry_error( + &mut errors, + Some(&name), + FsError::Io { path: entry_path.display().to_string(), source, - }); - } + }, + )?; + continue; } - } else { - None }; - let name = path_to_utf8_string(&entry.file_name(), &parent_str)?; + let kind = classify(&metadata); + let mut symlink_target = None; + if matches!(kind, FsEntryKind::Symlink) { + match std::fs::read_link(&entry_path) { + // A target we cannot represent leaves the entry in the + // listing with its target unknown, not the entry out of + // it: one weird symlink in `/tmp` used to take the + // whole directory down. + Ok(target) => match path_to_utf8_string(target.as_os_str(), &parent_str) { + Ok(target) => symlink_target = Some(target), + Err(error) => record_entry_error(&mut errors, Some(&name), error)?, + }, + Err(source) => record_entry_error( + &mut errors, + Some(&name), + FsError::Io { + path: entry_path.display().to_string(), + source, + }, + )?, + } + } out.push(FsDirEntry { name, kind, @@ -246,7 +346,33 @@ pub fn read_dir_blocking( symlink_target, }); } - Ok(out) + Ok(FsDirListing { + entries: out, + errors, + }) +} + +/// Route one per-entry failure: append it to the tolerant channel, or +/// propagate it when the caller asked for the fatal contract. +/// +/// `errors.is_none()` *is* [`ReadDirTolerance::Fatal`] --- keeping the +/// mode in the accumulator rather than passing it separately makes the +/// two impossible to disagree. +fn record_entry_error( + errors: &mut Option>, + name: Option<&str>, + error: FsError, +) -> Result<(), FsError> { + match errors { + Some(list) => { + list.push(FsDirEntryError { + name: name.map(ToOwned::to_owned), + message: error.to_string(), + }); + Ok(()) + } + None => Err(error), + } } /// Convert an [`std::ffi::OsStr`] to `String` strictly. Returns @@ -495,12 +621,23 @@ mod tests { CancellationToken::new() } + /// The fatal-mode shorthand every pre-Q#DR6 test used. + fn read_dir_fatal(path: &Path, cancel: &CancellationToken) -> Result, FsError> { + read_dir_blocking(path, cancel, ReadDirTolerance::Fatal).map(|listing| { + assert!( + listing.errors.is_none(), + "fatal mode must not open a per-entry channel" + ); + listing.entries + }) + } + #[test] fn read_dir_returns_entries_with_lstat_metadata() { let td = tempfile::tempdir().expect("tempdir"); std::fs::write(td.path().join("a.txt"), b"hello").expect("write"); std::fs::create_dir(td.path().join("subdir")).expect("mkdir"); - let entries = read_dir_blocking(td.path(), &token()).expect("read_dir"); + let entries = read_dir_fatal(td.path(), &token()).expect("read_dir"); let mut names: Vec<&str> = entries.iter().map(|e| e.name.as_str()).collect(); names.sort_unstable(); assert_eq!(names, vec!["a.txt", "subdir"]); @@ -516,7 +653,7 @@ mod tests { let td = tempfile::tempdir().expect("tempdir"); std::fs::write(td.path().join("real.txt"), b"x").expect("write"); symlink("real.txt", td.path().join("link")).expect("symlink"); - let entries = read_dir_blocking(td.path(), &token()).expect("read_dir"); + let entries = read_dir_fatal(td.path(), &token()).expect("read_dir"); let link = entries.iter().find(|e| e.name == "link").unwrap(); assert_eq!(link.kind, FsEntryKind::Symlink); assert_eq!(link.symlink_target.as_deref(), Some("real.txt")); @@ -535,7 +672,7 @@ mod tests { } let cancel = token(); cancel.cancel(); - let err = read_dir_blocking(td.path(), &cancel).expect_err("must observe cancel"); + let err = read_dir_fatal(td.path(), &cancel).expect_err("must observe cancel"); assert!(matches!(err, FsError::Cancelled), "got {err:?}"); } @@ -627,7 +764,7 @@ mod tests { fn read_dir_on_missing_path_reports_io_error() { let td = tempfile::tempdir().expect("tempdir"); let missing = td.path().join("does-not-exist"); - let err = read_dir_blocking(&missing, &token()).expect_err("must error"); + let err = read_dir_fatal(&missing, &token()).expect_err("must error"); match err { FsError::Io { path, .. } => { assert!( @@ -640,6 +777,109 @@ mod tests { } } + #[test] + fn read_dir_tolerant_opens_an_empty_error_channel_on_a_clean_directory() { + // `Some(vec![])` rather than `None` is the whole shape + // contract: the Lua boundary keys the bare-array-vs-table + // result on `errors.is_some()`, so a clean tolerant listing + // must still carry the channel. + let td = tempfile::tempdir().expect("tempdir"); + std::fs::write(td.path().join("a.txt"), b"x").expect("write"); + let listing = read_dir_blocking(td.path(), &token(), ReadDirTolerance::PerEntry) + .expect("tolerant read_dir"); + assert_eq!(listing.entries.len(), 1); + assert_eq!(listing.errors.as_deref(), Some(&[][..])); + } + + #[cfg(not(target_os = "macos"))] + #[test] + fn read_dir_tolerant_keeps_an_entry_whose_symlink_target_is_not_utf8() { + use std::os::unix::ffi::OsStrExt; + let td = tempfile::tempdir().expect("tempdir"); + std::fs::write(td.path().join("real.txt"), b"x").expect("write"); + // A legal Unix symlink target that is not representable as a + // Rust `String`. Before Q#DR6 this single entry took the whole + // listing down. + symlink( + std::ffi::OsStr::from_bytes(b"tgt-\xff"), + td.path().join("weird"), + ) + .expect("symlink"); + + let listing = read_dir_blocking(td.path(), &token(), ReadDirTolerance::PerEntry) + .expect("tolerant read_dir must survive a non-UTF-8 target"); + let weird = listing + .entries + .iter() + .find(|e| e.name == "weird") + .expect("the entry itself must be listed"); + assert_eq!(weird.kind, FsEntryKind::Symlink); + assert!( + weird.symlink_target.is_none(), + "an unrepresentable target reports as unknown" + ); + assert!( + listing.entries.iter().any(|e| e.name == "real.txt"), + "the readable sibling must survive too" + ); + let errors = listing.errors.expect("tolerant mode opens the channel"); + assert_eq!(errors.len(), 1, "one per-entry failure: {errors:?}"); + assert_eq!(errors[0].name.as_deref(), Some("weird")); + + // The same directory under the fatal contract still fails + // whole-listing --- the opt is what changes behavior, not the + // walk. + let err = read_dir_fatal(td.path(), &token()).expect_err("fatal mode must still fail"); + assert!( + matches!(err, FsError::NonUtf8Path { .. }), + "expected NonUtf8Path, got {err:?}" + ); + } + + #[test] + fn read_dir_tolerant_records_a_failed_lstat_and_lists_nothing_else_wrong() { + use std::os::unix::fs::PermissionsExt; + // Failure mode 1 from the framing: a directory readable but not + // searchable. `readdir` yields the names; every child `lstat` + // fails with EACCES. + let td = tempfile::tempdir().expect("tempdir"); + let dir = td.path().join("no-search"); + std::fs::create_dir(&dir).expect("mkdir"); + std::fs::write(dir.join("child"), b"x").expect("write child"); + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o400)).expect("chmod 400"); + let searchable = std::fs::symlink_metadata(dir.join("child")).is_ok(); + if searchable { + // Running as root (or on a filesystem that ignores the + // bits): the premise cannot be established, so assert + // nothing rather than pass vacuously. + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)) + .expect("restore perms"); + eprintln!("lstat still succeeds without search permission; skipping"); + return; + } + + let tolerant = read_dir_blocking(&dir, &token(), ReadDirTolerance::PerEntry); + let fatal = read_dir_fatal(&dir, &token()); + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)) + .expect("restore perms"); + + let listing = tolerant.expect("tolerant read_dir must not fail the listing"); + assert!( + listing.entries.is_empty(), + "the unreadable child cannot be described: {:?}", + listing.entries + ); + let errors = listing.errors.expect("tolerant mode opens the channel"); + assert_eq!(errors.len(), 1, "one per-entry failure: {errors:?}"); + assert_eq!( + errors[0].name.as_deref(), + Some("child"), + "an lstat failure has an entry in hand and must name it" + ); + let err = fatal.expect_err("fatal mode must still fail the whole listing"); + assert!(matches!(err, FsError::Io { .. }), "got {err:?}"); + } + #[cfg(not(target_os = "macos"))] #[test] fn read_dir_on_non_utf8_entry_name_reports_structured_error() { @@ -650,7 +890,7 @@ mod tests { // Rust `String`. let bad_name = std::ffi::OsStr::from_bytes(b"bad-\xff-name"); std::fs::write(td.path().join(bad_name), b"").expect("write entry"); - let err = read_dir_blocking(td.path(), &token()).expect_err("must error on non-UTF-8"); + let err = read_dir_fatal(td.path(), &token()).expect_err("must error on non-UTF-8"); match err { FsError::NonUtf8Path { parent, bytes } => { assert!( diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 29e3b47..4314e18 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -2439,6 +2439,7 @@ pub fn install( )?; pmacs.set("instance", install_instance_module(lua, registry)?)?; pmacs.set("ansi", install_ansi_module(lua)?)?; + pmacs.set("path", install_path_module(lua)?)?; pmacs.set("packages", install_packages_module(lua)?)?; pmacs.set("state", install_state_module(lua)?)?; pmacs.set("session", install_session_module(lua)?)?; @@ -3555,6 +3556,39 @@ impl UserData for AnsiParserLua { } } +/// Build the `pmacs.path.*` table: pure path arithmetic, no +/// filesystem access and no editor state. +/// +/// `canonicalize(path)` is [`crate::editor_core::normalize_buffer_path`] +/// itself — the function the buffer registry's path keys already go +/// through on write and that `find_buffer_for_path` looks up with. It +/// expands a leading `~`, absolutizes against the process cwd, folds +/// `.` / `..` lexically, and drops redundant separators (so a trailing +/// slash disappears everywhere except at root). Symlinks are +/// deliberately **not** resolved: dired's `..` must return where the +/// user navigated from, and a not-yet-created "[new file]" path has +/// nothing to resolve. +/// +/// Exposed rather than mirrored in Lua because dired keys one buffer per +/// directory on this form (Q#DR2). Two implementations that disagree on +/// an edge (`//tmp`, `~` with `HOME` unset, a `..` that would escape +/// root) would mint two buffers for one directory with no error +/// anywhere. +fn install_path_module(lua: &Lua) -> mlua::Result { + let path = lua.create_table()?; + path.set( + "canonicalize", + lua.create_function(|_, raw: String| { + Ok( + crate::editor_core::normalize_buffer_path(std::path::PathBuf::from(raw)) + .to_string_lossy() + .into_owned(), + ) + })?, + )?; + Ok(path) +} + /// Build the `pmacs.ansi.*` table. The only entry today is /// `parser()`; future additions (e.g. an event-table-validator /// helper) live alongside it. @@ -6479,6 +6513,40 @@ fn fs_dir_entry_to_lua(lua: &Lua, entry: &crate::fs::FsDirEntry) -> mlua::Result Ok(t) } +/// Convert a settled `read_dir` listing to its Lua result value. +/// +/// The shape is chosen by the listing itself (dired Q#DR6): a fatal-mode +/// listing carries no error channel and stays the **bare array** the +/// M8.1 surface documents --- the frozen M8.2 fixture consumes it with +/// `ipairs` --- while a tolerant listing becomes +/// `{ entries = { … }, errors = { { name = …?, message = … }, … } }`. +/// Keying on the payload rather than on the job keeps the additive +/// promise checkable in one place. +fn fs_dir_listing_to_lua(lua: &Lua, listing: crate::fs::FsDirListing) -> mlua::Result { + let entries = lua.create_table_with_capacity(listing.entries.len(), 0)?; + for (i, entry) in listing.entries.iter().enumerate() { + entries.set(i + 1, fs_dir_entry_to_lua(lua, entry)?)?; + } + let Some(errors) = listing.errors else { + return Ok(mlua::Value::Table(entries)); + }; + let rows = lua.create_table_with_capacity(errors.len(), 0)?; + for (i, error) in errors.iter().enumerate() { + let row = lua.create_table_with_capacity(0, 2)?; + // `name` is absent for a per-entry `readdir` iterator error: + // the entry never materialized, so there is nothing to name. + if let Some(name) = &error.name { + row.set("name", name.as_str())?; + } + row.set("message", error.message.as_str())?; + rows.set(i + 1, row)?; + } + let out = lua.create_table_with_capacity(0, 2)?; + out.set("entries", entries)?; + out.set("errors", rows)?; + Ok(mlua::Value::Table(out)) +} + fn stream_payload_to_lua(lua: &Lua, payload: StreamPayload) -> mlua::Result { match payload { StreamPayload::U64(v) => Ok(mlua::Value::Integer(i64::try_from(v).unwrap_or(i64::MAX))), @@ -6570,9 +6638,23 @@ pub fn install_async( let rt = runtime.clone(); async_mod.set( "_dispatch_fs_read_dir", - lua.create_function(move |_, (path, key): (String, Option)| { - Ok(rt.dispatch_fs_read_dir(std::path::PathBuf::from(path), key.as_deref())) - })?, + lua.create_function( + move |_, (path, key, tolerant): (String, Option, Option)| { + // dired Q#DR6: the tolerance is decided at dispatch + // and travels in the settled payload, so the result + // conversion below never has to look the job back up. + let tolerance = if tolerant == Some(true) { + crate::fs::ReadDirTolerance::PerEntry + } else { + crate::fs::ReadDirTolerance::Fatal + }; + Ok(rt.dispatch_fs_read_dir( + std::path::PathBuf::from(path), + tolerance, + key.as_deref(), + )) + }, + )?, )?; } @@ -6777,16 +6859,15 @@ pub fn install_async( i64::try_from(duration_ms).unwrap_or(i64::MAX), )); } - Some(JobOutcome::Complete(JobResult::ReadDir(entries))) => { + Some(JobOutcome::Complete(JobResult::ReadDir(listing))) => { // Lua surface for fs.read_dir settle: // status "ok", value = array of per-entry - // tables. T M8.1. + // tables (T M8.1), or the + // `{ entries = …, errors = … }` table when the + // caller opted into per-entry tolerance + // (dired Q#DR6). out.push_back(mlua::Value::String(lua.create_string("ok")?)); - let t = lua.create_table_with_capacity(entries.len(), 0)?; - for (i, entry) in entries.into_iter().enumerate() { - t.set(i + 1, fs_dir_entry_to_lua(lua, &entry)?)?; - } - out.push_back(mlua::Value::Table(t)); + out.push_back(fs_dir_listing_to_lua(lua, listing)?); } Some(JobOutcome::Complete(JobResult::Stat(entry))) => { // Lua surface for fs.stat settle: status @@ -6933,9 +7014,9 @@ fn workers_snapshot_to_lua(lua: &Lua, runtime: &SharedAsyncRuntime) -> mlua::Res "ok", mlua::Value::Integer(i64::try_from(*duration_ms).unwrap_or(i64::MAX)), ), - JobOutcome::Complete(JobResult::ReadDir(entries)) => ( + JobOutcome::Complete(JobResult::ReadDir(listing)) => ( "ok", - mlua::Value::Integer(i64::try_from(entries.len()).unwrap_or(i64::MAX)), + mlua::Value::Integer(i64::try_from(listing.entries.len()).unwrap_or(i64::MAX)), ), JobOutcome::Complete(JobResult::Stat(entry)) => { ("ok", mlua::Value::String(lua.create_string(&entry.name)?)) diff --git a/src/workers_buffer.rs b/src/workers_buffer.rs index 05e9a84..863f4ed 100644 --- a/src/workers_buffer.rs +++ b/src/workers_buffer.rs @@ -202,8 +202,18 @@ fn format_outcome(outcome: &JobOutcome) -> String { JobOutcome::Complete(JobResult::Parse { duration_ms }) => { format!("ok (parse {duration_ms}ms)") } - JobOutcome::Complete(JobResult::ReadDir(entries)) => { - format!("ok ({} entries)", entries.len()) + JobOutcome::Complete(JobResult::ReadDir(listing)) => { + // Per-entry failures (dired Q#DR6) are counted here too: a + // tolerant listing that dropped half a directory is not the + // same observable outcome as a clean one. + match listing.errors.as_deref() { + Some([_, ..]) => format!( + "ok ({} entries, {} unreadable)", + listing.entries.len(), + listing.errors.as_ref().map_or(0, Vec::len) + ), + _ => format!("ok ({} entries)", listing.entries.len()), + } } JobOutcome::Complete(JobResult::Stat(entry)) => { format!("ok (stat {:?})", entry.name) diff --git a/tests/dired_acceptance.rs b/tests/dired_acceptance.rs new file mode 100644 index 0000000..bcbd96f --- /dev/null +++ b/tests/dired_acceptance.rs @@ -0,0 +1,1469 @@ +// tests/dired_acceptance.rs --- dired arc Stage 1 acceptance. + +//! Acceptance for the dired view (`docs/dired-framing.md` §14 items +//! 1-16, Q#DR2-DR10). Item 17 --- "the fixture still passes" --- is a +//! gate item rather than a test here: `m8_1`/`m8_2`/`m8_3` prove the +//! `read_dir` opt is additive by continuing to pass unchanged. +//! +//! Discipline, following the Stage 0 suite: +//! +//! * every in-buffer claim is driven by a **real key** through +//! `dispatch_key`, so a dead mode-keymap entry cannot pass vacuously; +//! * `pmacs.dired.open` is called directly only where a test needs an +//! opt the interactive command does not carry (`display = "panel"`), +//! and it is the documented public entry point in those cases; +//! * every listing is async, so each dispatch is followed by `pump`, +//! which drives `tick_async` until the coroutine and its worker job +//! have both settled. +//! +//! Fixtures use `.txt` files and empty `pmacs.lsp.config`, so no +//! `buffer.after-load` hook spawns a language server. Note the suite +//! asserts nothing about LSP, so the wipe cannot make an assertion +//! vacuous (the Lean 4 round-1 trap). + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant, SystemTime}; + +use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; +use pmacs::cell::{CellGrid, CellSize, Glyph}; +use pmacs::editor::EditorState; +use pmacs::editor_core::normalize_buffer_path; +use pmacs::protocol::FrontendId; +use pmacs::window::WindowId; +use tempfile::TempDir; + +const ROWS: u32 = 24; +const COLS: u32 = 100; + +// --------------------------------------------------------------------------- +// Harness +// --------------------------------------------------------------------------- + +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 press(s: &mut EditorState, code: KeyCode) { + s.dispatch_key(FrontendId::LOCAL, key(code, KeyModifiers::NONE)); +} + +fn type_char(s: &mut EditorState, c: char) { + s.dispatch_key(FrontendId::LOCAL, key(KeyCode::Char(c), KeyModifiers::NONE)); +} + +fn type_str(s: &mut EditorState, text: &str) { + for ch in text.chars() { + type_char(s, ch); + } +} + +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() +} + +/// A fresh editor with a declared frame geometry (a grid frontend's real +/// frame size *is* its geometry declaration, and the panel tests need +/// one before any side window can be placed). +fn editor() -> EditorState { + let s = EditorState::new(); + exec(&s, "pmacs.lsp.config = {}"); + s.sync_frame_geometry(FrontendId::LOCAL, CellSize::new(ROWS, COLS)); + s +} + +/// An editor whose active buffer is a real file inside `dir`, so the +/// `C-x d` prompt prefills with that directory and `C-x C-j` has a file +/// to jump from. +fn editor_in(dir: &Path) -> (EditorState, PathBuf) { + let anchor = dir.join("anchor.txt"); + std::fs::write(&anchor, b"anchor\n").expect("write anchor"); + let s = editor(); + let anchor_str = anchor.display().to_string(); + exec(&s, &format!("pmacs.buffer.find_or_open({anchor_str:?})")); + (s, anchor) +} + +/// Drive the async runtime until no coroutine is parked and no worker +/// job is pending. Every dired command dispatches `read_dir` on a +/// worker and resumes on a later tick, so nothing dired does is +/// observable until this returns. +fn pump(s: &mut EditorState) { + let deadline = Instant::now() + Duration::from_secs(10); + let mut spins = 0u32; + loop { + let idle: bool = eval( + s, + "return pmacs._async.parked_count() == 0 and pmacs._async.pending_count() == 0", + ); + if idle { + return; + } + assert!(Instant::now() < deadline, "async pump deadline exceeded"); + s.tick_async(); + spins += 1; + if spins > 64 { + std::thread::sleep(Duration::from_millis(1)); + } + } +} + +/// The canonical form of `path` — the core's own normalizer, which is +/// exactly what `pmacs.path.canonicalize` calls. +fn canon(path: &Path) -> String { + normalize_buffer_path(path.to_path_buf()) + .to_string_lossy() + .into_owned() +} + +fn active_text(s: &EditorState) -> String { + eval( + s, + "local b = pmacs.window.buffer()\nreturn b:slice(0, b:len())", + ) +} + +fn active_lines(s: &EditorState) -> Vec { + active_text(s).lines().map(str::to_owned).collect() +} + +fn active_name(s: &EditorState) -> String { + eval( + s, + "return pmacs.describe.buffer(pmacs.window.buffer()).name", + ) +} + +fn active_path(s: &EditorState) -> Option { + eval( + s, + "local b = pmacs.window.buffer()\n\ + if b == nil then return nil end\n\ + local ok, p = pcall(function() return b:path() end)\n\ + if ok then return p end\n\ + return nil", + ) +} + +fn status(s: &EditorState) -> String { + s.core.borrow().status.clone() +} + +fn buffer_names(s: &EditorState) -> Vec { + eval( + s, + "local out = {}\n\ + for _, id in ipairs(pmacs.buffer.list()) do\n\ + out[#out + 1] = pmacs.describe.buffer(id).name\n\ + end\n\ + return out", + ) +} + +fn dired_buffer_names(s: &EditorState) -> Vec { + let mut names: Vec = buffer_names(s) + .into_iter() + .filter(|n| n.starts_with("*dired:")) + .collect(); + names.sort(); + names +} + +/// One layout offset from dired's own constants, so column assertions +/// cannot drift from the module that computes them. +fn layout(s: &EditorState, field: &str) -> usize { + let value: i64 = eval(s, &format!("return pmacs.dired._layout.{field}")); + usize::try_from(value).expect("layout offsets are non-negative") +} + +/// The rendered name column of one listing line. +fn line_name(s: &EditorState, line: &str) -> String { + let start = layout(s, "NAME_START"); + line.get(start..).unwrap_or("").to_owned() +} + +/// The 0-based line the entry named `name` renders on. +fn line_of(s: &EditorState, name: &str) -> usize { + let lines = active_lines(s); + for (index, line) in lines.iter().enumerate().skip(1) { + let rendered = line_name(s, line); + if rendered == name || rendered.starts_with(&format!("{name} -> ")) { + return index; + } + } + panic!("no listing line for {name:?} in {lines:#?}"); +} + +/// Seat the cursor on `name`'s line. Test scaffolding: the *keys* that +/// move by line are exercised separately (acceptance 6). +fn seat_on(s: &EditorState, name: &str) { + let line = line_of(s, name); + exec(s, &format!("pmacs.editor.move_to_line({line})")); +} + +fn cursor_line(s: &EditorState) -> usize { + let value: i64 = eval(s, "return pmacs.editor.cursor_line()"); + usize::try_from(value).expect("cursor lines are non-negative") +} + +/// The entry name under the cursor, or `None` on the header/footer. +fn cursor_entry(s: &EditorState) -> Option { + let line = cursor_line(s); + if line == 0 { + return None; + } + let lines = active_lines(s); + lines.get(line).map(|text| line_name(s, text)) +} + +/// Open `path` through the public entry point, pumping to settle. +/// Returns the raised message, if it raised. +fn open_dired(s: &mut EditorState, path: &str, opts: &str) -> Option { + exec( + s, + &format!( + "_G.DIRED_ERR = nil\n\ + pmacs.async(function()\n\ + local ok, err = pcall(pmacs.dired.open, {path:?}, {opts})\n\ + if not ok then\n\ + _G.DIRED_ERR = type(err) == 'table' and tostring(err.message) or tostring(err)\n\ + end\n\ + end)" + ), + ); + pump(s); + eval(s, "return _G.DIRED_ERR") +} + +fn open_ok(s: &mut EditorState, path: &Path, opts: &str) { + let raised = open_dired(s, &path.display().to_string(), opts); + assert!( + raised.is_none(), + "dired.open must succeed; raised {raised:?}" + ); +} + +fn side_window(s: &EditorState) -> Option { + s.core.borrow().side_window_for(FrontendId::LOCAL) +} + +fn window_buffer_name(s: &EditorState, window: WindowId) -> String { + let buffer_id = s + .core + .borrow() + .windows + .get(&window) + .map(|w| w.buffer_id) + .expect("window is live"); + let registry = s.lua_host.registry().borrow(); + registry + .get(buffer_id) + .expect("buffer is live") + .name() + .to_owned() +} + +fn active_window(s: &EditorState) -> WindowId { + s.core.borrow().active_window_id() +} + +/// Paint one real frame and return its rows as text. +fn painted_rows(s: &EditorState) -> Vec { + let size = CellSize::new(ROWS, COLS); + let mut cells = vec![pmacs::cell::Cell::default(); (ROWS * COLS) as usize]; + let mut grid = CellGrid { + cells: &mut cells, + stride: COLS, + size, + }; + pmacs::editor::paint_frame(s, FrontendId::LOCAL, &HashMap::new(), &mut grid, size); + (0..ROWS) + .map(|row| { + (0..COLS) + .map(|col| match &cells[(row * COLS + col) as usize].glyph { + Glyph::Char(ch) => *ch, + Glyph::Cluster(_) => '?', + Glyph::Continuation => ' ', + }) + .collect::() + .trim_end() + .to_owned() + }) + .collect() +} + +/// `a.txt` (5 bytes), `b.txt` (6 bytes), `subdir/`, and `link -> +/// a.txt`. +fn fixture_dir() -> TempDir { + let td = tempfile::tempdir().expect("tempdir"); + std::fs::write(td.path().join("a.txt"), b"hello").expect("write a"); + std::fs::write(td.path().join("b.txt"), b"world!").expect("write b"); + std::fs::create_dir(td.path().join("subdir")).expect("mkdir"); + std::fs::write(td.path().join("subdir").join("inner.txt"), b"deep\n").expect("write inner"); + std::os::unix::fs::symlink("a.txt", td.path().join("link")).expect("symlink"); + td +} + +// --------------------------------------------------------------------------- +// 1 --- listing shape +// --------------------------------------------------------------------------- + +/// Header line plus one line per entry, with kind char, perms, size, +/// mtime, and name; a symlink renders `l` with ` -> target`; the entry +/// count matches `read_dir`. Driven through the real `C-x d`, accepting +/// the prefilled directory. +#[test] +fn dired_renders_a_header_and_one_line_per_entry() { + let td = fixture_dir(); + let (mut s, _anchor) = editor_in(td.path()); + + ctrl(&mut s, 'x'); + type_char(&mut s, 'd'); + assert!( + eval::(&s, "return pmacs.minibuffer.is_active()"), + "C-x d must open a prompt" + ); + assert_eq!( + eval::(&s, "return pmacs.minibuffer.contents()"), + canon(td.path()), + "the prompt prefills with the current buffer's directory, so RET \ + opens where you are" + ); + press(&mut s, KeyCode::Enter); + pump(&mut s); + + let lines = active_lines(&s); + assert_eq!( + lines[0], + format!("{}:", canon(td.path())), + "line 0 is the header" + ); + let on_disk = std::fs::read_dir(td.path()).expect("read_dir").count(); + assert_eq!( + lines.len() - 1, + on_disk, + "one line per entry, no footer on a clean listing: {lines:#?}" + ); + + let kind_start = layout(&s, "KIND_START"); + let perms_start = layout(&s, "PERMS_START"); + let perms_end = layout(&s, "PERMS_END"); + let size_start = layout(&s, "SIZE_START"); + + let a = &lines[line_of(&s, "a.txt")]; + assert_eq!(&a[kind_start..=kind_start], "-", "a regular file: {a:?}"); + let perms = &a[perms_start..perms_end]; + assert_eq!(perms.len(), 9, "nine permission characters: {perms:?}"); + assert!( + perms.starts_with("rw"), + "owner may read and write a file we just wrote: {perms:?}" + ); + assert_eq!( + a[size_start..size_start + 10].trim(), + "5", + "the size column carries a.txt's five bytes: {a:?}" + ); + assert!( + a[perms_end..size_start].chars().all(char::is_whitespace), + "columns are space-separated: {a:?}" + ); + + let sub = &lines[line_of(&s, "subdir")]; + assert_eq!(&sub[kind_start..=kind_start], "d", "a directory: {sub:?}"); + + let link = &lines[line_of(&s, "link")]; + assert_eq!(&link[kind_start..=kind_start], "l", "a symlink: {link:?}"); + assert_eq!( + line_name(&s, link), + "link -> a.txt", + "a symlink shows its target" + ); + + // The mark column is reserved and blank in Stage 1 (Q#DR4): filling + // it in is Stage 2's job, and reserving it now is what keeps Stage + // 2 from moving every column right of it. + let mark_start = layout(&s, "MARK_START"); + for line in &lines[1..] { + assert_eq!( + &line[mark_start..kind_start], + " ", + "the mark column renders blank: {line:?}" + ); + } + + let mtime_start = layout(&s, "MTIME_START"); + let name_start = layout(&s, "NAME_START"); + let stamp = &a[mtime_start..name_start - 1]; + assert_eq!(stamp.len(), 16, "fixed-width mtime: {stamp:?}"); + assert!( + stamp.starts_with("20") && stamp.contains('-') && stamp.contains(':'), + "an ISO-ish minute-precision timestamp: {stamp:?}" + ); +} + +// --------------------------------------------------------------------------- +// 2 --- visit dispatches on kind, through the panel-safe primitive +// --------------------------------------------------------------------------- + +/// `RET` on a directory descends; on a file it opens the file; on the +/// header it does nothing. +#[test] +fn dired_visit_dispatches_on_entry_kind() { + let td = fixture_dir(); + let mut s = editor(); + open_ok(&mut s, td.path(), "nil"); + + // Header: no entry, so nothing happens. + exec(&s, "pmacs.editor.move_to_line(0)"); + let before = active_name(&s); + press(&mut s, KeyCode::Enter); + pump(&mut s); + assert_eq!( + active_name(&s), + before, + "RET on the header must not visit anything" + ); + + // Directory: descend into its own dired buffer. + seat_on(&s, "subdir"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + assert_eq!( + active_name(&s), + format!("*dired:{}*", canon(&td.path().join("subdir"))), + "RET on a directory opens that directory's dired buffer" + ); + assert_eq!( + line_name(&s, &active_lines(&s)[1]), + "inner.txt", + "the descended listing is the subdirectory's" + ); + + // File: the fixture's "requires the buffer-from-file API" error is + // gone --- `f` is the same command as RET. + seat_on(&s, "inner.txt"); + type_char(&mut s, 'f'); + pump(&mut s); + assert_eq!( + active_path(&s).map(PathBuf::from), + Some(PathBuf::from(canon( + &td.path().join("subdir").join("inner.txt") + ))), + "RET/f on a file opens the file bound to its path" + ); + assert_eq!( + eval::(&s, "return pmacs.window.buffer():slice(0, 4)"), + "deep", + "the file's real contents load" + ); +} + +/// The panel case, which is the real assertion (Q#DR10): with dired +/// displayed as a panel, `RET` on a file leaves the dired panel alive +/// and puts the file in the document window. Falsified by swapping +/// `display_file` for `find_or_open`, which switches the active window +/// in both branches before firing hooks --- the panel swallows itself. +#[test] +fn dired_visit_from_a_panel_keeps_the_panel_and_uses_the_document_window() { + let td = fixture_dir(); + let (mut s, anchor) = editor_in(td.path()); + let document = active_window(&s); + open_ok(&mut s, td.path(), r#"{ display = "panel" }"#); + + let panel = side_window(&s).expect("display = panel must create a side window"); + assert_eq!(active_window(&s), panel, "the panel is selected"); + assert_eq!( + window_buffer_name(&s, panel), + format!("*dired:{}*", canon(td.path())), + "the panel shows dired" + ); + + seat_on(&s, "a.txt"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + + let panel_after = side_window(&s).expect("the dired panel must survive a file visit"); + assert_eq!(panel_after, panel, "the same side window, not a new one"); + assert_eq!( + window_buffer_name(&s, panel_after), + format!("*dired:{}*", canon(td.path())), + "the panel still shows dired" + ); + assert_eq!( + window_buffer_name(&s, document), + canon(&td.path().join("a.txt")), + "the visited file lands in the document window" + ); + assert_eq!( + active_path(&s).map(PathBuf::from), + Some(PathBuf::from(canon(&td.path().join("a.txt")))), + "and it is what the visit selected" + ); + assert!( + anchor.exists(), + "fixture sanity: the anchor file was never touched" + ); +} + +// --------------------------------------------------------------------------- +// 3 --- one buffer per directory, canonicalized +// --------------------------------------------------------------------------- + +/// Descending twice then ascending twice yields the *same* buffers as +/// the first visit, and every dired buffer's name describes the +/// directory it displays. +#[test] +fn dired_navigation_reuses_one_buffer_per_directory() { + let td = tempfile::tempdir().expect("tempdir"); + let deep = td.path().join("one").join("two"); + std::fs::create_dir_all(&deep).expect("mkdir -p"); + std::fs::write(deep.join("leaf.txt"), b"leaf\n").expect("write leaf"); + + let mut s = editor(); + open_ok(&mut s, td.path(), "nil"); + exec(&s, "_G.ROOT = pmacs.window.buffer()"); + + seat_on(&s, "one"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + exec(&s, "_G.ONE = pmacs.window.buffer()"); + seat_on(&s, "two"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + exec(&s, "_G.TWO = pmacs.window.buffer()"); + assert_eq!( + active_name(&s), + format!("*dired:{}*", canon(&deep)), + "each buffer's name describes the directory it displays" + ); + + // Back up, with `^`. + type_char(&mut s, '^'); + pump(&mut s); + assert!( + eval::(&s, "return pmacs.window.buffer() == _G.ONE"), + "ascending returns to the SAME buffer, not a fresh one" + ); + assert_eq!( + cursor_entry(&s).as_deref(), + Some("two"), + "`^` seats the cursor on the directory it came from" + ); + type_char(&mut s, '^'); + pump(&mut s); + assert!( + eval::(&s, "return pmacs.window.buffer() == _G.ROOT"), + "and again at the next level up" + ); + + // Down again: still the same two buffers. + seat_on(&s, "one"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + assert!(eval::(&s, "return pmacs.window.buffer() == _G.ONE")); + seat_on(&s, "two"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + assert!(eval::(&s, "return pmacs.window.buffer() == _G.TWO")); + assert_eq!( + dired_buffer_names(&s).len(), + 3, + "three directories visited, three dired buffers: {:?}", + dired_buffer_names(&s) + ); +} + +/// Three spellings of one directory yield ONE buffer, because names and +/// lookups both go through the canonical form (Q#DR2). +#[test] +fn dired_canonicalizes_before_naming_and_lookup() { + let td = fixture_dir(); + let base = td.path().display().to_string(); + let name = td + .path() + .file_name() + .expect("tempdir has a basename") + .to_string_lossy() + .into_owned(); + + let mut s = editor(); + open_ok(&mut s, td.path(), "nil"); + let raised = open_dired(&mut s, &format!("{base}/"), "nil"); + assert!(raised.is_none(), "trailing slash must open: {raised:?}"); + let raised = open_dired(&mut s, &format!("{base}/../{name}"), "nil"); + assert!(raised.is_none(), "a `..` round trip must open: {raised:?}"); + + assert_eq!( + dired_buffer_names(&s), + vec![format!("*dired:{}*", canon(td.path()))], + "three spellings, one buffer" + ); +} + +/// `dired.kill-when-opening` (Emacs 28's opt-out): the departed buffer +/// is gone after a descent. +#[test] +fn dired_kill_when_opening_kills_the_departed_buffer() { + let td = fixture_dir(); + let mut s = editor(); + exec(&s, "pmacs.config.set('dired.kill-when-opening', true)"); + open_ok(&mut s, td.path(), "nil"); + assert_eq!(dired_buffer_names(&s).len(), 1); + + seat_on(&s, "subdir"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + + assert_eq!( + dired_buffer_names(&s), + vec![format!("*dired:{}*", canon(&td.path().join("subdir")))], + "descending killed the buffer it left" + ); + // And the setting is what did it: the default keeps both. + exec(&s, "pmacs.config.set('dired.kill-when-opening', false)"); + type_char(&mut s, '^'); + pump(&mut s); + assert_eq!( + dired_buffer_names(&s).len(), + 2, + "with the setting off, the departed buffer survives: {:?}", + dired_buffer_names(&s) + ); +} + +// --------------------------------------------------------------------------- +// 3b --- canonicalization parity +// --------------------------------------------------------------------------- + +/// The Lua canonicalizer and the core normalizer agree on every edge in +/// one shared list --- because they are the *same function* +/// (`pmacs.path.canonicalize` is `normalize_buffer_path`). Stage 1 +/// deliberately did not mirror the normalizer in Lua: a second +/// implementation that disagreed on `//tmp` or a `..` at root would +/// mint two buffers for one directory with no error anywhere, and the +/// mirror would then owe Stage 2 a removal. +#[test] +fn dired_canonicalization_is_the_cores_own_normalizer() { + let s = editor(); + let cases = [ + "//tmp", + "/tmp/", + "/tmp/../tmp", + "/tmp/./x/../y", + "/../..", + "/", + ".", + "relative/path", + "~", + "~/inside", + "~notauser/x", + ]; + for case in cases { + let from_lua: String = eval(&s, &format!("return pmacs.path.canonicalize({case:?})")); + let from_rust = normalize_buffer_path(PathBuf::from(case)) + .to_string_lossy() + .into_owned(); + assert_eq!( + from_lua, from_rust, + "canonicalization must not fork for {case:?}" + ); + } + + // And the form dired names buffers with is that same form. + let td = fixture_dir(); + let mut s = s; + open_ok(&mut s, td.path(), "nil"); + assert_eq!(active_name(&s), format!("*dired:{}*", canon(td.path()))); +} + +// --------------------------------------------------------------------------- +// 3c --- panel descent +// --------------------------------------------------------------------------- + +/// A directory descent in a panel-displayed dired stays in the *same* +/// side window (Q#DR10): the next directory is the same kind of thing as +/// the current one and belongs in the same slot. Neither replaced by a +/// document window nor duplicated. +#[test] +fn dired_directory_descent_stays_in_its_side_window() { + let td = fixture_dir(); + let (mut s, anchor) = editor_in(td.path()); + let document = active_window(&s); + open_ok(&mut s, td.path(), r#"{ display = "panel" }"#); + let panel = side_window(&s).expect("a side window"); + + seat_on(&s, "subdir"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + + assert_eq!( + side_window(&s), + Some(panel), + "the same side window, not a second one" + ); + assert_eq!( + window_buffer_name(&s, panel), + format!("*dired:{}*", canon(&td.path().join("subdir"))), + "showing the new directory" + ); + assert_eq!( + window_buffer_name(&s, document), + canon(&anchor), + "the document window is untouched" + ); + assert_eq!(active_window(&s), panel, "and dired keeps the focus"); +} + +/// A **dedicated** panel is a different story, and the framing's R2-3 +/// expectation ("the new dired buffer inherits the dedication") is +/// falsified by the substrate: `display_buffer` never replaces the +/// buffer in a slot dedicated to another one --- it discards every +/// side-specific parameter and falls back to the document window +/// (Q#BP3 2.iii). Dired does not try to unpin the user's panel, so the +/// pin holds and the new directory appears in the document area, which +/// is also what Emacs's `display-buffer` does with a dedicated window. +#[test] +fn dired_descent_from_a_dedicated_panel_leaves_the_pin_alone() { + let td = fixture_dir(); + let (mut s, _anchor) = editor_in(td.path()); + let document = active_window(&s); + open_ok(&mut s, td.path(), r#"{ display = "panel" }"#); + let panel = side_window(&s).expect("a side window"); + exec( + &s, + &format!( + "pmacs.window.set_params({}, {{ dedicated = true }})", + panel.raw() + ), + ); + + seat_on(&s, "subdir"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + + assert_eq!( + side_window(&s), + Some(panel), + "no second side window is created" + ); + assert_eq!( + window_buffer_name(&s, panel), + format!("*dired:{}*", canon(td.path())), + "the dedicated slot keeps the buffer it was pinned to" + ); + assert!( + eval::( + &s, + &format!("return pmacs.window.params({}).dedicated", panel.raw()) + ), + "and it is still dedicated afterward" + ); + assert_eq!( + window_buffer_name(&s, document), + format!("*dired:{}*", canon(&td.path().join("subdir"))), + "the new directory falls back to the document window" + ); +} + +// --------------------------------------------------------------------------- +// 4 --- ownership check +// --------------------------------------------------------------------------- + +/// A foreign buffer that merely *has* dired's name is not adopted (F7): +/// `pmacs.buffer.create` takes any caller-chosen name, and dired paints +/// with `bypass_intercept`, so adopting one would silently clobber a +/// user's data. +#[test] +fn dired_does_not_adopt_a_foreign_buffer_with_its_name() { + let td = fixture_dir(); + let mut s = editor(); + let name = format!("*dired:{}*", canon(td.path())); + exec( + &s, + &format!( + "local b = pmacs.buffer.create({name:?})\n\ + b:insert(0, 'FOREIGN CONTENTS')\n\ + _G.FOREIGN = b" + ), + ); + // Even with the major mode set, which is the weaker ownership test + // the framing floated: the handle table is the authority. + exec(&s, "pmacs.buffer.set_major_mode(_G.FOREIGN, 'dired')"); + + open_ok(&mut s, td.path(), "nil"); + + assert_eq!( + eval::(&s, "return _G.FOREIGN:slice(0, _G.FOREIGN:len())"), + "FOREIGN CONTENTS", + "the foreign buffer's contents must be byte-identical" + ); + assert!( + !eval::(&s, "return pmacs.window.buffer() == _G.FOREIGN"), + "dired must not display the foreign buffer" + ); + assert_eq!( + active_name(&s), + format!("{name}<2>"), + "dired opens under a disambiguated name instead" + ); + assert!( + active_lines(&s)[0].ends_with(':'), + "and it is a real listing: {:?}", + active_lines(&s)[0] + ); +} + +// --------------------------------------------------------------------------- +// 5 --- read-only discipline +// --------------------------------------------------------------------------- + +/// An ordinary self-insert is rejected by the intercept and leaves the +/// text byte-identical, while dired's own repaint succeeds through +/// `bypass_intercept`. `set_round_trip_input` is pinned through the +/// **production** seam a semantic frontend reads (`dispatch_idle_for`, +/// published as `DispatchIdle`) rather than by a direct-call assertion: +/// without it, a GPU session would optimistically apply `g` as an +/// insert instead of letting it reach the revert binding. +#[test] +fn dired_buffer_is_read_only_and_round_trips_input() { + let td = fixture_dir(); + let mut s = editor(); + open_ok(&mut s, td.path(), "nil"); + let before = active_text(&s); + + // A document window, deliberately: the panel arm of the same gate + // (`!window.is_side()`) would otherwise be what makes this pass. + assert!( + !s.core + .borrow() + .windows + .get(&active_window(&s)) + .expect("live window") + .is_side(), + "fixture premise: dired is in a document window here" + ); + assert!( + !s.dispatch_idle_for(FrontendId::LOCAL), + "a round-trip buffer must turn optimistic apply OFF" + ); + + // `z` is bound nowhere in dired mode, so it reaches self-insert. + type_char(&mut s, 'z'); + assert_eq!( + active_text(&s), + before, + "the read-only intercept must reject a self-insert" + ); + assert!( + status(&s).contains("read-only"), + "and say so; got {:?}", + status(&s) + ); + + // Dired's own writes still land: revert repaints the whole buffer. + std::fs::write(td.path().join("c.txt"), b"new\n").expect("write c"); + type_char(&mut s, 'g'); + pump(&mut s); + assert!( + active_text(&s).contains("c.txt"), + "dired's own repaint bypasses the intercept: {:?}", + active_text(&s) + ); +} + +// --------------------------------------------------------------------------- +// 6 --- mode keymap +// --------------------------------------------------------------------------- + +/// The keys resolve through `scope = "mode"` with no per-buffer +/// binding: a *second* dired buffer, created by a descent that calls no +/// `keymap.bind` of its own, still responds to `n`, `g`, and `^`. The +/// mode also shows in the statusline, through a real painted frame. +#[test] +fn dired_keys_resolve_through_the_mode_keymap() { + let td = fixture_dir(); + let mut s = editor(); + open_ok(&mut s, td.path(), "nil"); + seat_on(&s, "subdir"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + + assert_eq!( + eval::>(&s, "return pmacs.buffer.major_mode(pmacs.window.buffer())"), + Some("dired".to_owned()), + "the descended buffer carries the mode" + ); + assert_eq!( + eval::( + &s, + "local n = 0\n\ + for _, entry in ipairs(pmacs.keymap.list()) do\n\ + if entry.scope:find('buffer') then n = n + 1 end\n\ + end\n\ + return n" + ), + 0, + "and no buffer-scoped binding exists anywhere" + ); + + // `n` moves by line through the mode binding. + exec(&s, "pmacs.editor.move_to_line(0)"); + type_char(&mut s, 'n'); + assert_eq!(cursor_line(&s), 1, "`n` moves down one line"); + + // `g` reverts: a file added externally appears. + std::fs::write(td.path().join("subdir").join("second.txt"), b"x\n").expect("write second"); + type_char(&mut s, 'g'); + pump(&mut s); + assert!( + active_text(&s).contains("second.txt"), + "`g` re-read the directory: {:?}", + active_text(&s) + ); + + // `^` ascends. + type_char(&mut s, '^'); + pump(&mut s); + assert_eq!( + active_name(&s), + format!("*dired:{}*", canon(td.path())), + "`^` ascends from the second buffer too" + ); + + let rows = painted_rows(&s); + let mode_line = rows + .iter() + .rev() + .find(|row| row.contains("dired")) + .unwrap_or_else(|| panic!("no painted row mentions the mode: {rows:#?}")); + assert!( + mode_line.contains("dired"), + "the major mode shows in the statusline: {mode_line:?}" + ); +} + +// --------------------------------------------------------------------------- +// 7 --- cursor preservation +// --------------------------------------------------------------------------- + +/// The cursor is re-seated by BASENAME across a repaint (Q#DR9), and +/// falls back to the nearest surviving line when the entry is gone. +/// Every repaint is wholesale, so a dired that dropped to line 0 after +/// each revert would be unusable. +#[test] +fn dired_revert_reseats_the_cursor_by_basename() { + let td = tempfile::tempdir().expect("tempdir"); + for name in ["c.txt", "d.txt", "e.txt"] { + std::fs::write(td.path().join(name), b"x").expect("write"); + } + let mut s = editor(); + open_ok(&mut s, td.path(), "nil"); + seat_on(&s, "d.txt"); + let line_before = cursor_line(&s); + + // Two files that sort BEFORE it, so its line index has to change. + std::fs::write(td.path().join("a.txt"), b"x").expect("write a"); + std::fs::write(td.path().join("b.txt"), b"x").expect("write b"); + type_char(&mut s, 'g'); + pump(&mut s); + + assert_ne!( + cursor_line(&s), + line_before, + "fixture premise: the line index moved" + ); + assert_eq!( + cursor_entry(&s).as_deref(), + Some("d.txt"), + "the cursor follows the basename, not the line" + ); + + // Now the entry disappears: land on the nearest surviving line. + let vanished_line = cursor_line(&s); + std::fs::remove_file(td.path().join("d.txt")).expect("rm d"); + type_char(&mut s, 'g'); + pump(&mut s); + assert!( + cursor_line(&s) > 0, + "a vanished entry must not drop the cursor to the header" + ); + assert_eq!( + cursor_line(&s), + vanished_line.min(active_lines(&s).len() - 1), + "it lands on the nearest surviving line" + ); +} + +// --------------------------------------------------------------------------- +// 8 --- sort modes +// --------------------------------------------------------------------------- + +/// `s` cycles name -> mtime -> size -> name; mtime sorts newest first +/// and size largest first, each with a stable name tiebreak; the cursor +/// stays on its basename across the reorder. +#[test] +fn dired_sort_cycles_name_then_mtime_then_size() { + let td = tempfile::tempdir().expect("tempdir"); + // Explicit sizes and mtimes, so neither order depends on the + // filesystem's timestamp resolution or on write ordering. + let plan = [ + ("a.txt", 3usize, 1_000u64), + ("b.txt", 1, 3_000), + ("c.txt", 2, 2_000), + ]; + for (name, size, mtime) in plan { + let path = td.path().join(name); + std::fs::write(&path, vec![b'x'; size]).expect("write"); + let file = std::fs::File::options() + .write(true) + .open(&path) + .expect("open for set_modified"); + file.set_modified(SystemTime::UNIX_EPOCH + Duration::from_secs(mtime)) + .expect("set mtime"); + } + + let mut s = editor(); + open_ok(&mut s, td.path(), "nil"); + let names = |s: &EditorState| -> Vec { + active_lines(s) + .iter() + .skip(1) + .map(|line| line_name(s, line)) + .collect() + }; + assert_eq!( + names(&s), + vec!["a.txt", "b.txt", "c.txt"], + "the initial order is by name" + ); + + seat_on(&s, "c.txt"); + type_char(&mut s, 's'); + assert_eq!( + names(&s), + vec!["b.txt", "c.txt", "a.txt"], + "mtime sorts newest first" + ); + assert!( + status(&s).contains("mtime"), + "and reports the new mode: {:?}", + status(&s) + ); + assert_eq!( + cursor_entry(&s).as_deref(), + Some("c.txt"), + "the cursor stays on its basename across the reorder" + ); + + type_char(&mut s, 's'); + assert_eq!( + names(&s), + vec!["a.txt", "c.txt", "b.txt"], + "size sorts largest first" + ); + type_char(&mut s, 's'); + assert_eq!( + names(&s), + vec!["a.txt", "b.txt", "c.txt"], + "and cycles back" + ); +} + +// --------------------------------------------------------------------------- +// 9 --- tolerant listing +// --------------------------------------------------------------------------- + +/// A child whose `lstat` fails no longer fails the whole listing +/// (Q#DR6): the readable entries render, the footer counts what could +/// not be read, and the default (non-opt) call still returns a bare +/// array — both forms are exercised here, so the frozen fixture's +/// contract cannot regress unnoticed. +#[test] +fn dired_tolerant_listing_renders_what_it_can_and_counts_the_rest() { + use std::os::unix::fs::PermissionsExt; + let td = tempfile::tempdir().expect("tempdir"); + let dir = td.path().join("no-search"); + std::fs::create_dir(&dir).expect("mkdir"); + std::fs::write(dir.join("readable.txt"), b"x").expect("write readable"); + std::fs::write(dir.join("blocked.txt"), b"x").expect("write blocked"); + // Readable but not searchable: `readdir` yields the names, every + // child `lstat` fails. + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o400)).expect("chmod 400"); + if std::fs::symlink_metadata(dir.join("readable.txt")).is_ok() { + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)).expect("restore"); + eprintln!("lstat still succeeds without search permission (root?); skipping"); + return; + } + + let mut s = editor(); + let raised = open_dired(&mut s, &dir.display().to_string(), "nil"); + std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)).expect("restore"); + assert!( + raised.is_none(), + "a per-entry failure must not fail the listing: {raised:?}" + ); + + let lines = active_lines(&s); + assert_eq!( + lines.last().map(String::as_str), + Some("2 entries unreadable"), + "the footer names how much of the view is missing: {lines:#?}" + ); + + // Both call shapes, in one test: the bare array is what the frozen + // M8.2 fixture consumes with `ipairs`. + let shapes: Vec = eval( + &s, + &format!( + "local out = {{}}\n\ + pmacs.async(function()\n\ + local bare = pmacs.fs.read_dir({:?}):await()\n\ + local tolerant = pmacs.fs.read_dir({:?}, {{ tolerant = true }}):await()\n\ + _G.SHAPES = {{\n\ + #bare,\n\ + bare.entries == nil and 1 or 0,\n\ + #tolerant.entries,\n\ + tolerant.errors ~= nil and 1 or 0,\n\ + #tolerant.errors,\n\ + }}\n\ + end)\n\ + return out", + td.path().display().to_string(), + td.path().display().to_string() + ), + ); + assert!(shapes.is_empty(), "the async body has not run yet"); + pump(&mut s); + let shapes: Vec = eval(&s, "return _G.SHAPES"); + assert_eq!( + shapes, + vec![1, 1, 1, 1, 0], + "bare: one entry and no `entries` field; tolerant: one entry plus \ + an empty error channel" + ); + + // A failure on the parent itself is still fatal. + let missing = td.path().join("does-not-exist"); + let raised = open_dired(&mut s, &missing.display().to_string(), "nil"); + assert!( + raised.is_some(), + "an unopenable directory has no partial answer" + ); +} + +// --------------------------------------------------------------------------- +// 10 --- tolerant symlink targets +// --------------------------------------------------------------------------- + +/// A symlink whose target is not UTF-8 lists successfully with the +/// entry present and its target reported unknown (F5). Falsified by +/// reverting the `read_link`/target arm in `read_dir_blocking`, which +/// takes the whole listing down. +#[cfg(not(target_os = "macos"))] +#[test] +fn dired_lists_a_symlink_whose_target_is_not_utf8() { + use std::os::unix::ffi::OsStrExt; + let td = tempfile::tempdir().expect("tempdir"); + std::fs::write(td.path().join("real.txt"), b"x").expect("write real"); + std::os::unix::fs::symlink( + std::ffi::OsStr::from_bytes(b"target-\xff"), + td.path().join("weird"), + ) + .expect("symlink"); + + let mut s = editor(); + let raised = open_dired(&mut s, &td.path().display().to_string(), "nil"); + assert!( + raised.is_none(), + "one weird symlink must not take the directory down: {raised:?}" + ); + + let lines = active_lines(&s); + let weird = &lines[line_of(&s, "weird")]; + assert_eq!( + line_name(&s, weird), + "weird -> ?", + "the entry is listed with an unknown target" + ); + assert!( + lines.iter().any(|line| line_name(&s, line) == "real.txt"), + "and the readable sibling is still there: {lines:#?}" + ); + assert_eq!( + lines.last().map(String::as_str), + Some("1 entries unreadable"), + "the footer counts it: {lines:#?}" + ); +} + +// --------------------------------------------------------------------------- +// 11 --- unknown opts keys +// --------------------------------------------------------------------------- + +/// A typo'd opt errors naming the key instead of silently listing in +/// fatal mode (framing §8, minor c). Silently ignoring it is exactly +/// how a tolerant listing would degrade with no signal at all. +#[test] +fn read_dir_rejects_an_unknown_opts_key() { + let td = fixture_dir(); + let s = editor(); + let message: String = eval( + &s, + &format!( + "local ok, err = pcall(pmacs.fs.read_dir, {:?}, {{ tolerat = true }})\n\ + if ok then return 'NO ERROR' end\n\ + return tostring(err)", + td.path().display().to_string() + ), + ); + assert!( + message.contains("tolerat") && message.contains("unknown opts key"), + "the error must name the offending key; got {message:?}" + ); + + // A wrongly-typed known key is rejected too. + let message: String = eval( + &s, + &format!( + "local ok, err = pcall(pmacs.fs.read_dir, {:?}, {{ tolerant = 'yes' }})\n\ + if ok then return 'NO ERROR' end\n\ + return tostring(err)", + td.path().display().to_string() + ), + ); + assert!( + message.contains("tolerant must be a boolean"), + "got {message:?}" + ); +} + +// --------------------------------------------------------------------------- +// 12 --- non-UTF-8 names stay fatal +// --------------------------------------------------------------------------- + +/// A non-UTF-8 *name* is a path-representation problem, not a listing +/// one: dired reports the structured error and creates no buffer. +/// Rendering it tolerantly would hand dired a name it could not pass +/// back through `rename`. +#[cfg(not(target_os = "macos"))] +#[test] +fn dired_reports_a_non_utf8_name_and_creates_no_buffer() { + use std::os::unix::ffi::OsStrExt; + let td = tempfile::tempdir().expect("tempdir"); + std::fs::write( + td.path() + .join(std::ffi::OsStr::from_bytes(b"bad-\xff-name")), + b"", + ) + .expect("write entry"); + + let mut s = editor(); + let before = active_name(&s); + // Through the real command, so the reporting path is the one a user + // hits rather than `pmacs.dired.open`'s raise. + exec( + &s, + &format!( + "pmacs.async(function()\n\ + local ok, err = pcall(pmacs.dired.open, {:?})\n\ + if not ok then\n\ + pmacs.editor.set_status('dired: ' .. tostring(err.message))\n\ + end\n\ + end)", + td.path().display().to_string() + ), + ); + pump(&mut s); + + let line = status(&s); + assert!( + line.contains("non-UTF-8") && line.contains("255"), + "the structured error must surface with the offending raw bytes; \ + got {line:?}" + ); + assert!( + dired_buffer_names(&s).is_empty(), + "and no dired buffer was created: {:?}", + dired_buffer_names(&s) + ); + assert_eq!(active_name(&s), before, "the active buffer is untouched"); +} + +// --------------------------------------------------------------------------- +// 13 --- dired-jump +// --------------------------------------------------------------------------- + +/// `C-x C-j` opens dired on this file's directory with the cursor on +/// that file's line; from a buffer with no path it reports and creates +/// nothing. +#[test] +fn dired_jump_seats_the_cursor_on_the_visited_file() { + let td = fixture_dir(); + let (mut s, anchor) = editor_in(td.path()); + + ctrl(&mut s, 'x'); + ctrl(&mut s, 'j'); + pump(&mut s); + + assert_eq!( + active_name(&s), + format!("*dired:{}*", canon(td.path())), + "dired opens on the file's directory" + ); + assert_eq!( + cursor_entry(&s).as_deref(), + Some("anchor.txt"), + "with the cursor on the file we jumped from" + ); + assert!(anchor.exists()); + + // From a pathless buffer: report, create nothing. + exec( + &s, + "pmacs.window.switch_buffer(pmacs.buffer.create('*pathless*'))", + ); + let before = dired_buffer_names(&s); + ctrl(&mut s, 'x'); + ctrl(&mut s, 'j'); + pump(&mut s); + assert!( + status(&s).contains("no file"), + "the reason must surface; got {:?}", + status(&s) + ); + assert_eq!( + dired_buffer_names(&s), + before, + "and nothing new was created" + ); + assert_eq!(active_name(&s), "*pathless*", "nor was anything displayed"); +} + +// --------------------------------------------------------------------------- +// 14 --- quit +// --------------------------------------------------------------------------- + +/// `q` restores the previously active buffer; in a side window it +/// routes through `pmacs.window.quit`, matching `listview.quit`'s +/// Q#BP11b split. +#[test] +fn dired_quit_restores_the_previous_buffer_and_closes_a_panel() { + let td = fixture_dir(); + let (mut s, anchor) = editor_in(td.path()); + open_ok(&mut s, td.path(), "nil"); + type_char(&mut s, 'q'); + assert_eq!( + active_name(&s), + canon(&anchor), + "`q` returns to the buffer dired was opened from" + ); + + // The panel arm: `q` deletes the side window rather than switching + // the buffer inside it. + open_ok(&mut s, td.path(), r#"{ display = "panel" }"#); + assert!(side_window(&s).is_some(), "fixture premise: a side window"); + type_char(&mut s, 'q'); + assert_eq!( + side_window(&s), + None, + "`q` in a side window routes through window.quit" + ); + assert_eq!( + active_name(&s), + canon(&anchor), + "and focus lands back in the document window" + ); +} + +// --------------------------------------------------------------------------- +// 15 --- failure leaves nothing behind +// --------------------------------------------------------------------------- + +/// `C-x d` on a nonexistent directory creates no buffer, switches no +/// window, and reports the reason (the fixture's +/// `dired_open_failure_leaves_editor_unchanged` invariant), driven +/// through the real prompt. +#[test] +fn dired_open_failure_leaves_the_editor_unchanged() { + let td = fixture_dir(); + let (mut s, anchor) = editor_in(td.path()); + let before_window = active_window(&s); + let before_names = buffer_names(&s); + + ctrl(&mut s, 'x'); + type_char(&mut s, 'd'); + // The field prefills with the anchor's directory; append a path + // component that does not exist. + type_str(&mut s, "/nope"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + + let line = status(&s); + assert!( + line.starts_with("dired: "), + "the failure surfaces as dired's own status message; got {line:?}" + ); + assert_eq!( + buffer_names(&s), + before_names, + "no buffer was created: {:?}", + buffer_names(&s) + ); + assert_eq!(active_window(&s), before_window, "no window changed"); + assert_eq!( + active_name(&s), + canon(&anchor), + "the active buffer is intact" + ); +} + +// --------------------------------------------------------------------------- +// 16 --- scale +// --------------------------------------------------------------------------- + +/// A 10,000-entry directory renders within the fixture's established +/// 200 ms budget, on the builtin path. Carries the fixture's macOS +/// ignore gate: hosted macOS debug runners do not consistently satisfy +/// it. +#[test] +#[cfg_attr( + target_os = "macos", + ignore = "hosted macOS debug runners do not consistently satisfy this timing gate" +)] +fn dired_renders_10k_entries_within_200ms() { + let td = tempfile::tempdir().expect("tempdir"); + for i in 0..10_000 { + std::fs::write(td.path().join(format!("f{i:05}")), b"").expect("write fixture entry"); + } + + let mut s = editor(); + let started = Instant::now(); + open_ok(&mut s, td.path(), "nil"); + let elapsed = started.elapsed(); + + assert_eq!( + active_lines(&s).len(), + 10_001, + "header plus one line per entry" + ); + assert!( + elapsed < Duration::from_millis(200), + "10K entries must render within 200ms; took {elapsed:?}" + ); +} From e7fa9e9720641d2a4f2d17575be77241e5eb1f9e Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 15:03:19 -0400 Subject: [PATCH 2/7] test(dired): teach describe.key about mode scope; drop one overclaim `describe_key_identifies_every_default_binding` iterated every binding in the stack and asserted `pmacs.describe.key` resolves it context-free. That held only because no builtin had ever bound a mode-scoped key: dired is #129's first non-detection consumer, so its `n` / `p` / `g` correctly resolved to nothing and the test went red on the feature rather than on a defect. It now sets the effective context per binding -- the mode for a mode-scoped default, and explicitly NO mode for a global one, because a leaked mode legitimately shadows a global chord of the same name (dired's `RET` shadows `edit.newline-and-indent`, which is the point of the mode) and would make the assertion compare the wrong pair. A floor assertion keeps the new arm from going vacuous if the last mode-scoped default is ever removed. Also corrects a doc comment rather than leaving it to be believed: acceptance 3c does not pin the descent ROUTING. Dired holds focus in its own panel, so a raw `switch_buffer` lands in the same window and the mutation is vacuous against that test; dedication is what distinguishes the two paths, so the discriminating pin is the dedicated-panel test next to it. Verified by mutation, not assumed. --- src/editor.rs | 54 ++++++++++++++++++++++++++++++++++----- tests/dired_acceptance.rs | 8 ++++++ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/editor.rs b/src/editor.rs index 5951f01..79444f4 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -5659,17 +5659,25 @@ mod tests { // ---- T M2.11 acceptance -------------------------------------------------- - /// Every chord in the default global keymap must round-trip through + /// Every chord in the default keymap must round-trip through /// `pmacs.describe.key`: returning a non-nil table whose `command` /// matches the binding the keymap stack stores. + /// + /// `describe.key` resolves against the **effective context** + /// (buffer-local → mode → global), so a mode-scoped default is + /// asserted with a buffer that carries that mode rather than + /// context-free. Dired is the first builtin to bind mode-scoped keys + /// (#129's first non-detection consumer), and without the mode in + /// place its `n` / `p` / `g` correctly resolve to nothing. #[test] fn describe_key_identifies_every_default_binding() { + use crate::keymap_stack::Scope; let s = EditorState::new(); let kms = s.lua_host.keymaps().borrow(); - let bindings: Vec<(String, String)> = kms + let bindings: Vec<(Scope, String, String)> = kms .iter_all() .into_iter() - .map(|(_, seq, b)| (crate::key::display_sequence(&seq), b.command)) + .map(|(scope, seq, b)| (scope, crate::key::display_sequence(&seq), b.command)) .collect(); drop(kms); // Sanity floor: the default keymap binds at least the M1 surface. @@ -5678,18 +5686,50 @@ mod tests { "default keymap unexpectedly small: {} bindings", bindings.len() ); + let modes: usize = bindings + .iter() + .filter(|(scope, _, _)| matches!(scope, Scope::Mode(_))) + .count(); + assert!( + modes >= 1, + "a mode-scoped default is expected since dired Stage 1; \ + found none, so the mode arm below asserts nothing" + ); - for (seq, expected_command) in &bindings { + for (scope, seq, expected_command) in &bindings { + let mode = match scope { + Scope::Mode(name) => Some(name.clone()), + // No buffer-scoped defaults exist; a future one would + // need its own buffer context here. + Scope::Buffer(_) => continue, + Scope::Global => None, + }; + // Set the context explicitly on EVERY iteration, including + // the global one: a mode left over from a previous iteration + // legitimately shadows a global binding of the same chord + // (dired's mode-scoped `RET` shadows + // `edit.newline-and-indent`, which is the point of the + // mode), so a leaked mode would make this assert the wrong + // thing. + let context = match &mode { + Some(name) => { + format!("pmacs.buffer.set_major_mode(pmacs.window.buffer(), {name:?}); ") + } + None => "pmacs.buffer.set_major_mode(pmacs.window.buffer(), nil); ".to_owned(), + }; let script = format!( - "local r = pmacs.describe.key({seq:?}); \ + "{context}local r = pmacs.describe.key({seq:?}); \ if r == nil then return 'nil' else return r.command end" ); let got: String = s.lua_host.lua().load(&script).eval().unwrap_or_else(|e| { panic!("describe.key({seq}) raised: {e}"); }); assert_eq!( - &got, expected_command, - "describe.key for {seq:?} returned {got:?}, expected {expected_command:?}" + &got, + expected_command, + "describe.key for {seq:?} (scope {}) returned {got:?}, \ + expected {expected_command:?}", + scope.render() ); } } diff --git a/tests/dired_acceptance.rs b/tests/dired_acceptance.rs index bcbd96f..73b06f8 100644 --- a/tests/dired_acceptance.rs +++ b/tests/dired_acceptance.rs @@ -700,6 +700,14 @@ fn dired_canonicalization_is_the_cores_own_normalizer() { /// side window (Q#DR10): the next directory is the same kind of thing as /// the current one and belongs in the same slot. Neither replaced by a /// document window nor duplicated. +/// +/// **This test does not pin the routing itself, and says so rather than +/// implying otherwise:** dired holds the focus in its own panel here, so +/// a raw `switch_buffer` lands in that same window and the assertions +/// below hold either way (verified — the mutation is VACUOUS against +/// this test). What distinguishes `display { side = … }` from the raw +/// switch is dedication, so the discriminating pin is +/// `dired_descent_from_a_dedicated_panel_leaves_the_pin_alone` below. #[test] fn dired_directory_descent_stays_in_its_side_window() { let td = fixture_dir(); From 8b685dc127af1198b01722d1768b0302fa5400b1 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 15:08:24 -0400 Subject: [PATCH 3/7] docs: record dired Stage 1 (PR #165) and what it falsified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docs/dired-framing.md rev 6: §0 gains the Stage 1 implementation notes (S1-1..S1-9) -- the normalizer is exposed rather than mirrored (so B2 is false by one small binding, in the direction Q#DR2 preferred); R2-3's dedication claim is falsified by the display policy; acceptance 3c cannot pin the descent routing and now says so; dired is the first builtin to bind a mode-scoped key, which one pre-existing lib test assumed impossible; `C-x d` takes no completion source on purpose; ownership is the handle table alone; the mark column ships blank; a symlinked directory needs a probe; and interactive origin does not survive an await. COHERENCE.md, per its §25 (an audited claim this PR changes updates here, riding the PR): §1.1's interactive-file-opening fact, §2's journey step 7, §4's beginner-level `files`, §14's tree bullet (Stage 1 landed a flat listing and did NOT invent a tree convention), and §15's Priority 1 list. Step 3 stays **Missing at the CLI** with the mechanism spelled out: `pmacs .` still exits 1, and this arc deliberately does not claim the CLI path -- it supplies the buffer a directory should resolve to. docs/active-work.md: the dired lane rewritten for Stage 1, including why the branch is a fresh cut rather than a rebase of `dired`, the durable substrate facts, the bite results (one VACUOUS, recorded rather than relabelled), and the verification. Its canonical-base line was four merges stale and now names 8c86d34. docs/agent-handoff.md: one forward pointer only. The handoff describes merged state, so it absorbs the substance when this merges. --- COHERENCE.md | 37 +++++++---- docs/active-work.md | 139 +++++++++++++++++++++++++++++++----------- docs/agent-handoff.md | 5 ++ docs/dired-framing.md | 94 +++++++++++++++++++++++++++- 4 files changed, 223 insertions(+), 52 deletions(-) diff --git a/COHERENCE.md b/COHERENCE.md index 1594b69..b1557e7 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -119,10 +119,10 @@ detailed in §1.1–§1.3: **substrate without surface**, **the silence asymmetry**, and **per-arc coherence debt**. Coherence-shaped work already in flight at audit time: find-file / -dired Stage 0 (`C-x C-f`, PR #162, `docs/dired-framing.md`), bottom -panel Stage 1 (merged #155), multi-root LSP affinity (branch -`lsp-multi-root-affinity`), the config registry foundation (merged -#127). +dired Stage 0 (`C-x C-f`, merged #162, `docs/dired-framing.md`) and its +Stage 1 directory view (PR #165), bottom panel Stage 1 (merged #155), +multi-root LSP affinity (branch `lsp-multi-root-affinity`), the config +registry foundation (merged #127). --- @@ -192,9 +192,13 @@ working, unreachable capability: is Lua-bound; no builtin command opens `*lsp*` (§2, §9). - **Interactive file opening.** `pmacs.buffer.find_or_open` (`src/lua_bindings/mod.rs:3103`) had no interactive caller at audit - time; a complete 1,384-line dired exists as a frozen test fixture - (`tests/fixtures/pmacs-dired/init.lua`). Being fixed now: dired Stage - 0 (PR #162). + time; a complete 1,384-line dired existed only as a frozen test + fixture (`tests/fixtures/pmacs-dired/init.lua`). **Fixed:** dired + Stage 0 opens a path (`C-x C-f`, merged #162) and Stage 1 ships the + browsing view as a builtin (`C-x d` / `C-x C-j`, PR #165). The fixture + stays frozen — its `install_local` + `require` routing *is* the M8 + package-universality proof (Q#DR1) — and shrinking it is scheduled + after Stage 3. The strategic consequence: **most coherence gaps in pmacs are doors, not engines** — deliberately deferred surface, not design error. That is @@ -331,11 +335,11 @@ Full verdict table: |---|---|---|---| | 1 | Install | **Partial** | Source build only: `cargo build --release --workspace --features pmacs/crdt` (`README.md`). No binaries, no packaging. Runtime deps (`/bin/sh`, git, tar, coreutils) documented, never checked at runtime | | 2 | Launch unconfigured | **Works** | `EditorState::new()` → empty `*scratch*`; missing config is not an error (`src/config.rs:7-9`); recentf/saveplace/autosave default-on | -| 3 | Open real project | **Missing** | `pmacs .` exits 1 (above). No directory handling anywhere | +| 3 | Open real project | **Missing at the CLI** | `pmacs .` still exits 1 (above): `load_file` does `File::open` (which succeeds on a directory) then `read_to_end` → EISDIR, which is not `NotFound`, so `resolve_target_buffer`'s create-a-`[new file]` arm never fires. Dired Stage 1 (PR #165) supplies the buffer a directory should resolve *to*; routing `pmacs .` into it is Journey Stage 1's work, which must not invent a second directory surface | | 4 | Understand interface | **Partial** | Mode line gives name/modified/L:C/scroll + mode/LSP/terminal segments; but no welcome text (`EditorCore::new` sets `status: String::new()`), no cheat sheet, and `C-h` deletes a word (§18) | | 5 | Edit | **Works** | Full CUA + Emacs keymap in 161 lines (`builtin/keymaps/default.lua`); isearch, query-replace, kill ring, undo/redo, auto-indent/pair/comment, atomic save. Genuinely excellent zero-config | | 6 | Language intelligence | **Partial** | Rust grammar bundled and auto-attaches; rust-analyzer preconfigured (`builtin/runtime/lsp.lua:44-52`) — but a missing binary fails silently (§1.2) and highlighting masks it. No LSP status command exists to diagnose | -| 7 | Find symbol / file | **File: missing → in flight (PR #162). Symbol: works but undiscoverable** | No find-file/dired/picker existed at audit; `M-.`/`M-?`/`C-c o` bound but advertised nowhere and server-gated; no workspace-symbol command; `pmacs.index.*` has no UI | +| 7 | Find symbol / file | **File: fixed (open by path merged #162; browsing PR #165). Symbol: works but undiscoverable** | No find-file/dired/picker existed at audit. Now `C-x C-f` opens a known path and `C-x d` / `C-x C-j` browse (flat listing, `dired` mode keymap); `M-.`/`M-?`/`C-c o` still bound but advertised nowhere and server-gated; no workspace-symbol command; `pmacs.index.*` has no UI | | 8 | Open terminal | **Works but undiscoverable** | Full PTY with scrollback + modeline segment — reachable only as `M-x terminal`, no keybinding | | 9 | Build / test | **Partial** | `M-x compile.run` works, defaults cwd to detected project root, parses Rust `-->` errors — but no keybinding, an **empty first prompt** (`initial = last and last.cmdline or ""`, `builtin/runtime/compile.lua:1134-1138`), and no `cargo build`/`cargo test` suggestion despite `ProjectKind::Cargo` existing (`src/project.rs:77`) | | 10 | Inspect error | **Partial (good once reached)** | `E:n W:n` modeline counts, underlines, `M-g n/p` + ``C-x ` `` walking a unified compile/grep/diag source, message echo, `RET` visits. Gated entirely on step 6 or 9 succeeding first | @@ -428,7 +432,8 @@ level is the one missing. Audited level-by-level: **Beginner** (should see: files, buffers, search, diagnostics, terminal, build actions, menus, missing-tool guidance): -- files ✗ (no find-file at audit; PR #162 in flight) · buffers ✓ (`C-x +- files ✓ since #162 / PR #165 (`C-x C-f` opens a path, `C-x d` browses; + neither is advertised anywhere but the keymap) · buffers ✓ (`C-x b`, `*buffer-list*`) · search ✓ (`C-s`/`C-r`/`C-M-s`; project.search is M-x-only) · diagnostics ✓ once a server runs · terminal ✓ but M-x-only · build ✓ but M-x-only with empty prompt · menus △ @@ -1139,7 +1144,11 @@ Primitive-by-primitive against the list above: hierarchy, package dependency graph, worker trees, git status) will each need it; building it once *before* dired's directory view and the workers tree harden their own conventions is exactly this - section's point. + section's point. Dired Stage 1 (PR #165) landed **without** inventing + one: its listing is flat (Emacs parity), and the recursive + in-buffer case — `i` insert-subdirectory — is a named deferral in + `docs/dired-framing.md` §13, which is where a shared tree primitive + would land. - **Structured table / inspector / diff view** ✗ — none. (`describe.*` tables are the inspector's data model without a view; the wire-declared `ResourceOffer` family was reserved for diff/blame @@ -1346,8 +1355,10 @@ missing runtime entity — a real arc). Establish the end-to-end workflow; treat regressions as release blockers. **State: broken at step 3 (§2). Mostly wiring, and unusually -cheap:** directory-argument handling; a find-file surface (in flight, -PR #162); surfacing the LSP spawn failure with guidance (§1.2); a +cheap:** directory-argument handling (the remaining half of step 3 — +dired Stage 1 landed the buffer it should resolve to); a find-file +surface (**done**: #162 open-by-path, PR #165 browsing); surfacing the +LSP spawn failure with guidance (§1.2); a compile keybinding + `cargo build`/`test` default from the existing `ProjectKind::Cargo`; a terminal keybinding; a welcome buffer. The journey acceptance suite (§19) is the ratchet that keeps it fixed. diff --git a/docs/active-work.md b/docs/active-work.md index 6dc235f..b83a1bc 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -14,10 +14,11 @@ backlog. machine-local: `origin` may name this canonical URL, a release mirror, or something else, and therefore has no authority by name alone. - Canonical base at this snapshot: - `githubsucks/main` @ `0dd16a5` (GPU initial-target #148 atop folding Stage 2 - landed-doc refresh #150, folding Stage 2 #149, the ledger refresh #147, web - grammars HTML+CSS #146, and the LaTeX Stage 1 #144 / inline-math framing - #145 pair; protocol v20). + `githubsucks/main` @ `8c86d34` (the dired framing #164 atop find-file + #162, COHERENCE.md #163, Lean 4 Stage 1 #160, the minimap blank-slab fix + #159, bottom-panel Stage 1 #155, the inline-math re-scout #154, the vterm + PTY-flake fix #153, and the GPU initial-target doc refresh #152; protocol + v20). - On the transfer source, `origin/main` named a release mirror at `d3fa632` and lagged badly. On the current destination, `origin` names the canonical URL. This difference is why all recovery begins by @@ -51,7 +52,7 @@ git worktree list git status --short --branch ``` -The `git log` command must expose `0dd16a5` or a newer intentional main. +The `git log` command must expose `8c86d34` or a newer intentional main. If it does not, stop and repair the remote/fetch configuration. ## Lean 4 lane (Arc 8) — Stage 1 IN REVIEW (PR #160) @@ -124,44 +125,108 @@ If it does not, stop and repair the remote/fetch configuration. or markerless scratch files fragment into one server per directory for every language. -## Dired lane — framing APPROVED; Stage 0 MERGED, Stage 1 next +## Dired lane — Stage 0 MERGED; Stage 1 IN REVIEW (PR #165) -- Approved framing: `docs/dired-framing.md` (revision 5), landing as its - own docs PR off `githubsucks/main` @ `2af1ab3`, branch - `githubsucks/dired-framing`, worktree `../pmacs-dired-framing`. The - repo's `-framing`-branch convention (`vterm-framing`, - `gpu-initial-target-framing`, `tab-width-parity-framing`). +- Approved framing: `docs/dired-framing.md` **revision 6** — rev 5 is the + approved text (merged as its own docs PR #164), rev 6 adds §0's Stage 1 + implementation notes (S1-1…S1-9). Stages 2 (marks and operations) and 3 + (wdired) each get their own detailed framing after the prior stage lands. - **Stage 0 (`C-x C-f` find-file) MERGED as #162** (`main` @ `2af1ab3`, 2026-07-25, one review round, 12/12 CI green). Durable facts moved to `docs/agent-handoff.md` §1 per rule 3 below. -- **Stage 1 (the dired view) is next and unstarted.** Branch `dired` - (worktree `../pmacs-dired-arc`) carries the framing commits only and is - based on the now-superseded `0827dd1`; **rebase it onto the `main` - resulting from the framing PR before implementing**, or cut a fresh - branch — its framing commits become redundant once the docs PR lands. -- Stage 1's scope, from the framing §10: `builtin/runtime/dired.lua`; the - `dired` major mode + mode keymap; buffer-per-directory with lexical - canonicalization and the ownership check; read-only intercept + - `set_round_trip_input`; visit routing through `window.display_file`; - parent/sort/revert/quit; `C-x d` (with the `display` opt) / `C-x C-j`; - cursor preservation by basename; the `dired.kill-when-opening` config - key; **and the tolerant `read_dir` opt** — the only Rust in the stage. -- The one Rust change is load-bearing and is why Stage 1 is not - pure-Lua: `read_dir_blocking` (`src/fs.rs:201`) fails the **entire - listing** on any of five per-entry conditions, and the tolerant wrapper - its own module doc delegates to package authors **cannot be written in - Lua** — the primitive returns one error and no partial vec. -- Coherence (framing §0.5, required since #163): serves `COHERENCE.md` - §20 Priority 1, which names this work explicitly; journey steps 7 and - (partially) 3; **adds no interaction island** — keys are a mode-scoped - keymap, and wdired is a mode swap; adopts `pmacs.config` for - `dired.kill-when-opening`; inherits §9's worker-attribution gap for its - `read_dir` jobs without worsening it. +- **Stage 1 branch: `githubsucks/dired-stage1`**, worktree + `../pmacs-dired-stage1`, based on `githubsucks/main` @ `8c86d34` (the + framing merge #164). **A fresh cut, not a rebase:** the older `dired` + branch (`ffdd642`, worktree `../pmacs-dired-arc`) was based on the + superseded `0827dd1` and carried only the framing content #164 already + put on `main`, so merging it would have reconciled two histories of one + document. It is left untouched and carries nothing unmerged. +- **Stage 1 implemented; no wire change (protocol stays v20).** What + landed on the branch: + - `builtin/runtime/dired.lua`: one buffer per directory named + `*dired:*` with the handle-table ownership check; + read-only intercept + `set_round_trip_input`; the `dired` major mode + and its mode-scoped keymap (`RET`/`f`, `^`, `n`/`p`, `g`, `q`, `s`); + basename cursor re-seating across every wholesale repaint; + `display_file` for file visits and same-window reuse for directory + descent; `C-x d` / `C-x C-j`; the `dired.kill-when-opening` setting. + Loaded after `window.lua`. + - `src/fs.rs`: `ReadDirTolerance`, `FsDirEntryError`, `FsDirListing`, + and one walk that either fails on a per-entry condition or records it + (Q#DR6). `src/async_runtime.rs` carries the listing in + `ReplyKind::ReadDir` / `JobResult::ReadDir`; `src/lua_bindings/mod.rs` + keys the Lua result **shape** on `errors.is_some()`, so the bare array + the frozen M8.2 fixture consumes with `ipairs` is untouched; + `builtin/runtime/fs.lua` validates read-op opts and **rejects unknown + keys** (a typo'd `tolerant` used to degrade silently to fatal). + - `src/editor_core.rs` + `src/lua_bindings/mod.rs`: + `normalize_buffer_path` is `pub` and exposed as + `pmacs.path.canonicalize` — Q#DR2's preferred end state, so no Lua + mirror exists and Stage 2 owes no mirror removal. This makes B2 + ("tolerant `read_dir` is the only Rust change") false by one small + binding, deliberately. + - `tests/dired_acceptance.rs`: 22 tests over framing items 1–16, + dispatch-driven; item 17 is the m8_1/m8_2/m8_3 additivity gate. +- **The framing claim the substrate falsified (S1-2):** R2-3 expected a + dedicated dired panel to carry its dedication across a descent. + `display_buffer` never replaces the buffer in a slot dedicated to + another one — it discards every side-specific parameter and falls back + to the document window (Q#BP3 2.iii), and the exact-window arm errors. + Dired does not unpin the user's panel; both arms are pinned. +- **The vacuity the bites found (S1-3):** acceptance 3c cannot pin the + descent *routing*. Dired holds focus in its own panel, so a raw + `switch_buffer` lands in the same window and every 3c assertion holds + either way. Dedication is the only discriminator, so the + dedicated-panel test is the real pin — and the vacuity is documented at + the assertion rather than relabelled. +- **The pre-existing test dired's first mode-scoped binding broke + (S1-4):** `describe_key_identifies_every_default_binding` asserted every + binding in the stack resolves through `describe.key` context-free, which + held only while the modes table was empty. It now sets the effective + context per binding and explicitly *clears* the mode for global ones, + because a leaked mode legitimately shadows a global chord of the same + name (dired's `RET` shadows `edit.newline-and-indent`). +- Durable substrate facts, independent of this arc: + - `pmacs.buffer.kill` (not `remove`) redirects windows off a doomed + buffer before removal, so `kill-when-opening` kills **after** the + replacement is displayed. + - Interactive origin does **not** survive an await: work resumed in + `tick_async` sees no `InteractiveCommandOrigin`, so `pmacs.window.*` + acts for the *ambient* active frontend (S1-9). + - Kinds are lstat-based in both `read_dir` and `stat`, so nothing in an + entry says whether a symlink points at a directory; `RET` probes by + trying to list it (S1-8). + - A path-backed buffer's *name* is its full path, not its basename — + worth knowing before writing any name assertion. + - `C-x d` takes **no** completion source on purpose (S1-5): with one, + RET on an empty field opens whatever sorts first, and + RET-on-where-you-are is the gesture the binding exists for. The field + is prefilled instead. +- **Bite verification:** 15 claims, each mutated in place and required to + fail the test that names it. `dired.lua` is new, so `scripts/bite`'s + file swap does not apply; every mutation was applied and reverted with + `git checkout --`. One came back VACUOUS and is recorded above. +- Verification on this branch: `cargo fmt --check` clean; strict workspace + Clippy clean; 1,829 default + 2,006 CRDT library tests; dired acceptance + 22 default + 22 CRDT; m8_1 10 / m8_2 15 / m8_3 32 unchanged; M4 121; + required GPU 155; **isolated-`XDG_CONFIG_HOME` workspace sweep 3,186 + passed across 92 suites, zero failures**; `git diff --check` clean. The + sweep needs the isolated config for the reason recorded in the + bottom-panel lane below. +- Coherence (framing §0.5, required since #163): serves `COHERENCE.md` §20 + Priority 1, which names this work explicitly; journey step 7's file half + goes from no surface to a surface; **adds no interaction island** — keys + are a mode-scoped keymap, and wdired will be a mode swap; adopts + `pmacs.config` for `dired.kill-when-opening`; inherits §9's + worker-attribution gap for its `read_dir` jobs without worsening it. The + audited claims this changes are updated in `COHERENCE.md` itself, per its + §25. - **Boundary with the Journey Stage 1 arc** (`COHERENCE.md` §20 arc-cut 1): CLI directory-argument handling (`pmacs .` exits 1) belongs there, - not here. The two meet at `resolve_target_buffer`; dired supplies the - buffer a directory should resolve *to*, and `pmacs .` should route into - it rather than growing a second directory surface. + not here — Stage 1 does **not** fix it. The two meet at + `resolve_target_buffer`; dired supplies the buffer a directory should + resolve *to*, and `pmacs .` should route into it rather than growing a + second directory surface. ## Bottom-panel lane (window placement + side windows) — Stage 1 IN REVIEW diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index a844230..fe51463 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -70,6 +70,11 @@ commands, read `docs/active-work.md` immediately after this file. against an open buffer yet fails to load one that is not open — find-file expands the tilde Lua-side. Loading through the normalized path is a named deferral. + - **Stage 1 (the directory view) is IN REVIEW as PR #165** — the + builtin `dired.lua`, the per-entry-tolerant `read_dir` opt, and + `pmacs.path.canonicalize`. Its branch state, substrate facts, and + verification live in `docs/active-work.md`; this section absorbs them + when it merges. - **GPU initial target LANDED — #148** (`docs/gpu-initial-target-framing.md` rev 3; merge `0dd16a5`; two review rounds). `pmacs --gpu [--socket NAME|PATH] FILE` transports exact Unix path diff --git a/docs/dired-framing.md b/docs/dired-framing.md index 87b4573..3f6b8a5 100644 --- a/docs/dired-framing.md +++ b/docs/dired-framing.md @@ -1,11 +1,13 @@ # Dired — framing -**Revision 5 — 2026-07-25. Status: APPROVED; Stage 0 MERGED as #162.** +**Revision 6 — 2026-07-25. Status: APPROVED; Stage 0 MERGED as #162; +Stage 1 IN REVIEW as PR #165.** Rev 1 passed a ground-truth review; rev 2 fixed round 1's seven findings; rev 3 fixed round 2's six and was approved; rev 4 recorded what Stage 0's implementation falsified in the approved text (§0); rev 5 adds the **coherence impact** statement now required of every framing -(`CLAUDE.md`, `COHERENCE.md` §20) — see §0.5. Deliberately +(`CLAUDE.md`, `COHERENCE.md` §20) — see §0.5; rev 6 records what Stage +1's implementation falsified (§0, S1-1…S1-9). Deliberately unnumbered: the roadmap's Arc 8 is GPU structural parity but `docs/lean4-mode-framing.md` also claims Arc 8, so the arc space is already forked in uncommitted work. (Rev 2 also cited @@ -203,6 +205,94 @@ the correction belongs here rather than only in the code. same normalize-before-lookup family as Q#DR5's `apply_resource_op` correction. +### Stage 1 implementation notes (rev 5 → rev 6) + +Implementing Stage 1 (PR #165) falsified four things the approved text +asserted and settled five it left open. Recorded here rather than only +in the code, per the rev-4 precedent. + +- **S1-1. The normalizer is EXPOSED, not mirrored — so B2 is partly + false, in the direction Q#DR2 preferred.** Q#DR2 made the mirror + conditional (`Stage 1 may still mirror if exposure turns out to drag + in EditorCore borrow plumbing it does not otherwise need`). + `normalize_buffer_path` is a **free function** (`editor_core.rs`), so + exposure drags in nothing: it is now `pub` and reachable as + `pmacs.path.canonicalize`. Consequences, all deliberate: B2 ("tolerant + `read_dir` is the only Rust change Stage 1 needs") is false by one + small binding; acceptance 3b degenerates to the round-trip form the + framing described; and the Stage 2 mirror-removal follow-up **is not + owed** — there is no second canonical form to remove. The parity + acceptance is still carried, now as "the Lua binding and the Rust + function agree over one shared edge list", which is exactly the claim + a future re-mirroring would break. +- **S1-2. R2-3's dedication claim is falsified by the substrate.** It + read "a dedicated dired panel stays dedicated across descent and the + new dired buffer inherits it". `display_buffer` never replaces the + buffer in a slot dedicated to another one: it discards every + side-specific parameter and falls back to the document window (Q#BP3 + 2.iii), and the exact-window arm errors outright. Dired therefore does + **not** try to unpin the user's panel — which is also what Emacs's + `display-buffer` does with a dedicated window. Acceptance 3c is split: + a non-dedicated panel keeps the descent, and a dedicated one keeps its + buffer *and* its pin while the new directory appears in the document + window. +- **S1-3. Acceptance 3c cannot pin the descent ROUTING, and the test now + says so.** Dired holds the focus in its own panel, so a raw + `switch_buffer` lands in that same window and every 3c assertion holds + either way — the mutation is *vacuous* against it. Dedication is the + only thing that distinguishes `display { side = … }` from the raw + switch, so the dedicated-panel test is the discriminating pin. Found + by running the bite rather than by reading the test; the vacuity is + documented at the assertion instead of being left to be believed. +- **S1-4. Dired is the first builtin to bind a mode-scoped key, and one + pre-existing lib test assumed none existed.** + `describe_key_identifies_every_default_binding` iterated *every* + binding in the stack and asserted `pmacs.describe.key` resolves it + context-free, which held only while the modes table was empty. It now + sets the effective context per binding — and explicitly *clears* the + mode for a global one, because a mode left over from a previous + iteration legitimately shadows a global chord of the same name + (dired's `RET` shadows `edit.newline-and-indent`, which is the point + of the mode). +- **S1-5. `C-x d` deliberately takes NO completion source.** It is the + direct consequence of S0-1/S0-4: with a `files` source, RET on an + empty field opens whatever sorts first (the minibuffer selects + candidate 0 whenever the list is non-empty, and a selected candidate + shadows typed text), and RET-on-the-directory-you-are-in is exactly + the gesture `C-x d` exists for. The field is **prefilled** with the + current directory instead — Emacs's own shape here — and free text + always reaches `on_accept` because `CompletionSource::None` bypasses + candidate resolution entirely. Directory-name completion is what dired + itself replaces. +- **S1-6. Ownership is the handle table ALONE**, narrower than Q#DR2's + "present in dired's handle table, or `major_mode(buf) == "dired"`". A + foreign buffer that carries the mode *is* the case the check exists to + refuse, and a builtin's handle table cannot be lost the way a + reloadable package's can. Acceptance 4 sets the mode on the foreign + buffer to pin the stronger reading. +- **S1-7. The mark column ships in Stage 1, rendered blank.** Q#DR4 is a + Stage 2 decision, but reserving the two columns now means Stage 2 does + not move every offset and Stage 3's column-classifying intercept can + be written against constants that did not shift under it. The + constants are computed from the widths (the fixture hardcoded + `NAME_START = 39` and paid for it in every wdired test) and exported + as `pmacs.dired._layout` so acceptance cannot drift from them. +- **S1-8. A symlinked directory needs a probe, because kinds are + lstat-based.** Both `read_dir` and `stat` report a link as + `"symlink"`, so nothing in the entry says whether it points at a + directory. `RET` on a symlink therefore *tries* to list the target + (one extra syscall, on symlink lines only) and descends if that + succeeds, else visits it as a file. Q#DR10 specified only the + dir/file arms; this is the third. +- **S1-9. Interactive origin does not survive the await.** Every listing + is worker-dispatched, so the work after the first `:await()` resumes + inside `tick_async`, where `InteractiveCommandOrigin` is empty and + `pmacs.window.*` falls back to the **ambient** active frontend. Single + frontend: correct. Multi-frontend: a dired opened from peer B while A + is ambient would display for A. Not fixable from Lua (the display + surface takes no frontend argument) and named here rather than + discovered later. + ## 0.5. Coherence impact (`COHERENCE.md` §20) Required of every framing since #163. This arc was scouted and approved From 531fdf404ed0c082563065763b7d5db9372c0fd5 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 15:21:26 -0400 Subject: [PATCH 4/7] fix(dired): address PR #165 review round 1 F1 (real, small-window misbehavior). `dired.revert`'s re-seat runs after the read settles, and `pmacs.editor.move_to_line` is AMBIENT -- it moves whatever window is active. A user who switched buffers (or hit `q`) while the re-read was in flight had an unrelated buffer's cursor moved to a line index that only means something in the dired listing. The paint was already safe because it names its buffer; the seat now runs only while dired is still the active buffer, and `seat_cursor`'s doc says which callers are unconditionally in the right place and why. Pinned by a test that starts the revert, switches to a six-line file before the pump, and asserts that buffer's cursor never moved -- and that the dired buffer still reverts when it IS active. F2 (a trap set for Stage 3). `fmt_size` used `%10d`, so a size past ten digits -- 10 GB and up, ordinary for VM images and core dumps -- widened the field and shifted mtime and name right on that line alone. Cosmetic today, but `_layout` is exported as a contract and Stage 3's column-classifying intercept is planned against it. It now takes `fmt_mtime`'s discipline: exact bytes while they fit, else a fixed-width magnitude, so precision yields to the invariant rather than the other way round. This is not the deferred human-readable column -- the exact count still shows right up to where it cannot. Pinned with a sparse 12 GB fixture that skips if the filesystem refuses it. F3 (honesty and a doubled read). The symlink arm claimed the probe cost "one syscall"; it was a full `read_dir` -- opendir plus one lstat per child -- and on success `open_directory` immediately read the same directory again. Since `open_directory` reads before touching editor state and raises having changed nothing (acceptance 15's invariant), its failure IS the "not a directory" answer: the probe is gone, one read remains, and the comment says what it actually does. New test pins both arms -- a symlink to a directory descends under the path the user walked (canonicalization is lexical, so the link is not resolved), and a symlink to a file opens with the target's contents. F4 (deliberate failure mode). A tolerant listing recorded readdir iterator errors without bound, and `std::fs::ReadDir` need not terminate after yielding one. Cancellation is NOT an adequate backstop here -- which is the reason for a constant rather than a comment saying it is: a dired listing carries no supersede key, so nothing cancels it. A directory whose iterator produces nothing but errors now fails with the last error the way an unopenable directory does, after READDIR_MAX_CONSECUTIVE_ENTRY_ERRORS; the counter resets on any entry that materializes. Documented as untested and why: faking a failing iterator needs the walk generic over it, a refactor with no other consumer. Smaller notes, all taken: READ_ONLY_LIMIT renamed NAME_VARIANT_LIMIT (it caps `<2>`..`<99>`, nothing read-only); `fmt_perms`' omission of setuid/setgid/sticky documented as a decision tied to the M8.3 fixture's nine-bit parser; `format_outcome` binds the slice in the pattern instead of re-traversing; and `pmacs.path.canonicalize`'s `to_string_lossy` is noted as inside the existing non-UTF-8-path deferral rather than an exception to it. Process note, learned the hard way twice now: the round-1 dired.lua fixes were briefly wiped because a mutation-bite helper restores with `git checkout --`, which reverts to HEAD -- so a fix must be committed BEFORE it is bitten, not after. --- builtin/runtime/dired.lua | 78 +++++++++++++---- src/fs.rs | 41 +++++++-- src/lua_bindings/mod.rs | 7 ++ src/workers_buffer.rs | 4 +- tests/dired_acceptance.rs | 179 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 283 insertions(+), 26 deletions(-) diff --git a/builtin/runtime/dired.lua b/builtin/runtime/dired.lua index 78ef3b4..ceaf3d5 100644 --- a/builtin/runtime/dired.lua +++ b/builtin/runtime/dired.lua @@ -224,6 +224,11 @@ end -- `rwxr-xr-x`, without the leading kind char (rendered separately so a -- symlink shows `l` and a directory `d`). Arithmetic rather than bit -- ops: this file has to run on LuaJIT (5.1) as well as Lua 5.4. +-- +-- The nine basic bits only: setuid / setgid / sticky are deliberately +-- not surfaced as Emacs's `s` / `t`, matching the M8.3 fixture's +-- `parse_perm_string`, which edits exactly these nine. Rendering a bit +-- Stage 3 could not accept back would be worse than omitting it. local function fmt_perms(mode) local function tri(bits) local r = (bits >= 4) and "r" or "-" @@ -244,8 +249,33 @@ local function kind_char(kind) end end +-- Exact bytes while they fit the column; a magnitude past that. +-- +-- `%10d` holds ten digits, so a file of 10 GB or more (VM images, core +-- dumps --- ordinary things) widens the field and shifts mtime and name +-- right on that line alone. That is only cosmetic today, but +-- `_layout.NAME_START` is exported as a contract and Stage 3's +-- column-classifying intercept is planned against these constants, so a +-- line that violates them now is a Stage 3 trap. Same discipline as +-- `fmt_mtime`: the width is the invariant, and precision yields to it. +-- +-- This is NOT the deferred human-readable size column (§13): the exact +-- byte count is still what a listing shows, right up to the point where +-- it cannot be shown at all. +local SIZE_UNITS = { "K", "M", "G", "T", "P", "E" } + local function fmt_size(n) - return string.format("%" .. SIZE_BYTES .. "d", n) + local exact = string.format("%" .. SIZE_BYTES .. "d", n) + if #exact <= SIZE_BYTES then return exact end + local value, unit = n, SIZE_UNITS[#SIZE_UNITS] + for _, suffix in ipairs(SIZE_UNITS) do + value = value / 1024 + unit = suffix + if value < 1024 then break end + end + local scaled = string.format("%.1f%s", value, unit) + if #scaled > SIZE_BYTES then scaled = scaled:sub(1, SIZE_BYTES) end + return string.rep(" ", SIZE_BYTES - #scaled) .. scaled end local function fmt_mtime(secs) @@ -348,6 +378,13 @@ end -- Re-seat by BASENAME (Q#DR9), falling back to the nearest surviving -- line. Every repaint is wholesale, so without this a revert, a sort, -- or any Stage 2 operation would drop the cursor to the header. +-- +-- `move_to_line` is AMBIENT --- it moves the active window's cursor, not +-- `handle.buf`'s --- so every caller that can run after an `:await()` +-- has to check that dired is still the active buffer first. Painting is +-- safe either way (it names the buffer); seating is not. Callers that +-- activate the buffer themselves (an open, which displays first) are +-- unconditionally in the right place. local function seat_cursor(handle, name, fallback_line) local count = #handle.entries if count == 0 then @@ -416,7 +453,8 @@ end -- Buffer ownership -- --------------------------------------------------------------------------- -local READ_ONLY_LIMIT = 99 +-- How far the `<2>`, `<3>`, ... disambiguation walks before giving up. +local NAME_VARIANT_LIMIT = 99 -- `pmacs.buffer.create` takes any caller-chosen name, so a foreign -- buffer may already be called `*dired:/tmp*`. Painting into it through @@ -435,7 +473,7 @@ local function claim_handle(path) local name = buffer_name(path) if buffer_named(name) then local unique = nil - for i = 2, READ_ONLY_LIMIT do + for i = 2, NAME_VARIANT_LIMIT do local candidate = string.format("%s<%d>", name, i) if buffer_named(candidate) == nil then unique = candidate @@ -680,19 +718,20 @@ pmacs.command.define { return end if entry.kind == "symlink" then - -- `read_dir`/`stat` are lstat-based, so the only way to learn - -- whether a link points at a directory is to try to list it. A - -- symlinked directory is an ordinary thing to walk into, and the - -- probe costs one syscall on symlink lines only. + -- `read_dir` and `stat` are both lstat-based, so nothing in the + -- entry says whether the link points at a directory --- the only + -- way to find out is to try to list it. A symlinked directory is + -- an ordinary thing to walk into, so try the descent and fall back + -- to a file visit. + -- + -- `open_directory` is the try: it reads before touching any editor + -- state and raises having changed nothing (acceptance 15), so its + -- failure IS the "not a directory" answer. An explicit probe + -- followed by the real open would list the whole directory TWICE + -- --- opendir plus one lstat per child, each time. pmacs.async(function() - local ok = pcall(function() - return pmacs.fs.read_dir(target, { tolerant = true }):await() - end) - if ok then - local descended, err = pcall(open_directory, target, nil, handle) - if not descended then report("dired", err) end - return - end + local descended = pcall(open_directory, target, nil, handle) + if descended then return end local visited, err = pcall(pmacs.window.display_file, target, { select = true }) if not visited then report("dired", err) end end) @@ -742,7 +781,14 @@ pmacs.command.define { handle.entries = entries handle.errors = errors paint(handle) - seat_cursor(handle, name, line) + -- The re-read settles a tick or more later, and the user may have + -- left (a buffer switch, or `q`) in the meantime. The paint names + -- its buffer and is safe; seating is ambient, so a stale seat here + -- would move an unrelated buffer's cursor to a line index that + -- only means something in this listing. + if pmacs.window.buffer() == handle.buf then + seat_cursor(handle, name, line) + end end) end, } diff --git a/src/fs.rs b/src/fs.rs index 1767234..0e05cbc 100644 --- a/src/fs.rs +++ b/src/fs.rs @@ -42,6 +42,28 @@ use crate::worker::CancellationToken; /// directories. const READDIR_CANCEL_POLL_EVERY: usize = 32; +/// How many *consecutive* `readdir` iterator errors a tolerant listing +/// records before giving up and failing (dired Q#DR6). +/// +/// [`std::fs::ReadDir`] is not obliged to terminate after yielding an +/// `Err`: a directory pulled out from under a stalled network mount can +/// keep producing them. Tolerant mode records-and-continues, so without +/// a bound that is an unbounded error vector on a worker thread. +/// +/// Cancellation is **not** an adequate backstop here, which is the +/// reason this constant exists rather than a comment saying it is: a +/// dired listing carries no supersede key and nothing cancels it, so the +/// only thing that would stop the loop is the directory itself. A +/// directory whose iterator produces nothing but errors has no partial +/// answer worth rendering, so the listing fails with the last error the +/// way an unopenable directory does. +/// +/// Deliberately untested: forcing a real `readdir` to yield errors +/// repeatedly is not portable, and faking it would need the walk to be +/// generic over its iterator — a refactor with no other consumer. The +/// counter resets on any entry that materializes. +const READDIR_MAX_CONSECUTIVE_ENTRY_ERRORS: usize = 1024; + /// One directory entry as returned by [`read_dir_blocking`]. /// /// The shape is what `dired` / `magit-class` / `outline-class` @@ -277,6 +299,7 @@ pub fn read_dir_blocking( let mut errors: Option> = matches!(tolerance, ReadDirTolerance::PerEntry).then(Vec::new); let parent_str = path.display().to_string(); + let mut consecutive_entry_errors = 0usize; for (i, entry_result) in iter.enumerate() { if i % READDIR_CANCEL_POLL_EVERY == 0 && cancel.is_cancelled() { return Err(FsError::Cancelled); @@ -286,17 +309,19 @@ pub fn read_dir_blocking( Err(source) => { // R2-2: the entry never materialized, so there is no // name to report and the error names the parent. - record_entry_error( - &mut errors, - None, - FsError::Io { - path: parent_str.clone(), - source, - }, - )?; + let error = FsError::Io { + path: parent_str.clone(), + source, + }; + consecutive_entry_errors += 1; + if consecutive_entry_errors > READDIR_MAX_CONSECUTIVE_ENTRY_ERRORS { + return Err(error); + } + record_entry_error(&mut errors, None, error)?; continue; } }; + consecutive_entry_errors = 0; let entry_path = entry.path(); // Resolved first so a later per-entry failure can name it. let name = path_to_utf8_string(&entry.file_name(), &parent_str)?; diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 4314e18..d689f3f 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -3574,6 +3574,13 @@ impl UserData for AnsiParserLua { /// an edge (`//tmp`, `~` with `HOME` unset, a `..` that would escape /// root) would mint two buffers for one directory with no error /// anywhere. +/// +/// The result crosses the boundary through `to_string_lossy`, so a +/// non-UTF-8 `$HOME` (or a non-UTF-8 argument) can yield a Lua string +/// that no longer names the `PathBuf` the registry keys on. That is the +/// same limit `pmacs.fs` already documents — byte-preserving paths are +/// post-v0.1 work that widens every path in the API — and it is recorded +/// here so this binding is not read as an exception to it. fn install_path_module(lua: &Lua) -> mlua::Result
{ let path = lua.create_table()?; path.set( diff --git a/src/workers_buffer.rs b/src/workers_buffer.rs index 863f4ed..6a6eeb4 100644 --- a/src/workers_buffer.rs +++ b/src/workers_buffer.rs @@ -207,10 +207,10 @@ fn format_outcome(outcome: &JobOutcome) -> String { // tolerant listing that dropped half a directory is not the // same observable outcome as a clean one. match listing.errors.as_deref() { - Some([_, ..]) => format!( + Some(errors @ [_, ..]) => format!( "ok ({} entries, {} unreadable)", listing.entries.len(), - listing.errors.as_ref().map_or(0, Vec::len) + errors.len() ), _ => format!("ok ({} entries)", listing.entries.len()), } diff --git a/tests/dired_acceptance.rs b/tests/dired_acceptance.rs index 73b06f8..7d0f2c5 100644 --- a/tests/dired_acceptance.rs +++ b/tests/dired_acceptance.rs @@ -417,6 +417,75 @@ fn dired_renders_a_header_and_one_line_per_entry() { ); } +/// The columns are a CONTRACT, not a formatting preference: `_layout` is +/// exported and Stage 3's column-classifying intercept is planned +/// against it. A size that does not fit ten digits (10 GB and up — VM +/// images, core dumps) must therefore yield precision rather than width, +/// the way `fmt_mtime` already does. Without that, one line's mtime and +/// name shift right and nothing notices until Stage 3. +#[test] +fn dired_keeps_its_columns_when_a_size_exceeds_the_field() { + let td = tempfile::tempdir().expect("tempdir"); + std::fs::write(td.path().join("small.txt"), b"x").expect("write small"); + let huge = td.path().join("huge.img"); + // Sparse: `set_len` allocates nothing on any filesystem pmacs + // supports. If one refuses, the premise cannot be established. + let file = std::fs::File::create(&huge).expect("create huge"); + if file.set_len(12_000_000_000).is_err() { + eprintln!("filesystem refused a sparse 12 GB file; skipping"); + return; + } + drop(file); + let reported = std::fs::metadata(&huge).expect("stat huge").len(); + assert!( + reported > 9_999_999_999, + "fixture premise: the size must exceed ten digits, got {reported}" + ); + + let mut s = editor(); + open_ok(&mut s, td.path(), "nil"); + let size_start = layout(&s, "SIZE_START"); + let mtime_start = layout(&s, "MTIME_START"); + let name_start = layout(&s, "NAME_START"); + + let lines = active_lines(&s); + for name in ["huge.img", "small.txt"] { + let line = &lines[line_of(&s, name)]; + let size = &line[size_start..mtime_start - 1]; + assert_eq!( + size.len(), + 10, + "the size field must stay ten columns wide: {line:?}" + ); + let stamp = &line[mtime_start..name_start - 1]; + assert!( + stamp.starts_with("20") && stamp.contains(':'), + "so the mtime still starts where the layout says: {line:?}" + ); + assert_eq!( + line_name(&s, line), + name, + "and the name still starts at NAME_START" + ); + } + + // The oversized value degrades to a magnitude rather than a + // placeholder, so the listing still says how big the file is. + let huge_line = &lines[line_of(&s, "huge.img")]; + let size = huge_line[size_start..mtime_start - 1].trim(); + assert!( + size.ends_with('G') || size.ends_with('T'), + "an oversized size keeps its magnitude: {size:?}" + ); + // A size that DOES fit stays exact. + let small_line = &lines[line_of(&s, "small.txt")]; + assert_eq!( + small_line[size_start..mtime_start - 1].trim(), + "1", + "a size that fits is still the exact byte count" + ); +} + // --------------------------------------------------------------------------- // 2 --- visit dispatches on kind, through the panel-safe primitive // --------------------------------------------------------------------------- @@ -474,6 +543,54 @@ fn dired_visit_dispatches_on_entry_kind() { ); } +/// A symlink's kind is `"symlink"` in both `read_dir` and `stat` (both +/// are lstat-based), so nothing in the entry says what it points at. +/// `RET` therefore tries the descent and falls back to a file visit — +/// one read, since `open_directory` reads before touching any editor +/// state and its failure *is* the "not a directory" answer. +#[test] +fn dired_visit_follows_a_symlink_to_the_kind_of_its_target() { + let td = fixture_dir(); + std::os::unix::fs::symlink("subdir", td.path().join("linkdir")).expect("symlink to dir"); + + let mut s = editor(); + open_ok(&mut s, td.path(), "nil"); + + // A symlink to a directory descends. The path is NOT resolved + // (canonicalization is lexical), so the buffer names the way the user + // navigated — Emacs parity. + seat_on(&s, "linkdir"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + assert_eq!( + active_name(&s), + format!("*dired:{}*", canon(&td.path().join("linkdir"))), + "a symlinked directory descends under the path we walked" + ); + assert_eq!( + line_name(&s, &active_lines(&s)[1]), + "inner.txt", + "and shows the target directory's contents" + ); + + // A symlink to a file opens the file. + type_char(&mut s, '^'); + pump(&mut s); + seat_on(&s, "link"); + press(&mut s, KeyCode::Enter); + pump(&mut s); + let path = active_path(&s).expect("a file must be open"); + assert!( + path.ends_with("/link"), + "the visit keeps the link's own path; got {path}" + ); + assert_eq!( + eval::(&s, "return pmacs.window.buffer():slice(0, 5)"), + "hello", + "with the target's contents" + ); +} + /// The panel case, which is the real assertion (Q#DR10): with dired /// displayed as a panel, `RET` on a file leaves the dired panel alive /// and puts the file in the document window. Falsified by swapping @@ -1018,6 +1135,68 @@ fn dired_revert_reseats_the_cursor_by_basename() { ); } +/// A revert settles a tick or more later, and the user may have left in +/// the meantime. `pmacs.editor.move_to_line` is **ambient** — it moves +/// whatever window is active — so an unguarded re-seat moves an +/// unrelated buffer's cursor to a line index that only means something +/// in the dired listing. The paint is safe either way because it names +/// its buffer; this pins the half that does not. +#[test] +fn dired_revert_does_not_seat_a_buffer_the_user_switched_to() { + let td = tempfile::tempdir().expect("tempdir"); + for name in ["a.txt", "b.txt", "c.txt", "d.txt", "e.txt"] { + std::fs::write(td.path().join(name), b"x").expect("write"); + } + let notes = td.path().join("notes.txt"); + std::fs::write(¬es, b"one\ntwo\nthree\nfour\nfive\nsix\n").expect("write notes"); + + let mut s = editor(); + open_ok(&mut s, td.path(), "nil"); + exec(&s, "_G.DIRED_BUF = pmacs.window.buffer()"); + // A late line, so a stale seat would be visible in the other buffer. + seat_on(&s, "e.txt"); + let dired_line = cursor_line(&s); + assert!( + dired_line >= 4, + "fixture premise: a late line, got {dired_line}" + ); + + // Start the revert, then leave BEFORE the read settles. + type_char(&mut s, 'g'); + exec( + &s, + &format!( + "pmacs.buffer.find_or_open({:?})", + notes.display().to_string() + ), + ); + assert_eq!(cursor_line(&s), 0, "a freshly opened file starts at line 0"); + pump(&mut s); + + assert_eq!( + active_path(&s).map(PathBuf::from), + Some(PathBuf::from(canon(¬es))), + "the switch stands: the revert must not pull the user back" + ); + assert_eq!( + cursor_line(&s), + 0, + "and it must not move the cursor of the buffer they moved to" + ); + + // The revert itself still happened: the dired buffer is repainted, + // and returning to it seats normally on the next command. + std::fs::write(td.path().join("f.txt"), b"x").expect("write f"); + exec(&s, "pmacs.window.switch_buffer(_G.DIRED_BUF)"); + type_char(&mut s, 'g'); + pump(&mut s); + assert!( + active_text(&s).contains("f.txt"), + "the dired buffer still reverts when it is the active one: {:?}", + active_text(&s) + ); +} + // --------------------------------------------------------------------------- // 8 --- sort modes // --------------------------------------------------------------------------- From 14881b26c079588a50ce9080309aa02feacac4ab Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 15:24:25 -0400 Subject: [PATCH 5/7] docs: record dired Stage 1 review round 1 Framing rev 7 adds S1-10..S1-12 -- the three findings that changed behavior, each stated as the durable lesson rather than as a diff: painting takes a buffer and seating takes the world, so any post-await cursor operation needs an active-buffer guard; the rendered columns are a contract Stage 3 is planned against, so precision yields to width; and `open_directory`'s changed-nothing-on-failure invariant is itself a probe, which is why the symlink descent no longer lists the target twice. Plus the tolerant-channel note: cancellation was never a backstop for a dired listing, because nothing cancels one. The ledger records the round, the updated counts (dired 25 + 25 CRDT, sweep 3,189 across 92), and the process lesson that cost me the fixes once: a mutation-bite helper restores with `git checkout --`, so a fix must be committed before it is bitten. --- docs/active-work.md | 20 ++++++++++++++++-- docs/dired-framing.md | 48 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index b83a1bc..2897d07 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -206,10 +206,26 @@ If it does not, stop and repair the remote/fetch configuration. fail the test that names it. `dired.lua` is new, so `scripts/bite`'s file swap does not apply; every mutation was applied and reverted with `git checkout --`. One came back VACUOUS and is recorded above. +- **Review round 1 addressed** (framing rev 7, S1-10…S1-12). Three + behavioral fixes, each bite-verified: `dired.revert`'s re-seat is + guarded on the active buffer (an ambient `move_to_line` after an await + moved an unrelated buffer's cursor — the buffer-level instance of + S1-9); `fmt_size` keeps the column width past ten digits, because + `_layout` is a contract Stage 3 is planned against; and the symlink + descent dropped its probe, since `open_directory`'s + changed-nothing-on-failure invariant *is* the probe (it was listing the + target directory twice). Plus a consecutive-`readdir`-error cap, because + **nothing cancels a dired listing** — it carries no supersede key, so + cancellation was never the backstop the tolerant loop implicitly relied + on. Naming/comment findings taken as-is. + - Durable process lesson, hit twice now: a mutation-bite helper restores + with `git checkout --`, which reverts to **HEAD** — so a fix must be + committed *before* it is bitten. Round 1's fixes were briefly wiped by + exactly that. - Verification on this branch: `cargo fmt --check` clean; strict workspace Clippy clean; 1,829 default + 2,006 CRDT library tests; dired acceptance - 22 default + 22 CRDT; m8_1 10 / m8_2 15 / m8_3 32 unchanged; M4 121; - required GPU 155; **isolated-`XDG_CONFIG_HOME` workspace sweep 3,186 + **25 default + 25 CRDT**; m8_1 10 / m8_2 15 / m8_3 32 unchanged; M4 121; + required GPU 155; **isolated-`XDG_CONFIG_HOME` workspace sweep 3,189 passed across 92 suites, zero failures**; `git diff --check` clean. The sweep needs the isolated config for the reason recorded in the bottom-panel lane below. diff --git a/docs/dired-framing.md b/docs/dired-framing.md index 3f6b8a5..ada853e 100644 --- a/docs/dired-framing.md +++ b/docs/dired-framing.md @@ -1,13 +1,14 @@ # Dired — framing -**Revision 6 — 2026-07-25. Status: APPROVED; Stage 0 MERGED as #162; -Stage 1 IN REVIEW as PR #165.** +**Revision 7 — 2026-07-25. Status: APPROVED; Stage 0 MERGED as #162; +Stage 1 IN REVIEW as PR #165, review round 1 addressed.** Rev 1 passed a ground-truth review; rev 2 fixed round 1's seven findings; rev 3 fixed round 2's six and was approved; rev 4 recorded what Stage 0's implementation falsified in the approved text (§0); rev 5 adds the **coherence impact** statement now required of every framing (`CLAUDE.md`, `COHERENCE.md` §20) — see §0.5; rev 6 records what Stage -1's implementation falsified (§0, S1-1…S1-9). Deliberately +1's implementation falsified (§0, S1-1…S1-9); rev 7 adds what its first +review round found (§0, S1-10…S1-12). Deliberately unnumbered: the roadmap's Arc 8 is GPU structural parity but `docs/lean4-mode-framing.md` also claims Arc 8, so the arc space is already forked in uncommitted work. (Rev 2 also cited @@ -293,6 +294,47 @@ in the code, per the rev-4 precedent. surface takes no frontend argument) and named here rather than discovered later. +### Stage 1 review round 1 (rev 6 → rev 7) + +Three findings changed behavior; the rest were naming and comments. Each +fix is bite-verified against the test that names it. + +- **S1-10. An ambient re-seat is not safe after an await.** `dired.revert` + painted its own buffer by name (safe) and then re-seated through + `pmacs.editor.move_to_line`, which moves whatever window is + **active** — so a user who switched buffers while the re-read was in + flight had an unrelated buffer's cursor moved to a line index + meaningful only in the dired listing. This is the buffer-level instance + of the hazard S1-9 named at the frontend level, and it generalizes: in + this codebase, *painting takes a buffer and seating takes the world*. + Any post-await cursor operation needs an active-buffer guard; + `open_directory` is exempt only because it displays the buffer first. +- **S1-11. The rendered columns are a contract, so precision yields to + width.** `%10d` overflowed at 10 GB (VM images, core dumps), widening + the size field and shifting mtime and name right on that line alone. + Cosmetically harmless today, but `_layout` is exported and Stage 3's + column-classifying intercept is planned against it, so a + contract-violating line now is a Stage 3 trap. `fmt_size` took + `fmt_mtime`'s shape: exact bytes while they fit, else a fixed-width + magnitude. Not the deferred human-readable column (§13) — the exact + count still renders right up to the point where it cannot. +- **S1-12. `open_directory`'s "changed nothing on failure" invariant is + reusable as a PROBE.** S1-8's symlink descent originally listed the + target to learn its kind and then opened it — two full listings of the + same directory. Because a failed open touches no editor state + (acceptance 15), the open itself is the probe: try the descent, fall + back to `display_file`. One read. The comment that claimed "one + syscall" for a full `read_dir` is corrected rather than left as a + cost claim nobody would re-check. + +Also, on the tolerant channel (Q#DR6): a `readdir` iterator may keep +yielding errors without terminating, and **cancellation is not a backstop +for a dired listing** — it carries no supersede key, so nothing cancels +it. A consecutive-error cap now fails the listing the way an unopenable +directory fails, rather than accumulating error rows on a worker thread. +It is deliberately untested: faking a failing iterator would need the +walk generic over it, a refactor with no other consumer. + ## 0.5. Coherence impact (`COHERENCE.md` §20) Required of every framing since #163. This arc was scouted and approved From 08e2807fcca236fdff5f764eb91cc5963a59adf3 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 16:14:06 -0400 Subject: [PATCH 6/7] fix(dired): correct the reporting-channel claim #161 falsified The module doc said an uncaught raise inside a `pmacs.async` coroutine "goes to *errors*, not the status line". #161's COHERENCE finding shows that is wrong, and in the worse direction: `pmacs.error` is never defined in production, so `step()`'s guarded report is dead and the raise falls through to a bare `error()` inside `pmacs._async.tick()` -- whose result `EditorState::tick_async` discards with `let _ =`. The failure reaches nowhere at all, and dired would look like it silently did nothing. So the per-coroutine `pcall` plus `pmacs.editor.set_status` is load-bearing, not tidy, and the doc now says which channel is dead, which is live, and that the acceptance suite observes the live one -- the corollary COHERENCE draws from that finding. The ledger records the integration, the reruns on the merged tree, and the ops lesson that cost three CI runs: a conflicting PR has no merge ref, so GitHub creates no `pull_request` run and nothing reports the absence. --- builtin/runtime/dired.lua | 27 ++++++++++++++++++++++----- docs/active-work.md | 33 ++++++++++++++++++++++++++------- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/builtin/runtime/dired.lua b/builtin/runtime/dired.lua index ceaf3d5..9c6bc92 100644 --- a/builtin/runtime/dired.lua +++ b/builtin/runtime/dired.lua @@ -48,11 +48,28 @@ -- 3. EVERY LISTING IS ASYNC. `pmacs.fs.read_dir` is worker-dispatched, -- so each command spawns a coroutine and the work after the first -- `:await()` resumes on a later tick --- outside interactive --- dispatch. Two consequences: errors must be `pcall`ed and reported --- here (an uncaught raise inside `pmacs.async` goes to *errors*, not --- the status line), and `pmacs.window.*` calls made after the await --- act for the *ambient* active frontend, since interactive origin --- does not survive the tick boundary. +-- dispatch. Three consequences: +-- +-- * Errors MUST be `pcall`ed and reported here, and that is +-- load-bearing rather than tidy. An uncaught raise inside a +-- `pmacs.async` coroutine reaches `step()`, which reports through +-- `pmacs.error` --- a channel that **is never defined in +-- production** (`COHERENCE.md` §1.1) --- and so falls through to a +-- bare `error()` inside `pmacs._async.tick()`, whose result +-- `EditorState::tick_async` discards with `let _ =`. The failure +-- would not reach the status line, the `*errors*` buffer, or a log: +-- it would reach nowhere, and dired would look like it silently did +-- nothing. +-- * Reporting therefore goes through `pmacs.editor.set_status`, which +-- exists and which the acceptance suite observes --- the corollary +-- COHERENCE draws from that dead channel: report through a surface +-- a test can see, or the guard is indistinguishable from the +-- silence it was meant to fix. +-- * `pmacs.window.*` calls made after the await act for the *ambient* +-- active frontend, since interactive origin does not survive the +-- tick boundary; and `pmacs.editor.move_to_line` acts on the +-- ambient *buffer*, which is why every post-await re-seat is +-- guarded (see `seat_cursor`). -- Emacs 28's dired-kill-when-opening-new-dired-buffer, as a setting -- rather than a hardcoded policy: buffer-per-directory accumulates diff --git a/docs/active-work.md b/docs/active-work.md index 565375b..76a65fd 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -286,13 +286,32 @@ If it does not, stop and repair the remote/fetch configuration. with `git checkout --`, which reverts to **HEAD** — so a fix must be committed *before* it is bitten. Round 1's fixes were briefly wiped by exactly that. -- Verification on this branch: `cargo fmt --check` clean; strict workspace - Clippy clean; 1,829 default + 2,006 CRDT library tests; dired acceptance - **25 default + 25 CRDT**; m8_1 10 / m8_2 15 / m8_3 32 unchanged; M4 121; - required GPU 155; **isolated-`XDG_CONFIG_HOME` workspace sweep 3,189 - passed across 92 suites, zero failures**; `git diff --check` clean. The - sweep needs the isolated config for the reason recorded in the - bottom-panel lane below. +- **Canonical main integrated at `46a1b8f`** (multi-root LSP affinity + #161), merged rather than rebased per the #135/#137 precedent so the + review anchors stay addressable. Two things worth carrying: + - **A conflicting PR silently stops running CI.** GitHub builds + `pull_request` runs against the merge ref, which does not exist while + the PR conflicts, so no run is created and nothing reports a + failure — the checks list simply stays as it was. Three pushes to + this branch produced no CI at all before the cause was found. Watch + `mergeable` on a long-lived lane, not just the check list. + - #161's own COHERENCE finding **falsified a claim in this lane's + module doc**: `pmacs.error` is never defined in production, so an + uncaught raise inside a `pmacs.async` coroutine does not reach + `*errors*` as the comment said. It reaches a bare `error()` inside + `pmacs._async.tick()`, whose result `tick_async` discards with + `let _ =` — i.e. nowhere. That makes dired's per-coroutine `pcall` + + `set_status` load-bearing rather than tidy, and the comment now says + so. +- Verification on the merged tree: `cargo fmt --check` clean; strict + workspace Clippy clean; 1,829 default + 2,006 CRDT library tests; dired + acceptance **25 default + 25 CRDT**; m8_1 10 / m8_2 15 / m8_3 32 + unchanged; multi-root 13 (main's new suite, green under this lane's + `mod.rs` changes); M4 121; required GPU 155; + **isolated-`XDG_CONFIG_HOME` workspace sweep 3,202 passed across 93 + suites, zero failures**; `git diff --check` clean. The sweep needs the + isolated config for the reason recorded in the bottom-panel lane + below. - Coherence (framing §0.5, required since #163): serves `COHERENCE.md` §20 Priority 1, which names this work explicitly; journey step 7's file half goes from no surface to a surface; **adds no interaction island** — keys From b3c8230a84be37cd1a1aecc91f31b2e9477246dc Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 16:44:57 -0400 Subject: [PATCH 7/7] docs: record the second main integration and its gate rerun Main advanced twice inside one review round (#161, then #166), the second landing while the first integration's sweep was still running. The ledger now names both integrations, how each doc conflict was resolved, and the verification numbers for the twice-merged tree -- plus the lesson that a lane in review against a fast-moving main reruns its gates per integration, not per push. --- docs/active-work.md | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 608d527..c934c30 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -286,9 +286,13 @@ If it does not, stop and repair the remote/fetch configuration. with `git checkout --`, which reverts to **HEAD** — so a fix must be committed *before* it is bitten. Round 1's fixes were briefly wiped by exactly that. -- **Canonical main integrated at `46a1b8f`** (multi-root LSP affinity - #161), merged rather than rebased per the #135/#137 precedent so the - review anchors stay addressable. Two things worth carrying: +- **Canonical main integrated twice** — at `46a1b8f` (multi-root LSP + affinity #161) and again at `b889873` (GPU terminal input #166), both + merged rather than rebased per the #135/#137 precedent so the review + anchors stay addressable. Each conflict was a single doc hunk resolved + as the union: this lane owns COHERENCE's journey step 7 file half, #161 + owns the in-flight list, #166 owns step 8's GPU-terminal addendum. + Three things worth carrying: - **A conflicting PR silently stops running CI.** GitHub builds `pull_request` runs against the merge ref, which does not exist while the PR conflicts, so no run is created and nothing reports a @@ -303,12 +307,18 @@ If it does not, stop and repair the remote/fetch configuration. `let _ =` — i.e. nowhere. That makes dired's per-coroutine `pcall` + `set_status` load-bearing rather than tidy, and the comment now says so. -- Verification on the merged tree: `cargo fmt --check` clean; strict - workspace Clippy clean; 1,829 default + 2,006 CRDT library tests; dired - acceptance **25 default + 25 CRDT**; m8_1 10 / m8_2 15 / m8_3 32 - unchanged; multi-root 13 (main's new suite, green under this lane's - `mod.rs` changes); M4 121; required GPU 155; - **isolated-`XDG_CONFIG_HOME` workspace sweep 3,202 passed across 93 + - **A lane in review against a fast-moving `main` needs its gates rerun + per integration, not per push.** Main advanced twice inside this + review round, and the second time landed while the first + integration's sweep was still running. The numbers below describe the + twice-merged tree. +- Verification on the twice-merged tree (`main` @ `b889873`): + `cargo fmt --check` clean; strict workspace Clippy clean; **1,832 + default + 2,009 CRDT** library tests; dired acceptance **25 default + + 25 CRDT**; m8_1 10 / m8_2 15 / m8_3 32 unchanged; multi-root 13 and + vterm Stage 3 5 (both suites main added, green under this lane's + `mod.rs` and `editor.rs` changes); M4 121; required GPU 155; + **isolated-`XDG_CONFIG_HOME` workspace sweep 3,205 passed across 93 suites, zero failures**; `git diff --check` clean. The sweep needs the isolated config for the reason recorded in the bottom-panel lane below.