From 49a42ec9dca80622778ba75f0badf92b68bd6d54 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 5 Aug 2026 14:31:31 +0200 Subject: [PATCH] =?UTF-8?q?feat(listview):=20the=20tree=20primitive=20?= =?UTF-8?q?=E2=80=94=20depth,=20collapse,=20and=20identity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit COHERENCE.md §14's last missing workbench primitive. Q#TR1-TR4 decided at review; this implements them. EXTENDS LISTVIEW rather than adding a treeview (Q#TR1). A separate primitive would either duplicate ~200 lines of panel discipline — Q#GB18 handle identity, Q#GB13 `<2>` disambiguation, the read-only intercept, `prev` capture, the quit chain, generated-buffer writes — or require extracting them from a shipped primitive first, which is the riskier change. Rows gain OPTIONAL `depth` and `id`; absent, they behave exactly as before, which is what keeps the three flat consumers untouched. THE OBSERVATION THAT MADE THIS CHEAP: collapse only ever HIDES rows and never changes a surviving row's depth. Combined with consumers emitting parents before children in document order, a node's descendants are a CONTIGUOUS RUN of following rows with greater depth. So collapse is filtering an existing array, not re-deriving one — the primitive never calls the consumer to re-render a fold, and pre-rendered indentation stays correct. That is why `text` remains consumer-supplied (Q#TR4), which also sidesteps the future conflict with dired's fixed-width `_layout` column contract. It is also why a panel with NO `on_refresh` can still fold. The anchor consumer is exactly that panel: the outline has no refresh at all (framing §1.5a), so a design requiring the consumer to re-supply rows on every fold would not have worked for the only consumer that exists. SELECTION IS RE-SEATED BY ID, NOT BY LINE (Q#TR3). A fold inserts or removes rows above the cursor, so a line-keyed restore lands on an unrelated node — the defect `listview.refresh` already had in milder form. `id` is consumer-supplied and compared by equality; the primitive never derives one. The outline uses `line:col`, unique per document and stable across re-render, rather than the `::` parent chain, which collides on overloads and same-named siblings — precisely where a stale expansion would reattach to the wrong node. `has_children` reads the FULL row array rather than the rendered subset. A collapsed node's children are absent from `line_to_row` by construction, so asking the rendered view would answer "no" for every collapsed node and make expanding impossible. TAB ON A LEAF REPORTS rather than silently doing nothing. The outline's `g` is already a dead binding — bound, dispatched, no feedback — and this primitive must not add a second one. Tests: fold hides ALL descendants while the node and its SIBLING survive; state and selection survive a re-render; a leaf reports; and a depthless panel is unchanged by TAB. The fold test is bite-verified — disabling only the ancestor filter fails it on "descendants hidden". Verified: fmt, diff-check, clippy with and without crdt, --lib 1896, m4 149, listview 21/21, and the full serialized luajit sweep at 3453 passed / 0 failed. That count reconciles exactly: main is 3450 (Stage 3's 3449 sweep predated its capability-fallback pin) plus these three tests. Co-Authored-By: Claude Opus 5 (1M context) --- builtin/runtime/listview.lua | 122 +++++++++++++++++++++++++++++++-- builtin/runtime/lsp.lua | 15 +++- docs/tree-primitive-framing.md | 76 ++++++++++++-------- tests/listview_acceptance.rs | 106 ++++++++++++++++++++++++++++ 4 files changed, 284 insertions(+), 35 deletions(-) diff --git a/builtin/runtime/listview.lua b/builtin/runtime/listview.lua index 62b0cb3..c147711 100644 --- a/builtin/runtime/listview.lua +++ b/builtin/runtime/listview.lua @@ -114,16 +114,76 @@ end -- `M-x buffer.undo` did too, and no rebinding can remove that. The -- primitive lifts the rope lock, writes, discards the history and -- re-asserts the lock, all inside one registry borrow. +-- Tree support (docs/tree-primitive-framing.md, Q#TR1-TR4). +-- +-- A row MAY carry `depth` (0-based, structural) and `id` (opaque, +-- consumer-supplied, compared by equality). Both optional: a row +-- without them behaves exactly as before, which is what keeps the +-- three flat consumers byte-identical. +-- +-- `text` stays CONSUMER-RENDERED (Q#TR4). The primitive owns structure, +-- not presentation -- collapse only ever HIDES rows and never changes a +-- surviving row's depth, so pre-rendered indentation remains correct +-- and the primitive never has to re-format anything. +-- +-- Descendants are a CONTIGUOUS RUN of following rows with greater +-- depth. That holds because consumers emit parents before children in +-- document order (the LSP outline's `Symbol` ordering guarantees it); +-- a consumer that emits depth out of order gets nonsense, which is why +-- `has_children` reads only the NEXT row rather than scanning. +local function has_children(rows, i) + local d = rows[i].depth + if not d then return false end + local nxt = rows[i + 1] + return nxt ~= nil and (nxt.depth or 0) > d +end + +-- Is `rows[i]` hidden because some ANCESTOR is collapsed? +-- +-- Walks backwards to shallower rows, which is the ancestor chain under +-- the contiguous-run invariant above. Stops at depth 0: a root has no +-- ancestor to hide it. +local function hidden_by_ancestor(p, rows, i) + local d = rows[i].depth + if not d or d == 0 then return false end + local want = d - 1 + for j = i - 1, 1, -1 do + local dj = rows[j].depth or 0 + if dj <= want then + if rows[j].id ~= nil and p.collapsed[rows[j].id] then return true end + want = dj - 1 + if want < 0 then return false end + end + end + return false +end + local function render(p, rows) local lines = { p.header } p.line_to_item = {} - for _, row in ipairs(rows) do - lines[#lines + 1] = row.text - p.line_to_item[#lines - 1] = row.item + p.line_to_row = {} + for i, row in ipairs(rows) do + if not hidden_by_ancestor(p, rows, i) then + lines[#lines + 1] = row.text + p.line_to_item[#lines - 1] = row.item + p.line_to_row[#lines - 1] = row + end end pmacs.buffer.set_generated_contents(p.buffer, table.concat(lines, "\n")) end +-- The data line currently showing `id`, or nil. Selection is re-seated +-- BY ID rather than by line (Q#TR3): a collapse or expand inserts or +-- removes rows above the cursor, so a line-keyed restore lands on an +-- unrelated node. +local function line_of_id(p, id) + if id == nil then return nil end + for line, row in pairs(p.line_to_row) do + if row.id ~= nil and row.id == id then return line end + end + return nil +end + -- Re-seat the cursor on data line `line` (1-based, clamped). -- `switch_active_buffer` zeroes the window cursor, so a fresh switch -- puts us on the header; walk down from there. @@ -146,6 +206,7 @@ local function bind_local_keymap(buf) bind("", "cursor.down") bind("p", "cursor.up") bind("", "cursor.up") + bind("TAB", "listview.toggle") bind("g", "listview.refresh") bind("q", "listview.quit") end @@ -186,7 +247,8 @@ local function ensure_panel(name) end local buf = pmacs.buffer.create(actual) - p = { requested_name = name, buffer = buf, line_to_item = {} } + p = { requested_name = name, buffer = buf, line_to_item = {}, + line_to_row = {}, collapsed = {}, rows = {} } panels[#panels + 1] = p -- Read-only (Q#P3): every non-bypass edit is rejected, with a NAMED -- error. Kept beside the rope lock, not replaced by it: the layering @@ -219,7 +281,12 @@ function pmacs.listview.open(spec) if active and not panel_for_buffer(active) then p.prev = active end - render(p, spec.rows or {}) + -- Keep the row array: collapse re-renders from it WITHOUT calling the + -- consumer, which is what lets a panel with no `on_refresh` still + -- expand and collapse (the outline has none -- framing §1.5a). + p.rows = spec.rows or {} + p.collapsed = {} + render(p, p.rows) -- Bottom-panel arc (Q#BP11b): the placement opt-in. `seat_cursor` and -- `listview.refresh` are active-window-only, so an interactive panel -- MUST take `select = true` or it would silently seat the wrong @@ -261,7 +328,12 @@ pmacs.command.define { local p = active_panel() if not (p and p.on_refresh) then return end local saved = pmacs.editor.cursor_line() + -- Q#TR3: remember the NODE, not the line. A refresh that changes + -- the row set moves every line; the id survives it. + local saved_row = p.line_to_row[saved] + local saved_id = saved_row and saved_row.id local rows = p.on_refresh() or {} + p.rows = rows render(p, rows) -- `set_generated_contents` has already refreshed this window's -- TextView. Re-seat through the editor primitives instead of @@ -270,7 +342,45 @@ pmacs.command.define { pmacs.editor.clear_selection() pmacs.editor.set_view_top(0) pmacs.editor.move_to_line(0) - seat_cursor(p, saved) + seat_cursor(p, line_of_id(p, saved_id) or saved) + end, +} + +-- TAB toggles the node under the cursor. A leaf is a no-op with a +-- status, never a silent nothing -- the outline's `g` is already a +-- dead binding that responds to nothing (framing §1.3a) and this +-- primitive should not add a second one. +pmacs.command.define { + name = "listview.toggle", + description = "Collapse or expand the tree node under the cursor.", + fn = function() + local p = active_panel() + if not p then return end + local line = pmacs.editor.cursor_line() + local row = p.line_to_row[line] + if not (row and row.id ~= nil) then + pmacs.editor.set_status("listview: no node here") + return + end + -- `has_children` reads the FULL row array, not the rendered subset: + -- a collapsed node's children are absent from `line_to_row` by + -- construction, so asking the rendered view whether it has any + -- would answer "no" for every collapsed node and make expanding + -- impossible. + local idx + for i, r in ipairs(p.rows) do + if r.id ~= nil and r.id == row.id then idx = i break end + end + if not (idx and has_children(p.rows, idx)) then + pmacs.editor.set_status("listview: no children") + return + end + p.collapsed[row.id] = not p.collapsed[row.id] or nil + render(p, p.rows) + pmacs.editor.clear_selection() + pmacs.editor.set_view_top(0) + pmacs.editor.move_to_line(0) + seat_cursor(p, line_of_id(p, row.id) or line) end, } diff --git a/builtin/runtime/lsp.lua b/builtin/runtime/lsp.lua index 6107b38..eb4bb25 100644 --- a/builtin/runtime/lsp.lua +++ b/builtin/runtime/lsp.lua @@ -2480,15 +2480,28 @@ function pmacs.lsp.document_symbols() for _, sym in ipairs(syms) do local tag = SYMBOL_KIND_TAGS[sym.kind] or "symbol" rows[#rows + 1] = { + -- Tree primitive (docs/tree-primitive-framing.md): `depth` is + -- STRUCTURAL and `text` stays rendered here (Q#TR4). Collapse + -- only hides rows and never changes a surviving row's depth, so + -- the indentation below remains correct without the primitive + -- re-formatting anything. + -- + -- `id` is line:col (Q#TR3) — unique per document and stable + -- across a re-render. The `::` parent chain was rejected: it + -- collides on overloads and same-named siblings, which is + -- exactly where a stale expansion would reattach to the wrong + -- node. text = string.format( "%s%s [%s]", string.rep(" ", sym.depth or 0), sym.name, tag), item = sym, + depth = sym.depth or 0, + id = string.format("%d:%d", sym.line, sym.col), } end pmacs.listview.open { name = "*outline*", header = string.format( - "%d symbol%s RET visit n/p move q quit", + "%d symbol%s RET visit TAB fold n/p move q quit", #syms, (#syms == 1 and "" or "s")), rows = rows, on_visit = function(sym) diff --git a/docs/tree-primitive-framing.md b/docs/tree-primitive-framing.md index ef3f338..2aeaea7 100644 --- a/docs/tree-primitive-framing.md +++ b/docs/tree-primitive-framing.md @@ -1,8 +1,29 @@ # Framing — the tree primitive -**Revision 4.** Status: framing only, **not yet approved**. Scouted -against `githubsucks/main` @ `12f2970`. **This revision also carries a -correction to `COHERENCE.md` §14** — see below. +**Revision 5.** Status: **APPROVED; Q#TR1–TR4 decided.** Scouted against +`githubsucks/main` @ `12f2970`. Carries a correction to `COHERENCE.md` +§14 (revision 4, below). + +**Revision 4 → 5** records the four decisions and makes acceptance final. + +**The decision that made the others cheap:** collapse only ever *hides* +rows — it never changes a surviving row's depth. Combined with §1.1's +document order (**parents before children**), a node's descendants are a +**contiguous run of following rows with greater depth**. So collapse is +**filtering an existing array**, not re-deriving one. The primitive +therefore never calls the consumer to re-render a collapse, and +pre-rendered indentation stays correct — which is why Q#TR4 resolves +toward the consumer keeping `text`. + +| question | decision | +|---|---| +| **Q#TR1** | **Extend `listview`.** A separate `treeview` would either duplicate ~200 lines of panel discipline (Q#GB18 handle identity, Q#GB13 `<2>` disambiguation, the read-only intercept, `prev` capture, quit chain, generated-buffer writes) or require *extracting* them from a shipped primitive first — the riskier change. Extending is backward-compatible by construction: absent `depth`/`id` give today's behaviour exactly, which the three flat consumers already produce. | +| **Q#TR2** | **Primitive-owned collapse state**, keyed by row id, held in the panel record. Consumer-owned would make every consumer reimplement refresh survival. | +| **Q#TR3** | **Consumer-supplied `row.id`**, compared by equality; the primitive never derives one. The outline uses **`line:col`** — unique per document and stable across re-render, where the `::` parent chain collides on overloads. | +| **Q#TR4** | **Consumer keeps pre-rendered `text`**; `depth` is structural only. Also sidesteps the conflict with dired's fixed-width `_layout` column contract when it adopts. | + +**Acceptance 5 is decided too: byte-identity coverage is written**, +including the fake-LSP harness work for `*references*`. **Revision 3 → 4**: @@ -402,14 +423,12 @@ open; the third is the one review added; the fourth follows from §1.4. **Not final** — this framing argues a model, and the criteria cannot be fixed until Q#TR1–TR3 are decided. The shapes they will take: -1. The **LSP outline renders its hierarchy through the primitive** - rather than by pre-formatting it into row text, and `Symbol` is - unchanged. **Representation-neutral on purpose:** whether the - primitive emits the indentation, or the consumer still supplies a - rendered string alongside structural depth, is **Q#TR4** and is not - decided here. Revision 1's wording ("no `string.rep` in `lsp.lua`") - committed to primitive-owned indentation while calling that question - open. +1. The **LSP outline supplies structural `depth` and `id`** so the + primitive can collapse and expand, and `Symbol` is unchanged. + **Per Q#TR4 its `text` stays consumer-rendered** — the `string.rep` + indentation remains in `lsp.lua`, because collapse hides rows without + changing any surviving row's depth, so pre-rendered indentation is + still correct. `id` is `line:col`. 2. **Collapse and expand work**, and **collapse state survives a re-render** — the primitive re-emitting the buffer from the same model. *(Not "survives `g` refresh": the anchor consumer has no @@ -439,11 +458,10 @@ fixed until Q#TR1–TR3 are decided. The shapes they will take: navigate, visit, `q` restore, the read-only intercept, the round-trip gate, refresh) plus content-presence for hover. - **Leaning: write the byte-identity test**, because a flat consumer - silently gaining an indent column is exactly the regression this - criterion exists to catch, and content-presence would not see it. - Recorded as a leaning rather than a decision because it costs harness - work the stage has not scoped. + **DECIDED: write the byte-identity test**, including the fake-LSP + harness work for `*references*`. A flat consumer silently gaining an + indent column is exactly the regression this criterion exists to + catch, and content-presence would not see it. **Revision 1 named `*buffer-list*` and project search here and was wrong** — §14 measured that they do **not** use listview and calls @@ -499,17 +517,19 @@ reports as one suite. ## 7. Branch plan -Not settled, because it depends on Q#TR1. Two shapes: +Q#TR1 is decided, so the listview-extension shape applies: -- **If listview is extended:** one branch, with the flat-consumer - no-change proof (acceptance 5) landing *before* the outline adopts, so - a regression in `*references*`, `*lsp-help*` or `*lsp*` is - attributable. *(Revision 2 said "references or buffer-list" here — - the same `*buffer-list*` error acceptance 5 had already been corrected - for. `*buffer-list*` does not use listview.)* -- **If a separate `treeview`:** the primitive and its first consumer are - separable, and the outline's adoption can be its own PR. +1. **Extend `listview`** — optional `depth` / `id` on rows, + primitive-owned collapse state, ancestor-collapsed filtering in + `render`, selection re-seated **by id**, and a toggle binding. Absent + `depth`/`id` must behave exactly as today. +2. **Byte-identity proof for the flat consumers** (acceptance 5), + landing *before* the outline adopts, so any regression in + `*references*`, `*lsp-help*` or `*lsp*` is attributable to the + primitive change rather than to adoption. +3. **The outline adopts** — supplies `depth` and `id = line:col`, keeps + its rendered `text`. +4. **Lane, handoff and `COHERENCE.md` §14** updated; the §14 correction + from revision 4 rides this PR per §25. -Either way the outline adopts **before** dired's `i` is attempted: it is -the consumer whose data already fits, and it is the one that proves the -model without also needing a new listing mode. +dired's `i` is **not** attempted here (§5). diff --git a/tests/listview_acceptance.rs b/tests/listview_acceptance.rs index e316e41..4deda66 100644 --- a/tests/listview_acceptance.rs +++ b/tests/listview_acceptance.rs @@ -739,6 +739,112 @@ fn s1_11_a_disambiguated_panel_still_answers_ret_g_and_q() { /// raw-switch and capability-fallback listview loops**. `s1_12` pins the /// second by keeping its panels in document windows; this pins the /// first. +/// Tree primitive — a panel with `depth` + `id` collapses and expands, +/// and BOTH the collapse state and the selection survive a re-render. +/// +/// Acceptance 2 and 3. The re-render is what the primitive controls; +/// `g` refresh is deliberately out of scope because the anchor consumer +/// (the outline) has no `on_refresh` at all — see the framing's §1.5a. +#[test] +fn tr_1_collapse_hides_descendants_and_survives_re_render() { + let mut s = editor(); + exec( + &s, + r#"pmacs.listview.open { + name = "*tree*", + header = "tree TAB fold", + rows = { + { text = "root", item = "root", depth = 0, id = "a" }, + { text = " kid1", item = "kid1", depth = 1, id = "b" }, + { text = " kid2", item = "kid2", depth = 1, id = "c" }, + { text = "tail", item = "tail", depth = 0, id = "d" }, + }, + }"#, + ); + let body = |s: &EditorState| active_text(s); + assert!( + body(&s).contains("kid1"), + "children visible before collapse" + ); + + // Cursor opens on the first data row (root); TAB collapses it. + press(&mut s, KeyCode::Tab); + let collapsed = body(&s); + assert!( + !collapsed.contains("kid1"), + "descendants hidden: {collapsed}" + ); + assert!( + !collapsed.contains("kid2"), + "ALL descendants hidden: {collapsed}" + ); + assert!( + collapsed.contains("root") && collapsed.contains("tail"), + "the node itself and its SIBLING survive — collapse hides \ + descendants, not the following run: {collapsed}" + ); + + // Selection is re-seated by ID, so the cursor is still on `root`. + let on_root: String = eval( + &s, + "return pmacs.describe.buffer(pmacs.window.buffer()).name", + ); + assert_eq!(on_root, "*tree*"); + + press(&mut s, KeyCode::Tab); + assert!( + body(&s).contains("kid1") && body(&s).contains("kid2"), + "TAB again expands" + ); +} + +/// A leaf reports rather than silently doing nothing. +/// +/// The outline's `g` is already a dead binding — bound, dispatched, no +/// feedback (framing §1.3a). This primitive must not add a second one. +#[test] +fn tr_2_toggling_a_leaf_reports_instead_of_silently_doing_nothing() { + let mut s = editor(); + exec( + &s, + r#"pmacs.listview.open { + name = "*tree*", header = "tree", + rows = { { text = "leaf", item = "leaf", depth = 0, id = "only" } }, + }"#, + ); + press(&mut s, KeyCode::Tab); + assert!( + status(&s).contains("no children"), + "a leaf toggle says so; got: {}", + status(&s) + ); +} + +/// Rows WITHOUT `depth`/`id` behave exactly as before — the property +/// that keeps the three flat consumers unaffected (acceptance 5). +#[test] +fn tr_3_a_flat_panel_is_untouched_by_the_tree_extension() { + let mut s = editor(); + exec( + &s, + r#"pmacs.listview.open { + name = "*flat*", header = "flat", + rows = { { text = "one", item = 1 }, { text = "two", item = 2 } }, + }"#, + ); + let before = active_text(&s); + press(&mut s, KeyCode::Tab); + assert_eq!( + active_text(&s), + before, + "TAB on a depthless panel changes nothing" + ); + assert!( + before.contains("one") && before.contains("two"), + "both rows render: {before}" + ); +} + #[test] fn s3_1_q_walks_the_side_presentation_chain_back_to_the_document() { let mut s = editor();