fix(listview): flat panels keep their TAB, and a selection test that bites
FOUR REVIEW FINDINGS, and the first two were right about the tests.
1. THE SELECTION ACCEPTANCE WAS VACUOUS. `tr_1` toggles the selected
root, which sits on line 1 before and after collapsing — so it passes
unchanged under the line-based re-seating that id-keyed re-seating was
built to replace. It proves collapse hides descendants; it proves
nothing about selection. `tr_4` adds the case that discriminates: an
`on_refresh` inserts a row ABOVE the selected node, so the node moves,
and the assertion is that selection follows the NODE. Bitten by
restoring `seat_cursor(p, saved)`: `tr_4` fails with left " kid2",
right "sibling", while old `tr_1` passes — which is the finding,
reproduced.
The substantive assertion is deliberately ordered first. It was second
at one point, behind the fixture check that the node moved, and a
regression then reported as "the insert must move the selected node" —
reading like a broken fixture rather than a broken re-seat.
2. FLAT PANELS WERE NOT BEHAVIOUR-IDENTICAL. `bind_local_keymap` binds
TAB on every listview, so a depthless panel that previously fell
through to the global binding — and to Q#P3's read-only intercept —
began answering "listview: no node here". `listview.toggle` now
delegates to `buffer.tab` when no row carries an id, restoring the
prior path exactly; leaf feedback is kept for panels that really are
trees. `tr_3` asserts the absence of both tree messages rather than
merely that the panel still renders.
3 and 4 are documentation. The lane now lists 38e94dc, and no longer
says the PR is held "pending review of the documentation" that the same
commit supplied — it is held pending the decision to open it. §20 said
to BUILD the tree primitive while §14 already carried ◐; it now says
what actually remains, which is adoption: dired's `i` is the next
constraint source, DAP's variables view is why this was worth building
before them.
ONE RED, CLASSIFIED RATHER THAN RERUN AWAY. The crdt lib gate failed
`composition_overhead_under_ten_percent` at 30.7%. It is an incumbent
handoff hazard, and the branch cannot reach it — the diff versus main
touches no src/, no crate, no manifest. Alone it ran 5/5 green at
-0.6% to +0.2%; the next full run was green. Recorded in the handoff as
a MEASUREMENT, not a cause: five isolated greens establish that the
ratio is nowhere near the threshold when alone, not that contention is
what pushed it over. Not a registry row either — that file judges red
CI runs, and this was local.
Verified: fmt, clippy, diff-check, --lib 1896/0, --lib --features crdt
2081/0, listview 22/22, m4 150/0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
38e94dc33f
commit
c59de959e7
16
COHERENCE.md
16
COHERENCE.md
|
|
@ -1694,9 +1694,19 @@ and §18's floor ride on this.
|
|||
|
||||
**State: the bottom panel is DONE (§14) — both frontends, Stage 1 #155
|
||||
through Stage 2B-3, and Stage 3 flipped the adopter default so omitting
|
||||
`display` means the panel. Arc 7 is complete.** Remaining elsewhere: the tree primitive (build it before dired
|
||||
and the worker tree invent two), table/inspector/diff, help unification.
|
||||
Wiring plus one modest model piece (the tree model).
|
||||
`display` means the panel. Arc 7 is complete.**
|
||||
|
||||
**The tree primitive is IMPLEMENTED too (§14, ◐)** — `listview` carries
|
||||
optional `depth`/`id`, primitive-owned collapse, and selection re-seated
|
||||
by id. It was built before dired's recursive view and the worker tree
|
||||
could invent their own, which was this priority's stated reason for
|
||||
doing it early.
|
||||
|
||||
**What remains is ADOPTION, not construction.** The LSP outline is the
|
||||
only consumer; dired's `i` insert-subdirectory is the next real
|
||||
constraint source, and DAP's variables view is why the primitive was
|
||||
worth building first. Also remaining: table / inspector / diff, and help
|
||||
unification.
|
||||
|
||||
### Priority 6: Productize configuration
|
||||
|
||||
|
|
|
|||
|
|
@ -356,6 +356,24 @@ pmacs.command.define {
|
|||
fn = function()
|
||||
local p = active_panel()
|
||||
if not p then return end
|
||||
-- A FLAT panel must keep its pre-tree TAB behaviour exactly.
|
||||
--
|
||||
-- `bind_local_keymap` binds TAB for every listview, so this command
|
||||
-- now intercepts a key that previously fell through to the global
|
||||
-- `buffer.tab` and was refused by the Q#P3 read-only intercept.
|
||||
-- Emitting a listview status instead would be a behaviour change
|
||||
-- for the three flat consumers -- invisible to a byte-identity test,
|
||||
-- which sees the buffer and not the status line or the dispatch
|
||||
-- path. So a panel with no tree rows at all delegates.
|
||||
local is_tree = false
|
||||
for _, r in ipairs(p.rows) do
|
||||
if r.id ~= nil then is_tree = true break end
|
||||
end
|
||||
if not is_tree then
|
||||
pmacs.command.invoke("buffer.tab")
|
||||
return
|
||||
end
|
||||
|
||||
local line = pmacs.editor.cursor_line()
|
||||
local row = p.line_to_row[line]
|
||||
if not (row and row.id ~= nil) then
|
||||
|
|
|
|||
|
|
@ -163,8 +163,10 @@ why every lane below spells out the `-b` form.
|
|||
|
||||
## Tree primitive (P5) — IMPLEMENTED and GATED, PR HELD
|
||||
|
||||
**Held deliberately, not stalled.** The work is complete and green; the
|
||||
PR is not open pending review of the documentation this lane records.
|
||||
**Held deliberately, not stalled.** The work is complete, green, and
|
||||
documented — this lane, the handoff facts, and `COHERENCE.md` §14/§20
|
||||
are all in place. It is held pending the user's decision to open the
|
||||
PR, not pending further work.
|
||||
|
||||
- **Branch `tree-primitive-framing`**, base `githubsucks/main` @
|
||||
`12f2970`. **Unpushed** while held. Framing
|
||||
|
|
@ -173,7 +175,9 @@ PR is not open pending review of the documentation this lane records.
|
|||
- **Commits:** `61b1062` framing, `cf4ac1c` rev 2, `932b3ab` rev 3,
|
||||
`5186bfd` rev 4 (carrying the `COHERENCE.md` §14 call-site
|
||||
correction), `49a42ec` the primitive, `8f64c3b` byte-identity coverage
|
||||
plus the verification record.
|
||||
plus the verification record, `38e94dc` this lane, §14's ✗ → ◐ and the
|
||||
handoff facts, and a review round adding the moving-selection test and
|
||||
the flat-panel TAB delegation.
|
||||
|
||||
### What it ships
|
||||
|
||||
|
|
@ -200,7 +204,7 @@ The LSP outline adopts, supplying `depth` and `id = line:col`; its
|
|||
|---|---|
|
||||
| luajit sweep | **3453 / 0** (= `main` 3450 + 3 listview tests) |
|
||||
| crdt sweep, isolated ×2 | **3722 / 0** (= `main` 3718 + 4 tests) |
|
||||
| `listview_acceptance` | 21 / 21 |
|
||||
| `listview_acceptance` | 22 / 22 |
|
||||
| `m4_acceptance` | 150 / 150 (basedpyright skipped) |
|
||||
| `--lib` | 1896 / 0 |
|
||||
| fmt · diff-check · clippy ±crdt | pass |
|
||||
|
|
|
|||
|
|
@ -2066,7 +2066,16 @@ before trusting them:
|
|||
`m6_8_supervisor_reaps_all_children_across_cycles`) are timing-based;
|
||||
`editor::composition_overhead_under_ten_percent` is a render-ratio
|
||||
microbenchmark that fails ~1/3 even isolated single-threaded (already
|
||||
`cfg!(macos)`-disabled). Vterm Stage 3's merge CI saw one macOS timeout in
|
||||
`cfg!(macos)`-disabled). *(One local measurement, 2026-08-04, 16-core
|
||||
Linux, tree-primitive branch: it went red inside the full `--features
|
||||
crdt` lib run at `dispatch overhead 30.7%`, then 5/5 green run alone
|
||||
at ratios of -0.6% to +0.2%, and the next full-suite run was also
|
||||
green. Recorded as a measurement, NOT a cause — 5 isolated greens
|
||||
establish that the ratio is far from the threshold when alone, not
|
||||
that in-suite contention is what pushed it over. It is also not a
|
||||
`ci-red-signatures.md` row: that registry judges red **CI** runs, and
|
||||
this was local. The branch could not reach it — its diff versus `main`
|
||||
touched no `src/`, no crate, and no manifest.)* Vterm Stage 3's merge CI saw one macOS timeout in
|
||||
`real_tui_terminal_smoke_restores_host_after_output_input_resize_scroll_copy_and_bell`;
|
||||
the complete failed-job rerun passed. The required-GPU gate also failed once
|
||||
in `headless_diag_face_recolors_band_counter_despite_unchanged_text`, then
|
||||
|
|
|
|||
|
|
@ -798,6 +798,83 @@ fn tr_1_collapse_hides_descendants_and_survives_re_render() {
|
|||
);
|
||||
}
|
||||
|
||||
/// Selection survives a re-render that MOVES the selected node.
|
||||
///
|
||||
/// `tr_1` is not sufficient for this and was vacuous as a selection
|
||||
/// test: it toggles the ROOT, which occupies line 1 before and after the
|
||||
/// collapse, so the old line-based re-seating would pass it unchanged.
|
||||
/// A selection test has to move the node.
|
||||
///
|
||||
/// Here `on_refresh` inserts a child ABOVE the selected sibling, so the
|
||||
/// sibling's line shifts. Re-seating by line would land on the inserted
|
||||
/// row; re-seating by id stays on the sibling.
|
||||
#[test]
|
||||
fn tr_4_selection_follows_the_node_when_rows_are_inserted_above_it() {
|
||||
let mut s = editor();
|
||||
exec(
|
||||
&s,
|
||||
r#"_G.EXTRA = false
|
||||
pmacs.listview.open {
|
||||
name = "*tree*", header = "tree",
|
||||
rows = {
|
||||
{ text = "root", item = "root", depth = 0, id = "a" },
|
||||
{ text = " kid", item = "kid", depth = 1, id = "b" },
|
||||
{ text = "sibling", item = "sibling", depth = 0, id = "z" },
|
||||
},
|
||||
on_refresh = function()
|
||||
if _G.EXTRA then
|
||||
return {
|
||||
{ text = "root", item = "root", depth = 0, id = "a" },
|
||||
{ text = " kid", item = "kid", depth = 1, id = "b" },
|
||||
{ text = " kid2", item = "kid2", depth = 1, id = "c" },
|
||||
{ text = "sibling", item = "sib", depth = 0, id = "z" },
|
||||
}
|
||||
end
|
||||
return {
|
||||
{ text = "root", item = "root", depth = 0, id = "a" },
|
||||
{ text = " kid", item = "kid", depth = 1, id = "b" },
|
||||
{ text = "sibling", item = "sib", depth = 0, id = "z" },
|
||||
}
|
||||
end,
|
||||
}"#,
|
||||
);
|
||||
|
||||
// Select `sibling` — data line 3.
|
||||
press(&mut s, KeyCode::Char('n'));
|
||||
press(&mut s, KeyCode::Char('n'));
|
||||
let line_before: i64 = eval(&s, "return pmacs.editor.cursor_line()");
|
||||
let text_at = |s: &EditorState| -> String {
|
||||
let body = active_text(s);
|
||||
let line: i64 = eval(s, "return pmacs.editor.cursor_line()");
|
||||
body.lines()
|
||||
.nth(usize::try_from(line).expect("line fits"))
|
||||
.unwrap_or_default()
|
||||
.to_string()
|
||||
};
|
||||
assert_eq!(text_at(&s), "sibling", "premise: sibling is selected");
|
||||
|
||||
// Refresh inserts `kid2` ABOVE sibling, so its line moves.
|
||||
exec(&s, "_G.EXTRA = true");
|
||||
press(&mut s, KeyCode::Char('g'));
|
||||
|
||||
let line_after: i64 = eval(&s, "return pmacs.editor.cursor_line()");
|
||||
// Substantive claim first, so a regression reports as what it is.
|
||||
// Under line-based re-seating the cursor stays on line 3, which now
|
||||
// holds the INSERTED row.
|
||||
assert_eq!(
|
||||
text_at(&s),
|
||||
"sibling",
|
||||
"selection follows the NODE, not the line"
|
||||
);
|
||||
// …and the fixture really did move it, so the assertion above is not
|
||||
// satisfied by the node happening to stay put (which is exactly how
|
||||
// `tr_1` is vacuous as a selection test).
|
||||
assert_ne!(
|
||||
line_before, line_after,
|
||||
"fixture: the insert must move the selected node"
|
||||
);
|
||||
}
|
||||
|
||||
/// A leaf reports rather than silently doing nothing.
|
||||
///
|
||||
/// The outline's `g` is already a dead binding — bound, dispatched, no
|
||||
|
|
@ -833,12 +910,24 @@ fn tr_3_a_flat_panel_is_untouched_by_the_tree_extension() {
|
|||
}"#,
|
||||
);
|
||||
let before = active_text(&s);
|
||||
let status_before = status(&s);
|
||||
press(&mut s, KeyCode::Tab);
|
||||
assert_eq!(
|
||||
active_text(&s),
|
||||
before,
|
||||
"TAB on a depthless panel changes nothing"
|
||||
);
|
||||
// Byte-identity of the BUFFER is not enough: TAB is bound for every
|
||||
// listview, so the tree command intercepts a key that previously
|
||||
// fell through to `buffer.tab` and the read-only intercept. A
|
||||
// listview-specific status here would be a behaviour change the
|
||||
// flat consumers never had, and invisible to a buffer comparison.
|
||||
assert!(
|
||||
!status(&s).contains("no node here") && !status(&s).contains("no children"),
|
||||
"a flat panel must not gain tree feedback; status was {:?} (was {:?})",
|
||||
status(&s),
|
||||
status_before
|
||||
);
|
||||
assert!(
|
||||
before.contains("one") && before.contains("two"),
|
||||
"both rows render: {before}"
|
||||
|
|
|
|||
Loading…
Reference in New Issue