Merge gh/main into fix/uo2p-drill-apply-order

Resolved context/test-evidence/command.md by keeping both appended sides.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-22 22:51:43 -07:00
14 changed files with 1120 additions and 177 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.
+71 -6
View File
@@ -793,12 +793,40 @@ is gone rather than joined by a second one. A guard spelled here as
copy of `switch_to_view`'s own condition, free to disagree with it the day that
condition grows a clause.
Not fixed, and reachable only by construction: `ViewBack` and `ViewForward` call
`switch_to_view` just as unguardedly, pushing the current view onto the opposite
stack before the switch they cannot be sure of. They are safe **because
`ViewState::forget_view` purges both stacks when a view is deleted**, so a popped
frame always names a live view — a guard living one level away from the hazard,
which is the shape that keeps returning here.
`ViewBack` and `ViewForward` have the same shape now (improvise-8nd9). They used to
push the current view onto the opposite stack **before** a switch they could not be
sure of, and were safe only because `ViewState::forget_view` purges both stacks when
a view is deleted — a guard living one level away from the hazard. improvise-lrrx
proved the guard incomplete: `App::install_workbook` (`LoadModel`,
`AcceptWizardModel`) cleared `cursors` and not the history, so `<` after a load
walked the **outgoing** document's frames, landing on a view of the same name in the
incoming one that this client had never visited, or on a refused switch that still
left a phantom frame behind. Both halves are fixed, so neither relies on the other:
`install_workbook` clears both stacks beside `cursors`, for the same reason, and the
two steps go through `stepped_to`, which switches, observes whether the document
**accepted** the view (`App::switch_to_view` now returns that), and only then
restores the frame's mode and hands back the frame just left. A frame the document
refuses is consumed and leaves nothing.
The observation is *accepted*, not *moved*, and that is the one place the sibling
shapes differ. `SwitchView` asks "did `active_view` change" because asking for the
view you are on is not a move. A history frame is a **place**, meaning a view and a
mode, so a live frame naming the view you are on still restores its mode. Such
frames exist: `forget_view` collapses `[A, B, A]` to `[A, A]`. The client's
`reduce_view::stepped` also steps onto them unconditionally. A "moved" test
would therefore drop a frame the client keeps, and the host and the client
would disagree. The first version of this fix made exactly that mistake.
`colon_view_back_respects_whether_history_exists` caught it, because its
fixture is such a frame. `reduce_view`'s `stepped` is unchanged, because the
client holds no `Workbook` to observe against. That leaves the host and the
client able to disagree wherever a client's stack holds a frame the document
would refuse. An earlier version of this paragraph said such a frame could not
exist, because `ForgetView` purges deleted views and a load arrives as a fresh
`Snapshot`. **That is false.** `project.rs::fresh_view` starts from
`ViewState::default()` and folds `SwitchView(active)`, so every connected or
rebased client gets a phantom `{Default, Normal}` back-frame, and `Default`
need not exist in the document. That is improvise-lxg0, and until it is fixed
the divergence is live rather than theoretical.
### `ReturnToNormalUnlessMoved` — observation, not prediction (improvise-toc)
@@ -2894,6 +2922,43 @@ flag: tidy-up goes where an abort cannot reach it.
stays in `TileSelect` mode so the user can adjust several tiles. `axis: None` cycles;
`axis: Some(a)` sets.
**The tile bar offers `_Index` and `_Dim`, and that is deliberate (improvise-jgzw).**
Every tile call site passes `Model::category_names()`, virtual categories included,
so tile cursor 0 is `_Index`. The ticket asked whether that was a leak. It is not,
for three reasons, each checked rather than argued:
- **It is a decision the user already made.** improvise-kos: "`_Dim` and `_Index`
should work as pivot axes outside Records/Drill view". A category that works as a
pivot axis is one the axis picker offers.
- **The tile bar is the only way out of a records layout that was saved.**
improvise-jgzw's "check what it does" found that hand-assigning `_Index` → Row and
`_Dim` → Column in a real view (tile bar or `:set-axis`) is **written** to the
`.improv`, because a real view persists every axis, and it reopens as records mode.
(`VIRTUAL_VIEW_NAMES`' "never-persisted `_Index`/`_Dim` on axes" is true only of
`_Records`/`_Drill`.) In that document, `R` (`toggle-records-mode`) emits
`ViewBack` on an empty back stack, so it leaves the layout as it was and sets the
status to "Pivot mode". That is filed as improvise-gj5p. Moving the two axes back through
the tile bar is what gets out. In the browser, which declines
`toggle-records-mode`, the tile bar is also the only way in, and improvise-bd9o
already made that route safe instead of closing it
(`the_tile_bar_route_into_records_mode_does_not_strand_the_user`).
- **What filtering would cost was measured.** With `View::tileable_cats` filtering
on `CategoryKind::is_index_or_dim`, that bd9o test and the TUI's
`renders_each_tile_with_its_axis_label` fail, as well as the two characterization
tests here.
What the default cursor does cost, stated: one cycle key on tile 0 puts `_Index`
on Row alone, and in a pivot view that empties the grid (0 rows on
`web/demo.improv`). The status line says `_Index → Row`, and three more presses
undo it. If that turns out to matter, the fix is the order the tile bar lists
categories in, not taking entries out of it.
**`_Measure` must stay tileable.** It is on an axis in every shipped document, so
`regular_category_names()`, the advice for *prompts*, is the wrong filter here.
`the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included`
names the whole offered list with `_Measure` on a column, and fails if either
decision changes.
### `cmd/effect_cmds.rs`
The parseable model-mutation commands are thin `Cmd` wrappers around effects,
+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.
+75
View File
@@ -5150,6 +5150,57 @@ the list. Its seed is exact (`"007"`), but `typed()` classifies the committed
text as `Number(7.0)`. Probed here: both tests fail on it in both modes. That
is the classifier, not the seed, and it is improvise-ixxx.
### loading_a_document_forgets_the_outgoing_documents_view_history
improvise-lrrx: `App::install_workbook` cleared `cursors` but not
`view_back_stack`/`view_forward_stack`, so `<` after `LoadModel` walked the
outgoing document's views inside the incoming one. The history is two deep
(`Default`, `X`, then on `Z`), and the incoming document **has** a view named `X`
but opens on `Y`. Without that, a stale frame is a refused switch and looks the
same as "nothing happened" (the ticket's own fixture warning). Against the unfixed
code, `<` landed on `X`, a view this client never visited in the new document.
Rival injected: the two `clear()` lines removed from `install_workbook`, with the
improvise-8nd9 fix still in place. This test and its wizard twin failed at the
`view-back walked the outgoing document's history` assertion.
### accepting_an_import_forgets_the_outgoing_documents_view_history
improvise-lrrx, through the second door into `install_workbook`. A
wizard-built workbook has only `Default`, so the test creates an `X` in it after
accepting. Without that, the stale `X` frame would be refused and could not be
told apart from a cleared history. Same rival and same failure as the load test.
### a_step_back_the_document_refuses_leaves_no_frame_behind
improvise-8nd9: `ViewBack` pushed the current view onto the forward stack before
the guarded `switch_to_view`, so a frame naming a view the document lacks left a
phantom behind, and `,` would then "go forward" to where it already was. The
dead frame is pushed by hand, standing for any path that forgets
`forget_view`, with a live frame under it and the frame's mode set to `Help` so
that a restored mode is observable. Two rivals. First, the original code, which
pushes first and restores the mode unconditionally: it fails at `nothing moved,
so no mode`. Second, a push-first version that restores the mode only on a move,
so the mode assertion cannot hide the stack assertion: it fails at `a refused
step left X on the forward stack`. The last two assertions check that the live
frame beneath still works, so a fix that drops the whole stack cannot pass.
### a_step_back_to_the_view_you_are_on_is_the_step_the_client_folds
improvise-8nd9. This test holds the fix to the client's `reduce_view::stepped`.
A live frame naming the view you are on (a `forget_view` collapse leaves one)
is still a step. It restores its mode and leaves a forward frame, exactly as
the client folds it. Rival injected: `stepped_to` observing whether
`active_view` *changed* rather than whether the document *accepted* the view.
That was the first version of this fix. This test failed at the
mode assertion, as did
`command::cmd::text_buffer::tests::colon_view_back_respects_whether_history_exists`.
That test's fixture is such a frame, and it is what caught the first version.
### a_step_forward_the_document_refuses_leaves_no_frame_behind
The `ViewForward` half of the test above, with the stacks' roles swapped. It fails
against the unfixed code at `nothing moved, so no mode`.
### yy_then_p_in_records_mode_copies_between_real_records
improvise-6p8q. At `gh/main` `9e78051` it failed at the yank: `left: None,
@@ -5319,3 +5370,27 @@ is asserted over all 24 orders of four edits on one row: two renames, a measure
write and a measure clear. The test pins one value as well as the equality, so
a rewrite that is the same wrong answer in every order still fails. Rival 3
fails it.
## `command/cmd/tile.rs` — the tile bar's offered set, by name (improvise-jgzw)
### the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included
This test pins a decision, not a bug. The argument is in `context/code/command.md`
under `cmd/tile.rs`. Before this test, no tile assertion named the whole offered
set. The two older ones named only the category at cursor 0, so a filter that
changed what the bar offers past the cursor was invisible. The fixture puts
`_Measure` on **Column**, as every shipped document does. The test then walks the
cursor past the end and asserts the exact list
`[_Index, _Dim, _Measure, Type, Month]`.
Rivals, injected, full workspace run:
- **`regular_category_names()` in `TileAxisOp`**, the obvious "no virtual
categories in pickers" fix, which drops `_Measure` too. It fails this test
(`["Type", "Month"]`), the two characterization tests above it, and
`lockstep.rs::a_locally_resolved_tile_axis_change_agrees_with_the_host`.
- **`View::tileable_cats` filtering `is_index_or_dim`**, the ticket's option (b).
With the list asserted this test fails by construction, so the point is what
*else* fails: `renders_each_tile_with_its_axis_label` (TUI) and
`the_tile_bar_route_into_records_mode_does_not_strand_the_user`. The second one
is improvise-bd9o's own test of the route this decision keeps.
+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.
@@ -59,6 +59,26 @@ mod tests {
assert!(effects.is_empty());
}
/// improvise-jgzw: `_Index`/`_Dim` stay tileable on purpose, and `_Measure` on
/// a column must too — `context/test-evidence/command.md`.
#[test]
fn the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included() {
let mut m = two_cat_model();
m.active_view_mut().set_axis("_Measure", Axis::Column);
let layout = make_layout(&m);
let reg = make_registry();
let mut ctx = make_ctx(&m, &layout, &reg);
let mut offered = Vec::new();
for idx in 0..8 {
ctx.tile_cat_idx = idx;
let dbg = effects_debug(&TileAxisOp { axis: None }.execute(&ctx));
if let Some(rest) = dbg.split("CycleAxis(Active, \"").nth(1) {
offered.push(rest.split('"').next().unwrap_or_default().to_string());
}
}
assert_eq!(offered, ["_Index", "_Dim", "_Measure", "Type", "Month"]);
}
#[test]
fn move_tile_cursor_right() {
let m = two_cat_model();
+9 -4
View File
@@ -444,6 +444,9 @@ impl App {
pub fn install_workbook(&mut self, workbook: Workbook) {
self.view_state.active_view = workbook.active_view.clone();
self.view_state.cursors.clear();
// The history names the outgoing document's views too (improvise-lrrx).
self.view_state.view_back_stack.clear();
self.view_state.view_forward_stack.clear();
self.model_state.workbook = workbook;
// Every client's mirror is scoped to the outgoing document too, and an
// effect cannot reach the session list. The generation is how it hears.
@@ -581,12 +584,14 @@ impl App {
.flat_map(move |r| (0..layout.col_count()).filter_map(move |c| layout.cell_key(r, c)))
}
/// Point **this client's** slice at `name`, if the document has such a view.
/// The document's own default stays put (improvise-jytm, gotcha 29).
pub fn switch_to_view(&mut self, name: &str) {
if self.model_state.workbook.views.contains_key(name) {
/// Point **this client's** slice at `name`, if the document has such a view,
/// and say whether it did. The document's default stays put (gotcha 29).
pub fn switch_to_view(&mut self, name: &str) -> bool {
let accepted = self.model_state.workbook.views.contains_key(name);
if accepted {
self.view_state.active_view = name.to_string();
}
accepted
}
/// What `name` names in this document, falling back to the default for a name
+187 -16
View File
@@ -389,19 +389,30 @@ impl Effect for SwitchView {
}
}
/// Switch to `frame`'s view and restore its mode, returning the frame just left
/// — or `None`, touching nothing, if the document refused the view.
fn stepped_to(app: &mut App, frame: ViewFrame) -> Option<ViewFrame> {
let left = ViewFrame {
view_name: app.view_state.active_view.clone(),
mode: app.view_state.mode.clone(),
};
app.switch_to_view(&frame.view_name).then(|| {
app.view_state.mode = frame.mode;
left
})
}
/// Go back in view history (pop back stack, push current to forward stack).
#[derive(Debug)]
pub struct ViewBack;
impl Effect for ViewBack {
fn apply(&self, app: &mut App) {
if let Some(frame) = app.view_state.view_back_stack.pop() {
let current = app.view_state.active_view.clone();
app.view_state.view_forward_stack.push(ViewFrame {
view_name: current,
mode: app.view_state.mode.clone(),
});
app.switch_to_view(&frame.view_name);
app.view_state.mode = frame.mode;
// Observed, as `SwitchView` is: a frame the document refuses is dropped
// and leaves nothing behind (improvise-8nd9, `context/code/command.md`).
if let Some(frame) = app.view_state.view_back_stack.pop()
&& let Some(left) = stepped_to(app, frame)
{
app.view_state.view_forward_stack.push(left);
}
}
fn kind(&self) -> EffectKind {
@@ -414,14 +425,11 @@ impl Effect for ViewBack {
pub struct ViewForward;
impl Effect for ViewForward {
fn apply(&self, app: &mut App) {
if let Some(frame) = app.view_state.view_forward_stack.pop() {
let current = app.view_state.active_view.clone();
app.view_state.view_back_stack.push(ViewFrame {
view_name: current,
mode: app.view_state.mode.clone(),
});
app.switch_to_view(&frame.view_name);
app.view_state.mode = frame.mode;
// The mirror of `ViewBack`, observed the same way (improvise-8nd9).
if let Some(frame) = app.view_state.view_forward_stack.pop()
&& let Some(left) = stepped_to(app, frame)
{
app.view_state.view_back_stack.push(left);
}
}
fn kind(&self) -> EffectKind {
@@ -1984,6 +1992,169 @@ mod tests {
assert_eq!(app.view_state.active_view, before);
}
/// A history two deep through `X` and `Z`: back is `[Default, X]`.
fn app_with_history(storage: Box<dyn crate::storage::Storage>) -> App {
let mut wb = Workbook::new("Outgoing");
wb.create_view("X");
wb.create_view("Z");
let mut app = App::with_storage(wb, None, storage);
for name in ["X", "Z"] {
SwitchView(name.to_string()).apply(&mut app);
}
assert_eq!(app.view_state.view_back_stack.len(), 2, "setup: two deep");
app
}
/// Asserts `<` and `,` leave `view` put and the history empty.
fn assert_history_went_with_the_document(app: &mut App, view: &str) {
assert_eq!(app.view_state.active_view, view, "setup: opened on {view}");
for (name, step) in [
("view-back", Box::new(ViewBack) as Box<dyn Effect>),
("view-forward", Box::new(ViewForward)),
] {
step.apply(app);
assert_eq!(
app.view_state.active_view, view,
"{name} walked the outgoing document's history in the incoming one"
);
}
assert!(app.view_state.view_back_stack.is_empty());
assert!(app.view_state.view_forward_stack.is_empty());
}
/// improvise-lrrx. Evidence: `context/test-evidence/command.md`.
#[test]
fn loading_a_document_forgets_the_outgoing_documents_view_history() {
use crate::storage::{MemoryFs, Storage};
let fs = MemoryFs::new();
// The incoming document HAS a view named `X`, and opens somewhere else.
let mut incoming = Workbook::new("Incoming");
incoming.create_view("X");
incoming.create_view("Y");
incoming.active_view = "Y".to_string();
let text = crate::persistence::format_md(&incoming);
fs.write(std::path::Path::new("b.improv"), text.as_bytes())
.unwrap();
let mut app = app_with_history(Box::new(fs));
LoadModel("b.improv".into()).apply(&mut app);
assert_eq!(
app.model_state.workbook.model.name, "Incoming",
"setup: loaded"
);
assert_history_went_with_the_document(&mut app, "Y");
}
/// improvise-lrrx: the wizard is the other door into `install_workbook`.
#[test]
fn accepting_an_import_forgets_the_outgoing_documents_view_history() {
let mut app = app_with_history(Box::new(crate::storage::MemoryFs::new()));
let mut wizard = crate::import::wizard::ImportWizard::new(serde_json::json!([
{"region": "East", "revenue": 100.0},
{"region": "West", "revenue": 200.0},
]));
wizard.advance();
wizard.advance();
wizard.build();
assert!(wizard.built_summary().is_some(), "setup: a built workbook");
app.wizard = Some(wizard);
AcceptWizardModel.apply(&mut app);
// The new document gains an `X` of its own, so a stale frame could land.
app.model_state.workbook.create_view("X");
let opened = app.view_state.active_view.clone();
assert_ne!(opened, "X", "setup: the import must not open on X");
assert_history_went_with_the_document(&mut app, &opened);
}
/// improvise-8nd9: a frame naming a view the document lacks is refused, and
/// a refused step leaves no frame behind. See `context/test-evidence/command.md`.
#[test]
fn a_step_back_the_document_refuses_leaves_no_frame_behind() {
let mut app = test_app();
app.model_state.workbook.create_view("X");
SwitchView("X".to_string()).apply(&mut app);
// A path that forgot `forget_view`: the history names a dead view.
app.view_state.view_back_stack.push(ViewFrame {
view_name: "Gone".to_string(),
mode: AppMode::Help,
});
ViewBack.apply(&mut app);
assert_eq!(
app.view_state.active_view, "X",
"setup: the step is refused"
);
assert_eq!(
app.view_state.mode,
AppMode::Normal,
"nothing moved, so no mode"
);
assert!(
app.view_state.view_forward_stack.is_empty(),
"a refused step left X on the forward stack: `,` would go nowhere"
);
ViewBack.apply(&mut app);
assert_eq!(
app.view_state.active_view, "Default",
"the live frame survives"
);
assert_eq!(app.view_state.view_forward_stack.len(), 1);
}
/// improvise-8nd9: a live frame naming the view you are on is still a step, as
/// it is in the client's `reduce_view`. See `context/test-evidence/command.md`.
#[test]
fn a_step_back_to_the_view_you_are_on_is_the_step_the_client_folds() {
let mut app = test_app();
app.model_state.workbook.create_view("X");
SwitchView("X".to_string()).apply(&mut app);
app.view_state.view_back_stack.push(ViewFrame {
view_name: "X".to_string(),
mode: AppMode::Help,
});
let mut mirror = app.view_state.clone();
ViewBack.apply(&mut app);
improvise_protocol::reduce_view(&mut mirror, &improvise_protocol::Command::ViewBack);
assert_eq!(
app.view_state.mode,
AppMode::Help,
"the frame's mode is restored"
);
assert_eq!(app.view_state.view_back_stack, mirror.view_back_stack);
assert_eq!(app.view_state.view_forward_stack, mirror.view_forward_stack);
assert_eq!(app.view_state.mode, mirror.mode);
}
/// improvise-8nd9: the forward half of the step above.
#[test]
fn a_step_forward_the_document_refuses_leaves_no_frame_behind() {
let mut app = test_app();
app.model_state.workbook.create_view("X");
SwitchView("X".to_string()).apply(&mut app);
ViewBack.apply(&mut app);
app.view_state.view_forward_stack.push(ViewFrame {
view_name: "Gone".to_string(),
mode: AppMode::Help,
});
ViewForward.apply(&mut app);
assert_eq!(app.view_state.active_view, "Default", "setup: refused");
assert_eq!(
app.view_state.mode,
AppMode::Normal,
"nothing moved, so no mode"
);
assert!(
app.view_state.view_back_stack.is_empty(),
"a refused step left Default on the back stack: `<` would go nowhere"
);
ViewForward.apply(&mut app);
assert_eq!(app.view_state.active_view, "X", "the live frame survives");
assert_eq!(app.view_state.view_back_stack.len(), 1);
}
#[test]
fn create_and_delete_view() {
let mut app = test_app();
+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 {