Merge remote-tracking branch 'gh/main' into wave/km71-edit-formula
This commit is contained in:
+63
-18
@@ -592,10 +592,23 @@ the client was told nothing at all.
|
||||
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.
|
||||
own inline abort (gotcha 21). The item and view effects, and `AddFormula`'s
|
||||
missing-category arm, only refuse: the frame is rolled back by the refusal
|
||||
alone. On the key path, a failed `AddItem` or view edit no longer stops the
|
||||
rest of its batch, which is what it did before bok4.
|
||||
- **`AddFormula`'s parse-error arm also stops (improvise-v4hg).** It is the one
|
||||
item-or-formula failure a key-path batch was found to depend on. Enter in
|
||||
`FormulaEdit` is `[SetStatus, AddFormula, ChangeMode(FormulaPanel),
|
||||
ClearBuffer("formula")]`, and the last two assume the commit worked. Without
|
||||
the stop, `Total = (` was reported and then the editor closed and the typed
|
||||
text was cleared. With it, the user stays in `FormulaEdit` with the text, and
|
||||
the error is drawn beside the prompt. `fit-measure` emits `AddFormula` last, so
|
||||
there is nothing after it to stop. The missing-category arm is reachable only
|
||||
from a wire frame or a `:` line naming another category, and nothing in either
|
||||
depends on it, so it does not stop. PR #181's review searched for an
|
||||
`AddItem` case the same way and found none, so `AddItem` does not stop either.
|
||||
An abort is added where a later effect depends on the failed one, never for
|
||||
symmetry.
|
||||
|
||||
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
|
||||
@@ -615,8 +628,17 @@ Consequences:
|
||||
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
|
||||
status-only write (improvise-9fpl, improvise-0v8l). Two tables carry most of
|
||||
them. `every_failing_model_command_refuses_its_whole_frame` covers the wire.
|
||||
`every_refusal_site_refuses_its_failure_and_only_its_failure` covers the key
|
||||
path and the `:` line, which is what the headless exit code reads. The wire
|
||||
table cannot see the refusal half of a `refuse_and_stop` site, because
|
||||
`Host::mutating_frame` rejects on `refused || aborted`, and the abort alone
|
||||
suffices. So each `refuse_and_stop` site needs a case in both tables:
|
||||
the key-path table for its refusal, and
|
||||
`a_failed_load_or_import_does_not_run_the_quit_after_it` for its abort
|
||||
(improvise-514o). Each key-path case has a succeeding twin, so a site
|
||||
that refused unconditionally would fail too. 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
|
||||
@@ -3357,18 +3379,41 @@ one run in eight (measured: 46 of 400). So the arguments now include **every
|
||||
item of every regular category as a one-coordinate drill**, read off the parsed
|
||||
document. A one-coordinate drill spans every other category whole, so it matches
|
||||
the whole slice of records under that item, not one record picked by name. In
|
||||
the demo that is four records and eight cells for each of the eight drills. The
|
||||
test
|
||||
also asserts that the widest candidate it actually passes matches at least four
|
||||
records. That guard catches a demo with too little data, and it catches the
|
||||
derived drills being dropped from the argument list. It does **not** catch the
|
||||
comparison losing its wide drills some other way, because it measures what the
|
||||
argument lists match, not what the built commands ordered (improvise-b9f2). The
|
||||
review showed this with a rival: a `drill-into-cell` parser that refuses fewer
|
||||
than two arguments sends every one-coordinate drill to the one-record cursor
|
||||
fallback. The guard still reports four records, and with the store's sort
|
||||
removed the law passed 38 of 300 runs. With the store's sort removed and the
|
||||
parser intact, the law fails 400 of 400 runs.
|
||||
the demo that is four records and eight cells for each of the eight drills. With
|
||||
the store's sort removed, the law fails 400 of 400 runs.
|
||||
|
||||
**The width guard measures what the comparison compared (improvise-b9f2,
|
||||
improvise-btyw).** The test asserts that some `drill-into-cell` the comparison
|
||||
actually built and compared froze at least four records. For each one it applies
|
||||
those very effects to one scratch `App` and counts the records in its
|
||||
`drill_state`. It counts them only when their `Debug` text, which carries their
|
||||
order, appears in the text the comparison compared. The comparison reads `Debug`
|
||||
while the scratch `App` reads the applied state, and those are two
|
||||
representations. The review showed they can disagree: give `StartDrill` a
|
||||
`Debug` that prints only a row count, and the frozen records are still four
|
||||
while the compared text no longer holds their order. With the store's sort
|
||||
removed the law then passed 300 of 300 runs, with the guard green. Requiring
|
||||
the ordered records in the compared text ties the guard to what the comparison
|
||||
could actually see. A first
|
||||
version measured what the *argument lists* match in the store instead. That was
|
||||
a parallel derivation, and the review beat it with a `drill-into-cell` parser
|
||||
that refuses fewer than two arguments. Every one-coordinate drill then went to
|
||||
the one-record cursor fallback, the argument lists still matched four records,
|
||||
and with the store's sort removed the law passed 38 of 300 runs.
|
||||
|
||||
Counting frozen records is not enough by itself. A drill over no coordinates
|
||||
(any candidate with no `Cat/Item` argument, such as `["1", "1"]`) freezes
|
||||
**every** record, and it reads them through `iter_cells` in insertion order,
|
||||
never through the narrowing index where gotcha 18 lives. Under the parser rival
|
||||
that whole-store drill still froze 16 records, and a guard counting it stayed
|
||||
silent. So only a drill that **narrowed** the store counts: one that froze fewer
|
||||
records than the store holds. With the parser rival in place, the guard fires on
|
||||
every run ("froze 1 records"), and it fires on every run under the `Debug` rival
|
||||
too ("froze 0 records").
|
||||
|
||||
What the guard still cannot see is a narrowing drill whose records are frozen in
|
||||
an order that does not come from the store. It checks that the law had a wide
|
||||
drill to compare, not why the drill's order would differ.
|
||||
|
||||
**What it cannot see.** Only commands and arguments the fixture reaches. A
|
||||
command whose nondeterminism needs some other state, such as records mode, a
|
||||
|
||||
@@ -5316,6 +5316,81 @@ 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".
|
||||
|
||||
#### improvise-514o: every `refuse_and_stop` arm
|
||||
|
||||
The case list above covered `load` and the JSON cannot-read arm only. PR #181's
|
||||
review reverted each `refuse_and_stop` to `app.refuse` one at a time, which
|
||||
keeps the refusal and drops the abort. Five `ImportJsonHeadless` arms failed
|
||||
nothing: CSV cannot-read, CSV parse, JSON parse, array path and build. The wire
|
||||
table `every_failing_model_command_refuses_its_whole_frame` cannot see the
|
||||
abort, because `Host::mutating_frame` rejects on `refused || aborted`, and the
|
||||
refusal alone suffices. The list now has one case per arm, using the fixtures
|
||||
the wire table uses, which now come from `ui::host::import_fixture_storage`.
|
||||
Each case also asserts the arm's own status text, so a fixture that failed in a
|
||||
different arm cannot stand in for the one it names. The same scripted sweep,
|
||||
rerun after the change, fails this test at all seven sites. Before, it failed
|
||||
at two.
|
||||
|
||||
### ui::app::tests::a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text
|
||||
|
||||
improvise-v4hg. In `FormulaEdit`, Enter on `Total = (` reported the parse error
|
||||
and then did what a successful commit does: `CommitBuffer`'s
|
||||
`ChangeMode(FormulaPanel)` closed the editor and the keymap's welded
|
||||
`ClearBuffer("formula")` cleared the text. The user saw the error with their
|
||||
formula gone. Driven through `App::handle_key` from `FormulaPanel`: `a`, the
|
||||
characters, then Enter. The test failed first with "got FormulaPanel". The fix is
|
||||
`refuse_and_stop` on the parse-error arm, and a `Binding::Sequence` is one
|
||||
effect vec, so the stop also covers the `ClearBuffer` step.
|
||||
|
||||
The mode assertion alone would be vacuous against a fix that stops the mode
|
||||
change but still loses the text. That rival (clear the `formula` buffer, then
|
||||
`refuse_and_stop`) was run and fails "and must keep what they typed" with
|
||||
`left: None`. The control, `Total = 1`, asserts the ordinary path still stores
|
||||
the formula and closes the editor, so a stop on every arm would fail it. After
|
||||
the fix, the abort sweep (`refuse_and_stop` back to `app.refuse`) fails this
|
||||
test at that site.
|
||||
|
||||
### ui::app::tests::every_refusal_site_refuses_its_failure_and_only_its_failure
|
||||
|
||||
improvise-0v8l. PR #185 split refusals (`App::refuse`, which the headless exit
|
||||
code reads) from reports (`set_status`), and most of the refusal sites were
|
||||
unpinned. The ticket counted 23 of 27. At gh/main `608ba6a` (after #181's wire
|
||||
table), a script (`/tmp/refuse-fu/sweep.py`, not committed) reverted each of
|
||||
the **32** production refusal sites in `ui/effect.rs` and `command/cmd/*.rs`
|
||||
alone to a status-only write, and ran `cargo test --workspace --no-fail-fast`.
|
||||
For `effect::refuse(x)` the revert is `effect::set_status(x)`. For
|
||||
`app.refuse(x)` it is `app.view_state.status_msg = x.into()`. For
|
||||
`refuse_and_stop` it is the status write with the abort kept. **13 of 32 failed
|
||||
a test. 19 survived:** `AddCategory` at the limit, `apply_coordinate_rename`,
|
||||
both `StartImportWizard` arms, the five `ImportJsonHeadless` arms the wire
|
||||
table cannot see (the abort alone rejects the frame), `rename_record_coordinate`,
|
||||
`CommitExport`, both `ViewNavigate` arms, both `AddRecordRow` arms, both
|
||||
`EditOrDrill` arms, `SearchNavigate` and `ExecuteCommand`.
|
||||
|
||||
This table has one case per survivor. Each case drives the failure through a
|
||||
`:` line (`run_line`, which does what `improvise cmd` does, one batch per
|
||||
segment) or through Enter in the site's own minibuffer mode. It asserts that the
|
||||
batch refused and that the status names the site's own reason. Each case also
|
||||
has a **succeeding twin** that must not refuse, so a site that refused
|
||||
unconditionally fails too. That rival (`AddCategory` refusing on `Ok` as well)
|
||||
was run and fails "its succeeding twin was refused". Rerun after the change,
|
||||
the sweep catches **32 of 32**. The abort half, 10 sites before v4hg's eleventh,
|
||||
went from 5 caught to 11 of 11.
|
||||
|
||||
Three fixture choices. The second was found when the first run failed: the input meant to be refused was accepted.
|
||||
|
||||
- **`AddCategory` past the limit** needs eleven categories and a two-segment
|
||||
line. The first `add-category` fills the twelfth slot and the second is
|
||||
refused, so the succeeding twin is the same fixture with one segment.
|
||||
- **`add-record-row` onto an existing row** must page the category on the
|
||||
view the client is actually on. Records mode switches to `_Records`, so
|
||||
writing `Workbook::active_view_mut` (the document's default) changed a view
|
||||
nobody was looking at, and the case "succeeded". It goes through
|
||||
`App::view_mut_of(&ViewTarget::Active)` (gotcha 29).
|
||||
- **The drill rename** edits record 1, not record 0, following the
|
||||
cursor-at-index-0 rule in the wave agent rules. Nothing failed here: this is
|
||||
a precaution, not a finding.
|
||||
|
||||
### ui::effect::tests::a_drill_rows_measure_edits_follow_its_rename
|
||||
|
||||
improvise-uo2p. `ApplyAndClearDrill` walked `pending_edits`, a `HashMap`, and
|
||||
@@ -5434,14 +5509,41 @@ rivals it catches:
|
||||
still pin the store directly.
|
||||
- **The derived drills dropped from the argument list** (the
|
||||
`all.extend(one_coordinate_drills(model))` line commented out): the test
|
||||
fails before comparing anything, with "the widest drill orders 1 records".
|
||||
That is the only way of losing the wide drills the guard can see. It measures
|
||||
what the argument lists match, not what the built commands ordered, so it can
|
||||
pass while the comparison has lost its wide drills (improvise-b9f2). Review
|
||||
rival: `drill-into-cell`'s parser changed to refuse fewer than two arguments,
|
||||
which sends every one-coordinate drill to the one-record cursor fallback. The
|
||||
guard stays silent and, with gotcha 18 re-injected, the law passed 38 of 300
|
||||
runs.
|
||||
fails with "the widest narrowing drill compared froze 1 records".
|
||||
- **`drill-into-cell`'s parser changed to refuse fewer than two arguments**
|
||||
(improvise-b9f2): every one-coordinate drill then goes to the one-record cursor
|
||||
fallback. The first width guard measured what the argument lists match, so it
|
||||
stayed silent. With gotcha 18 re-injected the law passed 38 of 300 runs in
|
||||
review, and 31 of 300 here at `cfcc9bff`. The guard now applies the compared
|
||||
drills' own effects to a scratch `App` and counts the records they froze.
|
||||
Against this rival it fails on every run with "froze 1 records".
|
||||
- A first version of the fix counted all frozen records and **still passed**.
|
||||
A candidate with no `Cat/Item` argument drills the whole store (16 records),
|
||||
and it reads them in insertion order through `iter_cells`, which the
|
||||
injection cannot reach. So the guard counts only drills that froze fewer
|
||||
records than the store holds.
|
||||
- Measured at `7dfa90f1`, test binary in a loop:
|
||||
|
||||
| Faults | Law passes | Guard fired |
|
||||
|---|---|---|
|
||||
| gotcha 18 + parser rival | **0 of 300** | all 300 |
|
||||
| gotcha 18 alone | **0 of 300** | 0 (every failure a real divergence) |
|
||||
| neither | 300 of 300 | 0 |
|
||||
- **`StartDrill`'s `Debug` changed to print only its row count**
|
||||
(`StartDrill(4 rows)`, improvise-btyw): the law compares effects by their
|
||||
`Debug` text, and this rival takes the record order out of that text, while
|
||||
the frozen `drill_state` still holds four records. At `7dfa90f1`, with gotcha
|
||||
18 re-injected, the law **passed 200 of 200** runs and the guard stayed green.
|
||||
The guard now counts a drill only when its frozen records, as `Debug` prints
|
||||
them, appear in the text the comparison compared. The same measurements after
|
||||
the repair, 200 runs each:
|
||||
|
||||
| Faults | Law passes | Guard fired |
|
||||
|---|---|---|
|
||||
| gotcha 18 + `Debug` rival | **0 of 200** | all 200 ("froze 0 records") |
|
||||
| gotcha 18 + parser rival | **0 of 200** | all 200 |
|
||||
| gotcha 18 alone | **0 of 200** | 0 (every failure a real divergence) |
|
||||
| neither | 200 of 200 | 0 |
|
||||
- **A stale exemption**: `("quit", "stale")` listed in `OUTSIDE_THE_CONTEXT`.
|
||||
It fails as "listed … but did not".
|
||||
|
||||
@@ -5614,3 +5716,51 @@ store is exactly `{Clothing/Jan/Revenue = 1}`. Under the rival the clear lands
|
||||
on `Food/Jan/Profit`, where there is nothing to clear, and
|
||||
`Clothing/Jan/Profit = 5` survives. Injected alone into a full workspace run,
|
||||
the rival fails this test and nothing else.
|
||||
|
||||
### a_fresh_sessions_slice_is_the_hosts_startup_slice
|
||||
|
||||
improvise-v8ab. A fresh session's slice is built twice: `App::with_storage`
|
||||
builds the host's own, and `project.rs::fresh_view` builds a client's. Nothing
|
||||
asserted they agree. `tests/lockstep.rs` seeds most of its clients from
|
||||
`app.view_state.clone()`, and those could not see improvise-lxg0's phantom
|
||||
frame or the next drift between the two. There are three exceptions.
|
||||
`switching_between_a_records_view_and_a_pivot_view_settles_both_ways` and
|
||||
`a_locally_resolved_page_turn_lands_where_the_host_would_put_it` go through
|
||||
`Host::connect`, and the `published_basis` helper uses `Sessions::connect`.
|
||||
None of them compares the connected slice with the host's. The test connects to two documents, both opening on `Second`, one with a `Default` view
|
||||
and one without. It asserts the whole `ViewState` equals the host's, not only
|
||||
the back stack, so any field the two constructors disagree on fails it. The
|
||||
rival is the old fold, restored in `fresh_view`. It failed on the
|
||||
`["Default", "Second"]` case: the only difference was `view_back_stack:
|
||||
[ViewFrame { view_name: "Default", mode: Normal }]`.
|
||||
|
||||
The scope is **connect only**. A rebase deliberately resets the client's whole
|
||||
slice, while `install_workbook` keeps the host's mode and panels, so equality
|
||||
is not the contract there. `lockstep.rs` still copies `app.view_state`, and
|
||||
that is deliberate. Its helpers seed a client *after* mutating the host (mode,
|
||||
buffers, cursor), so a `Snapshot` would drop exactly the state those tests
|
||||
stage. This test is what makes the copy faithful at startup.
|
||||
|
||||
### a_document_opening_on_a_records_view_opens_the_host_in_records_normal
|
||||
|
||||
improvise-y5eo. `App::with_storage` set `view_state.mode` to `Normal` and never
|
||||
settled it against the layout. The host settles only at the end of a batch
|
||||
(`settle_normal_mode` in `apply_effects`). A session's mirror settles in
|
||||
`project_diff`, so `Sessions::connect` handed a client `RecordsNormal` while
|
||||
the host sat in `Normal` over the same records grid. The host's first `i` then
|
||||
resolved through `Normal`'s keymap and entered plain `Editing`. The fixture's
|
||||
active view puts `_Index` on rows and `_Dim` on columns, which the `.improv`
|
||||
writer does persist. The test asserts the starting mode, then presses `i` and
|
||||
asserts the host is still in a records mode. The fix calls
|
||||
`settle_normal_mode` once at the end of construction, so the startup mode
|
||||
comes from the same rule every batch ends with, and so from the same
|
||||
`improvise_protocol::settled_mode` a client uses.
|
||||
|
||||
Rival injected: that one call removed. The whole workspace had exactly two
|
||||
failures. This test failed at the mode assertion (`left: Normal, right:
|
||||
RecordsNormal`), and `a_fresh_sessions_slice_is_the_hosts_startup_slice`
|
||||
failed on its new records case: the client's `RecordsNormal` against the
|
||||
host's `Normal`, and nothing else different. `install_workbook` needs no
|
||||
change of its own, because both of its callers (`LoadModel`,
|
||||
`AcceptWizardModel`) run inside `apply_effects`, which settles after the batch.
|
||||
|
||||
|
||||
@@ -719,3 +719,47 @@ every target-specific entry, whose **normal** serde_json dependency lacks
|
||||
The `checked.len() >= 2` assertion is the positive signal. If a cargo change
|
||||
renamed the metadata fields, the check would find no serde_json dependency and
|
||||
pass vacuously without it.
|
||||
|
||||
### The re-poll a second boundary could skip — `scripts/pr-reviews-test.sh` (improvise-qnpi)
|
||||
|
||||
**What broke.** "an inline comment arriving mid-wait is seen" failed once in
|
||||
two identical runs of `checkers.sh` on a loaded machine. `pr-reviews --wait`
|
||||
computed `deadline = now + WAIT` and then tested `now < deadline` *before* its
|
||||
first re-poll. With `--wait 1`, a second boundary falling between those two
|
||||
reads of the clock made the loop run zero times: exit 1 with one poll, whatever
|
||||
the second poll would have shown.
|
||||
|
||||
**Reproduced under load.** A loop running that fixture with one busy loop per
|
||||
core failed once, as `exit 1, polls=1`, around run 52; the fixed script ran
|
||||
200 times under the same load with no failure. That is evidence of the
|
||||
mechanism, not proof: the window is a few milliseconds per run.
|
||||
|
||||
**The case that proves it.** "a second passing before the first re-poll does
|
||||
not skip it" puts a `date` on `PATH` that advances one second per read (a
|
||||
loaded machine, compressed) and a no-op `sleep`. Against the old loop it fails
|
||||
every time, exit 1 (22 passed, 1 failed). The fix checks the deadline *after*
|
||||
each poll, so any `--wait` above 0 polls at least once more. A larger timeout
|
||||
would not have helped: the case uses the shimmed clock, where only the order of
|
||||
check and poll matters.
|
||||
|
||||
### The harness lock that every build rewrote — `harness/build.sh` (improvise-mkbp)
|
||||
|
||||
**What broke.** `harness/opfs-contract/Cargo.lock` pinned `pest 2.9.1` while
|
||||
its path dependencies require `pest = "2.9.2"`, so the committed lock was
|
||||
invalid for its own manifest and every cargo invocation rewrote it.
|
||||
|
||||
**Why both halves.** The resolved lock is committed, and `build.sh` now starts
|
||||
with `cargo metadata --locked` against it. The lock alone would repeat the bug
|
||||
at the next workspace dependency bump; `--locked` alone fails today. The check
|
||||
is its own step because `wasm-pack build … -- --locked` does not work: with the
|
||||
stale lock restored, that version exited 0 and still rewrote the lock, because
|
||||
wasm-pack re-resolves before the flag reaches `cargo build`. The standalone
|
||||
check, against the same stale lock, exits 101 in seconds and leaves the file
|
||||
untouched.
|
||||
|
||||
**The assemble half.** `web/assemble.sh` starts with `rm -rf` of its output,
|
||||
which is what keeps the deployed tree free of stale files, so it is not taught
|
||||
to spare a harness directory. `opfs.spec.js` instead asserts `/opfs/index.html`
|
||||
is served, naming the fix. Injected by running `web/assemble.sh harness/serve`
|
||||
alone: 4 chromium failures, each carrying that message, where it used to fail
|
||||
with no hint of the cause.
|
||||
|
||||
@@ -1,14 +1,14 @@
|
||||
//! A command is a function of its context: two contexts built independently from
|
||||
//! equal data give equal effects — `context/code/command.md`, improvise-hzjo.
|
||||
|
||||
use std::collections::{BTreeSet, HashSet};
|
||||
use std::collections::BTreeSet;
|
||||
|
||||
use super::core::{Cmd, CmdContext};
|
||||
use super::grid::DrillIntoCell;
|
||||
use super::registry::tests::candidate_args;
|
||||
use crate::command::key::{KeyCode, KeyModifiers};
|
||||
use crate::model::Model;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::model::record::record_of;
|
||||
use crate::ui::app::App;
|
||||
use crate::ui::effect::{self, Effect};
|
||||
|
||||
@@ -61,27 +61,23 @@ fn one_coordinate_drills(model: &Model) -> Vec<Vec<String>> {
|
||||
drills
|
||||
}
|
||||
|
||||
/// How many distinct records the widest all-`Cat/Item` candidate matches.
|
||||
fn widest_drill(model: &Model, candidates: &[Vec<String>]) -> usize {
|
||||
let coordinates = |args: &Vec<String>| -> Option<Vec<(String, String)>> {
|
||||
args.iter()
|
||||
.map(|a| {
|
||||
a.split_once('/')
|
||||
.map(|(c, i)| (c.to_string(), i.to_string()))
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
candidates
|
||||
.iter()
|
||||
.filter(|args| !args.is_empty())
|
||||
.filter_map(coordinates)
|
||||
.map(|partial| {
|
||||
let cells = model.data.matching_cells(&partial);
|
||||
let records: HashSet<CellKey> = cells.iter().map(|(k, _)| record_of(k)).collect();
|
||||
records.len()
|
||||
})
|
||||
.max()
|
||||
.unwrap_or(0)
|
||||
/// Records a compared drill froze, counted only if it narrowed the store and its
|
||||
/// ordered records are in `compared`, the text the law compared (improvise-btyw).
|
||||
fn records_narrowed(scratch: &mut App, effects: Vec<Box<dyn Effect>>, compared: &str) -> usize {
|
||||
let store = scratch
|
||||
.model_state
|
||||
.workbook
|
||||
.model
|
||||
.data
|
||||
.live_records()
|
||||
.count();
|
||||
scratch.apply_effects(effects);
|
||||
let frozen = scratch
|
||||
.drill_state
|
||||
.as_ref()
|
||||
.filter(|drill| compared.contains(&format!("{:?}", drill.records)))
|
||||
.map_or(0, |drill| drill.records.len());
|
||||
if frozen < store { frozen } else { 0 }
|
||||
}
|
||||
|
||||
/// The registry's candidates, plus names the demo really has: a drill or a fit
|
||||
@@ -108,9 +104,9 @@ fn args_the_document_names(model: &Model) -> Vec<Vec<String>> {
|
||||
all
|
||||
}
|
||||
|
||||
fn effects_of(name: &str, args: &[String], ctx: &CmdContext) -> Option<String> {
|
||||
fn effects_of(name: &str, args: &[String], ctx: &CmdContext) -> Option<Vec<Box<dyn Effect>>> {
|
||||
let cmd = ctx.registry.interactive(name, args, ctx).ok()?;
|
||||
Some(format!("{:?}", cmd.execute(ctx)))
|
||||
Some(cmd.execute(ctx))
|
||||
}
|
||||
|
||||
/// What running every registered command under `a` and under each of `others` found.
|
||||
@@ -120,20 +116,34 @@ struct Comparison {
|
||||
shown: Vec<String>,
|
||||
never_built: Vec<&'static str>,
|
||||
compared: usize,
|
||||
widest_drill: usize,
|
||||
}
|
||||
|
||||
fn compare(a: &CmdContext, others: &[CmdContext]) -> Comparison {
|
||||
let mut found = Comparison::default();
|
||||
let candidates = args_the_document_names(a.model);
|
||||
let drill = DrillIntoCell {
|
||||
key: CellKey::new(vec![]),
|
||||
};
|
||||
let mut scratch = fixture();
|
||||
for name in a.registry.names().collect::<Vec<_>>() {
|
||||
let before = found.compared;
|
||||
for args in &candidates {
|
||||
let Some(left) = effects_of(name, args, a) else {
|
||||
let Some(effects) = effects_of(name, args, a) else {
|
||||
continue;
|
||||
};
|
||||
let left = format!("{effects:?}");
|
||||
if name == drill.name() {
|
||||
found.widest_drill =
|
||||
found
|
||||
.widest_drill
|
||||
.max(records_narrowed(&mut scratch, effects, &left));
|
||||
}
|
||||
found.compared += 1;
|
||||
for b in others {
|
||||
let right = effects_of(name, args, b).unwrap_or_default();
|
||||
let right = effects_of(name, args, b)
|
||||
.map(|e| format!("{e:?}"))
|
||||
.unwrap_or_default();
|
||||
if left != right {
|
||||
found.diverged.insert(name);
|
||||
found
|
||||
@@ -157,13 +167,12 @@ fn every_command_gives_equal_effects_in_independently_built_equal_contexts() {
|
||||
.iter()
|
||||
.map(|app| app.cmd_context(KeyCode::Null, KeyModifiers::NONE))
|
||||
.collect();
|
||||
let model = contexts[0].model;
|
||||
let widest = widest_drill(model, &args_the_document_names(model));
|
||||
assert!(
|
||||
widest >= 4,
|
||||
"the widest drill orders {widest} records, so the store's order is nearly a coin toss"
|
||||
);
|
||||
let found = compare(&contexts[0], &contexts[1..]);
|
||||
assert!(
|
||||
found.widest_drill >= 4,
|
||||
"the widest narrowing drill compared froze {} records, so the store's order is nearly a coin toss",
|
||||
found.widest_drill
|
||||
);
|
||||
assert!(
|
||||
found.never_built.is_empty(),
|
||||
"no candidate args build these in the fixture, so the law never ran them: {:?}",
|
||||
|
||||
@@ -386,7 +386,9 @@ impl App {
|
||||
let view = workbook.active_view();
|
||||
GridLayout::with_frozen_records(&workbook.model, view, None)
|
||||
};
|
||||
Self {
|
||||
// A document may open on a records grid, so the first mode is settled
|
||||
// by the rule every batch ends with (improvise-y5eo).
|
||||
let mut app = Self {
|
||||
model_state: ModelState {
|
||||
workbook,
|
||||
file_path,
|
||||
@@ -416,7 +418,9 @@ impl App {
|
||||
last_reframed_elsewhere: std::collections::BTreeSet::new(),
|
||||
keymap_set: KeymapSet::default_keymaps(),
|
||||
storage,
|
||||
}
|
||||
};
|
||||
app.settle_normal_mode();
|
||||
app
|
||||
}
|
||||
|
||||
/// This client's cursor in the active view. Absent means the origin, so
|
||||
@@ -2758,6 +2762,29 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-y5eo: a document that opens on a records grid opens the host in
|
||||
/// `RecordsNormal`. See `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_document_opening_on_a_records_view_opens_the_host_in_records_normal() {
|
||||
use crate::view::Axis;
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_category("Region").unwrap();
|
||||
wb.add_item("Region", "East").unwrap();
|
||||
wb.active_view_mut().set_axis("_Index", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Dim", Axis::Column);
|
||||
let mut app = App::new(wb, None);
|
||||
assert!(app.layout.is_records_mode(), "setup: the grid is records");
|
||||
assert_eq!(app.view_state.mode, AppMode::RecordsNormal);
|
||||
|
||||
app.handle_key(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE))
|
||||
.unwrap();
|
||||
assert!(
|
||||
app.view_state.mode.is_records(),
|
||||
"the host's first `i` left records mode: {:?}",
|
||||
app.view_state.mode
|
||||
);
|
||||
}
|
||||
|
||||
/// The settle's `RecordsNormal` arm, driven from the mode it names. The
|
||||
/// `:` crossing above enters the settle in `Normal` both ways — the evidence.
|
||||
#[test]
|
||||
@@ -3001,16 +3028,34 @@ mod tests {
|
||||
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")] {
|
||||
// One case per `refuse_and_stop` site (improvise-514o).
|
||||
let cases: [(&str, &[&str], &str); 7] = [
|
||||
("load", &["absent.improv"], "Load error"),
|
||||
("import-json", &["absent.json"], "Cannot read 'absent.json'"),
|
||||
("import-json", &["absent.csv"], "Cannot read 'absent.csv'"),
|
||||
("import-json", &["bad.csv"], "CSV error"),
|
||||
("import-json", &["bad.json"], "JSON parse error"),
|
||||
(
|
||||
"import-json",
|
||||
&["nested.json", "M", "nope"],
|
||||
"No array at path",
|
||||
),
|
||||
("import-json", &["measures.json"], "Import error"),
|
||||
];
|
||||
for (command, args, arm) in cases {
|
||||
let path = args.join(" ");
|
||||
let mut app = App::new(Workbook::new("T"), None);
|
||||
app.storage = Box::new(crate::storage::MemoryFs::new());
|
||||
app.storage = Box::new(crate::ui::host::import_fixture_storage());
|
||||
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![])],
|
||||
vec![
|
||||
(command, args.iter().map(|a| a.to_string()).collect()),
|
||||
("force-quit", vec![]),
|
||||
],
|
||||
);
|
||||
let effects = {
|
||||
let ctx = app.cmd_context(key, mods);
|
||||
@@ -3020,8 +3065,8 @@ mod tests {
|
||||
app.apply_effects(effects);
|
||||
|
||||
assert!(
|
||||
app.last_batch_refused(),
|
||||
"setup: `{command}` must actually have failed, got {:?}",
|
||||
app.last_batch_refused() && app.view_state.status_msg.contains(arm),
|
||||
"setup: `{command} {path}` must fail in the {arm:?} arm, got {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
assert_ne!(
|
||||
@@ -3032,6 +3077,392 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-v4hg: Enter on a formula that does not parse threw the typed text
|
||||
/// away. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text() {
|
||||
let type_and_commit = |text: &str| {
|
||||
let mut app = two_col_model();
|
||||
app.view_state.mode = AppMode::FormulaPanel;
|
||||
let press = |app: &mut App, code| {
|
||||
app.handle_key(KeyEvent::new(code, KeyModifiers::NONE))
|
||||
.unwrap()
|
||||
};
|
||||
press(&mut app, KeyCode::Char('a'));
|
||||
assert!(
|
||||
matches!(app.view_state.mode, AppMode::FormulaEdit { .. }),
|
||||
"setup: `a` in the formula panel opens the editor"
|
||||
);
|
||||
for c in text.chars() {
|
||||
press(&mut app, KeyCode::Char(c));
|
||||
}
|
||||
press(&mut app, KeyCode::Enter);
|
||||
app
|
||||
};
|
||||
|
||||
// The control: a formula that parses is stored and the editor closes.
|
||||
let ok = type_and_commit("Total = 1");
|
||||
assert_eq!(ok.view_state.mode, AppMode::FormulaPanel);
|
||||
assert!(
|
||||
ok.model_state
|
||||
.workbook
|
||||
.model
|
||||
.formulas()
|
||||
.iter()
|
||||
.any(|f| f.target == "Total"),
|
||||
"the control formula must land"
|
||||
);
|
||||
|
||||
let bad = type_and_commit("Total = (");
|
||||
assert!(
|
||||
bad.last_batch_refused() && bad.view_state.status_msg.contains("Formula error"),
|
||||
"the parse error must be reported, got {:?}",
|
||||
bad.view_state.status_msg
|
||||
);
|
||||
assert!(
|
||||
matches!(bad.view_state.mode, AppMode::FormulaEdit { .. }),
|
||||
"a refused formula must leave the user in the editor, got {:?}",
|
||||
bad.view_state.mode
|
||||
);
|
||||
assert_eq!(
|
||||
bad.view_state.buffers.get("formula").map(String::as_str),
|
||||
Some("Total = ("),
|
||||
"and must keep what they typed"
|
||||
);
|
||||
}
|
||||
|
||||
/// Run a `:` line the way `improvise cmd` does, one batch per segment; true
|
||||
/// when any segment refused, which is what its exit code reads.
|
||||
fn run_line(app: &mut App, line: &str) -> bool {
|
||||
let mut refused = false;
|
||||
for segment in crate::command::parse::segments(line) {
|
||||
let effects = {
|
||||
let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
crate::command::parse::parse_segment_interactive(segment, &ctx)
|
||||
.unwrap_or_else(|e| panic!("`{segment}` must parse: {e}"))
|
||||
.map(|cmd| cmd.execute(&ctx))
|
||||
.unwrap_or_default()
|
||||
};
|
||||
app.apply_effects(effects);
|
||||
refused |= app.last_batch_refused();
|
||||
}
|
||||
refused
|
||||
}
|
||||
|
||||
/// Commit `text` in a minibuffer mode with Enter; true when that batch refused.
|
||||
fn commit_in(app: &mut App, mode: AppMode, buffer: &str, text: &str) -> bool {
|
||||
app.view_state.mode = mode;
|
||||
app.view_state
|
||||
.buffers
|
||||
.insert(buffer.to_string(), text.to_string());
|
||||
app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE))
|
||||
.unwrap();
|
||||
app.last_batch_refused()
|
||||
}
|
||||
|
||||
fn with_import_fixtures(mut app: App) -> App {
|
||||
app.storage = Box::new(crate::ui::host::import_fixture_storage());
|
||||
app
|
||||
}
|
||||
|
||||
/// Eleven regular categories: one short of the limit.
|
||||
fn one_category_short_of_the_limit() -> App {
|
||||
let mut wb = Workbook::new("T");
|
||||
for n in 0..11 {
|
||||
wb.add_category(format!("C{n}")).unwrap();
|
||||
}
|
||||
App::new(wb, None)
|
||||
}
|
||||
|
||||
/// Stage one coordinate rename in a drill over the records fixture, then commit it.
|
||||
fn commit_drill_rename(app: &mut App, to: &str) -> bool {
|
||||
let records = app.layout.records.clone().expect("records mode");
|
||||
let mut pending_edits = HashMap::new();
|
||||
pending_edits.insert(
|
||||
(1, crate::view::RecordCol::Category("Region".into())),
|
||||
to.into(),
|
||||
);
|
||||
app.drill_state = Some(DrillState {
|
||||
records,
|
||||
pending_edits,
|
||||
});
|
||||
app.apply_effects(vec![Box::new(crate::ui::effect::ApplyAndClearDrill)]);
|
||||
app.last_batch_refused()
|
||||
}
|
||||
|
||||
/// Records mode with `Region` paged to `page`: `add-record-row` then names
|
||||
/// the row whose coordinates are exactly that page.
|
||||
fn records_paged_to(app: &mut App, page: &str) {
|
||||
app.model_state.workbook.add_item("Region", "West").unwrap();
|
||||
let view = app
|
||||
.view_mut_of(&ViewTarget::Active)
|
||||
.expect("the active view");
|
||||
view.set_axis("Region", crate::view::Axis::Page);
|
||||
view.set_page_selection("Region", page);
|
||||
app.rebuild_layout();
|
||||
}
|
||||
|
||||
/// A grid with a regular category on None, so every cell is an aggregate.
|
||||
fn aggregated_grid(with_items: bool) -> App {
|
||||
let mut wb = Workbook::new("T");
|
||||
for cat in ["Row", "Col", "Hidden"] {
|
||||
wb.add_category(cat).unwrap();
|
||||
if with_items {
|
||||
wb.add_item(cat, "A").unwrap();
|
||||
}
|
||||
}
|
||||
wb.active_view_mut()
|
||||
.set_axis("Hidden", crate::view::Axis::None);
|
||||
let mut app = App::new(wb, None);
|
||||
app.rebuild_layout();
|
||||
app
|
||||
}
|
||||
|
||||
/// `Region` down, `_Measure` across: `Revenue` at column 0, then `Total`,
|
||||
/// which a formula owns, at column 1.
|
||||
fn a_formula_column() -> App {
|
||||
let mut wb = Workbook::new("T");
|
||||
wb.add_category("Region").unwrap();
|
||||
wb.add_item("Region", "North").unwrap();
|
||||
wb.add_item("_Measure", "Revenue").unwrap();
|
||||
wb.active_view_mut()
|
||||
.set_axis("_Measure", crate::view::Axis::Column);
|
||||
let mut app = App::new(wb, None);
|
||||
app.apply_effects(vec![Box::new(crate::ui::effect::AddFormula {
|
||||
raw: "Total = Revenue * 2".into(),
|
||||
target_category: "_Measure".into(),
|
||||
})]);
|
||||
app
|
||||
}
|
||||
|
||||
fn with_a_cell(mut app: App) -> App {
|
||||
app.model_state.workbook.model.set_cell(
|
||||
CellKey::new(vec![("Row".into(), "A".into()), ("Col".into(), "X".into())]),
|
||||
CellValue::Number(42.0),
|
||||
);
|
||||
app.rebuild_layout();
|
||||
app
|
||||
}
|
||||
|
||||
fn with_a_second_view(mut app: App) -> App {
|
||||
app.model_state.workbook.create_view("Second");
|
||||
app
|
||||
}
|
||||
|
||||
fn switch_to_second(app: &mut App) {
|
||||
app.apply_effects(vec![Box::new(crate::ui::effect::SwitchView(
|
||||
"Second".into(),
|
||||
))]);
|
||||
}
|
||||
|
||||
/// improvise-0v8l: one case per refusal site no other test pinned, each with a
|
||||
/// succeeding twin. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn every_refusal_site_refuses_its_failure_and_only_its_failure() {
|
||||
type Setup = fn() -> App;
|
||||
type Drive = fn(&mut App) -> bool;
|
||||
let cases: Vec<(&str, Setup, Drive, &str, Setup, Drive)> = vec![
|
||||
(
|
||||
"add-category past the limit",
|
||||
one_category_short_of_the_limit,
|
||||
|app| run_line(app, "add-category A . add-category B"),
|
||||
"Maximum of 12 categories",
|
||||
one_category_short_of_the_limit,
|
||||
|app| run_line(app, "add-category A"),
|
||||
),
|
||||
(
|
||||
"a drill commit that empties a coordinate",
|
||||
records_app_for_drill,
|
||||
|app| commit_drill_rename(app, ""),
|
||||
crate::ui::effect::RECORD_COORDS_CANNOT_BE_EMPTY,
|
||||
records_app_for_drill,
|
||||
|app| commit_drill_rename(app, "West"),
|
||||
),
|
||||
(
|
||||
"import of a JSON that does not parse",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import bad.json"),
|
||||
"JSON parse error",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import good.json"),
|
||||
),
|
||||
(
|
||||
"import of an absent file",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import absent.json"),
|
||||
"Cannot read file",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import good.json"),
|
||||
),
|
||||
(
|
||||
"import-json of an absent CSV",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json absent.csv"),
|
||||
"Cannot read 'absent.csv'",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json good.json"),
|
||||
),
|
||||
(
|
||||
"import-json of a CSV that is not text",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json bad.csv"),
|
||||
"CSV error",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json good.json"),
|
||||
),
|
||||
(
|
||||
"import-json of JSON that does not parse",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json bad.json"),
|
||||
"JSON parse error",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json good.json"),
|
||||
),
|
||||
(
|
||||
"import-json at a path with no array",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json nested.json M nope"),
|
||||
"No array at path",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json good.json"),
|
||||
),
|
||||
(
|
||||
"import-json that builds no model",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json measures.json"),
|
||||
"Import error",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| run_line(app, "import-json good.json"),
|
||||
),
|
||||
(
|
||||
"a records coordinate edited to nothing",
|
||||
records_model_with_two_rows,
|
||||
|app| {
|
||||
app.cursor_mut().selected = (1, 0);
|
||||
commit_in(app, AppMode::records_editing(), "edit", "")
|
||||
},
|
||||
crate::ui::effect::RECORD_COORDS_CANNOT_BE_EMPTY,
|
||||
records_model_with_two_rows,
|
||||
|app| {
|
||||
app.cursor_mut().selected = (1, 0);
|
||||
commit_in(app, AppMode::records_editing(), "edit", "West")
|
||||
},
|
||||
),
|
||||
(
|
||||
"an export with no path",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| commit_in(app, AppMode::export_prompt(), "export", ""),
|
||||
"not exported",
|
||||
|| with_import_fixtures(two_col_model()),
|
||||
|app| commit_in(app, AppMode::export_prompt(), "export", "out.csv"),
|
||||
),
|
||||
(
|
||||
"view-forward with nowhere to go",
|
||||
|| with_a_second_view(two_col_model()),
|
||||
|app| run_line(app, "view-forward"),
|
||||
"No forward view",
|
||||
|| with_a_second_view(two_col_model()),
|
||||
|app| {
|
||||
switch_to_second(app);
|
||||
run_line(app, "view-back");
|
||||
run_line(app, "view-forward")
|
||||
},
|
||||
),
|
||||
(
|
||||
"view-back with nowhere to go",
|
||||
|| with_a_second_view(two_col_model()),
|
||||
|app| run_line(app, "view-back"),
|
||||
"No previous view",
|
||||
|| with_a_second_view(two_col_model()),
|
||||
|app| {
|
||||
switch_to_second(app);
|
||||
run_line(app, "view-back")
|
||||
},
|
||||
),
|
||||
(
|
||||
"add-record-row outside records mode",
|
||||
two_col_model,
|
||||
|app| run_line(app, "add-record-row"),
|
||||
"only works in records mode",
|
||||
records_model_with_two_rows,
|
||||
|app| run_line(app, "add-record-row"),
|
||||
),
|
||||
(
|
||||
"add-record-row onto a row that is already there",
|
||||
records_model_with_two_rows,
|
||||
|app| {
|
||||
records_paged_to(app, "South");
|
||||
run_line(app, "add-record-row")
|
||||
},
|
||||
"already here",
|
||||
records_model_with_two_rows,
|
||||
|app| {
|
||||
records_paged_to(app, "West");
|
||||
run_line(app, "add-record-row")
|
||||
},
|
||||
),
|
||||
(
|
||||
"edit-or-drill on an aggregate grid with no cell",
|
||||
|| aggregated_grid(false),
|
||||
|app| run_line(app, "edit-or-drill editing"),
|
||||
"No cell at cursor",
|
||||
|| aggregated_grid(true),
|
||||
|app| run_line(app, "edit-or-drill editing"),
|
||||
),
|
||||
(
|
||||
"edit-or-drill on a cell a formula owns",
|
||||
a_formula_column,
|
||||
|app| {
|
||||
app.cursor_mut().selected = (0, 1);
|
||||
run_line(app, "edit-or-drill editing")
|
||||
},
|
||||
"computed by a formula",
|
||||
a_formula_column,
|
||||
|app| {
|
||||
app.cursor_mut().selected = (0, 0);
|
||||
run_line(app, "edit-or-drill editing")
|
||||
},
|
||||
),
|
||||
(
|
||||
"a search with no matches",
|
||||
|| with_a_cell(two_col_model()),
|
||||
|app| {
|
||||
app.view_state.search_query = "zzz".into();
|
||||
run_line(app, "search-navigate forward")
|
||||
},
|
||||
"No matches",
|
||||
|| with_a_cell(two_col_model()),
|
||||
|app| {
|
||||
app.view_state.search_query = "42".into();
|
||||
run_line(app, "search-navigate forward")
|
||||
},
|
||||
),
|
||||
(
|
||||
"a : line that does not parse",
|
||||
two_col_model,
|
||||
|app| commit_in(app, AppMode::command_mode(), "command", "no-such-command"),
|
||||
":no-such-command",
|
||||
two_col_model,
|
||||
|app| commit_in(app, AppMode::command_mode(), "command", "add-category Z"),
|
||||
),
|
||||
];
|
||||
for (what, fail_setup, fails, says, ok_setup, succeeds) in cases {
|
||||
let mut app = fail_setup();
|
||||
let refused = fails(&mut app);
|
||||
assert!(
|
||||
refused && app.view_state.status_msg.contains(says),
|
||||
"{what}: expected a refusal saying {says:?}, got refused={refused} {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
let mut app = ok_setup();
|
||||
assert!(
|
||||
!succeeds(&mut app),
|
||||
"{what}: its succeeding twin was refused: {:?}",
|
||||
app.view_state.status_msg
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// 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]
|
||||
|
||||
@@ -47,8 +47,8 @@ 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).
|
||||
/// Refuse AND stop the batch, where what follows assumes the change happened:
|
||||
/// `load x . q` must not quit, a bad formula must not close its editor (cz8d).
|
||||
fn refuse_and_stop(app: &mut App, why: impl Into<String>) {
|
||||
app.refuse(why);
|
||||
app.abort_effects = true;
|
||||
@@ -221,9 +221,9 @@ impl Effect for AddFormula {
|
||||
}
|
||||
app.model_state.workbook.model.add_formula(formula);
|
||||
}
|
||||
Err(e) => {
|
||||
app.refuse(format!("Formula error: {e}"));
|
||||
}
|
||||
// Stop too: what follows it on the key path closes the editor and
|
||||
// clears the buffer, which would throw the user's text away (v4hg).
|
||||
Err(e) => refuse_and_stop(app, format!("Formula error: {e}")),
|
||||
}
|
||||
}
|
||||
fn kind(&self) -> EffectKind {
|
||||
|
||||
@@ -266,6 +266,27 @@ impl Host {
|
||||
}
|
||||
}
|
||||
|
||||
/// One file per way `ImportJsonHeadless` can fail, plus one that imports; the
|
||||
/// `absent.*` paths are left unwritten. Shared by the frame and key-path tests.
|
||||
#[cfg(test)]
|
||||
pub(crate) fn import_fixture_storage() -> crate::storage::MemoryFs {
|
||||
let storage = crate::storage::MemoryFs::new();
|
||||
let files: [(&str, &[u8]); 5] = [
|
||||
// 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"),
|
||||
("good.json", br#"[{"Region": "North", "Revenue": 1}]"#),
|
||||
];
|
||||
for (name, bytes) in files {
|
||||
crate::storage::Storage::write(&storage, std::path::Path::new(name), bytes)
|
||||
.expect("fixture writes");
|
||||
}
|
||||
storage
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::path::{Path, PathBuf};
|
||||
@@ -1946,19 +1967,7 @@ mod tests {
|
||||
/// 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");
|
||||
}
|
||||
let storage = super::import_fixture_storage();
|
||||
// A second view, so deleting an absent one is not refused as "the last".
|
||||
let mut wb = workbook();
|
||||
wb.create_view("Scratch");
|
||||
|
||||
@@ -1603,6 +1603,32 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-v8ab: a fresh client starts from the host's own startup slice.
|
||||
/// See `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_fresh_sessions_slice_is_the_hosts_startup_slice() {
|
||||
for (names, records) in [
|
||||
(&["Default", "Second"][..], false),
|
||||
(&["Main", "Second"][..], false),
|
||||
(&["Main", "Second"][..], true),
|
||||
] {
|
||||
let mut wb = opening_on(names, "Second");
|
||||
if records {
|
||||
// `Second` opens on a records grid (improvise-y5eo).
|
||||
wb.active_view_mut().set_axis("_Index", Axis::Row);
|
||||
wb.active_view_mut().set_axis("_Dim", Axis::Column);
|
||||
}
|
||||
let app = App::new(wb, None);
|
||||
assert_eq!(app.view_state.active_view, "Second", "setup: {names:?}");
|
||||
assert_eq!(app.layout.is_records_mode(), records, "setup: {names:?}");
|
||||
let (_, snapshot) = Sessions::default().connect(&app, 10, 10);
|
||||
assert_eq!(
|
||||
snapshot.view, app.view_state,
|
||||
"{names:?}, records {records}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-lxg0: a document swap reseeds through the same door.
|
||||
#[test]
|
||||
fn a_rebased_session_has_no_history_to_step_back_through() {
|
||||
|
||||
@@ -9,10 +9,21 @@
|
||||
# half is the *deployed* arrangement rather than `web/` itself, because a
|
||||
# harness that runs against a different tree from the one that ships is
|
||||
# testing something nobody deploys (improvise-vvl3).
|
||||
#
|
||||
# Re-run THIS script, never `web/assemble.sh harness/serve` alone: assembling
|
||||
# starts with `rm -rf` of its output, which deletes opfs/ and the fixtures
|
||||
# (improvise-mkbp). opfs.spec.js says so if it finds opfs/ missing.
|
||||
set -eu
|
||||
root=$(cd "$(dirname "$0")/.." && pwd)
|
||||
serve="$root/harness/serve"
|
||||
|
||||
# A stale opfs-contract/Cargo.lock fails HERE rather than being rewritten in
|
||||
# every agent's tree (improvise-mkbp). wasm-pack cannot enforce this: it
|
||||
# re-resolves the lock itself before any `-- --locked` reaches cargo build.
|
||||
# Fix a failure with `cargo update -w` in harness/opfs-contract, and commit it.
|
||||
cargo metadata --locked --format-version 1 \
|
||||
--manifest-path "$root/harness/opfs-contract/Cargo.toml" >/dev/null
|
||||
|
||||
"$root/web/build.sh" >/dev/null
|
||||
"$root/web/assemble.sh" "$serve" harness >/dev/null
|
||||
|
||||
|
||||
Generated
+8
-8
@@ -308,9 +308,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "pest"
|
||||
version = "2.9.1"
|
||||
version = "2.9.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "6d45aeb61b4bf818e12d4205f2466f8c4748f85f4fce0146d1c03d69d753f0ad"
|
||||
checksum = "45d3aca230fad2e6f6317ca0a72724338c4960cb97168a85cdee66df4a9a21a8"
|
||||
dependencies = [
|
||||
"memchr",
|
||||
"ucd-trie",
|
||||
@@ -318,9 +318,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "pest_derive"
|
||||
version = "2.9.1"
|
||||
version = "2.9.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "89cc5a242e25ed4e7704d0be240f2cfbe20a8c27e7e252d94835be93d92dc39f"
|
||||
checksum = "284b60557f2c4a2e72ad3f2d34d42685a2fa4a6a61d0d2a10c0ae2a5e916c2cf"
|
||||
dependencies = [
|
||||
"pest",
|
||||
"pest_generator",
|
||||
@@ -328,9 +328,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "pest_generator"
|
||||
version = "2.9.1"
|
||||
version = "2.9.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "7abf21475cc3820fe4b2ca2dc2142902f67a02189f3b5b3a229f4febc01a43e5"
|
||||
checksum = "1d9d1f08a115309ee99268cf85e5228e0e56aa9caf8841ec12866b6be07c3109"
|
||||
dependencies = [
|
||||
"pest",
|
||||
"pest_meta",
|
||||
@@ -341,9 +341,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "pest_meta"
|
||||
version = "2.9.1"
|
||||
version = "2.9.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "adba4db388f687393c18c51348d44a41d870ca9df71a2c98172ea3035dc6936e"
|
||||
checksum = "ed93ba1a9ffcca32130a5188701c81c0c49cf00d4b7c5007d5148951d743adcb"
|
||||
dependencies = [
|
||||
"pest",
|
||||
]
|
||||
|
||||
@@ -23,7 +23,11 @@ const EXPECTED = [
|
||||
|
||||
test.describe("OpfsFs in a real browser", () => {
|
||||
test.beforeEach(async ({ page, browserName }, testInfo) => {
|
||||
await page.goto("/opfs/index.html");
|
||||
const served = await page.goto("/opfs/index.html");
|
||||
expect(
|
||||
served?.ok(),
|
||||
"harness/serve/opfs is missing: re-run harness/build.sh, not web/assemble.sh alone",
|
||||
).toBe(true);
|
||||
|
||||
// The skip is conditional on the PROBE, not on the browser name, so it
|
||||
// cannot quietly swallow a real regression. See clients.md.
|
||||
|
||||
+6
-2
@@ -9,7 +9,8 @@ usage() {
|
||||
usage: pr-reviews <pr-number> [--wait <sec>] [--interval <sec>]
|
||||
|
||||
--wait <sec> wait up to this long for a review or an inline review
|
||||
comment whose commit is the current head
|
||||
comment whose commit is the current head; any --wait
|
||||
above 0 polls at least once more after the first read
|
||||
(default 0 = print what exists and exit)
|
||||
--interval <sec> seconds between polls (default 30)
|
||||
|
||||
@@ -157,8 +158,10 @@ report "$json"
|
||||
|
||||
if has_head_review "$json"; then exit 0; fi
|
||||
|
||||
# The deadline is checked AFTER each poll: checked first, a second ticking over
|
||||
# between two reads of the clock skipped the only re-poll (improvise-qnpi).
|
||||
deadline=$(( $(date +%s) + WAIT ))
|
||||
while [ "$(date +%s)" -lt "$deadline" ]; do
|
||||
while :; do
|
||||
sleep "$INTERVAL"
|
||||
json=$(fetch)
|
||||
if has_head_review "$json"; then
|
||||
@@ -166,6 +169,7 @@ while [ "$(date +%s)" -lt "$deadline" ]; do
|
||||
report "$json"
|
||||
exit 0
|
||||
fi
|
||||
[ "$(date +%s)" -lt "$deadline" ] || break
|
||||
done
|
||||
|
||||
printf '\npr-reviews: waited %ss; nothing against the current head.\n' "$WAIT"
|
||||
|
||||
@@ -197,6 +197,26 @@ mk "$d" last.issue.json <<'EOF'
|
||||
EOF
|
||||
run_case "an inline comment arriving mid-wait is seen" 0 "$d" --wait 1 --interval 1
|
||||
|
||||
# The same fixture on a clock that ticks one second per read: a loaded machine,
|
||||
# compressed. A second passing between two reads must not cost the only poll.
|
||||
clock="$work/clock"
|
||||
mkdir -p "$clock"
|
||||
cat > "$clock/date" <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
[ "${1:-}" = "+%s" ] || exec /bin/date "$@"
|
||||
n=$(( $(cat "$CLOCK_TICKS" 2>/dev/null || echo 0) + 1 ))
|
||||
echo "$n" > "$CLOCK_TICKS"
|
||||
echo $(( 1000 + n ))
|
||||
EOF
|
||||
printf '#!/usr/bin/env bash\nexit 0\n' > "$clock/sleep"
|
||||
chmod +x "$clock/date" "$clock/sleep"
|
||||
rm -f "$d/.poll" "$work/ticks"
|
||||
CLOCK_TICKS="$work/ticks" PATH="$clock:$PATH" PR_REVIEWS_FIXTURE_DIR="$d" \
|
||||
"$REVIEWS" 1 --wait 1 --interval 1 > "$work/out" 2>&1
|
||||
got=$?
|
||||
[ "$got" = 0 ] && ok "a second passing before the first re-poll does not skip it" \
|
||||
|| bad "a second passing before the first re-poll does not skip it" "exit $got, wanted 0"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# `gh api --paginate` emits one array PER PAGE, so a thread past the page size
|
||||
# arrives as several concatenated documents. Passing that straight to
|
||||
|
||||
Reference in New Issue
Block a user