Merge remote-tracking branch 'gh/main' into fix/ixxx-untouched-commit
# Conflicts: # context/test-evidence/command.md
This commit is contained in:
Edward Langley
@@ -226,3 +226,9 @@ learns no coordinate space, and an invalid operation is unconstructible rather
|
||||
than rejected. What the extension adds is that the enum is now **four** answers
|
||||
to "what may be done here", so `None` finally means only what this ADR always
|
||||
said it meant — no projected cell at all.
|
||||
|
||||
**A fifth kind (2026-09-22, improvise-exdg).** The records-mode coordinate cell
|
||||
was the last `None` that named a visible cell. It is now `CellId::Coordinate`.
|
||||
The variant carries no operand, because no client composes a coordinate rename.
|
||||
The previous paragraph's "four answers" is therefore five. `None` now means
|
||||
only a padded or blanked slot. Reasoning: `context/code/protocol.md` §3.
|
||||
|
||||
+50
-19
@@ -161,7 +161,7 @@ limitation that is not new: `ExecuteCommand` returns one effect batch, so the
|
||||
segments of `:a . b` all see the pre-batch cursor, exactly as a
|
||||
`Binding::Sequence` does (gotchas 21 and 23).
|
||||
|
||||
## 8 — the interactive arm is `parse`, then context; it is never a second parser
|
||||
## 8 — the interactive arm is `parse`, then the cursor; it is never a second parser
|
||||
|
||||
#151 fixed three instances of *a registry with two construction arms demotes
|
||||
whichever caller reaches the weaker one*, one patch each. A post-merge review
|
||||
@@ -175,31 +175,59 @@ and the key from the cursor. So a script that wrote 5 to `Region/East`
|
||||
The shape rather than the instances is what changed. `CmdRegistry::register`
|
||||
used to take two free functions, `parse(args)` and `interactive(args, ctx)`,
|
||||
and nothing related them; each command re-derived its arguments twice and every
|
||||
disagreement was silent. It now takes three pieces, and the interactive arm is
|
||||
**built from `parse`**, not beside it:
|
||||
disagreement was silent. The interactive arm is now **built from `parse`**, not
|
||||
beside it:
|
||||
|
||||
- `parse(args) -> C` — the only function that reads arguments a caller supplied.
|
||||
- `contextualize(C, &CmdContext) -> C` — fills the fields that are context and
|
||||
never argument (a `Move`'s origin cursor, a `MovePanelCursor`'s bound). It is
|
||||
handed the built command and **not the args**, so it cannot discard them.
|
||||
- `fallback: Option<(args, ctx) -> C>` — reached **only** when `parse` refused.
|
||||
This is where "no coordinates means the cursor" lives: `clear-cell` bare, and
|
||||
`commit-cell-edit <mode>` bare, which is what the keymap binds.
|
||||
- For `register_at_cursor` only, the registry itself then sets the command's
|
||||
**cursor** — its `CursorState`, reached through `AtCursor::cursor_mut` — to the
|
||||
live one. That is the whole of what the interactive arm adds to a parsed
|
||||
command, and no registration supplies it.
|
||||
|
||||
Two consequences hold by construction rather than by test. **Whatever `parse`
|
||||
accepts, the interactive arm accepts** — `contextualize` is infallible — so the
|
||||
§7 gate cannot pass a segment the loop then refuses. And **an accepted argument
|
||||
list cannot be overridden by the cursor**, because the only function that sees
|
||||
both args and context never runs on an accepted list. `register_pure` and
|
||||
`register_nullary` already had one arm; they are the `contextualize = identity,
|
||||
fallback = None` case.
|
||||
Three consequences hold by construction rather than by test. **Whatever `parse`
|
||||
accepts, the interactive arm accepts** — nothing after `parse` can fail — so the
|
||||
§7 gate cannot pass a segment the loop then refuses. **An accepted argument list
|
||||
cannot be overridden by the cursor**, because the only function that sees both
|
||||
args and context never runs on an accepted list. And **no field `parse` set can
|
||||
be overwritten from context**, because nothing a registration writes can name
|
||||
one: the cursor slot is typed `&mut CursorState`, and no command parses a
|
||||
`CursorState` out of its arguments. `register_pure` and `register_nullary`
|
||||
already had one arm; they are `register` with no fallback.
|
||||
|
||||
That last one is improvise-yf7p, and it is why the registry lost a parameter.
|
||||
#171 took a third function, `contextualize(C, &CmdContext) -> C`, handed the
|
||||
built command and not the args, and stated honestly that it could still
|
||||
**overwrite** a parsed field — naming the field was "a visible line in review".
|
||||
Review then injected five such overwrites on commands other than the one with a
|
||||
test (yank, paste and drill-into-cell taking their key from the cursor over the
|
||||
parsed one; toggle-panel-visibility ignoring its parsed panel; move-panel-cursor
|
||||
clamping its parsed delta to ±1) and the whole workspace stayed green. A hole
|
||||
guarded by review discipline plus one command's test is not guarded. Every
|
||||
contextualizer in the tree fell into one of two shapes, and each had an answer
|
||||
that removes the function rather than checking it:
|
||||
|
||||
- **The cursor** (`Move`, `EnterAdvance`, `CommitAndAdvance`). Always one
|
||||
`CursorState` field, always `CursorState::from_ctx`. So the registry does it,
|
||||
through a slot whose *type* is the guard: an `AtCursor` impl pointing at a
|
||||
`CellKey` or a delta is a compile error, which was tried.
|
||||
- **A read of context that belonged in `execute`** (`toggle-panel-visibility`'s
|
||||
`currently_open`, `cycle-panel-focus`'s three flags, `move-panel-cursor`'s
|
||||
cursor and bound). These copied context into the command only to read it back;
|
||||
`execute` now reads `ctx` directly, the same answer `toggle-panel-and-focus`
|
||||
got below. Those three are `register`/`register_nullary` now, so their
|
||||
interactive arm is exactly `parse`.
|
||||
|
||||
What `execute` does with a parsed field is still the command's own logic and is
|
||||
tested as such — `toggle_panel_visibility_reads_the_panel_it_names` and
|
||||
`move_panel_cursor_moves_from_the_named_panels_own_cursor` fail if the panel or
|
||||
the delta is ignored there. The difference is that the registry is no longer a
|
||||
second place such a bug can be written.
|
||||
|
||||
What is still representable, stated rather than hidden:
|
||||
|
||||
- `contextualize` can overwrite a field `parse` set. It would have to name the
|
||||
field, which is a visible line in review, where the old `|_args, ctx|` was an
|
||||
invisible omission. `commit_cell_edit_honours_an_explicit_value_and_cell_over_the_cursor`
|
||||
fails if `commit-cell-edit`'s does.
|
||||
- A `fallback` sees the args `parse` refused and could ignore some of them.
|
||||
`commit-cell-edit`'s refuses anything but a lone mode, so the malformed
|
||||
`commit-cell-edit editing 5` is an error on the `:` line — which has no pure
|
||||
@@ -207,9 +235,12 @@ What is still representable, stated rather than hidden:
|
||||
- The gate is **stricter** than the loop wherever a fallback exists: a bare
|
||||
`clear-cell` is refused in a script. That is the safe direction — the whole
|
||||
line is refused — and it is unchanged from before #151.
|
||||
- A new command could declare a *second* `CursorState` field and parse it from
|
||||
args; `cursor_mut` could then point at the wrong one. No command has one, and
|
||||
a parsed cursor would be a new idea deserving its own argument.
|
||||
|
||||
`toggle-panel-and-focus` did not fit `contextualize`, because its pure arm
|
||||
defaulted the omitted flags to `true` and a contextualizer cannot tell an
|
||||
`toggle-panel-and-focus` did not fit #171's `contextualize` either, because its
|
||||
pure arm defaulted the omitted flags to `true` and a contextualizer cannot tell an
|
||||
omitted flag from an explicit one. So the omission is now in the type —
|
||||
`target: Option<(bool, bool)>`, `None` meaning *toggle* — and the toggle is
|
||||
resolved in `execute` against the same context. That is the honest home: the
|
||||
|
||||
@@ -1706,6 +1706,31 @@ watchdog must be DISARMED and not merely pre-empted**: it fires later and would
|
||||
overwrite the better message with "did not answer" (PR #13 review). The watchdog
|
||||
is for silence only, and `setShellFailure` clears the timer.
|
||||
|
||||
**Success is claimed by the first frame, and only if nothing failed first**
|
||||
(improvise-i0v9). The worker transport used to write `in-page worker · <doc>`
|
||||
the moment it spawned the worker — 13ms after navigation, before anything had
|
||||
answered — so a worker that could not start showed a success-looking status
|
||||
over a blank page. It now writes `starting the worker · <doc>…`, and
|
||||
`createShell().arm(message, { answer })` holds the claim until `sawFrame`. The
|
||||
rule the claim obeys is **the watchdog's own state**: it is written only if the
|
||||
watchdog is still armed when the first frame lands. That is not a convenience.
|
||||
`worker.js` posts a `ReadOnly` (or `SeedMissing`) notice straight after
|
||||
`start()` and **before** the Snapshot that answers the handshake, so an
|
||||
unconditional "first frame ⇒ say we are up" would overwrite the precise reason
|
||||
the notice just wrote. `fail` already disarms the watchdog, so "armed" answers
|
||||
"has anything been said since arming" without a second flag to keep in step.
|
||||
`fail` disarms **directly** rather than through `sawFrame`, or it would
|
||||
briefly flash the success text on its way to the failure. The starting text
|
||||
deliberately does not contain `in-page worker`, so the older specs asserting
|
||||
that substring now need the first frame to pass. The socket path passes no
|
||||
`answer`: `open` is already a real fact, and its status was never premature.
|
||||
|
||||
One consequence changes what the watchdog guards. In the read-only case a
|
||||
Snapshot always follows the notice, and `sawFrame` disarms the watchdog either
|
||||
way, so a notice that forgot to disarm would **not** be overwritten eight
|
||||
seconds later. It is overwritten **at once**, by the claim. The eight-second
|
||||
overwrite survives only where no frame follows: `BootFailed`.
|
||||
|
||||
**Did this socket ever open?** The page has to record that itself, because
|
||||
nothing on the close event carries it: `readyState` is already CLOSED by the time
|
||||
a close listener runs, whichever happened, and a refused connection and a
|
||||
|
||||
+78
-6
@@ -553,6 +553,78 @@ status line.
|
||||
one, and the limit is checked in exactly one place — `Model::add_category` — so
|
||||
nothing in the effect restates it.
|
||||
|
||||
### A model effect that cannot do what it names refuses: `App::refuse`, `on_view` and `refuse_and_stop` (improvise-bok4, cz8d)
|
||||
|
||||
`AddCategory` was the only wire-reachable model effect that aborted when it
|
||||
failed. The others did one of two things:
|
||||
|
||||
- **Narrated.** `AddItem`, `AddItemInGroup`, `AddFormula`, `LoadModel` and
|
||||
`ImportJsonHeadless` wrote a sentence to the *host's* `status_msg` and
|
||||
carried on.
|
||||
- **Swallowed.** Every view-addressed effect (`SetAxis`, `SetPageSelection`,
|
||||
`ToggleGroup`, `HideItem`, `ShowItem`, `TransposeAxes`, `CycleAxis`,
|
||||
`SetNumberFormat`, `TogglePruneEmpty`) did nothing when `view_mut_of` found
|
||||
no such view. `DeleteView` discarded "cannot delete the last view" with
|
||||
`let _`, and answered `Ok` for a view the document did not have
|
||||
(improvise-40j4). `AddFormula` stored a formula for a category that was gone,
|
||||
through `let _ = add_item(…)` (improvise-40j4).
|
||||
|
||||
Either way the batch ran to its end, and `Host::mutating_frame` reported the
|
||||
frame **accepted**. That is clause 4's partial-and-silent shape, one layer below
|
||||
the record tier that `broken_chain` guards. The reviewer measured two cases.
|
||||
`[DeleteView("Scratch"), SetNumberFormat { view: "Scratch" }]` was accepted with
|
||||
half of it a no-op. `[RemoveCategory("Region"), AddItem { category: "Region" }]`
|
||||
was accepted too, and its reason went only to the host's own status line, so
|
||||
the client was told nothing at all.
|
||||
|
||||
**One writer, and the abort is separate (improvise-cz8d).** improvise-c5ip
|
||||
(PR #185) had meanwhile added `App::refuse`, which writes the reason, sets
|
||||
`batch_refused` and does **not** abort. The combination is now:
|
||||
|
||||
- **`App::refuse` is the only writer** of a refusal. bok4's own `refused()`
|
||||
helper, which aborted, is deleted.
|
||||
- **`on_view(app, target, write)`** writes the named view, or calls
|
||||
`App::refuse` naming the missing one.
|
||||
- **`Host::mutating_frame` rejects on `last_batch_refused() ||
|
||||
last_batch_aborted()`.** That is sound because each `ModelCommand` maps to a
|
||||
one-effect batch, so either flag belongs to the mutation that raised it.
|
||||
- **Aborting is an explicit extra step**, taken only where a later effect in a
|
||||
key-path batch assumes the failed one happened. `refuse_and_stop` does it for
|
||||
`LoadModel` and every `ImportJsonHeadless` arm, so `load x . q` and
|
||||
`import-json x . q` do not quit having loaded nothing. `AddCategory` keeps its
|
||||
own inline abort (gotcha 21). The item, formula and view effects only refuse:
|
||||
the frame is rolled back by the refusal alone. On the key path, a failed
|
||||
`AddItem`, `AddFormula` or view edit no longer stops the rest of its batch,
|
||||
which is what it did before bok4.
|
||||
|
||||
In a client's frame, any refusal becomes `Rejected(MutationRefused)`. The frame
|
||||
is undone whole, and the client is told, which is the channel the narrated
|
||||
reason never had.
|
||||
|
||||
**This takes the ticket's first option, scoped to the effects the wire can
|
||||
reach.** Failure is *reported* by the effect that knows it failed. The second
|
||||
option, growing `broken_chain` to ask whether every view, category and item
|
||||
target still resolves, would have been a second description of which targets
|
||||
each effect needs. That second copy would be free to disagree with the effect
|
||||
itself, which is gotcha 24's reason for observing rather than predicting.
|
||||
|
||||
Consequences:
|
||||
|
||||
- **A failed load is refused**, where it used to be accepted with a host status
|
||||
line. So `a_load_that_fails_is_not_a_rebase` now witnesses the failure by the
|
||||
refusal, not by `status_msg`. The frame's rollback also rolls the host's
|
||||
status line back.
|
||||
- **Every refusal site has a test that fails when it is reverted** to a
|
||||
status-only write (improvise-9fpl). Most are one table,
|
||||
`every_failing_model_command_refuses_its_whole_frame`. That matters because
|
||||
these are exactly the lines a merge conflict could silently drop.
|
||||
- **Not covered:** a no-op *inside* a view that does resolve. Examples are
|
||||
`SetAxis` on a category the view does not have, `HideItem` of an absent
|
||||
item, and `RemoveItem`/`RemoveCategory`/`RemoveFormula` of something already
|
||||
gone. The removals are idempotent by intent. The others are the remaining
|
||||
instances of this class, and each needs its own `View` method to say whether
|
||||
it changed anything.
|
||||
|
||||
### `AddFormula` and non-`_Measure` targets
|
||||
|
||||
For non-`_Measure` targets the item is added to the category so it appears in the
|
||||
@@ -2494,12 +2566,12 @@ available for the rare command that needs to enumerate all views, such as the vi
|
||||
panel.
|
||||
|
||||
The interactive arm (keymap dispatch, scripts, the `:` line) is built **from**
|
||||
`parse`: `register` takes `parse`, a `contextualize` that fills context-only
|
||||
fields of the command `parse` built without seeing the args, and an optional
|
||||
`fallback` reached only for argument lists `parse` refused. So an accepted
|
||||
argument list is never overridden by context (improvise-2bac; the argument is
|
||||
`context/code/cli.md` §8). `register_pure` is the case with nothing to
|
||||
contextualize and no fallback: both arms are `parse`.
|
||||
`parse`: `register` takes `parse` and an optional `fallback` reached only for
|
||||
argument lists `parse` refused, and `register_at_cursor` additionally has the
|
||||
registry set the command's `CursorState` to the live cursor. No registration
|
||||
writes any other field, so an accepted argument list is never overridden by
|
||||
context (improvise-2bac, improvise-yf7p; the argument is `context/code/cli.md`
|
||||
§8). `register_pure` is the case with no fallback: both arms are `parse`.
|
||||
|
||||
`write_buffer` is the effect that writes a named buffer — the mirror of
|
||||
`read_buffer`, and the only place that knows "search" is not an entry in the buffers
|
||||
|
||||
+70
-18
@@ -230,11 +230,33 @@ guard below (improvise-c7mu). It removes three things a scan must not see.
|
||||
composers, so no verdict was wrong yet. It also had a mirror: a comment
|
||||
explaining that a `NOT_COMPOSED` variant is held back would have raised a
|
||||
false "stale entry" alarm.
|
||||
- **Test items inside a file.** A line scan that relies on rustfmt putting an
|
||||
item's closing brace at the attribute's own indentation. It treats
|
||||
`cfg(any(test, …))` as test code and `cfg(not(test))` as production.
|
||||
Over-stripping **fails** the test rather than passing it, which is the safe
|
||||
direction.
|
||||
- **Test items inside a file.** Each test-only attribute's item is bounded by
|
||||
**balanced brackets on the blanked code** (improvise-59q8). It ends at a `;`
|
||||
at depth zero, or at the `}` closing its body unless `;`, `.`, `?` or `else`
|
||||
follows. The first version was a line scan that relied on rustfmt putting an
|
||||
item's closing brace at the attribute's own indentation. The #172 review
|
||||
found three shapes it never bounded:
|
||||
- an item ending in `];` or `);`;
|
||||
- a `#[cfg(test)]` statement spanning lines;
|
||||
- a one-line `#[cfg(test)] use x;`.
|
||||
|
||||
In each case the scan read on to the end of the file and **hid every line of
|
||||
production code after it**, which is the silent direction for the forward
|
||||
guard. An item the scan cannot bound is now a **panic**, never a scan to EOF.
|
||||
|
||||
**Which `cfg` is test-only is evaluated, not substring-matched.** The
|
||||
predicate is read off the file's *text*, because a `feature` value is a
|
||||
literal and blanked in the code. It is evaluated in three-valued logic for a
|
||||
build without `test`: `test` is false, and so is `feature = "test-support"`,
|
||||
which this repo never ships (improvise-26dm). Everything else a scan cannot
|
||||
know, such as a target or an ordinary feature, is unknown. An item is
|
||||
test-only when its predicate is **false**. So
|
||||
`cfg(all(test, not(target_arch = "wasm32")))` is test code, which the old
|
||||
"any line containing `not(`" rule got wrong. `cfg(any(test, feature =
|
||||
"serde"))` and `cfg(not(test))` are production. An inner `#![cfg(test)]`
|
||||
heading a file makes the whole file test code. One appearing anywhere else
|
||||
is a panic, because this scan cannot bound an inline module's inner
|
||||
attribute.
|
||||
- **Test-only files.** A `#[cfg(test)] mod x;` makes `x.rs` and everything under
|
||||
`x/` test code, even though nothing inside the file says so. Before
|
||||
improvise-c7mu, this checker's own file and `improvise-io`'s
|
||||
@@ -592,14 +614,37 @@ broader than its reason — improvise-gcvb, which also records the two things
|
||||
that make it more than a one-liner: an inserted `+cat` column means the TUI
|
||||
does **not** advance, and `AddCategory` can abort.
|
||||
|
||||
### `CellId::Coordinate` — a drawn coordinate is not "no cell" (improvise-exdg)
|
||||
|
||||
A records-mode coordinate cell (the `Region` column holding `North`) used to
|
||||
project `None`. That was a **third** meaning of `None`, next to the padded slot
|
||||
and the blanked position: a visible cell that holds text and whose write
|
||||
belongs to the host. So absence was encoding a permission, the same conflation
|
||||
`Create` removed for the creating columns (improvise-poia). The pinning test,
|
||||
`a_coordinate_column_is_projected_with_no_identity`, asserted the bug and is
|
||||
gone.
|
||||
|
||||
`Coordinate` carries **nothing**. Editing a coordinate renames it across every
|
||||
measure cell in the row (gotcha 1), and no client composes that operation: it
|
||||
needs `record_col`, and the browser declines every records-mode edit before
|
||||
reaching a cell. Giving the variant a `RecordHash` would hand a client a
|
||||
surrogate with nothing to spend it on. The operand belongs with the command
|
||||
that consumes it, whenever that command exists. Nothing a client can do has
|
||||
changed. `render.rs::editable` is now an exhaustive `match` rather than a
|
||||
`matches!`, so the next variant has to be classified. `typed_cell_command`,
|
||||
`record_at_cursor` and `record_cell_at_cursor` still answer `None` for it.
|
||||
So paste onto a coordinate is still a silent `Upstream` in the browser. The
|
||||
TUI has declined it out loud since PR #184. That divergence is
|
||||
improvise-kkcy, and a named kind is what lets the browser decline deliberately
|
||||
now.
|
||||
|
||||
### `CellDisplay::id` — `None` is not "aggregate"
|
||||
|
||||
That is `Some(CellId::Aggregate(..))`, and the distinction is the whole point
|
||||
of the enum. `None` means the slot holds **no projected cell**: a `GridCache`
|
||||
position padded by `fit_to_labels`, blanked by a reframing, or a
|
||||
records-mode column that is not a cell to write into at all (a category column
|
||||
renames a coordinate across the whole row, gotcha 1, which is a different
|
||||
intent from `SetCell`). Conflating the two either hides a cell or offers a
|
||||
position padded by `fit_to_labels`, or one blanked by a reframing. Every
|
||||
records-mode column is some kind of cell (`Record`, `Derived`, `Create` or
|
||||
`Coordinate`). Conflating the two either hides a cell or offers a
|
||||
write the type system exists to make unconstructible.
|
||||
|
||||
### `CellDisplay::edit_text` — the difference corrupts data
|
||||
@@ -615,8 +660,8 @@ and an editor's seed is exactly a feedback path.
|
||||
It is the projection of `App::display_value_at_cursor`, which is what
|
||||
`EditOrDrill` seeds the TUI's buffer from, so both front ends start from one
|
||||
string rather than each deriving one. `None` where there is nothing to edit:
|
||||
the same set of positions `id` is `None` for, plus cells whose value is
|
||||
absent. An error cell carries none, because its text is a diagnostic and not
|
||||
positions whose `id` is `None`, plus pivot cells with no key. A records-mode
|
||||
cell always carries one, even an empty one. An error cell carries none, because its text is a diagnostic and not
|
||||
a value.
|
||||
|
||||
A browser found this: `tests/lockstep.rs` drove `i` through the host and the
|
||||
@@ -812,9 +857,16 @@ one**. `AxisFrame::flat` leaves `cats` empty on purpose, so it is not a way to
|
||||
smuggle a multi-level axis in flattened.
|
||||
|
||||
`FrameGroup::collapsed` is on the wire rather than left to the client because
|
||||
it is a fact about the *view*, which only the server holds. `FrameGroup::cat`
|
||||
travels so a client can echo it back in `ToggleGroup` without ever working out
|
||||
which category a label came from — which slash-joined labels made impossible.
|
||||
it is a fact about the *view*, which only the server holds. There is no
|
||||
`FrameGroup::cat` any more (improvise-7xyx). Its doc comment promised that a
|
||||
client echoes it back in `ToggleGroup`, but no client ever did. `resolve.rs`
|
||||
has no `toggle-group-under-cursor` arm (`z` is on the still-needs-the-host
|
||||
list), and neither renderer read the field. A promise about an interaction no
|
||||
code performs is the stale-sibling defect, so the field went with it. It is
|
||||
not a loss for `z`. The TUI's `ToggleGroupAtCursor` also places the cursor on
|
||||
the layout the toggle *will produce*, which a thin client cannot compute. So
|
||||
composing `z` in a browser needs its own design, and the operand should come
|
||||
back with that design rather than wait for it.
|
||||
|
||||
### `AxisFrame`: four accessors that are **not for a draw loop**
|
||||
|
||||
@@ -1321,13 +1373,13 @@ frozen behaviour reads as decided afterwards and nothing says it was only frozen
|
||||
for the duration of the move.
|
||||
|
||||
**The ticket's own proposed fix does not work, and that is worth more than the
|
||||
fix.** h0iv suggested comparing the `(cat, name)` pair, `FrameGroup` carrying
|
||||
`cat` precisely so a client need not work out which category a label came from.
|
||||
fix.** h0iv suggested comparing the `(cat, name)` pair; `FrameGroup` carried
|
||||
`cat` then (improvise-7xyx has since removed it).
|
||||
But its own reachability case is *one* category: `Month` grouped into `Q1`,
|
||||
nested under `Region`, giving `North/Q1` and `South/Q1` adjacent — the two
|
||||
headers are `(Month, Q1)` and `(Month, Q1)`, **equal as values**, so the pair
|
||||
separates nothing. `the_two_headers_are_equal_as_values_so_comparing_cat_and_name_cannot_split_them`
|
||||
pins that, so the next reader does not re-propose it. What separates them is
|
||||
separates nothing. `the_two_headers_are_equal_as_values_so_comparing_them_cannot_split_them`
|
||||
pins that as whole-value equality, so the next reader does not re-propose it. What separates them is
|
||||
that they are two different `FrameEntry::Group` entries, and `groups()` already
|
||||
borrows each from `self.entries` — so the comparison is `std::ptr::eq` on the
|
||||
header, which is the literal statement of "the same header opened both".
|
||||
|
||||
@@ -298,8 +298,9 @@ The unit-level form of the headless test, against `CmdRegistry::interactive`
|
||||
directly: cursor on `Type=Food, Month=Jan`, a decoy `edit` buffer, and an
|
||||
explicit `5` for `Type/Clothing Month/Feb`. It asserts the built command's key
|
||||
and value. Rival (a) above — a `contextualize` that overwrites the parsed
|
||||
fields — fails it; this is the one hole the new registry shape leaves
|
||||
representable, so it is the one that needs a test.
|
||||
fields — failed it. improvise-yf7p then removed `contextualize` altogether
|
||||
(`context/code/cli.md` §8), so rival (a) no longer compiles; the test stays as
|
||||
the behavioural statement of what the signature now guarantees.
|
||||
|
||||
### every_argument_list_the_gate_accepts_the_run_accepts_too
|
||||
|
||||
|
||||
@@ -3054,6 +3054,23 @@ The third is the gotcha 48 property, and it needs `timesArmed() > 0` first or it
|
||||
denies an unreachable event: the precise reason must still be on screen after
|
||||
the 8s window, not replaced by the watchdog. `page.clock` walks past it.
|
||||
|
||||
**Strengthened by improvise-i0v9's second pass.** Every assertion above read the
|
||||
*final* `#status`, so two failures could still pass it. A watchdog that fired
|
||||
first and was then replaced by a late notice passed. So did a success claim
|
||||
shown before the notice. The test now records every status with `logStatus` and
|
||||
asserts, **before** the page clock is walked, that the log ends in `read-only`
|
||||
and never contained `did not answer` or the exact `in-page worker · <doc>`. A
|
||||
reason that arrived after the watchdog would sit in that log behind "did not
|
||||
answer"; that is the "well before 8s" property, stated without a wall-clock
|
||||
bound. Measured failing against `gh/main` at `da2e5ab`, whose log was
|
||||
`["in-page worker · two-tab-….improv", "read-only — …"]`: the premature claim
|
||||
the ticket measured at 13ms is still there once 1wft's notice exists. The
|
||||
brief's rival, `onNotice` calling `setShell` instead of `fail` (the watchdog not
|
||||
disarmed), fails it at the first `toContainText("read-only")`. The Snapshot's
|
||||
claim replaced the reason at once, which is why the eight-second form of this
|
||||
rival is now exercised by the invalid-`?doc=` spec instead (see
|
||||
`context/code/clients.md`, "The watchdog").
|
||||
|
||||
It runs on Chromium and Firefox — the two engines with OPFS — rather than
|
||||
Chromium alone, because the exclusivity semantics are the engine's and the two
|
||||
word the refusal completely differently.
|
||||
@@ -4446,6 +4463,38 @@ registers to agree; a TUI-only blank filter (`Text("")` to `None`) fails it on
|
||||
`+`, which is why that filter was removed rather than kept. The evidence entries
|
||||
for the host-side tests are in `command.md`.
|
||||
|
||||
### the worker is not called up until its first frame has arrived
|
||||
|
||||
improvise-i0v9. The worker transport wrote `in-page worker · <doc>` when it
|
||||
spawned the worker, before anything had answered. A worker that then failed
|
||||
therefore showed a success-looking status over a blank page. The spec records
|
||||
every `#status` with `logStatus` and requires the whole log to be exactly
|
||||
`["starting the worker · <doc>…", "in-page worker · <doc>"]`. It pins the order
|
||||
as well as the final text, since a final read cannot see a claim made too early.
|
||||
Measured failing against `gh/main` at `da2e5ab`. The rival, restoring the
|
||||
immediate `setShell(\`in-page worker · ${DOC}\`)` in `workerTransport`, fails it
|
||||
on the log's first entry. The final read is `toHaveText`, not `toContainText`.
|
||||
The older specs' `toContainText("in-page worker")` could not tell the starting
|
||||
text from the claim if the one contained the other, and the starting text
|
||||
deliberately does not.
|
||||
|
||||
### only the first frame claims the peer answered, and not before it
|
||||
|
||||
improvise-i0v9, the unit half of the spec above. `arm(message, { answer })`
|
||||
must write nothing until `sawFrame`, then write `answer` once, however many
|
||||
frames follow. Fails against `gh/main`'s `createShell`, which has no `answer`
|
||||
and so never writes the claim at all.
|
||||
|
||||
### a reason given before the first frame is not replaced by the frame's claim
|
||||
|
||||
The order `worker.js` really produces on a locked document: the `ReadOnly`
|
||||
notice is posted after `start()` and **before** the Snapshot. So `fail` then
|
||||
`sawFrame` must leave the notice's text as the only thing ever shown. The rival
|
||||
is the obvious fix, `sawFrame` writing `answer` unconditionally. It fails both
|
||||
this test and the one above: the first on its exact log, which is then written
|
||||
twice. This test passes against `gh/main` by construction, since no claim exists
|
||||
there. It is not evidence of the bug; it guards against the naive fix.
|
||||
|
||||
## Numbers across the serialized wire (improvise-6oy4)
|
||||
|
||||
No manifest enabled serde_json's `float_roundtrip`, so its default f64 parser
|
||||
|
||||
@@ -1978,12 +1978,27 @@ Two measure columns of one row are two cells of one record, and the next row is
|
||||
different record — the same shape the pivot case has, checked here because records
|
||||
mode reaches it by a different path.
|
||||
|
||||
### a_coordinate_column_is_projected_with_no_identity
|
||||
### a_coordinate_cell_is_named_rather_than_left_as_no_cell
|
||||
|
||||
A coordinate column is not a cell to write into: editing it renames that coordinate
|
||||
across *every* measure cell in the row (gotcha 1), which is a different intent from
|
||||
`SetCell`. So it is projected with no id at all, and a client cannot compose a
|
||||
write against it.
|
||||
improvise-exdg. This entry replaces `a_coordinate_column_is_projected_with_no_identity`,
|
||||
a test that **pinned the bug**. It asserted that a records-mode coordinate cell
|
||||
projects `id: None`. ADR 002 and gotcha 42 define `None` as "no projected cell",
|
||||
yet the Region column is drawn and holds text. So a client could not tell it
|
||||
from a padded slot. Absence was being used to encode a permission ("the host's"),
|
||||
which is the conflation ADR 002's rejected `Option<CellId>` draft was about.
|
||||
`improvise-poia` removed the same conflation for the creating columns.
|
||||
|
||||
The cell now projects `CellId::Coordinate`. That keeps the old test's real
|
||||
property: it is still not a `SetCell` target, because `typed_cell_command`,
|
||||
`record_cell_at_cursor` and `render.rs::editable` all match `Record` alone.
|
||||
The test seats the cursor on the **second** row, so the first-row and
|
||||
first-item cases cannot coincide. It then enumerates every cell of the records
|
||||
grid and requires each to carry an id, which is the ticket's claim with no
|
||||
case picked by hand. Before the fix it failed at `a drawn coordinate projected
|
||||
as no cell`. Rival injected afterwards: `RecordCol::Category(_) => return None`
|
||||
restored. This test failed at the `Some(CellId::Coordinate)` assertion, and
|
||||
`a_creating_column_is_named_rather_than_left_as_no_cell` (whose `None`-count
|
||||
now expects zero) failed too.
|
||||
|
||||
### a_write_moves_the_version_under_the_clients_own_surrogate
|
||||
|
||||
@@ -3140,10 +3155,13 @@ Injected fault: hardcode the `category_panel_open` propagator to `true`. Fails.
|
||||
|
||||
### toggle_panel_visibility_reads_the_panel_it_names
|
||||
|
||||
The behavioural half, and the one that reaches the registry. Its interactive arm
|
||||
passes `currently_open` straight through, so with exactly one panel open the command
|
||||
built for each of the three names carries its own panel's flag. Both injections above
|
||||
fail here too.
|
||||
The behavioural half, and the one that reaches the registry. With exactly one panel
|
||||
open, the command built for each of the three names and executed must emit
|
||||
`SetPanelOpen` for **its own** panel, flipping **its own** flag. Both injections above
|
||||
fail here too. Since improvise-yf7p the flag is read in `execute` rather than copied
|
||||
in by a registry contextualizer, so the assertion moved from the built command's
|
||||
`Debug` to its effects; it also names the panel, so an `execute` that ignores the
|
||||
parsed panel (`ctx.panel_open(Panel::Formula)`) fails it — injected and run.
|
||||
|
||||
### a_focused_open_panel_closes_and_a_closed_one_opens
|
||||
|
||||
@@ -3295,11 +3313,12 @@ aggregates, arriving a third time.
|
||||
|
||||
The test walks **every** column of a records grid and counts, rather than
|
||||
asserting the two creating columns alone. That matters in the direction a
|
||||
weaker test fails: a producer that answered `Create` for everything, or that
|
||||
made the `Region` coordinate column nameable, would pass a two-position check
|
||||
and fails this one. The fault injected — returning `None` for both creating
|
||||
columns, which is the pre-fix code — reports three nameless columns where one
|
||||
is allowed.
|
||||
weaker test fails: a producer that answered `Create` for everything would pass
|
||||
a two-position check and fails this one. The fault injected — returning `None`
|
||||
for both creating columns, which is the pre-fix code — reported three nameless
|
||||
columns where one was allowed. Since improvise-exdg the coordinate column is
|
||||
named too (`CellId::Coordinate`), so the count of nameless columns is now zero.
|
||||
See `a_coordinate_cell_is_named_rather_than_left_as_no_cell`.
|
||||
|
||||
### a_records_coordinate_is_text_however_it_is_spelled
|
||||
|
||||
@@ -4728,19 +4747,52 @@ Its mirror, and the one that pins the wrap: from the first item `[` must land on
|
||||
`SetPanelCursor` carries a panel *and* a cursor and neither was checked. Both
|
||||
are now named, so moving the right cursor on the wrong panel fails.
|
||||
|
||||
### move_panel_cursor_moves_from_the_named_panels_own_cursor
|
||||
|
||||
improvise-yf7p. `move-panel-cursor formula 2` through the registry's interactive
|
||||
arm, with the formula cursor at **1** of five formulas and the category and view
|
||||
cursors at 0, must land on **3**. Before yf7p the cursor and bound were copied in
|
||||
by a registry `contextualize`, and review showed one that clamped the parsed delta
|
||||
to `delta.signum()` left the whole workspace green: every test moved by ±1, where
|
||||
a clamp is invisible. They are read in `execute` now, and this test is what sees a
|
||||
bug there. Rival `self.delta.signum()` in `execute` — injected, run, fails (lands
|
||||
on 2). Reading another panel's cursor fails too, because the fixture seats them at
|
||||
different values — the index-0 fixture could not separate them.
|
||||
|
||||
### delete_formula_at_cursor_with_formulas
|
||||
|
||||
Asserts the `RemoveFormula` names `Profit` in `Type` — the formula the cursor is
|
||||
on — rather than merely that a formula was removed.
|
||||
Asserts the `RemoveFormula` names the formula the cursor is on, rather than merely
|
||||
that a formula was removed. PR #148 named the field, but its fixture was **one**
|
||||
formula with the cursor at **0**, where "the formula under the cursor" and "the
|
||||
first formula" are the same formula (improvise-y1av). The rival it could not see —
|
||||
`DeleteFormulaAtCursor` reading index `0` instead of `ctx.formula_cursor` — passed
|
||||
the **whole workspace**, so `d` in the formula panel deleting the wrong formula had
|
||||
no guard anywhere. Now two formulas, cursor at **1**: it must remove `Tax` and must
|
||||
not name `Profit`. Rival `let cursor = 0usize.min(..)` injected, full suite run:
|
||||
this is the only test that fails.
|
||||
|
||||
### switch_view_at_cursor_with_valid_cursor
|
||||
|
||||
Names `Default`, the view under the cursor.
|
||||
Names the view under the cursor. Same fixture defect as above (improvise-y1av):
|
||||
`two_cat_model` has one view, so the cursor at 0 named `Default` whether or not
|
||||
the command read it. Now `Default, Other` with the cursor at **1**, asserting
|
||||
`Other`. Rival `get(cursor).and(first())` — in bounds only, so the out-of-bounds
|
||||
arm is unchanged and `switch_view_at_cursor_out_of_bounds_returns_empty` cannot be
|
||||
what catches it — injected and run: fails here, in
|
||||
`delete_view_at_cursor_deletes_the_view_under_a_nonzero_cursor`, and in the two
|
||||
`wasm-client/tests/lockstep.rs` tests that were previously the only guard.
|
||||
|
||||
### delete_view_at_cursor_deletes_the_view_under_a_nonzero_cursor
|
||||
|
||||
improvise-y1av. `Default, Other, Third` with the cursor at **1**: deletes `Other`
|
||||
and steps the panel cursor back to 0. The same rival as above fails it.
|
||||
|
||||
### delete_view_at_cursor_zero_does_not_adjust_cursor
|
||||
|
||||
Names `Default` too. Its existing negative half — no `SetPanelCursor` at
|
||||
position 0 — is unchanged.
|
||||
position 0 — is unchanged. Position 0 is this test's subject, so it cannot
|
||||
separate "the view under the cursor" from "the first view" and does not try to;
|
||||
the test above does.
|
||||
|
||||
### commit_category_add_with_name_produces_add_effect
|
||||
|
||||
@@ -5006,6 +5058,31 @@ run alone with the ones before it removed, and each fails: the diff returns
|
||||
test passes. See
|
||||
`render::tests::a_cell_equals_another_exactly_when_the_wire_cannot_tell_them_apart`.
|
||||
|
||||
### ui::host::tests::a_frame_writing_a_view_its_own_first_command_deleted_is_refused
|
||||
|
||||
improvise-bok4. This is the reviewer's first measured case:
|
||||
`[DeleteView("Scratch"), SetNumberFormat { view: "Scratch" }]`. Before the fix
|
||||
the asker's reply was `[Commands([ForgetView("Scratch")]), Commands([Project…])]`
|
||||
and then `Accepted`. The second command had done nothing, because
|
||||
`view_mut_of` found no view and every view-addressed effect swallowed the
|
||||
`None`. Now `on_view` refuses, the abort becomes
|
||||
`Rejected(MutationRefused)`, and the assertion that `Scratch` still exists
|
||||
proves the deletion was undone with the rest of the frame.
|
||||
|
||||
Its sibling, `a_frame_adding_an_item_to_a_category_its_first_command_removed_is_refused`,
|
||||
is the reviewer's second case: `[RemoveCategory("Region"), AddItem { category:
|
||||
"Region" }]`. It was accepted as well, with `AddItem`'s error written only to
|
||||
the host's `status_msg`. Both tests failed first against the unfixed effects.
|
||||
|
||||
`deleting_the_documents_last_view_is_refused_rather_than_accepted` covers the
|
||||
one-command case, which `DeleteView`'s `let _` swallowed. The fault injected
|
||||
was to put the `let _` back. The test then failed with a projection batch
|
||||
where the refusal should have been, and it was reverted.
|
||||
|
||||
The control is `a_frame_whose_commands_all_land_is_accepted`, the same two
|
||||
commands in an order where both land. Without it, a host that refused every
|
||||
two-command frame would pass all three tests.
|
||||
|
||||
### ui::reduce::vocabulary_is_complete::every_effect_is_on_the_wire_or_has_a_verdict
|
||||
|
||||
improvise-h3y7, the forward direction of the vocabulary derivation. It is the
|
||||
@@ -5139,6 +5216,53 @@ the first message does not mention drilling. At `0ed2e41` it failed with the
|
||||
old text. The rival is restoring that text. Real drill paste, which means
|
||||
staging a typed `CellValue`, is still open as improvise-48jd.
|
||||
|
||||
### ui::host::tests::every_failing_model_command_refuses_its_whole_frame
|
||||
|
||||
improvise-9fpl, found by PR #181's review. Eight of bok4's refusal sites had no
|
||||
test: reverting any one of them to a status-only write failed nothing. They
|
||||
were `AddItemInGroup`, `AddFormula`'s parse error, and all six
|
||||
`ImportJsonHeadless` arms. Two improvise-40j4 cases were added to the same
|
||||
table: `DeleteView` of an absent view, and `AddFormula` into an absent
|
||||
category.
|
||||
|
||||
Each case sends a frame of two commands. The first, `AddItem { Region,
|
||||
"Landed" }`, lands. The second fails. The test asserts that the asker hears
|
||||
exactly `[Rejected(MutationRefused)]` and that `Landed` is gone. The fixture
|
||||
has a second view, `Scratch`, because with one view `DeleteView("Ghost")` is
|
||||
refused as "the last view" and the absent-name case is never reached. The
|
||||
first run found exactly that: the case passed before the fix.
|
||||
|
||||
Faults injected by a script (`/tmp/wire2_inject2.py`, not committed): each of
|
||||
the 14 refusal sites was reverted **alone** to a status-only write, after the
|
||||
merge with #185. Every one failed at least one test. The eight that were
|
||||
unwitnessed fail this test, `DeleteView`'s last-view arm fails
|
||||
`deleting_the_documents_last_view_is_refused_rather_than_accepted`, `on_view`
|
||||
fails `a_frame_writing_a_view_its_own_first_command_deleted_is_refused`, and
|
||||
`LoadModel` fails both `a_load_that_fails_is_not_a_rebase` and
|
||||
`a_failed_load_or_import_does_not_run_the_quit_after_it`. The file was
|
||||
restored and compared.
|
||||
|
||||
### ui::host::tests::a_frame_adding_a_formula_to_a_category_its_first_command_removed_is_refused
|
||||
|
||||
improvise-40j4. `[RemoveCategory("Region"), AddFormula { "Total = 1", Region }]`
|
||||
was accepted, and the document held a formula for a category that did not
|
||||
exist; `format_md` wrote it and `parse_md` reloaded it. `AddFormula` ran
|
||||
`let _ = add_item(…)` and then `add_formula` anyway. The test failed first with
|
||||
the frame accepted. `AddFormula` now refuses when `add_item` errs, before
|
||||
storing anything.
|
||||
|
||||
### ui::app::tests::a_failed_load_or_import_does_not_run_the_quit_after_it
|
||||
|
||||
improvise-cz8d. After #185, `App::refuse` does not abort. So on gh/main a failed
|
||||
`load` refused, and the `force-quit` after it in the same batch still ran:
|
||||
`load absent.improv . q` quit having loaded nothing. bok4's aborting helper had
|
||||
prevented that, and the merge would have silently lost it. The test binds
|
||||
`[load|import-json <absent>, force-quit]` as one sequence and asserts that the
|
||||
refusal is recorded and the mode is not `Quit`. `import-json` failed first,
|
||||
before `refuse_and_stop` was applied to its arms. For `load`, the injected fault
|
||||
is removing its abort (`refuse_and_stop` back to `app.refuse`), and the test
|
||||
then fails with "`load absent.improv . q` quit having loaded nothing".
|
||||
|
||||
### `an_untouched_pivot_edit_of_a_misread_value_writes_nothing`
|
||||
### `an_untouched_records_edit_of_a_misread_value_writes_nothing`
|
||||
|
||||
|
||||
@@ -557,7 +557,7 @@ the evidence that nothing was relying on the permissive parse.
|
||||
|
||||
### render::tests::two_runs_under_two_headers_of_one_name_are_two_runs
|
||||
|
||||
### render::tests::the_two_headers_are_equal_as_values_so_comparing_cat_and_name_cannot_split_them
|
||||
### render::tests::the_two_headers_are_equal_as_values_so_comparing_them_cannot_split_them
|
||||
|
||||
improvise-h0iv. `group_runs` coalesced adjacent positions by group **name**, so
|
||||
two runs under two different headers sharing a name read as one and both front
|
||||
@@ -571,10 +571,12 @@ and nothing ungrouped between the runs. Before the fix it returns
|
||||
at the precondition rather than pass with a single run.
|
||||
|
||||
The second test exists because the ticket proposed the wrong fix and a reader
|
||||
will propose it again. h0iv says `FrameGroup` carries `cat` and "comparing the
|
||||
pair would fix it" — but its own reachability case is a single category nested
|
||||
under an outer one, so both headers are `(Month, Q1)`, equal as values. The test
|
||||
asserts that equality directly. What actually separates them is that they are
|
||||
will propose it again. h0iv said `FrameGroup` carried `cat` and that "comparing
|
||||
the pair would fix it". But its own reachability case is a single category
|
||||
nested under an outer one, so both headers are `(Month, Q1)`, equal as values.
|
||||
The test asserts that equality directly. Since improvise-7xyx removed
|
||||
`FrameGroup::cat`, it asserts equality of the whole header, which is stronger:
|
||||
no field-wise comparison can separate two equal values. What actually separates them is that they are
|
||||
two distinct entries in `self.entries`, which `std::ptr::eq` on the borrowed
|
||||
header states exactly.
|
||||
|
||||
@@ -1050,3 +1052,51 @@ Faults injected, one per half, each compiling only `improvise-protocol`:
|
||||
A field added to an existing variant is caught too, but not in isolation,
|
||||
because every constructor of that variant also fails. Reverted with
|
||||
`git restore`.
|
||||
|
||||
### source_scan::tests::a_test_only_item_hides_nothing_after_it
|
||||
|
||||
improvise-59q8, found by the PR #172 review. The scanner's line-based
|
||||
`strip_test_items` looked for `}` at the attribute's indentation, or for an item
|
||||
line ending in `;`. So three shapes were never bounded, and it dropped
|
||||
**everything to the end of the file**:
|
||||
- a test-only `const … = &[\n … \n];`;
|
||||
- a `#[cfg(test)] let x = foo(\n 1,\n);` statement;
|
||||
- a one-line `#[cfg(test)] use foo::bar;`, where the attribute's own line is
|
||||
also the item line.
|
||||
|
||||
In `vocabulary_is_complete` that silently hides every effect after such an
|
||||
item. Each shape is a case here, asserting that the production code after it
|
||||
(`Hidden1`, `Visible`) survives. Against the line scan all three failed; the
|
||||
first printed `"impl X {\n"` and nothing else. They now pass because items are
|
||||
bounded by balanced brackets on the blanked code.
|
||||
`an_unbounded_test_only_item_is_refused_not_swallowed` pins the other half of
|
||||
the acceptance criteria: an item that cannot be bounded panics instead of
|
||||
consuming the rest of the file. Against the line scan it failed, because
|
||||
nothing panicked.
|
||||
|
||||
### source_scan::tests::a_test_only_cfg_is_recognised_whatever_its_spelling
|
||||
|
||||
improvise-59q8, the other direction: test code counted as production, which
|
||||
makes `vocabulary_is_composed` weaker. `cfg(all(test, not(target_arch =
|
||||
"wasm32")))` was kept because the old rule rejected any line containing
|
||||
`not(`. A leading `#![cfg(test)]` was kept because only `#[cfg(` was
|
||||
recognised. An attribute split across lines was not seen at all. All three
|
||||
failed against the old scanner. The predicate is now parsed and evaluated in
|
||||
three-valued logic (`context/code/protocol.md` §2). The controls
|
||||
(`cfg(not(test))`, `cfg(any(test, feature = "serde"))` and `cfg(target_arch =
|
||||
…)`) must stay production, so a scanner that dropped every `cfg` item could
|
||||
not pass.
|
||||
|
||||
A consequence to know about: `cfg(any(test, feature = "x"))` for an ordinary
|
||||
feature is now production. Only `feature = "test-support"`, the one test
|
||||
feature this repo has, counts as a test build.
|
||||
`test_only_items_and_modules_are_not_production` was updated to that spelling.
|
||||
|
||||
### source_scan::tests::a_c_string_literal_is_blanked
|
||||
|
||||
improvise-59q8's minor case. In `cr#"x" Command::InRawC(2) "y"#` the `r` has a
|
||||
word character before it, so the literal was lexed as two ordinary strings,
|
||||
and the text between the inner quotes leaked. It failed as
|
||||
`InRawC survived`. `c"…"` and `cr#"…"#` are now literals. A first version of
|
||||
this test used a C string with no inner quotes and passed against the bug,
|
||||
because the ordinary lexing happened to blank it too.
|
||||
|
||||
@@ -3,6 +3,8 @@ use std::fmt::Debug;
|
||||
|
||||
use crate::command::key::KeyCode;
|
||||
|
||||
use super::navigation::CursorState;
|
||||
|
||||
use crate::model::Model;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::ui::app::AppMode;
|
||||
@@ -141,13 +143,15 @@ pub trait Cmd: Debug + Send + Sync {
|
||||
/// Factory that constructs a Cmd from text arguments (headless/script).
|
||||
pub type ParseFn = fn(&[String]) -> Result<Box<dyn Cmd>, String>;
|
||||
|
||||
/// Fills the context-only fields of a command `parse` already built. It never
|
||||
/// sees the args, so it cannot discard them — `context/code/cli.md` §8.
|
||||
pub type ContextualizeFn<C> = fn(C, &CmdContext) -> C;
|
||||
|
||||
/// Builds a command from context, reached ONLY for args `parse` refused.
|
||||
pub type FallbackFn<C> = fn(&[String], &CmdContext) -> Result<C, String>;
|
||||
|
||||
/// A command whose one context-only field is the cursor it moves from. The
|
||||
/// registry fills it, and no registration can — `context/code/cli.md` §8.
|
||||
pub trait AtCursor: Cmd {
|
||||
fn cursor_mut(&mut self) -> &mut CursorState;
|
||||
}
|
||||
|
||||
type BoxParseFn = Box<dyn Fn(&[String]) -> Result<Box<dyn Cmd>, String>>;
|
||||
type BoxInteractiveFn = Box<dyn Fn(&[String], &CmdContext) -> Result<Box<dyn Cmd>, String>>;
|
||||
|
||||
@@ -188,25 +192,48 @@ impl CmdRegistry {
|
||||
name
|
||||
}
|
||||
|
||||
/// Register a command whose interactive arm is `parse` then `contextualize`, so
|
||||
/// args `parse` accepts are honoured whole — `context/code/cli.md` §8.
|
||||
/// Register a command whose interactive arm is `parse`, reaching `fallback`
|
||||
/// only where `parse` refused — `context/code/cli.md` §8.
|
||||
pub fn register<C: Cmd + 'static>(
|
||||
&mut self,
|
||||
prototype: &C,
|
||||
parse: fn(&[String]) -> Result<C, String>,
|
||||
contextualize: ContextualizeFn<C>,
|
||||
fallback: Option<FallbackFn<C>>,
|
||||
) {
|
||||
self.push(prototype.name(), parse, fallback, |_, _| {});
|
||||
}
|
||||
|
||||
/// As [`register`](Self::register), then the interactive arm moves the
|
||||
/// command's cursor to the live one. The cursor is the only field it writes.
|
||||
pub fn register_at_cursor<C: AtCursor + 'static>(
|
||||
&mut self,
|
||||
prototype: &C,
|
||||
parse: fn(&[String]) -> Result<C, String>,
|
||||
fallback: Option<FallbackFn<C>>,
|
||||
) {
|
||||
self.push(prototype.name(), parse, fallback, |cmd, ctx| {
|
||||
*cmd.cursor_mut() = CursorState::from_ctx(ctx);
|
||||
});
|
||||
}
|
||||
|
||||
fn push<C: Cmd + 'static>(
|
||||
&mut self,
|
||||
name: &'static str,
|
||||
parse: fn(&[String]) -> Result<C, String>,
|
||||
fallback: Option<FallbackFn<C>>,
|
||||
at_cursor: fn(&mut C, &CmdContext),
|
||||
) {
|
||||
self.entries.push(CmdEntry {
|
||||
name: prototype.name(),
|
||||
name,
|
||||
parse: Box::new(move |args| parse(args).map(|c| Box::new(c) as Box<dyn Cmd>)),
|
||||
interactive: Box::new(move |args, ctx| {
|
||||
let built = match (parse(args), fallback) {
|
||||
(Ok(cmd), _) => Ok(contextualize(cmd, ctx)),
|
||||
(Err(_), Some(fallback)) => fallback(args, ctx),
|
||||
(Err(err), None) => Err(err),
|
||||
let mut built = match (parse(args), fallback) {
|
||||
(Ok(cmd), _) => cmd,
|
||||
(Err(_), Some(fallback)) => fallback(args, ctx)?,
|
||||
(Err(err), None) => return Err(err),
|
||||
};
|
||||
built.map(|c| Box::new(c) as Box<dyn Cmd>)
|
||||
at_cursor(&mut built, ctx);
|
||||
Ok(Box::new(built) as Box<dyn Cmd>)
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -118,6 +118,22 @@ pub(super) mod test_helpers {
|
||||
wb
|
||||
}
|
||||
|
||||
/// `two_cat_model` with one `_Measure` formula per target, in order.
|
||||
pub fn with_formulas(targets: &[&str]) -> Workbook {
|
||||
let mut wb = two_cat_model();
|
||||
for target in targets {
|
||||
wb.model.add_formula(crate::formula::ast::Formula {
|
||||
raw: format!("{target} = 0"),
|
||||
target: target.to_string(),
|
||||
target_category: "_Measure".to_string(),
|
||||
expr: crate::formula::ast::Expr::Number(0.0),
|
||||
filter: None,
|
||||
format: None,
|
||||
});
|
||||
}
|
||||
wb
|
||||
}
|
||||
|
||||
pub fn effects_debug(effects: &[Box<dyn Effect>]) -> String {
|
||||
format!("{:?}", effects)
|
||||
}
|
||||
|
||||
@@ -192,11 +192,7 @@ mod tests {
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: false,
|
||||
view_open: false,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert!(effects.is_empty());
|
||||
}
|
||||
@@ -208,11 +204,7 @@ mod tests {
|
||||
let reg = make_registry();
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.formula_panel_open = true;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: true,
|
||||
category_open: false,
|
||||
view_open: false,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = format!("{:?}", effects[0]);
|
||||
@@ -231,11 +223,7 @@ mod tests {
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.formula_panel_open = true;
|
||||
ctx.category_panel_open = true;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: true,
|
||||
category_open: true,
|
||||
view_open: false,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
@@ -256,11 +244,7 @@ mod tests {
|
||||
ctx.formula_panel_open = true;
|
||||
ctx.category_panel_open = true;
|
||||
ctx.mode = &AppMode::FormulaPanel;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: true,
|
||||
category_open: true,
|
||||
view_open: false,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
@@ -281,11 +265,7 @@ mod tests {
|
||||
ctx.category_panel_open = true;
|
||||
ctx.view_panel_open = true;
|
||||
ctx.mode = &AppMode::CategoryPanel;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: true,
|
||||
view_open: true,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
@@ -305,11 +285,7 @@ mod tests {
|
||||
ctx.formula_panel_open = true;
|
||||
ctx.view_panel_open = true;
|
||||
ctx.mode = &AppMode::ViewPanel;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: true,
|
||||
category_open: false,
|
||||
view_open: true,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
@@ -329,11 +305,7 @@ mod tests {
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.category_panel_open = true;
|
||||
ctx.mode = &AppMode::CategoryPanel;
|
||||
let cmd = CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: true,
|
||||
view_open: false,
|
||||
};
|
||||
let cmd = CyclePanelFocus;
|
||||
let effects = cmd.execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
@@ -344,15 +316,13 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn move_panel_cursor_down_from_zero() {
|
||||
let m = two_cat_model();
|
||||
let m = with_formulas(&["A", "B"]);
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let cmd = MovePanelCursor {
|
||||
panel: effect::Panel::Formula,
|
||||
delta: 1,
|
||||
current: 0,
|
||||
max: 5,
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
@@ -363,17 +333,35 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn move_panel_cursor_moves_from_the_named_panels_own_cursor() {
|
||||
let m = with_formulas(&["A", "B", "C", "D", "E"]);
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.formula_cursor = 1;
|
||||
let args = ["formula".to_string(), "2".to_string()];
|
||||
let effects = reg
|
||||
.interactive("move-panel-cursor", &args, &ctx)
|
||||
.expect("move-panel-cursor resolves")
|
||||
.execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("SetPanelCursor { panel: Formula, cursor: 3 }"),
|
||||
"must move from the formula cursor (1) by the parsed delta (2), got: {dbg}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn move_panel_cursor_clamps_at_zero() {
|
||||
let m = two_cat_model();
|
||||
let m = with_formulas(&["A", "B"]);
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let cmd = MovePanelCursor {
|
||||
panel: effect::Panel::Formula,
|
||||
delta: -1,
|
||||
current: 0,
|
||||
max: 5,
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert!(effects.is_empty());
|
||||
@@ -388,46 +376,55 @@ mod tests {
|
||||
let cmd = MovePanelCursor {
|
||||
panel: effect::Panel::Formula,
|
||||
delta: 1,
|
||||
current: 0,
|
||||
max: 0,
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert!(effects.is_empty());
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn delete_formula_at_cursor_with_formulas() {
|
||||
let mut m = two_cat_model();
|
||||
m.model.add_formula(crate::formula::ast::Formula {
|
||||
raw: "Profit = Revenue - Cost".to_string(),
|
||||
target: "Profit".to_string(),
|
||||
target_category: "Type".to_string(),
|
||||
expr: crate::formula::ast::Expr::Number(0.0),
|
||||
filter: None,
|
||||
format: None,
|
||||
});
|
||||
for (target, raw) in [("Profit", "Profit = 1"), ("Tax", "Tax = 2")] {
|
||||
m.model.add_formula(crate::formula::ast::Formula {
|
||||
raw: raw.to_string(),
|
||||
target: target.to_string(),
|
||||
target_category: "Type".to_string(),
|
||||
expr: crate::formula::ast::Expr::Number(0.0),
|
||||
filter: None,
|
||||
format: None,
|
||||
});
|
||||
}
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.formula_cursor = 1;
|
||||
let effects = DeleteFormulaAtCursor.execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("RemoveFormula { target: \"Profit\", target_category: \"Type\" }"),
|
||||
"must remove the formula AT THE CURSOR, got: {dbg}"
|
||||
dbg.contains("RemoveFormula { target: \"Tax\", target_category: \"Type\" }"),
|
||||
"must remove the formula AT THE CURSOR (Tax, index 1), got: {dbg}"
|
||||
);
|
||||
assert!(
|
||||
!dbg.contains("\"Profit\""),
|
||||
"removed the first formula, not the one under the cursor: {dbg}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn switch_view_at_cursor_with_valid_cursor() {
|
||||
let m = two_cat_model();
|
||||
let mut m = two_cat_model();
|
||||
m.create_view("Other");
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.view_panel_cursor = 1;
|
||||
let effects = SwitchViewAtCursor.execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("SwitchView(\"Default\")"),
|
||||
"must switch to the view UNDER THE CURSOR, got: {dbg}"
|
||||
dbg.contains("SwitchView(\"Other\")"),
|
||||
"must switch to the view UNDER THE CURSOR (Other, index 1), got: {dbg}"
|
||||
);
|
||||
assert!(dbg.contains("Normal"), "Expected Normal mode, got: {dbg}");
|
||||
}
|
||||
@@ -504,6 +501,27 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn delete_view_at_cursor_deletes_the_view_under_a_nonzero_cursor() {
|
||||
let mut m = two_cat_model();
|
||||
m.create_view("Other");
|
||||
m.create_view("Third");
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.view_panel_cursor = 1;
|
||||
let dbg = effects_debug(&DeleteViewAtCursor.execute(&ctx));
|
||||
assert!(
|
||||
dbg.contains("DeleteView(\"Other\")"),
|
||||
"must delete the view UNDER THE CURSOR (Other, index 1), got: {dbg}"
|
||||
);
|
||||
assert!(
|
||||
dbg.contains("SetPanelCursor { panel: View, cursor: 0 }"),
|
||||
"a delete off position 0 steps the cursor back, got: {dbg}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delete_view_at_cursor_zero_does_not_adjust_cursor() {
|
||||
let m = two_cat_model();
|
||||
@@ -559,16 +577,15 @@ impl Cmd for TogglePanelAndFocus {
|
||||
#[derive(Debug)]
|
||||
pub struct TogglePanelVisibility {
|
||||
pub panel: Panel,
|
||||
pub currently_open: bool,
|
||||
}
|
||||
impl Cmd for TogglePanelVisibility {
|
||||
fn name(&self) -> &'static str {
|
||||
"toggle-panel-visibility"
|
||||
}
|
||||
fn execute(&self, _ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
vec![Box::new(effect::SetPanelOpen {
|
||||
panel: self.panel,
|
||||
open: !self.currently_open,
|
||||
open: !ctx.panel_open(self.panel),
|
||||
})]
|
||||
}
|
||||
}
|
||||
@@ -576,20 +593,16 @@ impl Cmd for TogglePanelVisibility {
|
||||
/// Tab through open panels Formula → Category → View, wrapping, starting from the
|
||||
/// focused one. One open panel self-transitions; none open is a no-op.
|
||||
#[derive(Debug)]
|
||||
pub struct CyclePanelFocus {
|
||||
pub formula_open: bool,
|
||||
pub category_open: bool,
|
||||
pub view_open: bool,
|
||||
}
|
||||
pub struct CyclePanelFocus;
|
||||
impl Cmd for CyclePanelFocus {
|
||||
fn name(&self) -> &'static str {
|
||||
"cycle-panel-focus"
|
||||
}
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
let order = [
|
||||
(self.formula_open, AppMode::FormulaPanel),
|
||||
(self.category_open, AppMode::CategoryPanel),
|
||||
(self.view_open, AppMode::ViewPanel),
|
||||
(ctx.formula_panel_open, AppMode::FormulaPanel),
|
||||
(ctx.category_panel_open, AppMode::CategoryPanel),
|
||||
(ctx.view_panel_open, AppMode::ViewPanel),
|
||||
];
|
||||
// Index of the first candidate: the panel after the focused one,
|
||||
// or the first panel when invoked from a non-panel mode.
|
||||
@@ -614,16 +627,17 @@ impl Cmd for CyclePanelFocus {
|
||||
pub struct MovePanelCursor {
|
||||
pub panel: Panel,
|
||||
pub delta: i32,
|
||||
pub current: usize,
|
||||
pub max: usize,
|
||||
}
|
||||
impl Cmd for MovePanelCursor {
|
||||
fn name(&self) -> &'static str {
|
||||
"move-panel-cursor"
|
||||
}
|
||||
fn execute(&self, _ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
let cursor = self.current;
|
||||
let max = self.max;
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
let (cursor, max) = match self.panel {
|
||||
Panel::Formula => (ctx.formula_cursor, ctx.model.formulas().len()),
|
||||
Panel::Category => (ctx.cat_panel_cursor, ctx.cat_tree_len()),
|
||||
Panel::View => (ctx.view_panel_cursor, ctx.workbook.views.len()),
|
||||
};
|
||||
if max == 0 {
|
||||
return vec![];
|
||||
}
|
||||
|
||||
@@ -27,19 +27,21 @@ fn cursor_key(ctx: &CmdContext) -> Result<CellKey, String> {
|
||||
ctx.cell_key().clone().ok_or("no cell at cursor".into())
|
||||
}
|
||||
|
||||
/// A `Move` built from args takes the live cursor as its origin.
|
||||
fn move_from_cursor(cmd: Move, ctx: &CmdContext) -> Move {
|
||||
Move {
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
..cmd
|
||||
impl AtCursor for Move {
|
||||
fn cursor_mut(&mut self) -> &mut CursorState {
|
||||
&mut self.cursor
|
||||
}
|
||||
}
|
||||
|
||||
/// A commit built from args still advances from the live cursor.
|
||||
fn commit_from_cursor(cmd: CommitAndAdvance, ctx: &CmdContext) -> CommitAndAdvance {
|
||||
CommitAndAdvance {
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
..cmd
|
||||
impl AtCursor for EnterAdvance {
|
||||
fn cursor_mut(&mut self) -> &mut CursorState {
|
||||
&mut self.cursor
|
||||
}
|
||||
}
|
||||
|
||||
impl AtCursor for CommitAndAdvance {
|
||||
fn cursor_mut(&mut self) -> &mut CursorState {
|
||||
&mut self.cursor
|
||||
}
|
||||
}
|
||||
|
||||
@@ -62,11 +64,6 @@ fn commit_at_cursor(
|
||||
})
|
||||
}
|
||||
|
||||
/// For a command with no context-only fields: `parse`'s result is used as built.
|
||||
fn as_parsed<C>(cmd: C, _ctx: &CmdContext) -> C {
|
||||
cmd
|
||||
}
|
||||
|
||||
/// Build the default command registry with all commands.
|
||||
/// Registry names MUST match the `Cmd::name()` return value.
|
||||
pub fn default_registry() -> CmdRegistry {
|
||||
@@ -91,7 +88,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(ClearCellCommand {
|
||||
key: cursor_key(ctx)?,
|
||||
@@ -122,7 +118,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
r.register_pure(&HelpPagePrevCmd(vec![]), HelpPagePrevCmd::parse);
|
||||
|
||||
// ── Navigation (unified Move) ──────────────────────────────────────
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&Move {
|
||||
kind: MoveKind::Relative(0, 0),
|
||||
cursor: CursorState::default(),
|
||||
@@ -138,13 +134,12 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: "move-selection",
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
// Jump-to-edge commands: first/last row/col
|
||||
macro_rules! reg_jump {
|
||||
($r:expr, $is_row:expr, $to_end:expr, $name:expr) => {
|
||||
$r.register(
|
||||
$r.register_at_cursor(
|
||||
&Move {
|
||||
kind: if $to_end {
|
||||
MoveKind::ToEnd($is_row)
|
||||
@@ -165,7 +160,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: $name,
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
};
|
||||
@@ -174,7 +168,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
reg_jump!(r, true, true, "jump-last-row");
|
||||
reg_jump!(r, false, false, "jump-first-col");
|
||||
reg_jump!(r, false, true, "jump-last-col");
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&Move {
|
||||
kind: MoveKind::Relative(0, 0),
|
||||
cursor: CursorState::default(),
|
||||
@@ -189,10 +183,9 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: "scroll-rows",
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&Move {
|
||||
kind: MoveKind::Page(0),
|
||||
cursor: CursorState::default(),
|
||||
@@ -207,10 +200,9 @@ pub fn default_registry() -> CmdRegistry {
|
||||
cmd_name: "page-scroll",
|
||||
})
|
||||
},
|
||||
move_from_cursor,
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&EnterAdvance {
|
||||
cursor: CursorState::default(),
|
||||
},
|
||||
@@ -228,9 +220,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
},
|
||||
})
|
||||
},
|
||||
|_, ctx| EnterAdvance {
|
||||
cursor: CursorState::from_ctx(ctx),
|
||||
},
|
||||
None,
|
||||
);
|
||||
|
||||
@@ -247,7 +236,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(YankCell {
|
||||
key: cursor_key(ctx)?,
|
||||
@@ -266,7 +254,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(PasteCell {
|
||||
key: cursor_key(ctx)?,
|
||||
@@ -319,7 +306,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
})
|
||||
},
|
||||
as_parsed,
|
||||
Some(|_, ctx| {
|
||||
Ok(DrillIntoCell {
|
||||
key: cursor_key(ctx)?,
|
||||
@@ -357,77 +343,30 @@ pub fn default_registry() -> CmdRegistry {
|
||||
});
|
||||
Ok(TogglePanelAndFocus { panel, target })
|
||||
},
|
||||
as_parsed,
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
&TogglePanelVisibility {
|
||||
panel: Panel::Formula,
|
||||
currently_open: false,
|
||||
},
|
||||
|args| {
|
||||
require_args(args, 1)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
Ok(TogglePanelVisibility {
|
||||
panel,
|
||||
currently_open: false,
|
||||
})
|
||||
},
|
||||
|cmd, ctx| TogglePanelVisibility {
|
||||
currently_open: ctx.panel_open(cmd.panel),
|
||||
..cmd
|
||||
},
|
||||
None,
|
||||
);
|
||||
r.register(
|
||||
&CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: false,
|
||||
view_open: false,
|
||||
},
|
||||
|_| {
|
||||
Ok(CyclePanelFocus {
|
||||
formula_open: false,
|
||||
category_open: false,
|
||||
view_open: false,
|
||||
})
|
||||
},
|
||||
|_, ctx| CyclePanelFocus {
|
||||
formula_open: ctx.formula_panel_open,
|
||||
category_open: ctx.category_panel_open,
|
||||
view_open: ctx.view_panel_open,
|
||||
Ok(TogglePanelVisibility { panel })
|
||||
},
|
||||
None,
|
||||
);
|
||||
r.register_nullary(|| Box::new(CyclePanelFocus));
|
||||
r.register(
|
||||
&MovePanelCursor {
|
||||
panel: Panel::Formula,
|
||||
delta: 0,
|
||||
current: 0,
|
||||
max: 0,
|
||||
},
|
||||
|args| {
|
||||
require_args(args, 2)?;
|
||||
let panel = parse_panel(&args[0])?;
|
||||
let delta = args[1].parse::<i32>().map_err(|e| e.to_string())?;
|
||||
Ok(MovePanelCursor {
|
||||
panel,
|
||||
delta,
|
||||
current: 0,
|
||||
max: 0,
|
||||
})
|
||||
},
|
||||
|cmd, ctx| {
|
||||
let (current, max) = match cmd.panel {
|
||||
Panel::Formula => (ctx.formula_cursor, ctx.model.formulas().len()),
|
||||
Panel::Category => (ctx.cat_panel_cursor, ctx.cat_tree_len()),
|
||||
Panel::View => (ctx.view_panel_cursor, ctx.workbook.views.len()),
|
||||
};
|
||||
MovePanelCursor {
|
||||
current,
|
||||
max,
|
||||
..cmd
|
||||
}
|
||||
Ok(MovePanelCursor { panel, delta })
|
||||
},
|
||||
None,
|
||||
);
|
||||
@@ -494,7 +433,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
|
||||
// ── Commit ───────────────────────────────────────────────────────────
|
||||
// These take a mode-name arg as args[0]; the command never inspects ctx.mode.
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&CommitAndAdvance {
|
||||
key: CellKey::new(vec![]),
|
||||
value: String::new(),
|
||||
@@ -517,10 +456,9 @@ pub fn default_registry() -> CmdRegistry {
|
||||
opened: None,
|
||||
})
|
||||
},
|
||||
commit_from_cursor,
|
||||
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Down)),
|
||||
);
|
||||
r.register(
|
||||
r.register_at_cursor(
|
||||
&CommitAndAdvance {
|
||||
key: CellKey::new(vec![]),
|
||||
value: String::new(),
|
||||
@@ -530,7 +468,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
opened: None,
|
||||
},
|
||||
|_| Err("commit-and-advance-right requires context".into()),
|
||||
commit_from_cursor,
|
||||
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Right)),
|
||||
);
|
||||
r.register(
|
||||
@@ -542,7 +479,6 @@ pub fn default_registry() -> CmdRegistry {
|
||||
let kind = parse_commit_kind(&args[0], &AppMode::Normal)?;
|
||||
Ok(CommitBuffer { kind })
|
||||
},
|
||||
as_parsed,
|
||||
Some(|args, ctx| {
|
||||
require_args(args, 1)?;
|
||||
let kind = parse_commit_kind(&args[0], ctx.mode)?;
|
||||
@@ -867,12 +803,16 @@ mod panel_open_tests {
|
||||
let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
for asked in Panel::ALL {
|
||||
let args = [asked.name().to_string()];
|
||||
let cmd = ctx
|
||||
let effects = ctx
|
||||
.registry
|
||||
.interactive("toggle-panel-visibility", &args, &ctx)
|
||||
.expect("toggle-panel-visibility resolves");
|
||||
let dbg = format!("{cmd:?}");
|
||||
let expected = format!("currently_open: {}", asked == opened);
|
||||
.expect("toggle-panel-visibility resolves")
|
||||
.execute(&ctx);
|
||||
let dbg = format!("{effects:?}");
|
||||
let expected = format!(
|
||||
"SetPanelOpen {{ panel: {asked:?}, open: {} }}",
|
||||
asked != opened
|
||||
);
|
||||
assert!(
|
||||
dbg.contains(&expected),
|
||||
"with only {opened:?} open, toggling {asked:?} read another \
|
||||
|
||||
@@ -2991,6 +2991,43 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-cz8d: a refusal need not abort, so a failed document swap must
|
||||
/// ask for the abort itself. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_failed_load_or_import_does_not_run_the_quit_after_it() {
|
||||
use improvise_protocol::keymap::Keymap;
|
||||
|
||||
for (command, path) in [("load", "absent.improv"), ("import-json", "absent.json")] {
|
||||
let mut app = App::new(Workbook::new("T"), None);
|
||||
app.storage = Box::new(crate::storage::MemoryFs::new());
|
||||
let key = KeyCode::Char('x');
|
||||
let mods = KeyModifiers::NONE;
|
||||
let mut km = Keymap::new();
|
||||
km.bind_seq(
|
||||
key,
|
||||
mods,
|
||||
vec![(command, vec![path.to_string()]), ("force-quit", vec![])],
|
||||
);
|
||||
let effects = {
|
||||
let ctx = app.cmd_context(key, mods);
|
||||
app.keymap_set.dispatch_transient(&km, &ctx, key, mods)
|
||||
}
|
||||
.expect("both steps resolve in the registry");
|
||||
app.apply_effects(effects);
|
||||
|
||||
assert!(
|
||||
app.last_batch_refused(),
|
||||
"setup: `{command}` must actually have failed, got {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
assert_ne!(
|
||||
app.view_state.mode,
|
||||
AppMode::Quit,
|
||||
"`{command} {path} . q` quit having loaded nothing"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A prefix is consumed whether or not the next key matches, so a stray key
|
||||
/// after `g` does nothing rather than acting as a fresh Normal binding.
|
||||
#[test]
|
||||
|
||||
@@ -44,6 +44,31 @@ pub enum ViewTarget {
|
||||
Named(String),
|
||||
}
|
||||
|
||||
/// Refuse AND stop the batch, for a failed document swap: what follows it in
|
||||
/// a `:` line assumes it happened, so `load x . q` must not quit (cz8d).
|
||||
fn refuse_and_stop(app: &mut App, why: impl Into<String>) {
|
||||
app.refuse(why);
|
||||
app.abort_effects = true;
|
||||
}
|
||||
|
||||
/// Write the view a geometry effect names, or refuse: a view the document no
|
||||
/// longer has is a failure to report, not a no-op to swallow (improvise-bok4).
|
||||
fn on_view(app: &mut App, target: &ViewTarget, write: impl FnOnce(&mut crate::view::View)) {
|
||||
let missing = match app.view_mut_of(target) {
|
||||
Some(view) => {
|
||||
write(view);
|
||||
None
|
||||
}
|
||||
None => Some(match target {
|
||||
ViewTarget::Named(name) => format!("No view named \"{name}\""),
|
||||
ViewTarget::Active => "No active view".to_string(),
|
||||
}),
|
||||
};
|
||||
if let Some(why) = missing {
|
||||
app.refuse(why);
|
||||
}
|
||||
}
|
||||
|
||||
/// A discrete state change produced by a command.
|
||||
/// Effects know how to apply themselves to the App.
|
||||
pub trait Effect: Debug {
|
||||
@@ -183,11 +208,13 @@ impl Effect for AddFormula {
|
||||
// grid; _Measure targets arrive via `measure_item_names()`.
|
||||
if !crate::model::category::CategoryKind::of_name(&formula.target_category)
|
||||
.is_measure()
|
||||
{
|
||||
let _ = app
|
||||
&& let Err(e) = app
|
||||
.model_state
|
||||
.workbook
|
||||
.add_item(&formula.target_category, &formula.target);
|
||||
.add_item(&formula.target_category, &formula.target)
|
||||
{
|
||||
// Its category may be gone: store no formula for it (40j4).
|
||||
return app.refuse(e.to_string());
|
||||
}
|
||||
app.model_state.workbook.model.add_formula(formula);
|
||||
}
|
||||
@@ -255,9 +282,9 @@ impl Effect for EnterEditAtCursor {
|
||||
pub struct TogglePruneEmpty(pub ViewTarget);
|
||||
impl Effect for TogglePruneEmpty {
|
||||
fn apply(&self, app: &mut App) {
|
||||
if let Some(v) = app.view_mut_of(&self.0) {
|
||||
on_view(app, &self.0, |v| {
|
||||
v.prune_empty = !v.prune_empty;
|
||||
}
|
||||
});
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
EffectKind::Model(ModelChange::LayoutGeometry)
|
||||
@@ -321,7 +348,14 @@ impl Effect for CreateView {
|
||||
pub struct DeleteView(pub String);
|
||||
impl Effect for DeleteView {
|
||||
fn apply(&self, app: &mut App) {
|
||||
let _ = app.model_state.workbook.delete_view(&self.0);
|
||||
// `delete_view` answers Ok for a name it lacks, which is a failure to
|
||||
// report, not a deletion (improvise-40j4).
|
||||
if !app.model_state.workbook.views.contains_key(&self.0) {
|
||||
return app.refuse(format!("No view named \"{}\"", self.0));
|
||||
}
|
||||
if let Err(e) = app.model_state.workbook.delete_view(&self.0) {
|
||||
return app.refuse(e.to_string());
|
||||
}
|
||||
// This client may have been the one on it. Idempotent for every other
|
||||
// view; the mirrors owe the same move (`Sessions::adopt_deleted_views`).
|
||||
app.view_state.active_view = app.view_name().to_string();
|
||||
@@ -405,9 +439,9 @@ pub struct SetAxis {
|
||||
}
|
||||
impl Effect for SetAxis {
|
||||
fn apply(&self, app: &mut App) {
|
||||
if let Some(v) = app.view_mut_of(&self.view) {
|
||||
on_view(app, &self.view, |v| {
|
||||
v.set_axis(&self.category, self.axis);
|
||||
}
|
||||
});
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
EffectKind::Model(ModelChange::LayoutGeometry)
|
||||
@@ -422,9 +456,9 @@ pub struct SetPageSelection {
|
||||
}
|
||||
impl Effect for SetPageSelection {
|
||||
fn apply(&self, app: &mut App) {
|
||||
if let Some(v) = app.view_mut_of(&self.view) {
|
||||
on_view(app, &self.view, |v| {
|
||||
v.set_page_selection(&self.category, &self.item);
|
||||
}
|
||||
});
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
EffectKind::Model(ModelChange::LayoutGeometry)
|
||||
@@ -439,9 +473,9 @@ pub struct ToggleGroup {
|
||||
}
|
||||
impl Effect for ToggleGroup {
|
||||
fn apply(&self, app: &mut App) {
|
||||
if let Some(v) = app.view_mut_of(&self.view) {
|
||||
on_view(app, &self.view, |v| {
|
||||
v.toggle_group_collapse(&self.category, &self.group);
|
||||
}
|
||||
});
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
EffectKind::Model(ModelChange::LayoutGeometry)
|
||||
@@ -456,9 +490,9 @@ pub struct HideItem {
|
||||
}
|
||||
impl Effect for HideItem {
|
||||
fn apply(&self, app: &mut App) {
|
||||
if let Some(v) = app.view_mut_of(&self.view) {
|
||||
on_view(app, &self.view, |v| {
|
||||
v.hide_item(&self.category, &self.item);
|
||||
}
|
||||
});
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
EffectKind::Model(ModelChange::LayoutGeometry)
|
||||
@@ -473,9 +507,9 @@ pub struct ShowItem {
|
||||
}
|
||||
impl Effect for ShowItem {
|
||||
fn apply(&self, app: &mut App) {
|
||||
if let Some(v) = app.view_mut_of(&self.view) {
|
||||
on_view(app, &self.view, |v| {
|
||||
v.show_item(&self.category, &self.item);
|
||||
}
|
||||
});
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
EffectKind::Model(ModelChange::LayoutGeometry)
|
||||
@@ -486,9 +520,9 @@ impl Effect for ShowItem {
|
||||
pub struct TransposeAxes(pub ViewTarget);
|
||||
impl Effect for TransposeAxes {
|
||||
fn apply(&self, app: &mut App) {
|
||||
if let Some(v) = app.view_mut_of(&self.0) {
|
||||
on_view(app, &self.0, |v| {
|
||||
v.transpose_axes();
|
||||
}
|
||||
});
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
EffectKind::Model(ModelChange::LayoutGeometry)
|
||||
@@ -500,9 +534,9 @@ pub struct CycleAxis(pub ViewTarget, pub String);
|
||||
impl Effect for CycleAxis {
|
||||
fn apply(&self, app: &mut App) {
|
||||
let category = self.1.clone();
|
||||
if let Some(v) = app.view_mut_of(&self.0) {
|
||||
on_view(app, &self.0, |v| {
|
||||
v.cycle_axis(&category);
|
||||
}
|
||||
});
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
EffectKind::Model(ModelChange::LayoutGeometry)
|
||||
@@ -514,9 +548,9 @@ pub struct SetNumberFormat(pub ViewTarget, pub String);
|
||||
impl Effect for SetNumberFormat {
|
||||
fn apply(&self, app: &mut App) {
|
||||
let format = self.1.clone();
|
||||
if let Some(v) = app.view_mut_of(&self.0) {
|
||||
on_view(app, &self.0, |v| {
|
||||
v.number_format = format;
|
||||
}
|
||||
});
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
EffectKind::Model(ModelChange::LayoutGeometry)
|
||||
@@ -939,9 +973,7 @@ impl Effect for LoadModel {
|
||||
app.install_workbook(loaded);
|
||||
app.view_state.status_msg = format!("Loaded from {}", self.0.display());
|
||||
}
|
||||
Err(e) => {
|
||||
app.refuse(format!("Load error: {e}"));
|
||||
}
|
||||
Err(e) => refuse_and_stop(app, format!("Load error: {e}")),
|
||||
}
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
@@ -998,7 +1030,7 @@ impl Effect for ImportJsonHeadless {
|
||||
let bytes = match app.storage.read(&self.path) {
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
app.refuse(format!("Cannot read '{}': {e}", self.path.display()));
|
||||
refuse_and_stop(app, format!("Cannot read '{}': {e}", self.path.display()));
|
||||
return;
|
||||
}
|
||||
};
|
||||
@@ -1009,7 +1041,7 @@ impl Effect for ImportJsonHeadless {
|
||||
(recs, raw)
|
||||
}
|
||||
Err(e) => {
|
||||
app.refuse(format!("CSV error: {e}"));
|
||||
refuse_and_stop(app, format!("CSV error: {e}"));
|
||||
return;
|
||||
}
|
||||
}
|
||||
@@ -1017,21 +1049,21 @@ impl Effect for ImportJsonHeadless {
|
||||
let content = match app.storage.read(&self.path) {
|
||||
Ok(c) => c,
|
||||
Err(e) => {
|
||||
app.refuse(format!("Cannot read '{}': {e}", self.path.display()));
|
||||
refuse_and_stop(app, format!("Cannot read '{}': {e}", self.path.display()));
|
||||
return;
|
||||
}
|
||||
};
|
||||
let value: serde_json::Value = match serde_json::from_slice(&content) {
|
||||
Ok(v) => v,
|
||||
Err(e) => {
|
||||
app.refuse(format!("JSON parse error: {e}"));
|
||||
refuse_and_stop(app, format!("JSON parse error: {e}"));
|
||||
return;
|
||||
}
|
||||
};
|
||||
match json_import_records(&value, self.array_path.as_deref()) {
|
||||
Ok(recs) => (recs, value),
|
||||
Err(msg) => {
|
||||
app.refuse(msg);
|
||||
refuse_and_stop(app, msg);
|
||||
return;
|
||||
}
|
||||
}
|
||||
@@ -1070,7 +1102,7 @@ impl Effect for ImportJsonHeadless {
|
||||
};
|
||||
}
|
||||
Err(e) => {
|
||||
app.refuse(format!("Import error: {e}"));
|
||||
refuse_and_stop(app, format!("Import error: {e}"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -166,11 +166,12 @@ impl Host {
|
||||
if let Some(reason) = reduce::broken_chain(&self.app, cmd.command()) {
|
||||
return self.undo_frame(id, undo, mirrors, reason);
|
||||
}
|
||||
// Only a mutation runs `apply_effects`, and only `apply_effects`
|
||||
// resets the flag — so after anything else it is a STALE answer.
|
||||
// Only a mutation runs `apply_effects`, which resets both flags — so
|
||||
// after anything else they are STALE. A refusal need not abort (cz8d).
|
||||
let mutation = matches!(cmd.command(), Command::Model { .. });
|
||||
out.extend(self.command(id, cmd));
|
||||
if mutation && self.app.last_batch_aborted() {
|
||||
let failed = self.app.last_batch_refused() || self.app.last_batch_aborted();
|
||||
if mutation && failed {
|
||||
return self.undo_frame(id, undo, mirrors, Rejection::MutationRefused);
|
||||
}
|
||||
}
|
||||
@@ -1541,10 +1542,12 @@ mod tests {
|
||||
.any(|f| matches!(f.msg, ServerFrame::Snapshot(_))),
|
||||
"a failed load cost every client its cursors: {frames:?}"
|
||||
);
|
||||
assert!(
|
||||
host.app().view_state.status_msg.starts_with("Load error"),
|
||||
"the fixture did not actually fail the load: {:?}",
|
||||
host.app().view_state.status_msg
|
||||
// Since improvise-bok4 a failed load is REFUSED, so the refusal is the
|
||||
// witness that it failed; the host's status line is rolled back with it.
|
||||
assert_eq!(
|
||||
to(&frames, asker),
|
||||
vec![ServerFrame::Rejected(Rejection::MutationRefused)],
|
||||
"the fixture did not actually fail the load"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1778,4 +1781,283 @@ mod tests {
|
||||
let json = serde_json::to_string(&inbound).unwrap();
|
||||
assert_eq!(serde_json::from_str::<ClientFrame>(&json).unwrap(), inbound);
|
||||
}
|
||||
|
||||
/// Two model commands, each caught up against the document as the client
|
||||
/// saw it, so no basis tier can refuse the frame first (improvise-bok4).
|
||||
fn two_command_frame(host: &Host, first: ModelCommand, then: ModelCommand) -> ClientFrame {
|
||||
ClientFrame::Commands(vec![
|
||||
crate::ui::reduce::caught_up(host.app(), first),
|
||||
crate::ui::reduce::caught_up(host.app(), then),
|
||||
])
|
||||
}
|
||||
|
||||
/// improvise-bok4, the view-addressed case: the second command's view is
|
||||
/// gone. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_frame_writing_a_view_its_own_first_command_deleted_is_refused() {
|
||||
let mut host = host();
|
||||
host.app_mut().model_state.workbook.create_view("Scratch");
|
||||
let (asker, _) = host.connect(10, 10);
|
||||
|
||||
let answer = host.handle(
|
||||
asker,
|
||||
two_command_frame(
|
||||
&host,
|
||||
ModelCommand::DeleteView("Scratch".into()),
|
||||
ModelCommand::SetNumberFormat {
|
||||
view: "Scratch".into(),
|
||||
format: ",.2f".into(),
|
||||
},
|
||||
),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
to(&answer, asker),
|
||||
vec![ServerFrame::Rejected(Rejection::MutationRefused)],
|
||||
"half the frame did nothing and was reported accepted: {answer:?}"
|
||||
);
|
||||
assert!(
|
||||
host.app()
|
||||
.model_state
|
||||
.workbook
|
||||
.views
|
||||
.contains_key("Scratch"),
|
||||
"the refused frame's deletion must be undone"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-bok4, the category-addressed case: it narrated its failure into
|
||||
/// the HOST's status line and told the client nothing.
|
||||
#[test]
|
||||
fn a_frame_adding_an_item_to_a_category_its_first_command_removed_is_refused() {
|
||||
let mut host = host();
|
||||
let (asker, _) = host.connect(10, 10);
|
||||
|
||||
let answer = host.handle(
|
||||
asker,
|
||||
two_command_frame(
|
||||
&host,
|
||||
ModelCommand::RemoveCategory("Region".into()),
|
||||
ModelCommand::AddItem {
|
||||
category: "Region".into(),
|
||||
item: "East".into(),
|
||||
},
|
||||
),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
to(&answer, asker),
|
||||
vec![ServerFrame::Rejected(Rejection::MutationRefused)],
|
||||
"half the frame did nothing and was reported accepted: {answer:?}"
|
||||
);
|
||||
assert!(
|
||||
host.app()
|
||||
.model_state
|
||||
.workbook
|
||||
.model
|
||||
.category("Region")
|
||||
.is_some(),
|
||||
"the refused frame's removal must be undone"
|
||||
);
|
||||
}
|
||||
|
||||
/// The one-command case of the same class: deleting the last view was
|
||||
/// swallowed with `let _`, so the frame was accepted having done nothing.
|
||||
#[test]
|
||||
fn deleting_the_documents_last_view_is_refused_rather_than_accepted() {
|
||||
let mut host = host();
|
||||
let (asker, _) = host.connect(10, 10);
|
||||
let only = host.app().view_name().to_string();
|
||||
|
||||
let answer = host.model(asker, ModelCommand::DeleteView(only.clone()));
|
||||
|
||||
assert_eq!(
|
||||
to(&answer, asker),
|
||||
vec![ServerFrame::Rejected(Rejection::MutationRefused)],
|
||||
"a deletion the workbook refused was reported accepted: {answer:?}"
|
||||
);
|
||||
assert!(host.app().model_state.workbook.views.contains_key(&only));
|
||||
}
|
||||
|
||||
/// The control: two commands that both land are accepted, so the refusals
|
||||
/// above are about a failure and not about the frame's shape.
|
||||
#[test]
|
||||
fn a_frame_whose_commands_all_land_is_accepted() {
|
||||
let mut host = host();
|
||||
host.app_mut().model_state.workbook.create_view("Scratch");
|
||||
let (asker, _) = host.connect(10, 10);
|
||||
|
||||
let answer = host.handle(
|
||||
asker,
|
||||
two_command_frame(
|
||||
&host,
|
||||
ModelCommand::SetNumberFormat {
|
||||
view: "Scratch".into(),
|
||||
format: ",.2f".into(),
|
||||
},
|
||||
ModelCommand::AddItem {
|
||||
category: "Region".into(),
|
||||
item: "East".into(),
|
||||
},
|
||||
),
|
||||
);
|
||||
|
||||
assert!(
|
||||
to(&answer, asker).contains(&ServerFrame::Accepted),
|
||||
"a frame with nothing to refuse was refused: {answer:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-40j4: the formula was stored for a category the frame's own
|
||||
/// first command removed, and it survived save and reload.
|
||||
#[test]
|
||||
fn a_frame_adding_a_formula_to_a_category_its_first_command_removed_is_refused() {
|
||||
let mut host = host();
|
||||
let (asker, _) = host.connect(10, 10);
|
||||
|
||||
let answer = host.handle(
|
||||
asker,
|
||||
two_command_frame(
|
||||
&host,
|
||||
ModelCommand::RemoveCategory("Region".into()),
|
||||
ModelCommand::AddFormula {
|
||||
raw: "Total = 1".into(),
|
||||
target_category: "Region".into(),
|
||||
},
|
||||
),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
to(&answer, asker),
|
||||
vec![ServerFrame::Rejected(Rejection::MutationRefused)],
|
||||
"a formula for a vanished category was accepted: {answer:?}"
|
||||
);
|
||||
let model = &host.app().model_state.workbook.model;
|
||||
assert!(
|
||||
model.category("Region").is_some(),
|
||||
"the removal must be undone"
|
||||
);
|
||||
assert!(
|
||||
model.formulas().iter().all(|f| f.target != "Total"),
|
||||
"and the formula must not be stored"
|
||||
);
|
||||
}
|
||||
|
||||
/// A host whose storage holds one file per way `ImportJsonHeadless` can
|
||||
/// fail, plus nothing at the two paths that must fail to read.
|
||||
fn host_with_import_fixtures() -> Host {
|
||||
let storage = MemoryFs::new();
|
||||
let files: [(&str, &[u8]); 4] = [
|
||||
// Every field numeric, so no category is accepted and the build fails.
|
||||
("measures.json", br#"[{"x": 1}, {"x": 2}]"#),
|
||||
("bad.json", b"{not json"),
|
||||
("nested.json", br#"{"data": [{"Region": "North"}]}"#),
|
||||
// A record that is not UTF-8 is a CSV read error, not a warning.
|
||||
("bad.csv", b"Region,Revenue\n\xff\xfe,1\n"),
|
||||
];
|
||||
for (name, bytes) in files {
|
||||
crate::storage::Storage::write(&storage, Path::new(name), bytes)
|
||||
.expect("fixture writes");
|
||||
}
|
||||
// A second view, so deleting an absent one is not refused as "the last".
|
||||
let mut wb = workbook();
|
||||
wb.create_view("Scratch");
|
||||
let mut app = App::new(wb, None);
|
||||
app.storage = Box::new(storage);
|
||||
app.rebuild_layout();
|
||||
Host::new(app)
|
||||
}
|
||||
|
||||
fn import(path: &str, array_path: Option<&str>) -> ModelCommand {
|
||||
ModelCommand::ImportJsonHeadless {
|
||||
path: PathBuf::from(path),
|
||||
model_name: None,
|
||||
array_path: array_path.map(String::from),
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-9fpl: one case per refusal #181 converted that no other test
|
||||
/// witnesses. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn every_failing_model_command_refuses_its_whole_frame() {
|
||||
let cases: Vec<(&str, ModelCommand)> = vec![
|
||||
(
|
||||
"AddItemInGroup into an absent category",
|
||||
ModelCommand::AddItemInGroup {
|
||||
category: "Ghost".into(),
|
||||
item: "East".into(),
|
||||
group: "Coastal".into(),
|
||||
},
|
||||
),
|
||||
(
|
||||
"AddFormula that does not parse",
|
||||
ModelCommand::AddFormula {
|
||||
raw: "Total = (".into(),
|
||||
target_category: "_Measure".into(),
|
||||
},
|
||||
),
|
||||
(
|
||||
"ImportJsonHeadless of an absent JSON",
|
||||
import("absent.json", None),
|
||||
),
|
||||
(
|
||||
"ImportJsonHeadless of an absent CSV",
|
||||
import("absent.csv", None),
|
||||
),
|
||||
(
|
||||
"ImportJsonHeadless of a CSV that is not text",
|
||||
import("bad.csv", None),
|
||||
),
|
||||
(
|
||||
"ImportJsonHeadless of JSON that does not parse",
|
||||
import("bad.json", None),
|
||||
),
|
||||
(
|
||||
"ImportJsonHeadless at a path with no array",
|
||||
import("nested.json", Some("nope")),
|
||||
),
|
||||
(
|
||||
"ImportJsonHeadless that builds no model",
|
||||
import("measures.json", None),
|
||||
),
|
||||
// improvise-40j4: `delete_view` answers Ok for a name it lacks.
|
||||
(
|
||||
"DeleteView of a view the document lacks",
|
||||
ModelCommand::DeleteView("Ghost".into()),
|
||||
),
|
||||
(
|
||||
"AddFormula into a category the document lacks",
|
||||
ModelCommand::AddFormula {
|
||||
raw: "Total = 1".into(),
|
||||
target_category: "Ghost".into(),
|
||||
},
|
||||
),
|
||||
];
|
||||
for (what, failing) in cases {
|
||||
let mut host = host_with_import_fixtures();
|
||||
let (asker, _) = host.connect(10, 10);
|
||||
let landed = ModelCommand::AddItem {
|
||||
category: "Region".into(),
|
||||
item: "Landed".into(),
|
||||
};
|
||||
|
||||
let answer = host.handle(asker, two_command_frame(&host, landed, failing));
|
||||
|
||||
assert_eq!(
|
||||
to(&answer, asker),
|
||||
vec![ServerFrame::Rejected(Rejection::MutationRefused)],
|
||||
"{what}: the frame was not refused: {answer:?}"
|
||||
);
|
||||
assert!(
|
||||
!host
|
||||
.app()
|
||||
.model_state
|
||||
.workbook
|
||||
.model
|
||||
.category("Region")
|
||||
.is_some_and(|c| c.ordered_item_names().contains(&"Landed")),
|
||||
"{what}: the half of the frame that landed was not undone"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -222,7 +222,6 @@ fn axis_frame(
|
||||
pending.clear();
|
||||
}
|
||||
pending.push(FrameEntry::Group(FrameGroup {
|
||||
cat: cat_name.clone(),
|
||||
name: group_name.clone(),
|
||||
collapsed: view.is_group_collapsed(cat_name, group_name),
|
||||
}));
|
||||
@@ -324,7 +323,7 @@ fn cell_id(app: &App, layout: &GridLayout, row: usize, col: usize) -> Option<Cel
|
||||
RecordCol::NewCategory => return Some(CellId::Create(CreateKind::Category)),
|
||||
// Renaming a coordinate moves every measure cell in the row, which is a
|
||||
// different intent from `SetCell` and still the host's (gotcha 1).
|
||||
RecordCol::Category(_) => return None,
|
||||
RecordCol::Category(_) => return Some(CellId::Coordinate),
|
||||
};
|
||||
return Some(CellId::Record(RecordCell {
|
||||
record: RecordHash::of(record),
|
||||
@@ -493,7 +492,10 @@ pub fn published_basis(app: &App, grid: &GridCache) -> Basis {
|
||||
.flatten()
|
||||
.filter_map(|cell| match cell.id.as_ref()? {
|
||||
CellId::Record(rc) => Some((rc.record, app.record_version_of(rc.record))),
|
||||
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => None,
|
||||
CellId::Aggregate(_)
|
||||
| CellId::Derived { .. }
|
||||
| CellId::Create(_)
|
||||
| CellId::Coordinate => None,
|
||||
})
|
||||
.collect();
|
||||
basis::published(app.document_generation(), app.layout_version(), records)
|
||||
@@ -4853,20 +4855,6 @@ mod cell_identity {
|
||||
);
|
||||
}
|
||||
|
||||
/// A coordinate column is not a cell to write into (gotcha 1), so it is projected
|
||||
/// with no id and a client cannot compose a write against it.
|
||||
#[test]
|
||||
fn a_coordinate_column_is_projected_with_no_identity() {
|
||||
let app = records_app();
|
||||
let region = column_named(&app, "Region");
|
||||
assert_eq!(
|
||||
grid_of(&app, WHOLE)
|
||||
.cell(0, region)
|
||||
.and_then(|c| c.id.clone()),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
/// A document where `Profit` is BOTH an explicit `_Measure` item and a
|
||||
/// formula target — which the `+` column lets a user build.
|
||||
fn shadowed_measure_records_app() -> App {
|
||||
@@ -5138,10 +5126,9 @@ mod cell_identity {
|
||||
let ids: Vec<Option<CellId>> = (0..app.layout.col_count())
|
||||
.map(|c| grid.cell(0, c).and_then(|cell| cell.id.clone()))
|
||||
.collect();
|
||||
assert_eq!(
|
||||
ids.iter().filter(|id| id.is_none()).count(),
|
||||
1,
|
||||
"only the one Region coordinate column may be nameless: {ids:?}"
|
||||
assert!(
|
||||
ids.iter().all(Option::is_some),
|
||||
"no records column is nameless since improvise-exdg: {ids:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
ids.iter()
|
||||
@@ -5162,6 +5149,30 @@ mod cell_identity {
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-exdg: a coordinate cell is drawn and holds text, so it is not
|
||||
/// "no cell". `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_coordinate_cell_is_named_rather_than_left_as_no_cell() {
|
||||
let app = records_app();
|
||||
let grid = grid_of(&app, WHOLE);
|
||||
let region = column_named(&app, "Region");
|
||||
let north = grid.cell(1, region).expect("the second row's coordinate");
|
||||
assert_eq!(north.text, "North", "setup: a visible coordinate cell");
|
||||
assert_eq!(
|
||||
north.id,
|
||||
Some(CellId::Coordinate),
|
||||
"a drawn coordinate is named"
|
||||
);
|
||||
let nameless: Vec<(usize, usize)> = (0..app.layout.row_count())
|
||||
.flat_map(|r| (0..app.layout.col_count()).map(move |c| (r, c)))
|
||||
.filter(|&(r, c)| grid.cell(r, c).is_some_and(|cell| cell.id.is_none()))
|
||||
.collect();
|
||||
assert!(
|
||||
nameless.is_empty(),
|
||||
"every records cell is something: {nameless:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The version counter is keyed by the surrogate the client holds, so one
|
||||
/// identity answers ADR 002's question and ADR 001's.
|
||||
#[test]
|
||||
|
||||
@@ -55,6 +55,9 @@ pub enum CellId {
|
||||
/// A records-mode creating column: typing a **name** into it adds that
|
||||
/// measure or category and writes no cell at all (gotcha 1, improvise-poia).
|
||||
Create(CreateKind),
|
||||
/// A records-mode coordinate. Renaming it moves every measure cell in the
|
||||
/// row (gotcha 1), which no client composes: `protocol.md` §3 (improvise-exdg).
|
||||
Coordinate,
|
||||
}
|
||||
|
||||
/// What a creating column creates. The name typed into it is the whole
|
||||
@@ -194,9 +197,6 @@ pub const MAX_ROW_HEADER_W: u16 = 24;
|
||||
/// because it is a fact about the *view*, which only the server holds.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct FrameGroup {
|
||||
/// The category the group belongs to, echoed back in `ToggleGroup` so a
|
||||
/// client never works out which category a label came from.
|
||||
pub cat: String,
|
||||
pub name: String,
|
||||
pub collapsed: bool,
|
||||
}
|
||||
@@ -1137,7 +1137,6 @@ mod tests {
|
||||
FrameEntry::Item(vec!["A".into(), "x".into()]),
|
||||
FrameEntry::Item(vec!["A".into(), "y".into()]),
|
||||
FrameEntry::Group(FrameGroup {
|
||||
cat: "Type".into(),
|
||||
name: "Late".into(),
|
||||
collapsed: true,
|
||||
}),
|
||||
@@ -1150,7 +1149,6 @@ mod tests {
|
||||
/// ungrouped between them — improvise-h0iv's reachable shape.
|
||||
fn repeated_group_frame() -> AxisFrame {
|
||||
let q1 = || FrameGroup {
|
||||
cat: "Month".into(),
|
||||
name: "Q1".into(),
|
||||
collapsed: false,
|
||||
};
|
||||
@@ -1176,7 +1174,6 @@ mod tests {
|
||||
cats: vec!["Month".into()],
|
||||
entries: vec![
|
||||
FrameEntry::Group(FrameGroup {
|
||||
cat: "Month".into(),
|
||||
name: "Q1".into(),
|
||||
collapsed: false,
|
||||
}),
|
||||
@@ -1214,7 +1211,6 @@ mod tests {
|
||||
cats: vec!["Month".into()],
|
||||
entries: vec![
|
||||
FrameEntry::Group(FrameGroup {
|
||||
cat: "Month".into(),
|
||||
name: "Q1".into(),
|
||||
collapsed: true,
|
||||
}),
|
||||
@@ -1266,7 +1262,7 @@ mod tests {
|
||||
/// The pair `FrameGroup` carries cannot separate them — h0iv's own proposed
|
||||
/// fix. See `context/test-evidence/protocol.md`.
|
||||
#[test]
|
||||
fn the_two_headers_are_equal_as_values_so_comparing_cat_and_name_cannot_split_them() {
|
||||
fn the_two_headers_are_equal_as_values_so_comparing_them_cannot_split_them() {
|
||||
let frame = repeated_group_frame();
|
||||
let headers: Vec<&FrameGroup> = frame
|
||||
.entries
|
||||
@@ -1277,10 +1273,9 @@ mod tests {
|
||||
})
|
||||
.collect();
|
||||
assert_eq!(
|
||||
(&headers[0].cat, &headers[0].name),
|
||||
(&headers[1].cat, &headers[1].name),
|
||||
"both headers name the same category AND the same group, so the \
|
||||
ticket's (cat, name) comparison would still merge them"
|
||||
headers[0], headers[1],
|
||||
"both headers are the same group in the same state, so no \
|
||||
comparison of their fields can split them"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1344,7 +1339,7 @@ mod tests {
|
||||
assert_eq!(frame.item(2), Some(&vec!["B".to_string(), "z".to_string()]));
|
||||
assert!(frame.group_of(0).is_none(), "[A,x] is in no group");
|
||||
let group = frame.group_of(2).expect("[B,z] sits under Late");
|
||||
assert_eq!((group.cat.as_str(), group.name.as_str()), ("Type", "Late"));
|
||||
assert_eq!(group.name, "Late");
|
||||
assert!(group.collapsed, "the collapse state travels with the group");
|
||||
}
|
||||
|
||||
|
||||
@@ -755,6 +755,7 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
|
||||
// Both creating columns are records-only, so the guard above takes
|
||||
// them; declining rather than `None` keeps that a door, not a hole.
|
||||
CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]),
|
||||
CellId::Coordinate => Some(vec![Command::SetStatus(not_yet("renaming a coordinate"))]),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -966,7 +967,9 @@ fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option
|
||||
fn record_at_cursor(state: &ClientState) -> Option<crate::RecordHash> {
|
||||
match cell_at_cursor(state)?.id.as_ref()? {
|
||||
CellId::Record(cell) => Some(cell.record),
|
||||
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => None,
|
||||
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) | CellId::Coordinate => {
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1046,7 +1049,9 @@ fn model(state: &ClientState, cmd: ModelCommand) -> Option<Command> {
|
||||
fn record_cell_at_cursor(state: &ClientState) -> Option<RecordCell> {
|
||||
match cell_at_cursor(state)?.id.as_ref()? {
|
||||
CellId::Record(cell) => Some(cell.clone()),
|
||||
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => None,
|
||||
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) | CellId::Coordinate => {
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -11,28 +11,28 @@ pub fn production_code(roots: &[PathBuf]) -> Vec<(PathBuf, String)> {
|
||||
for root in roots {
|
||||
collect_rust(root, &mut files);
|
||||
}
|
||||
let code: Vec<(PathBuf, String)> = files
|
||||
let read: Vec<(PathBuf, Source)> = files
|
||||
.into_iter()
|
||||
.map(|file| {
|
||||
let text = std::fs::read_to_string(&file).expect("a listed source reads");
|
||||
let code = code_only(&text);
|
||||
(file, code)
|
||||
let source = Source::of(&text, &file);
|
||||
(file, source)
|
||||
})
|
||||
.collect();
|
||||
let test_modules: BTreeSet<PathBuf> = code
|
||||
let test_modules: BTreeSet<PathBuf> = read
|
||||
.iter()
|
||||
.flat_map(|(file, code)| test_module_paths(file, code))
|
||||
.flat_map(|(file, source)| test_module_paths(file, source))
|
||||
.collect();
|
||||
code.into_iter()
|
||||
read.into_iter()
|
||||
.filter(|(file, _)| !test_modules.iter().any(|m| is_module_file(m, file)))
|
||||
.map(|(file, code)| (file, strip_test_items(&code)))
|
||||
.map(|(file, source)| (file, source.production()))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// One file's text as [`production_code`] cleans it. Which *files* are test
|
||||
/// modules needs their declaring file too, so only `production_code` knows.
|
||||
pub fn production_text(text: &str) -> String {
|
||||
strip_test_items(&code_only(text))
|
||||
Source::of(text, Path::new("<text>")).production()
|
||||
}
|
||||
|
||||
fn collect_rust(dir: &Path, into: &mut Vec<PathBuf>) {
|
||||
@@ -55,83 +55,319 @@ fn is_module_file(m: &Path, file: &Path) -> bool {
|
||||
file == m.with_extension("rs") || file.starts_with(m)
|
||||
}
|
||||
|
||||
/// One file, read once. Its `cfg` predicates are read off the TEXT, whose
|
||||
/// `feature` values are literals; every bracket off the code (improvise-59q8).
|
||||
struct Source {
|
||||
code: Vec<char>,
|
||||
/// Test-only items, as `(attribute start, item start, item end)`.
|
||||
items: Vec<(usize, usize, usize)>,
|
||||
/// A leading `#![cfg(test)]`: the whole file is test code.
|
||||
whole_file: bool,
|
||||
}
|
||||
|
||||
impl Source {
|
||||
fn of(text: &str, file: &Path) -> Self {
|
||||
let code: Vec<char> = code_only(text).chars().collect();
|
||||
let text: Vec<char> = text.chars().collect();
|
||||
let (items, whole_file) = test_only_items(&text, &code, file);
|
||||
Self {
|
||||
code,
|
||||
items,
|
||||
whole_file,
|
||||
}
|
||||
}
|
||||
|
||||
/// The code with every test-only item removed.
|
||||
fn production(&self) -> String {
|
||||
if self.whole_file {
|
||||
return String::new();
|
||||
}
|
||||
let mut kept = String::with_capacity(self.code.len());
|
||||
let mut at = 0;
|
||||
for &(start, _, end) in &self.items {
|
||||
kept.extend(&self.code[at..start]);
|
||||
at = end;
|
||||
}
|
||||
kept.extend(&self.code[at..]);
|
||||
kept
|
||||
}
|
||||
|
||||
fn item_code(&self, start: usize, end: usize) -> String {
|
||||
self.code[start..end].iter().collect()
|
||||
}
|
||||
}
|
||||
|
||||
/// The out-of-line modules `file` declares under a test-only `cfg`, as paths
|
||||
/// without extension (`#[cfg(test)] mod x;` is `<dir>/x`).
|
||||
fn test_module_paths(file: &Path, code: &str) -> Vec<PathBuf> {
|
||||
fn test_module_paths(file: &Path, source: &Source) -> Vec<PathBuf> {
|
||||
let dir = match file.file_stem().and_then(|s| s.to_str()) {
|
||||
Some("lib" | "main" | "mod") => file.parent().map(Path::to_path_buf),
|
||||
Some(stem) => file.parent().map(|p| p.join(stem)),
|
||||
None => None,
|
||||
}
|
||||
.expect("a source file has a directory and a stem");
|
||||
let lines: Vec<&str> = code.lines().map(str::trim).collect();
|
||||
let mut found = Vec::new();
|
||||
for (i, line) in lines.iter().enumerate() {
|
||||
if !is_test_cfg(line) {
|
||||
continue;
|
||||
}
|
||||
let item = lines[i + 1..]
|
||||
.iter()
|
||||
.find(|l| !l.starts_with("#[") && !l.is_empty());
|
||||
if let Some(name) = item.and_then(|l| out_of_line_mod(l)) {
|
||||
found.push(dir.join(name));
|
||||
}
|
||||
}
|
||||
found
|
||||
source
|
||||
.items
|
||||
.iter()
|
||||
.filter_map(|&(_, start, end)| out_of_line_mod(source.item_code(start, end).trim()))
|
||||
.map(|name| dir.join(name))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// `mod x;`, `pub mod x;`, `pub(crate) mod x;` → `x`.
|
||||
fn out_of_line_mod(line: &str) -> Option<&str> {
|
||||
let rest = line.split_once("mod ")?.1.strip_suffix(';')?;
|
||||
let prefix = line.split_once("mod ")?.0.trim();
|
||||
(prefix.is_empty() || prefix.starts_with("pub")).then_some(rest.trim())
|
||||
fn out_of_line_mod(item: &str) -> Option<String> {
|
||||
let (prefix, rest) = item.split_once("mod ")?;
|
||||
let name = rest.strip_suffix(';')?.trim();
|
||||
let prefix = prefix.trim();
|
||||
(prefix.is_empty() || prefix.starts_with("pub")).then(|| name.to_string())
|
||||
}
|
||||
|
||||
/// A `cfg` attribute that compiles its item only under `test`. `cfg(not(test))`
|
||||
/// is production, and `"test-support"` is a literal, already blanked.
|
||||
fn is_test_cfg(line: &str) -> bool {
|
||||
let line = line.trim();
|
||||
line.starts_with("#[cfg(")
|
||||
&& !line.contains("not(")
|
||||
&& line
|
||||
.split(|c: char| !c.is_alphanumeric() && c != '_')
|
||||
.any(|word| word == "test")
|
||||
/// Every test-only item in a file, bounded by balanced brackets on the code
|
||||
/// rather than by indentation. An item it cannot bound is a panic, never EOF.
|
||||
fn test_only_items(
|
||||
text: &[char],
|
||||
code: &[char],
|
||||
file: &Path,
|
||||
) -> (Vec<(usize, usize, usize)>, bool) {
|
||||
let mut items = Vec::new();
|
||||
let mut k = 0;
|
||||
while k < code.len() {
|
||||
let Some((inner, end)) = attribute_at(code, k) else {
|
||||
k += 1;
|
||||
continue;
|
||||
};
|
||||
let predicate: String = text[k..end].iter().collect();
|
||||
if !is_test_only(&predicate) {
|
||||
k = end;
|
||||
continue;
|
||||
}
|
||||
if inner {
|
||||
// An inner attribute heads the file or it heads an inline module;
|
||||
// only the first is a shape this scan can honour.
|
||||
let leading = code[..k].iter().all(|c| c.is_whitespace());
|
||||
assert!(
|
||||
leading,
|
||||
"{}: an inner test-only cfg could not be bounded",
|
||||
file.display()
|
||||
);
|
||||
return (Vec::new(), true);
|
||||
}
|
||||
let start = past_attributes(code, end);
|
||||
let item_end = item_end(code, start).unwrap_or_else(|| {
|
||||
panic!(
|
||||
"{}: a test-only item at char {k} could not be bounded",
|
||||
file.display()
|
||||
)
|
||||
});
|
||||
items.push((k, start, item_end));
|
||||
k = item_end;
|
||||
}
|
||||
(items, false)
|
||||
}
|
||||
|
||||
/// Drop every test-only item. rustfmt puts an item's closing brace at the
|
||||
/// attribute's own indentation, which is what makes this a line scan (§2).
|
||||
fn strip_test_items(text: &str) -> String {
|
||||
let lines: Vec<&str> = text.lines().collect();
|
||||
let mut kept = String::new();
|
||||
let mut i = 0;
|
||||
while i < lines.len() {
|
||||
let line = lines[i];
|
||||
match is_test_cfg(line) {
|
||||
false => {
|
||||
kept.push_str(line);
|
||||
kept.push('\n');
|
||||
i += 1;
|
||||
}
|
||||
true => {
|
||||
let indent = line.len() - line.trim_start().len();
|
||||
let close = format!("{}}}", " ".repeat(indent));
|
||||
i += 1;
|
||||
while i < lines.len() && lines[i].trim_start().starts_with("#[") {
|
||||
i += 1;
|
||||
}
|
||||
match lines.get(i).is_some_and(|l| l.trim_end().ends_with(';')) {
|
||||
true => i += 1,
|
||||
false => {
|
||||
while i < lines.len() && lines[i].trim_end() != close {
|
||||
i += 1;
|
||||
}
|
||||
i += 1;
|
||||
}
|
||||
/// An attribute starting at `k`: whether it is inner (`#!`) and where it ends.
|
||||
fn attribute_at(code: &[char], k: usize) -> Option<(bool, usize)> {
|
||||
if code[k] != '#' || (k > 0 && (code[k - 1].is_alphanumeric() || code[k - 1] == '_')) {
|
||||
return None;
|
||||
}
|
||||
let inner = code.get(k + 1) == Some(&'!');
|
||||
let open = k + 1 + usize::from(inner);
|
||||
(code.get(open) == Some(&'[')).then(|| matching(code, open).map(|close| (inner, close + 1)))?
|
||||
}
|
||||
|
||||
/// The index of the bracket closing the one at `open`, counting all three kinds.
|
||||
fn matching(code: &[char], open: usize) -> Option<usize> {
|
||||
let mut depth = 0usize;
|
||||
for (k, &c) in code.iter().enumerate().skip(open) {
|
||||
match c {
|
||||
'(' | '[' | '{' => depth += 1,
|
||||
')' | ']' | '}' => {
|
||||
depth -= 1;
|
||||
if depth == 0 {
|
||||
return Some(k);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
kept
|
||||
None
|
||||
}
|
||||
|
||||
/// Skip whitespace and any further outer attributes on the same item.
|
||||
fn past_attributes(code: &[char], mut k: usize) -> usize {
|
||||
loop {
|
||||
while code.get(k).is_some_and(|c| c.is_whitespace()) {
|
||||
k += 1;
|
||||
}
|
||||
let next = (k < code.len()).then(|| attribute_at(code, k)).flatten();
|
||||
match next {
|
||||
Some((false, end)) => k = end,
|
||||
_ => return k,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Where the item starting at `start` ends: a `;` at depth zero, or the `}`
|
||||
/// closing its body. `None` at the end of the file, which is not an end.
|
||||
fn item_end(code: &[char], start: usize) -> Option<usize> {
|
||||
let mut depth = 0usize;
|
||||
let mut k = start;
|
||||
while k < code.len() {
|
||||
match code[k] {
|
||||
'(' | '[' | '{' => depth += 1,
|
||||
';' if depth == 0 => return Some(k + 1),
|
||||
// The enclosing block closed first: the item was its last expression.
|
||||
')' | ']' | '}' if depth == 0 => return Some(k),
|
||||
')' | ']' => depth -= 1,
|
||||
'}' => {
|
||||
depth -= 1;
|
||||
if depth == 0 && !continues(code, k + 1) {
|
||||
return Some(k + 1);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
k += 1;
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Whether the item goes on after a `}` at depth zero: `S { .. };`, a method
|
||||
/// call on a block, or an `if` with an `else`.
|
||||
fn continues(code: &[char], after: usize) -> bool {
|
||||
let rest: String = code[after..]
|
||||
.iter()
|
||||
.skip_while(|c| c.is_whitespace())
|
||||
.take(5)
|
||||
.collect();
|
||||
rest.starts_with([';', '.', '?']) || rest.starts_with("else")
|
||||
}
|
||||
|
||||
/// A `cfg` attribute whose predicate no shipped build satisfies. Evaluated,
|
||||
/// not substring-matched (improvise-59q8): `context/code/protocol.md` §2.
|
||||
fn is_test_only(attribute: &str) -> bool {
|
||||
let tokens = cfg_tokens(attribute);
|
||||
match tokens.as_slice() {
|
||||
[Token::Word(cfg), Token::Open, rest @ ..] if cfg == "cfg" => {
|
||||
let mut at = 0;
|
||||
predicate(rest, &mut at) == Truth::No
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// What a `cfg` predicate is in a build without `test`. `Unknown` for what a
|
||||
/// source scan cannot know, such as a target or an ordinary feature.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum Truth {
|
||||
Yes,
|
||||
No,
|
||||
Unknown,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
enum Token {
|
||||
Word(String),
|
||||
Str(String),
|
||||
Open,
|
||||
Close,
|
||||
Comma,
|
||||
Equals,
|
||||
}
|
||||
|
||||
/// The tokens of an attribute's text, from its first word: `#`, `!` and the
|
||||
/// brackets around it are dropped.
|
||||
fn cfg_tokens(attribute: &str) -> Vec<Token> {
|
||||
let inner = attribute
|
||||
.trim_start_matches('#')
|
||||
.trim_start_matches('!')
|
||||
.trim()
|
||||
.trim_start_matches('[')
|
||||
.trim_end()
|
||||
.trim_end_matches(']');
|
||||
let chars: Vec<char> = inner.chars().collect();
|
||||
let mut tokens = Vec::new();
|
||||
let mut k = 0;
|
||||
while k < chars.len() {
|
||||
let c = chars[k];
|
||||
match c {
|
||||
'(' => tokens.push(Token::Open),
|
||||
')' => tokens.push(Token::Close),
|
||||
',' => tokens.push(Token::Comma),
|
||||
'=' => tokens.push(Token::Equals),
|
||||
'"' => {
|
||||
let end = quoted_end(&chars, k + 1, '"');
|
||||
tokens.push(Token::Str(chars[k + 1..end - 1].iter().collect()));
|
||||
k = end;
|
||||
continue;
|
||||
}
|
||||
c if c.is_alphanumeric() || c == '_' => {
|
||||
let word: String = chars[k..]
|
||||
.iter()
|
||||
.take_while(|c| c.is_alphanumeric() || **c == '_')
|
||||
.collect();
|
||||
k += word.chars().count();
|
||||
tokens.push(Token::Word(word));
|
||||
continue;
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
k += 1;
|
||||
}
|
||||
tokens
|
||||
}
|
||||
|
||||
/// One predicate, advancing `at` past it. `test` is false; so is the
|
||||
/// `test-support` feature, which this repo never ships (improvise-26dm).
|
||||
fn predicate(tokens: &[Token], at: &mut usize) -> Truth {
|
||||
let Some(Token::Word(word)) = tokens.get(*at) else {
|
||||
return Truth::Unknown;
|
||||
};
|
||||
*at += 1;
|
||||
match (word.as_str(), tokens.get(*at)) {
|
||||
(op @ ("all" | "any" | "not"), Some(Token::Open)) => {
|
||||
*at += 1;
|
||||
let mut args = Vec::new();
|
||||
while !matches!(tokens.get(*at), Some(Token::Close) | None) {
|
||||
args.push(predicate(tokens, at));
|
||||
if tokens.get(*at) == Some(&Token::Comma) {
|
||||
*at += 1;
|
||||
}
|
||||
}
|
||||
*at += 1;
|
||||
combine(op, &args)
|
||||
}
|
||||
(key, Some(Token::Equals)) => {
|
||||
*at += 1;
|
||||
let value = match tokens.get(*at) {
|
||||
Some(Token::Str(v)) => v.clone(),
|
||||
_ => String::new(),
|
||||
};
|
||||
*at += 1;
|
||||
match (key, value.as_str()) {
|
||||
("feature", "test-support") => Truth::No,
|
||||
_ => Truth::Unknown,
|
||||
}
|
||||
}
|
||||
("test", _) => Truth::No,
|
||||
_ => Truth::Unknown,
|
||||
}
|
||||
}
|
||||
|
||||
/// Kleene's three-valued `all`, `any` and `not`.
|
||||
fn combine(op: &str, args: &[Truth]) -> Truth {
|
||||
match op {
|
||||
"all" if args.contains(&Truth::No) => Truth::No,
|
||||
"all" if args.iter().all(|a| *a == Truth::Yes) => Truth::Yes,
|
||||
"any" if args.contains(&Truth::Yes) => Truth::Yes,
|
||||
"any" if args.iter().all(|a| *a == Truth::No) => Truth::No,
|
||||
"not" => match args.first() {
|
||||
Some(Truth::Yes) => Truth::No,
|
||||
Some(Truth::No) => Truth::Yes,
|
||||
_ => Truth::Unknown,
|
||||
},
|
||||
_ => Truth::Unknown,
|
||||
}
|
||||
}
|
||||
|
||||
/// `text` with comments, string, byte-string, raw-string and char literals
|
||||
@@ -166,8 +402,8 @@ fn literal_or_comment_end(c: &[char], i: usize) -> Option<usize> {
|
||||
(Some('/'), Some('/')) => Some(line_end(c, i)),
|
||||
(Some('/'), Some('*')) => Some(block_end(c, i)),
|
||||
(Some('"'), _) => Some(quoted_end(c, i + 1, '"')),
|
||||
(Some('b'), Some('"')) if !word_before => Some(quoted_end(c, i + 2, '"')),
|
||||
(Some('r' | 'b'), Some('r' | '#' | '"')) if !word_before => raw_end(c, i),
|
||||
(Some('b' | 'c'), Some('"')) if !word_before => Some(quoted_end(c, i + 2, '"')),
|
||||
(Some('r' | 'b' | 'c'), Some('r' | '#' | '"')) if !word_before => raw_end(c, i),
|
||||
(Some('\''), _) => char_end(c, i),
|
||||
_ => None,
|
||||
}
|
||||
@@ -211,9 +447,10 @@ fn quoted_end(c: &[char], mut k: usize, quote: char) -> usize {
|
||||
c.len()
|
||||
}
|
||||
|
||||
/// `r"…"`, `r#"…"#`, `br#"…"#`. `None` when the `r` is only an identifier.
|
||||
/// `r"…"`, `r#"…"#`, `br#"…"#`, `cr#"…"#`. `None` when the `r` is only an
|
||||
/// identifier.
|
||||
fn raw_end(c: &[char], i: usize) -> Option<usize> {
|
||||
let mut k = i + usize::from(c[i] == 'b');
|
||||
let mut k = i + usize::from(matches!(c[i], 'b' | 'c'));
|
||||
if c.get(k) != Some(&'r') {
|
||||
return None;
|
||||
}
|
||||
@@ -283,21 +520,20 @@ mod tests {
|
||||
"}\n",
|
||||
"#[cfg(test)]\n",
|
||||
"mod out_of_line;\n",
|
||||
"#[cfg(any(test, feature = \"x\"))]\n",
|
||||
"#[cfg(any(test, feature = \"test-support\"))]\n",
|
||||
"pub mod gated;\n",
|
||||
"#[cfg(not(test))]\n",
|
||||
"fn production_only() {}\n",
|
||||
"fn also_kept() {}\n",
|
||||
);
|
||||
let code = code_only(text);
|
||||
let stripped = strip_test_items(&code);
|
||||
let stripped = production_text(text);
|
||||
for kept in ["fn kept", "fn production_only", "fn also_kept"] {
|
||||
assert!(stripped.contains(kept), "{kept} was dropped: {stripped}");
|
||||
}
|
||||
assert!(!stripped.contains("gone_inline"), "{stripped}");
|
||||
let lib = Path::new("/c/src/lib.rs");
|
||||
assert_eq!(
|
||||
test_module_paths(lib, &code),
|
||||
test_module_paths(lib, &Source::of(text, lib)),
|
||||
vec![
|
||||
PathBuf::from("/c/src/out_of_line"),
|
||||
PathBuf::from("/c/src/gated")
|
||||
@@ -305,7 +541,7 @@ mod tests {
|
||||
);
|
||||
let nested = Path::new("/c/src/ui/reduce.rs");
|
||||
assert_eq!(
|
||||
test_module_paths(nested, &code)[0],
|
||||
test_module_paths(nested, &Source::of(text, nested))[0],
|
||||
PathBuf::from("/c/src/ui/reduce/out_of_line")
|
||||
);
|
||||
assert!(is_module_file(
|
||||
@@ -322,6 +558,112 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
/// improvise-59q8, the shapes that HID production code: a test-only item
|
||||
/// whose end the line scan could not find. Evidence: test-evidence/protocol.md.
|
||||
#[test]
|
||||
fn a_test_only_item_hides_nothing_after_it() {
|
||||
let cases = [
|
||||
(
|
||||
"a multi-line item ending in `];`",
|
||||
concat!(
|
||||
"impl X {\n",
|
||||
" #[cfg(test)]\n",
|
||||
" const NAMES: &[&str] = &[\n",
|
||||
" \"a\",\n",
|
||||
" ];\n",
|
||||
" fn prod() { let c = Command::Hidden1(1); }\n",
|
||||
"}\n",
|
||||
"fn after() { Command::Visible(2) }\n",
|
||||
),
|
||||
),
|
||||
(
|
||||
"a multi-line statement ending in `);`",
|
||||
concat!(
|
||||
"fn f() {\n",
|
||||
" #[cfg(test)]\n",
|
||||
" let x = foo(\n",
|
||||
" 1,\n",
|
||||
" );\n",
|
||||
" let c = Command::Hidden1(1);\n",
|
||||
"}\n",
|
||||
"fn after() { Command::Visible(2) }\n",
|
||||
),
|
||||
),
|
||||
(
|
||||
"a one-line `#[cfg(test)] use`",
|
||||
concat!(
|
||||
"#[cfg(test)] use foo::bar;\n",
|
||||
"fn prod() { Command::Hidden1(1) }\n",
|
||||
"fn after() { Command::Visible(2) }\n",
|
||||
),
|
||||
),
|
||||
];
|
||||
for (what, text) in cases {
|
||||
let kept = production_text(text);
|
||||
for name in ["Hidden1", "Visible"] {
|
||||
assert!(kept.contains(name), "{what}: {name} was hidden: {kept:?}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-59q8, the shapes that COUNTED test code as production.
|
||||
#[test]
|
||||
fn a_test_only_cfg_is_recognised_whatever_its_spelling() {
|
||||
let cases = [
|
||||
(
|
||||
"`all(test, not(..))`",
|
||||
"#[cfg(all(test, not(target_arch = \"wasm32\")))]\nfn t() { Command::TestOnly(1) }\n",
|
||||
),
|
||||
(
|
||||
"an inner `#![cfg(test)]` heading the file",
|
||||
"#![cfg(test)]\nfn t() { Command::TestOnly(1) }\n",
|
||||
),
|
||||
(
|
||||
"an attribute split across lines",
|
||||
"#[cfg(\n test\n)]\nfn t() { Command::TestOnly(1) }\n",
|
||||
),
|
||||
];
|
||||
for (what, text) in cases {
|
||||
let kept = production_text(text);
|
||||
assert!(
|
||||
!kept.contains("TestOnly"),
|
||||
"{what} was read as production: {kept:?}"
|
||||
);
|
||||
}
|
||||
// The controls: a predicate a shipped build can satisfy is production.
|
||||
for text in [
|
||||
"#[cfg(not(test))]\nfn p() { Command::Shipped(1) }\n",
|
||||
"#[cfg(any(test, feature = \"serde\"))]\nfn p() { Command::Shipped(1) }\n",
|
||||
"#[cfg(target_arch = \"wasm32\")]\nfn p() { Command::Shipped(1) }\n",
|
||||
] {
|
||||
assert!(
|
||||
production_text(text).contains("Shipped"),
|
||||
"{text:?} was dropped"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-59q8's minor case: a C string literal leaked its contents.
|
||||
#[test]
|
||||
fn a_c_string_literal_is_blanked() {
|
||||
// The raw one quotes inside itself, so lexing it as ordinary strings
|
||||
// leaves the text BETWEEN the inner quotes in the open.
|
||||
let kept = code_only(
|
||||
"let a = c\"Command::InC(1)\"; let b = cr#\"x\" Command::InRawC(2) \"y\"#;\n",
|
||||
);
|
||||
for gone in ["InC", "InRawC"] {
|
||||
assert!(!kept.contains(gone), "{gone} survived: {kept}");
|
||||
}
|
||||
}
|
||||
|
||||
/// A test-only item the scan cannot bound is REFUSED, never read to the
|
||||
/// end of the file, which is what silently hid every shape above.
|
||||
#[test]
|
||||
#[should_panic(expected = "could not be bounded")]
|
||||
fn an_unbounded_test_only_item_is_refused_not_swallowed() {
|
||||
let _ = production_text("#[cfg(test)]\nfn broken() {\n Command::X(1)\n");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn comments_and_literals_are_blanked_and_code_is_kept() {
|
||||
let text = concat!(
|
||||
|
||||
@@ -173,7 +173,10 @@ impl Client {
|
||||
pub fn record_cell_at(&self, row: usize, col: usize) -> Option<RecordCell> {
|
||||
match self.state.grid.cell(row, col)?.id.clone()? {
|
||||
CellId::Record(cell) => Some(cell),
|
||||
CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => None,
|
||||
CellId::Aggregate(_)
|
||||
| CellId::Derived { .. }
|
||||
| CellId::Create(_)
|
||||
| CellId::Coordinate => None,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -144,7 +144,16 @@ impl RenderCell {
|
||||
text: cell.text.clone(),
|
||||
numeric: cell.numeric,
|
||||
error: cell.error,
|
||||
editable: matches!(cell.id, Some(CellId::Record(_))),
|
||||
editable: match cell.id {
|
||||
Some(CellId::Record(_)) => true,
|
||||
Some(
|
||||
CellId::Aggregate(_)
|
||||
| CellId::Derived { .. }
|
||||
| CellId::Create(_)
|
||||
| CellId::Coordinate,
|
||||
)
|
||||
| None => false,
|
||||
},
|
||||
selected,
|
||||
edit_text: cell.edit_text.clone().unwrap_or_default(),
|
||||
}
|
||||
@@ -563,9 +572,8 @@ mod tests {
|
||||
|
||||
/// A 2x3 grid whose cells cover the four id shapes plus a padded slot, so
|
||||
/// one fixture answers every editability question.
|
||||
fn group(cat: &str, name: &str, collapsed: bool) -> FrameEntry {
|
||||
fn group(name: &str, collapsed: bool) -> FrameEntry {
|
||||
FrameEntry::Group(improvise_protocol::FrameGroup {
|
||||
cat: cat.into(),
|
||||
name: name.into(),
|
||||
collapsed,
|
||||
})
|
||||
@@ -583,11 +591,11 @@ mod tests {
|
||||
first: 0,
|
||||
cats: vec!["Month".into()],
|
||||
entries: vec![
|
||||
group("Month", "Q1", false),
|
||||
group("Q1", false),
|
||||
item("Jan"),
|
||||
group("Fiscal", "Q1", true),
|
||||
group("Q1", true),
|
||||
item("Feb"),
|
||||
group("Month", "Q2", false),
|
||||
group("Q2", false),
|
||||
item("Apr"),
|
||||
],
|
||||
};
|
||||
@@ -628,6 +636,10 @@ mod tests {
|
||||
}),
|
||||
),
|
||||
((1, 0), CellDisplay::text("pending")),
|
||||
(
|
||||
(1, 2),
|
||||
CellDisplay::text("North").with_id(CellId::Coordinate),
|
||||
),
|
||||
])),
|
||||
],
|
||||
});
|
||||
@@ -659,6 +671,10 @@ mod tests {
|
||||
!model.rows[1].cells()[1].editable,
|
||||
"and neither is a slot fit_to_labels padded"
|
||||
);
|
||||
assert!(
|
||||
!model.rows[1].cells()[2].editable,
|
||||
"renaming a coordinate moves the whole row, which is not a cell write"
|
||||
);
|
||||
}
|
||||
|
||||
/// The numeric/error flags are the server's answers, echoed — so the test
|
||||
|
||||
@@ -187,6 +187,21 @@ test.describe("the standalone deployment", () => {
|
||||
await expect(page.locator("#grid td.editable").first()).toBeVisible();
|
||||
});
|
||||
|
||||
// improvise-i0v9: "in-page worker · <doc>" was written at 13ms, before the
|
||||
// worker had answered anything, and read as success over a blank page.
|
||||
test("the worker is not called up until its first frame has arrived", async ({ page }) => {
|
||||
const doc = `first-frame-${Date.now()}.improv`;
|
||||
await page.addInitScript(logStatus);
|
||||
await page.goto(`/app/index.html?doc=${doc}`);
|
||||
await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
|
||||
await expect(status(page)).toHaveText(`in-page worker · ${doc}`);
|
||||
const log = await page.evaluate(() => window.__statusLog);
|
||||
expect(log, `#status said: ${json(log)}`).toEqual([
|
||||
`starting the worker · ${doc}…`,
|
||||
`in-page worker · ${doc}`,
|
||||
]);
|
||||
});
|
||||
|
||||
// improvise-9cma: a `?doc=` this backend cannot hold used to reach the
|
||||
// generic watchdog 8s later. The REASON now crosses the port (improvise-1wft).
|
||||
test("an invalid ?doc= is named on screen, and not blamed on the browser", async ({
|
||||
@@ -235,6 +250,7 @@ test.describe("the standalone deployment", () => {
|
||||
|
||||
const second = await context.newPage();
|
||||
await second.clock.install();
|
||||
await second.addInitScript(logStatus);
|
||||
await second.goto(`/app/index.html?doc=${doc}`);
|
||||
|
||||
// It PAINTS. That is the whole point: a locked document is read-only, not
|
||||
@@ -242,6 +258,15 @@ test.describe("the standalone deployment", () => {
|
||||
await expect(second.locator("#grid table")).toBeVisible({ timeout: 20_000 });
|
||||
await expect(second.locator("#status")).toContainText("read-only");
|
||||
await expect(second.locator("#status")).toContainText("another tab");
|
||||
// The page's clock has not been walked, so a reason that arrived AFTER the
|
||||
// watchdog would have been logged behind "did not answer". Nor was success
|
||||
// ever claimed: the Snapshot that follows the notice must not overwrite it.
|
||||
const early = await second.evaluate(() => window.__statusLog);
|
||||
expect(early.at(-1), `#status said: ${json(early)}`).toContain("read-only");
|
||||
expect(
|
||||
early.filter((t) => t.includes("did not answer") || t === `in-page worker · ${doc}`),
|
||||
`#status said: ${json(early)}`,
|
||||
).toEqual([]);
|
||||
|
||||
// And the reason STANDS: it is not replaced 8s later by a watchdog
|
||||
// blaming the browser for something the code knew precisely.
|
||||
@@ -252,6 +277,10 @@ test.describe("the standalone deployment", () => {
|
||||
await second.clock.runFor(HANDSHAKE_TIMEOUT_MS + 2_000);
|
||||
await expect(second.locator("#status")).toContainText("read-only");
|
||||
await expect(second.locator("#status")).not.toContainText("did not answer");
|
||||
const late = await second.evaluate(() => window.__statusLog);
|
||||
expect(late.some((t) => t.includes("did not answer")), `#status said: ${json(late)}`).toBe(
|
||||
false,
|
||||
);
|
||||
|
||||
await second.close();
|
||||
await first.close();
|
||||
|
||||
+3
-1
@@ -181,7 +181,8 @@ function workerTransport() {
|
||||
shell.fail(`worker failed to start: ${event.message ?? "see console"}`);
|
||||
console.error("improvise: worker error", event);
|
||||
});
|
||||
shell.setShell(`in-page worker · ${DOC}`);
|
||||
// Not yet a claim that it works: the first frame makes that (improvise-i0v9).
|
||||
shell.setShell(`starting the worker · ${DOC}…`);
|
||||
// Frames posted before the worker has finished `start()` are held by
|
||||
// worker.js in arrival order, so the handshake can go out immediately.
|
||||
return { send: (text) => worker.postMessage(text), ready: Promise.resolve() };
|
||||
@@ -798,6 +799,7 @@ shell.arm(
|
||||
WS_URL
|
||||
? `${WS_URL} never answered — is improvise-ws-server running, and did it send a document?`
|
||||
: "the in-page worker did not answer (see console)",
|
||||
{ answer: WS_URL ? null : `in-page worker · ${DOC}` },
|
||||
);
|
||||
await transport.ready;
|
||||
sendViewport(); // the handshake: the snapshot is built at this extent
|
||||
|
||||
+13
-7
@@ -339,7 +339,12 @@ export function createShell({
|
||||
clearTimer = clearTimeout,
|
||||
}) {
|
||||
let watchdog = null;
|
||||
let answered = null;
|
||||
let arms = 0;
|
||||
const disarm = () => {
|
||||
clearTimer(watchdog);
|
||||
watchdog = null;
|
||||
};
|
||||
|
||||
return {
|
||||
setShell(text) {
|
||||
@@ -350,24 +355,25 @@ export function createShell({
|
||||
},
|
||||
/** A peer that never answers the handshake. Deliberately about the *first
|
||||
* frame*, not either peer's own failure modes — clients.md. */
|
||||
arm(message) {
|
||||
arm(message, { answer = null } = {}) {
|
||||
arms += 1;
|
||||
clearTimer(watchdog);
|
||||
answered = answer;
|
||||
disarm();
|
||||
watchdog = setTimer(() => {
|
||||
watchdog = null;
|
||||
this.setShell(message);
|
||||
}, timeoutMs);
|
||||
},
|
||||
/** A frame arrived, so the peer answered: the watchdog has no question
|
||||
* left to ask. */
|
||||
/** A frame arrived, so the peer answered. Only the FIRST, and only if no
|
||||
* failure spoke first, may say so (improvise-i0v9, clients.md). */
|
||||
sawFrame() {
|
||||
clearTimer(watchdog);
|
||||
watchdog = null;
|
||||
if (watchdog !== null && answered !== null) this.setShell(answered);
|
||||
disarm();
|
||||
},
|
||||
/** A peer that failed *loudly* has already said something more precise, so
|
||||
* the watchdog must be DISARMED and not merely pre-empted (PR #13). */
|
||||
fail(text) {
|
||||
this.sawFrame();
|
||||
disarm();
|
||||
this.setShell(text);
|
||||
},
|
||||
armed: () => watchdog !== null,
|
||||
|
||||
@@ -351,6 +351,43 @@ test("a notice DISARMS the watchdog rather than being replaced by it", () => {
|
||||
);
|
||||
});
|
||||
|
||||
// improvise-i0v9: "in-page worker · <doc>" was written before the worker had
|
||||
// answered anything, and read as success over a blank page for eight seconds.
|
||||
test("only the first frame claims the peer answered, and not before it", () => {
|
||||
const clock = fakeClock();
|
||||
const seen = [];
|
||||
const shell = createShell({
|
||||
onShell: (t) => seen.push(t),
|
||||
timeoutMs: 8000,
|
||||
setTimer: clock.setTimer,
|
||||
clearTimer: clock.clearTimer,
|
||||
});
|
||||
shell.arm("did not answer", { answer: "in-page worker · a.improv" });
|
||||
assert.deepEqual(seen, [], "the claim was made before any frame arrived");
|
||||
shell.sawFrame();
|
||||
shell.sawFrame();
|
||||
assert.deepEqual(seen, ["in-page worker · a.improv"]);
|
||||
});
|
||||
|
||||
// The order worker.js really produces on a locked document: the ReadOnly
|
||||
// notice is posted after `start()` and BEFORE the Snapshot answers the handshake.
|
||||
test("a reason given before the first frame is not replaced by the frame's claim", () => {
|
||||
const clock = fakeClock();
|
||||
const seen = [];
|
||||
const shell = createShell({
|
||||
onShell: (t) => seen.push(t),
|
||||
timeoutMs: 8000,
|
||||
setTimer: clock.setTimer,
|
||||
clearTimer: clock.clearTimer,
|
||||
});
|
||||
shell.arm("did not answer", { answer: "in-page worker · a.improv" });
|
||||
shell.fail(noticeText({ kind: "ReadOnly", document: "a.improv", detail: "" }));
|
||||
shell.sawFrame();
|
||||
clock.fire();
|
||||
assert.equal(seen.length, 1, `#status said: ${JSON.stringify(seen)}`);
|
||||
assert.match(seen[0], /read-only/);
|
||||
});
|
||||
|
||||
// The notice's remembered state. The value crosses `localStorage`, which is
|
||||
// allowed to forget and allowed to throw, so the interesting inputs are the
|
||||
// ones that are NOT either spelling.
|
||||
|
||||
Reference in New Issue
Block a user