Merge commit 'e406edc8e846f147dc0828f7087ab2a4008a41c9' into fix/we1a-abort-refuses
# Conflicts: # context/test-evidence/command.md
This commit is contained in:
@@ -5269,6 +5269,97 @@ 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.
|
||||
|
||||
The drilled paste now declines at the cursor door (`value_cell_key`) before
|
||||
`PasteCell` runs, so the status carries the registry's `paste:` prefix and a
|
||||
wording shared with `clear-cell`. The assertion was re-aimed from equality with
|
||||
`PASTE_INSIDE_DRILL` to `ends_with(INSIDE_A_DRILL_REFUSAL)` (improvise-gw18's
|
||||
doors). The advice half is unchanged.
|
||||
|
||||
### drilling_into_a_fold_is_refused_and_o_after_it_writes_no_group_coordinate
|
||||
|
||||
improvise-gw18. On `gh/main` at `800348a`, `>` on the Q2 fold (row 2, column 1)
|
||||
opened a `_Drill` with no rows, and `o` in it stored `Month=Q2` in `## Data`. The
|
||||
test presses both keys through `App::handle_key`, then saves and reloads, and
|
||||
asserts that no coordinate names a non-item (`non_item_coordinates`, which asks
|
||||
the category, not the spelling). It failed on the first assertion: the drill
|
||||
opened. Rival: the registry's cursor fallback on the reading accessor
|
||||
(`cell_key`) in place of `unfolded_cell_key`. This test and
|
||||
`no_normal_mode_key_pressed_on_a_fold_stores_a_group_coordinate` fail. The
|
||||
control, `drilling_into_the_row_above_a_fold_still_opens_a_drill`, holds the
|
||||
refusal to the fold alone.
|
||||
|
||||
### no_normal_mode_key_pressed_on_a_fold_stores_a_group_coordinate
|
||||
|
||||
improvise-gw18's enumeration, made executable. Every key bound in Normal mode,
|
||||
prefix sub-keys included, is pressed on a fresh app with the cursor on a fold
|
||||
and a value yanked, and no stored coordinate may name a non-item. The sweep is
|
||||
**enumerated** rather than hand-picked, which is how it found `p` (the fold half
|
||||
of improvise-l4mp) without being told. The first run failed on
|
||||
`[(Char('p'), NONE)] on a fold stored [("Month", "Q2")]`. It asserts that the
|
||||
enumeration reaches `p`, so an empty sweep cannot pass. What it cannot see is a
|
||||
write two keys deep (`>` then `o`). That is why the drill test above exists. A
|
||||
two-key sweep was **estimated**, not measured, at several seconds and not built.
|
||||
|
||||
Widened after PR #197's review, which showed that `H` without its fold check
|
||||
passed the sweep: a ghost `hidden:` entry is not a stored coordinate. Every
|
||||
stroke must now also leave the `## Data` section and the view's hidden set
|
||||
exactly as they were. With the `H` guard disabled, this test fails as well as
|
||||
the yk2s one. What no sweep can see is `clear-cell` back on the reading
|
||||
accessor, because clearing a key nothing is stored at changes nothing. The
|
||||
lx7w tests and the fold lockstep case catch that.
|
||||
|
||||
### hiding_a_folds_row_is_refused_and_persists_no_hidden_group
|
||||
|
||||
improvise-yk2s. `H` on the fold emitted `HideItem { Month, "Q2" }`. `View`
|
||||
accepts an unknown name, so a ghost `hidden:` line was saved, with no visible
|
||||
change and no message. The test asserts no hidden entry, a message, and no
|
||||
`hidden:` line in the saved text. The control hides `Feb` one row up. Rival: the
|
||||
`row_folds_a_group` guard disabled (`if false && …`). Only this test fails. The
|
||||
guard asks the row half of `folds_a_group`, so an inner-level fold
|
||||
(`North/Q1`) is refused too, although `North` is a real item. That is a
|
||||
deliberate over-refusal, taken to keep one predicate.
|
||||
|
||||
### enter_advancing_onto_a_fold_commits_nothing_there
|
||||
|
||||
The write half of improvise-enz6, closed by the commit door. `i` on Feb, then
|
||||
`Enter`, which commits and advances onto the Q2 fold and re-opens the editor
|
||||
there. That entry is still enz6's open half. Then `7` and `Enter`. At the
|
||||
previous commit door (`cell_key`) the second commit stored `Month=Q2`. Rival:
|
||||
`commit_at_cursor` on `cell_key` again. This test and
|
||||
`tab_advancing_onto_a_formula_cell_commits_nothing_under_it` fail.
|
||||
|
||||
### x_in_records_mode_clears_the_real_record_under_the_cursor
|
||||
|
||||
improvise-lx7w. In records mode `x` built `ClearCell` from the synthetic
|
||||
`(_Index, _Dim)` key, which names nothing stored, so it was a silent no-op. The
|
||||
cursor is on West × Cost (row 1, column 3). The test asserts that West/Cost is
|
||||
gone, that East/Cost is untouched (so a clear of the wrong row cannot pass), and
|
||||
that nothing synthetic reaches the reloaded file. Rival: `clear-cell`'s cursor
|
||||
fallback on the old door. This test and the coordinate-column one fail.
|
||||
|
||||
### x_on_a_records_coordinate_column_changes_nothing_and_says_so
|
||||
|
||||
improvise-lx7w's decline arm, the answer #184 gave paste: a coordinate column
|
||||
stores no value, so `x` there changes nothing and says so. Before the fix it
|
||||
changed nothing *silently*, and it fails on the message.
|
||||
|
||||
### p_onto_a_formula_owned_cell_writes_nothing_and_names_the_formula
|
||||
|
||||
improvise-54g1. `p` on West × Profit (`Profit = Revenue - Cost`, row 1,
|
||||
column 2) stored `_Measure=Profit` under the formula. The recompute hid it, but
|
||||
it survived in `## Data`. The test asserts that the saved text has no Profit
|
||||
line, that the reload has no such cell, and that the status names the formula.
|
||||
The control, one column left, must write, so a door that refused everything
|
||||
cannot pass. Rival: the old door. It fails on the saved text.
|
||||
|
||||
### tab_advancing_onto_a_formula_cell_commits_nothing_under_it
|
||||
|
||||
A door the tickets did not list, found by working the enumeration. `i` declines
|
||||
to edit a formula's cell, but `Tab` from West/Cost commits and advances onto
|
||||
Profit, `EnterEditAtCursor` re-opens the editor there, and `Enter` stored
|
||||
`7` under the formula. Found by this test failing at `90b93d4`, before
|
||||
`committable_cell_key` existed. The rival is the one above.
|
||||
|
||||
### 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
|
||||
@@ -5621,6 +5712,31 @@ cannot see the feature removed from `improvise-command` alone, because
|
||||
`improvise-io`, a normal dependency, enables it in the same build. That case is
|
||||
held by `every_crate_that_can_parse_json_enables_float_roundtrip` (`tooling.md`).
|
||||
|
||||
### the_grid_and_a_csv_export_agree_on_a_fold_row
|
||||
|
||||
The projection-level pin PR #203's review asked to keep. The review's own
|
||||
scratch test is made permanent here. Q1 = {Jan, Feb, Mar} with March hidden and
|
||||
funded (50/50), so a fold that covered hidden members would read 90 / 58 / 0.36.
|
||||
`grid_of` (what a client is sent) and `export_csv_to` (what a file gets) must
|
||||
both read `40.00, 8.00, 0.80` on the Q1 row. The two go through different
|
||||
producers (`viewport::grid_body` and `render_csv`), and both reach
|
||||
`GridLayout::evaluate`. Rival: that evaluation covering every member. This test
|
||||
fails with `["90.00", "58.00", "0.36"]`.
|
||||
|
||||
### p_onto_a_sparse_record_writes_it
|
||||
|
||||
improvise-q9eg, a regression introduced by PR #197 and found in its review. In
|
||||
records mode a category is added (as `+cat` does), so every existing record
|
||||
lacks it and it sits on `Axis::None`. `yy` on East/Cost, then `p` on West/Cost
|
||||
(row 1): gh/main pasted 700, and #197 refused with the aggregate message.
|
||||
`value_cell_key` handed `PasteCell` the real record key, and `paste_into` then
|
||||
re-ran `aggregates(ctx, key)`. That key-shape check is a second derivation
|
||||
beside `cell_id`, and it reads a record missing a None-axis category as an
|
||||
aggregate. Every records fixture before this one was fully populated, which is
|
||||
why nothing saw it. The fix is `PasteAt::{Resolved, Spelled}`: a resolved key is
|
||||
written as it stands. Rival: routing `Resolved` back through `paste_into`. This
|
||||
test and `p_onto_a_sparse_record_writes_what_the_browser_writes` fail.
|
||||
|
||||
## `ui/effect.rs` — `RecordRewrite`'s untested branches (improvise-phg1)
|
||||
|
||||
### renaming_a_drill_row_held_by_a_lone_measureless_cell_moves_that_cell
|
||||
@@ -5724,6 +5840,124 @@ 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.
|
||||
|
||||
### `an_untouched_pivot_edit_of_a_misread_value_writes_nothing`
|
||||
### `an_untouched_records_edit_of_a_misread_value_writes_nothing`
|
||||
|
||||
**improvise-ixxx.** After umi4 the seed was exact, but the commit still ran it
|
||||
through `typed()`, which reads text and not the cell the text came from. So
|
||||
`i` then `Enter` with nothing typed stored `Number(7.0)` over `Text("007")`,
|
||||
and `Text("ERR:boom")` over a raw `Error("boom")`. The values are the ones
|
||||
where `typed(edit_text(v)) != v`: `"007"`, `"1e3"`, `"inf"`, `"2024"` and a raw
|
||||
`Error`. They use `no_op_commit`, the umi4 tests' own path: the real key path,
|
||||
on (West, Revenue), which is row 1 and never the first cell.
|
||||
|
||||
Both failed before the fix. With the list as written they failed on
|
||||
`Text("007")` stored as `Number(7.0)`. With `Error("boom")` moved to the front
|
||||
they failed on it, stored as `Text("ERR:boom")`, so the error case is red on
|
||||
its own and not only behind the text one. **Rival:** `untouched` answering
|
||||
`false` always, i.e. the fix removed. Both fail again.
|
||||
|
||||
`no_op_commit` now also asserts that the buffer after `i` is
|
||||
`value.edit_text()`. **That assertion is what keeps the umi4 tests above
|
||||
alive.** With ixxx fixed, an untouched commit writes nothing, so a seed
|
||||
regressed to the rounded `Display` is never written either, and
|
||||
`an_untouched_{pivot,records}_edit_stores_the_identical_value` pass with umi4
|
||||
reverted. That was run: with the seed assertion removed and
|
||||
`display_value_in` seeding from `to_string()`, all six untouched tests pass.
|
||||
With the assertion in, both umi4 tests fail.
|
||||
|
||||
### `an_untouched_edit_does_not_overwrite_a_write_made_while_it_was_open`
|
||||
|
||||
The reviewer's first constraint on improvise-ixxx: the seed is the one
|
||||
**captured when the editor opened**. The test opens the editor on
|
||||
`Text("007")`, then applies a `SetCell(Number(5.0))` directly, standing in for
|
||||
another client's write that lands while the editor is open. Then it presses
|
||||
`Enter` with nothing typed. The cell must still be `5`.
|
||||
|
||||
Failed before the fix: the commit wrote `Number(7.0)`. **Rival:**
|
||||
`commit_at_cursor` taking the seed from `ctx.display_value`, which is
|
||||
recomputed at commit time, instead of `ctx.opened_editor`. This test fails
|
||||
alone, with `Number(7.0)`, and every other untouched test passes. So it is the
|
||||
only guard on the choice of seed, and a fixture where the cell does not move
|
||||
cannot tell the two seeds apart.
|
||||
|
||||
### `an_untouched_commit_still_advances_and_reopens_the_editor`
|
||||
|
||||
The reviewer's second constraint: only the **write** is skipped. The fixture
|
||||
puts `Text("007")` at (West, Cost), row 1 and column 0. `Enter` there advances
|
||||
to (0, 1) rather than aborting at the corner, which is where (West, Revenue)
|
||||
sits in pivot mode. The test asserts four things: the cell is unchanged, the
|
||||
cursor moved to (0, 1), the mode is editing again, and the reopened editor
|
||||
captured `"2"`, the cell it landed on.
|
||||
|
||||
**Rival:** `CommitAndAdvance::execute` returning straight after the mode change
|
||||
when untouched, which skips the advance along with the write. This test fails,
|
||||
and so do `tab_on_an_empty_new_measure_column_still_starts_a_record_row` and
|
||||
`ui::app::tests::tab_on_bottom_right_of_records_inserts_below`. Those two are
|
||||
the records Tab contract: the empty `+` column's seed is `""`, so an empty Tab
|
||||
there is now an untouched commit too.
|
||||
|
||||
The buffer after `Enter` is **not** asserted. The keymap welds `clear-buffer`
|
||||
after every commit key, so it empties the buffer `EnterEditAtCursor` has just
|
||||
seeded, and the reopened editor opens blank while its captured seed is `"2"`.
|
||||
That is existing behaviour (`enter_commits_the_cell_advances_and_re_enters_editing`
|
||||
in `clients.md`). An `Enter` there with nothing typed therefore clears the landed cell,
|
||||
measured here; that is filed as improvise-fy9o rather than fixed here.
|
||||
|
||||
### `an_editor_typed_back_to_its_seed_still_writes`
|
||||
|
||||
**improvise-2054**, found by the #205 review. `untouched` compared content
|
||||
only, so an edit that ended back on the seed text was treated as untouched. The
|
||||
test opens the editor on `Number(7)` at (West, Revenue), which is row 1 and
|
||||
column 1. It then applies `SetCell(Number(5))` as another client's write,
|
||||
presses Backspace and `7`, and presses Enter. The cell must be `7`. After the
|
||||
Backspace alone, the test also asserts `opened_editor.touched` is set.
|
||||
|
||||
Failed before the fix: the cell stayed `Number(5.0)`, and nothing told the
|
||||
user. **Rivals:**
|
||||
- `untouched` ignoring `touched` fails this test.
|
||||
- `PopChar` not marking fails this test at the post-Backspace assertion.
|
||||
- `AppendChar` not marking cannot fail it, because the pop has already marked
|
||||
the editor. That rival fails `appending_or_popping_marks_the_editor_touched`,
|
||||
which presses each key on its own.
|
||||
|
||||
### `an_editor_opened_by_enter_mode_does_not_inherit_an_earlier_seed`
|
||||
|
||||
**improvise-wv3h.** Deleting `SetOpenedEditor(None)` from `EnterMode` failed
|
||||
no test. The reviewer's repro types `007` after `:enter-mode editing`, and it
|
||||
no longer discriminates now that improvise-2054 marks the editor on the first
|
||||
append, so this test commits with **nothing typed**. The steps:
|
||||
|
||||
1. `i` on an empty cell, (North, Cost), captures the seed `""`.
|
||||
2. Esc. `clear-buffer` does not mark the editor, so that stale seed stays
|
||||
untouched.
|
||||
3. Move to (West, Revenue), which holds 2.
|
||||
4. `:enter-mode editing`. That is typed into the `command` buffer, which also
|
||||
does not mark the cell editor.
|
||||
5. Enter, with the edit buffer `""`.
|
||||
|
||||
The cell must be cleared. **Rival:** the guard deleted. This test fails alone,
|
||||
leaving `Number(2.0)`: the empty commit was skipped against the earlier
|
||||
editor's seed.
|
||||
|
||||
This test is only possible because `clear-buffer` and keys typed into another
|
||||
buffer do not mark the editor. A rule that marked on every buffer write would
|
||||
make the guard redundant, and no test could pin it (`protocol.md` §3).
|
||||
|
||||
### `command::cmd::commit::tests::the_editor_an_advance_reopens_starts_untouched`
|
||||
|
||||
**improvise-465t**, found by the #205 re-review. Every open resets `touched`,
|
||||
but no test pinned the reset on the editor that `CommitAndAdvance` reopens
|
||||
through `EnterEditAtCursor`. The test works on the 2×2 pivot. It presses `i`
|
||||
on (West, Cost), row 1 and column 0, types `9`, and asserts the first editor
|
||||
really is touched. Then it presses Enter. The cursor must land on (0, 1), and
|
||||
the reopened editor must be exactly `OpenedEditor::on("2")`, which means seeded
|
||||
from the landed cell and untouched.
|
||||
|
||||
**Injection:** `EnterEditAtCursor` carrying the previous editor's `touched`
|
||||
flag. This test fails alone. Every other test stays green, which was the gap.
|
||||
Once improvise-fy9o re-seeds the reopened editor, a stale `touched` would make
|
||||
every untouched Tab or Enter write its seed.
|
||||
|
||||
## `ui/host.rs` — the abort arm of `Host::mutating_frame` (improvise-we1a)
|
||||
|
||||
@@ -5752,6 +5986,19 @@ Faults injected:
|
||||
`a_formula_that_does_not_parse_keeps_the_user_in_the_editor_with_their_text`.
|
||||
The wire table passed.
|
||||
|
||||
What this table cannot see (improvise-lasd, found in review of #229): a site it
|
||||
does not list. The fault injected was `on_view`'s missing-view branch changed to
|
||||
write the status and abort, without refusing. The workspace stayed green, and
|
||||
only `Host::mutating_frame`'s `|| aborted` arm caught it. That is why
|
||||
`App::apply_effects` now carries a `debug_assert!`: a `Model` effect that aborts
|
||||
must have refused. Under the same fault, the assertion fails two existing tests:
|
||||
- `a_frame_writing_a_view_its_own_first_command_deleted_is_refused`, with
|
||||
`SetNumberFormat(Named("Scratch"), ",.2f")`;
|
||||
- `a_command_naming_a_view_the_document_has_not_got_writes_nothing`, with
|
||||
`SetAxis { view: Named("NoSuchView"), .. }`.
|
||||
|
||||
With the assertion removed, the same fault fails nothing.
|
||||
|
||||
### every_refusal_site_refuses_its_failure_and_only_its_failure — three cases (improvise-rtud)
|
||||
|
||||
This entry covers the rows `load of an absent file`, `import-json of an absent
|
||||
|
||||
Reference in New Issue
Block a user