Merge #221 (c7b5b825, improvise-fdk1 coordinate rename) into the editor integration branch

Resolutions:
- CellId: #221 gives Coordinate fields { record, category }; main (#292)
  adds NonItem { category, item }. Every exhaustive match now names both
  (render.rs, core.rs x2, project.rs, resolve.rs x3, wasm-client lib.rs and
  render.rs); project.rs's basis arm keeps #221's Coordinate-carries-a-record.
- resolve.rs edit_or_drill: #221 drops the "renaming a coordinate" decline
  (Coordinate opens an editor); main's NonItem decline arm is kept.
  coordinate_at_cursor's None arm gains NonItem. reopened_at keeps #295's
  declined_at shape; declined_at's editable arm names Coordinate { .. }.
- client.rs: kept main's status_seen/status_owner and #221's opened +
  OpenedCoordinate; every settle site calls both settle_opened and
  settle_status. Main's new rebase() also gets settle_opened, since it
  rebuilds the view exactly as apply() does.
- view/mod.rs: re-export both GridEval (main) and record_row (#221).
- context/code/command.md and context/test-evidence/clients.md: both sides
  appended independent sections; kept both.

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:
Edward Langley
2026-09-24 12:13:42 -07:00
22 changed files with 1177 additions and 49 deletions
@@ -933,7 +933,13 @@ fn commit_plain_records_edit(
return;
};
match col {
RecordCol::Category(_) | RecordCol::Measure(_) => {
// The one door the wire's `RenameRecordCoordinate` also takes (improvise-fdk1).
RecordCol::Category(category) => effects.push(Box::new(effect::RenameRecordCoordinate {
record: row.coords.clone(),
category: category.clone(),
item: value.to_string(),
})),
RecordCol::Measure(_) => {
effects.extend(effect::RecordRewrite::of(ctx.model, row, [(col, value)]).effects());
}
RecordCol::NewMeasure => add_measure(value, effects),
@@ -40,7 +40,7 @@ pub fn uneditable(model: &Model, layout: &GridLayout, row: usize, col: usize) ->
),
Some(CellId::Derived { target }) => Some(derived_refusal(&target)),
Some(CellId::NonItem { category, item }) => Some(non_item_refusal(&category, &item)),
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate) | None => None,
Some(CellId::Record(_) | CellId::Create(_) | CellId::Coordinate { .. }) | None => None,
}
}
@@ -130,7 +130,9 @@ impl<'a> CmdContext<'a> {
Some(CellId::Record(_)) => self.stored_cell_key(),
Some(CellId::Aggregate(_)) => Err(self.aggregate_refusal()),
Some(CellId::Derived { target }) => Err(derived_refusal(&target)),
Some(CellId::Create(_) | CellId::Coordinate) => Err(NO_VALUE_REFUSAL.to_string()),
Some(CellId::Create(_) | CellId::Coordinate { .. }) => {
Err(NO_VALUE_REFUSAL.to_string())
}
Some(CellId::NonItem { category, item }) => Err(non_item_refusal(&category, &item)),
None => Err(match self.cell_key() {
Some(_) => NO_VALUE_REFUSAL,
+24 -1
View File
@@ -13,7 +13,7 @@ use super::app::{App, AppMode, ViewFrame};
/// inverses) and re-exported here, where most callers already look for it.
pub use super::app::Panel;
pub(crate) const RECORD_COORDS_CANNOT_BE_EMPTY: &str = "Record coordinates cannot be empty";
pub(crate) use improvise_protocol::RECORD_COORDS_CANNOT_BE_EMPTY;
/// What an effect changes, so a server can decide whether connected clients
/// have to be told about it (improvise-mae).
@@ -759,6 +759,29 @@ impl Effect for ApplyAndClearDrill {
}
}
/// Move the record at `record` to `category = item`: the key path's and the wire's
/// one door, through the same [`RecordRewrite`] a drill uses (improvise-fdk1).
#[derive(Debug)]
pub struct RenameRecordCoordinate {
pub record: CellKey,
pub category: String,
pub item: String,
}
impl Effect for RenameRecordCoordinate {
fn apply(&self, app: &mut App) {
let model = &app.model_state.workbook.model;
let row = crate::view::record_row(model, &self.record);
let col = RecordCol::Category(self.category.clone());
let rewrite = RecordRewrite::of(model, &row, [(&col, self.item.as_str())]);
for effect in rewrite.effects() {
effect.apply(app);
}
}
fn kind(&self) -> EffectKind {
EffectKind::Model(ModelChange::CellData)
}
}
/// A records row's edits composed into the one record they mean, read off the model
/// before anything is written. Why composed: `context/code/command.md`, `RecordRewrite`.
#[derive(Debug, Default, PartialEq)]
+74
View File
@@ -1926,6 +1926,80 @@ mod tests {
);
}
/// improvise-fdk1: a surrogate names a record as it WAS, and a rename ends that
/// name. See `context/test-evidence/command.md`.
#[test]
fn a_write_to_a_record_its_own_frame_renamed_is_a_broken_chain() {
use improvise_protocol::ModelCommand;
let mut host = host();
host.app
.model_state
.workbook
.model
.set_cell(cell("North"), CellValue::Number(1.0));
host.app.rebuild_layout();
// A records host: its layout names positions, so an old surrogate cannot
// resolve through it. A pivot host differs — see the evidence.
crate::ui::reduce::reduce_key(
&mut host.app,
crate::command::key::KeyEvent::new(
crate::command::key::KeyCode::Char('R'),
crate::command::key::KeyModifiers::NONE,
),
);
assert!(host.app.layout.is_records_mode(), "setup: a records host");
let (asker, _) = host.connect(10, 10);
let north = addressed("North").record;
let rename = || ModelCommand::RenameRecordCoordinate {
record: north,
category: "Region".into(),
item: "West".into(),
};
let stored = |host: &Host, region: &str| {
host.app()
.model_state
.workbook
.model
.get_cell(&cell(region))
.cloned()
};
let pair = vec![
crate::ui::reduce::caught_up(host.app(), rename()),
crate::ui::reduce::caught_up(
host.app(),
ModelCommand::SetCell {
cell: addressed("North"),
value: CellValue::Number(7.0),
},
),
];
let answer = host.handle(asker, ClientFrame::Commands(pair));
assert!(
answer.iter().any(|a| matches!(
a.msg,
ServerFrame::Rejected(Rejection::ChainBroken(h)) if h == north
)),
"the write names a record the rename moved away: {answer:?}"
);
assert_eq!(stored(&host, "North"), Some(CellValue::Number(1.0)));
assert_eq!(stored(&host, "West"), None, "clause 4 undid the rename too");
// The control: the rename alone is accepted and moves the record, or
// the refusal above is "renames never work" and proves nothing.
let alone = crate::ui::reduce::caught_up(host.app(), rename());
let answer = host.handle(asker, ClientFrame::Commands(vec![alone]));
assert!(
answer
.iter()
.any(|a| matches!(a.msg, ServerFrame::Accepted)),
"{answer:?}"
);
assert_eq!(stored(&host, "West"), Some(CellValue::Number(1.0)));
assert_eq!(stored(&host, "North"), None);
}
// ── the broadcast ───────────────────────────────────────────────────
/// Which view a client is on is that client's (improvise-jytm): an accepted
+56 -7
View File
@@ -355,8 +355,13 @@ pub(crate) fn cell_id(
RecordCol::NewMeasure => return Some(CellId::Create(CreateKind::Measure)),
RecordCol::NewCategory => return Some(CellId::Create(CreateKind::Category)),
// Renaming a coordinate moves every measure cell in the row, which is a
// different intent from `SetCell` and still the host's (gotcha 1).
RecordCol::Category(_) => return Some(CellId::Coordinate),
// different intent from `SetCell`: its own operands (gotcha 1, fdk1).
RecordCol::Category(category) => {
return Some(CellId::Coordinate {
record: RecordHash::of(record),
category: category.clone(),
});
}
};
// The refusal's own question, asked here too, so no cell is offered
// for a write the host would then refuse (improvise-gk51).
@@ -547,11 +552,12 @@ pub fn published_basis(app: &App, grid: &GridCache) -> Basis {
.iter()
.flatten()
.filter_map(|cell| match cell.id.as_ref()? {
CellId::Record(rc) => Some((rc.record, app.record_version_of(rc.record))),
CellId::Record(RecordCell { record, .. }) | CellId::Coordinate { record, .. } => {
Some((*record, app.record_version_of(*record)))
}
CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::Coordinate
| CellId::NonItem { .. } => None,
})
.collect();
@@ -5387,6 +5393,43 @@ mod cell_identity {
);
}
/// improvise-fdk1: a record named only by its coordinate cell is still
/// published, or its rename is RecordUnstated. `context/test-evidence/command.md`.
#[test]
fn a_record_named_only_by_its_coordinate_is_in_the_published_basis() {
let mut wb = Workbook::new("Anchors");
wb.add_category("Region").unwrap();
wb.add_item("Region", "North").unwrap();
let north = CellKey::new(vec![("Region".to_string(), "North".to_string())]);
wb.model.set_cell(north.clone(), CellValue::Number(1.0));
let mut app = App::new(wb, None);
app.handle_key(crate::command::key::KeyEvent::new(
crate::command::key::KeyCode::Char('R'),
crate::command::key::KeyModifiers::NONE,
))
.expect("R enters records mode");
let grid = grid_of(&app, WHOLE);
let ids: Vec<CellId> = grid
.cells
.iter()
.flatten()
.filter_map(|c| c.id.clone())
.collect();
assert!(
!ids.iter().any(|id| matches!(id, CellId::Record(_))),
"setup: no measure column, so only the coordinate names the record: {ids:?}"
);
assert!(
ids.iter().any(|id| matches!(id, CellId::Coordinate { .. })),
"setup: the coordinate is projected: {ids:?}"
);
let basis = published_basis(&app, &grid);
assert!(
basis.record(RecordHash::of(&north)).is_some(),
"the coordinate's record must be published: {basis:?}"
);
}
/// improvise-exdg: a coordinate cell is drawn and holds text, so it is not
/// "no cell". `context/test-evidence/command.md`.
#[test]
@@ -5396,10 +5439,14 @@ mod cell_identity {
let region = column_named(&app, "Region");
let north = grid.cell(1, region).expect("the second row's coordinate");
assert_eq!(north.text, "North", "setup: a visible coordinate cell");
let record = app.layout.record_row(1).expect("row 1").coords.clone();
assert_eq!(
north.id,
Some(CellId::Coordinate),
"a drawn coordinate is named"
Some(CellId::Coordinate {
record: RecordHash::of(&record),
category: "Region".into(),
}),
"a drawn coordinate is named, with its own record and category (fdk1)"
);
// Each cell's kind against its column's, not merely "some kind" (improvise-w1bu).
let misnamed: Vec<(usize, usize, Option<CellId>)> = (0..app.layout.row_count())
@@ -5417,7 +5464,9 @@ mod cell_identity {
/// so `Derived` is never right here (improvise-hoxy).
fn names_its_column(col: Option<&RecordCol>, id: Option<&CellId>) -> bool {
match (col, id) {
(Some(RecordCol::Category(_)), Some(CellId::Coordinate)) => true,
(Some(RecordCol::Category(c)), Some(CellId::Coordinate { category, .. })) => {
category == c
}
(Some(RecordCol::Measure(m)), Some(CellId::Record(cell))) => {
cell.measure.as_deref() == Some(m.as_str())
}
+20
View File
@@ -149,6 +149,20 @@ pub fn model_effects(app: &App, cmd: &ModelCommand) -> Vec<Box<dyn Effect>> {
(None, None) => return vec![],
(None, Some(key)) => Box::new(effect::ClearCell(key)),
},
ModelCommand::RenameRecordCoordinate {
record,
category,
item,
} => {
let Some(record) = app.resolve_record(*record) else {
return vec![];
};
Box::new(effect::RenameRecordCoordinate {
record,
category: category.clone(),
item: item.clone(),
})
}
ModelCommand::AddItem { category, item } => Box::new(effect::AddItem {
category: category.clone(),
item: item.clone(),
@@ -667,6 +681,11 @@ pub(crate) mod tests {
value: CellValue::Number(1.0),
},
ModelCommand::ClearCell(record_cell(&a_stored_cell())),
ModelCommand::RenameRecordCoordinate {
record: record_cell(&a_stored_cell()).record,
category: "Region".into(),
item: "West".into(),
},
ModelCommand::AddItem {
category: "Region".into(),
item: "South".into(),
@@ -755,6 +774,7 @@ pub(crate) mod tests {
match cmd {
ModelCommand::SetCell { .. } => "SetCell",
ModelCommand::ClearCell(_) => "ClearCell",
ModelCommand::RenameRecordCoordinate { .. } => "RenameRecordCoordinate",
ModelCommand::AddItem { .. } => "AddItem",
ModelCommand::AddItemInGroup { .. } => "AddItemInGroup",
ModelCommand::RemoveItem { .. } => "RemoveItem",
+12
View File
@@ -107,6 +107,18 @@ pub fn drill_snapshot(model: &Model, constraint: &CellKey) -> Vec<RecordRow> {
RecordRow::group(cells)
}
/// The records row `record` is, as the records grid groups it; empty when no
/// cell stores it. The one lookup a rename by record uses (improvise-fdk1).
pub fn record_row(model: &Model, record: &CellKey) -> RecordRow {
drill_snapshot(model, record)
.into_iter()
.find(|row| &row.coords == record)
.unwrap_or_else(|| RecordRow {
coords: record.clone(),
values: Vec::new(),
})
}
/// One step of a view configuration: **what** a view must be, with no answer
/// to how a front end gets there. The model half of drill — core.md §8.
#[derive(Debug, Clone, PartialEq, Eq)]
+2 -2
View File
@@ -5,8 +5,8 @@ pub mod types;
pub use axis::{Axis, AxisColor};
pub use layout::{
AxisEntry, GridEval, GridLayout, NEW_CATEGORY_COL, NEW_MEASURE_COL, RecordCol, RecordRow,
ViewStep, drill_constraint, drill_snapshot, drill_steps, records_spine, records_steps,
synthetic_record_info,
ViewStep, drill_constraint, drill_snapshot, drill_steps, record_row, records_spine,
records_steps, synthetic_record_info,
};
pub use types::{
DEFAULT_VIEW, DRILL_VIEW, RECORDS_VIEW, VIRTUAL_VIEW_NAMES, View, unused_view_name,
+43
View File
@@ -6,6 +6,7 @@ use crate::{
Projection, Rejection, Snapshot, TileBarCache, ViewListCache, ViewState, WidgetCache,
reduce_view, settled_mode,
};
use improvise_core::model::record::RecordHash;
use serde::Serialize;
/// One client's whole state: the portable session slice plus one cache per
@@ -49,6 +50,21 @@ pub struct ClientState {
pub status_seen: String,
#[serde(skip)]
pub status_owner: Option<EditorSession>,
/// The coordinate an open editor renames, captured as the user saw it.
/// When it is taken, and when not yet: `context/code/clients.md`.
#[serde(skip)]
pub opened: Option<OpenedCoordinate>,
}
/// A coordinate editor's target: the record, the category, and the record's
/// write counter when the editor opened (improvise-fdk1, improvise-0j0f).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct OpenedCoordinate {
pub record: RecordHash,
pub category: String,
pub version: u64,
/// The document the editor opened on, so a swap refuses it (improvise-vndi).
pub document_generation: u64,
}
/// One frame this client has sent. `awaited` is true exactly when the host
@@ -84,6 +100,7 @@ impl ClientState {
}
self.settle_mode();
self.settle_editor();
self.settle_opened();
self.settle_status();
self.revision += 1;
}
@@ -105,6 +122,7 @@ impl ClientState {
self.replay();
self.settle_mode();
self.settle_editor();
self.settle_opened();
self.settle_status();
self.revision += 1;
}
@@ -139,6 +157,7 @@ impl ClientState {
}
self.settle_mode();
self.settle_editor();
self.settle_opened();
self.settle_status();
self.revision += 1;
}
@@ -174,6 +193,7 @@ impl ClientState {
}
self.settle_sent();
self.settle_editor();
self.settle_opened();
self.revision += 1;
}
@@ -190,6 +210,7 @@ impl ClientState {
self.last_rejection = Some(rejection);
self.settle_mode();
self.settle_editor();
self.settle_opened();
self.settle_status();
self.revision += 1;
}
@@ -233,6 +254,28 @@ 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
/// 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);
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)| {
let basis = self.basis.current()?;
Some(OpenedCoordinate {
record,
category,
version: basis.record(record)?,
document_generation: basis.document_generation,
})
})
}
(true, None) => None,
};
}
/// An editor that arrived without its buffer is seeded here, so what is
/// drawn and what Enter commits read one buffer (improvise-ktlk).
fn settle_editor(&mut self) {
+3 -1
View File
@@ -27,7 +27,9 @@ pub use frame::{ClientFrame, ServerFrame};
pub use help::{HelpLine, Hint, hint_line, hints, mode_label};
pub use key::{KeyCode, KeyEvent, KeyModifiers};
pub use keymap::{Binding, KeyPattern, Keymap, Keymaps, ModeKey};
pub use reduce::{Command, HelpPage, ModelCommand, records_named, reduce_view};
pub use reduce::{
Command, HelpPage, ModelCommand, RECORD_COORDS_CANNOT_BE_EMPTY, records_named, reduce_view,
};
pub use render::{
AxisFrame, CategoryRow, CategoryTreeCache, CategoryTreeUpdate, CellDisplay, CellId, CreateKind,
DocumentCache, FormulaCache, FormulaEntry, FrameEntry, FrameGroup, GridCache, GridExtent,
+18 -1
View File
@@ -79,7 +79,7 @@ pub enum Command {
Project(Projection),
}
/// A mutation of the shared document: 25 variants, derived not invented (§2). A
/// A mutation of the shared document: 26 variants, derived not invented (§2). A
/// view-configuration variant **names its view** (improvise-lltt).
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub enum ModelCommand {
@@ -90,6 +90,13 @@ pub enum ModelCommand {
value: CellValue,
},
ClearCell(RecordCell),
/// Move a whole record to a new coordinate, creating the item if need be.
/// Position-free: it names the record, never a row (improvise-fdk1).
RenameRecordCoordinate {
record: RecordHash,
category: String,
item: String,
},
AddItem {
category: String,
item: String,
@@ -176,11 +183,16 @@ pub enum ModelCommand {
},
}
/// Why a rename to an empty coordinate is refused. One home, read by the host's
/// refusal and by a client composing the same answer (improvise-fdk1).
pub const RECORD_COORDS_CANNOT_BE_EMPTY: &str = "Record coordinates cannot be empty";
/// Which records a [`ModelCommand`] writes. The **one** home: the server
/// checks against it and a client claims by it — `context/code/protocol.md` §2.
pub fn records_named(cmd: &ModelCommand) -> Vec<RecordHash> {
match cmd {
ModelCommand::SetCell { cell, .. } | ModelCommand::ClearCell(cell) => vec![cell.record],
ModelCommand::RenameRecordCoordinate { record, .. } => vec![*record],
ModelCommand::AddItem { .. }
| ModelCommand::AddItemInGroup { .. }
@@ -623,6 +635,11 @@ mod tests {
match cmd {
ModelCommand::SetCell { cell, value: v } => record(cell) + value(v),
ModelCommand::ClearCell(cell) => record(cell),
ModelCommand::RenameRecordCoordinate {
record: r,
category,
item,
} => surrogate(r) + name(category) + name(item),
ModelCommand::AddItem { category, item }
| ModelCommand::RemoveItem { category, item } => name(category) + name(item),
ModelCommand::AddItemInGroup {
+6 -3
View File
@@ -57,9 +57,12 @@ pub enum CellId {
/// A records-mode creating column: typing a **name** into it adds that
/// measure or category and writes no cell at all (gotcha 1, improvise-poia).
Create(CreateKind),
/// A records-mode coordinate. Renaming it moves every measure cell in the
/// row (gotcha 1), which no client composes: `protocol.md` §3 (improvise-exdg).
Coordinate,
/// A records-mode coordinate: the row's record and the category this column
/// names. Renaming it moves the whole record — `protocol.md` §3 (improvise-fdk1).
Coordinate {
record: RecordHash,
category: String,
},
/// A stored cell whose `category` coordinate names no item of it, so no write
/// can address it: `context/code/command.md` (improvise-gk51).
NonItem { category: String, item: String },
+66 -9
View File
@@ -10,7 +10,8 @@ use crate::keymap::{Binding, Keymaps, ModeKey};
use crate::viewmodel::{CatTreeEntry, axis_change_status, flatten_cat_tree};
use crate::{
AppMode, CellId, CellValue, Command, CreateKind, FormulaId, FormulaSlot, GridExtent, HelpPage,
KeyCode, KeyEvent, ModelCommand, Panel, RecordCell, ViewState, records_named,
KeyCode, KeyEvent, ModelCommand, Panel, RECORD_COORDS_CANNOT_BE_EMPTY, RecordCell, ViewState,
records_named,
};
/// The virtual category a new measure is filed under, spelled as
@@ -858,8 +859,8 @@ fn cell_at_cursor(state: &ClientState) -> Option<&crate::CellDisplay> {
/// declining out loud rather than upstream. See `context/code/clients.md`.
fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
// The grid, not the mode: a records GRID is the observed fact and the mode
// follows it (improvise-bd9o, `context/code/clients.md`).
if state.grid.records {
// follows it (improvise-bd9o). A coordinate is the one records cell it commits.
if state.grid.records && coordinate_at_cursor(state).is_none() {
return Some(vec![Command::SetStatus(not_yet("editing in records mode"))]);
}
let (row, col) = state.view.cursor().selected;
@@ -870,7 +871,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(opened_on(
CellId::Record(_) | CellId::Coordinate { .. } => Some(opened_on(
state,
cell.edit_text.clone().unwrap_or_default(),
mode,
@@ -883,13 +884,25 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
// Both creating columns are records-only, so the guard above takes
// them; declining rather than `None` keeps that a door, not a hole.
CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]),
CellId::Coordinate => Some(vec![Command::SetStatus(not_yet("renaming a coordinate"))]),
CellId::NonItem { category, item } => Some(vec![Command::SetStatus(
crate::non_item_refusal(category, item),
)]),
}
}
/// 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)> {
match cell_at_cursor(state)?.id.as_ref()? {
CellId::Coordinate { record, category } => Some((*record, category.clone())),
CellId::Record(_)
| CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::NonItem { .. } => None,
}
}
/// Open the cell editor on `seed`, or await it while any write is unanswered: the
/// cache predates that write (improvise-mdfc/p6v6, `protocol.md` §3).
fn opened_on(state: &ClientState, seed: String, mode: AppMode) -> Vec<Command> {
@@ -1029,7 +1042,7 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
return create_column(state, edit_mode, advance, kind, &text);
}
if state.grid.records {
return None;
return rename_coordinate(state, edit_mode, advance, id, &text);
}
let mut commands = written(state, id, &text)?;
commands.push(Command::ChangeMode(AppMode::Normal));
@@ -1039,6 +1052,50 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
Some(commands)
}
/// `CommitAndAdvance` over a coordinate: rename the record the editor OPENED on,
/// at the version it was shown — `context/code/clients.md` (improvise-fdk1).
fn rename_coordinate(
state: &ClientState,
edit_mode: AppMode,
advance: Advance,
id: &CellId,
text: &str,
) -> Option<Vec<Command>> {
let CellId::Coordinate { .. } = id else {
return None;
};
// Untouched writes nothing (improvise-ixxx); empty is refused as the host
// refuses it, where composing the write would undo the whole frame.
let unchanged = untouched(state.view.opened_editor.as_ref(), text);
let mut commands = match (unchanged, text.is_empty(), state.opened.as_ref()) {
(true, _, _) => Vec::new(),
(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, 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(),
},
}],
};
let writes = commands.iter().any(|c| matches!(c, Command::Model { .. }));
commands.push(Command::ChangeMode(AppMode::Normal));
commands.extend(cleared(&edit_mode));
commands.extend(advanced(state, edit_mode, advance, writes)?);
Some(commands)
}
/// The commit's own clear of `edit_mode`'s buffer, at its head: the keymap
/// welds none to these keys, so a re-seed survives (improvise-fy9o).
fn cleared(edit_mode: &AppMode) -> Option<Command> {
@@ -1139,7 +1196,7 @@ fn record_at_cursor(state: &ClientState) -> Option<crate::RecordHash> {
CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::Coordinate
| CellId::Coordinate { .. }
| CellId::NonItem { .. } => None,
}
}
@@ -1188,7 +1245,7 @@ pub(crate) fn declined_at(state: &ClientState, row: usize, col: usize) -> Option
CellId::Aggregate(_) => Some(AGGREGATE_DECLINE.to_string()),
CellId::Derived { target } => Some(derived_decline(target)),
CellId::NonItem { category, item } => Some(crate::non_item_refusal(category, item)),
CellId::Record(_) | CellId::Create(_) | CellId::Coordinate => None,
CellId::Record(_) | CellId::Create(_) | CellId::Coordinate { .. } => None,
}
}
@@ -1320,7 +1377,7 @@ fn record_cell_at_cursor(state: &ClientState) -> Option<RecordCell> {
CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::Coordinate
| CellId::Coordinate { .. }
| CellId::NonItem { .. } => None,
}
}
+1 -1
View File
@@ -201,7 +201,7 @@ impl Client {
CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::Coordinate
| CellId::Coordinate { .. }
| CellId::NonItem { .. } => None,
}
}
+5 -2
View File
@@ -150,7 +150,7 @@ impl RenderCell {
CellId::Aggregate(_)
| CellId::Derived { .. }
| CellId::Create(_)
| CellId::Coordinate
| CellId::Coordinate { .. }
| CellId::NonItem { .. },
)
| None => false,
@@ -700,7 +700,10 @@ mod tests {
((1, 0), CellDisplay::text("pending")),
(
(1, 2),
CellDisplay::text("North").with_id(CellId::Coordinate),
CellDisplay::text("North").with_id(CellId::Coordinate {
record: surrogate(),
category: "Region".into(),
}),
),
])),
],
@@ -500,6 +500,7 @@ fn the_tile_bar_route_into_records_mode_does_not_strand_the_user() {
cols >= 3,
"the fixture must reach past the coordinate column"
);
let mut coordinates = 0;
for col in 0..cols {
walk_to(
&mut host,
@@ -518,6 +519,17 @@ fn the_tile_bar_route_into_records_mode_does_not_strand_the_user() {
// a keypress this client might decline.
client.state_mut().view.status_msg = "\u{0}unsaid".to_string();
press(&mut host, id, &mut client, "i");
// A coordinate's Enter composes a rename since improvise-fdk1, so its
// editor is a door rather than a black hole.
if kind.starts_with("Some(Coordinate") {
coordinates += 1;
assert!(
client.state().view.mode.is_editing(),
"column {col} ({kind}): a coordinate's editor must open"
);
press(&mut host, id, &mut client, "Escape");
continue;
}
assert_ne!(
client.state().view.status_msg,
"\u{0}unsaid",
@@ -530,6 +542,10 @@ fn the_tile_bar_route_into_records_mode_does_not_strand_the_user() {
client cannot commit — an Enter that is a black hole"
);
}
assert!(
coordinates > 0 && coordinates < cols,
"the walk must cover both a coordinate and a declined column"
);
}
// ── Ctrl-S, all the way to storage (improvise-jz6w) ─────────────────────
@@ -1018,3 +1034,322 @@ fn a_typed_number_crosses_the_serialized_wire_exactly() {
"the host must store the number the user typed, not its neighbour"
);
}
// ── a coordinate rename over the wire (improvise-fdk1) ───────────────────
/// Every frame the host addressed, to whichever of the two clients it names.
fn route(
frames: &[Addressed],
a: (improvise_command::ui::project::SessionId, &mut Client),
b: (improvise_command::ui::project::SessionId, &mut Client),
) {
for Addressed { to, msg } in frames {
let json = serde_json::to_string(msg).expect("a server frame serializes");
if *to == a.0 {
a.1.on_frame(&json).expect("a folds every frame");
}
if *to == b.0 {
b.1.on_frame(&json).expect("b folds every frame");
}
}
}
/// What client B does while A's rename editor is open.
#[derive(PartialEq)]
enum Race {
Quiet,
WriteRevenue,
SwapDocument,
}
/// Client A renames North's `Region` to `Zed` in records mode, while B does
/// `race`. Evidence file.
fn rename_north(race: Race) -> (Host, Vec<Addressed>) {
let mut host = host();
let path = std::path::PathBuf::from("other.improv");
host.app_mut().storage = Box::new(improvise_command::storage::MemoryFs::new());
let other = host.app().model_state.workbook.clone();
improvise_command::persistence::save_to(&*host.app().storage, &other, &path).unwrap();
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]);
let press_a = |host: &mut Host, a: &mut Client, b: &mut Client, key: &str| {
let Some(frame) = a.key(&BrowserKey::new(key)) else {
return Vec::new();
};
let json = serde_json::to_string(&frame).expect("a client frame serializes");
let frames = send(host, a_id, &json);
route(&frames, (a_id, &mut *a), (b_id, &mut *b));
frames
};
press_a(&mut host, &mut a, &mut b, "T");
for (category, key) in [("_Index", "r"), ("_Dim", "c")] {
let at = a
.state()
.tiles
.tiles
.iter()
.position(|t| t.category == category)
.unwrap_or_else(|| panic!("the tile bar must carry {category}"));
while a.state().view.tile_cat_idx < at {
press_a(&mut host, &mut a, &mut b, "l");
}
while a.state().view.tile_cat_idx > at {
press_a(&mut host, &mut a, &mut b, "h");
}
press_a(&mut host, &mut a, &mut b, key);
}
press_a(&mut host, &mut a, &mut b, "Escape");
assert!(a.state().grid.records, "A must be standing in records mode");
let coordinate = (0..a.state().grid.extent.cols)
.find(|&c| {
format!("{:?}", a.state().grid.cell(0, c).and_then(|x| x.id.clone()))
.starts_with("Some(Coordinate")
})
.expect("records mode projects a coordinate column");
while a.state().view.cursor().selected.1 < coordinate {
press_a(&mut host, &mut a, &mut b, "l");
}
while a.state().view.cursor().selected.1 > coordinate {
press_a(&mut host, &mut a, &mut b, "h");
}
press_a(&mut host, &mut a, &mut b, "i");
assert!(
a.state().view.mode.is_editing(),
"A's editor must open on the coordinate, or nothing here is renamed"
);
assert_eq!(
a.state().view.buffers.get("edit").map(String::as_str),
Some("North")
);
if race == Race::SwapDocument {
let generation = host.app().document_generation();
let load = improvise_command::ui::reduce::caught_up(
host.app(),
improvise_protocol::ModelCommand::LoadModel(path),
);
let frames = host.handle(b_id, ClientFrame::command(load));
assert_eq!(
host.app().document_generation(),
generation + 1,
"setup: the load must really replace the document"
);
route(&frames, (a_id, &mut a), (b_id, &mut b));
assert!(
!a.state().view.mode.is_editing() && a.state().opened.is_none(),
"the rebase must close A's rename editor and drop its capture"
);
return (host, frames);
}
if race == Race::WriteRevenue {
let revenue = (0..b.state().grid.extent.cols)
.find(|&c| {
b.record_cell_at(0, c)
.is_some_and(|cell| cell.measure.as_deref() == Some("Revenue"))
})
.expect("B sees North's Revenue");
let frame = b
.typed_cell_frame(0, revenue, "999")
.expect("B composes a write");
let frames = send(
&mut host,
b_id,
&serde_json::to_string(&frame).expect("json"),
);
assert!(
frames.iter().any(|f| f.to == b_id
&& matches!(f.msg, improvise_protocol::ServerFrame::Accepted)),
"B's write must land, or A has nothing to be stale against: {frames:?}"
);
route(&frames, (a_id, &mut a), (b_id, &mut b));
}
for key in ["Backspace"; 5] {
press_a(&mut host, &mut a, &mut b, key);
}
for key in ["Z", "e", "d"] {
press_a(&mut host, &mut a, &mut b, key);
}
let answers = press_a(&mut host, &mut a, &mut b, "Enter");
(host, answers)
}
/// improvise-vndi for a rename: a document swap closes the rename editor, so
/// no rename crosses it. Evidence: `context/test-evidence/clients.md`.
#[test]
fn a_document_swap_closes_an_open_rename_editor() {
let (host, _) = rename_north(Race::SwapDocument);
assert_eq!(
region(&host, "Zed"),
None,
"the loaded document was renamed"
);
assert!(
region(&host, "North").is_some(),
"North must survive the swap"
);
}
fn region(host: &Host, item: &str) -> Option<CellValue> {
host.app()
.model_state
.workbook
.model
.get_cell(&CellKey::new(vec![
("Region".into(), item.into()),
("_Measure".into(), "Revenue".into()),
]))
.cloned()
}
/// improvise-fdk1: the rename moves the record, claimed at the version the
/// editor opened on. Evidence: `context/test-evidence/clients.md`.
#[test]
fn a_browser_renames_a_records_coordinate_and_the_host_moves_the_record() {
let (host, answers) = rename_north(Race::Quiet);
assert!(
answers
.iter()
.any(|f| matches!(f.msg, improvise_protocol::ServerFrame::Accepted)),
"the rename must be accepted: {answers:?}"
);
assert_eq!(region(&host, "Zed"), Some(CellValue::Number(1200.0)));
assert_eq!(region(&host, "North"), None, "the record moved, not copied");
}
/// improvise-fdk1 / improvise-0j0f: another client wrote the record while the
/// editor was open, so the rename is stale. Evidence file.
#[test]
fn a_coordinate_rename_another_client_moved_is_refused() {
let (host, answers) = rename_north(Race::WriteRevenue);
assert!(
answers.iter().any(|f| matches!(
f.msg,
improvise_protocol::ServerFrame::Rejected(improvise_protocol::Rejection::RecordMoved(
_
))
)),
"a rename composed against a record someone else wrote must be refused: {answers:?}"
);
assert_eq!(
region(&host, "North"),
Some(CellValue::Number(999.0)),
"the other client's write must survive"
);
assert_eq!(region(&host, "Zed"), None, "nothing was renamed");
}
/// Three records, so a rename that re-sorts the rows moves another record under
/// the cursor.
fn three_records_host() -> Host {
let mut wb = Workbook::new("Test");
wb.add_category("Region").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
for (item, value) in [("North", 1.0), ("South", 2.0), ("West", 3.0)] {
wb.add_item("Region", item).unwrap();
wb.model.set_cell(
CellKey::new(vec![
("Region".into(), item.into()),
("_Measure".into(), "Revenue".into()),
]),
CellValue::Number(value),
);
}
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
let mut app = App::new(wb, None);
app.rebuild_layout();
Host::new(app)
}
/// improvise-fdk1: the editor's target is captured once the host has answered
/// this client's own rename, not from the cache it rolled forward. Evidence file.
#[test]
fn a_second_rename_after_a_reorder_renames_the_row_the_host_shows() {
let mut host = three_records_host();
let mut a = Client::new();
a.set_viewport(10, 10);
let (id, snapshot) = host.connect(10, 10);
deliver(&mut a, &[snapshot]);
press(&mut host, id, &mut a, "T");
tile_to(&mut host, id, &mut a, "_Index", "r");
tile_to(&mut host, id, &mut a, "_Dim", "c");
press(&mut host, id, &mut a, "Escape");
assert!(a.state().grid.records, "setup: records mode");
let coordinate = (0..a.state().grid.extent.cols)
.find(|&c| {
format!("{:?}", a.state().grid.cell(0, c).and_then(|x| x.id.clone()))
.starts_with("Some(Coordinate")
})
.expect("a coordinate column");
while a.state().view.cursor().selected.1 < coordinate {
press(&mut host, id, &mut a, "l");
}
let text_at = |a: &Client, row: usize| {
a.state()
.grid
.cell(row, coordinate)
.map(|c| c.text.clone())
.unwrap_or_default()
};
assert_eq!(text_at(&a, 1), "South", "setup: South is row 1");
assert_eq!(text_at(&a, 2), "West", "setup: West is row 2");
press(&mut host, id, &mut a, "j");
// South -> Zed re-sorts the rows; the commit re-enters editing one row
// down, on West as this client's cache stood before the host answered.
press(&mut host, id, &mut a, "i");
for key in ["Backspace"; 5] {
press(&mut host, id, &mut a, key);
}
for key in ["Z", "e", "d", "Enter"] {
press(&mut host, id, &mut a, key);
}
assert!(
a.state().view.mode.is_editing(),
"the commit re-enters editing"
);
let row = a.state().view.cursor().selected.0;
let shown = text_at(&a, row);
assert_ne!(
shown, "West",
"the answered grid must put another record under the cursor than the \
stale re-entry did, or nothing separates the two captures"
);
// Since improvise-fy9o/p6v6 the reopened editor is seeded from the answered cell.
assert_eq!(
a.state().view.buffers.get("edit"),
Some(&shown),
"the reopened editor must hold the answered cell, not the stale one"
);
let seeded = shown.chars().count();
for key in std::iter::repeat_n("Backspace", seeded).chain(["A", "l", "p", "h", "a", "Enter"]) {
press(&mut host, id, &mut a, key);
}
// Zed is reachable since improvise-wdmp: the asker keeps its own placement.
let values = [("North", 1.0), ("Zed", 2.0), ("West", 3.0)];
let (_, moved) = values
.iter()
.find(|(name, _)| *name == shown)
.unwrap_or_else(|| panic!("the cursor shows {shown:?}"));
assert_eq!(
region(&host, "Alpha"),
Some(CellValue::Number(*moved)),
"the second rename must move {shown}, the record the client showed"
);
assert_eq!(region(&host, &shown), None);
assert_eq!(
region(&host, "West").is_some(),
shown != "West",
"West untouched"
);
}
@@ -2895,6 +2895,161 @@ fn every_projected_edit_text_commits_back_to_its_value() {
}
}
// A coordinate rename (improvise-fdk1).
/// Two records, North renamed past South so the sorted records order moves.
/// Evidence: `context/test-evidence/clients.md`.
#[test]
fn a_locally_committed_coordinate_rename_agrees_with_the_host() {
let r = KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE);
let two_records = || {
let mut app = furnished_host();
let south = north_revenue().with("Region", "South");
app.model_state
.workbook
.model
.set_cell(south, improvise_protocol::CellValue::Number(900.0));
app.rebuild_layout();
reduce_key(&mut app, r);
app
};
let (mut app, mut twin) = (two_records(), two_records());
let region = (0..app.layout.col_count())
.find(|&c| app.layout.col_label(c) == "Region")
.expect("a Region column");
let north = 0;
assert_eq!(
app.layout.record_row(north).unwrap().coords.get("Region"),
Some("North"),
"the fixture renames North, at row 0"
);
for host in [&mut app, &mut twin] {
host.cursor_mut().selected = (north, region);
}
let mut client = records_client(&app);
both_ends(&mut app, &mut twin, &mut client, "i");
assert_eq!(
client.view.buffers.get("edit").map(String::as_str),
Some("North"),
"the editor opens on the coordinate it renames"
);
for key in [
"Backspace",
"Backspace",
"Backspace",
"Backspace",
"Backspace",
"Z",
"e",
"d",
] {
both_ends(&mut app, &mut twin, &mut client, key);
}
both_ends(&mut app, &mut twin, &mut client, "Enter");
let text = document_text(&app);
assert!(
text.contains("Region=Zed") && !text.contains("Region=North"),
"the rename must move North's cells to Zed: {text}"
);
assert!(
text.contains("Region=South"),
"South is a bystander and must survive: {text}"
);
}
/// `i` then `Enter` on a coordinate: both ends open one editor and write
/// nothing. Evidence: `context/test-evidence/clients.md`.
#[test]
fn an_untouched_coordinate_enter_writes_nothing_on_either_end() {
let r = KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE);
let records = || {
let mut app = furnished_host();
app.rebuild_layout();
reduce_key(&mut app, r);
app
};
let (mut app, mut twin) = (records(), records());
let region = (0..app.layout.col_count())
.find(|&c| app.layout.col_label(c) == "Region")
.expect("a Region column");
for host in [&mut app, &mut twin] {
host.cursor_mut().selected = (0, region);
}
let mut client = records_client(&app);
let before = document_text(&app);
both_ends(&mut app, &mut twin, &mut client, "i");
assert_eq!(
client.view.opened_editor, app.view_state.opened_editor,
"the two ends captured different editors"
);
assert_eq!(
client.view.opened_editor,
Some(improvise_protocol::OpenedEditor::on("North".into())),
"the coordinate editor recorded no opening"
);
let enter = BrowserKey::new("Enter").to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, enter) else {
panic!("Enter: the client must answer this key locally");
};
assert!(
!cmds
.iter()
.any(|c| matches!(c, improvise_protocol::Command::Model { .. })),
"an untouched coordinate Enter composed a write: {cmds:?}"
);
assert!(
cmds.contains(&improvise_protocol::Command::ChangeMode(AppMode::Normal)),
"Enter did not take the commit path, so this proves nothing: {cmds:?}"
);
both_ends(&mut app, &mut twin, &mut client, "Enter");
assert_eq!(
document_text(&app),
before,
"an untouched Enter changed the document"
);
}
/// An empty coordinate is refused on the host's key path, which says why and
/// advances; the browser must say the same. Evidence: `context/test-evidence/clients.md`.
#[test]
fn an_empty_coordinate_rename_is_refused_the_way_the_host_refuses_it() {
let r = KeyEvent::new(KeyCode::Char('R'), KeyModifiers::NONE);
let records = || {
let mut app = furnished_host();
reduce_key(&mut app, r);
app
};
let (mut app, mut twin) = (records(), records());
let region = (0..app.layout.col_count())
.find(|&c| app.layout.col_label(c) == "Region")
.expect("a Region column");
for host in [&mut app, &mut twin] {
host.cursor_mut().selected = (0, region);
}
let mut client = records_client(&app);
let before = document_text(&app);
both_ends(&mut app, &mut twin, &mut client, "i");
for key in ["Backspace"; 5] {
both_ends(&mut app, &mut twin, &mut client, key);
}
both_ends(&mut app, &mut twin, &mut client, "Enter");
assert_eq!(
document_text(&app),
before,
"an empty rename writes nothing"
);
assert!(
app.view_state.status_msg.contains("cannot be empty"),
"the host must have refused, or this proves nothing: {:?}",
app.view_state.status_msg
);
}
/// North/South/West = 1/2/3 on one measure, with `prune_empty` on: emptying a
/// row's only value takes the row away and renumbers the rest.
fn pruned_host() -> App {