Merge pull request #59 from fiddlerwoaroof/fix/web-drill-interim

fix(protocol): the advertised drill key says something true
This commit is contained in:
Ed Langley
2026-09-20 14:10:55 -07:00
committed by GitHub
2 changed files with 48 additions and 14 deletions
+33 -1
View File
@@ -242,6 +242,12 @@ fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Op
cmd: ModelCommand::TransposeAxes,
}]),
// The hint line advertises `>`, and an upstream key is a black hole
// (gotcha 47), so it said nothing at all. Interim: improvise-gd28.
"drill-into-cell" => Some(vec![Command::SetStatus(
"drill is not available in the browser yet".to_string(),
)]),
// ── editing ──────────────────────────────────────────────────
"edit-or-drill" => edit_or_drill(state, AppMode::from_name(args.first()?)?),
"commit-cell-edit" => commit(state, AppMode::from_name(args.first()?)?, Advance::Down),
@@ -1222,7 +1228,8 @@ mod tests {
(ev(KeyCode::Char('Z')), "<the Z prefix>"),
(ev(KeyCode::Char('H')), "hide-selected-row-item"),
(ev(KeyCode::Char('z')), "toggle-group-under-cursor"),
(ev(KeyCode::Char('>')), "drill-into-cell"),
// `>` is NOT here any more: it answers with a status of its own,
// because it is advertised and silence read as broken (gd28).
(ev(KeyCode::Char('<')), "view-back"),
(ev(KeyCode::Char('/')), "search"),
(ev(KeyCode::Char('n')), "search-navigate"),
@@ -1259,6 +1266,31 @@ mod tests {
}
}
/// The advertised key must say what is actually true. Two refusals, two
/// subjects — see `context/test-evidence/clients.md`.
#[test]
fn the_drill_key_and_an_aggregate_edit_refuse_for_different_reasons() {
let mut state = in_mode(AppMode::Normal);
state.grid.extent = GridExtent { rows: 2, cols: 2 };
state.window = (2, 2);
let KeyOutcome::Local(drill) = resolve(&state, ev(KeyCode::Char('>'))) else {
panic!("`>` is advertised in the hint line; it must answer");
};
let [Command::SetStatus(said)] = &drill[..] else {
panic!("expected one status, got {drill:?}");
};
assert!(
said.contains("drill") && said.contains("browser"),
"the status must name drill and say it is the browser that lacks it: {said}"
);
assert!(
!said.contains("aggregate"),
"a sentence about aggregates is TRUE-SOUNDING about the wrong thing — \
`>` is refused whatever the cell is: {said}"
);
}
/// The refusal above is only half the property: the bug guarded is
/// *entering a room with no exit*. See `context/test-evidence/clients.md`.
#[test]
+15 -13
View File
@@ -48,27 +48,29 @@ test.describe("drill is refused in the browser, not broken", () => {
await expect(page.locator("#grid input")).toHaveCount(0);
});
// The silent half, and the one worth pinning. `>` is `drill-into-cell`,
// which `resolve::step` does not name, so it becomes `KeyOutcome::Upstream`
// -> `ClientFrame::Key` -> `Host::handle`, whose Key arm is an empty vec
// (gotcha 47). An upstream key is a black hole, not a slow path.
test("the drill key does nothing at all, and the page is not merely dead", async ({
// `>` is the key the hint line advertises as ":drill", and it used to be a
// name `resolve::step` did not handle — so `KeyOutcome::Upstream` ->
// `ClientFrame::Key` -> `Host::handle`'s empty vec, a black hole rather than
// a slow path (gotcha 47). It now answers. Interim until improvise-gd28.
test("the drill key says drill is unavailable, not something about aggregates", async ({
page,
}) => {
await anAggregateGrid(page);
const before = await page.locator("#status").textContent();
await page.keyboard.press(">");
// Nothing moves: not the mode, not the status, not the grid.
await expect(page.locator("#status")).toContainText(
"drill is not available in the browser yet",
);
await expect(page.locator("#mode")).toHaveText("NORMAL");
await expect(page.locator("#status")).toHaveText(before);
await expect(page.locator("#grid td.editable")).toHaveCount(0);
// check_the_checker: a test that asserts "nothing happened" passes against
// a page that has stopped listening. This proves the keyboard is live, so
// `>`'s silence is specific to `>` — the refusal below DOES change #status.
// The two refusals have two SUBJECTS, and conflating them would be a
// true-sounding sentence about the wrong thing — worse than silence,
// because it stops the reader looking. `>` must not talk about aggregates.
await expect(page.locator("#status")).not.toContainText("aggregate");
// And `i` still gives the OTHER refusal, which is what proves the two are
// distinct rather than one message reached by two keys.
await page.keyboard.press("i");
await expect(page.locator("#status")).not.toHaveText(before);
await expect(page.locator("#status")).toContainText(
"drilling into an aggregate needs the host",
);