diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index 00fcf442..9e218bc7 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -257,6 +257,41 @@ fn record_fingerprints(data: &DataStore) -> std::collections::HashMap impl Iterator + '_ { + let rows = match layout.is_records_mode() { + true => 0, + false => layout.row_count(), + }; + (0..rows).flat_map(move |r| { + (0..layout.col_count()).filter_map(move |c| layout.cell_key(r, c).map(|k| (r, c, k))) + }) +} + +/// The record of some cell `layout` names, if any names it. +fn record_in(layout: &GridLayout, hash: RecordHash) -> Option { + cells_of(layout) + .map(|(_, _, key)| key) + .find(|key| RecordHash::of(key) == hash) + .map(|key| record_of(&key)) +} + +/// Why `key` is no one cell to write in `layout`: its position there projects as +/// an aggregate. By the whole key, since a row's columns share one `RecordHash`. +fn aggregate_at( + model: &crate::model::types::Model, + layout: &GridLayout, + key: &CellKey, +) -> Option { + let (row, col) = cells_of(layout) + .find(|(_, _, at)| at == key) + .map(|(row, col, _)| (row, col))?; + let projected = crate::ui::project::cell_id(model, layout, row, col); + matches!(projected, Some(CellId::Aggregate(_))) + .then(|| "an aggregate combines several records, so there is no one cell to write".into()) +} + /// Drill-down state: frozen record snapshot + pending edits that have not /// yet been applied to the model. #[derive(Debug, Clone, Default)] @@ -540,8 +575,8 @@ impl App { } /// Resolve a client's record surrogate to the cell it names (ADR 002). `None` - /// means reject. Index first, layout second — see `context/code/command.md`. - pub fn resolve_record(&self, hash: RecordHash) -> Option { + /// means reject. Index first, then `view`'s layout — `context/code/command.md`. + pub fn resolve_record(&self, view: &str, hash: RecordHash) -> Option { let stored = self .model_state .workbook @@ -549,13 +584,13 @@ impl App { .data .resolve_record(hash) .cloned(); - stored.or_else(|| self.record_in_layout(hash)) + stored.or_else(|| self.with_layout_of(view, |layout| record_in(layout, hash))) } - /// The full cell key a [`RecordCell`] names: its record, plus the - /// measure coordinate that picks one of the record's cells. - pub fn resolve_cell(&self, cell: &RecordCell) -> Option { - let record = self.resolve_record(cell.record)?; + /// The full cell key a [`RecordCell`] drawn in `view` names: its record, plus + /// the measure coordinate that picks one of the record's cells. + pub fn resolve_cell(&self, view: &str, cell: &RecordCell) -> Option { + let record = self.resolve_record(view, cell.record)?; Some(match &cell.measure { Some(measure) => record.with(crate::model::category::MEASURE_CATEGORY, measure), None => record, @@ -565,21 +600,25 @@ impl App { /// Resolve an aggregate surrogate (ADR 002) against the layout and only the /// layout: an aggregate has no existence outside the layout deriving it. pub fn resolve_aggregate(&self, hash: AggregateHash) -> Option { - self.layout_cell_keys() + cells_of(&self.layout) + .map(|(_, _, key)| key) .find(|key| AggregateHash::of(key) == hash) } - /// The record of some cell the current layout names, if any names it. - fn record_in_layout(&self, hash: RecordHash) -> Option { - self.layout_cell_keys() - .find(|key| RecordHash::of(key) == hash) - .map(|key| record_of(&key)) + /// Run `f` over the grid a session on `view` is drawn — the host's own where the + /// names agree, as `project::grid_in` does (improvise-wpxr). + fn with_layout_of(&self, view: &str, f: impl FnOnce(&GridLayout) -> T) -> T { + let (name, view) = self.resolve_view(view); + let derived = crate::ui::project::rederived_for(self, name, view); + let model = derived.as_ref().unwrap_or(&self.model_state.workbook.model); + let elsewhere = crate::ui::project::layout_elsewhere(self, name, view, model); + f(elsewhere.as_ref().unwrap_or(&self.layout)) } - /// Why a wire write to `cell` is refused: the host itself projects it as no - /// `CellId::Record` (improvise-ciw8, ADR 002). `None` for a writable cell. - pub fn unwritable(&self, cell: &RecordCell) -> Option { - let key = self.resolve_cell(cell)?; + /// Why a wire write to `cell`, drawn in `view`, is refused: that view projects + /// it as no `CellId::Record` (improvise-ciw8, ADR 002). `None` for a writable cell. + pub fn unwritable(&self, view: &str, cell: &RecordCell) -> Option { + let key = self.resolve_cell(view, cell)?; let model = &self.model_state.workbook.model; let derived = model.formula_for(&key).map(|f| { format!( @@ -588,7 +627,7 @@ impl App { ) }); let refusal = self.group_refusal(&key).or(derived); - refusal.or_else(|| self.aggregate_refusal(cell.record, &key)) + refusal.or_else(|| self.aggregate_refusal(view, cell.record, &key)) } /// A coordinate naming a group rather than an item is no cell in ANY view, so @@ -605,40 +644,12 @@ impl App { /// A record the store does not hold is known only by its layout position, and /// is a record only where the projection at ITS CELL says so (improvise-sjdb). - fn aggregate_refusal(&self, hash: RecordHash, key: &CellKey) -> Option { - let stored = self.model_state.workbook.model.data.resolve_record(hash); - let (row, col) = stored.map_or_else(|| self.cell_position(key), |_| None)?; - let projected = - crate::ui::project::cell_id(&self.model_state.workbook.model, &self.layout, row, col); - matches!(projected, Some(CellId::Aggregate(_))).then(|| { - "an aggregate combines several records, so there is no one cell to write".into() - }) - } - - /// Where in the current layout the cell `key` sits, if anywhere. By the whole - /// key: a `RecordHash` drops `_Measure`, so a row's columns all share one. - fn cell_position(&self, key: &CellKey) -> Option<(usize, usize)> { - self.layout_cells() - .find(|(_, _, at)| at == key) - .map(|(row, col, _)| (row, col)) - } - - /// Every cell key the current grid names. Records mode is excluded: its - /// `cell_key` carries *positions*, the trap ADR 002 rejected `_Index` for. - fn layout_cell_keys(&self) -> impl Iterator + '_ { - self.layout_cells().map(|(_, _, key)| key) - } - - /// [`Self::layout_cell_keys`] with each key's position. - fn layout_cells(&self) -> impl Iterator + '_ { - let layout = &self.layout; - let rows = match layout.is_records_mode() { - true => 0, - false => layout.row_count(), - }; - (0..rows).flat_map(move |r| { - (0..layout.col_count()).filter_map(move |c| layout.cell_key(r, c).map(|k| (r, c, k))) - }) + fn aggregate_refusal(&self, view: &str, hash: RecordHash, key: &CellKey) -> Option { + let model = &self.model_state.workbook.model; + model.data.resolve_record(hash).map_or_else( + || self.with_layout_of(view, |layout| aggregate_at(model, layout, key)), + |_| None, + ) } /// Point **this client's** slice at `name`, if the document has such a view, diff --git a/crates/improvise-command/src/ui/host.rs b/crates/improvise-command/src/ui/host.rs index de6babe9..8ab1a710 100644 --- a/crates/improvise-command/src/ui/host.rs +++ b/crates/improvise-command/src/ui/host.rs @@ -510,6 +510,7 @@ mod tests { let frames = host.model( id, ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(42.0), }, @@ -540,6 +541,7 @@ mod tests { let frames = host.model( asker, ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(7.0), }, @@ -610,6 +612,7 @@ mod tests { records: Vec::new(), }, cmd: ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(42.0), }, @@ -634,6 +637,7 @@ mod tests { use improvise_protocol::ModelCommand; let edit = |value: f64| ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(value), }; @@ -712,6 +716,7 @@ mod tests { use improvise_protocol::ModelCommand; let write = |region: &str, value: f64| ModelCommand::SetCell { + view: "Default".into(), cell: addressed(region), value: CellValue::Number(value), }; @@ -860,6 +865,7 @@ mod tests { let write = crate::ui::reduce::caught_up( host.app(), ModelCommand::SetCell { + view: "Default".into(), cell: addressed("East"), value: CellValue::Number(9.0), }, @@ -905,6 +911,7 @@ mod tests { host.model( asker, ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(1.0), }, @@ -926,6 +933,7 @@ mod tests { Command::Model { basis, cmd: ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(5.0), }, @@ -984,6 +992,7 @@ mod tests { host.model( asker, ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(1.0), }, @@ -1015,6 +1024,7 @@ mod tests { Command::Model { basis, cmd: ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(5.0), }, @@ -1103,7 +1113,7 @@ mod tests { let basis = frame_basis(&host, &["North"]); assert!( host.app() - .resolve_record(addressed("North").record) + .resolve_record("Default", addressed("North").record) .is_some(), "North must resolve BEFORE the load, or a refusal naming it proves nothing" ); @@ -1120,6 +1130,7 @@ mod tests { Command::Model { basis, cmd: ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(5.0), }, @@ -1212,7 +1223,7 @@ mod tests { let basis = frame_basis(&host, &["North"]); assert!( host.app() - .resolve_record(addressed("North").record) + .resolve_record("Default", addressed("North").record) .is_some(), "North must resolve BEFORE the load, or a refusal naming it proves nothing" ); @@ -1229,6 +1240,7 @@ mod tests { Command::Model { basis, cmd: ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(5.0), }, @@ -1309,6 +1321,7 @@ mod tests { crate::ui::reduce::caught_up( host.app(), improvise_protocol::ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(3.0), }, @@ -1361,6 +1374,7 @@ mod tests { let write = crate::ui::reduce::caught_up( host.app(), ModelCommand::SetCell { + view: "Default".into(), cell: addressed("West"), value: CellValue::Number(7.0), }, @@ -1431,8 +1445,12 @@ mod tests { measure: key.get("_Measure").map(String::from), }; let cmd = match clear { - true => ModelCommand::ClearCell(cell), + true => ModelCommand::ClearCell { + view: "Default".into(), + cell, + }, false => ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(7.0), }, @@ -1524,6 +1542,7 @@ mod tests { // so a lone write is answered up front too. let key = month_cell("Nowhere", "Cost"); let cmd = ModelCommand::SetCell { + view: "Default".into(), cell: RecordCell { record: RecordHash::of(&key), measure: Some("Cost".into()), @@ -1712,6 +1731,7 @@ mod tests { let write = crate::ui::reduce::caught_up( host.app(), ModelCommand::SetCell { + view: "Default".into(), cell: addressed("West"), value: CellValue::Number(7.0), }, @@ -1872,6 +1892,7 @@ mod tests { records: Vec::new(), }, cmd: ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(42.0), }, @@ -2003,6 +2024,7 @@ mod tests { host.model( client, ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(41.0), }, @@ -2052,6 +2074,7 @@ mod tests { let edited = host.model( client, ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(41.0), }, @@ -2140,6 +2163,7 @@ mod tests { let frames = host.model( stays, ModelCommand::SetCell { + view: "Default".into(), cell: addressed("North"), value: CellValue::Number(1.0), }, @@ -2167,6 +2191,7 @@ mod tests { frames.extend(host.model( id, ModelCommand::SetCell { + view: "Default".into(), cell: addressed("South"), value: CellValue::Number(3.5), }, @@ -2191,6 +2216,7 @@ mod tests { ClientFrame::command(crate::ui::reduce::caught_up( Host::new(App::new(workbook(), None)).app(), ModelCommand::SetCell { + view: "Default".into(), cell: addressed("South"), value: CellValue::Text("pending".into()), }, @@ -2485,7 +2511,10 @@ mod tests { let ServerFrame::Snapshot(snapshot) = bootstrap.msg else { panic!("connect did not send a Snapshot"); }; - assert_eq!(snapshot.view.active_view, "Default", "setup: client on Default"); + assert_eq!( + snapshot.view.active_view, "Default", + "setup: client on Default" + ); assert!(host.app.switch_to_view("Other"), "setup: host on Other"); host.app.rebuild_layout(); assert_eq!(host.app().layout.row_count(), 1, "setup: Other hides South"); @@ -2501,6 +2530,7 @@ mod tests { let answer = host.model( client, ModelCommand::SetCell { + view: "Default".into(), cell: addressed("South"), value: CellValue::Number(3.0), }, @@ -2512,7 +2542,11 @@ mod tests { .collect(); assert_eq!(refused, vec![], "the write was refused"); assert_eq!( - host.app().model_state.workbook.model.get_cell(&cell("South")), + host.app() + .model_state + .workbook + .model + .get_cell(&cell("South")), Some(&CellValue::Number(3.0)), "South was not written" ); diff --git a/crates/improvise-command/src/ui/project.rs b/crates/improvise-command/src/ui/project.rs index c40392e8..704cbb9c 100644 --- a/crates/improvise-command/src/ui/project.rs +++ b/crates/improvise-command/src/ui/project.rs @@ -136,10 +136,7 @@ pub fn grid_in(app: &App, view_name: &str, viewport: Viewport) -> GridCache { // `none_cats` aggregated over — improvise-ac2z, `context/code/command.md`. let derived = rederived_for(app, name, view); let model = derived.as_ref().unwrap_or(&workbook.model); - // `with_frozen_records`, not `new`: `new` skips `prune_empty`, so a client on - // another view was projected rows its own config prunes — `context/code/command.md`. - let elsewhere = - (name != app.view_name()).then(|| GridLayout::with_frozen_records(model, view, None)); + let elsewhere = layout_elsewhere(app, name, view, model); let layout = elsewhere.as_ref().unwrap_or(&app.layout); let plan = FormatPlan::of(model, view); let rows = viewport.rows_in(layout.row_count()); @@ -179,9 +176,26 @@ pub fn grid_in(app: &App, view_name: &str, viewport: Viewport) -> GridCache { } } +/// The layout a session on `name` is drawn, where it is not the host's own. Shared +/// by `grid_in` and `App::resolve_record` (improvise-wpxr), so both see one grid. +pub(crate) fn layout_elsewhere( + app: &App, + name: &str, + view: &View, + model: &crate::model::types::Model, +) -> Option { + // `with_frozen_records`, not `new`: `new` skips `prune_empty`, so a client on + // another view was projected rows its own config prunes — `context/code/command.md`. + (name != app.view_name()).then(|| GridLayout::with_frozen_records(model, view, None)) +} + /// A copy of the model whose formulas were evaluated for **this** view, where the /// host's own derivation does not answer for it — `context/code/command.md`. -fn rederived_for(app: &App, name: &str, view: &View) -> Option { +pub(crate) fn rederived_for( + app: &App, + name: &str, + view: &View, +) -> Option { let workbook = &app.model_state.workbook; // Three conditions, and each one is what makes the clone rare: another view, // a different aggregation, and something derived to get wrong. @@ -2807,13 +2821,16 @@ mod tests { let applied = sessions.apply_caught_up( &mut app, asker, - ModelCommand::ClearCell(RecordCell { - record: RecordHash::of(&CellKey::new(vec![( - "Region".to_string(), - "Service".to_string(), - )])), - measure: Some("Revenue".to_string()), - }), + ModelCommand::ClearCell { + view: "Default".into(), + cell: RecordCell { + record: RecordHash::of(&CellKey::new(vec![( + "Region".to_string(), + "Service".to_string(), + )])), + measure: Some("Revenue".to_string()), + }, + }, ); assert_ne!( @@ -4570,7 +4587,7 @@ mod cell_identity { for col in 0..2 { let id = record_of_id(id_at(&app, row, col)); assert_eq!( - app.resolve_cell(&id), + app.resolve_cell(app.view_name(), &id), app.layout.cell_key(row, col), "({row}, {col})" ); @@ -4663,7 +4680,7 @@ mod cell_identity { let mut app = raw_app(); let east = record_of_id(id_at(&app, 2, 0)); let north = record_of_id(id_at(&app, 0, 0)); - let north_key = app.resolve_cell(&north); + let north_key = app.resolve_cell(app.view_name(), &north); assert!( north_key.is_some(), "the fixture must resolve to begin with" @@ -4673,12 +4690,12 @@ mod cell_identity { app.rebuild_layout(); assert_eq!( - app.resolve_cell(&east), + app.resolve_cell(app.view_name(), &east), None, "a surrogate for a row that is gone must not resolve to anything" ); assert_eq!( - app.resolve_cell(&north), + app.resolve_cell(app.view_name(), &north), north_key, "and the rows that survived must be untouched" ); @@ -4705,6 +4722,7 @@ mod cell_identity { let wire = caught_up( &app, ModelCommand::SetCell { + view: "Default".into(), cell: target, value: CellValue::Number(42.0), }, @@ -4731,6 +4749,7 @@ mod cell_identity { let wire = caught_up( &app, ModelCommand::SetCell { + view: "Default".into(), cell: target, value: CellValue::Number(999.0), }, @@ -4773,6 +4792,7 @@ mod cell_identity { let effects = crate::ui::reduce::model_effects( &app, &ModelCommand::SetCell { + view: "Default".into(), cell: east, value: CellValue::Number(13.0), }, @@ -4922,14 +4942,14 @@ mod cell_identity { let id = record_of_id(id_at(&app, 0, revenue)); assert_eq!( - app.resolve_cell(&id), + app.resolve_cell(app.view_name(), &id), Some(CellKey::new(vec![ ("Region".to_string(), "East".to_string()), ("_Measure".to_string(), "Revenue".to_string()), ])) ); assert_ne!( - app.resolve_cell(&id), + app.resolve_cell(app.view_name(), &id), app.layout.cell_key(0, revenue), "the synthetic (_Index, _Dim) key must not be what a client holds" ); @@ -5296,6 +5316,7 @@ mod cell_identity { let wire = caught_up( &app, ModelCommand::SetCell { + view: "Default".into(), cell: target.clone(), value: CellValue::Number(999.0), }, @@ -5443,6 +5464,7 @@ mod basis_check { Command::Model { basis, cmd: ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(99.0), }, diff --git a/crates/improvise-command/src/ui/reduce.rs b/crates/improvise-command/src/ui/reduce.rs index a598f68b..8ec522a6 100644 --- a/crates/improvise-command/src/ui/reduce.rs +++ b/crates/improvise-command/src/ui/reduce.rs @@ -2,8 +2,8 @@ //! state (improvise-gxi/s17o). See `context/code/command.md`. use improvise_core::model::record::RecordHash; -pub use improvise_protocol::records_named; use improvise_protocol::{Basis, Command, ModelCommand, Rejection}; +pub use improvise_protocol::{cells_named, records_named}; use crate::command::key::KeyEvent; use crate::ui::app::App; @@ -53,10 +53,10 @@ pub fn broken_chain(app: &App, cmd: &Command) -> Option { }; // No create-exemption here, unlike the pre-frame check: a record that does // not resolve NOW is one `resolve_cell` will drop the write for, silently. - records_named(model) + cells_named(model) .into_iter() - .find(|hash| app.resolve_record(*hash).is_none()) - .map(Rejection::ChainBroken) + .find(|(view, cell)| app.resolve_record(view, cell.record).is_none()) + .map(|(_, cell)| Rejection::ChainBroken(cell.record)) } impl<'a> IntoIterator for CheckedBatch<'a> { @@ -135,20 +135,22 @@ pub fn apply_command(app: &mut App, cmd: Checked<'_>) -> Dirty { /// was derived from (improvise-kmgs). Exhaustive on purpose. pub fn model_effects(app: &App, cmd: &ModelCommand) -> Vec> { let one: Box = match cmd { - // A cell the host projects as no `Record` is refused, so the frame is + // A cell its view projects as no `Record` is refused, so the frame is // Rejected rather than written (improvise-ciw8, ADR 002). - ModelCommand::SetCell { cell, value } => { - match (app.unwritable(cell), app.resolve_cell(cell)) { + ModelCommand::SetCell { view, cell, value } => { + match (app.unwritable(view, cell), app.resolve_cell(view, cell)) { (Some(why), _) => effect::refuse(why), (None, None) => return vec![], (None, Some(key)) => Box::new(effect::SetCell(key, value.clone())), } } - ModelCommand::ClearCell(cell) => match (app.unwritable(cell), app.resolve_cell(cell)) { - (Some(why), _) => effect::refuse(why), - (None, None) => return vec![], - (None, Some(key)) => Box::new(effect::ClearCell(key)), - }, + ModelCommand::ClearCell { view, cell } => { + match (app.unwritable(view, cell), app.resolve_cell(view, cell)) { + (Some(why), _) => effect::refuse(why), + (None, None) => return vec![], + (None, Some(key)) => Box::new(effect::ClearCell(key)), + } + } ModelCommand::AddItem { category, item } => Box::new(effect::AddItem { category: category.clone(), item: item.clone(), @@ -274,29 +276,29 @@ pub fn stale_basis(app: &App, basis: &Basis, cmd: &ModelCommand) -> Option Option { - records_named(cmd) + cells_named(cmd) .into_iter() - .filter(|hash| !will_be_created_by_the_frame(app, basis, *hash)) - .find_map(|hash| stale_record(app, basis, hash)) + .filter(|(view, cell)| !will_be_created_by_the_frame(app, basis, view, cell.record)) + .find_map(|(view, cell)| stale_record(app, basis, view, cell.record)) } /// Is this a record the frame is about to create? **A property, not a list of /// commands** — see `context/code/command.md`. Only the pair exempts. -fn will_be_created_by_the_frame(app: &App, basis: &Basis, hash: RecordHash) -> bool { - app.resolve_record(hash).is_none() && basis.record(hash).is_none() +fn will_be_created_by_the_frame(app: &App, basis: &Basis, view: &str, hash: RecordHash) -> bool { + app.resolve_record(view, hash).is_none() && basis.record(hash).is_none() } -fn stale_record(app: &App, basis: &Basis, hash: RecordHash) -> Option { - match (app.resolve_record(hash), basis.record(hash)) { +fn stale_record(app: &App, basis: &Basis, view: &str, hash: RecordHash) -> Option { + match (app.resolve_record(view, hash), basis.record(hash)) { (None, _) => Some(Rejection::RecordGone(hash)), (Some(_), None) => Some(Rejection::RecordUnstated(hash)), (Some(_), Some(claimed)) => { @@ -667,10 +669,14 @@ pub(crate) mod tests { pub(crate) fn every_model_command() -> Vec { vec![ ModelCommand::SetCell { + view: "Default".into(), cell: record_cell(&a_stored_cell()), value: CellValue::Number(1.0), }, - ModelCommand::ClearCell(record_cell(&a_stored_cell())), + ModelCommand::ClearCell { + view: "Default".into(), + cell: record_cell(&a_stored_cell()), + }, ModelCommand::AddItem { category: "Region".into(), item: "South".into(), @@ -757,7 +763,7 @@ pub(crate) mod tests { fn variant_name(cmd: &ModelCommand) -> &'static str { match cmd { ModelCommand::SetCell { .. } => "SetCell", - ModelCommand::ClearCell(_) => "ClearCell", + ModelCommand::ClearCell { .. } => "ClearCell", ModelCommand::AddItem { .. } => "AddItem", ModelCommand::AddItemInGroup { .. } => "AddItemInGroup", ModelCommand::RemoveItem { .. } => "RemoveItem", @@ -1086,6 +1092,7 @@ pub(crate) mod tests { fn a_wire_model_command_marks_the_document_dirty() { for cmd in [ ModelCommand::SetCell { + view: "Default".into(), cell: RecordCell { record: RecordHash::of(&crate::model::cell::CellKey::new(vec![( "Region".to_string(), diff --git a/crates/improvise-protocol/src/lib.rs b/crates/improvise-protocol/src/lib.rs index 84b55876..94e88168 100644 --- a/crates/improvise-protocol/src/lib.rs +++ b/crates/improvise-protocol/src/lib.rs @@ -27,7 +27,7 @@ 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, cells_named, records_named, reduce_view}; pub use render::{ AxisFrame, CategoryRow, CategoryTreeCache, CategoryTreeUpdate, CellDisplay, CellId, CreateKind, DocumentCache, FormulaCache, FormulaEntry, FrameEntry, FrameGroup, GridCache, GridExtent, diff --git a/crates/improvise-protocol/src/reduce.rs b/crates/improvise-protocol/src/reduce.rs index bb62a98e..39fbc267 100644 --- a/crates/improvise-protocol/src/reduce.rs +++ b/crates/improvise-protocol/src/reduce.rs @@ -84,12 +84,17 @@ pub enum Command { #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub enum ModelCommand { // ── ModelChange::CellData ─────────────────────────────────────── - /// Write one stored cell: a [`RecordCell`] and **nothing else** (ADR 002). + /// Write one stored cell: a [`RecordCell`] and the view that drew it (ADR 002, + /// improvise-wpxr). The view resolves a record no cell of which is stored yet. SetCell { + view: String, cell: RecordCell, value: CellValue, }, - ClearCell(RecordCell), + ClearCell { + view: String, + cell: RecordCell, + }, AddItem { category: String, item: String, @@ -179,8 +184,19 @@ pub enum ModelCommand { /// 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 { + cells_named(cmd) + .into_iter() + .map(|(_, cell)| cell.record) + .collect() +} + +/// The cells a [`ModelCommand`] writes, each with the view it names to resolve +/// in (improvise-wpxr). The exhaustive list [`records_named`] is read off. +pub fn cells_named(cmd: &ModelCommand) -> Vec<(&str, &RecordCell)> { match cmd { - ModelCommand::SetCell { cell, .. } | ModelCommand::ClearCell(cell) => vec![cell.record], + ModelCommand::SetCell { view, cell, .. } | ModelCommand::ClearCell { view, cell } => { + vec![(view.as_str(), cell)] + } ModelCommand::AddItem { .. } | ModelCommand::AddItemInGroup { .. } @@ -518,6 +534,7 @@ mod tests { // One populated value per payload family: `RecordCell`, `Axis`, // `PathBuf`. The exhaustive 26-variant walk is in the host crate. modelled(ModelCommand::SetCell { + view: "Default".into(), cell: RecordCell { record: RecordHash::of(&CellKey::new(vec![("Region".into(), "East".into())])), measure: Some("Revenue".into()), @@ -621,8 +638,12 @@ mod tests { } // No `..` anywhere: a field added to a variant must be classified here. match cmd { - ModelCommand::SetCell { cell, value: v } => record(cell) + value(v), - ModelCommand::ClearCell(cell) => record(cell), + ModelCommand::SetCell { + view: v_name, + cell, + value: v, + } => name(v_name) + record(cell) + value(v), + ModelCommand::ClearCell { view, cell } => name(view) + record(cell), ModelCommand::AddItem { category, item } | ModelCommand::RemoveItem { category, item } => name(category) + name(item), ModelCommand::AddItemInGroup { diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index 981b5227..9bc874fd 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -549,7 +549,10 @@ fn composed( // ── cell operations ────────────────────────────────────────── "clear-cell" => Some(vec![model( state, - ModelCommand::ClearCell(record_cell_at_cursor(state)?), + ModelCommand::ClearCell { + view: state.view.active_view.clone(), + cell: record_cell_at_cursor(state)?, + }, )?]), // `YankCell::execute`: the value, then the status. The projected // value, never text re-parsed: improvise-dsxq. @@ -561,6 +564,7 @@ fn composed( Some(value) => vec![model( state, ModelCommand::SetCell { + view: state.view.active_view.clone(), cell: record_cell_at_cursor(state)?, value, }, @@ -928,7 +932,7 @@ fn written(state: &ClientState, id: &CellId, text: &str) -> Option> true => Some(Vec::new()), false => Some(vec![Command::Model { basis: state.basis.claim([record_at_cursor(state)?])?, - cmd: typed_cell_command(id, text)?, + cmd: typed_cell_command(&state.view.active_view, id, text)?, }]), } } @@ -1084,15 +1088,16 @@ pub fn typed(text: &str) -> Option { } /// The `ModelCommand` for "the user typed `text` into this cell": the one -/// composition, shared by the mouse path and the keyboard path. -pub fn typed_cell_command(id: &CellId, text: &str) -> Option { +/// composition, shared by the mouse path and the keyboard path. `view` drew it. +pub fn typed_cell_command(view: &str, id: &CellId, text: &str) -> Option { let CellId::Record(cell) = id else { return None; }; let cell: RecordCell = cell.clone(); + let view = view.to_string(); Some(match typed(text) { - Some(value) => ModelCommand::SetCell { cell, value }, - None => ModelCommand::ClearCell(cell), + Some(value) => ModelCommand::SetCell { view, cell, value }, + None => ModelCommand::ClearCell { view, cell }, }) } @@ -3391,6 +3396,7 @@ mod tests { Command::Model { basis: published(), cmd: ModelCommand::SetCell { + view: "Default".into(), cell: RecordCell { record: surrogate(), measure: Some("Revenue".into()), @@ -3620,7 +3626,7 @@ mod tests { commands.iter().any(|c| matches!( c, Command::Model { - cmd: ModelCommand::ClearCell(_), + cmd: ModelCommand::ClearCell { .. }, .. } )), @@ -3684,7 +3690,7 @@ mod tests { assert!(matches!( &commands[0], Command::Model { - cmd: ModelCommand::ClearCell(_), + cmd: ModelCommand::ClearCell { .. }, .. } )); diff --git a/crates/improvise-wasm-client/src/socket.rs b/crates/improvise-wasm-client/src/socket.rs index 1f1c0d21..6bda0b0d 100644 --- a/crates/improvise-wasm-client/src/socket.rs +++ b/crates/improvise-wasm-client/src/socket.rs @@ -14,6 +14,8 @@ use serde::{Deserialize, Serialize}; pub struct EditTarget { /// The document the editor opened on; a swap under it refuses the commit (improvise-vndi). pub document_generation: u64, + /// The view that drew the cell: an empty record resolves only there (improvise-wpxr). + pub view: String, pub cell: RecordCell, /// The record's write counter as this client had it at open. pub version: u64, @@ -125,6 +127,7 @@ impl Client { let seed = self.state().grid.cell(row, col)?.edit_text.clone(); Some(EditTarget { document_generation, + view: self.state().view.active_view.clone(), cell, version, opened: OpenedEditor::on(seed.unwrap_or_default()), @@ -143,7 +146,11 @@ impl Client { )?; let cmds = vec![Command::Model { basis, - cmd: typed_cell_command(&improvise_protocol::CellId::Record(target.cell), text)?, + cmd: typed_cell_command( + &target.view, + &improvise_protocol::CellId::Record(target.cell), + text, + )?, }]; // Folds nothing — a `Command::Model` is not the view slice's business — // but it **registers**, or the host's answers address the wrong frame. @@ -402,6 +409,7 @@ mod tests { Some(ClientFrame::command(Command::Model { basis: published_basis(), cmd: ModelCommand::SetCell { + view: "Default".into(), cell: record("Revenue"), value: CellValue::Number(42.5), }, @@ -412,6 +420,7 @@ mod tests { Some(ClientFrame::command(Command::Model { basis: published_basis(), cmd: ModelCommand::SetCell { + view: "Default".into(), cell: record("Revenue"), value: CellValue::Text("pending".into()), }, @@ -421,7 +430,10 @@ mod tests { client.typed_cell_frame(0, 0, ""), Some(ClientFrame::command(Command::Model { basis: published_basis(), - cmd: ModelCommand::ClearCell(record("Revenue")), + cmd: ModelCommand::ClearCell { + view: "Default".into(), + cell: record("Revenue"), + }, })), "an empty string clears the cell; it is not a Text write" ); diff --git a/crates/improvise-wasm-client/tests/browser_round_trip.rs b/crates/improvise-wasm-client/tests/browser_round_trip.rs index 1ab86c87..65b3130d 100644 --- a/crates/improvise-wasm-client/tests/browser_round_trip.rs +++ b/crates/improvise-wasm-client/tests/browser_round_trip.rs @@ -736,6 +736,7 @@ fn an_edit_is_refused_when_the_cell_was_written_while_the_editor_was_open() { let write = caught_up( host.app(), ModelCommand::SetCell { + view: "Default".into(), cell: clicked, value: CellValue::Number(999.0), }, @@ -788,7 +789,13 @@ fn an_edit_is_refused_when_the_cell_was_cleared_while_the_editor_was_open() { .expect("serialization must not fail") .expect("a stored cell has an edit target"); - let clear = caught_up(host.app(), ModelCommand::ClearCell(clicked)); + let clear = caught_up( + host.app(), + ModelCommand::ClearCell { + view: "Default".into(), + cell: clicked, + }, + ); let relayed = host.handle(b, ClientFrame::command(clear)); let for_a: Vec = relayed.into_iter().filter(|f| f.to == a).collect(); deliver(&mut editor, &for_a); diff --git a/crates/improvise-worker-server/src/worker.rs b/crates/improvise-worker-server/src/worker.rs index ca882eb9..cafd0607 100644 --- a/crates/improvise-worker-server/src/worker.rs +++ b/crates/improvise-worker-server/src/worker.rs @@ -333,6 +333,7 @@ mod tests { let cmd = reduce::caught_up( worker.app(), ModelCommand::SetCell { + view: "Default".into(), cell: addressed(&grid, 0, 0), value: CellValue::Number(4242.0), }, @@ -399,6 +400,7 @@ mod tests { let cmd = reduce::caught_up( worker.app(), ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(42.0), }, @@ -436,6 +438,7 @@ mod tests { let cmd = reduce::caught_up( worker.app(), ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(42.0), }, @@ -466,6 +469,7 @@ mod tests { let cmd = reduce::caught_up( worker.app(), ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(42.0), }, @@ -489,6 +493,7 @@ mod tests { let wire = reduce::caught_up( worker.app(), ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(99.0), }, @@ -535,6 +540,7 @@ mod tests { let stale = reduce::caught_up( worker.app(), ModelCommand::SetCell { + view: "Default".into(), cell: cell.clone(), value: CellValue::Number(1.0), }, @@ -543,6 +549,7 @@ mod tests { let other = reduce::caught_up( worker.app(), ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(2.0), }, diff --git a/crates/improvise-worker-server/tests/post_message.rs b/crates/improvise-worker-server/tests/post_message.rs index f6d5dcbc..644a7d8c 100644 --- a/crates/improvise-worker-server/tests/post_message.rs +++ b/crates/improvise-worker-server/tests/post_message.rs @@ -167,6 +167,7 @@ fn the_worker_loads_projects_edits_and_persists_over_post_message() { let cell = main.cell(2, 0); main.post(&main.caught_up(ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(31.0), })); @@ -205,6 +206,7 @@ fn a_reload_over_the_same_origin_reopens_what_was_saved() { main.post(&ClientFrame::Viewport { rows: 10, cols: 4 }); let cell = main.cell(0, 0); main.post(&main.caught_up(ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(77.0), })); @@ -242,10 +244,12 @@ fn a_batch_composed_against_a_moved_document_is_refused() { let cell = main.cell(0, 0); let stale = main.caught_up(ModelCommand::SetCell { + view: "Default".into(), cell: cell.clone(), value: CellValue::Number(1.0), }); let winner = main.caught_up(ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(2.0), }); @@ -359,6 +363,7 @@ fn the_shipped_demo_seeds_a_worker_whose_raw_cells_are_settable() { // formula beside it, through the projection stream. let revenue = main.cell(0, 0); main.post(&main.caught_up(ModelCommand::SetCell { + view: "Default".into(), cell: revenue, value: CellValue::Number(20000.0), })); @@ -379,6 +384,7 @@ fn the_automatic_persist_tells_the_client_the_document_is_clean() { main.dirty_marks.clear(); let cell = main.cell(0, 0); main.post(&main.caught_up(ModelCommand::SetCell { + view: "Default".into(), cell, value: CellValue::Number(31.0), })); diff --git a/crates/improvise-ws-server/src/registry.rs b/crates/improvise-ws-server/src/registry.rs index 190b56fd..d2827a59 100644 --- a/crates/improvise-ws-server/src/registry.rs +++ b/crates/improvise-ws-server/src/registry.rs @@ -293,6 +293,7 @@ mod tests { fn set_cell(row: &str, value: f64) -> ModelCommand { ModelCommand::SetCell { + view: "Default".into(), cell: addressed(row), value: CellValue::Number(value), } diff --git a/crates/improvise-ws-server/tests/socket.rs b/crates/improvise-ws-server/tests/socket.rs index e3b7b383..42504501 100644 --- a/crates/improvise-ws-server/tests/socket.rs +++ b/crates/improvise-ws-server/tests/socket.rs @@ -111,6 +111,7 @@ fn a_client_on_a_socket_is_seeded_and_gets_its_edit_back() { .claim([target.record]) .expect("the bootstrap published a basis"), cmd: ModelCommand::SetCell { + view: "Default".into(), cell: target, value: CellValue::Number(12.0), },