fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #205 from fiddlerwoaroof/fix/ixxx-untouched-commit

Browse Source 
An untouched cell edit writes nothing (improvise-ixxx)
This commit is contained in:
Ed Langley
2026-09-23 08:24:48 -07:00
committed by GitHub
parent 705c92d9ff 0051e57341
commit 194d8f529c
19 changed files with 1177 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/clients.md
+10 -2
View File
@@ -1412,6 +1412,14 @@ than a check; or this client has not been told the document's basis and so has
nothing truthful to say about what it composed against (`BasisCache::claim`
withholds rather than invents).
A **fifth** `None` is not a failure: the text is the seed the editor captured
when it opened (improvise-ixxx). The seed is part of `EditTarget`, captured by
`edit_target_at` from the cache's `edit_text` beside the record and its
version, so the page passes no seed and cannot pass a wrong one.
`typed_target_frame` asks `resolve::untouched`, the one home beside `typed()`,
before anything else. The argument is in `protocol.md` §3, "An untouched editor
writes nothing".
**Nothing is folded there.** The new value comes back as a `GridUpdate::Cells`
from the host, which is the side that formats it; guessing it locally is how the
two ends drift apart.
@@ -1616,8 +1624,8 @@ whether a typed string is a number. Five calls are the whole harness:
| `client.set_viewport(rows, cols)` | the handshake frame, and every resize; also records the window so keys resolve here |
| `client.on_frame(json)` | fold one frame, in arrival order |
| `client.render_model()` | everything to paint, already decided |
| `client.edit_target(r, c)` | what an editor opened here edits (the record and the version seen): an opaque string the page holds, or `null` |
| `client.typed_cell(target, s)` | the frame for "the user typed s into that cell", by identity (improvise-2wu3) |
| `client.edit_target(r, c)` | what an editor opened here edits (the record, the version seen, and the seed it opened on): an opaque string the page holds, or `null` |
| `client.typed_cell(target, s)` | the frame for "the user typed s into that cell", by identity (improvise-2wu3), or none if `s` is still the target's seed (improvise-ixxx) |
| `client.on_key_event(k,c,a,s,m)` | the frame for a keypress, folded locally first where this client may answer it |
**The two rules this file must not break are ordering and framing**: frames are
context/code/protocol.md
+80
View File
@@ -674,6 +674,86 @@ typed stored the rounded number in both front ends. Every arm that seeds an
editor now goes through `CellValue::edit_text` (`core.md` §17). That is the
shortest decimal that parses back to the same `f64`.
### An untouched editor writes nothing (improvise-ixxx)
An exact seed is not enough, because the commit reclassifies it. `typed()`
reads text, not the cell it came from, so `i` then `Enter` on `Text("007")`
stored `Number(7)`. A raw `CellValue::Error` has the edit text `ERR:msg`, and
the same keys stored it as `Text("ERR:msg")`. After umi4 the seed differs from
the stored value only when `typed(edit_text(v)) != v`. In every such case the
right answer is to keep `v`, which is what an untouched commit means.
So **a commit whose text is the seed the editor captured as it opened writes
nothing**. `resolve::untouched(Option<&OpenedEditor>, &str)` is the one home
for that, beside `typed()`. Every commit path reaches it:
- **TUI, pivot and records.** `CommitAndAdvance` carries `opened`, which
`commit_at_cursor` takes from `CmdContext::opened_editor`. It skips only
`commit_cell_value`: the mode change, the cursor restore, the advance and the
re-entry all still run, so records Tab still advances. The script form
(`commit-cell-edit <mode> <value> <coords>`) carries `None` and always
writes, since no editor was opened. The drill's staging goes through the same
`commit_cell_value`, so it is covered too.
- **Browser keyboard.** `resolve::commit` → `written`, which returns an empty
write for an untouched buffer and leaves `ChangeMode` and `advanced` alone.
- **Browser mouse.** The seed is a field of #188's `EditTarget` (`opened`),
captured by `edit_target_at` from the cache's `edit_text` beside the record
and its version. `typed_target_frame` starts with the `untouched` check, and
the page passes no seed (`typed_cell(target, text)`).
**The seed is captured at open, never recomputed at commit.** Where the editor
opened is where `OpenedEditor` is written. For the TUI that is `EditOrDrill`
and `EnterEditAtCursor`. For the browser it is `resolve::opened_on`, which
`edit_or_drill` and `advanced` share, and `ClientState::settle_editor` for an
editor that arrived without its buffer. For the mouse it is `edit_target_at`.
The reason is concurrency. If another client writes the cell while this editor
is open, a seed recomputed at commit has moved to the new value. The untouched buffer then differs from it, so the
stale text gets written over the other client's value. The browser's basis
claim would not stop that write, because it is taken when the commit is
composed. Comparing against the captured seed avoids that lost update as well.
`OpenedEditor` lives on `ViewState` (`opened_editor`) and travels as
`Command::SetOpenedEditor`, with the `ViewOnly` effect of the same name. It is
a struct on purpose. What else an open editor must capture, namely which record
it edits (improvise-2wu3) and at what version (improvise-0j0f), belongs in the
same object. `EnterMode` into an editing mode writes `None`, because an editor
opened that way has no seed and must not inherit an earlier one.
**Untouched means not edited, not merely equal** (improvise-2054). Comparing
content alone dropped a deliberate write. Open on `7`, let another client write
`5`, then press Backspace and `7`: the buffer spelled the seed again, so the
commit wrote nothing and the user was not told. `OpenedEditor` therefore
carries `touched`, and `untouched` requires `!touched && text == seed`. The
edited write is then composed, and if the cell moved, the basis check
(improvise-0j0f) refuses it loudly rather than dropping it silently.
`resolve::edited(opened, buffer)` is the one rule for what marks an editor. It
is an edit of **the editor's own buffer**, `edit`, by `append-char` or
`pop-char`. `AppendChar`/`PopChar` in the TUI and the browser's two arms each
call it and emit `SetOpenedEditor` once, on the first edit. Two things
deliberately do **not** mark it:
- **A key into another buffer.** Typing `:enter-mode editing` goes into
`command`. If that marked the stale editor, it would hide the stale-seed case
the `EnterMode` guard exists for, and no test could see the guard.
- **`clear-buffer`.** Esc and the post-commit clear are not the user's edits.
If they marked the editor, the guard above would again be redundant and
unpinnable.
The comparison stays exact: whitespace is content, and nothing trims.
**The mouse editor tracks no `touched`.** Its edits happen in a DOM `<input>`
that Rust never sees, so on that path `untouched` compares content alone, and
the 2054 case survives there (improvise-jibp). The fix belongs with
`EditTarget`, for example an `input` listener that reports the first edit. A
second gap sits on the same path. The `<input>` is still seeded from the
**painted** `editText`, while `EditTarget` captures the **cache**. So a paint
that went stale while another editor was open shows old text against a newer
seed and version (improvise-ugj4).
This does **not** change classification. `007` typed fresh is still
`Number(7)`, and `a_touched_browser_commit_still_writes` pins that.
`numeric` and `error` are decided by the server for the same reason: it is
the side that knows which it was, and re-deriving client-side is how the two
ends drift apart. `CellDisplay` is deliberately not *drawn from* a `CellValue`
context/test-evidence/clients.md
+167
View File
@@ -4417,6 +4417,15 @@ with no `SetBuffer`, then presses the keys: Enter must compose
`an_edit_buffer_the_user_has_emptied_is_not_refilled_from_the_cell` fails,
as it did against the same rival in `render.rs`.
**Amended by improvise-ixxx.** Its first `Enter` is untouched now, so it
composes no write. The assertion that it "must not clear" is therefore
`committed == None`, where `committed` is the first `Command::Model` of any
kind. It still fails against the pre-ktlk tree:
that tree's buffer is `""`, and `settle_editor` never ran, so no seed was
captured and the empty buffer composes `ClearCell`. The typed-`5` half is
unchanged. `settle_editor` now captures `OpenedEditor` beside the buffer,
because the editor opens at that moment.
## improvise-dsxq — the browser's `yy` copies the value, not a string
PR #161 (improvise-lut7) made the browser's `yank` arm compose
@@ -4836,3 +4845,161 @@ exactly that string to be the topmost element at its own centre, which is the
help overlay". With the fix and `position: sticky` changed to `static`, it
fails the same way: the footer exists but sits below the fold of a scrolling
overlay, so presence alone cannot satisfy the case.
**Faults injected:** the feature removed from `improvise-wasm-client` alone,
which fails naming that crate while the two tests above pass. Then removed from
`improvise-ws-server` alone, which fails naming it. The native path runs
without `--target`, so the two arms are different invocations.
### `an_untouched_browser_commit_writes_nothing_and_still_advances`
**improvise-ixxx**, browser keyboard. The fixture is a 3×1 grid with the cursor
on row 1. For each seed in `"007"`, `"1e3"`, `"inf"`, `"ERR:boom"` and
`"1.23456789"`, the test folds `i`, checks the buffer is that seed, and resolves
`Enter`. The composed commands must contain no `Command::Model`. They must also
contain `SetSelected { row: 2, col: 0 }`, the re-entry
`ChangeMode(editing)`, and `SetOpenedEditor` capturing `"800"`, the cell it
landed on.
Failed before the fix: `"007"` composed `SetCell(Number(7.0))`. **Rival:**
`resolve::commit` returning `[ChangeMode(Normal)]` when untouched, which skips
the advance along with the write. This test fails alone.
### `an_untouched_browser_edit_does_not_overwrite_a_write_made_while_it_was_open`
The captured-seed constraint, in the browser. The test opens the editor on
`"007"`, then folds a `GridUpdate::Cells` that moves the cell to `"5"`. That is
the projection another client's write produces. `Enter` with nothing typed must
compose no `Command::Model`.
**Rival:** `written` comparing against `edit_seed(state)`, which re-reads the
cache at commit time, instead of `ViewState::opened_editor`. This test fails
alone, composing `SetCell(Number(7.0))`. The basis claim is taken when the
commit is composed, so it would not have refused that write.
### `a_touched_browser_commit_still_writes`
Scope guard. The reviewer's fourth constraint says classification is out of
scope. `Backspace` over the seed `"7"` and then typing `007` must still compose
`SetCell(Number(7.0))`. It passes before and after. It is here so that a fix
which suppressed number-shaped text in general, rather than untouched text,
fails.
### `an_untouched_browser_edit_of_a_misread_value_writes_nothing`
The same shape as `an_untouched_browser_edit_stores_the_identical_value`, with
the values `typed()` misreads: `Text("007")`, `Text("1e3")`, `Text("inf")` and a
raw `Error("boom")`. It runs through the resolver and into a real host. Both
tests share `untouched_browser_edit`, which now also asserts that the buffer
after `i` is `value.edit_text()`. That assertion keeps the umi4 test
discriminating once untouched commits write nothing (see
`command.md`, `an_untouched_pivot_edit_of_a_misread_value_writes_nothing`).
With `project.rs` seeding from `to_string()`, both umi4 browser tests fail.
**Rival:** `untouched` always `false`. This test fails.
### `a_mouse_commit_of_the_untouched_seed_composes_nothing`
**improvise-ixxx**, browser mouse. The page's click editor used to commit
unconditionally, so an untouched click-edit re-sent the seed through `typed()`.
Since the merge with #188, the seed is part of `EditTarget` (`opened`).
`edit_target_at` captures it from the cache's `edit_text`, and
`typed_target_frame` starts with `untouched`. The page passes no seed, so the
JS rival of the first version (a page passing the wrong string) cannot be
written any more.
For `"007"`, `"ERR:boom"` and `"1.23456789"`, the test opens a target on a
record cell whose edit text is that value. It asserts the captured seed, then
commits the same text, which must compose nothing. The cell's **drawn** text is
different on purpose (`"drawn …"`). The control commits `"12000"` and `"1200 "`
against the seed `"1200"`, and both must compose a write, so a
`typed_target_frame` that never composes cannot pass, and neither can one that
trims. **Rivals:**
- `edit_target_at` capturing the drawn `text` instead of `edit_text` fails the
seed assertion.
- `typed_target_frame` without the `untouched` check fails this test and the
next.
- `seed.trim() == text.trim()` in `untouched` fails the `"1200 "` control.
### `a_mouse_commit_is_judged_against_the_seed_it_opened_with`
The captured-seed constraint, on the mouse path. The target is taken on
`"1200"`. A projected write then moves the cell to `"5"`, and a guard asserts
the cache really holds `"5"`. Committing that target with `"1200"` must compose
nothing. **Rival:** `typed_target_frame` comparing against the cache's
`edit_text` at commit instead of `target.opened`. This test fails alone.
The page half is `harness/tests/contract.spec.js`, "an untouched mouse editor
sends no write, and a touched one does". It wraps `Worker.prototype.postMessage`,
opens the editor on cell 4, and presses `Enter`. No `SetCell` may be posted.
Then it types `1` and presses `Enter`, and exactly one must be.
**Known gap, filed as improvise-ugj4.** The `<input>` is still seeded from the
painted `editText`, while `EditTarget` captures the cache. A paint made stale
while another editor was open (`repaint` returns early while `editing` is set)
therefore shows old text against a newer seed and version.
**improvise-jibp** is the other residue: the mouse editor has no `touched` flag.
### `a_browser_editor_typed_back_to_its_seed_still_writes`
**improvise-2054**, the browser keyboard. The fixture is a 3×1 grid, cursor on
row 1, with the cell's edit text `"7"`. The test folds `i`, then a projected
write that moves the cell to `"5"`, then Backspace and `7`. Enter must compose
`SetCell(Number(7.0))`. After the Backspace, `opened_editor.touched` must be
set. **Rivals:**
- `untouched` ignoring `touched` fails this test and
`untouched_is_exact_and_honours_an_edit`.
- The `pop-char` arm not marking fails this test.
- The `append-char` arm not marking fails only
`only_an_edit_of_the_editors_buffer_marks_it_touched`, because the pop has
already marked the editor.
### `only_an_edit_of_the_editors_buffer_marks_it_touched`
`x` and Backspace, each on its own on a freshly opened editor, must each set
`touched`. `typed_into` of the `command` buffer with an editor open must
compose the `SetBuffer` and nothing else. **Rivals:**
- The `append-char` arm not marking fails the `x` case.
- `edited` without its `buffer == "edit"` check fails the last assertion.
That second rival is the one that matters. Without the buffer check, typing
`:enter-mode …` would mark the stale editor. That would make the `EnterMode`
guard redundant, and
`a_browser_editor_opened_by_enter_mode_does_not_inherit_an_earlier_seed`
could no longer see the guard.
### `a_browser_editor_opened_by_enter_mode_does_not_inherit_an_earlier_seed`
**improvise-wv3h**, the browser half. The steps:
1. `i` on a cell whose edit text is `""` captures the seed `""`.
2. Esc.
3. A projected write moves the cell to `"9"`.
4. `composed(enter-mode editing)` is folded.
5. Enter, with the buffer `""`.
Enter must compose `ClearCell`. **Rival:** the `SetOpenedEditor(None)` in
`resolve::entered` deleted. This test fails alone, composing no write.
No browser key is bound to `enter-mode editing`, and `:` is declined in the
browser (`stranding`). So `entered` is reached here through `composed` directly,
and that guard is currently defence in depth.
### `untouched_is_exact_and_honours_an_edit`
The shared function's contract, stated once: `"7"` against its own untouched
seed is untouched. `"7 "` and `" 7"` against the seed `"7"`, `"7"` against
`" 7"`, and `"A"` against `"a"` are all touched, and so is a `touched` editor spelling its seed. **Rival:** comparing
`seed.trim() == text.trim()`. This test and
`socket::tests::a_mouse_commit_of_the_untouched_seed_composes_nothing` fail. On
the keyboard paths a trimmed comparison cannot be seen through the keys, since
any whitespace edit also sets `touched`, so this unit test and the mouse path
are where it shows. The mouse test now also asserts that `"7 "` against the
seed `"7"` is a write.
### `resolve::tests::the_editor_an_advance_reopens_starts_untouched`
**improvise-465t**, the browser half. The fixture is a 3×1 grid with the cursor
on row 1. The test presses `i`, types `9`, and asserts the first editor really
is touched. Enter must then land on (2, 0), and the folded `opened_editor` must
be exactly `OpenedEditor::on("800")`. **Injection:** `resolve::advanced`
carrying the previous editor's `touched` flag. This test fails alone.
context/test-evidence/command.md
+118
View File
@@ -5829,3 +5829,121 @@ 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.
crates/improvise-command/src/command/cmd/commit.rs
+238 -2
View File
@@ -3,6 +3,7 @@ use crate::model::cell::CellKey;
use crate::ui::app::AppMode;
use crate::ui::effect::{self, Effect};
use crate::view::RecordCol;
use improvise_protocol::{OpenedEditor, untouched};
use super::core::{Cmd, CmdContext, read_buffer};
use super::grid::AddRecordRow;
@@ -181,6 +182,7 @@ mod tests {
advance: super::AdvanceDir::Down,
cursor: super::CursorState::from_ctx(&ctx),
edit_mode: AppMode::records_editing(),
opened: None,
};
let effects = cmd.execute(&ctx);
let dbg = effects_debug(&effects);
@@ -285,6 +287,7 @@ mod tests {
advance: super::AdvanceDir::Down,
cursor: super::CursorState::from_ctx(&ctx),
edit_mode: AppMode::records_editing(),
opened: None,
}
.execute(&ctx)
};
@@ -571,6 +574,11 @@ mod tests {
app.view_state.mode.is_editing(),
"`i` did not open an editor"
);
assert_eq!(
app.view_state.buffers.get("edit"),
Some(&value.edit_text()),
"the editor was not seeded from the stored value (improvise-umi4)"
);
press(&mut app, KeyCode::Enter);
let stored = app.model_state.workbook.model.get_cell(&target).cloned();
let same = match (&stored, &value) {
@@ -599,6 +607,227 @@ mod tests {
no_op_commit(true, value);
}
}
/// Stored values whose edit text `typed()` reads back as something else.
fn values_typed_misreads() -> Vec<crate::model::cell::CellValue> {
use crate::model::cell::CellValue::{Error, Text};
vec![
Text("007".into()),
Text("1e3".into()),
Text("inf".into()),
Text("2024".into()),
Error("boom".into()),
]
}
/// BUG (improvise-ixxx) — see `context/test-evidence/command.md`.
#[test]
fn an_untouched_pivot_edit_of_a_misread_value_writes_nothing() {
for value in values_typed_misreads() {
no_op_commit(false, value);
}
}
/// BUG (improvise-ixxx) — see `context/test-evidence/command.md`.
#[test]
fn an_untouched_records_edit_of_a_misread_value_writes_nothing() {
for value in values_typed_misreads() {
no_op_commit(true, value);
}
}
/// BUG (improvise-ixxx) — see `context/test-evidence/command.md`.
#[test]
fn an_untouched_edit_does_not_overwrite_a_write_made_while_it_was_open() {
use crate::model::cell::{CellKey, CellValue};
use improvise_protocol::key::KeyCode;
let target = CellKey::new(vec![
("Region".into(), "West".into()),
("_Measure".into(), "Revenue".into()),
]);
let mut app = grid_holding(CellValue::Text("007".into()), false);
app.cursor_mut().selected = (1, 1);
press(&mut app, KeyCode::Char('i'));
app.apply_effects(vec![Box::new(effect::SetCell(
target.clone(),
CellValue::Number(5.0),
))]);
press(&mut app, KeyCode::Enter);
assert_eq!(
app.model_state.workbook.model.get_cell(&target),
Some(&CellValue::Number(5.0)),
"an editor nobody typed in overwrote the write that landed while it was open"
);
}
/// BUG (improvise-2054) — see `context/test-evidence/command.md`.
#[test]
fn an_editor_typed_back_to_its_seed_still_writes() {
use crate::model::cell::{CellKey, CellValue};
use improvise_protocol::key::KeyCode;
let target = CellKey::new(vec![
("Region".into(), "West".into()),
("_Measure".into(), "Revenue".into()),
]);
let mut app = grid_holding(CellValue::Number(7.0), false);
app.cursor_mut().selected = (1, 1);
press(&mut app, KeyCode::Char('i'));
app.apply_effects(vec![Box::new(effect::SetCell(
target.clone(),
CellValue::Number(5.0),
))]);
press(&mut app, KeyCode::Backspace);
assert_eq!(
app.view_state.opened_editor.as_ref().map(|o| o.touched),
Some(true),
"Backspace alone did not mark the editor touched"
);
press(&mut app, KeyCode::Char('7'));
assert_eq!(
app.view_state.buffers.get("edit").map(String::as_str),
Some("7")
);
press(&mut app, KeyCode::Enter);
assert_eq!(
app.model_state.workbook.model.get_cell(&target),
Some(&CellValue::Number(7.0)),
"a value the user typed was dropped because it spelled the seed"
);
}
/// Either user edit marks the open editor, mirroring the browser's
/// `only_an_edit_of_the_editors_buffer_marks_it_touched`.
#[test]
fn appending_or_popping_marks_the_editor_touched() {
use crate::model::cell::CellValue;
use improvise_protocol::key::KeyCode;
for key in [KeyCode::Char('x'), KeyCode::Backspace] {
let mut app = grid_holding(CellValue::Number(7.0), false);
app.cursor_mut().selected = (1, 1);
press(&mut app, KeyCode::Char('i'));
assert_eq!(
app.view_state.opened_editor.as_ref().map(|o| o.touched),
Some(false)
);
press(&mut app, key);
assert_eq!(
app.view_state.opened_editor.as_ref().map(|o| o.touched),
Some(true),
"{key:?} did not mark the editor touched"
);
}
}
/// BUG (improvise-wv3h) — see `context/test-evidence/command.md`.
#[test]
fn an_editor_opened_by_enter_mode_does_not_inherit_an_earlier_seed() {
use crate::model::cell::{CellKey, CellValue};
use improvise_protocol::key::KeyCode;
let empty = CellKey::new(vec![
("Region".into(), "North".into()),
("_Measure".into(), "Cost".into()),
]);
let target = CellKey::new(vec![
("Region".into(), "West".into()),
("_Measure".into(), "Revenue".into()),
]);
let mut app = grid_holding(CellValue::Number(2.0), false);
app.model_state.workbook.model.clear_cell(&empty);
app.rebuild_layout();
app.cursor_mut().selected = (0, 0);
press(&mut app, KeyCode::Char('i'));
assert_eq!(
app.view_state
.opened_editor
.as_ref()
.map(|o| o.seed.as_str()),
Some(""),
"the first editor did not open on the empty cell"
);
press(&mut app, KeyCode::Esc);
app.cursor_mut().selected = (1, 1);
press(&mut app, KeyCode::Char(':'));
for c in "enter-mode editing".chars() {
press(&mut app, KeyCode::Char(c));
}
press(&mut app, KeyCode::Enter);
assert!(
app.view_state.mode.is_editing(),
"`:enter-mode editing` did not open an editor"
);
press(&mut app, KeyCode::Enter);
assert_eq!(
app.model_state.workbook.model.get_cell(&target),
None,
"the empty commit was skipped against the earlier editor's seed"
);
}
/// improvise-465t: the editor an advance reopens is a new open, so it starts
/// untouched even when the commit before it was typed. See command.md.
#[test]
fn the_editor_an_advance_reopens_starts_untouched() {
use improvise_protocol::key::KeyCode;
let mut app = grid_holding(CellValue::Number(4.0), false);
app.cursor_mut().selected = (1, 0);
press(&mut app, KeyCode::Char('i'));
press(&mut app, KeyCode::Char('9'));
assert_eq!(
app.view_state.opened_editor.as_ref().map(|o| o.touched),
Some(true),
"the first editor was never touched, so this proves nothing"
);
press(&mut app, KeyCode::Enter);
assert_eq!(app.cursor().selected, (0, 1), "the advance did not run");
assert_eq!(
app.view_state.opened_editor,
Some(OpenedEditor::on("2".into())),
"the reopened editor inherited the previous editor's touched flag"
);
}
/// BUG (improvise-ixxx) — see `context/test-evidence/command.md`.
#[test]
fn an_untouched_commit_still_advances_and_reopens_the_editor() {
use crate::model::cell::{CellKey, CellValue};
use improvise_protocol::key::KeyCode;
let target = CellKey::new(vec![
("Region".into(), "West".into()),
("_Measure".into(), "Cost".into()),
]);
let mut app = grid_holding(CellValue::Number(4.0), false);
app.model_state
.workbook
.model
.set_cell(target.clone(), CellValue::Text("007".into()));
app.rebuild_layout();
app.cursor_mut().selected = (1, 0);
press(&mut app, KeyCode::Char('i'));
press(&mut app, KeyCode::Enter);
assert_eq!(
app.model_state.workbook.model.get_cell(&target),
Some(&CellValue::Text("007".into())),
"the untouched commit wrote"
);
assert_eq!(app.cursor().selected, (0, 1), "the advance did not run");
assert!(
app.view_state.mode.is_editing(),
"the editor was not reopened"
);
assert_eq!(
app.view_state
.opened_editor
.as_ref()
.map(|o| o.seed.as_str()),
Some("2"),
"the reopened editor did not capture the cell it landed on"
);
}
}
// ── Commit commands (mode-specific buffer consumers) ────────────────────────
@@ -729,6 +958,9 @@ pub struct CommitAndAdvance {
pub advance: AdvanceDir,
pub cursor: CursorState,
pub edit_mode: AppMode,
/// What the editor captured as it opened; `None` for a script line, which
/// always writes (improvise-ixxx).
pub opened: Option<OpenedEditor>,
}
impl Cmd for CommitAndAdvance {
fn name(&self) -> &'static str {
@@ -742,14 +974,18 @@ impl Cmd for CommitAndAdvance {
// Drop to normal **before** the commit, which can abort: tidy-up goes where
// an abort cannot reach it (improvise-2zq, gotcha 21).
effects.push(effect::change_mode(exit_mode_for(&self.edit_mode)));
commit_cell_value(ctx, &self.key, &self.value, &mut effects);
// An untouched editor writes nothing; everything around the write still runs.
let writes = !untouched(self.opened.as_ref(), &self.value);
if writes {
commit_cell_value(ctx, &self.key, &self.value, &mut effects);
}
// The commit may reshape the grid and so reset the cursor. Restore it before
// the advance, where an aborting advance cannot skip it (gotcha 21).
effects.push(Box::new(effect::SetSelected(
self.cursor.row,
self.cursor.col,
)));
if inserts_a_column(ctx, &self.key, &self.value) {
if writes && inserts_a_column(ctx, &self.key, &self.value) {
// Staying put is what lands on the inserted column (improvise-2zq); the
// restore above is what makes "staying put" something we do, not inherit.
effects.push(Box::new(effect::RebuildLayout));
crates/improvise-command/src/command/cmd/core.rs
+2
View File
@@ -54,6 +54,8 @@ pub struct CmdContext<'a> {
pub tile_cat_idx: usize,
/// Named text buffers
pub buffers: &'a HashMap<String, String>,
/// What the open cell editor captured as it opened (improvise-ixxx).
pub opened_editor: Option<&'a improvise_protocol::OpenedEditor>,
/// View navigation stacks (for drill back/forward)
pub view_back_stack: &'a [crate::ui::app::ViewFrame],
pub view_forward_stack: &'a [crate::ui::app::ViewFrame],
crates/improvise-command/src/command/cmd/mod.rs
+1
View File
@@ -74,6 +74,7 @@ pub(super) mod test_helpers {
view_panel_cursor: 0,
tile_cat_idx: 0,
buffers: &EMPTY_BUFFERS,
opened_editor: None,
view_back_stack: &[],
view_forward_stack: &[],
has_drill_state: false,
crates/improvise-command/src/command/cmd/mode.rs
+9 -2
View File
@@ -117,7 +117,7 @@ mod tests {
edit_mode: AppMode::editing(),
}
.execute(&ctx);
assert_eq!(effects.len(), 2);
assert_eq!(effects.len(), 3, "buffer, opened editor, mode");
let dbg = effects_debug(&effects);
assert!(dbg.contains("Editing"), "Expected Editing mode, got: {dbg}");
}
@@ -135,7 +135,7 @@ mod tests {
edit_mode: AppMode::records_editing(),
}
.execute(&ctx);
assert_eq!(effects.len(), 2);
assert_eq!(effects.len(), 3, "buffer, opened editor, mode");
let dbg = effects_debug(&effects);
assert!(
dbg.contains("RecordsEditing"),
@@ -281,6 +281,10 @@ impl Cmd for EnterMode {
if let Some(mb) = self.0.minibuffer() {
effects.push(write_buffer(&mb.buffer_key, String::new()));
}
// An editor opened this way captured no seed, so no earlier one may stand.
if self.0.is_editing() {
effects.push(Box::new(effect::SetOpenedEditor(None)));
}
effects.push(effect::change_mode(self.0.clone()));
effects
}
@@ -389,6 +393,9 @@ impl Cmd for EditOrDrill {
name: "edit".to_string(),
value: ctx.display_value.clone(),
}),
Box::new(effect::SetOpenedEditor(Some(
improvise_protocol::OpenedEditor::on(ctx.display_value.clone()),
))),
effect::change_mode(self.edit_mode.clone()),
]
}
crates/improvise-command/src/command/cmd/registry.rs
+4
View File
@@ -71,6 +71,7 @@ fn commit_at_cursor(
key: ctx.committable_cell_key()?,
advance,
cursor: CursorState::from_ctx(ctx),
opened: ctx.opened_editor.cloned(),
})
}
@@ -450,6 +451,7 @@ pub fn default_registry() -> CmdRegistry {
advance: AdvanceDir::Down,
cursor: CursorState::default(),
edit_mode: AppMode::editing(),
opened: None,
},
|args| {
if args.len() < 3 {
@@ -462,6 +464,7 @@ pub fn default_registry() -> CmdRegistry {
advance: AdvanceDir::Down,
cursor: CursorState::default(),
edit_mode,
opened: None,
})
},
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Down)),
@@ -473,6 +476,7 @@ pub fn default_registry() -> CmdRegistry {
advance: AdvanceDir::Right,
cursor: CursorState::default(),
edit_mode: AppMode::editing(),
opened: None,
},
|_| Err("commit-and-advance-right requires context".into()),
Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Right)),
crates/improvise-command/src/command/cmd/text_buffer.rs
+13 -2
View File
@@ -297,7 +297,7 @@ impl Cmd for AppendChar {
if let KeyCode::Char(c) = ctx.key_code {
let mut val = read_buffer(ctx, &self.buffer);
val.push(c);
vec![write_buffer(&self.buffer, val)]
typed_into(ctx, &self.buffer, val)
} else {
vec![]
}
@@ -316,10 +316,21 @@ impl Cmd for PopChar {
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
let mut val = read_buffer(ctx, &self.buffer);
val.pop();
vec![write_buffer(&self.buffer, val)]
typed_into(ctx, &self.buffer, val)
}
}
/// A user's edit of buffer `name`: the write, then the open editor marked
/// touched, so an edit back to the seed still commits (improvise-2054).
fn typed_into(ctx: &CmdContext, name: &str, value: String) -> Vec<Box<dyn Effect>> {
let mut effects = vec![write_buffer(name, value)];
effects.extend(
improvise_protocol::edited(ctx.opened_editor, name)
.map(|opened| Box::new(effect::SetOpenedEditor(Some(opened))) as Box<dyn Effect>),
);
effects
}
/// Handle backspace in command mode — pop char or return to Normal if empty.
#[derive(Debug)]
pub struct CommandModeBackspace;
crates/improvise-command/src/ui/app.rs
+1
View File
@@ -816,6 +816,7 @@ impl App {
category_panel_open: self.view_state.category_panel_open,
view_panel_open: self.view_state.view_panel_open,
buffers: &self.view_state.buffers,
opened_editor: self.view_state.opened_editor.as_ref(),
formula_cursor: self.view_state.formula_cursor,
cat_panel_cursor: self.view_state.cat_panel_cursor,
view_panel_cursor: self.view_state.view_panel_cursor,
crates/improvise-command/src/ui/effect.rs
+6 -1
View File
@@ -268,7 +268,10 @@ pub struct EnterEditAtCursor {
impl Effect for EnterEditAtCursor {
fn apply(&self, app: &mut App) {
let value = app.display_value_at_cursor();
app.view_state.buffers.insert("edit".to_string(), value);
app.view_state
.buffers
.insert("edit".to_string(), value.clone());
app.view_state.opened_editor = Some(improvise_protocol::OpenedEditor::on(value));
app.view_state.mode = self.target_mode.clone();
}
fn kind(&self) -> EffectKind {
@@ -640,6 +643,8 @@ impl Effect for SetBuffer {
simple_effect!(SetTileCatIdx(usize) => |app| app.view_state.tile_cat_idx, EffectKind::ViewOnly);
simple_effect!(SetOpenedEditor(Option<improvise_protocol::OpenedEditor>) => |app| app.view_state.opened_editor, EffectKind::ViewOnly);
/// Populate the drill state with a frozen snapshot of records.
/// Clears any previous drill state.
#[derive(Debug)]
crates/improvise-protocol/src/client.rs
+2 -1
View File
@@ -172,7 +172,8 @@ impl ClientState {
.map(|mb| mb.buffer_key.clone())
.filter(|key| !self.view.buffers.contains_key(key));
if let (Some(key), Some(seed)) = (absent, crate::resolve::edit_seed(self)) {
self.view.buffers.insert(key, seed);
self.view.buffers.insert(key, seed.clone());
self.view.opened_editor = Some(crate::OpenedEditor::on(seed));
}
}
crates/improvise-protocol/src/lib.rs
+5 -1
View File
@@ -36,7 +36,8 @@ pub use render::{
WizardProposal, WizardStepKind, header_level_widths,
};
pub use resolve::{
Click, KeyOutcome, Request, click, read_buffer, resolve, typed, typed_cell_command,
Click, KeyOutcome, OpenedEditor, Request, click, edited, read_buffer, resolve, typed,
typed_cell_command, untouched,
};
pub use viewmodel::{CatTreeEntry, flatten_cat_tree, view_axis_summary};
// `improvise-core`'s, because the index that resolves one lives in
@@ -433,6 +434,8 @@ pub struct ViewState {
/// The prefix key currently armed (g→gg, y→yy, Z→ZZ). The key that opened
/// it, not the sub-keymap it names — gotcha 13, improvise-jc8.
pub transient_prefix: Option<(KeyCode, KeyModifiers)>,
/// What the open cell editor captured when it opened (improvise-ixxx).
pub opened_editor: Option<OpenedEditor>,
}
impl Default for ViewState {
@@ -458,6 +461,7 @@ impl Default for ViewState {
expanded_cats: std::collections::HashSet::new(),
buffers: HashMap::new(),
transient_prefix: None,
opened_editor: None,
}
}
}
crates/improvise-protocol/src/reduce.rs
+5
View File
@@ -10,6 +10,7 @@ use improvise_core::view::Axis;
use crate::basis::Basis;
use crate::render::{Projection, RecordCell};
use crate::resolve::OpenedEditor;
use crate::{AppMode, HELP_PAGE_COUNT, KeyCode, KeyModifiers, Panel, ViewState};
use improvise_core::model::record::RecordHash;
@@ -39,6 +40,9 @@ pub enum Command {
value: String,
},
SetTileCatIdx(usize),
/// What a cell editor captured as it opened, or `None` for one opened
/// with nothing to compare against (improvise-ixxx).
SetOpenedEditor(Option<OpenedEditor>),
SetPanelOpen {
panel: Panel,
open: bool,
@@ -268,6 +272,7 @@ pub fn reduce_view(view: &mut ViewState, cmd: &Command) {
view.buffers.insert(name.clone(), value.clone());
}
Command::SetTileCatIdx(i) => view.tile_cat_idx = *i,
Command::SetOpenedEditor(opened) => view.opened_editor = opened.clone(),
Command::SetPanelOpen { panel, open } => *view.panel_open_mut(*panel) = *open,
Command::SetPanelCursor { panel, cursor } => *view.panel_cursor_mut(*panel) = *cursor,
Command::ToggleCatExpand(cat) => {
crates/improvise-protocol/src/resolve.rs
+352 -29
View File
@@ -2,6 +2,7 @@
//! by every front end; every decision here is in `context/code/clients.md`.
use improvise_core::view::Axis;
use serde::{Deserialize, Serialize};
use crate::client::ClientState;
use crate::frame::ClientFrame;
@@ -370,12 +371,12 @@ fn composed(
};
let mut value = read_buffer(&state.view, args.first()?);
value.push(c);
Some(vec![write_buffer(args.first()?, value)])
Some(typed_into(state, args.first()?, value))
}
"pop-char" => {
let mut value = read_buffer(&state.view, args.first()?);
value.pop();
Some(vec![write_buffer(args.first()?, value)])
Some(typed_into(state, args.first()?, value))
}
"clear-buffer" => Some(vec![write_buffer(args.first()?, String::new())]),
@@ -572,6 +573,17 @@ pub fn read_buffer(view: &ViewState, name: &str) -> String {
}
}
/// A user's edit of buffer `name`: the write, then the open editor marked
/// touched. `AppendChar`/`PopChar`, line for line (improvise-2054).
fn typed_into(state: &ClientState, name: &str, value: String) -> Vec<Command> {
let mut commands = vec![write_buffer(name, value)];
commands.extend(
edited(state.view.opened_editor.as_ref(), name)
.map(|opened| Command::SetOpenedEditor(Some(opened))),
);
commands
}
fn write_buffer(name: &str, value: String) -> Command {
match name {
"search" => Command::SetSearchQuery(value),
@@ -599,6 +611,10 @@ fn entered(mode: AppMode) -> Vec<Command> {
if let Some(mb) = mode.minibuffer() {
commands.push(write_buffer(&mb.buffer_key, String::new()));
}
// An editor opened this way captured no seed, so no earlier one may stand.
if mode.is_editing() {
commands.push(Command::SetOpenedEditor(None));
}
// Plain `Normal`: which member of the pair it settles to is the grid's, and
// `ClientState::fold` answers that — improvise-e5pf, `context/code/clients.md`.
commands.push(Command::ChangeMode(mode));
@@ -745,13 +761,7 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
match cell.id.as_ref()? {
// `edit_text`, never `text`: the drawn text has been through
// `format_f64`. `tests/lockstep.rs` found exactly that.
CellId::Record(_) => Some(vec![
Command::SetBuffer {
name: "edit".to_string(),
value: cell.edit_text.clone().unwrap_or_default(),
},
Command::ChangeMode(mode),
]),
CellId::Record(_) => Some(opened_on(cell.edit_text.clone().unwrap_or_default(), mode)),
CellId::Aggregate(_) => Some(vec![Command::SetStatus(format!(
"an aggregate is edited by drilling in — {}",
not_yet("drill")
@@ -766,6 +776,19 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
}
}
/// Open the cell editor on `seed`: the buffer, what it captured, then the mode.
/// `EditOrDrill`'s and `EnterEditAtCursor`'s shared shape (improvise-ixxx).
fn opened_on(seed: String, mode: AppMode) -> Vec<Command> {
vec![
Command::SetBuffer {
name: "edit".to_string(),
value: seed.clone(),
},
Command::SetOpenedEditor(Some(OpenedEditor::on(seed))),
Command::ChangeMode(mode),
]
}
/// One direction of the view history. An empty stack is answered out loud:
/// `reduce_view`'s own arm is silent there — `context/code/clients.md`.
fn step_history(stack: &[crate::ViewFrame], step: Command, empty: &str) -> Vec<Command> {
@@ -872,15 +895,24 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
if state.grid.records {
return None;
}
let mut commands = vec![Command::Model {
basis: state.basis.claim([record_at_cursor(state)?])?,
cmd: typed_cell_command(id, &text)?,
}];
let mut commands = written(state, id, &text)?;
commands.push(Command::ChangeMode(AppMode::Normal));
commands.extend(advanced(state, edit_mode, advance)?);
Some(commands)
}
/// The write a commit makes: none when the buffer is the seed the editor
/// captured as it opened, and only the write is skipped (improvise-ixxx).
fn written(state: &ClientState, id: &CellId, text: &str) -> Option<Vec<Command>> {
match untouched(state.view.opened_editor.as_ref(), text) {
true => Some(Vec::new()),
false => Some(vec![Command::Model {
basis: state.basis.claim([record_at_cursor(state)?])?,
cmd: typed_cell_command(id, text)?,
}]),
}
}
/// `CommitAndAdvance` over a creating column: a schema change over the typed
/// NAME. Why +cat does not advance: clients.md (improvise-gcvb).
fn create_column(
@@ -943,11 +975,7 @@ fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option
let mut commands = scrolled_to(state, nr, nc);
// `EnterEditAtCursor`: seed from where we landed, then change mode. The
// new cell is a different cell, so a stale cache is correct here.
commands.push(Command::SetBuffer {
name: "edit".to_string(),
value: seed_at(state, nr, nc),
});
commands.push(Command::ChangeMode(edit_mode));
commands.extend(opened_on(seed_at(state, nr, nc), edit_mode));
Some(commands)
}
@@ -984,6 +1012,45 @@ fn seed_at(state: &ClientState, row: usize, col: usize) -> String {
.unwrap_or_default()
}
/// What a cell editor captured as it **opened**, so its commit is judged
/// against that and never a seed read again at commit (improvise-ixxx).
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct OpenedEditor {
pub seed: String,
/// Whether the user has edited the buffer since it opened (improvise-2054).
pub touched: bool,
}
impl OpenedEditor {
/// An editor that has just opened on `seed`, untouched.
pub fn on(seed: String) -> Self {
Self {
seed,
touched: false,
}
}
}
/// The buffer every cell editor types into.
const EDIT_BUFFER: &str = "edit";
/// Whether committing `text` is the editor's own seed back, untouched: such a
/// commit writes nothing. The **one** home, beside [`typed`] (improvise-ixxx).
pub fn untouched(opened: Option<&OpenedEditor>, text: &str) -> bool {
opened.is_some_and(|opened| !opened.touched && opened.seed == text)
}
/// The open editor marked touched by a user edit of `buffer`, or `None` when
/// that edit changes nothing to record (improvise-2054, `protocol.md` §3).
pub fn edited(opened: Option<&OpenedEditor>, buffer: &str) -> Option<OpenedEditor> {
opened
.filter(|opened| buffer == EDIT_BUFFER && !opened.touched)
.map(|opened| OpenedEditor {
touched: true,
..opened.clone()
})
}
/// How a typed string becomes a [`CellValue`]: the **one** home, called by the
/// browser's compose path and by the host's own commit (improvise-j7v6).
pub fn typed(text: &str) -> Option<CellValue> {
@@ -3091,6 +3158,7 @@ mod tests {
name: "edit".into(),
value: "1200".into(),
},
Command::SetOpenedEditor(Some(OpenedEditor::on("1200".into()))),
Command::ChangeMode(AppMode::editing()),
])
);
@@ -3303,6 +3371,262 @@ mod tests {
);
}
/// Three rows by one column, the cursor on row 1, whose record cell's edit
/// text is `edit_text`: never the first cell.
fn client_on_row_one(edit_text: &str) -> ClientState {
let mut cell = CellDisplay::text(edit_text);
cell.edit_text = Some(edit_text.into());
cell.id = Some(record_cell());
let mut client = ClientState::default();
client.bootstrap(Snapshot {
view: ViewState::default(),
seed: vec![
Projection::Grid(GridUpdate::RowFrame(AxisFrame::flat([
"North", "South", "West",
]))),
Projection::Grid(GridUpdate::ColFrame(AxisFrame::flat(["Revenue"]))),
Projection::Grid(GridUpdate::Cells(vec![
((0, 0), CellDisplay::number("900")),
((1, 0), cell),
((2, 0), CellDisplay::number("800")),
])),
Projection::Grid(GridUpdate::Extent(GridExtent { rows: 3, cols: 1 })),
Projection::Basis(published()),
],
});
client.window = (3, 1);
client.view.cursor_mut().selected = (1, 0);
client
}
/// Resolve `code` against `client`, fold what it composed, and return it.
fn press_folded(client: &mut ClientState, code: KeyCode) -> Vec<Command> {
let KeyOutcome::Local(commands) = resolve(client, ev(code)) else {
panic!("{code:?} must resolve here");
};
for command in &commands {
client.fold(command);
}
commands
}
/// BUG (improvise-ixxx) — see `context/test-evidence/clients.md`.
#[test]
fn an_untouched_browser_commit_writes_nothing_and_still_advances() {
for seed in ["007", "1e3", "inf", "ERR:boom", "1.23456789"] {
let mut client = client_on_row_one(seed);
press_folded(&mut client, KeyCode::Char('i'));
assert_eq!(read_buffer(&client.view, "edit"), seed, "wrong seed");
let commands = press_folded(&mut client, KeyCode::Enter);
assert!(
!commands.iter().any(|c| matches!(c, Command::Model { .. })),
"{seed}: an untouched commit wrote: {commands:?}"
);
assert!(
commands.contains(&Command::SetSelected { row: 2, col: 0 })
&& commands.contains(&Command::ChangeMode(AppMode::editing())),
"{seed}: the advance and re-entry did not run: {commands:?}"
);
assert!(
commands.contains(&Command::SetOpenedEditor(Some(OpenedEditor::on(
"800".into()
)))),
"{seed}: the reopened editor did not capture its own seed: {commands:?}"
);
}
}
/// BUG (improvise-ixxx) — see `context/test-evidence/clients.md`.
#[test]
fn an_untouched_browser_edit_does_not_overwrite_a_write_made_while_it_was_open() {
let mut client = client_on_row_one("007");
press_folded(&mut client, KeyCode::Char('i'));
let mut moved = CellDisplay::number("5");
moved.id = Some(record_cell());
client.fold(&Command::Project(Projection::Grid(GridUpdate::Cells(
vec![((1, 0), moved)],
))));
let commands = press_folded(&mut client, KeyCode::Enter);
assert!(
!commands.iter().any(|c| matches!(c, Command::Model { .. })),
"the stale seed was written over the other client's value: {commands:?}"
);
}
/// improvise-465t: the editor an advance reopens starts untouched, even
/// after a typed commit. See `context/test-evidence/clients.md`.
#[test]
fn the_editor_an_advance_reopens_starts_untouched() {
let mut client = client_on_row_one("7");
press_folded(&mut client, KeyCode::Char('i'));
press_folded(&mut client, KeyCode::Char('9'));
assert_eq!(
client.view.opened_editor.as_ref().map(|o| o.touched),
Some(true),
"the first editor was never touched, so this proves nothing"
);
press_folded(&mut client, KeyCode::Enter);
assert_eq!(
client.view.cursor().selected,
(2, 0),
"the advance did not run"
);
assert_eq!(
client.view.opened_editor,
Some(OpenedEditor::on("800".into())),
"the reopened editor inherited the previous editor's touched flag"
);
}
/// BUG (improvise-2054) — see `context/test-evidence/clients.md`.
#[test]
fn a_browser_editor_typed_back_to_its_seed_still_writes() {
let mut client = client_on_row_one("7");
press_folded(&mut client, KeyCode::Char('i'));
let mut moved = CellDisplay::number("5");
moved.id = Some(record_cell());
client.fold(&Command::Project(Projection::Grid(GridUpdate::Cells(
vec![((1, 0), moved)],
))));
press_folded(&mut client, KeyCode::Backspace);
assert_eq!(
client.view.opened_editor.as_ref().map(|o| o.touched),
Some(true),
"Backspace alone did not mark the editor touched"
);
press_folded(&mut client, KeyCode::Char('7'));
assert_eq!(read_buffer(&client.view, "edit"), "7");
let commands = press_folded(&mut client, KeyCode::Enter);
assert!(
commands.iter().any(|c| matches!(
c,
Command::Model {
cmd: ModelCommand::SetCell {
value: CellValue::Number(7.0),
..
},
..
}
)),
"a value the user typed was dropped because it spelled the seed: {commands:?}"
);
}
/// Either user edit marks the open editor, and nothing else does: a key
/// into another buffer leaves it alone. Evidence: clients.md.
#[test]
fn only_an_edit_of_the_editors_buffer_marks_it_touched() {
for key in [KeyCode::Char('x'), KeyCode::Backspace] {
let mut client = client_on_row_one("7");
press_folded(&mut client, KeyCode::Char('i'));
assert_eq!(
client.view.opened_editor.as_ref().map(|o| o.touched),
Some(false)
);
press_folded(&mut client, key);
assert_eq!(
client.view.opened_editor.as_ref().map(|o| o.touched),
Some(true),
"{key:?} did not mark the editor touched"
);
}
let mut state = client_on_row_one("7");
state.view.opened_editor = Some(OpenedEditor::on("7".into()));
assert_eq!(
typed_into(&state, "command", "q".into()),
vec![Command::SetBuffer {
name: "command".into(),
value: "q".into()
}],
"typing into another buffer marked the cell editor"
);
}
/// BUG (improvise-wv3h) — see `context/test-evidence/clients.md`.
#[test]
fn a_browser_editor_opened_by_enter_mode_does_not_inherit_an_earlier_seed() {
let mut client = client_on_row_one("");
press_folded(&mut client, KeyCode::Char('i'));
press_folded(&mut client, KeyCode::Esc);
assert!(
!client.view.mode.is_editing(),
"Esc did not close the editor"
);
let mut moved = CellDisplay::number("9");
moved.id = Some(record_cell());
client.fold(&Command::Project(Projection::Grid(GridUpdate::Cells(
vec![((1, 0), moved)],
))));
let entered = composed(
&client,
ev(KeyCode::Enter),
"enter-mode",
&["editing".into()],
)
.expect("enter-mode editing composes here");
for command in &entered {
client.fold(command);
}
assert!(
client.view.mode.is_editing(),
"enter-mode did not open an editor"
);
let commands = press_folded(&mut client, KeyCode::Enter);
assert!(
commands.iter().any(|c| matches!(
c,
Command::Model {
cmd: ModelCommand::ClearCell(_),
..
}
)),
"the empty commit was skipped against the earlier editor's seed: {commands:?}"
);
}
/// `untouched` is exact: whitespace is content, and an edited editor is
/// never untouched even when it spells its seed (improvise-wv3h, 2054).
#[test]
fn untouched_is_exact_and_honours_an_edit() {
let fresh = |seed: &str| OpenedEditor::on(seed.into());
assert!(untouched(Some(&fresh("7")), "7"));
for (seed, text) in [("7", "7 "), ("7", " 7"), (" 7", "7"), ("a", "A")] {
assert!(!untouched(Some(&fresh(seed)), text), "{seed:?} vs {text:?}");
}
let edited = OpenedEditor {
touched: true,
..fresh("7")
};
assert!(!untouched(Some(&edited), "7"));
assert!(!untouched(None, ""));
}
/// A typed change is still a write, and `007` typed fresh is still a
/// number: classification is `typed()`'s and out of this ticket's scope.
#[test]
fn a_touched_browser_commit_still_writes() {
let mut client = client_on_row_one("7");
press_folded(&mut client, KeyCode::Char('i'));
press_folded(&mut client, KeyCode::Backspace);
for c in ['0', '0', '7'] {
press_folded(&mut client, KeyCode::Char(c));
}
let commands = press_folded(&mut client, KeyCode::Enter);
assert!(
commands.iter().any(|c| matches!(
c,
Command::Model {
cmd: ModelCommand::SetCell {
value: CellValue::Number(7.0),
..
},
..
}
)),
"{commands:?}"
);
}
/// An empty buffer is `ClearCell`, not `SetCell` of an empty string: the
/// decision `typed()` makes, reached by keyboard and by mouse alike.
#[test]
@@ -3344,18 +3668,15 @@ mod tests {
let KeyOutcome::Local(commands) = resolve(client, ev(KeyCode::Enter)) else {
panic!("Enter in Editing must resolve here");
};
match &commands[0] {
Command::Model {
cmd: ModelCommand::SetCell { value, .. },
..
} => Some(value.clone()),
commands.into_iter().find_map(|command| match command {
Command::Model { cmd, .. } => Some(cmd),
_ => None,
}
})
};
assert_eq!(
committed(&client),
Some(CellValue::Number(1200.5)),
"Enter on an editor showing the cell must not clear it"
None,
"Enter on an editor showing the cell must not clear it; untouched, it writes nothing"
);
let KeyOutcome::Local(typed) = resolve(&client, ev(KeyCode::Char('5'))) else {
@@ -3364,9 +3685,11 @@ mod tests {
for command in &typed {
client.fold(command);
}
assert_eq!(
committed(&client),
Some(CellValue::Number(1200.55)),
assert!(
matches!(
committed(&client),
Some(ModelCommand::SetCell { value: CellValue::Number(n), .. }) if n == 1200.55
),
"a typed character extends what the editor showed, not replaces it"
);
}
crates/improvise-wasm-client/src/socket.rs
+76 -3
View File
@@ -3,7 +3,8 @@
use crate::Client;
use improvise_protocol::{
ClientFrame, Command, RecordCell, Rejection, ServerFrame, typed_cell_command,
ClientFrame, Command, OpenedEditor, RecordCell, Rejection, ServerFrame, typed_cell_command,
untouched,
};
use serde::{Deserialize, Serialize};
@@ -14,6 +15,8 @@ pub struct EditTarget {
pub cell: RecordCell,
/// The record's write counter as this client had it at open.
pub version: u64,
/// The text the editor opened on, so an untouched commit writes nothing (improvise-ixxx).
pub opened: OpenedEditor,
}
/// What a refused batch says to the person who composed it. Worded here rather
@@ -101,12 +104,20 @@ impl Client {
pub fn edit_target_at(&self, row: usize, col: usize) -> Option<EditTarget> {
let cell = self.record_cell_at(row, col)?;
let version = self.state().basis.current()?.record(cell.record)?;
Some(EditTarget { cell, version })
let seed = self.state().grid.cell(row, col)?.edit_text.clone();
Some(EditTarget {
cell,
version,
opened: OpenedEditor::on(seed.unwrap_or_default()),
})
}
/// The write an editor opened earlier commits: to the record it opened on,
/// claimed at the version it read then (improvise-2wu3, improvise-0j0f).
pub fn typed_target_frame(&mut self, target: EditTarget, text: &str) -> Option<ClientFrame> {
if untouched(Some(&target.opened), text) {
return None;
}
let basis = self
.state()
.basis
@@ -272,6 +283,68 @@ mod tests {
client
}
/// BUG (improvise-ixxx) — see `context/test-evidence/clients.md`.
#[test]
fn a_mouse_commit_of_the_untouched_seed_composes_nothing() {
for seed in ["007", "ERR:boom", "1.23456789"] {
let mut cell = CellDisplay::text(format!("drawn {seed}"));
cell.edit_text = Some(seed.into());
let mut client = client_holding(cell);
let target = client.edit_target_at(0, 0).expect("a record cell");
assert_eq!(target.opened, OpenedEditor::on(seed.into()), "{seed}");
assert_eq!(client.typed_target_frame(target, seed), None, "{seed}");
}
let mut client = client_showing(Some(CellId::Record(record("Revenue"))));
for text in ["12000", "1200 "] {
let target = client.edit_target_at(0, 0).expect("a record cell");
assert!(
client.typed_target_frame(target, text).is_some(),
"{text:?} differs from the seed 1200, whitespace included, so it is a write"
);
}
}
/// BUG (improvise-ixxx) — see `context/test-evidence/clients.md`.
#[test]
fn a_mouse_commit_is_judged_against_the_seed_it_opened_with() {
let mut client = client_showing(Some(CellId::Record(record("Revenue"))));
let target = client.edit_target_at(0, 0).expect("a record cell");
let mut moved = CellDisplay::number("5");
moved.id = Some(CellId::Record(record("Revenue")));
client.fold_frame(ServerFrame::Commands(vec![Command::Project(
Projection::Grid(GridUpdate::Cells(vec![((0, 0), moved)])),
)]));
assert_eq!(
client
.state()
.grid
.cell(0, 0)
.and_then(|c| c.edit_text.clone()),
Some("5".into()),
"the other client's write did not reach this cache, so this proves nothing"
);
assert_eq!(
client.typed_target_frame(target, "1200"),
None,
"an untouched editor wrote its stale seed over the other client's value"
);
}
fn client_holding(mut cell: CellDisplay) -> Client {
cell.id = Some(CellId::Record(record("Revenue")));
let mut client = init();
client.bootstrap(Snapshot {
view: ViewState::default(),
seed: vec![
Projection::Grid(GridUpdate::RowFrame(AxisFrame::flat(["North"]))),
Projection::Grid(GridUpdate::ColFrame(AxisFrame::flat(["Revenue"]))),
Projection::Grid(GridUpdate::Cells(vec![((0, 0), cell)])),
Projection::Basis(published_basis()),
],
});
client
}
/// The basis a server would publish for the one record this fixture's
/// grid names.
fn published_basis() -> improvise_protocol::Basis {
@@ -329,7 +402,7 @@ mod tests {
);
assert_eq!(client_showing(None).typed_cell_frame(0, 0, "42"), None);
assert_eq!(
client_showing(Some(CellId::Record(record("Revenue")))).typed_cell_frame(9, 9, "42"),
client_showing(Some(CellId::Record(record("Revenue")))).typed_cell_frame(9, 9, "42",),
None,
"and neither is a position outside the grid"
);
crates/improvise-wasm-client/tests/lockstep.rs
+55 -28
View File
@@ -2108,7 +2108,6 @@ fn precise_host(value: improvise_protocol::CellValue) -> App {
#[test]
fn an_untouched_browser_edit_stores_the_identical_value() {
use improvise_protocol::CellValue::{Number, Text};
let window = (2, 2);
for value in [
Number(1.23456789),
Number(0.1 + 0.2),
@@ -2119,36 +2118,64 @@ fn an_untouched_browser_edit_stores_the_identical_value() {
Number(f64::NEG_INFINITY),
Text("0.1 + 0.2".into()),
] {
let mut host = precise_host(value.clone());
let mut client = client_ready_to_edit(&host, window);
for key in ["i", "Enter"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("{key}: the client must answer this key locally");
};
for cmd in &cmds {
client.fold(cmd);
}
apply_on(&mut host, &cmds);
}
assert!(
!client.view.mode.is_editing(),
"Enter did not commit, so nothing was written and this proves nothing"
);
let stored = host
.model_state
.workbook
.model
.get_cell(&region_measure("South", "Revenue"))
.cloned();
let same = match (&stored, &value) {
(Some(Number(a)), Number(b)) => a.to_bits() == b.to_bits(),
(stored, value) => stored.as_ref() == Some(value),
};
assert!(same, "i then Enter on {value:?} stored {stored:?}");
untouched_browser_edit(value);
}
}
/// BUG (improvise-ixxx) — see `context/test-evidence/clients.md`.
#[test]
fn an_untouched_browser_edit_of_a_misread_value_writes_nothing() {
use improvise_protocol::CellValue::{Error, Text};
for value in [
Text("007".into()),
Text("1e3".into()),
Text("inf".into()),
Error("boom".into()),
] {
untouched_browser_edit(value);
}
}
/// `i` then `Enter` on (South, Revenue) holding `value`, through the resolver
/// and a real host: the seed is the stored value's, and the cell is unchanged.
fn untouched_browser_edit(value: improvise_protocol::CellValue) {
use improvise_protocol::CellValue::Number;
let mut host = precise_host(value.clone());
let mut client = client_ready_to_edit(&host, (2, 2));
for key in ["i", "Enter"] {
let event = BrowserKey::new(key).to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("{key}: the client must answer this key locally");
};
for cmd in &cmds {
client.fold(cmd);
}
apply_on(&mut host, &cmds);
if key == "i" {
assert_eq!(
improvise_protocol::read_buffer(&client.view, "edit"),
value.edit_text(),
"the editor was not seeded from the stored value (improvise-umi4)"
);
}
}
assert!(
!client.view.mode.is_editing(),
"Enter did not commit, so this proves nothing"
);
let stored = host
.model_state
.workbook
.model
.get_cell(&region_measure("South", "Revenue"))
.cloned();
let same = match (&stored, &value) {
(Some(Number(a)), Number(b)) => a.to_bits() == b.to_bits(),
(stored, value) => stored.as_ref() == Some(value),
};
assert!(same, "i then Enter on {value:?} stored {stored:?}");
}
/// BUG (improvise-umi4) — see `context/test-evidence/clients.md`.
#[test]
fn every_projected_edit_text_commits_back_to_its_value() {
harness/tests/contract.spec.js
+33
View File
@@ -287,6 +287,39 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
await expect(page.locator("#grid tbody td.selected")).toHaveCount(1);
});
// improvise-ixxx. The editor hands wasm the seed it captured at open, and
// only wasm decides an untouched commit writes nothing — JS must pass it.
test("an untouched mouse editor sends no write, and a touched one does", async ({
page,
}) => {
await page.addInitScript(() => {
window.__posted = [];
const post = Worker.prototype.postMessage;
Worker.prototype.postMessage = function (message, ...rest) {
window.__posted.push(String(message));
return post.call(this, message, ...rest);
};
});
await openApp(page);
const cells = page.locator("#grid tbody td");
const writes = () =>
page.evaluate(() => window.__posted.filter((m) => m.includes("SetCell")).length);
await cells.nth(4).click();
await expect(cells.nth(4).locator("input")).toHaveCount(1);
await page.keyboard.press("Enter");
await expect(page.locator("#grid input")).toHaveCount(0);
await page.waitForTimeout(300);
expect(await writes()).toBe(0);
// The control: without it a page that never sends anything passes above.
await cells.nth(4).click();
await expect(cells.nth(4).locator("input")).toHaveCount(1);
await page.keyboard.type("1");
await page.keyboard.press("Enter");
await expect.poll(writes).toBe(1);
});
// improvise-b8ot moved these five surfaces onto one handler per stable root.
// The three tests below are the properties a naive delegation loses, each of
// which is silent: the page still paints, and only the pointer stops working.