Merge pull request #180 from fiddlerwoaroof/fix/help-keys-checks

Help and advertised-key checks that can see what they claim (wk18, 3t5x, 3ij0, jhdf)
This commit is contained in:
Ed Langley
2026-09-22 22:44:46 -07:00
committed by GitHub
9 changed files with 758 additions and 151 deletions
+2 -1
View File
@@ -98,7 +98,8 @@ The message states the **shape** rather than the arity, because
## 6 — the help's ex-commands were the unguarded surface
`help.rs` derives `HelpLine::Keys` rows from the keymap and drops any row whose
command does not resolve, so a key row cannot go stale. `HelpLine::Literal` and
command does not resolve, so a key row cannot go stale. `HelpLine::Literal` (an
ex-command only, since improvise-wk18: built by `ex!`) and
`HelpLine::Accent` rows are hand-written prose and had nothing checking them —
which is how all three documented spellings of `:formula` came to be ones the
parser refuses.
+31 -7
View File
@@ -2064,14 +2064,38 @@ nothing in the table to look up, and its hint is a hand-written tail. That is a
real hole and it is where it is visible, not papered over: a keymap cannot
describe keys a command resolves privately.
**`gg`, `yy`, `ZZ` and the ex-commands.** A prefix sequence is two addresses and
not one binding (gotcha 13), and `:q`/`:w` are parsed text rather than keys.
Both are `HelpLine::Literal`, which says out loud that the row is spelled rather
than derived.
**The ex-commands.** `:q`/`:w` are parsed text rather than keys, so they are
`HelpLine::Literal` — built only by the `ex!` macro, whose leading colon is its
own — and the parser, not the keymap, is what checks them
(`every_ex_command_the_help_pages_show_is_one_the_parser_accepts`).
**The captions.** `i:edit` where `i` entered records mode would still pass every
test here. A caption is English beside a command name and only a reader judges
it — the same limit gotcha 14 recorded for the old hint test.
This paragraph used to say the same of `gg`, `yy` and `ZZ`: "a prefix sequence
is two addresses and not one binding". Two addresses is true and was no reason
to spell them by hand — `Keymap::prefixed_keys_running` looks the pair up the
way `keys_running` looks up one key, and `keys()` asks both (improvise-wk18). A
key string with no command name was also the one row no guard could reach.
**Every row the pages ask for is visible to a guard.** The page builders take
the `keys` lookup as a parameter; `pages()` passes the real one, and
`help_bindings()` (tests only) passes one that records `(mode, command, args,
description)`. So a check over the help overlay reads the *same builders* the
overlay is drawn from, never a list kept beside them. That premise is a type,
not a convention (improvise-v29m): `HelpLine::Keys` carries a `KeyRow` whose
only constructor does the lookup, so a builder cannot type a key row the
recorder would never see; and `ex!` refuses at compile time any usage with a
bare word after the command name, so an ex row cannot smuggle a key either.
`resolve::tests::every_advertised_command_answers_in_its_own_mode` walks it
beside `advertised`, which is how the browser's black holes on the help page
became visible (improvise-uln5).
**The captions.** `i:edit` where `i` entered records mode would still pass most
tests here: a caption is English beside a command name and only a reader judges
it — the same limit gotcha 14 recorded for the old hint test. Two checks now
bite at its edges. The browser guard requires an advertised key to *act*, so a
key whose caption promises an action and which only refuses must be named
(improvise-3t5x); and `only_the_edit_key_is_captioned_as_editing_the_cell` asks
by command that no other Normal row claims to edit (improvise-jhdf). A false
caption in words neither looks for still passes.
### The browser half
+103 -3
View File
@@ -1458,6 +1458,17 @@ is prose, and nothing checks it against the command it documents. That is a
neighbour of improvise-wk18, not the same gap — wk18 is about `literal()` rows
whose *keys* go unchecked; here the keys were right and the words were wrong.
**improvise-jhdf closed the paraphrase, and the paragraph above is what it
found.** Rewording Enter's caption to "Edit the cell (same as i)" — the same lie
in new words — passed both help tests. The test now asks by **command**, which
wk18's `help::help_bindings()` made possible: every Normal-mode help row whose
caption says "edit" in any case must run `edit-or-drill`. It fails on that
rival naming `(Normal, "enter-advance", None, "Edit the cell (same as i)")`.
Two controls stop it passing by looking away: `edit-or-drill` and
`enter-advance` must each still have a Normal row, or the filter has nothing to
catch. What it still cannot see is a false caption that avoids the word — "Opens
the cell" — which is the honest limit of any check over prose.
### a_view_row_click_focuses_moves_and_switches_to_the_row_clicked
improvise-iioi, and it is the **same trap** as the category click one level
@@ -2071,6 +2082,50 @@ mode `declined` refuses at the door, so a browser never stands in front of the
hint; the other two are improvise-gcvb's records-cell commit, which needs
`record_col`.
**improvise-3t5x and improvise-wk18 widened it again, in two directions, and the
list above is superseded by the one in the test.**
*What "answers" means.* It counted any `Local` as answered, so a lone
`SetStatus` refusal passed — which is why improvise-s8h7's false "Edit current
cell" stayed green. Injected to show it: make `edit_or_drill`'s `Record` arm
return only `SetStatus("RIVAL")`, so `i` refuses on every cell — the old guard
**passed**. The line drawn now: a caption is a promise, so an advertised key
must *act*. A refusal is legitimate — a feature a browser honestly lacks should
say so rather than go silent — but it is not what the caption says, so it has to
be **named** in the list with its reason, like a black hole. The test therefore
sorts every key into three outcomes, `Acts`, `Declines` (said something, did
nothing) and `Silent`, and lists the last two. The same injection now fails
naming `NORMAL: a/i edit-or-drill Declines`.
*Judged by what moved, not by the variant.* The first version of the new
classifier called anything but an all-status `Local` an action, and a rival that
unhooked `jump-first-row` from `composed` **passed** — because a chord's second
key always returns `Local([SetTransientPrefix(None), ..])`, the disarm, even
when nothing else happens. That is the laundering this entry already warns
about, one level down: `KeyOutcome::of` launders an empty vec into `Ignored`,
and `chord` launders a no-op into a `Local`. The disarm is now ignored when
deciding whether a key did anything, and the same rival fails naming `NORMAL:
gg jump-first-row Silent`.
*Which keys.* It read `help::advertised`, the status-bar table, and the help
overlay is a second body of advertising the browser also paints
(improvise-uln5). It now also reads `help::help_bindings()`, which records every
keys row the page builders ask for — the same builders `pages()` renders, so the
two cannot drift — including prefix sequences (`gg`, `yy`, `ZZ`), which it
presses by arming the prefix first. That is what added the five host-only
black holes (`/`, `n`, `H`, `z`, Normal's `Enter`) to the list: each was already
in `the_command_names_that_still_need_the_host_are_these`, and the help page
advertised it anyway.
*Which cells.* `furnished` stood on a record cell only, so `i` on an aggregate
(s8h7's whole subject) was outside its reach. Normal mode is now pressed on a
record, an aggregate and a derived cell, and a non-record cell is reported only
where it changes the answer, so the list does not triple. `furnished_on` also
carries a history in both directions and a yanked value, so `<`, `,` and `p`
are not refused by an empty stack — state refusals are correct behaviour and
not this test's subject, the same argument that gave `furnished` three of
everything.
### a_locally_committed_creating_column_agrees_with_the_host
improvise-gcvb. `resolve`'s commit returned `None` for the whole of records
@@ -3341,9 +3396,22 @@ over `Ctrl-R` is in `context/code/tui.md`), the test is renamed, and the second
half now asserts the binding **exists** — with `view-back` kept as the control,
which now guards the opposite failure: a table that binds everything, or a
sweep that reports names it never read. A third assertion pins the key itself,
`composed(&state, ev(Char(',')), "view-forward", &[]) == [Command::ViewForward]`,
so the binding cannot drift onto some other key while the presence check stays
happy.
`resolve(&state, ev(Char(','))) == Local([Command::ViewForward])`, so the
binding cannot drift onto some other key while the presence check stays happy.
**That third assertion was vacuous as first written, and the paragraph above
said it was not** (improvise-3ij0). It called `composed(&state, ev(Char(',')),
"view-forward", &[])`, and `composed` dispatches on the command **name**: the
`view-forward` arm never reads the event, so the `,` was decoration and the
assertion restated the first half of the test. Rebinding `view-forward` from `,`
to `~` in `keymap.rs` left it green; the key was pinned only by
`ui::app::tests::the_view_history_walks_both_ways_from_the_keyboard`, in another
crate. It now goes through `resolve`, which looks the key up in the shared
keymap before anything reads a name — so the rival fails **here**, at this
assertion (resolve.rs, "and the key is `,`"), and the pre-fix code under the
same rival was run and passes. The general shape: a helper that takes a key
*and* a name, and dispatches on the name, cannot pin the key; only the lookup
can.
**The rival: bind `,` to `view-back` instead** — the misspelling the original
control was written against, and the one a copy-paste actually produces. It
@@ -4540,3 +4608,35 @@ normal edges only, because a dev edge would unify the feature back in
which fails naming that crate while the two tests above pass. Then removed from
`improvise-ws-server` alone, which fails naming it. The native path runs
without `--target`, so the two arms are different invocations.
### resolve::tests::every_grid_move_lands_where_its_binding_says
improvise-6562, found reviewing PR #180 at 949810c, and it is the same
laundering as the disarm, one level over. `heard()` said it judged by what moved
and in fact judged by command **type**: any `Local` command besides a status or a
disarm counted as `Acts`. The fixture sat at every edge — a 2x1 grid with the
cursor at (0,0), one tile, help page 0 — so about twenty movement keys that
clamp to nothing were counted as acting. Rebinding `jump-last-col` to
`GridMove::ToStart(false)`, so `$` and `End` go to the *first* column, left the
whole workspace green.
Two changes, because either alone is not enough. `heard()` now **folds** the
outcome and compares the view slice before and after (status and disarm aside),
counting a `Command::Model` as sent and a `Request` as an act; and `furnished_on`
stands on a 3x3 grid with the cursor at (1,1), three tiles with the cursor on
the middle one, a page axis on its middle item and help page 1, so no move is
clamped by an edge. Injecting `jump-last-col` as a `SetSelected` to where the
cursor already is now fails `every_advertised_command_answers_in_its_own_mode`
naming `NORMAL: $/End jump-last-col Silent`.
**The reviewer's own rival still passes that guard, and should.** From (1,1),
`ToStart(false)` moves to (1,0) — the view moved, so the key "acts". No
did-anything-happen check can see *direction*; only an expected destination can.
This test supplies one without a second table: a `move-selection` binding's
expected landing is read off its own args (the deltas), and the four jumps are
named by what they are called. Every key that runs one is pressed from the
centre — fourteen of them, with a floor so a blind walk fails. Under the rival
it fails naming `End` and `$`. The general point: the fixture that made the
laundering invisible (the edge) is also the one that makes a wrong-direction
jump visible, so moving off the edge fixed one blind spot and opened the other,
and each needs its own question.
+59
View File
@@ -1100,3 +1100,62 @@ and the text between the inner quotes leaked. It failed as
`InRawC survived`. `c"…"` and `cr#"…"#` are now literals. A first version of
this test used a C string with no inner quotes and passed against the bug,
because the ordinary lexing happened to blank it too.
### help::tests::no_help_row_spells_a_key_by_hand
improvise-wk18. The help pages had one kind of row whose keys the keymap never
saw: `literal("gg", …)`, `literal("yy", …)`, `literal("ZZ", …)`, and a dim tip
reading "press Tab or l/n". Every other key row comes off `keys_for`, so a
rebinding reaches it; these would have gone on saying `gg` after `g g` stopped
meaning anything, and no guard could see them because a string carries no
command name. At HEAD the test failed naming all four.
The fix is structural rather than a second scanner. The three prefix rows are
ordinary `keys(..)` rows now — `keys` also asks `Keymap::prefixed_keys_running`
for two-key sequences. The tip is **gone** rather than derived: made a `keys` row
in `ModeKey::Help`, it gave the Welcome page its one row with a description,
and `harness/tests/help.spec.js` ("? opens help, and its rows carry the keys and
the words") failed in CI on `welcome.every(row => row.description === null)` —
Welcome is prose by contract. The Help mode's own status hint already derives
`next-page` from the keymap, so the literal was a second table (improvise-rr8n's
rule: keep the one derived from the thing it describes).
`literal()` and `dim()` are gone; the only way left to build a
`HelpLine::Literal` is the `ex!` macro, which writes the leading colon itself, so
what remains hand-written is ex-command text — checked against the parser by
`every_ex_command_the_help_pages_show_is_one_the_parser_accepts`, which reads
exactly the text a user would type. This test is what stops a direct
`HelpLine::Literal { keys: "gg", .. }` coming back past the macro.
### help::tests::every_keys_row_the_pages_ask_for_finds_its_keys
A `keys` row whose lookup finds nothing is **dropped**, by design — a page never
advertises a key that does not exist. The cost of that design is that a broken
lookup looks like a shorter page: `every_help_page_key_row_names_keys_that_exist`
only checks the rows that survived, plus a floor of twenty. So this asks the
question from the other end, over `help_bindings()` — every row the builders
*asked for* — and requires each to find a key.
The injection: comment out the `prefixed_labels` extension in `keys`. The three
prefix rows vanish from the page, every other help test stays green, and this
fails naming `Normal jump-first-row`, `Normal yank` and `Normal wq`.
### help::tests::an_ex_row_can_name_placeholders_and_nothing_else
improvise-v29m, found reviewing PR #180 at 949810c. `no_help_row_spells_a_key_by_hand`
read only `HelpLine::Literal`, and its premise — that `keys()` is the only way to
make a key row — was a convention: `HelpLine::Keys { keys: Vec<String>, .. }`
was a public variant with public fields, and `help_bindings()` records only what
the builders *ask* for, so `HelpLine::Keys { keys: vec!["Q"], description: "Quit
instantly" }` typed straight into `panels()`, with `Q` bound to nothing, passed
the whole workspace. So did `ex!("q or press Q", ..)`: the parser test splits on
colons and `parse_line` accepts trailing words.
Both are now **compile errors**, which is why this test is not where the guard
lives. `HelpLine::Keys` carries a `KeyRow` whose fields are private to a module
whose one constructor, `KeyRow::looked_up`, does the keymap lookup; the injected
row fails with E0559/E0451. `ex!` evaluates `ex_usage_is_placeholders` in a
`const` item, and the injected usage fails with `evaluation panicked: assertion
failed: ex_usage_is_placeholders("q or press Q")`. The rule cost one row:
`:add-items <cat> a b c ...` had three bare words and is now
`:add-items <cat> <item> ...`. This test pins the rule the macro asks, in both
directions, including that shipped usage and the rival.
+240 -86
View File
@@ -304,14 +304,11 @@ pub enum HelpLine {
Heading(&'static str),
Text(&'static str),
Accent(&'static str),
/// A keys-and-description row whose keys came off the keymap, so it
/// cannot go stale against the bindings it documents.
Keys {
keys: Vec<String>,
description: &'static str,
},
/// A row naming something the keymap does not own: an ex-command, or a
/// prefix sequence, which is two addresses and not one binding.
/// A keys-and-description row whose keys came off the keymap. A [`KeyRow`]
/// has no public constructor, so no builder can type one (improvise-v29m).
Keys(KeyRow),
/// An ex-command, which the keymap does not own. Built only by `ex!`, whose
/// colon is what stops a key being typed here (improvise-wk18).
Literal {
keys: &'static str,
description: &'static str,
@@ -326,38 +323,125 @@ pub struct HelpPage {
pub lines: Vec<HelpLine>,
}
/// A keys row, looked up in `mode`. An unbound command yields no keys and the
/// row is dropped, so the page never advertises a key that does not exist.
pub use key_row::KeyRow;
/// The one door to a keys row: its fields are private to this module, and its
/// only constructor looks the keys up.
mod key_row {
use super::{ModeKey, key_label, keys_for, prefixed_labels};
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct KeyRow {
keys: Vec<String>,
description: &'static str,
}
impl KeyRow {
pub fn keys(&self) -> &[String] {
&self.keys
}
pub fn description(&self) -> &'static str {
self.description
}
/// An unbound command yields no keys and no row, so the page never
/// advertises a key that does not exist.
pub(super) fn looked_up(
mode: &ModeKey,
command: &str,
args: Option<&[String]>,
description: &'static str,
) -> Option<Self> {
let mut keys: Vec<String> = keys_for(mode, command, args)
.iter()
.map(|(c, m)| key_label(c, *m))
.collect();
keys.extend(prefixed_labels(mode, command, args));
(!keys.is_empty()).then_some(Self { keys, description })
}
}
}
/// A keys row, looked up in `mode`.
fn keys(
mode: &ModeKey,
command: &str,
command: &'static str,
args: Option<&[&str]>,
description: &'static str,
) -> Option<HelpLine> {
let args: Option<Vec<String>> = args.map(|a| a.iter().map(|s| (*s).to_string()).collect());
let found = keys_for(mode, command, args.as_deref());
match found.is_empty() {
true => None,
false => Some(HelpLine::Keys {
keys: found.iter().map(|(c, m)| key_label(c, *m)).collect(),
description,
}),
}
KeyRow::looked_up(mode, command, args.as_deref(), description).map(HelpLine::Keys)
}
fn literal(keys: &'static str, description: &'static str) -> HelpLine {
HelpLine::Literal {
keys,
description,
dim: false,
/// `gg`, `yy`, `ZZ`: a prefix key and the key after it, both off the keymap.
fn prefixed_labels(mode: &ModeKey, command: &str, args: Option<&[String]>) -> Vec<String> {
let mut labels: Vec<String> = match Keymaps::shared().get(mode) {
Some(keymap) => keymap.prefixed_keys_running(command, args),
None => Vec::new(),
}
.iter()
.map(|[(c1, m1), (c2, m2)]| format!("{}{}", key_label(c1, *m1), key_label(c2, *m2)))
.collect();
labels.sort();
labels.dedup();
labels
}
fn dim(keys: &'static str, description: &'static str) -> HelpLine {
HelpLine::Literal {
keys,
description,
dim: true,
/// How a page asks for a keys row. `pages` answers with [`keys`]; a test
/// answers by recording the question, so a guard reads commands, not text.
type KeysRow<'a> =
&'a dyn Fn(&ModeKey, &'static str, Option<&[&str]>, &'static str) -> Option<HelpLine>;
/// An ex-command row. The colon is the macro's, and every word after the
/// command's name must be a placeholder, checked when the page compiles.
macro_rules! ex {
($text:literal, $description:expr $(,)?) => {{
const _: () = assert!(ex_usage_is_placeholders($text));
HelpLine::Literal {
keys: concat!(":", $text),
description: $description,
dim: false,
}
}};
(dim $text:literal, $description:expr $(,)?) => {{
const _: () = assert!(ex_usage_is_placeholders($text));
HelpLine::Literal {
keys: concat!(" :", $text),
description: $description,
dim: true,
}
}};
}
/// After the name, every word opens with `<`, `[` or `"`, or is `...`: so an
/// ex row cannot also advertise a key ("q or press Q"). Quotes span spaces.
const fn ex_usage_is_placeholders(text: &str) -> bool {
let b = text.as_bytes();
let mut i = 0;
while i < b.len() && b[i] != b' ' {
i += 1;
}
loop {
while i < b.len() && b[i] == b' ' {
i += 1;
}
if i == b.len() {
return true;
}
let start = i;
let mut quoted = false;
while i < b.len() && (quoted || b[i] != b' ') {
if b[i] == b'"' {
quoted = !quoted;
}
i += 1;
}
let ellipsis =
i - start == 3 && b[start] == b'.' && b[start + 1] == b'.' && b[start + 2] == b'.';
if !(ellipsis || b[start] == b'<' || b[start] == b'[' || b[start] == b'"') {
return false;
}
}
}
@@ -367,30 +451,48 @@ pub fn pages() -> Vec<HelpPage> {
let pages = vec![
HelpPage {
title: "Welcome",
lines: welcome(),
lines: welcome(&keys),
},
HelpPage {
title: "Navigation",
lines: navigation(),
lines: navigation(&keys),
},
HelpPage {
title: "Editing",
lines: editing(),
lines: editing(&keys),
},
HelpPage {
title: "Panels & Views",
lines: panels(),
lines: panels(&keys),
},
HelpPage {
title: "Commands",
lines: commands(),
lines: commands(&keys),
},
];
debug_assert_eq!(pages.len(), HELP_PAGE_COUNT);
pages
}
fn welcome() -> Vec<HelpLine> {
/// Every keys row the pages ask for, as `(mode, command, args, description)`
/// — the same builders, so a guard over it cannot drift from what is drawn.
#[cfg(test)]
pub(crate) fn help_bindings() -> Vec<(ModeKey, &'static str, Option<Vec<String>>, &'static str)> {
let asked = std::cell::RefCell::new(Vec::new());
let record: KeysRow = &|mode, command, args, description| {
let args = args.map(|a| a.iter().map(|s| (*s).to_string()).collect());
asked
.borrow_mut()
.push((mode.clone(), command, args, description));
None
};
for build in [welcome, navigation, editing, panels, commands] {
build(record);
}
asked.into_inner()
}
fn welcome(_: KeysRow) -> Vec<HelpLine> {
vec![
HelpLine::Blank,
HelpLine::Banner(" improvise"),
@@ -427,12 +529,10 @@ fn welcome() -> Vec<HelpLine> {
),
HelpLine::Text(" Tile The row/column/page assignment of a category."),
HelpLine::Text(" Formula A computed item: derives its value from other items."),
HelpLine::Blank,
dim("Tip: press Tab or l/n to go to the next page.", ""),
]
}
fn navigation() -> Vec<HelpLine> {
fn navigation(keys: KeysRow) -> Vec<HelpLine> {
let n = &ModeKey::Normal;
[
Some(HelpLine::Blank),
@@ -445,7 +545,7 @@ fn navigation() -> Vec<HelpLine> {
None,
"Move down, wrapping to the next column",
),
Some(literal("gg", "Jump to first row")),
keys(n, "jump-first-row", None, "Jump to first row"),
keys(n, "jump-last-row", None, "Jump to last row"),
keys(n, "jump-first-col", None, "Jump to first column"),
keys(n, "jump-last-col", None, "Jump to last column"),
@@ -512,7 +612,7 @@ fn navigation() -> Vec<HelpLine> {
.collect()
}
fn editing() -> Vec<HelpLine> {
fn editing(keys: KeysRow) -> Vec<HelpLine> {
let n = &ModeKey::Normal;
let e = &ModeKey::Editing;
[
@@ -555,7 +655,7 @@ fn editing() -> Vec<HelpLine> {
Some(HelpLine::Blank),
Some(HelpLine::Heading("Copy and paste")),
Some(HelpLine::Blank),
Some(literal("yy", "Yank (copy) the current cell value")),
keys(n, "yank", None, "Yank (copy) the current cell value"),
keys(
n,
"paste",
@@ -593,7 +693,7 @@ fn editing() -> Vec<HelpLine> {
.collect()
}
fn panels() -> Vec<HelpLine> {
fn panels(keys: KeysRow) -> Vec<HelpLine> {
let n = &ModeKey::Normal;
[
Some(HelpLine::Blank),
@@ -741,14 +841,14 @@ fn panels() -> Vec<HelpLine> {
"Toggle collapse of nearest group above cursor",
),
keys(n, "hide-selected-row-item", None, "Hide current row item"),
Some(dim(" :show-item <cat> <item>", "Restore a hidden item")),
Some(ex!(dim "show-item <cat> <item>", "Restore a hidden item")),
]
.into_iter()
.flatten()
.collect()
}
fn commands() -> Vec<HelpLine> {
fn commands(keys: KeysRow) -> Vec<HelpLine> {
let n = &ModeKey::Normal;
[
Some(HelpLine::Blank),
@@ -763,55 +863,40 @@ fn commands() -> Vec<HelpLine> {
Some(HelpLine::Blank),
Some(HelpLine::Heading("File operations")),
Some(HelpLine::Blank),
Some(literal(
":w [path]",
"Save (path optional after first save)",
)),
Some(literal(":wq", "Save and quit")),
Some(literal(":q", "Quit (warns if unsaved changes)")),
Some(literal(":q!", "Force quit without saving")),
Some(literal("ZZ", "Save and quit (same as :wq)")),
Some(ex!("w [path]", "Save (path optional after first save)",)),
Some(ex!("wq", "Save and quit")),
Some(ex!("q", "Quit (warns if unsaved changes)")),
Some(ex!("q!", "Force quit without saving")),
keys(n, "wq", None, "Save and quit (same as :wq)"),
keys(n, "save", None, "Save (same as :w)"),
Some(HelpLine::Blank),
Some(HelpLine::Heading("Import and export")),
Some(HelpLine::Blank),
Some(literal(":import <path>", "Open the JSON/CSV import wizard")),
Some(literal(
":export [path.csv]",
"Export the active view to CSV",
)),
Some(ex!("import <path>", "Open the JSON/CSV import wizard")),
Some(ex!("export [path.csv]", "Export the active view to CSV",)),
Some(HelpLine::Blank),
Some(HelpLine::Heading("Model building")),
Some(HelpLine::Blank),
Some(literal(":add-cat <name>", "Add a new category")),
Some(literal(
":add-item <cat> <item>",
"Add one item to a category",
)),
Some(literal(
":add-items <cat> a b c ...",
Some(ex!("add-cat <name>", "Add a new category")),
Some(ex!("add-item <cat> <item>", "Add one item to a category",)),
Some(ex!(
"add-items <cat> <item> ...",
"Add multiple items at once",
)),
Some(literal(
":formula <cat> \"<Name> = <expr>\"",
"Add a formula",
)),
Some(ex!("formula <cat> \"<Name> = <expr>\"", "Add a formula",)),
Some(HelpLine::Blank),
Some(HelpLine::Heading("Views")),
Some(HelpLine::Blank),
Some(literal(":add-view [name]", "Create a new view")),
Some(ex!("add-view [name]", "Create a new view")),
Some(HelpLine::Blank),
Some(HelpLine::Heading("Display")),
Some(HelpLine::Blank),
Some(literal(
":set-format <fmt>",
"Set number format (e.g. ',.2')",
)),
Some(ex!("set-format <fmt>", "Set number format (e.g. ',.2')",)),
Some(HelpLine::Blank),
Some(HelpLine::Heading("Other keys")),
Some(HelpLine::Blank),
keys(n, "enter-mode", Some(&["help"]), "Open this help screen"),
Some(literal(":help", "Open this help screen")),
Some(ex!("help", "Open this help screen")),
]
.into_iter()
.flatten()
@@ -884,8 +969,12 @@ mod tests {
for page in &pages {
assert!(!page.title.is_empty());
for line in &page.lines {
if let HelpLine::Keys { keys, description } = line {
assert!(!keys.is_empty(), "{description} resolved to no keys");
if let HelpLine::Keys(row) = line {
assert!(
!row.keys().is_empty(),
"{} resolved to no keys",
row.description()
);
rows += 1;
}
}
@@ -896,6 +985,61 @@ mod tests {
);
}
/// improvise-wk18: a key the pages name is looked up, never typed. The only
/// hand-written row left is an ex-command. See `context/test-evidence/protocol.md`.
#[test]
fn no_help_row_spells_a_key_by_hand() {
let spelled: Vec<&str> = pages()
.iter()
.flat_map(|page| page.lines.iter())
.filter_map(|line| match line {
HelpLine::Literal { keys, .. } => Some(*keys),
_ => None,
})
.filter(|keys| !keys.trim_start().starts_with(':'))
.collect();
assert!(
spelled.is_empty(),
"these rows name keys the keymap never saw: {spelled:?}"
);
}
/// improvise-v29m: `ex!` refuses a usage with a bare word in it at compile
/// time; this pins the rule it asks. See `context/test-evidence/protocol.md`.
#[test]
fn an_ex_row_can_name_placeholders_and_nothing_else() {
for usage in [
"w [path]",
"q!",
"formula <cat> \"<Name> = <expr>\"",
"add-items <cat> <item> ...",
] {
assert!(ex_usage_is_placeholders(usage), "{usage} is refused");
}
for usage in ["q or press Q", "add-items <cat> a b c", "help ?"] {
assert!(!ex_usage_is_placeholders(usage), "{usage} is accepted");
}
}
/// A row whose lookup finds no key is dropped silently, so a broken lookup
/// looks like a shorter page. See `context/test-evidence/protocol.md`.
#[test]
fn every_keys_row_the_pages_ask_for_finds_its_keys() {
let asked = help_bindings();
assert!(asked.len() > 20, "only {} rows asked", asked.len());
let dropped: Vec<String> = asked
.iter()
.filter(|(mode, command, args, description)| {
let args: Option<Vec<&str>> = args
.as_ref()
.map(|a| a.iter().map(String::as_str).collect());
keys(mode, command, args.as_deref(), description).is_none()
})
.map(|(mode, command, _, _)| format!("{mode:?} {command}"))
.collect();
assert!(dropped.is_empty(), "rows with no key: {dropped:?}");
}
/// `compact`'s three shapes, including the one no shipped mode reaches:
/// keys that are all chords leave nothing to draw.
#[test]
@@ -966,7 +1110,7 @@ mod tests {
.into_iter()
.flat_map(|page| page.lines)
.filter_map(|line| match line {
HelpLine::Keys { keys, description } => Some((keys, description)),
HelpLine::Keys(row) => Some((row.keys().to_vec(), row.description())),
_ => None,
})
.collect()
@@ -998,16 +1142,26 @@ mod tests {
/// See `context/test-evidence/clients.md`.
#[test]
fn only_the_edit_key_is_captioned_as_editing_the_cell() {
let edit = labels_of("edit-or-drill", Some(&["editing".to_string()]));
let claims: Vec<_> = rows()
let normal: Vec<_> = help_bindings()
.into_iter()
.filter(|(_, d)| d.contains("Edit current cell"))
.filter(|(mode, ..)| *mode == ModeKey::Normal)
.collect();
assert!(!claims.is_empty(), "the check needs a row to check");
for (keys, description) in claims {
// By COMMAND, not phrase (improvise-jhdf): any wording of "edit" on a
// Normal row that is not edit-or-drill's is the lie s8h7 removed.
let liars: Vec<_> = normal
.iter()
.filter(|(_, command, _, d)| {
d.to_lowercase().contains("edit") && *command != "edit-or-drill"
})
.collect();
assert!(
liars.is_empty(),
"captioned as editing but do not: {liars:?}"
);
for command in ["edit-or-drill", "enter-advance"] {
assert!(
keys.iter().all(|k| edit.contains(k)),
"{keys:?} is captioned {description:?} but only {edit:?} edit"
normal.iter().any(|(_, c, ..)| *c == command),
"{command} has no Normal help row, so the check above cannot see it"
);
}
}
@@ -1024,8 +1178,8 @@ mod tests {
println!("\n=== {} ===", page.title);
for line in &page.lines {
match line {
HelpLine::Keys { keys, description } => {
println!(" {:<28}{description}", keys.join(" "))
HelpLine::Keys(row) => {
println!(" {:<28}{}", row.keys().join(" "), row.description())
}
HelpLine::Literal {
keys, description, ..
+26
View File
@@ -235,6 +235,32 @@ impl Keymap {
keys
}
/// Every two-key sequence here or in a parent that runs `command`: a
/// prefix key, then a key of the sub-keymap it opens (flat, gotcha 13).
pub fn prefixed_keys_running(
&self,
command: &str,
args: Option<&[String]>,
) -> Vec<[(KeyCode, KeyModifiers); 2]> {
let mut keys: Vec<[(KeyCode, KeyModifiers); 2]> = self
.bindings
.iter()
.filter_map(|(pattern, binding)| match (pattern, binding) {
(KeyPattern::Key(code, mods), Binding::Prefix(sub)) => Some((*code, *mods, sub)),
_ => None,
})
.flat_map(|(code, mods, sub)| {
sub.keys_running(command, args)
.into_iter()
.map(move |second| [(code, mods), second])
})
.collect();
if let Some(parent) = &self.parent {
keys.extend(parent.prefixed_keys_running(command, args));
}
keys
}
/// Look up the binding for a key in this keymap's own bindings.
fn lookup_local(&self, key: KeyCode, mods: KeyModifiers) -> Option<&Binding> {
self.bindings
+291 -48
View File
@@ -2044,18 +2044,60 @@ mod tests {
// ── what is bound and what is not ───────────────────────────────
/// A three-by-three grid with `id` at (1,1) and the cursor on it: no move
/// from there is clamped by an edge (improvise-6562).
fn centred_on(id: CellId) -> ClientState {
let mut centre = CellDisplay::number("1200");
centre.id = Some(id);
let mut cells: Vec<_> = (0..3)
.flat_map(|r| (0..3).map(move |c| ((r, c), CellDisplay::number("7"))))
.collect();
cells[4].1 = centre;
let mut client = ClientState::default();
client.bootstrap(Snapshot {
view: ViewState::default(),
seed: vec![
Projection::Grid(GridUpdate::RowFrame(AxisFrame::flat([
"North", "South", "East",
]))),
Projection::Grid(GridUpdate::ColFrame(AxisFrame::flat([
"Revenue", "Cost", "Profit",
]))),
Projection::Grid(GridUpdate::Cells(cells)),
Projection::Grid(GridUpdate::Extent(GridExtent { rows: 3, cols: 3 })),
Projection::Basis(published()),
],
});
client.window = (3, 3);
client.view.cursor_mut().selected = (1, 1);
client
}
/// A client whose every widget has something in it, so an `Upstream` names
/// an **unhandled command** and never a state that said no (improvise-rmvx).
fn furnished(mode: AppMode) -> ClientState {
let mut state = client_on(record_cell());
furnished_on(mode, record_cell())
}
/// `furnished`, with `cell` under the cursor. A history in both directions
/// and a yanked value, so no `Cmd` is refused by an empty stack (3t5x).
fn furnished_on(mode: AppMode, cell: CellId) -> ClientState {
let mut state = centred_on(cell);
let frame = |name: &str| crate::ViewFrame {
view_name: name.into(),
mode: AppMode::Normal,
};
state.view.view_back_stack = vec![frame("Second")];
state.view.view_forward_stack = vec![frame("Third")];
state.view.yanked = Some(CellValue::Number(5.0));
// A records MODE over a non-records GRID is a state no host can
// produce, so the fixture projects both (improvise-e5pf).
crate::WidgetCache::apply(&mut state.grid, &GridUpdate::Records(mode.is_records()));
state.view.mode = mode;
state.grid.page_coords = vec![crate::PageAxis {
cat: "Quarter".into(),
selected: "Q1".into(),
items: Some(vec!["Q1".into(), "Q2".into()]),
selected: "Q2".into(),
items: Some(vec!["Q1".into(), "Q2".into(), "Q3".into()]),
}];
// THREE of everything, every panel cursor seated in the MIDDLE: an edge
// makes `move-panel-cursor` answer `Ignored` for the wrong reason.
@@ -2087,10 +2129,15 @@ mod tests {
axes: Vec::new(),
})
.collect();
state.tiles.tiles = vec![crate::TileEntry {
category: "Region".into(),
axis: Axis::Row,
}];
state.tiles.tiles = ["Region", "Product", "Quarter"]
.into_iter()
.map(|category| crate::TileEntry {
category: category.into(),
axis: Axis::Row,
})
.collect();
state.view.tile_cat_idx = 1;
state.view.help_page = 1;
state.view.cat_panel_cursor = 1;
state.view.formula_cursor = 1;
state.view.view_panel_cursor = 1;
@@ -2107,59 +2154,255 @@ mod tests {
state
}
/// improvise-rmvx. An advertised key that resolves upstream is a black hole
/// (gotcha 47). See `context/test-evidence/clients.md`.
/// What a user sees an advertised key do. A status alone is `Declines`:
/// honest, but not what the caption promised (improvise-3t5x).
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
enum Heard {
Acts,
Declines,
Silent,
}
/// Judged by what the fold MOVED, never by a command's type: a clamped move
/// or a lone disarm changes nothing (improvise-6562). `Model` is sent.
fn heard(state: &mut ClientState, outcome: KeyOutcome) -> Heard {
let commands = match outcome {
KeyOutcome::Upstream(_) | KeyOutcome::Ignored => return Heard::Silent,
KeyOutcome::Request(_) => return Heard::Acts,
KeyOutcome::Local(commands) => commands,
};
let quiet = |view: &ViewState| ViewState {
status_msg: String::new(),
transient_prefix: None,
..view.clone()
};
let before = quiet(&state.view);
commands.iter().for_each(|c| state.fold(c));
let sent = commands.iter().any(|c| matches!(c, Command::Model { .. }));
let said = commands.iter().any(|c| matches!(c, Command::SetStatus(_)));
match (sent || quiet(&state.view) != before, said) {
(true, _) => Heard::Acts,
(false, true) => Heard::Declines,
(false, false) => Heard::Silent,
}
}
/// Every `(mode, search, command, args)` either table advertises: the
/// status-bar hint and the help pages (improvise-wk18).
fn advertised_everywhere() -> Vec<(AppMode, bool, &'static str, Option<Vec<String>>)> {
let mut rows = Vec::new();
for mode in AppMode::all() {
for (command, args, _) in crate::help::advertised(&mode) {
rows.push((mode.clone(), false, command, args));
}
}
for (key, command, args, _) in crate::help::help_bindings() {
let (mode, search) = match key {
ModeKey::SearchMode => (AppMode::Normal, true),
key => match AppMode::all()
.into_iter()
.find(|m| ModeKey::from_app_mode(m, false).as_ref() == Some(&key))
{
Some(mode) => (mode, false),
None => panic!("a help row names {key:?}, which no mode reaches"),
},
};
rows.push((mode, search, command, args));
}
rows.sort_by_key(|(m, s, c, a)| (crate::mode_label(m), *s, *c, a.clone()));
rows.dedup();
rows
}
/// Every key sequence that runs `command` in `keymap`: one key, or a
/// prefix and the key after it.
fn sequences_running(
keymap: &Keymap,
command: &str,
args: Option<&[String]>,
) -> Vec<Vec<KeyEvent>> {
let one = keymap
.keys_running(command, args)
.into_iter()
.map(|(code, mods)| vec![KeyEvent::new(code, mods)]);
let two = keymap
.prefixed_keys_running(command, args)
.into_iter()
.map(|pair| pair.map(|(code, mods)| KeyEvent::new(code, mods)).to_vec());
one.chain(two).collect()
}
/// What pressing `sequence` in `state` does, arming any prefix first.
fn press(mut state: ClientState, sequence: &[KeyEvent]) -> Heard {
let (last, prefix) = sequence.split_last().expect("a sequence has a key");
for key in prefix {
if let KeyOutcome::Local(commands) = resolve(&state, *key) {
commands.iter().for_each(|c| state.fold(c));
}
}
let outcome = resolve(&state, *last);
heard(&mut state, outcome)
}
/// improvise-rmvx, wk18, 3t5x. Every advertised key does what it says, or
/// is named below saying why not. See `context/test-evidence/clients.md`.
#[test]
fn every_advertised_command_answers_in_its_own_mode() {
let mut unanswered = Vec::new();
for mode in AppMode::all() {
let Some(mode_key) = ModeKey::from_app_mode(&mode, false) else {
continue;
let aggregate = CellId::Aggregate(
serde_json::from_str("\"aa4c8e5e9cbd578e0d388f4b2b5a7879\"").unwrap(),
);
let derived = CellId::Derived {
target: "Profit".into(),
};
// (mode, cell, command, heard) -> the keys that heard it.
let mut short: std::collections::BTreeMap<(String, &str, Heard), Vec<String>> =
Default::default();
for (mode, search, command, args) in advertised_everywhere() {
let key = ModeKey::from_app_mode(&mode, search).expect("a keyed mode");
let keymap = Keymaps::shared().get(&key).expect("a keymap");
let at = |cell: &CellId| {
let mut state = furnished_on(mode.clone(), cell.clone());
state.view.search_mode = search;
state
};
let Some(keymap) = Keymaps::shared().get(&mode_key) else {
continue;
};
let state = furnished(mode.clone());
for (command, args, caption) in crate::help::advertised(&mode) {
for (code, mods) in keymap.keys_running(command, args.as_deref()) {
let event = KeyEvent::new(code, mods);
// BOTH silent variants. To a user `Ignored` is the same
// black hole as `Upstream` — nothing is on the screen.
let answered = !matches!(
resolve(&state, event),
KeyOutcome::Upstream(_) | KeyOutcome::Ignored
);
if !answered {
unanswered.push(format!(
"{}: {:?} → `{command} {args:?}` ({caption})",
crate::mode_label(&mode),
code
));
let label = crate::mode_label(&mode).to_string() + if search { " SEARCH" } else { "" };
for sequence in sequences_running(keymap, command, args.as_deref()) {
let keys: String = sequence
.iter()
.map(|k| crate::keymap::key_label(&k.code, k.modifiers))
.collect();
let on_record = press(at(&record_cell()), &sequence);
let mut report = vec![(label.clone(), on_record)];
// Only a grid mode stands on another kind of cell, and a cell
// matters only where it changes the answer.
if mode == AppMode::Normal {
for (on, cell) in [
(" on an aggregate", &aggregate),
(" on a derived cell", &derived),
] {
let got = press(at(cell), &sequence);
if got != on_record {
report.push((label.clone() + on, got));
}
}
}
for (where_, got) in report.into_iter().filter(|(_, got)| *got != Heard::Acts) {
short
.entry((where_, command, got))
.or_default()
.push(keys.clone());
}
}
}
unanswered.sort();
unanswered.dedup();
// The five that remain, each with the reason it is not a black hole.
// Closing one has to come here and say so; opening a sixth fails.
let expected = [
let short: Vec<String> = short
.into_iter()
.map(|((at, command, got), mut keys)| {
keys.sort();
keys.dedup();
format!("{at}: {} {command} {got:?}", keys.join("/"))
})
.collect();
// Closing one has to come here and say so; adding one fails. A caption
// is a promise, and each line here is a promise a browser does not keep.
let expected: &[&str] = &[
// Reached only from a mode `declined` refuses at the door, so a
// browser is never standing in front of this hint.
"COMMAND: Enter → `execute-command None` (run)",
"EXPORT: Enter → `commit-export None` (export)",
"SAVE AS: Enter → `commit-save None` (save)",
// browser is never standing in front of these.
"COMMAND: Enter execute-command Silent",
"EXPORT: Enter commit-export Silent",
"SAVE AS: Enter commit-save Silent",
"NORMAL SEARCH: Enter/Esc exit-search-mode Silent",
// Commands the browser has no arm for: upstream, so a black hole
// (improvise-uln5). Each is also in the host-only list below.
"NORMAL: Enter enter-advance Silent",
"NORMAL: H hide-selected-row-item Silent",
"NORMAL: / search Silent",
"NORMAL: n search-navigate Silent",
"NORMAL: z toggle-group-under-cursor Silent",
// Honestly unavailable, and saying so in `declined`'s words.
"FORMULAS: Del/d delete-formula-at-cursor Declines",
"NORMAL: > drill-into-cell Declines",
"NORMAL: : enter-mode Declines",
"NORMAL: P toggle-prune-empty Declines",
"NORMAL: R toggle-records-mode Declines",
"NORMAL: ZZ wq Declines",
"RECORDS: o add-record-row Declines",
"RECORDS: a/i edit-or-drill Declines",
"RECORDS: : enter-mode Declines",
"RECORDS: P toggle-prune-empty Declines",
"RECORDS: R toggle-records-mode Declines",
// `i` on an aggregate IS drill, which the browser lacks (s8h7); on a
// derived cell the browser names the formula instead (ir7l).
"NORMAL on an aggregate: a/i edit-or-drill Declines",
"NORMAL on a derived cell: a/i edit-or-drill Declines",
// No record under the cursor to write. The TUI writes onto the
// aggregate's partial key, which is its own bug (improvise-l4mp).
"NORMAL on an aggregate: p paste Silent",
"NORMAL on an aggregate: x clear-cell Silent",
"NORMAL on a derived cell: p paste Silent",
"NORMAL on a derived cell: x clear-cell Silent",
// A records cell commit needs `record_col` (gotcha 1); only the two
// creating columns resolve, and this fixture is on a record cell.
"RECORDS INSERT: Enter → `commit-cell-edit None` (commit)",
"RECORDS INSERT: Tab → `commit-and-advance-right None` (commit+right)",
"RECORDS INSERT: Tab commit-and-advance-right Silent",
"RECORDS INSERT: Enter commit-cell-edit Silent",
];
let mut expected: Vec<String> = expected.iter().map(|s| s.to_string()).collect();
expected.sort();
let mut short = short;
short.sort();
assert_eq!(
unanswered, expected,
"advertised and unanswerable: anything here that is not in the \
named list above reads as broken rather than as unavailable"
short,
expected,
"advertised keys that do not do what they say, in full:\n{}",
short.join("\n")
);
}
/// improvise-6562: "it moved" cannot see a move the WRONG way, so every grid
/// key is pressed from the centre and must land where it says. Evidence file.
#[test]
fn every_grid_move_lands_where_its_binding_says() {
let keymap = Keymaps::shared().get(&ModeKey::Normal).expect("Normal");
let mut moves = Vec::new();
bound_commands(keymap, &mut moves);
// A move's destination is its own args; a jump's is its name.
let mut expected: Vec<(String, Vec<String>, (usize, usize))> = moves
.into_iter()
.filter(|(name, _)| name == "move-selection")
.map(|(name, args)| {
let d = |i: usize| args[i].parse::<isize>().expect("a delta");
let to = ((1 + d(0)) as usize, (1 + d(1)) as usize);
(name, args, to)
})
.collect();
for (name, to) in [
("jump-first-row", (0, 1)),
("jump-last-row", (2, 1)),
("jump-first-col", (1, 0)),
("jump-last-col", (1, 2)),
] {
expected.push((name.to_string(), Vec::new(), to));
}
let mut pressed = 0;
for (name, args, to) in expected {
let wanted = (!args.is_empty()).then_some(args.as_slice());
for sequence in sequences_running(keymap, &name, wanted) {
let mut state = furnished(AppMode::Normal);
for key in &sequence {
if let KeyOutcome::Local(commands) = resolve(&state, *key) {
commands.iter().for_each(|c| state.fold(c));
}
}
assert_eq!(
state.view.cursor().selected,
to,
"{sequence:?} runs `{name} {args:?}` from (1,1)"
);
pressed += 1;
}
}
assert!(
pressed >= 14,
"only {pressed} keys pressed; the walk went blind"
);
}
@@ -2431,8 +2674,8 @@ mod tests {
not vacuous over a table that binds everything or nothing"
);
assert_eq!(
composed(&state, ev(KeyCode::Char(',')), "view-forward", &[]),
Some(vec![Command::ViewForward]),
resolve(&state, ev(KeyCode::Char(','))),
KeyOutcome::Local(vec![Command::ViewForward]),
"and the key is `,` — the unshifted twin of `<`, so both halves of \
the history sit on one physical key"
);
+3 -3
View File
@@ -427,9 +427,9 @@ impl RenderHelpRow {
L::Heading(t) => prose("heading", t),
L::Text(t) => prose("text", t),
L::Accent(t) => prose("accent", t),
L::Keys { keys, description } => Self {
keys: keys.join(" "),
description: description.to_string(),
L::Keys(row) => Self {
keys: row.keys().join(" "),
description: row.description().to_string(),
kind: "keys",
},
L::Literal {
+3 -3
View File
@@ -69,9 +69,9 @@ fn page_content(page: usize) -> Vec<Row> {
HelpLine::Heading(text) => plain(text, s.heading),
HelpLine::Text(text) => plain(text, s.normal),
HelpLine::Accent(text) => plain(text, s.accent),
HelpLine::Keys { keys, description } => Row {
key: format!(" {}", keys.join(" ")),
desc: description,
HelpLine::Keys(row) => Row {
key: format!(" {}", row.keys().join(" ")),
desc: row.description(),
style: s.key,
},
HelpLine::Literal {