diff --git a/context/code/clients.md b/context/code/clients.md index 05ca6bdd..c4d3a2f4 100644 --- a/context/code/clients.md +++ b/context/code/clients.md @@ -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 `Host::handle` answers a `Key` frame with an empty vec, so each one is a binding 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 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. - **Anything touching storage or an arbitrary command line.** The host's by definition. -- **The records-mode commit.** A wide-records row is one coordinate tuple and - editing a category column moves every measure cell in it; - `commit_plain_records_edit` needs `GridLayout::record_col` to know which. +- **The records-mode commit** — *except the two creating columns*. A wide-records + row is one coordinate tuple and editing a category column moves every measure + 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 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 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 offer. -The predicate is named as a property rather 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 +It returns the **subject** rather than a bool (improvise-rmvx): the refusal is +right and the silence was not, so `:` now says the command line is unavailable +instead of doing nothing at all. The predicate is named as a property rather +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 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. @@ -946,6 +1005,19 @@ Four smaller decisions that would be re-derived wrongly: broken grid. `editing` is `AppMode::is_editing()` plus the `edit` buffer: without it, a browser that resolved `i` locally would change its mode indicator 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 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 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 -it about", and the browser half has six: +it about", and the browser half has seven: | 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) | | 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` | +| 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` | | 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 | @@ -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 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.` 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 *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 diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index 36e43b6d..56eca3e3 100644 --- a/context/test-evidence/clients.md +++ b/context/test-evidence/clients.md @@ -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 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.` 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 diff --git a/crates/improvise-protocol/src/help.rs b/crates/improvise-protocol/src/help.rs index 20ec0e86..1a5828a5 100644 --- a/crates/improvise-protocol/src/help.rs +++ b/crates/improvise-protocol/src/help.rs @@ -53,7 +53,7 @@ fn hint_tail(mode: &AppMode) -> &'static str { /// 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. -fn advertised(mode: &AppMode) -> Vec<(&'static str, Option>, &'static str)> { +pub(crate) fn advertised(mode: &AppMode) -> Vec<(&'static str, Option>, &'static str)> { let arg = |a: &str| Some(vec![a.to_string()]); match mode { AppMode::Normal => vec![ @@ -782,32 +782,35 @@ fn commands() -> Vec { .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 { + 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)] mod tests { use super::*; - fn all_modes() -> Vec { - 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 /// tokenizer to approximate. See `context/test-evidence/protocol.md`. #[test] diff --git a/crates/improvise-protocol/src/keymap.rs b/crates/improvise-protocol/src/keymap.rs index 437b5a8e..23bda05b 100644 --- a/crates/improvise-protocol/src/keymap.rs +++ b/crates/improvise-protocol/src/keymap.rs @@ -208,22 +208,40 @@ impl Keymap { &self.bindings } - /// Every key label here or in a parent that runs `command`. `args` of - /// `None` accepts any; §8 says why that is the useful granularity. - pub fn labels_for(&self, command: &str, args: Option<&[String]>) -> Vec { - let mut labels: Vec = self + /// The keymap this one falls through to, if any. + pub fn parent(&self) -> Option<&Arc> { + self.parent.as_ref() + } + + /// 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 .iter() .filter(|(_, binding)| runs(binding, command, args)) .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, }) .collect(); 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 { + 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. diff --git a/crates/improvise-protocol/src/lib.rs b/crates/improvise-protocol/src/lib.rs index 6b7e2f64..45ab75a3 100644 --- a/crates/improvise-protocol/src/lib.rs +++ b/crates/improvise-protocol/src/lib.rs @@ -29,7 +29,7 @@ pub use render::{ ViewListCache, ViewSummary, WidgetCache, WizardCache, WizardDateField, WizardFieldKind, 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}; // `improvise-core`'s, because the index that resolves one lives in // `DataStore`. Re-exported so a client depends on the protocol alone. diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index a1d3a2db..c42644d4 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -7,10 +7,14 @@ use crate::client::ClientState; use crate::keymap::{Binding, Keymaps, ModeKey}; use crate::viewmodel::{CatTreeEntry, flatten_cat_tree}; use crate::{ - AppMode, CellId, CellValue, Command, GridExtent, HelpPage, KeyCode, KeyEvent, ModelCommand, - Panel, RecordCell, ViewState, records_named, + AppMode, CellId, CellValue, Command, CreateKind, GridExtent, HelpPage, KeyCode, KeyEvent, + 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. #[derive(Debug, Clone, PartialEq)] 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`. Binding::Prefix(_) => KeyOutcome::Upstream(event), Binding::Cmd { name, args } => match step(state, event, name, args) { - Some(commands) => KeyOutcome::of(commands), - None => KeyOutcome::Upstream(event), + Step::Local(commands) | Step::Declined(commands) => KeyOutcome::of(commands), + Step::Upstream => KeyOutcome::Upstream(event), }, // All or nothing, each step resolved against the *unchanged* state. // See `context/code/clients.md`. @@ -185,7 +189,8 @@ fn focus(state: &ClientState, panel: Panel) -> Vec { 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( state: &ClientState, event: KeyEvent, @@ -193,14 +198,61 @@ fn sequence( ) -> Option> { let mut commands = Vec::new(); 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) } +/// 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), + Declined(Vec), + 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 { + 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` /// 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> { +fn composed( + state: &ClientState, + event: KeyEvent, + name: &str, + args: &[String], +) -> Option> { match name { // ── the grid cursor ────────────────────────────────────────── "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 ─────────────────────────────────────────────── "enter-mode" => { let mode = AppMode::from_name(args.first()?)?; - match strands_the_user(&mode) { - true => None, - false => Some(entered(mode)), - } + // `declined` answers this first, and both halves hold the rule: + // no path opens a room this client cannot commit. + stranding(&mode).is_none().then(|| entered(mode)) } // ── panels ─────────────────────────────────────────────────── @@ -280,12 +331,6 @@ fn step(state: &ClientState, event: KeyEvent, name: &str, args: &[String]) -> Op cmd: ModelCommand::TransposeAxes, }]), - // The hint line advertises `>`, and an upstream key is a black hole - // (gotcha 47), so it said nothing at all. Interim: improvise-gd28. - "drill-into-cell" => Some(vec![Command::SetStatus( - "drill is not available in the browser yet".to_string(), - )]), - // ── editing ────────────────────────────────────────────────── "edit-or-drill" => edit_or_drill(state, AppMode::from_name(args.first()?)?), "commit-cell-edit" => commit(state, AppMode::from_name(args.first()?)?, Advance::Down), @@ -372,7 +417,7 @@ fn num(args: &[String], i: usize) -> Option { // The named buffers. A runtime-named buffer may not be an entry in `buffers` // 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 { "search" => view.search_query.clone(), 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 -/// than a locked door. All three arms, and who may delete each: clients.md. -fn strands_the_user(mode: &AppMode) -> bool { - matches!( - mode, - AppMode::SavePrompt { .. } | AppMode::ExportPrompt { .. } | AppMode::CommandMode { .. } - ) +/// A prompt this client can *enter* but cannot *commit*; `Some` is the subject +/// to decline by name. A dead room is worse than a locked door: clients.md. +fn stranding(mode: &AppMode) -> Option<&'static str> { + match mode { + AppMode::SavePrompt { .. } => Some("saving to a path"), + AppMode::ExportPrompt { .. } => Some("exporting"), + AppMode::CommandMode { .. } => Some("the command line"), + _ => None, + } } /// `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 /// plain grid cell. The three-way fork and the mirror order: clients.md. fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option> { - let cursor = state.view.cursor(); let (window_rows, window_cols) = state.window; if window_rows == 0 || window_cols == 0 { return None; } - // Records mode is the fork this client does not have: a category column - // renames every measure cell in the row, which needs `record_col`. + let text = read_buffer(&state.view, "edit"); + 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() { return None; } - let text = read_buffer(&state.view, "edit"); let mut commands = vec![Command::Model { 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.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> { + // `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> { let GridExtent { rows, cols } = state.grid.extent; let row_max = rows.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 { Advance::Down if row < row_max => Some((row + 1, col)), Advance::Down if col < col_max => Some((0, col + 1)), @@ -654,15 +756,14 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option Some((row, (col + 1).min(col_max))), }; 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 // new cell is a different cell, so a stale cache is correct here. - let seeded = seed_at(state, nr, nc); commands.push(Command::SetBuffer { name: "edit".to_string(), - value: seeded, + value: seed_at(state, nr, nc), }); commands.push(Command::ChangeMode(edit_mode)); Some(commands) @@ -1008,6 +1109,7 @@ fn commit_buffer(state: &ClientState, kind: CommitKind) -> Option> #[cfg(test)] mod tests { use super::*; + use crate::keymap::Keymap; use crate::{AxisFrame, CellDisplay, GridUpdate, KeyModifiers, Projection, Snapshot}; fn ev(code: KeyCode) -> KeyEvent { @@ -1549,6 +1651,179 @@ mod tests { // ── 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 = 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)>) { + 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 /// closing one of these gaps has to come here and say so. Evidence file. #[test] @@ -1562,14 +1837,11 @@ mod tests { (ev(KeyCode::Char('Z')), ""), (ev(KeyCode::Char('H')), "hide-selected-row-item"), (ev(KeyCode::Char('z')), "toggle-group-under-cursor"), - // `>` is NOT here any more: it answers with a status of its own, - // because it is advertised and silence read as broken (gd28). - (ev(KeyCode::Char('<')), "view-back"), + // `>`, `<`, `R`, `P`, Ctrl-W and Ctrl-E are NOT here any more: each + // answers with a status of its own (improvise-gd28, improvise-rmvx). (ev(KeyCode::Char('/')), "search"), (ev(KeyCode::Char('n')), "search-navigate"), (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"), ( KeyEvent::new(KeyCode::Char('s'), KeyModifiers::CONTROL), @@ -1579,16 +1851,6 @@ mod tests { KeyEvent::new(KeyCode::Char('q'), KeyModifiers::CONTROL), "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!( resolve(&state, key), @@ -1630,19 +1892,31 @@ mod tests { #[test] fn a_prompt_this_client_cannot_commit_is_never_entered() { let state = in_mode(AppMode::Normal); - for (key, mode) in [ + for (key, mode, subject) in [ ( KeyEvent::new(KeyCode::Char('w'), KeyModifiers::CONTROL), "save-prompt", + "saving", ), ( KeyEvent::new(KeyCode::Char('e'), KeyModifiers::CONTROL), "export-prompt", + "exporting", ), + (ev(KeyCode::Char(':')), "command-mode", "command line"), ] { - assert_eq!( - resolve(&state, key), - KeyOutcome::Upstream(key), + let KeyOutcome::Local(said) = resolve(&state, key) else { + panic!("{mode} must be refused *out loud*, not in silence"); + }; + 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" ); } diff --git a/crates/improvise-wasm-client/src/render.rs b/crates/improvise-wasm-client/src/render.rs index e792f090..30a20693 100644 --- a/crates/improvise-wasm-client/src/render.rs +++ b/crates/improvise-wasm-client/src/render.rs @@ -55,6 +55,20 @@ pub struct RenderModel { /// 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`. pub editing: Option, + /// 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, +} + +/// 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 @@ -251,6 +265,11 @@ pub fn render_model(state: &ClientState) -> RenderModel { .mode .is_editing() .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. #[test] fn the_render_model_crosses_as_json() { diff --git a/crates/improvise-wasm-client/tests/lockstep.rs b/crates/improvise-wasm-client/tests/lockstep.rs index 65334060..7bafab85 100644 --- a/crates/improvise-wasm-client/tests/lockstep.rs +++ b/crates/improvise-wasm-client/tests/lockstep.rs @@ -1070,3 +1070,69 @@ fn page_selections(app: &App) -> Vec<(String, Option)> { .map(|cat| (cat.to_string(), view.page_selection(cat).map(String::from))) .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) + ); + } +} diff --git a/harness/tests/keys.spec.js b/harness/tests/keys.spec.js new file mode 100644 index 00000000..7939dec0 --- /dev/null +++ b/harness/tests/keys.spec.js @@ -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"); + }); +}); diff --git a/web/app.js b/web/app.js index cf4fac63..9c496209 100644 --- a/web/app.js +++ b/web/app.js @@ -30,6 +30,7 @@ const el = { views: document.getElementById("views"), tiles: document.getElementById("tiles"), mode: document.getElementById("mode"), + minibuffer: document.getElementById("minibuffer"), hint: document.getElementById("hint"), help: document.getElementById("help"), status: document.getElementById("status"), @@ -158,6 +159,7 @@ function repaint() { // itself; see `RenderModel::hint`. el.hint.textContent = model.hint; paintHelp(model); + paintMinibuffer(model); shell.setModel(model.status); el.viewName.textContent = model.view_name; el.grid.replaceChildren(gridTable(model)); @@ -179,6 +181,16 @@ function repaint() { 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 * 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 diff --git a/web/index.html b/web/index.html index bf2f40fa..8e3dd905 100644 --- a/web/index.html +++ b/web/index.html @@ -83,6 +83,13 @@
+ + + diff --git a/web/style.css b/web/style.css index f3de89d9..03908006 100644 --- a/web/style.css +++ b/web/style.css @@ -272,3 +272,21 @@ td input { .help-dim { 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; +}