diff --git a/context/code/clients.md b/context/code/clients.md index 12bd469d..d1ff7085 100644 --- a/context/code/clients.md +++ b/context/code/clients.md @@ -2851,9 +2851,11 @@ they answer different questions: text it opened on, and whether the user has changed it**. It is portable state. It is composed optimistically at the keypress and folded the same way on the host, where `CommitAndAdvance` reads it. -- `ClientState::opened` (`OpenedCoordinate { record, category, version, document_generation }`) - answers **which record the rename names, and at which version the basis - claims it**. It is session-local and never serialized. +- `ClientState::opened` (`OpenedCell { record, target, version, document_generation }`) + answers **which cell the commit writes, and at which version the basis + claims it**. `target` is `Coordinate { category }` for a rename and + `Value { measure }` for a stored value. It is session-local and never + serialized. Neither answers "is an editor open": the mode does, for both. `opened_editor` is not cleared when the editor closes, only replaced when the next one opens. @@ -2875,3 +2877,30 @@ editor reopens with the typed text. `settle_opened` then captures the record at its **current** version, so a second `Enter` succeeds. That is the one-keystroke force `RecordMoved` promises (gotcha 43). The user has just been told that someone else changed the row. + +**A value editor commits to the cell it opened on, too (improvise-8xq0).** The +keyboard commit over a plain grid cell used to read the cursor at `Enter`: the +record from `record_at_cursor`, the measure from the `CellId` under it, and the +basis from `claim`, which reads the version as it is *now*. Another client's +`HideItem` renumbers the rows under an open editor, the cursor then names a +different cell, and the write landed there with the text typed for the first. +The inline `EditTarget` fixed this for the mouse editor (improvise-2wu3), but it +lives in the page, so the keyboard path never saw it. The fix reuses the rename +capture rather than adding a second one: `settle_opened` captures a `Record` +cell as `OpenedTarget::Value`, and `written` builds the `RecordCell` and the +`claim_as_read` basis from it. Nothing about the write is read at `Enter` except +the text. Whether the commit is a rename, a creating column or a value is still +decided by the cell under the cursor, as it was. + +Two things come with it. **The version is the one read at open**, so a write by +another client while the editor is open is refused `RecordMoved` on this path +as well: improvise-0j0f's rule, which the keyboard path had not had +(improvise-xz2z). And **a +value commit can now wait**: the capture waits for the host's answer to this +client's own last write, so `Enter` pressed again inside one round trip has no +target yet. It declines with the rename's "still waiting" status and writes +nothing. With no write in flight and no capture, the cursor is on no cell an +editor writes, and the commit answers `None` as before. The waiting decline is +still followed by the `Enter` binding's welded `clear-buffer`, so the typed +text is lost on that path, for a rename as for a value (improvise-joed). + diff --git a/context/code/protocol.md b/context/code/protocol.md index 10d6af24..f2975dd2 100644 --- a/context/code/protocol.md +++ b/context/code/protocol.md @@ -622,8 +622,8 @@ it, and the address it needs is the name. What was missing was therefore never an address but a **recognition**. `Create` carries the kind and nothing else, because the kind plus the typed name is the whole operand. It is not a `SetCell` target, so `render.rs::editable` stays -`Record`-only and `record_at_cursor` still answers `None` — both by matching, -which the compiler now forces. +`Record`-only and `opened_at_cursor` captures no editor target for it — both +by matching, which the compiler now forces. **No client composes it yet**, and that is one gap rather than this one: `keys.rs::commit` returns early for every records-mode commit, because a @@ -665,9 +665,9 @@ by surrogate and the new coordinate by value. `records_named` names the record, because a rename moves it (gotcha 43). Creating the item is internal to the effect. `render.rs::editable` still answers `false` for a coordinate: the page's inline editor writes stored cells only, and the rename's producer is the -keyboard path (`context/code/clients.md`). `typed_cell_command`, -`record_at_cursor` and `record_cell_at_cursor` still answer `None` for it, so -paste onto a coordinate is still improvise-kkcy. +keyboard path (`context/code/clients.md`). `typed_cell_command` and +`record_cell_at_cursor` still answer `None` for it, so paste onto a coordinate +is still improvise-kkcy. ### `CellDisplay::id` — `None` is not "aggregate" diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index 96102811..7183c150 100644 --- a/context/test-evidence/clients.md +++ b/context/test-evidence/clients.md @@ -5235,7 +5235,7 @@ still pinned by `an_edit_writes_the_cell_the_user_clicked_after_the_rows_renumbe Added when #221 merged #219 (improvise-vndi). vndi changed `claim_as_read` to take the document generation captured when an editor opened, so an inline edit opened before a `LoadModel` is refused `DocumentReplaced` rather than written -into the new document. The rename capture (`OpenedCoordinate`) now records +into the new document. The rename capture (`OpenedCoordinate`, now `OpenedCell`) records `document_generation` at open for the same reason, and the rename claims it. **No rival of that choice is observable, and this test says why.** B loads a @@ -5261,3 +5261,72 @@ sits on row 2, which in the answered grid is Zed, the record the first rename produced. The test's principle is unchanged: the second rename moves the row the client shows, and that row differs from the stale capture (West). Only its value table assumed Zed could not come up, so Zed was added to it. + +### a_keyboard_edit_writes_the_cell_it_opened_on_after_the_rows_renumber + +improvise-8xq0, the ticket's probe, through a real `Host` with two clients. A +presses `l`, `i` on North/Cost at (0,1), and types `42`. B hides North, and A +folds the relayed frames, so the rows renumber under the open editor. A then +presses `Enter`. `resolve.rs::commit` composed `SetCell` from the cell under +the cursor **at Enter**. The cursor had been reset to the origin, which now +held South/Revenue, so South/Revenue became 42 and the host accepted it. That +failed at the `South was written` assertion, on `gh/main` at `202e7eac` and on +#221's head `39821d5f` alike, so #221's coordinate capture did not reach this +path. + +The editor opens on North/**Cost**, not the first column, so the cell at Enter +differs from the opened one in both record and measure. A fix that kept the +record and read the measure at Enter would still write the wrong column. + +- **Rival: `written` takes the whole cell from the cursor at Enter** (the bug). + Fails only this test, at `South was written`. +- **Rival: the record from the capture, the measure from the cursor.** Fails + only this test, at `the edit must land on the cell the editor opened on`: + North/Cost stays empty. +- **Rival: the basis claimed at commit (`claim`) instead of as read.** Fails + this test too: after the hide, A's basis no longer names North's record, so + `claim` leaves it out and the host answers `RecordUnstated`. + +### a_keyboard_edit_is_refused_when_its_record_was_written_while_open + +improvise-xz2z: improvise-0j0f's rule on the keyboard path, which it had not +had. xz2z was filed separately (by agent-227-fixes, scoping improvise-3al1) and +has the same cause as 8xq0: `written` read its operands at `Enter`. Same setup, +but B writes 999 into North/Revenue while A's editor is open on North/Cost. A +record is the coordinate tuple minus `_Measure` (gotcha 40), so that write +moves the version of the record A is editing. The keyboard commit claimed the +version current at Enter, which A had already folded, so the host accepted it. +It failed at the `RecordMoved` assertion, on `gh/main` and on #221's head. + +- **Rival: the basis claimed at commit instead of as read.** Fails this test + (and the renumber test, above). + +### a_second_keyboard_commit_writes_the_cell_the_advance_landed_on + +A regression guard for the capture's lifetime, which the fix made +load-bearing for value cells. `i`, `5`, `Enter` on North/Revenue writes 12005, +advances to South/Revenue and reopens the editor there; `7`, `Enter` must write +South/Revenue. The capture is dropped when the commit's `ChangeMode(Normal)` +folds and retaken for the reopened editor once the host answers. + +- **Rival: `settle_opened` keeps a capture after the editor closes.** Fails + this test, the second commit writing North again. Before this test only #221's + two coordinate tests saw that rival + (`a_second_rename_after_a_reorder_renames_the_row_the_host_shows` and + `a_document_swap_closes_an_open_rename_editor`); nothing on a value cell did. + +### a_value_commit_before_the_host_answers_the_last_one_waits_out_loud + +With the value target captured only after the host answers this client's own +last write, `Enter` pressed again before that answer has no target. The test +commits once, folds the frame as sent and not yet answered, and presses +`Enter` again in the reopened editor. The fixture's second row is given a +`Record` identity, or `commit` stops at the cursor's cell before it reaches the +capture and the test would pass for the wrong reason. + +- **Rival: no waiting decline (`None`, as for a cell no editor writes).** Fails + only this test: the key goes upstream, which in a browser does nothing. + +The test asserts the status and that nothing is written, not the exact +command list: the `Enter` binding's welded `clear-buffer` still runs after the +decline and empties the buffer. That loss is improvise-joed. diff --git a/crates/improvise-protocol/src/client.rs b/crates/improvise-protocol/src/client.rs index df670a92..8a4b6502 100644 --- a/crates/improvise-protocol/src/client.rs +++ b/crates/improvise-protocol/src/client.rs @@ -44,23 +44,32 @@ pub struct ClientState { /// replay the rest. Why a queue: `context/code/clients.md`. #[serde(skip)] pub in_flight: std::collections::VecDeque, - /// The coordinate an open editor renames, captured as the user saw it. + /// The cell an open editor commits to, captured as the user saw it. /// When it is taken, and when not yet: `context/code/clients.md`. #[serde(skip)] - pub opened: Option, + pub opened: Option, } -/// A coordinate editor's target: the record, the category, and the record's -/// write counter when the editor opened (improvise-fdk1, improvise-0j0f). +/// A cell editor's target: the record, which cell of it, and the record's write +/// counter when the editor opened (improvise-fdk1, improvise-0j0f, improvise-8xq0). #[derive(Debug, Clone, PartialEq, Eq)] -pub struct OpenedCoordinate { +pub struct OpenedCell { pub record: RecordHash, - pub category: String, + pub target: OpenedTarget, pub version: u64, /// The document the editor opened on, so a swap refuses it (improvise-vndi). pub document_generation: u64, } +/// Which cell of the record an editor opened on, as the projection named it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum OpenedTarget { + /// A stored value: `SetCell`/`ClearCell` names it (improvise-8xq0). + Value { measure: Option }, + /// A records-mode coordinate: `RenameRecordCoordinate` names it. + Coordinate { category: String }, +} + /// One frame this client has sent. `awaited` is true exactly when the host /// will answer it, which is exactly when it can be refused — `clients.md`. #[derive(Debug, Clone, PartialEq)] @@ -181,19 +190,25 @@ impl ClientState { self.view.mode = settled_mode(&self.view.mode, self.grid.records); } - /// Capture the coordinate an open editor renames, but only from a cache that + /// Whether a frame the host will answer is still in flight, so this + /// client's cache does not yet show the host's answer to its own writes. + pub(crate) fn awaiting_answer(&self) -> bool { + self.in_flight.iter().any(|sent| sent.awaited) + } + + /// Capture the cell an open editor commits to, but only from a cache that /// shows the host's answer to this client's own writes (improvise-fdk1). fn settle_opened(&mut self) { - let answered = !self.in_flight.iter().any(|sent| sent.awaited); + let answered = !self.awaiting_answer(); self.opened = match (self.view.mode.is_editing(), self.opened.take()) { (false, _) => None, (true, Some(opened)) => Some(opened), (true, None) if answered => { - crate::resolve::coordinate_at_cursor(self).and_then(|(record, category)| { + crate::resolve::opened_at_cursor(self).and_then(|(record, target)| { let basis = self.basis.current()?; - Some(OpenedCoordinate { + Some(OpenedCell { record, - category, + target, version: basis.record(record)?, document_generation: basis.document_generation, }) diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index cdb2c78c..cd9e833c 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -4,7 +4,7 @@ use improvise_core::view::Axis; use serde::{Deserialize, Serialize}; -use crate::client::ClientState; +use crate::client::{ClientState, OpenedTarget}; use crate::frame::ClientFrame; use crate::keymap::{Binding, Keymaps, ModeKey}; use crate::viewmodel::{CatTreeEntry, axis_change_status, flatten_cat_tree}; @@ -800,7 +800,7 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option> { /// The record and category of the coordinate cell under the cursor, as the /// projection names them — never borrowed from a sibling cell (ADR 002). -pub(crate) fn coordinate_at_cursor(state: &ClientState) -> Option<(crate::RecordHash, String)> { +fn coordinate_at_cursor(state: &ClientState) -> Option<(crate::RecordHash, String)> { match cell_at_cursor(state)?.id.as_ref()? { CellId::Coordinate { record, category } => Some((*record, category.clone())), CellId::Record(_) | CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => { @@ -809,6 +809,26 @@ pub(crate) fn coordinate_at_cursor(state: &ClientState) -> Option<(crate::Record } } +/// The record and cell an editor opened on the cursor commits to, as the +/// projection names them (improvise-8xq0). `None` for a cell no editor writes. +pub(crate) fn opened_at_cursor(state: &ClientState) -> Option<(crate::RecordHash, OpenedTarget)> { + match cell_at_cursor(state)?.id.as_ref()? { + CellId::Record(cell) => Some(( + cell.record, + OpenedTarget::Value { + measure: cell.measure.clone(), + }, + )), + CellId::Coordinate { record, category } => Some(( + *record, + OpenedTarget::Coordinate { + category: category.clone(), + }, + )), + CellId::Aggregate(_) | CellId::Derived { .. } | CellId::Create(_) => None, + } +} + /// 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 { @@ -928,7 +948,9 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option vec![Command::SetStatus( RECORD_COORDS_CANNOT_BE_EMPTY.to_string(), )], - (false, false, None) => { - return Some(vec![Command::SetStatus( - "still waiting for the host to answer your last edit".to_string(), - )]); + (false, false, None) => return Some(still_waiting()), + (false, false, Some(opened)) => { + let OpenedTarget::Coordinate { category } = &opened.target else { + return Some(still_waiting()); + }; + vec![Command::Model { + basis: state.basis.claim_as_read( + opened.document_generation, + [(opened.record, opened.version)], + )?, + cmd: ModelCommand::RenameRecordCoordinate { + record: opened.record, + category: category.clone(), + item: text.to_string(), + }, + }] } - (false, false, Some(opened)) => vec![Command::Model { - basis: state.basis.claim_as_read( - opened.document_generation, - [(opened.record, opened.version)], - )?, - cmd: ModelCommand::RenameRecordCoordinate { - record: opened.record, - category: opened.category.clone(), - item: text.to_string(), - }, - }], }; 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> { - 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)?, - }]), +/// The write a commit makes: none when untouched (improvise-ixxx), else to the +/// cell the editor OPENED on, at the version it read then (improvise-8xq0). +fn written(state: &ClientState, text: &str) -> Option> { + if untouched(state.view.opened_editor.as_ref(), text) { + return Some(Vec::new()); } + let opened = state.opened.as_ref()?; + let OpenedTarget::Value { measure } = &opened.target else { + return None; + }; + let cell = RecordCell { + record: opened.record, + measure: measure.clone(), + }; + Some(vec![Command::Model { + basis: state.basis.claim_as_read( + opened.document_generation, + [(opened.record, opened.version)], + )?, + cmd: typed_cell_command(&CellId::Record(cell), text)?, + }]) +} + +/// A commit whose target is not captured yet, because the host has not answered +/// this client's own last write. The editor stays open (improvise-fdk1). +fn still_waiting() -> Vec { + vec![Command::SetStatus( + "still waiting for the host to answer your last edit".to_string(), + )] } /// `CommitAndAdvance` over a creating column: a schema change over the typed @@ -1054,18 +1096,6 @@ fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option Some(commands) } -/// The surrogate naming the record under the cursor, or `None` when the -/// cursor is not on a writable cell. -fn record_at_cursor(state: &ClientState) -> Option { - match cell_at_cursor(state)?.id.as_ref()? { - CellId::Record(cell) => Some(cell.record), - CellId::Aggregate(_) - | CellId::Derived { .. } - | CellId::Create(_) - | CellId::Coordinate { .. } => None, - } -} - /// What an editor opened on the cursor starts with, or `None` with no cell /// there to seed from. Read by the fold, never a renderer (improvise-ktlk). pub(crate) fn edit_seed(state: &ClientState) -> Option { @@ -2265,7 +2295,7 @@ mod tests { // A records MODE over a non-records GRID is a state no host can // produce, so the fixture projects both (improvise-e5pf). crate::WidgetCache::apply(&mut state.grid, &GridUpdate::Records(mode.is_records())); - state.view.mode = mode; + state.fold(&Command::ChangeMode(mode)); state.grid.page_coords = vec![crate::PageAxis { cat: "Quarter".into(), selected: "Q2".into(), @@ -2978,7 +3008,7 @@ mod tests { // Half two: the predicate is about *finishing*, so something has to // press Enter and observe. let mut client = client_on(record_cell()); - client.view.mode = AppMode::editing(); + client.fold(&Command::ChangeMode(AppMode::editing())); client.view.buffers.insert("edit".into(), "42".into()); let KeyOutcome::Local(commands) = resolve(&client, ev(KeyCode::Enter)) else { panic!("Enter in Editing must resolve here — that is the arm"); @@ -3432,12 +3462,43 @@ mod tests { ); } + /// improvise-8xq0: a commit whose target is not captured yet, because the + /// last write is unanswered, declines out loud. Evidence file. + #[test] + fn a_value_commit_before_the_host_answers_the_last_one_waits_out_loud() { + let mut client = client_on(record_cell()); + // The row the advance lands on is a stored cell too, so the commit gets + // past the cursor's cell and reaches the capture. + let mut below = CellDisplay::number("900"); + below.id = Some(record_cell()); + client.fold(&Command::Project(Projection::Grid(GridUpdate::Cells( + vec![((1, 0), below)], + )))); + client.fold(&Command::ChangeMode(AppMode::editing())); + client.view.buffers.insert("edit".into(), "42".into()); + let KeyOutcome::Local(first) = resolve(&client, ev(KeyCode::Enter)) else { + panic!("Enter in Editing must resolve here"); + }; + client.fold_outgoing(&first); + assert!(client.view.mode.is_editing(), "setup: the advance reopened"); + assert_eq!(client.opened, None, "setup: nothing captured yet"); + client.view.buffers.insert("edit".into(), "43".into()); + let KeyOutcome::Local(second) = resolve(&client, ev(KeyCode::Enter)) else { + panic!("a commit with no target yet must answer, not go upstream"); + }; + assert_eq!(second[..1], still_waiting()[..], "it says so: {second:?}"); + assert!( + !second.iter().any(|c| matches!(c, Command::Model { .. })), + "and it must write nothing: {second:?}" + ); + } + /// Enter in `Editing` is `[commit-cell-edit editing, clear-buffer edit]` /// and every part shows up here — including the odd-looking trailing clear. #[test] fn enter_commits_the_cell_advances_and_re_enters_editing() { let mut client = client_on(record_cell()); - client.view.mode = AppMode::editing(); + client.fold(&Command::ChangeMode(AppMode::editing())); client.view.buffers.insert("edit".into(), "42".into()); let KeyOutcome::Local(commands) = resolve(&client, ev(KeyCode::Enter)) else { panic!("Enter in Editing must resolve here"); @@ -3733,7 +3794,7 @@ mod tests { #[test] fn committing_an_empty_buffer_clears_the_cell() { let mut client = client_on(record_cell()); - client.view.mode = AppMode::editing(); + client.fold(&Command::ChangeMode(AppMode::editing())); client.view.buffers.insert("edit".into(), String::new()); let KeyOutcome::Local(commands) = resolve(&client, ev(KeyCode::Enter)) else { panic!("Enter in Editing must resolve here"); @@ -3814,7 +3875,7 @@ mod tests { ], }); client.window = (1, 1); - client.view.mode = AppMode::editing(); + client.fold(&Command::ChangeMode(AppMode::editing())); client.view.buffers.insert("edit".into(), "7".into()); let KeyOutcome::Local(commands) = resolve(&client, ev(KeyCode::Enter)) else { diff --git a/crates/improvise-wasm-client/tests/browser_round_trip.rs b/crates/improvise-wasm-client/tests/browser_round_trip.rs index 279cb473..eb6f5127 100644 --- a/crates/improvise-wasm-client/tests/browser_round_trip.rs +++ b/crates/improvise-wasm-client/tests/browser_round_trip.rs @@ -1311,3 +1311,151 @@ fn a_second_rename_after_a_reorder_renames_the_row_the_host_shows() { "West untouched" ); } + +// ── the keyboard editor's target (improvise-8xq0) ──────────────────────── + +/// What client B does while A's keyboard editor is open on North/Cost. +#[derive(PartialEq)] +enum Meanwhile { + HideNorth, + WriteNorth, +} + +/// Client A presses `i` on North/Cost (0,1) and types `42`; B does `meanwhile` +/// before A presses Enter. Evidence file. +fn commit_north_cost(meanwhile: Meanwhile) -> (Host, Vec) { + use improvise_command::ui::reduce::caught_up; + use improvise_protocol::ModelCommand; + + let mut host = host(); + let (mut a, mut b) = (Client::new(), Client::new()); + let (a_id, a_snap) = host.connect(10, 10); + let (b_id, b_snap) = host.connect(10, 10); + a.set_viewport(10, 10); + b.set_viewport(10, 10); + deliver(&mut a, &[a_snap]); + deliver(&mut b, &[b_snap]); + for key in ["l", "i", "4", "2"] { + press(&mut host, a_id, &mut a, key); + } + assert_eq!( + a.state().view.cursor().selected, + (0, 1), + "setup: on North/Cost" + ); + assert!( + a.state().view.mode.is_editing(), + "setup: the editor is open" + ); + + let view = host.app().view_name().to_string(); + let cmd = match meanwhile { + Meanwhile::HideNorth => ModelCommand::HideItem { + view, + category: "Region".into(), + item: "North".into(), + }, + Meanwhile::WriteNorth => ModelCommand::SetCell { + cell: a.record_cell_at(0, 0).expect("North/Revenue is stored"), + value: CellValue::Number(999.0), + }, + }; + let relayed = host.handle(b_id, ClientFrame::command(caught_up(host.app(), cmd))); + route(&relayed, (a_id, &mut a), (b_id, &mut b)); + assert!(a.state().view.mode.is_editing(), "setup: still editing"); + if meanwhile == Meanwhile::HideNorth { + assert_eq!( + a.state().grid.row_frame.leaf_labels().first().cloned(), + Some("South".to_string()), + "setup: the rows renumbered under the open editor" + ); + } + + let frame = a + .key(&BrowserKey::new("Enter")) + .expect("a commit composes a frame"); + let json = serde_json::to_string(&frame).expect("a client frame serializes"); + let answers = send(&mut host, a_id, &json); + (host, answers) +} + +fn cell(host: &Host, region: &str, measure: &str) -> Option { + host.app() + .model_state + .workbook + .model + .get_cell(&CellKey::new(vec![ + ("Region".into(), region.into()), + ("_Measure".into(), measure.into()), + ])) + .cloned() +} + +/// improvise-8xq0: B hid the row under A's open editor, and Enter wrote the +/// cell under the cursor at Enter. Evidence: `context/test-evidence/clients.md`. +#[test] +fn a_keyboard_edit_writes_the_cell_it_opened_on_after_the_rows_renumber() { + let (host, answers) = commit_north_cost(Meanwhile::HideNorth); + assert!( + answers + .iter() + .any(|f| matches!(f.msg, improvise_protocol::ServerFrame::Accepted)), + "the write must be accepted: {answers:?}" + ); + assert_eq!(cell(&host, "South", "Revenue"), None, "South was written"); + assert_eq!(cell(&host, "South", "Cost"), None, "South was written"); + assert_eq!( + cell(&host, "North", "Cost"), + Some(CellValue::Number(42.0)), + "the edit must land on the cell the editor opened on" + ); +} + +/// improvise-8xq0: the editor an advance reopens commits to the cell it landed +/// on, not the one the last commit captured. Evidence file. +#[test] +fn a_second_keyboard_commit_writes_the_cell_the_advance_landed_on() { + let mut host = host(); + let mut a = Client::new(); + a.set_viewport(10, 10); + let (id, snapshot) = host.connect(10, 10); + deliver(&mut a, &[snapshot]); + for key in ["i", "5", "Enter"] { + press(&mut host, id, &mut a, key); + } + assert_eq!(a.state().view.cursor().selected, (1, 0), "setup: advanced"); + assert!(a.state().view.mode.is_editing(), "setup: reopened"); + for key in ["7", "Enter"] { + press(&mut host, id, &mut a, key); + } + assert_eq!( + cell(&host, "North", "Revenue"), + Some(CellValue::Number(12005.0)) + ); + assert_eq!( + cell(&host, "South", "Revenue"), + Some(CellValue::Number(7.0)), + "the second commit must write the cell the advance landed on" + ); +} + +/// improvise-xz2z, 0j0f's rule on the keyboard path: the basis is the one read +/// when the editor opened. Evidence: `context/test-evidence/clients.md`. +#[test] +fn a_keyboard_edit_is_refused_when_its_record_was_written_while_open() { + let (host, answers) = commit_north_cost(Meanwhile::WriteNorth); + assert!( + answers.iter().any(|f| matches!( + f.msg, + improvise_protocol::ServerFrame::Rejected(improvise_protocol::Rejection::RecordMoved( + _ + )) + )), + "an edit composed against a record someone else wrote must be refused: {answers:?}" + ); + assert_eq!( + cell(&host, "North", "Revenue"), + Some(CellValue::Number(999.0)) + ); + assert_eq!(cell(&host, "North", "Cost"), None, "nothing was written"); +} diff --git a/crates/improvise-wasm-client/tests/lockstep.rs b/crates/improvise-wasm-client/tests/lockstep.rs index ee3bc6f2..d1a0e23e 100644 --- a/crates/improvise-wasm-client/tests/lockstep.rs +++ b/crates/improvise-wasm-client/tests/lockstep.rs @@ -1610,13 +1610,15 @@ fn a_records_commit_is_declined_off_the_grid_and_not_off_the_mode() { // The control: the same mode over a REGULAR grid must still commit, or the // decline above is "this client never writes" and proves nothing. let plain = furnished_host(); - let mut ordinary = paged_client(&plain); - ordinary.view.mode = AppMode::editing(); + // Seeded as `records_client` seeds, basis included, over a regular grid; + // opened through the fold, which captures the cell the commit writes. + let mut ordinary = records_client(&plain); + ordinary.view.cursor_mut().selected = stored_cell_at(&ordinary); + ordinary.fold(&improvise_protocol::Command::ChangeMode(AppMode::editing())); ordinary .view .buffers .insert("edit".into(), "42".to_string()); - ordinary.view.cursor_mut().selected = stored_cell_at(&ordinary); assert!(!ordinary.grid.records, "the control must be a regular grid"); assert!( composes_a_write(&ordinary, enter),