refactor(panel): one shared rule for the adopter `display` vocabulary

Stage 3 step 2 (Q#S3-1). DEFAULT-PRESERVING WITH ONE INTENTIONAL
NORMALIZATION — not "behaviour-preserving", which would be too broad a
claim. Every adopter keeps its current default, and the full serialized
suite is 3447 passed / 0 failed with ZERO suites differing from the
pre-change baseline. But invalid-input behaviour DID move, deliberately,
and that is pinned rather than asserted in prose.

Before this, FOUR adopters validated the same three-value vocabulary in
four places: Rust for the terminal, and hand-written Lua copies in
listview.lua, compile.lua and dired.lua, each carrying its own copy of
the error string. `parse_adopter_placement` read like the shared parser
its doc comment implied but had exactly one caller. Four copies of one
rule is how the next adopter gets it subtly wrong, and the next adopter
is DAP.

`resolve_adopter_display(operation, raw, default)` now owns exactly
three things: the vocabulary, the error text, and the default policy.
Reachable from Lua as the internal seam `pmacs.window._resolve_display`.

THE DEFAULT IS A PARAMETER, NOT A CONSTANT, and that is load-bearing
rather than stylistic. listview/compile/terminal will resolve omission
to the panel in step 3; DIRED MUST NOT, because
`pmacs.path.set_directory_handler` calls it with `{ dest = dest }` and
no `display` key at all — a flipped default would open `pmacs .` in a
bottom panel. Passing the default in makes dired's exemption visible at
its call site instead of hidden in a divergent copy.

TERMINAL'S `window` MUTUAL-EXCLUSION STAYS IN ITS OWN WRAPPER. Only the
terminal accepts a `window` id and only it must reject `window` combined
with `display = "panel"`. A helper pretending the four parsers were
identical would be its own defect.

THE NORMALIZATION, DECIDED AND PINNED. Terminal read
`get::<Option<String>>("display")?`, so a non-string value raised mlua's
TYPE error before reaching any custom message, while the Lua copies
stringified it into their own. Nothing pinned either behaviour — every
existing assertion passes an unknown STRING, which takes the same path
under both designs and therefore could not have caught a regression
here. The custom error wins because it names the legal vocabulary; the
value is rendered by TYPE ALONE (`unknown display (integer)`) so the
message cannot imply a string was passed.

Pinned at the terminal entry point in acc19 — the one adopter whose
behaviour changed — asserting the shared error AND that nothing is
created. The type SPELLING is deliberately not pinned: Lua 5.4 says
`integer` where LuaJIT has no integer subtype, so asserting either
literal would pass on one CI flavor and fail on the other. Verified
46/46 under both.

COMPILE NEEDED AN EXPLICIT OMISSION CAPTURE, and finding that out is
what this step is for. The resolver collapses omission into its default,
but compile's recompile gate distinguishes them: it fires on OMISSION
only, never on an explicit `display = "current"`, which is the
documented opt-out and must reach the raw switch even when the previous
run was panel-placed. Resolving first and testing `== "current"`
afterwards would have silently merged the two and broken the opt-out
with every test still green. `display_omitted` is captured before the
resolver call and the gate keys on it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Levi Neuwirth 2026-08-01 20:13:03 -04:00
parent 0224c68b73
commit 41d37fcbcb
No known key found for this signature in database
7 changed files with 236 additions and 46 deletions

View File

@ -803,12 +803,17 @@ local function start_run(slot, cmdline, opts)
-- supersedes anything, rewrites the buffer, or spawns a process, so
-- an unknown value leaves no half-started run behind. In Stages 1-2
-- omission means "current"; Stage 3 flips the default.
local display = opts.display
if display ~= nil and display ~= "current" and display ~= "panel" then
error(string.format(
"compile.run: unknown display %q (expected \"current\" or \"panel\")",
tostring(display)))
end
-- Q#S3-1: one shared rule for vocabulary, error text and default.
--
-- `display_omitted` is captured SEPARATELY and deliberately. The
-- resolver collapses omission into its default, but the recompile gate
-- below distinguishes them: it fires on OMISSION only, never on an
-- explicit `display = "current"`, which is the documented opt-out and
-- must reach the raw switch even when the previous run was
-- panel-placed. Resolving first and testing `== "current"` afterwards
-- would silently merge the two and break that opt-out.
local display_omitted = opts.display == nil
local display = pmacs.window._resolve_display("compile.run", opts.display, "current")
-- q-target discipline (Q#CM11): capture only when coming from a
-- non-generated buffer, so `g` reruns don't re-capture and
-- compile → g → q restores the original buffer.
@ -896,7 +901,7 @@ local function start_run(slot, cmdline, opts)
-- so it must reach the raw switch even when the previous run was
-- panel-placed. The duplicate presentation that produces is the
-- escape hatch's documented cost (R3-rp2).
if display == "panel" or (display == nil and already_in_panel(slot.buf)) then
if display == "panel" or (display_omitted and already_in_panel(slot.buf)) then
pmacs.window.display(slot.buf, { side = "bottom", select = false })
else
pmacs.window.switch_buffer(slot.buf)

View File

@ -642,11 +642,13 @@ local function open_directory(path, opts, departed)
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
-- Q#S3-1/§1.1a: the shared rule, with dired's default passed
-- EXPLICITLY as "current" and kept there through Stage 3. The
-- `pmacs.path.directory_handler` slot calls this with no `display` at
-- all, so flipping dired's default would open `pmacs .` in a bottom
-- panel. Dired produces a document the user works in, not output they
-- consult; the panel default is right for the latter only.
local wanted = pmacs.window._resolve_display("pmacs.dired.open", opts.display, "current")
local canonical = canonicalize(path)
-- Read first: a failure must leave no buffer, no window change, and

View File

@ -226,12 +226,10 @@ function pmacs.listview.open(spec)
-- window. In Stages 1-2 omitting `display` keeps today's raw switch;
-- Stage 3 flips the default. An unknown value errors before anything
-- is displayed.
local display = spec.display
if display ~= nil and display ~= "current" and display ~= "panel" then
error(string.format(
"listview.open: unknown display %q (expected \"current\" or \"panel\")",
tostring(display)))
end
-- Q#S3-1: the vocabulary, the error and the default policy are one
-- rule (`window._resolve_display`), not a copy per adopter. The
-- default is passed in because the adopters do not share one.
local display = pmacs.window._resolve_display("listview.open", spec.display, "current")
if display == "panel" then
pmacs.window.display(p.buffer, { side = "bottom", select = true })
else

View File

@ -196,21 +196,17 @@ fallback is invisible from the adopter's side.
### 1.6 What is NOT established
- **Nothing has been implemented or measured.** Unlike the distribution
lane, there is no artifact to inspect; the evidence is the existing
suites' behaviour before and after.
- ~~The blast radius on existing acceptance suites is unmeasured.~~
**MEASURED — see §1.6b: 37 failures across 5 suites.** Stage 1
shipped the mechanism opt-in precisely so existing suites kept their
meaning. Flipping the default changes where output lands for every
suite that exercises listview, compile or terminal **without** passing
`display`. Criterion 58 says the default-placement suites are
*updated*; how many others move is a measurement this framing has not
taken and the implementation must take first.
- **Interaction with dired.** `dired.lua:18` documents
`opts.display = "current" | "panel"` with **default `"current"`**.
Dired is not in Q#BP12's table. Whether the flip reaches it, or dired
keeps an explicit `"current"`, is Q#S3-2.
- **The blast radius is MEASURED — §1.6b: 37 failures across 5 suites**,
classified per test in §1.6c. Stage 1 shipped the mechanism opt-in
precisely so existing suites kept their meaning; flipping the default
changes where output lands for every suite that exercises listview,
compile or terminal **without** passing `display`.
- **Steps 1 and 2 of §7 are done** (`0224c68`, `a2f4411`). The flip
itself, the test revisions, and the capability-fallback criterion are
not.
- ~~Interaction with dired.~~ **DECIDED — §1.1a and Q#S3-2: dired keeps
`"current"`**, passed explicitly to the shared resolver so the
exemption is visible at its call site.
---
@ -333,8 +329,26 @@ got both wrong.
message is ever reached. The Lua callers instead `tostring()` whatever
they got and report it inside their own error. These are different
observable behaviours for the same bad input, and unifying the error
text without deciding this would silently change one of them. The
stage must state which it standardizes on and pin it.
text without deciding this would silently change one of them.
**RESOLVED and PINNED at step 2.** The custom error wins, because it
names the legal vocabulary where mlua's type error does not.
Non-strings render by **type alone** (`unknown display (integer)`),
never quoted, so the message cannot imply a string was passed. Pinned
in `bottom_panel_stage1_acceptance::acc19` at the **terminal** entry
point — the one adopter whose behaviour actually changed — asserting
the shared error *and* that nothing is created, exactly as the
unknown-string case already does.
**The type SPELLING is deliberately not pinned.** Lua 5.4 reports
`integer`; LuaJIT has no integer subtype. Asserting either literal
would pass on one CI flavor and fail on the other, so the test pins
the shape (`unknown display (`, the vocabulary, and the *absence* of a
quoted value). Verified 46/46 under both flavors.
Consequently step 2 is **default-preserving with one intentional
normalization**, not "behaviour-preserving": every adopter kept its
default, but invalid-input behaviour moved on purpose.
- **Q#S3-2 — DECIDED: dired does not flip.** It keeps `"current"` as
its default, expressed as the `default` argument to the shared
resolver so the exemption is visible at the call site rather than

View File

@ -9320,12 +9320,18 @@ fn install_terminal(
// Bottom-panel arc (Q#BP11b): parse placement BEFORE the
// session, process, buffer, or wrapper exists, so an
// unknown `display` value creates nothing to roll back.
// Stage 3 step 2: the shared resolver, still carrying the
// PRE-FLIP default. The unification must be provably
// behaviour-preserving before the default moves, so the
// flip to `AdopterDefault::Panel` is its own commit.
let display_value = spec_table.get::<mlua::Value>("display")?;
let placement = window_panel::parse_adopter_placement(
&core,
frontend_id,
"pmacs.terminal.open",
spec_table.get::<Option<String>>("display")?.as_deref(),
Some(&display_value),
spec_table.get::<Option<u64>>("window")?,
window_panel::AdopterDefault::Current,
)?;
let buffer_id = {
let mut manager = manager.borrow_mut();

View File

@ -251,9 +251,97 @@ pub(crate) enum AdopterPlacement {
Window(WindowId),
}
/// The one rule for the adopter `display` vocabulary: which values are
/// legal, what the error says, and what omission means.
///
/// **Bottom-panel Stage 3 (Q#S3-1).** Before this, four adopters
/// validated the same three-value vocabulary in three places — Rust for
/// the terminal, and hand-written Lua copies in `listview.lua`,
/// `compile.lua` and `dired.lua`, each with its own copy of the error
/// string. Four copies of one rule is how the next adopter gets it
/// subtly wrong, and the next adopter is DAP.
///
/// `default` is a **parameter, not a constant**, because the adopters do
/// not share one. listview / compile / terminal resolve omission to the
/// panel; **dired resolves it to `"current"`** and must keep doing so —
/// `pmacs.path.directory_handler` calls it with no `display` at all, so
/// a flipped default would open `pmacs .` in a bottom panel (§1.1a).
/// Passing the default in is what makes dired's exemption visible at its
/// call site instead of hidden in a divergent copy.
///
/// **Non-string values are reported as unknown, not as type errors**,
/// and this is a deliberate normalization (Q#S3-1). Terminal previously
/// read `get::<Option<String>>` and so raised mlua's type error *before*
/// reaching any custom message, while the Lua copies stringified and
/// reported their own. Nothing pinned either behaviour. The custom error
/// wins because it names the legal vocabulary and mlua's does not.
///
/// # Errors
/// Any non-nil value that is not `"current"` or `"panel"`.
pub(crate) fn resolve_adopter_display(
operation: &str,
raw: Option<&mlua::Value>,
default: AdopterDefault,
) -> mlua::Result<AdopterPlacement> {
let raw = match raw {
None | Some(mlua::Value::Nil) => return Ok(default.placement()),
Some(value) => value,
};
match raw.as_str().as_deref() {
Some("current") => Ok(AdopterPlacement::Current),
Some("panel") => Ok(AdopterPlacement::Panel),
_ => Err(mlua::Error::runtime(format!(
"{operation}: unknown display {} (expected \"current\" or \"panel\")",
display_for_error(raw)
))),
}
}
/// What omission means for one adopter.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub(crate) enum AdopterDefault {
/// listview, compile, terminal — Stage 3's flip.
Panel,
/// dired, and anything else whose output is a document the user
/// works in rather than output they consult.
Current,
}
impl AdopterDefault {
fn placement(self) -> AdopterPlacement {
match self {
Self::Panel => AdopterPlacement::Panel,
Self::Current => AdopterPlacement::Current,
}
}
}
/// Render a rejected `display` value for the error message.
///
/// Strings are quoted so `display = "sideways"` reads as `"sideways"`;
/// anything else is shown **by type alone** — `unknown display
/// (number)` — because quoting a non-string as `"42"` would imply the
/// caller passed a string when they passed a number.
///
/// The value itself is deliberately not interpolated: it is already
/// wrong, `mlua::Value`'s `Display` is not guaranteed useful for tables
/// or userdata, and the type is what tells the caller what to fix.
fn display_for_error(value: &mlua::Value) -> String {
value.as_str().map_or_else(
|| format!("({})", value.type_name()),
|s| format!("{:?}", &*s),
)
}
/// Parse an adopter's placement **before** it creates a buffer, session,
/// process, or wrapper — so an unknown value leaves nothing to roll back.
///
/// Terminal-specific wrapper around [`resolve_adopter_display`]: only
/// the terminal accepts a `window` id, and only it must reject `window`
/// combined with `display = "panel"`. That asymmetry stays here rather
/// than in the shared resolver, because a helper that pretended the four
/// parsers were identical would be its own defect.
///
/// # Errors
/// An unknown `display` value, a `window` combined with
/// `display = "panel"`, or a window id that is not live in the acting
@ -262,18 +350,11 @@ pub(crate) fn parse_adopter_placement(
core: &SharedCore,
fid: FrontendId,
operation: &str,
display: Option<&str>,
display: Option<&mlua::Value>,
window: Option<u64>,
default: AdopterDefault,
) -> mlua::Result<AdopterPlacement> {
let display = match display {
None | Some("current") => AdopterPlacement::Current,
Some("panel") => AdopterPlacement::Panel,
Some(other) => {
return Err(mlua::Error::runtime(format!(
"{operation}: unknown display {other:?} (expected \"current\" or \"panel\")"
)));
}
};
let display = resolve_adopter_display(operation, display, default)?;
match (window, &display) {
(Some(_), AdopterPlacement::Panel) => Err(mlua::Error::runtime(format!(
"{operation}: `window` and `display = \"panel\"` are mutually exclusive"
@ -481,6 +562,43 @@ pub(crate) fn install(lua: &Lua, core: &SharedCore, win: &Table) -> mlua::Result
)?;
}
{
// Q#S3-1 — the shared adopter-display rule, reachable from Lua.
//
// Underscore-prefixed because it is an internal seam between the
// builtin runtime modules and this one, not user-facing API:
// `listview.lua`, `compile.lua` and `dired.lua` call it instead
// of each keeping a hand-written copy of the same three-value
// check and error string.
//
// Returns the resolved `"current"` / `"panel"` rather than an
// opaque handle, so the Lua callers keep their existing
// `if display == "panel"` dispatch and this change stays a
// validation unification rather than a control-flow rewrite.
win.set(
"_resolve_display",
lua.create_function(
|_,
(operation, raw, default): (String, mlua::Value, String)|
-> mlua::Result<&'static str> {
let default = match default.as_str() {
"panel" => AdopterDefault::Panel,
"current" => AdopterDefault::Current,
other => {
return Err(mlua::Error::runtime(format!(
"window._resolve_display: bad default {other:?}"
)));
}
};
match resolve_adopter_display(&operation, Some(&raw), default)? {
AdopterPlacement::Panel => Ok("panel"),
AdopterPlacement::Current | AdopterPlacement::Window(_) => Ok("current"),
}
},
)?,
)?;
}
{
let cc = core.clone();
win.set(

View File

@ -1291,6 +1291,53 @@ fn acc19_adopters_place_side_affinely_through_real_entry_points() {
"unknown display fails before session/process/buffer creation"
);
assert_eq!(s.core.borrow().registry.borrow().ids().len(), before);
// Bottom-panel Stage 3, Q#S3-1 — the NON-STRING normalization.
//
// Pinned because step 2 CHANGED this deliberately and nothing else
// covers it. Before the shared resolver, terminal read
// `get::<Option<String>>("display")?`, so a number raised mlua's
// TYPE error before any custom message existed; the Lua adopters
// stringified instead and reported their own. Unifying the error
// text without pinning this would have let the two drift back apart
// unnoticed, and the surrounding assertions could not have caught it
// — they all pass unknown STRINGS, which take the same path in both
// designs.
//
// The custom error wins because it names the legal vocabulary. The
// type is reported WITHOUT the value, so the message cannot imply a
// string was passed.
let before = s.core.borrow().registry.borrow().ids().len();
let err = try_exec(
&s,
"pmacs.terminal.open { command = \"/bin/sh\", display = 42 }",
)
.expect_err("a non-string display is rejected");
// The TYPE SPELLING is deliberately not pinned: Lua 5.4 reports
// `integer` where LuaJIT has no integer subtype, so asserting either
// literal would pass on one CI flavor and fail on the other. What is
// pinned is the shape — our operation name, a parenthesised type
// rather than a quoted value, and the vocabulary.
assert!(
err.contains("pmacs.terminal.open: unknown display ("),
"a non-string display takes the shared unknown-display error naming the \
operation and a type, not mlua's type error; got: {err}"
);
assert!(
err.contains("expected \"current\" or \"panel\""),
"the error still names the legal values; got: {err}"
);
assert!(
!err.contains("\"42\""),
"the rejected value is reported by TYPE, not quoted as though it were a \
string; got: {err}"
);
assert_eq!(
s.core.borrow().registry.borrow().ids().len(),
before,
"…and still creates nothing, exactly as an unknown string does"
);
exec(
&s,
"TERM_BUF = pmacs.terminal.open { command = \"/bin/sh\", display = \"panel\" }",