Merge pull request #71 from fiddlerwoaroof/wave/browser-keys

fix(web): an advertised key is never silent, a formula is visible, and +cat commits
This commit is contained in:
Ed Langley
2026-09-20 17:23:23 -07:00
committed by GitHub
12 changed files with 858 additions and 95 deletions
+94 -10
View File
@@ -456,7 +456,54 @@ immutable `CmdContext`; the commands are then folded in order.
`the_command_names_that_still_need_the_host_are_these` names them, and `the_command_names_that_still_need_the_host_are_these` names them, and
`Host::handle` answers a `Key` frame with an empty vec, so each one is a binding `Host::handle` answers a `Key` frame with an empty vec, so each one is a binding
that **does nothing at all** in a browser that **does nothing at all** in a browser
(`doctrine(an_upstream_key_is_a_black_hole)`). The classes: (`doctrine(an_upstream_key_is_a_black_hole)`).
**An upstream key that the app also ADVERTISES is a different severity, and it
had its own class of defect** (improvise-rmvx). A key nobody offers doing
nothing is a gap; a key named in the mode's own hint line doing nothing reads as
*broken*. The two are crossed by `every_advertised_command_answers_in_its_own
_mode`, which asks `help::advertised` — a table of command **names**, so the
cross-reference is exact and nothing tokenises the rendered string — against
`resolve` in every mode, over a fixture furnished enough that an `Upstream` can
only mean an unhandled name. The old guard asked only about `Normal`, which is
why `add-record-row` and `delete-formula-at-cursor` were outside its reach
entirely; `view-back` in RecordsNormal was a seventh instance nobody had
noticed.
**`Ignored` is a black hole too**, and the guard watches both variants.
`KeyOutcome::of` turning an empty `Local` into an honest `Ignored` is good
design and also a **laundering** step — every wrong answer that composes nothing
arrives wearing the variant that is usually innocent. A guard's discriminator
must therefore be *state* (did anything the user can see change) and never the
shape of the return value. Watching only `Upstream` let three of the six
declines go completely silent with 1397 Rust, 19 harness and 26 node tests
green, because no browser spec presses those three keys (PR #71 review).
`nothing_declined_is_also_composed` closes the other direction: `step` reaches
`declined` first, so an arm in `composed` for a declined name is **unreachable**,
and the browser would go on saying "not available yet" over working code. It is
derived from `declined` itself over every bound `(name, args)` in every mode, so
a seventh decline is covered without editing it. `enter-mode` was a real overlap
on its first run; rather than move the check, **both halves hold the rule** —
`composed` returns `None` for a stranded mode — so removing one fails towards
`Upstream` rather than towards a dead room.
`declined` is the answer, and it is one list rather than an arm apiece: six
command names, each with **its own true subject**. Wording matters here more
than it looks — the `>` fix was nearly routed through a message about
aggregates, which would have been true-sounding about the wrong thing and worse
than silence, because a reader who is told something plausible stops looking.
`step` checks `declined` first, so `composed` has no arm for any of them and the
two lists cannot overlap.
A declining step takes its **whole sequence** with it: `o` in RecordsNormal is
`[add-record-row, enter-edit-at-cursor]`, and the second step's premise is false
once the first has declined. Returning only the decline, rather than the decline
plus whatever composed before it, is what keeps `Binding::Sequence`'s "every
step or none of them" true — an abort is not a rollback (gotcha 21), and this
client has no way to undo a `Command` it has already put on the wire.
The classes that remain:
- **Prefix keys.** `Command::SetTransientPrefix` exists and this client could - **Prefix keys.** `Command::SetTransientPrefix` exists and this client could
arm one; what it cannot do is what the *second* key then runs, which is arm one; what it cannot do is what the *second* key then runs, which is
@@ -474,13 +521,23 @@ that **does nothing at all** in a browser
in the formula panel, not in the grid. in the formula panel, not in the grid.
- **Anything touching storage or an arbitrary command line.** The host's by - **Anything touching storage or an arbitrary command line.** The host's by
definition. definition.
- **The records-mode commit.** A wide-records row is one coordinate tuple and - **The records-mode commit** — *except the two creating columns*. A wide-records
editing a category column moves every measure cell in it; row is one coordinate tuple and editing a category column moves every measure
`commit_plain_records_edit` needs `GridLayout::record_col` to know which. cell in it; `commit_plain_records_edit` needs `GridLayout::record_col` to know
which. A `CellId::Create` does not: it commits the typed NAME as one existing
`ModelCommand` (`AddItem` into `_Measure`, or `AddCategory`), reads no
`record_col` and writes no cell, so improvise-gcvb narrowed the early return
rather than removing it. The `+cat` half **must not advance**: an inserted
column renumbers every index to its right and the advance was computed from
the pre-commit layout (gotcha 23), so `CommitAndAdvance` returns straight
after it and a client that advanced anyway would land one column off,
silently. `AddCategory` can also be refused by the 12-category limit and sets
`abort_effects`; the mode change therefore goes **first**, where an abort
cannot reach it (gotcha 21).
- **Drill.** Session-local residue, and the one case that does **not** go - **Drill.** Session-local residue, and the one case that does **not** go
upstream silently — see below. upstream silently — see below.
### `strands_the_user`: a prompt this client can enter but cannot commit ### `stranding`: a prompt this client can enter but cannot commit
`enter-mode` is trivially resolvable here, and that is the trap: the user `enter-mode` is trivially resolvable here, and that is the trap: the user
reaches the prompt, types a path, presses Enter — and the commit resolves to reaches the prompt, types a path, presses Enter — and the commit resolves to
@@ -497,10 +554,12 @@ meaningless: the worker persists on its own behalf whenever the document is
dirty, so the document *is* the OPFS entry and there is no Ctrl-S semantics to dirty, so the document *is* the OPFS entry and there is no Ctrl-S semantics to
offer. offer.
The predicate is named as a property rather than matched at the call site It returns the **subject** rather than a bool (improvise-rmvx): the refusal is
(`doctrine(two_questions_one_enum)`): "can this client finish what entering the right and the silence was not, so `:` now says the command line is unavailable
mode starts" is not the same question as "is this a text-entry mode", and a instead of doing nothing at all. The predicate is named as a property rather
third such prompt belongs here. Found by Copilot on PR #27 against the newly than matched at the call site (`doctrine(two_questions_one_enum)`): "can this
client finish what entering the mode starts" is not the same question as "is
this a text-entry mode", and a third such prompt belongs here. Found by Copilot on PR #27 against the newly
added `SavePrompt`; `ExportPrompt` had the identical trap already and was fixed added `SavePrompt`; `ExportPrompt` had the identical trap already and was fixed
with it — fixing one arm of a rule with two is the shape with it — fixing one arm of a rule with two is the shape
`doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)` names. `doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)` names.
@@ -946,6 +1005,19 @@ Four smaller decisions that would be re-derived wrongly:
broken grid. `editing` is `AppMode::is_editing()` plus the `edit` buffer: broken grid. `editing` is `AppMode::is_editing()` plus the `edit` buffer:
without it, a browser that resolved `i` locally would change its mode indicator without it, a browser that resolved `i` locally would change its mode indicator
and then swallow every keystroke invisibly. and then swallow every keystroke invisibly.
- **`minibuffer` is the same argument, for the other six text-entry modes**
(improvise-fdt6). `editing` drives the *inline cell editor*, so it is scoped to
the two cell-editing modes and reads the `"edit"` buffer by name — which left
FormulaEdit, CategoryAdd, ItemAdd, SavePrompt, ExportPrompt and CommandMode
opening a prompt and swallowing every character typed into it. `minibuffer` is
`AppMode::minibuffer()` plus **the buffer that config names**, read with
`improvise_protocol::read_buffer` so the `"search"` special case keeps its one
home (gotcha 11); `None` is "no text-entry mode is open", so the page asks
nothing about the mode. The colour stays semantic — `MinibufColor` crosses and
the CSS class is the page's last translation, exactly as `draw.rs::minibuf_fg`
is the TUI's (gotcha 26). It is drawn as a line of its own rather than *in
place of* `#status` as the TUI's bottom bar does, because `#status` also
carries the transport's failures and a repaint must not erase one (gotcha 48).
`RenderCell::edit_text` is the stored value, not the formatted one: seeding an `RenderCell::edit_text` is the stored value, not the formatted one: seeding an
editor from `text` and committing turns `1200` into the string `"1,200"`. The editor from `text` and committing turns `1200` into the string `"1,200"`. The
@@ -1308,13 +1380,14 @@ to check that `refresh` skips one, has to happen outside it.
**E2E and integration tests are rare and pin boundary contracts; unit tests **E2E and integration tests are rare and pin boundary contracts; unit tests
assume those contracts and test between them.** (User ruling, 2026-09-20.) So assume those contracts and test between them.** (User ruling, 2026-09-20.) So
the question is not "can this assertion be made cheaper" but "which boundary is the question is not "can this assertion be made cheaper" but "which boundary is
it about", and the browser half has six: it about", and the browser half has seven:
| boundary | the contract | where it is pinned | | boundary | the contract | where it is pinned |
|---|---|---| |---|---|---|
| `RenderModel` → DOM | every field the render model projects reaches an element | `web/tests/render_surface.test.js` (statically) **and** `harness/tests/contract.spec.js` (on the screen) | | `RenderModel` → DOM | every field the render model projects reaches an element | `web/tests/render_surface.test.js` (statically) **and** `harness/tests/contract.spec.js` (on the screen) |
| JS → wasm | the calls the page makes exist, with the arity the bundle declares, and their answers are painted | `web/tests/bundle/wasm_surface.test.js` (the surface) **and** `contract.spec.js`'s pointer test (the wiring) | | JS → wasm | the calls the page makes exist, with the arity the bundle declares, and their answers are painted | `web/tests/bundle/wasm_surface.test.js` (the surface) **and** `contract.spec.js`'s pointer test (the wiring) |
| page → peer | the handshake goes out first, a frame is answered whole, a failure is named, **and the page routes each failure through the call that disarms the watchdog** | `harness/tests/shell.spec.js`, `harness/tests/drill.spec.js` | | page → peer | the handshake goes out first, a frame is answered whole, a failure is named, **and the page routes each failure through the call that disarms the watchdog** | `harness/tests/shell.spec.js`, `harness/tests/drill.spec.js` |
| key → screen | a key the mode's hint line ADVERTISES produces something the user can see, and a text-entry mode paints what is typed into it | `harness/tests/keys.spec.js` |
| `help::pages()` → DOM | the overlay's rows are the shared content — the keys off the keymap and the words beside them — and not the right number of blank rows | `harness/tests/help.spec.js` | | `help::pages()` → DOM | the overlay's rows are the shared content — the keys off the keymap and the words beside them — and not the right number of blank rows | `harness/tests/help.spec.js` |
| worker → OPFS | `storage_contract`, run against a real `OpfsFs` | `harness/tests/opfs.spec.js` | | worker → OPFS | `storage_contract`, run against a real `OpfsFs` | `harness/tests/opfs.spec.js` |
| `AxisFrame` → DOM | the grid's *frame* — header levels, group spans, the collapse indicator and the group row that carries no cells — is drawn as the frame says, not slash-joined | `harness/tests/grouped.spec.js`, over `harness/fixtures/grouped.improv`; `web/tests/grid_plan.test.js` pins the plan it draws from | | `AxisFrame` → DOM | the grid's *frame* — header levels, group spans, the collapse indicator and the group row that carries no cells — is drawn as the frame says, not slash-joined | `harness/tests/grouped.spec.js`, over `harness/fixtures/grouped.improv`; `web/tests/grid_plan.test.js` pins the plan it draws from |
@@ -1398,6 +1471,17 @@ enough to make twice, from different sides:
widget and asserts the element is *on the screen*. Node cannot see that, and widget and asserts the element is *on the screen*. Node cannot see that, and
the moment a reduction would have to weaken it, it stays here. the moment a reduction would have to weaken it, it stays here.
**And the static half is weaker than it reads, which is worth knowing before
leaning on it.** `render_surface.test.js` asks whether the string
`model.<field>` occurs anywhere in the page's code — so a field *mentioned* by a
function nothing ever calls passes. That happened on improvise-fdt6: the new
`minibuffer` field had a `paintMinibuffer` reading it, and the call site and the
`getElementById` were both missing, so the node suite went green over a page
that still painted nothing. `keys.spec.js` failed on its first run, in Chromium,
naming the hidden element. The static check is a **completeness** check — it
tells you a field was not forgotten — and never an **execution** check; only
`harness/` is that (gotcha 48).
What is deliberately **not** in Playwright any more is what each key or click What is deliberately **not** in Playwright any more is what each key or click
*means*: `improvise_protocol::resolve` decides that and its tests are where the *means*: `improvise_protocol::resolve` decides that and its tests are where the
meaning is pinned. One instance of "a resolved change repaints" is kept, because meaning is pinned. One instance of "a resolved change repaints" is kept, because
+160
View File
@@ -1761,6 +1761,166 @@ the injection failed, which is the only reason it is worth having — and it is
the same lesson as the eleven tests above: a class is a flag, and a flag is not the same lesson as the eleven tests above: a class is a flag, and a flag is not
a pixel. a pixel.
### every_advertised_command_answers_in_its_own_mode
improvise-rmvx. Gotcha 47 says an upstream key is a black hole, and
`the_command_names_that_still_need_the_host_are_these` is the guard that was
supposed to keep the list of them honest. It was built with
`in_mode(AppMode::Normal)`, so it only ever asked about Normal-mode bindings:
`add-record-row` (RecordsNormal) and `delete-formula-at-cursor` (FormulaPanel)
were outside its reach entirely and could never have failed it. **A named list
that silently scopes itself to one mode is a guard that looks broader than it
is.**
This asks it of every mode, and it asks it the only way that is not
over-reporting. The naive version resolves every advertised name against an
EMPTY `ClientState` and flags `edit-or-drill`, `commit-cell-edit`,
`create-and-switch-view` and `page-prev`/`page-next` — but those are *handled*
names refused by STATE, which is correct behaviour and not this bug. So the
fixture is `furnished`: every widget has something in it, a basis is published,
and a window is announced, so an `Upstream` can only mean the name is
unhandled.
Two things it does NOT do, each because the first attempt did and was wrong.
It does not call `step` with the advertised args: `advertised` carries `None`
for "any binding of this command", and `step`'s numeric and mode arguments are
mandatory, so that reports `move-selection`, `move-panel-cursor` and
`move-tile-cursor` as gaps in every mode. And it does not tokenise the rendered
hint string: `help::advertised` is a table of command NAMES, so the
cross-reference is exact and anything reading the rendered text is solving a
harder problem than exists. The keys come off `Keymap::keys_running`, which
`labels_for` now shares so "which keys" and "how are they spelled" cannot give
two answers.
**It watches BOTH silent variants, and the first version did not.** `KeyOutcome`
has two ways of saying nothing: `Upstream`, and `Ignored` — which
`KeyOutcome::of(vec![])` produces whenever a resolver composes an empty vector.
To a user they are the same black hole. The first version asserted
`resolve(..) == Upstream(event)`, so **three of the six commands this whole
change exists to make audible could go completely silent with the entire
evidence base green**: degrading only `add-record-row`, `delete-formula-at-cursor`
and `view-back` to `Step::Declined(Vec::new())` left 1397 Rust tests, 19 harness
tests and 26 node tests passing, because no browser spec presses those three
keys. Found on review of PR #71. The check is now `!matches!(.., Upstream(_) |
Ignored)`.
**The generalisation is worth more than the fix.** `KeyOutcome::of` turning an
empty `Local` into an honest `Ignored` is good design and it is also a
**laundering** step: every wrong answer that composes nothing arrives wearing
the variant that is usually innocent. So a guard's discriminator has to be
*state* — did anything the user can see change — and never the shape of the
return value. That is exactly why the `+cat` injection in
`a_locally_committed_creating_column_agrees_with_the_host` failed (it compares
documents and cursors) and the decline injection passed everywhere (nothing
compared a screen).
Widening the check costs ten more entries, all one legitimate boundary case:
`move-panel-cursor` mirrors `MovePanelCursor`, which returns nothing when the
cursor does not move, so a one-row panel or a cursor on an edge answers
`Ignored` for a reason that is not this test's subject. `furnished` therefore
carries **three** of everything with all three panel cursors seated at index 1.
Note what that rules out: seating them at 0 and calling the `Up` results
expected would have hidden the real defect in the same list as the boundary
case.
**The injection**: delete the `"toggle-prune-empty"` arm from `declined`. The
test fails naming `NORMAL: Char('P')` and `RECORDS: Char('P')` — both modes,
which is the widening the ticket was about, and a build that ran rather than a
compile error. The review's own injection is also kept as a check: degrading the
three unpressed declines to empty now fails naming `FORMULAS: Char('d')`,
`FORMULAS: Delete`, `RECORDS: Char('<')` and `RECORDS: Char('o')`.
What it found that the ticket did not predict: `view-back` is advertised in
RecordsNormal (`<`, captioned "back"), so it was a seventh instance of the same
class. The ticket had it filed under the Normal-mode host list instead, where
being upstream is not a defect.
The five that remain are listed by name with a reason each. Three of them —
`commit-export`, `commit-save`, `execute-command` — are reachable only from a
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`.
### a_locally_committed_creating_column_agrees_with_the_host
improvise-gcvb. `resolve`'s commit returned `None` for the whole of records
mode, and the reason given was sound for the coordinate and measure columns —
a category column renames every measure cell in the row, which needs
`record_col` (gotcha 1). The guard was broader than its reason: a cell at the
cursor now projects `CellId::Create`, and committing one is a single existing
`ModelCommand` over the typed NAME that reads no `record_col` and writes no
cell.
Both halves are proved by injection, and the second is the one that matters.
Putting the broad `is_records()` early return back makes the test fail at "the
client must answer this key locally". Deleting the `CreateKind::Category` early
return — so `+cat` advances like any other column — fails with the client in
`RecordsEditing` where the host is in `RecordsNormal`: the TUI returns straight
after a `+cat` commit with a `RebuildLayout` and no cursor move, because an
inserted column renumbers every index to its right and the advance was computed
from the pre-commit layout (gotcha 23). A client that advanced anyway would land
one column off, silently.
`both_ends` compares the document as `.improv` text, the mode, the status and
every cursor, so neither half can pass by agreeing on the wrong thing. The
fixture drives *both* hosts into records mode with the `R` key rather than
hand-building a `_Records` view, because `twin_of` copies only the session
slice and a twin without the view would refuse the commit for the wrong reason.
### every_minibuffer_mode_paints_what_is_being_typed
improvise-fdt6. `commit-buffer formula` already resolved on the client — that
half of the ticket was closed by an earlier wave — so the browser could enter
FormulaEdit, type a formula and commit it. What did not exist was any way to
SEE it: `RenderModel::editing` is populated only when `mode.is_editing()`, which
is the two *cell*-editing modes, and it reads the `"edit"` buffer by name. Every
character typed into FormulaEdit, CategoryAdd, ItemAdd, SavePrompt,
ExportPrompt or CommandMode went into a buffer nothing painted. The mode opened,
the user typed, and the screen did not move — which is exactly the dead-room
shape the ticket names, arrived at from the rendering side rather than the
resolution side.
The value is read with `improvise_protocol::read_buffer` rather than
`buffers.get`, so the `"search"` special case (gotcha 11) keeps its one home.
**The injection**: `.filter(|_| state.view.mode.is_editing())` on the new
field, which is the pre-fix behaviour exactly. It fails naming `"formula"`.
**And the static check was not enough, which is the part worth keeping.**
`web/tests/render_surface.test.js` asserts that every `pub` field of
`RenderModel` appears as `model.<field>` somewhere in `app.js`. It passed while
the page was still broken: the string `model.minibuffer` was inside
`paintMinibuffer`, and `paintMinibuffer` was never called and `el.minibuffer`
was never looked up. A field can be *mentioned* by a function nothing invokes.
`harness/tests/keys.spec.js` caught it on the first run, in the browser, which
is the whole argument for that directory existing (gotcha 48).
### nothing_declined_is_also_composed
`step` is `declined` then `composed`, and until this test the claim that the two
lists cannot overlap was a **doc comment**. The dangerous direction is not a
typo: it is a future ticket implementing `view-back` (or `toggle-prune-empty`,
which is one projection field away) in `composed` and not thinking to delete its
`declined` arm. The browser then goes on saying "not available in the browser
yet" while the working code sits unreachable, and nothing notices — the classic
shape of a check passing while the thing it names is broken. Raised on review of
PR #71.
It is **derived from `declined` itself**, not from a list beside it: it walks
every `(name, args)` bound in every mode's keymap — parents and prefix
sub-keymaps included, sequence steps included — and for each one `declined`
answers, asserts `composed` does not. A seventh decline is covered without
coming here, which is the property a hand-written list could not have.
**The injection**: add `"view-back" => Some(vec![Command::SwitchView(..)])` to
`composed`. It fails naming `NORMAL: view-back []` and `RECORDS: view-back []`.
It also found a real overlap on its first run. `enter-mode` is answered by
`declined` for the three stranding modes *and* by `composed` for every mode, so
`composed`'s arm would have opened `SavePrompt`, `ExportPrompt` or `CommandMode`
if `declined` were ever bypassed. Rather than moving the check, **both halves now
hold the rule** — `composed` returns `None` for a stranded mode — so the failure
direction if one is removed is the pre-PR `Upstream`, not a dead room.
--- ---
## `harness/tests/help.spec.js` — the overlay, asserted as words ## `harness/tests/help.spec.js` — the overlay, asserted as words
+26 -23
View File
@@ -53,7 +53,7 @@ fn hint_tail(mode: &AppMode) -> &'static str {
/// What each mode advertises: command names and captions, never keys. The /// What each mode advertises: command names and captions, never keys. The
/// keys come off the keymap, so a rebinding reaches the status bar for free. /// keys come off the keymap, so a rebinding reaches the status bar for free.
fn advertised(mode: &AppMode) -> Vec<(&'static str, Option<Vec<String>>, &'static str)> { pub(crate) fn advertised(mode: &AppMode) -> Vec<(&'static str, Option<Vec<String>>, &'static str)> {
let arg = |a: &str| Some(vec![a.to_string()]); let arg = |a: &str| Some(vec![a.to_string()]);
match mode { match mode {
AppMode::Normal => vec![ AppMode::Normal => vec![
@@ -782,32 +782,35 @@ fn commands() -> Vec<HelpLine> {
.collect() .collect()
} }
/// Every `AppMode`, as one list. Shared with `resolve`'s tests so a
/// cross-reference cannot silently scope itself to one mode (improvise-rmvx).
#[cfg(test)]
pub(crate) fn all_modes() -> Vec<AppMode> {
vec![
AppMode::Normal,
AppMode::editing(),
AppMode::formula_edit(),
AppMode::FormulaPanel,
AppMode::CategoryPanel,
AppMode::category_add(),
AppMode::item_add("Region".into()),
AppMode::ViewPanel,
AppMode::TileSelect,
AppMode::ImportWizard,
AppMode::export_prompt(),
AppMode::save_prompt(),
AppMode::command_mode(),
AppMode::Help,
AppMode::Quit,
AppMode::RecordsNormal,
AppMode::records_editing(),
]
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
fn all_modes() -> Vec<AppMode> {
vec![
AppMode::Normal,
AppMode::editing(),
AppMode::formula_edit(),
AppMode::FormulaPanel,
AppMode::CategoryPanel,
AppMode::category_add(),
AppMode::item_add("Region".into()),
AppMode::ViewPanel,
AppMode::TileSelect,
AppMode::ImportWizard,
AppMode::export_prompt(),
AppMode::save_prompt(),
AppMode::command_mode(),
AppMode::Help,
AppMode::Quit,
AppMode::RecordsNormal,
AppMode::records_editing(),
]
}
/// The property the old hand-written `hint_text` needed a 60-line /// The property the old hand-written `hint_text` needed a 60-line
/// tokenizer to approximate. See `context/test-evidence/protocol.md`. /// tokenizer to approximate. See `context/test-evidence/protocol.md`.
#[test] #[test]
+25 -7
View File
@@ -208,22 +208,40 @@ impl Keymap {
&self.bindings &self.bindings
} }
/// Every key label here or in a parent that runs `command`. `args` of /// The keymap this one falls through to, if any.
/// `None` accepts any; §8 says why that is the useful granularity. pub fn parent(&self) -> Option<&Arc<Keymap>> {
pub fn labels_for(&self, command: &str, args: Option<&[String]>) -> Vec<String> { self.parent.as_ref()
let mut labels: Vec<String> = self }
/// Every key here or in a parent that runs `command`. `args` of `None`
/// accepts any; §8 says why that is the useful granularity.
pub fn keys_running(
&self,
command: &str,
args: Option<&[String]>,
) -> Vec<(KeyCode, KeyModifiers)> {
let mut keys: Vec<(KeyCode, KeyModifiers)> = self
.bindings .bindings
.iter() .iter()
.filter(|(_, binding)| runs(binding, command, args)) .filter(|(_, binding)| runs(binding, command, args))
.filter_map(|(pattern, _)| match pattern { .filter_map(|(pattern, _)| match pattern {
KeyPattern::Key(code, mods) => Some(key_label(code, *mods)), KeyPattern::Key(code, mods) => Some((*code, *mods)),
KeyPattern::AnyChar | KeyPattern::Any => None, KeyPattern::AnyChar | KeyPattern::Any => None,
}) })
.collect(); .collect();
if let Some(parent) = &self.parent { if let Some(parent) = &self.parent {
labels.extend(parent.labels_for(command, args)); keys.extend(parent.keys_running(command, args));
} }
labels keys
}
/// The same keys, spelled. One walk, so a caller asking "which keys" and
/// a caller asking "how are they written" cannot get different answers.
pub fn labels_for(&self, command: &str, args: Option<&[String]>) -> Vec<String> {
self.keys_running(command, args)
.iter()
.map(|(code, mods)| key_label(code, *mods))
.collect()
} }
/// Look up the binding for a key in this keymap's own bindings. /// Look up the binding for a key in this keymap's own bindings.
+1 -1
View File
@@ -29,7 +29,7 @@ pub use render::{
ViewListCache, ViewSummary, WidgetCache, WizardCache, WizardDateField, WizardFieldKind, ViewListCache, ViewSummary, WidgetCache, WizardCache, WizardDateField, WizardFieldKind,
WizardProposal, WizardStepKind, header_level_widths, WizardProposal, WizardStepKind, header_level_widths,
}; };
pub use resolve::{Click, KeyOutcome, click, resolve, typed_cell_command}; pub use resolve::{Click, KeyOutcome, click, read_buffer, resolve, typed_cell_command};
pub use viewmodel::{CatTreeEntry, flatten_cat_tree, view_axis_summary}; pub use viewmodel::{CatTreeEntry, flatten_cat_tree, view_axis_summary};
// `improvise-core`'s, because the index that resolves one lives in // `improvise-core`'s, because the index that resolves one lives in
// `DataStore`. Re-exported so a client depends on the protocol alone. // `DataStore`. Re-exported so a client depends on the protocol alone.
+328 -54
View File
@@ -7,10 +7,14 @@ use crate::client::ClientState;
use crate::keymap::{Binding, Keymaps, ModeKey}; use crate::keymap::{Binding, Keymaps, ModeKey};
use crate::viewmodel::{CatTreeEntry, flatten_cat_tree}; use crate::viewmodel::{CatTreeEntry, flatten_cat_tree};
use crate::{ use crate::{
AppMode, CellId, CellValue, Command, GridExtent, HelpPage, KeyCode, KeyEvent, ModelCommand, AppMode, CellId, CellValue, Command, CreateKind, GridExtent, HelpPage, KeyCode, KeyEvent,
Panel, RecordCell, ViewState, records_named, ModelCommand, Panel, RecordCell, ViewState, records_named,
}; };
/// The virtual category a new measure is filed under, spelled as
/// `commit.rs::add_measure` spells it.
const MEASURE_CATEGORY: &str = "_Measure";
/// What this client decided to do with a keypress. /// What this client decided to do with a keypress.
#[derive(Debug, Clone, PartialEq)] #[derive(Debug, Clone, PartialEq)]
pub enum KeyOutcome { pub enum KeyOutcome {
@@ -54,8 +58,8 @@ pub fn resolve(state: &ClientState, event: KeyEvent) -> KeyOutcome {
// the second key then runs is not. See `context/code/clients.md`. // the second key then runs is not. See `context/code/clients.md`.
Binding::Prefix(_) => KeyOutcome::Upstream(event), Binding::Prefix(_) => KeyOutcome::Upstream(event),
Binding::Cmd { name, args } => match step(state, event, name, args) { Binding::Cmd { name, args } => match step(state, event, name, args) {
Some(commands) => KeyOutcome::of(commands), Step::Local(commands) | Step::Declined(commands) => KeyOutcome::of(commands),
None => KeyOutcome::Upstream(event), Step::Upstream => KeyOutcome::Upstream(event),
}, },
// All or nothing, each step resolved against the *unchanged* state. // All or nothing, each step resolved against the *unchanged* state.
// See `context/code/clients.md`. // See `context/code/clients.md`.
@@ -185,7 +189,8 @@ fn focus(state: &ClientState, panel: Panel) -> Vec<Command> {
commands commands
} }
/// Every step of a `Binding::Sequence`, or none of them. /// Every step of a `Binding::Sequence`, or none of them. A declining step takes
/// the whole sequence with it — `context/code/clients.md` (gotcha 21).
fn sequence( fn sequence(
state: &ClientState, state: &ClientState,
event: KeyEvent, event: KeyEvent,
@@ -193,14 +198,61 @@ fn sequence(
) -> Option<Vec<Command>> { ) -> Option<Vec<Command>> {
let mut commands = Vec::new(); let mut commands = Vec::new();
for (name, args) in steps { for (name, args) in steps {
commands.extend(step(state, event, name, args)?); match step(state, event, name, args) {
Step::Local(more) => commands.extend(more),
Step::Declined(said) => return Some(said),
Step::Upstream => return None,
}
} }
Some(commands) Some(commands)
} }
/// What one command name resolved to. `Declined` is composed here like
/// `Local`, and differs only in what it does to a sequence.
enum Step {
Local(Vec<Command>),
Declined(Vec<Command>),
Upstream,
}
/// One name, answered. A decline is checked first, so [`composed`] has no arm
/// for any of them and the two lists cannot overlap.
fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Step {
match declined(name, args) {
Some(said) => Step::Declined(vec![Command::SetStatus(said)]),
None => match composed(state, event, name, args) {
Some(commands) => Step::Local(commands),
None => Step::Upstream,
},
}
}
/// The commands this client cannot compose and answers **out loud**; the one
/// home for the wording. Why silence was worse: clients.md (improvise-rmvx).
fn declined(name: &str, args: &[String]) -> Option<String> {
let what = match name {
"drill-into-cell" => "drill",
"toggle-prune-empty" => "hiding empty rows and columns",
"toggle-records-mode" => "records mode",
"add-record-row" => "adding a record row",
"delete-formula-at-cursor" => "deleting a formula",
"view-back" => "stepping back to the previous view",
// A door this client will not open still has to say so: entering is
// resolvable and the commit behind it is not.
"enter-mode" => stranding(&AppMode::from_name(args.first()?)?)?,
_ => return None,
};
Some(format!("{what} is not available in the browser yet"))
}
/// The subset of command names this client can turn into `Command`s; `None` /// The subset of command names this client can turn into `Command`s; `None`
/// means "the host's". Each arm mirrors its `Cmd` line for line — clients.md. /// means "the host's". Each arm mirrors its `Cmd` line for line — clients.md.
fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Option<Vec<Command>> { fn composed(
state: &ClientState,
event: KeyEvent,
name: &str,
args: &[String],
) -> Option<Vec<Command>> {
match name { match name {
// ── the grid cursor ────────────────────────────────────────── // ── the grid cursor ──────────────────────────────────────────
"move-selection" => grid_move(state, GridMove::Relative(num(args, 0)?, num(args, 1)?)), "move-selection" => grid_move(state, GridMove::Relative(num(args, 0)?, num(args, 1)?)),
@@ -234,10 +286,9 @@ fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Op
// ── mode entry ─────────────────────────────────────────────── // ── mode entry ───────────────────────────────────────────────
"enter-mode" => { "enter-mode" => {
let mode = AppMode::from_name(args.first()?)?; let mode = AppMode::from_name(args.first()?)?;
match strands_the_user(&mode) { // `declined` answers this first, and both halves hold the rule:
true => None, // no path opens a room this client cannot commit.
false => Some(entered(mode)), stranding(&mode).is_none().then(|| entered(mode))
}
} }
// ── panels ─────────────────────────────────────────────────── // ── panels ───────────────────────────────────────────────────
@@ -280,12 +331,6 @@ fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Op
cmd: ModelCommand::TransposeAxes, cmd: ModelCommand::TransposeAxes,
}]), }]),
// The hint line advertises `>`, and an upstream key is a black hole
// (gotcha 47), so it said nothing at all. Interim: improvise-gd28.
"drill-into-cell" => Some(vec![Command::SetStatus(
"drill is not available in the browser yet".to_string(),
)]),
// ── editing ────────────────────────────────────────────────── // ── editing ──────────────────────────────────────────────────
"edit-or-drill" => edit_or_drill(state, AppMode::from_name(args.first()?)?), "edit-or-drill" => edit_or_drill(state, AppMode::from_name(args.first()?)?),
"commit-cell-edit" => commit(state, AppMode::from_name(args.first()?)?, Advance::Down), "commit-cell-edit" => commit(state, AppMode::from_name(args.first()?)?, Advance::Down),
@@ -372,7 +417,7 @@ fn num(args: &[String], i: usize) -> Option<i32> {
// The named buffers. A runtime-named buffer may not be an entry in `buffers` // The named buffers. A runtime-named buffer may not be an entry in `buffers`
// at all: "search" is `ViewState::search_query` (improvise-gdl). // at all: "search" is `ViewState::search_query` (improvise-gdl).
fn read_buffer(view: &ViewState, name: &str) -> String { pub fn read_buffer(view: &ViewState, name: &str) -> String {
match name { match name {
"search" => view.search_query.clone(), "search" => view.search_query.clone(),
other => view.buffers.get(other).cloned().unwrap_or_default(), other => view.buffers.get(other).cloned().unwrap_or_default(),
@@ -389,13 +434,15 @@ fn write_buffer(name: &str, value: String) -> Command {
} }
} }
/// A prompt this client can *enter* but cannot *commit*: a dead room is worse /// A prompt this client can *enter* but cannot *commit*; `Some` is the subject
/// than a locked door. All three arms, and who may delete each: clients.md. /// to decline by name. A dead room is worse than a locked door: clients.md.
fn strands_the_user(mode: &AppMode) -> bool { fn stranding(mode: &AppMode) -> Option<&'static str> {
matches!( match mode {
mode, AppMode::SavePrompt { .. } => Some("saving to a path"),
AppMode::SavePrompt { .. } | AppMode::ExportPrompt { .. } | AppMode::CommandMode { .. } AppMode::ExportPrompt { .. } => Some("exporting"),
) AppMode::CommandMode { .. } => Some("the command line"),
_ => None,
}
} }
/// `EnterMode::execute`: clear the incoming mode's minibuffer, then change. /// `EnterMode::execute`: clear the incoming mode's minibuffer, then change.
@@ -622,27 +669,82 @@ enum Advance {
/// `CommitAndAdvance::execute`, for the one case a thin client can compose: a /// `CommitAndAdvance::execute`, for the one case a thin client can compose: a
/// plain grid cell. The three-way fork and the mirror order: clients.md. /// plain grid cell. The three-way fork and the mirror order: clients.md.
fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<Vec<Command>> { fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<Vec<Command>> {
let cursor = state.view.cursor();
let (window_rows, window_cols) = state.window; let (window_rows, window_cols) = state.window;
if window_rows == 0 || window_cols == 0 { if window_rows == 0 || window_cols == 0 {
return None; return None;
} }
// Records mode is the fork this client does not have: a category column let text = read_buffer(&state.view, "edit");
// renames every measure cell in the row, which needs `record_col`. let id = cell_at_cursor(state)?.id.as_ref()?;
// Records mode needs `record_col` (gotcha 1) — except the two CREATING
// columns, which name nothing but themselves (improvise-gcvb).
if let CellId::Create(kind) = id {
return create_column(state, edit_mode, advance, kind, &text);
}
if state.view.mode.is_records() { if state.view.mode.is_records() {
return None; return None;
} }
let text = read_buffer(&state.view, "edit");
let mut commands = vec![Command::Model { let mut commands = vec![Command::Model {
basis: state.basis.claim([record_at_cursor(state)?])?, basis: state.basis.claim([record_at_cursor(state)?])?,
cmd: typed_cell_command(cell_at_cursor(state)?.id.as_ref()?, &text)?, cmd: typed_cell_command(id, &text)?,
}]; }];
commands.push(Command::ChangeMode(AppMode::Normal)); commands.push(Command::ChangeMode(AppMode::Normal));
commands.extend(advanced(state, edit_mode, advance)?);
Some(commands)
}
/// `CommitAndAdvance` over a creating column: a schema change over the typed
/// NAME. Why +cat does not advance: clients.md (improvise-gcvb).
fn create_column(
state: &ClientState,
edit_mode: AppMode,
advance: Advance,
kind: &CreateKind,
text: &str,
) -> Option<Vec<Command>> {
// `exit_mode_for`: the tidy-up goes FIRST, where an aborting
// `AddCategory` cannot reach it (gotcha 21).
let mut commands = vec![Command::ChangeMode(match state.view.mode.is_records() {
true => AppMode::RecordsNormal,
false => AppMode::Normal,
})];
let name = text.trim();
if !name.is_empty() {
commands.push(model(
state,
match kind {
CreateKind::Measure => ModelCommand::AddItem {
category: MEASURE_CATEGORY.to_string(),
item: name.to_string(),
},
CreateKind::Category => ModelCommand::AddCategory(name.to_string()),
},
)?);
}
let (row, col) = state.view.cursor().selected;
commands.push(Command::SetSelected { row, col });
// An inserted column renumbers every index to its right, so staying put is
// what lands on the new one (improvise-2zq, gotcha 23).
if matches!(kind, CreateKind::Category) {
return Some(commands);
}
commands.extend(advanced(state, edit_mode, advance)?);
Some(commands)
}
/// The advance and the re-entry `CommitAndAdvance` ends in. `None` where the
/// host would emit `AddRecordRow`, which this client cannot name (gotcha 1).
fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<Vec<Command>> {
let GridExtent { rows, cols } = state.grid.extent; let GridExtent { rows, cols } = state.grid.extent;
let row_max = rows.saturating_sub(1); let row_max = rows.saturating_sub(1);
let col_max = cols.saturating_sub(1); let col_max = cols.saturating_sub(1);
let (row, col) = cursor.selected; let (row, col) = state.view.cursor().selected;
if matches!(advance, Advance::Right)
&& state.view.mode.is_records()
&& row >= row_max
&& col >= col_max
{
return None;
}
let landed = match advance { let landed = match advance {
Advance::Down if row < row_max => Some((row + 1, col)), Advance::Down if row < row_max => Some((row + 1, col)),
Advance::Down if col < col_max => Some((0, col + 1)), Advance::Down if col < col_max => Some((0, col + 1)),
@@ -654,15 +756,14 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
Advance::Right => Some((row, (col + 1).min(col_max))), Advance::Right => Some((row, (col + 1).min(col_max))),
}; };
let Some((nr, nc)) = landed else { let Some((nr, nc)) = landed else {
return Some(commands); return Some(Vec::new());
}; };
commands.extend(scrolled_to(state, nr, nc)); let mut commands = scrolled_to(state, nr, nc);
// `EnterEditAtCursor`: seed from where we landed, then change mode. The // `EnterEditAtCursor`: seed from where we landed, then change mode. The
// new cell is a different cell, so a stale cache is correct here. // new cell is a different cell, so a stale cache is correct here.
let seeded = seed_at(state, nr, nc);
commands.push(Command::SetBuffer { commands.push(Command::SetBuffer {
name: "edit".to_string(), name: "edit".to_string(),
value: seeded, value: seed_at(state, nr, nc),
}); });
commands.push(Command::ChangeMode(edit_mode)); commands.push(Command::ChangeMode(edit_mode));
Some(commands) Some(commands)
@@ -1008,6 +1109,7 @@ fn commit_buffer(state: &ClientState, kind: CommitKind) -> Option<Vec<Command>>
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::keymap::Keymap;
use crate::{AxisFrame, CellDisplay, GridUpdate, KeyModifiers, Projection, Snapshot}; use crate::{AxisFrame, CellDisplay, GridUpdate, KeyModifiers, Projection, Snapshot};
fn ev(code: KeyCode) -> KeyEvent { fn ev(code: KeyCode) -> KeyEvent {
@@ -1549,6 +1651,179 @@ mod tests {
// ── what is bound and what is not ─────────────────────────────── // ── what is bound and what is not ───────────────────────────────
/// 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());
state.view.mode = mode;
state.grid.page_coords = vec![crate::PageAxis {
cat: "Quarter".into(),
selected: "Q1".into(),
items: Some(vec!["Q1".into(), "Q2".into()]),
}];
// THREE of everything, every panel cursor seated in the MIDDLE: an edge
// makes `move-panel-cursor` answer `Ignored` for the wrong reason.
crate::WidgetCache::apply(
&mut state.categories,
&crate::CategoryTreeUpdate::Categories(
["Region", "Product", "Quarter"]
.into_iter()
.map(|name| crate::CategoryRow {
name: name.into(),
items: vec!["North".into(), "South".into()],
axis: Axis::Row,
})
.collect(),
),
);
state.formulas.formulas = [
"Profit = Revenue - Cost",
"Tax = Revenue * 0.1",
"Net = Profit",
]
.into_iter()
.map(str::to_string)
.collect();
state.views.views = ["Default", "Second", "Third"]
.into_iter()
.map(|name| crate::ViewSummary {
name: name.into(),
axes: Vec::new(),
})
.collect();
state.tiles.tiles = vec![crate::TileEntry {
category: "Region".into(),
axis: Axis::Row,
}];
state.view.cat_panel_cursor = 1;
state.view.formula_cursor = 1;
state.view.view_panel_cursor = 1;
state
.view
.buffers
.insert("formula".into(), "Profit = Revenue".into());
state
.view
.buffers
.insert("category".into(), "Product".into());
state.view.buffers.insert("item".into(), "Shirts".into());
state.view.buffers.insert("edit".into(), "42".into());
state
}
/// improvise-rmvx. An advertised key that resolves upstream is a black hole
/// (gotcha 47). See `context/test-evidence/clients.md`.
#[test]
fn every_advertised_command_answers_in_its_own_mode() {
let mut unanswered = Vec::new();
for mode in crate::help::all_modes() {
let Some(mode_key) = ModeKey::from_app_mode(&mode, false) else {
continue;
};
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
));
}
}
}
}
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 = [
// 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)",
// 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)",
];
let mut expected: Vec<String> = expected.iter().map(|s| s.to_string()).collect();
expected.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"
);
}
/// improvise-rmvx. `step` checks `declined` first, so a name answered
/// there makes an arm in `composed` UNREACHABLE. Evidence file.
#[test]
fn nothing_declined_is_also_composed() {
let mut shadowed = Vec::new();
for mode in crate::help::all_modes() {
let Some(mode_key) = ModeKey::from_app_mode(&mode, false) else {
continue;
};
let Some(keymap) = Keymaps::shared().get(&mode_key) else {
continue;
};
let state = furnished(mode.clone());
// Derived from `declined` itself rather than from a list beside
// it: a seventh decline is covered without coming here.
let mut vocabulary = Vec::new();
bound_commands(keymap, &mut vocabulary);
for (name, args) in vocabulary {
if declined(&name, &args).is_none() {
continue;
}
if composed(&state, ev(KeyCode::Char('x')), &name, &args).is_some() {
shadowed.push(format!("{}: {name} {args:?}", crate::mode_label(&mode)));
}
}
}
shadowed.sort();
shadowed.dedup();
assert!(
shadowed.is_empty(),
"these are answered by BOTH `declined` and `composed`, and `step` \
reaches `declined` first — so the browser keeps saying \"not \
available yet\" while the working code sits unreachable:\n{}",
shadowed.join("\n")
);
}
/// Every `(name, args)` a keymap, its parents and its prefixes bind —
/// the whole vocabulary a keypress in that mode can reach.
fn bound_commands(keymap: &Keymap, out: &mut Vec<(String, Vec<String>)>) {
for binding in keymap.bindings().values() {
match binding {
Binding::Cmd { name, args } => out.push((name.to_string(), args.clone())),
Binding::Sequence(steps) => out.extend(
steps
.iter()
.map(|(name, args)| (name.to_string(), args.clone())),
),
// Prefixes are flat (gotcha 13), so this terminates.
Binding::Prefix(sub) => bound_commands(sub, out),
}
}
if let Some(parent) = keymap.parent() {
bound_commands(parent, out);
}
}
/// The command **names** this client cannot execute, one by one, so that /// The command **names** this client cannot execute, one by one, so that
/// closing one of these gaps has to come here and say so. Evidence file. /// closing one of these gaps has to come here and say so. Evidence file.
#[test] #[test]
@@ -1562,14 +1837,11 @@ mod tests {
(ev(KeyCode::Char('Z')), "<the Z prefix>"), (ev(KeyCode::Char('Z')), "<the Z prefix>"),
(ev(KeyCode::Char('H')), "hide-selected-row-item"), (ev(KeyCode::Char('H')), "hide-selected-row-item"),
(ev(KeyCode::Char('z')), "toggle-group-under-cursor"), (ev(KeyCode::Char('z')), "toggle-group-under-cursor"),
// `>` is NOT here any more: it answers with a status of its own, // `>`, `<`, `R`, `P`, Ctrl-W and Ctrl-E are NOT here any more: each
// because it is advertised and silence read as broken (gd28). // answers with a status of its own (improvise-gd28, improvise-rmvx).
(ev(KeyCode::Char('<')), "view-back"),
(ev(KeyCode::Char('/')), "search"), (ev(KeyCode::Char('/')), "search"),
(ev(KeyCode::Char('n')), "search-navigate"), (ev(KeyCode::Char('n')), "search-navigate"),
(ev(KeyCode::Char('N')), "search-or-category-add"), (ev(KeyCode::Char('N')), "search-or-category-add"),
(ev(KeyCode::Char('R')), "toggle-records-mode"),
(ev(KeyCode::Char('P')), "toggle-prune-empty"),
(ev(KeyCode::Enter), "enter-advance"), (ev(KeyCode::Enter), "enter-advance"),
( (
KeyEvent::new(KeyCode::Char('s'), KeyModifiers::CONTROL), KeyEvent::new(KeyCode::Char('s'), KeyModifiers::CONTROL),
@@ -1579,16 +1851,6 @@ mod tests {
KeyEvent::new(KeyCode::Char('q'), KeyModifiers::CONTROL), KeyEvent::new(KeyCode::Char('q'), KeyModifiers::CONTROL),
"force-quit", "force-quit",
), ),
// The two doors this client refuses rather than opens —
// `enter-mode` is resolvable, the commit behind it is not.
(
KeyEvent::new(KeyCode::Char('w'), KeyModifiers::CONTROL),
"enter-mode save-prompt (commit-save is the host's)",
),
(
KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL),
"enter-mode export-prompt (commit-export is the host's)",
),
] { ] {
assert_eq!( assert_eq!(
resolve(&state, key), resolve(&state, key),
@@ -1630,19 +1892,31 @@ mod tests {
#[test] #[test]
fn a_prompt_this_client_cannot_commit_is_never_entered() { fn a_prompt_this_client_cannot_commit_is_never_entered() {
let state = in_mode(AppMode::Normal); let state = in_mode(AppMode::Normal);
for (key, mode) in [ for (key, mode, subject) in [
( (
KeyEvent::new(KeyCode::Char('w'), KeyModifiers::CONTROL), KeyEvent::new(KeyCode::Char('w'), KeyModifiers::CONTROL),
"save-prompt", "save-prompt",
"saving",
), ),
( (
KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL), KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL),
"export-prompt", "export-prompt",
"exporting",
), ),
(ev(KeyCode::Char(':')), "command-mode", "command line"),
] { ] {
assert_eq!( let KeyOutcome::Local(said) = resolve(&state, key) else {
resolve(&state, key), panic!("{mode} must be refused *out loud*, not in silence");
KeyOutcome::Upstream(key), };
let [Command::SetStatus(msg)] = &said[..] else {
panic!("{mode}: expected one status, got {said:?}");
};
assert!(
msg.contains(subject),
"{mode}: the refusal must name its own subject, got {msg}"
);
assert!(
!said.iter().any(|c| matches!(c, Command::ChangeMode(_))),
"{mode} must not be entered by a client that cannot commit it" "{mode} must not be entered by a client that cannot commit it"
); );
} }
@@ -55,6 +55,20 @@ pub struct RenderModel {
/// What the user has typed so far while a keyboard edit is open on the /// What the user has typed so far while a keyboard edit is open on the
/// selected cell. The decision is made here — `context/code/clients.md`. /// selected cell. The decision is made here — `context/code/clients.md`.
pub editing: Option<String>, pub editing: Option<String>,
/// The bottom-bar prompt while ANY text-entry mode is open, cell edits
/// included. `None` elsewhere, so the page asks nothing about the mode.
pub minibuffer: Option<RenderMinibuffer>,
}
/// The prompt, what has been typed into it, and the colour. WHICH buffer is
/// the mode's own answer (gotcha 11) — never a name this module spells.
#[derive(Debug, Clone, PartialEq, Serialize)]
pub struct RenderMinibuffer {
pub prompt: String,
pub value: String,
/// Semantic, as `MinibufColor` is: the last translation to a CSS class is
/// the page's, exactly as `draw.rs::minibuf_fg` is the TUI's (gotcha 26).
pub color: improvise_protocol::MinibufColor,
} }
/// A grid row: either a group header spanning the row, or data. An enum so a /// A grid row: either a group header spanning the row, or data. An enum so a
@@ -251,6 +265,11 @@ pub fn render_model(state: &ClientState) -> RenderModel {
.mode .mode
.is_editing() .is_editing()
.then(|| state.view.buffers.get("edit").cloned().unwrap_or_default()), .then(|| state.view.buffers.get("edit").cloned().unwrap_or_default()),
minibuffer: state.view.mode.minibuffer().map(|mb| RenderMinibuffer {
prompt: mb.prompt.clone(),
value: improvise_protocol::read_buffer(&state.view, &mb.buffer_key),
color: mb.color,
}),
} }
} }
@@ -813,6 +832,40 @@ mod tests {
); );
} }
/// improvise-fdt6. A room the browser could enter, typing into a buffer
/// nothing painted. See `context/test-evidence/clients.md`.
#[test]
fn every_minibuffer_mode_paints_what_is_being_typed() {
for mode in [
improvise_protocol::AppMode::formula_edit(),
improvise_protocol::AppMode::category_add(),
improvise_protocol::AppMode::item_add("Region".into()),
improvise_protocol::AppMode::editing(),
] {
let config = mode.minibuffer().expect("a text-entry mode").clone();
let mut client = client_with_grid(ViewState::default());
assert_eq!(
client.model().minibuffer,
None,
"Normal has no minibuffer to paint"
);
client.state_mut().fold(&Command::ChangeMode(mode.clone()));
client.state_mut().fold(&Command::SetBuffer {
name: config.buffer_key.clone(),
value: "Profit = Revenue - Cost".into(),
});
let painted = client
.model()
.minibuffer
.unwrap_or_else(|| panic!("{:?} must paint its minibuffer", config.buffer_key));
assert_eq!(painted.prompt, config.prompt);
assert_eq!(
painted.value, "Profit = Revenue - Cost",
"the page must draw what the user typed, not an empty line"
);
}
}
/// The whole model crosses as JSON, so the page needs no bindings. /// The whole model crosses as JSON, so the page needs no bindings.
#[test] #[test]
fn the_render_model_crosses_as_json() { fn the_render_model_crosses_as_json() {
@@ -1070,3 +1070,69 @@ fn page_selections(app: &App) -> Vec<(String, Option<String>)> {
.map(|cat| (cat.to_string(), view.page_selection(cat).map(String::from))) .map(|cat| (cat.to_string(), view.page_selection(cat).map(String::from)))
.collect() .collect()
} }
/// Both hosts driven into records mode the way a user gets there, so the
/// `_Records` view and its `_Index`/`_Dim` axes are the real ones.
fn records_pair() -> (App, App) {
let r = KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE);
let mut app = furnished_host();
let mut twin = furnished_host();
reduce_key(&mut app, r);
reduce_key(&mut twin, r);
(app, twin)
}
/// Where the client's own projected grid puts a creating column. Asked of the
/// projection, never re-derived from the axes — that is ADR 002's whole point.
fn creating_cell(client: &ClientState, kind: improvise_protocol::CreateKind) -> (usize, usize) {
for (r, row) in client.grid.cells.iter().enumerate() {
for (c, cell) in row.iter().enumerate() {
if cell.id.as_ref() == Some(&improvise_protocol::CellId::Create(kind)) {
return (r, c);
}
}
}
panic!("records mode must project a {kind:?} creating column");
}
/// Put all three ends on one cell with one buffer, mid-edit.
fn typing_at(
app: &mut App,
twin: &mut App,
client: &mut ClientState,
at: (usize, usize),
text: &str,
) {
for host in [&mut *app, &mut *twin] {
host.view_state.mode = AppMode::records_editing();
host.view_state.cursor_mut().selected = at;
host.view_state.buffers.insert("edit".into(), text.into());
host.rebuild_layout();
}
client.view = app.view_state.clone();
}
/// improvise-gcvb. The records-mode early return was broader than its reason.
/// Why `+cat` must NOT advance: `context/test-evidence/clients.md`.
#[test]
fn a_locally_committed_creating_column_agrees_with_the_host() {
for (kind, typed) in [
(improvise_protocol::CreateKind::Category, "Season"),
(improvise_protocol::CreateKind::Measure, "Cost"),
] {
let (mut app, mut twin) = records_pair();
let mut client = furnished_client(&app, (12, 12));
let at = creating_cell(&client, kind);
typing_at(&mut app, &mut twin, &mut client, at, typed);
client = furnished_client(&app, (12, 12));
client.view = app.view_state.clone();
both_ends(&mut app, &mut twin, &mut client, "Enter");
assert!(
document_text(&app).contains(typed),
"{kind:?}: the commit must reach the document: {}",
document_text(&app)
);
}
}
+68
View File
@@ -0,0 +1,68 @@
// Keys the page ADVERTISES, executed. Two properties: a key this client cannot
// compose declines out loud (improvise-rmvx), and the one text-entry room the
// browser could enter but never showed you typing in (improvise-fdt6).
import { test, expect } from "@playwright/test";
/// The shipped demo, focused, with a grid on screen. `#grid` must have focus
/// or the page's keydown listener never sees a key and every assertion below
/// would pass or fail for a reason that has nothing to do with resolution.
async function theDemo(page) {
await page.goto("/app/index.html");
await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
await page.locator("#grid").focus();
await expect(page.locator("#grid td.editable").first()).toBeVisible();
}
test.describe("an advertised key is never silent", () => {
// Each of these is named in the mode's own hint line, so silence read as
// broken rather than as unavailable (gotcha 47). The subjects must differ:
// one message reached by three keys would tell the user nothing.
test("the three Normal-mode keys this client cannot compose each say their own subject", async ({
page,
}) => {
await theDemo(page);
for (const [key, said] of [
["P", "hiding empty rows and columns is not available in the browser yet"],
["R", "records mode is not available in the browser yet"],
[":", "the command line is not available in the browser yet"],
]) {
// A stale status would let the NEXT key pass on the previous key's
// message, so each assertion is preceded by a state it is not.
await expect(page.locator("#status")).not.toContainText(said);
await page.keyboard.press(key);
await expect(page.locator("#status")).toContainText(said);
// A refusal that changed the mode would be a door opened anyway.
await expect(page.locator("#mode")).toHaveText("NORMAL");
}
});
});
test.describe("a formula can be composed in the browser", () => {
// improvise-fdt6. `commit-buffer formula` resolved here already; what did
// not exist was any way to SEE the typing, so the room swallowed it.
test("the formula minibuffer shows what is typed and the commit reaches the panel", async ({
page,
}) => {
await theDemo(page);
await expect(page.locator("#minibuffer")).toBeHidden();
await page.keyboard.press("F");
await expect(page.locator("#mode")).toHaveText("FORMULAS");
await page.keyboard.press("n");
await expect(page.locator("#mode")).toHaveText("FORMULA");
// The property: every character is visible while it is being typed.
await expect(page.locator("#minibuffer")).toBeVisible();
await page.keyboard.type("Margin = Profit");
await expect(page.locator("#minibuffer")).toContainText("Margin = Profit");
await page.keyboard.press("Enter");
await expect(page.locator("#minibuffer")).toBeHidden();
// And it reached the DOCUMENT, not just the buffer: the panel is a
// projection the host sends back.
await expect(page.locator("#formulas")).toContainText("Margin = Profit");
});
});
+12
View File
@@ -30,6 +30,7 @@ const el = {
views: document.getElementById("views"), views: document.getElementById("views"),
tiles: document.getElementById("tiles"), tiles: document.getElementById("tiles"),
mode: document.getElementById("mode"), mode: document.getElementById("mode"),
minibuffer: document.getElementById("minibuffer"),
hint: document.getElementById("hint"), hint: document.getElementById("hint"),
help: document.getElementById("help"), help: document.getElementById("help"),
status: document.getElementById("status"), status: document.getElementById("status"),
@@ -158,6 +159,7 @@ function repaint() {
// itself; see `RenderModel::hint`. // itself; see `RenderModel::hint`.
el.hint.textContent = model.hint; el.hint.textContent = model.hint;
paintHelp(model); paintHelp(model);
paintMinibuffer(model);
shell.setModel(model.status); shell.setModel(model.status);
el.viewName.textContent = model.view_name; el.viewName.textContent = model.view_name;
el.grid.replaceChildren(gridTable(model)); el.grid.replaceChildren(gridTable(model));
@@ -179,6 +181,16 @@ function repaint() {
if (changed) sendViewport(); if (changed) sendViewport();
} }
/** The bottom-bar prompt. `null` is "no text-entry mode is open", decided in
* `render.rs`, so the page asks nothing about the mode (improvise-fdt6). */
function paintMinibuffer(model) {
const mb = model.minibuffer;
el.minibuffer.hidden = mb === null || mb === undefined;
if (el.minibuffer.hidden) return;
el.minibuffer.className = mb.color.toLowerCase();
el.minibuffer.textContent = mb.prompt + mb.value;
}
/** The help overlay. An empty `help` is what says it is closed -- the page /** The help overlay. An empty `help` is what says it is closed -- the page
* asks no question about the mode, and writes none of the content: the rows, * asks no question about the mode, and writes none of the content: the rows,
* their keys and the page titles are `improvise_protocol::help`'s, the same * their keys and the page titles are `improvise_protocol::help`'s, the same
+7
View File
@@ -83,6 +83,13 @@
<div id="grid" tabindex="0" aria-label="Grid"></div> <div id="grid" tabindex="0" aria-label="Grid"></div>
</main> </main>
<!--
The bottom bar. The TUI draws the minibuffer INSTEAD of the status line;
here it is a line of its own, because #status also carries the transport's
own failures and a repaint must not erase one (gotcha 48).
-->
<div id="minibuffer" role="status" hidden></div>
<div id="help" role="dialog" aria-label="Help" hidden></div> <div id="help" role="dialog" aria-label="Help" hidden></div>
<script type="module" src="./app.js"></script> <script type="module" src="./app.js"></script>
+18
View File
@@ -272,3 +272,21 @@ td input {
.help-dim { .help-dim {
opacity: 0.7; opacity: 0.7;
} }
/* The bottom-bar prompt while a text-entry mode is open. The colour is the
mode's own answer (`MinibufColor`); this is the last translation, exactly
as `draw.rs::minibuf_fg` is the TUI's. */
#minibuffer {
padding: 0.25rem 0.5rem;
font-family: inherit;
border-top: 1px solid var(--accent);
}
#minibuffer.green {
color: #7ec699;
}
#minibuffer.cyan {
color: #6fd0d8;
}
#minibuffer.yellow {
color: #d8c96f;
}