The keyboard editor commits to the cell it opened on (improvise-8xq0, improvise-xz2z)
resolve.rs::written read its operands at Enter: the record and measure from the cell under the cursor, and the basis from claim(), i.e. the version current at commit. So a HideItem from another client that renumbered the rows under an open editor sent the write to whichever cell the cursor then named (8xq0), and a write by another client to the record while the editor was open was silently overwritten instead of refused RecordMoved (xz2z, 0j0f's rule on the keyboard door). The fix reuses #221's capture rather than adding a second one. ClientState::opened becomes OpenedCell { record, target, version, document_generation }, where OpenedTarget is Value { measure } or Coordinate { category }. settle_opened captures a Record cell as a Value target, and written builds the RecordCell and the claim_as_read basis from it. With no capture while the host has yet to answer this client's own last write, the commit declines with the rename's "still waiting" status. Red first: both round-trip probes failed on gh/main202e7eacand on #221's head39821d5f. Unit-test fixtures that set Editing by assignment now open the editor through the fold, which is where the capture is taken. Stacked on #221 (fix/fdk1-coordinate-rename). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
+32
-3
@@ -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).
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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<Sent>,
|
||||
/// 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<OpenedCoordinate>,
|
||||
pub opened: Option<OpenedCell>,
|
||||
}
|
||||
|
||||
/// 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<String> },
|
||||
/// 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,
|
||||
})
|
||||
|
||||
@@ -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<Vec<Command>> {
|
||||
|
||||
/// 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<Command> {
|
||||
@@ -928,7 +948,9 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
|
||||
if state.grid.records {
|
||||
return rename_coordinate(state, edit_mode, advance, id, &text);
|
||||
}
|
||||
let mut commands = written(state, id, &text)?;
|
||||
let Some(mut commands) = written(state, &text) else {
|
||||
return state.awaiting_answer().then(still_waiting);
|
||||
};
|
||||
commands.push(Command::ChangeMode(AppMode::Normal));
|
||||
commands.extend(advanced(state, edit_mode, advance)?);
|
||||
Some(commands)
|
||||
@@ -954,38 +976,58 @@ fn rename_coordinate(
|
||||
(false, true, _) => 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<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)?,
|
||||
}]),
|
||||
/// 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<Vec<Command>> {
|
||||
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<Command> {
|
||||
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<crate::RecordHash> {
|
||||
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<String> {
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<Addressed>) {
|
||||
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<CellValue> {
|
||||
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");
|
||||
}
|
||||
|
||||
@@ -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),
|
||||
|
||||
Reference in New Issue
Block a user