From 1dd6950ed385571ca2064846d3beaf2fc9d8ca7e Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Mon, 21 Sep 2026 10:23:27 -0700 Subject: [PATCH] A view-scoped ModelCommand names its view (improvise-lltt) Nine ModelCommand variants configure a view and named it implicitly: the effect read App::view_mut(), the HOST's slice. Since improvise-jytm made ViewState::active_view authoritative per client, a client on another view moved the host's view instead of its own -- its grid did not change and every other client's did. Each of the nine now carries `view: String`; model_effects maps it to effect::ViewTarget::Named, and the nine effects carry a ViewTarget rather than reading an ambient one. There is no operandless form left: App::view_mut() is deleted and replaced by view_mut_of(&ViewTarget), so a future geometry effect has to say which view it writes. The host's key path passes ViewTarget::Active, which is right -- the TUI is the host and has no session to ask. A Named view the document has not got is a no-op rather than resolve_view's fallback onto the default. Second half, which the ticket did not predict: naming the view made a reframe of a view the host is NOT on reachable, and adopt_cursor_epoch could not see one -- cursor_epoch is bumped only by the host's own reset_cursor, so those clients were reset by nobody and told nothing (gotcha 24's strictly-worse-than-nothing state). App::last_reframed_elsewhere is the second trigger; its opening frames are taken lazily inside view_mut_of, the one door such a write goes through, so the held-key path still hashes one view and improvise-mcq4's declined cost is not paid. Evidence and the three injected faults: context/test-evidence/command.md. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH --- benches/reshape_observation.rs | 7 +- context/code/command.md | 18 +- context/test-evidence/command.md | 74 +++++++ .../improvise-command/src/command/cmd/cell.rs | 4 +- .../src/command/cmd/effect_cmds.rs | 12 +- .../improvise-command/src/command/cmd/grid.rs | 18 +- .../src/command/cmd/navigation.rs | 4 +- .../src/command/cmd/panel.rs | 6 +- .../improvise-command/src/command/cmd/tile.rs | 5 +- crates/improvise-command/src/ui/app.rs | 83 ++++++- crates/improvise-command/src/ui/effect.rs | 136 ++++++++---- crates/improvise-command/src/ui/host.rs | 3 +- crates/improvise-command/src/ui/project.rs | 203 ++++++++++++++++-- crates/improvise-command/src/ui/reduce.rs | 117 ++++++++-- crates/improvise-protocol/src/reduce.rs | 47 ++-- crates/improvise-protocol/src/resolve.rs | 34 ++- .../improvise-wasm-client/tests/lockstep.rs | 1 + crates/improvise-worker-server/src/worker.rs | 12 +- crates/improvise-ws-server/src/registry.rs | 16 +- 19 files changed, 671 insertions(+), 129 deletions(-) diff --git a/benches/reshape_observation.rs b/benches/reshape_observation.rs index 2f6f89e6..5fc1f2c3 100644 --- a/benches/reshape_observation.rs +++ b/benches/reshape_observation.rs @@ -8,7 +8,7 @@ use improvise::model::cell::{CellKey, CellValue}; use improvise::view::Axis; use improvise::workbook::Workbook; use improvise_command::ui::app::App; -use improvise_command::ui::effect::{CycleAxis, Effect, SetCell}; +use improvise_command::ui::effect::{CycleAxis, Effect, SetCell, ViewTarget}; /// The fixture `grid_projection` uses, so the two numbers are comparable. fn dense_app(regions: usize, products: usize, measures: usize) -> App { @@ -108,7 +108,10 @@ fn geometry_at( let (rows, cols) = (app.layout.row_count(), app.layout.col_count()); let mut best = Duration::MAX; for i in 0..iters.max(3) { - let effects: Vec> = vec![Box::new(CycleAxis("Product".to_string()))]; + let effects: Vec> = vec![Box::new(CycleAxis( + ViewTarget::Active, + "Product".to_string(), + ))]; let start = Instant::now(); std::hint::black_box(app.apply_effects(effects)); best = best.min(start.elapsed()); diff --git a/context/code/command.md b/context/code/command.md index ba7fd1e7..7f3e5b9a 100644 --- a/context/code/command.md +++ b/context/code/command.md @@ -956,13 +956,27 @@ way, and the effect that got here cannot reach the session list. Bumping `document_generation` is how the reset reaches them: `ClientSession::project` compares it and reseeds. -### `adopt_cursor_epoch` — the epoch is the trigger, the view is the audience +### `adopt_cursor_epoch` — the epoch is one trigger of two, the view is the audience `reset_cursor` resets the host's own slice; every connected client owes the same invalidation, and `cursor_epoch` is how the reset reaches them. **The trigger stays -the epoch moving** — which effects renumber the grid is a per-effect decision, so a +observed** — which effects renumber the grid is a per-effect decision, so a table of variants here would be a second description of it (gotcha 24). +**The epoch is the host's own cursor, so it is blind to a view the host is not on**, +and since improvise-lltt a view-configuration command names its view, so such a +write is reachable. The second trigger is `App::last_reframed_elsewhere` — the +non-active views this batch was observed to reframe — and it is a *precise* set +where `last_reframed_views` is deliberately a union with the reshaped set. Two +consequences. A batch that reframes only elsewhere relays against that precise set, +because the union would invalidate a client whose view merely *reshaped* (a page +selection that prunes nothing), which `paging_with_nothing_pruned_invalidates_no_cursor` +forbids. And the opening frames it diffs against are taken **lazily, at +`App::view_mut_of`** — the one door a non-active view can be written through — so +the held-key path still hashes one view and pays nothing for the wider question +(the premise improvise-mcq4 narrowed that arm on was `view_mut()` writing only the +active view, and that premise is gone). + **The audience is not everybody**, and has not been since improvise-jytm made "which view a client is on" per-client: a reshape of view A threw away the cursor of a client sitting on view B, which nothing reshaped. `cursor_invalidated` asks diff --git a/context/test-evidence/command.md b/context/test-evidence/command.md index 445b548b..b064cdbd 100644 --- a/context/test-evidence/command.md +++ b/context/test-evidence/command.md @@ -3496,3 +3496,77 @@ The fix is not local and none is chosen: `grid_in` takes `&App` and `recompute_formulas` needs `&mut Model`, so `grid_in` cannot recompute for the view it is projecting. Candidates are on the ticket. + +### a_view_named_command_lands_on_the_view_it_names + +improvise-lltt, the oldest open P1 here, opened by improvise-jytm making +`ViewState::active_view` authoritative per client. + +Nine `ModelCommand` variants configure a view — `SetAxis`, `CycleAxis`, +`TransposeAxes`, `SetPageSelection`, `HideItem`, `ShowItem`, `ToggleGroup`, +`TogglePruneEmpty`, `SetNumberFormat` — and each named its view *implicitly*: +`model_effects` built an effect that wrote `App::view_mut()`, which is the +**host's** slice. A client that had switched to `Second` therefore moved +`Default`: its own grid did not change, and every other client's did. + +The fix is the protocol change the ticket asks for. Each of the nine carries a +`view: String`, `model_effects` maps it to `ViewTarget::Named`, and the nine +effects carry a `ViewTarget` rather than reading an ambient one. There is no +operandless form left: `App::view_mut()` is **deleted**, replaced by +`view_mut_of(&ViewTarget)`, so a future geometry effect has to say which view it +writes. The host's own key path passes `ViewTarget::Active`, which is right +because the TUI *is* the host and has no session to ask. + +**Fault injected**: `model_effects`' `SetAxis` arm rebuilt with +`ViewTarget::Active` (the pre-fix behaviour, with `view` bound and discarded so +it still compiles). The test fails on `the asker's own view never moved` — +`Second`'s `Channel` stayed on `Row` while `Default`'s moved to `Page`. + +The fixture must seat the asker somewhere the host is not, and the two views must +disagree about the category being moved: `Channel` is `Axis::None` in `Default` +and `Axis::Row` in `Second`, and the command asks for `Axis::Page`, so both +assertions separate the fixed code from the broken code in both directions. + +### a_reframe_of_a_view_the_host_is_not_on_invalidates_its_clients + +improvise-lltt's second half, and the part the ticket did not predict. Naming the +view made a reframe of a view the host is not on reachable for the first time, +and `Sessions::adopt_cursor_epoch` could not see one: it returned early unless +`App::cursor_epoch` had moved, and the epoch is bumped only by `reset_cursor`, +which `apply_effects` calls only for the **host's own** view. So the clients on +the reframed view were reset by nobody and told nothing — gotcha 24's +strictly-worse-than-nothing state, permanently stale rather than stale until the +next scroll. + +`App::last_reframed_elsewhere` is the second trigger. Its opening frames are +taken lazily inside `App::view_mut_of`, which is the one door such a write goes +through, so the held-key path still hashes exactly one view: the wide-observation +cost improvise-mcq4 declined is not paid. + +**Fault injected**: the gate restored to `if !host_moved { return Vec::new(); }`. +The test fails on `a client on the reframed view was never told`. + +Two negative arms, and they are not decoration. A third client left on `Default` +must keep its offset, or "invalidate everyone" passes; and +`paging_with_nothing_pruned_invalidates_no_cursor` is what forbids relaying +against `last_reframed_views`, which is a union with the *reshaped* set and so +fires for a page selection that pruned nothing. That test failed during +development and is why the elsewhere set is precise. + +### a_command_naming_a_view_the_document_has_not_got_writes_nothing + +`App::resolve_view` falls back to the document's default for a name the document +has not got, which is right for *reading* — a slice naming a deleted view must +not panic in `rebuild_layout` — and wrong for **writing**, where it would move a +view nobody asked about. `view_mut_of` therefore looks the name up directly and +returns `None`. + +**Fault injected**: `view_mut_of`'s `Named` arm routed through +`self.resolve_view(name).0`. The test fails on `a name the document has not got +fell back onto a view that has one`. + +The window this closes is narrow — `Sessions::adopt_deleted_views` already moves +a mirror off a deleted view and tells it — but it is a race, not an +impossibility, and the silent alternative is a write to the wrong document state. +Not closed here: the client hears nothing. A `Rejection` variant for it is filed +rather than smuggled in, since `Rejection` is wire vocabulary. diff --git a/crates/improvise-command/src/command/cmd/cell.rs b/crates/improvise-command/src/command/cmd/cell.rs index 3f50d8c8..bd347e0c 100644 --- a/crates/improvise-command/src/command/cmd/cell.rs +++ b/crates/improvise-command/src/command/cmd/cell.rs @@ -1,5 +1,5 @@ use crate::model::Aggregation; -use crate::ui::effect::{self, Effect}; +use crate::ui::effect::{self, Effect, ViewTarget}; use super::core::{Cmd, CmdContext}; @@ -178,7 +178,7 @@ impl Cmd for TransposeAxes { "transpose" } fn execute(&self, _ctx: &CmdContext) -> Vec> { - vec![Box::new(effect::TransposeAxes)] + vec![Box::new(effect::TransposeAxes(ViewTarget::Active))] } } diff --git a/crates/improvise-command/src/command/cmd/effect_cmds.rs b/crates/improvise-command/src/command/cmd/effect_cmds.rs index deccf179..4051a1b1 100644 --- a/crates/improvise-command/src/command/cmd/effect_cmds.rs +++ b/crates/improvise-command/src/command/cmd/effect_cmds.rs @@ -1,5 +1,5 @@ use crate::model::cell::CellValue; -use crate::ui::effect::{self, Effect}; +use crate::ui::effect::{self, Effect, ViewTarget}; use crate::view::Axis; use super::core::{Cmd, CmdContext, require_args}; @@ -283,6 +283,7 @@ effect_cmd!( _ => return vec![], // parse step already validated }; vec![Box::new(effect::SetAxis { + view: ViewTarget::Active, category: args[0].clone(), axis, })] @@ -295,6 +296,7 @@ effect_cmd!( |args: &[String]| require_args("set-page", args, 2), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::SetPageSelection { + view: ViewTarget::Active, category: args[0].clone(), item: args[1].clone(), })] @@ -307,6 +309,7 @@ effect_cmd!( |args: &[String]| require_args("toggle-group", args, 2), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::ToggleGroup { + view: ViewTarget::Active, category: args[0].clone(), group: args[1].clone(), })] @@ -319,6 +322,7 @@ effect_cmd!( |args: &[String]| require_args("hide-item", args, 2), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::HideItem { + view: ViewTarget::Active, category: args[0].clone(), item: args[1].clone(), })] @@ -331,6 +335,7 @@ effect_cmd!( |args: &[String]| require_args("show-item", args, 2), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::ShowItem { + view: ViewTarget::Active, category: args[0].clone(), item: args[1].clone(), })] @@ -351,7 +356,10 @@ effect_cmd!( "set-format", |args: &[String]| require_args("set-format", args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { - vec![Box::new(effect::SetNumberFormat(args.join(" ")))] + vec![Box::new(effect::SetNumberFormat( + ViewTarget::Active, + args.join(" "), + ))] } ); diff --git a/crates/improvise-command/src/command/cmd/grid.rs b/crates/improvise-command/src/command/cmd/grid.rs index 490085bc..0805483e 100644 --- a/crates/improvise-command/src/command/cmd/grid.rs +++ b/crates/improvise-command/src/command/cmd/grid.rs @@ -1,7 +1,7 @@ use crate::model::category::{CategoryKind, DIM_CATEGORY, INDEX_CATEGORY, MEASURE_CATEGORY}; use crate::model::cell::CellValue; use crate::ui::app::AppMode; -use crate::ui::effect::{self, Effect}; +use crate::ui::effect::{self, Effect, ViewTarget}; use crate::view::AxisEntry; use super::core::{Cmd, CmdContext}; @@ -223,11 +223,11 @@ mod tests { let dbg = effects_debug(&effects); assert!( - dbg.contains(r#"SetAxis { category: "Region", axis: Filter }"#), + dbg.contains(r#"SetAxis { view: Active, category: "Region", axis: Filter }"#), "the drilled coordinate should be fixed on Filter, got: {dbg}" ); assert!( - !dbg.contains(r#"SetAxis { category: "Region", axis: Page }"#), + !dbg.contains(r#"SetAxis { view: Active, category: "Region", axis: Page }"#), "and must not also be offered as a page dimension, got: {dbg}" ); } @@ -258,6 +258,7 @@ impl Cmd for ToggleGroupAtCursor { return vec![]; }; vec![Box::new(effect::ToggleGroup { + view: ViewTarget::Active, category: cat, group, })] @@ -293,6 +294,7 @@ impl Cmd for HideSelectedRowItem { }; let item_name = items[0].clone(); vec![Box::new(effect::HideItem { + view: ViewTarget::Active, category: cat_name, item: item_name, })] @@ -390,10 +392,12 @@ impl Cmd for DrillIntoCell { // Records mode: _Index on Row, _Dim on Column effects.push(Box::new(effect::SetAxis { + view: ViewTarget::Active, category: INDEX_CATEGORY.to_string(), axis: crate::view::Axis::Row, })); effects.push(Box::new(effect::SetAxis { + view: ViewTarget::Active, category: DIM_CATEGORY.to_string(), axis: crate::view::Axis::Column, })); @@ -404,10 +408,12 @@ impl Cmd for DrillIntoCell { self.key.0.iter().map(|(c, _)| c.clone()).collect(); for (cat, item) in &self.key.0 { effects.push(Box::new(effect::SetAxis { + view: ViewTarget::Active, category: cat.clone(), axis: crate::view::Axis::Filter, })); effects.push(Box::new(effect::SetPageSelection { + view: ViewTarget::Active, category: cat.clone(), item: item.clone(), })); @@ -419,6 +425,7 @@ impl Cmd for DrillIntoCell { continue; } effects.push(Box::new(effect::SetAxis { + view: ViewTarget::Active, category: cat.clone(), axis: crate::view::Axis::None, })); @@ -439,7 +446,7 @@ impl Cmd for TogglePruneEmpty { fn execute(&self, ctx: &CmdContext) -> Vec> { let currently_on = ctx.view.prune_empty; vec![ - Box::new(effect::TogglePruneEmpty), + Box::new(effect::TogglePruneEmpty(ViewTarget::Active)), effect::set_status(if currently_on { "Showing all rows/columns" } else { @@ -481,16 +488,19 @@ impl Cmd for ToggleRecordsMode { // _Index on Row, _Dim on Column, everything else -> None effects.push(Box::new(effect::SetAxis { + view: ViewTarget::Active, category: INDEX_CATEGORY.to_string(), axis: crate::view::Axis::Row, })); effects.push(Box::new(effect::SetAxis { + view: ViewTarget::Active, category: DIM_CATEGORY.to_string(), axis: crate::view::Axis::Column, })); for name in ctx.model.categories.keys() { if !CategoryKind::of_name(name).is_index_or_dim() { effects.push(Box::new(effect::SetAxis { + view: ViewTarget::Active, category: name.clone(), axis: crate::view::Axis::None, })); diff --git a/crates/improvise-command/src/command/cmd/navigation.rs b/crates/improvise-command/src/command/cmd/navigation.rs index a0c9b41a..3d54c5b0 100644 --- a/crates/improvise-command/src/command/cmd/navigation.rs +++ b/crates/improvise-command/src/command/cmd/navigation.rs @@ -1,4 +1,4 @@ -use crate::ui::effect::{self, Effect}; +use crate::ui::effect::{self, Effect, ViewTarget}; use super::core::{Cmd, CmdContext}; @@ -198,6 +198,7 @@ impl Cmd for PageNext { .enumerate() .map(|(i, (cat, items, _))| { Box::new(effect::SetPageSelection { + view: ViewTarget::Active, category: cat.clone(), item: items[indices[i]].clone(), }) as Box @@ -235,6 +236,7 @@ impl Cmd for PagePrev { .enumerate() .map(|(i, (cat, items, _))| { Box::new(effect::SetPageSelection { + view: ViewTarget::Active, category: cat.clone(), item: items[indices[i]].clone(), }) as Box diff --git a/crates/improvise-command/src/command/cmd/panel.rs b/crates/improvise-command/src/command/cmd/panel.rs index 68fa9721..150b7ce2 100644 --- a/crates/improvise-command/src/command/cmd/panel.rs +++ b/crates/improvise-command/src/command/cmd/panel.rs @@ -1,5 +1,5 @@ use crate::ui::app::AppMode; -use crate::ui::effect::{self, Effect, Panel}; +use crate::ui::effect::{self, Effect, Panel, ViewTarget}; use super::core::{Cmd, CmdContext}; @@ -509,7 +509,7 @@ impl Cmd for CycleAxisAtCursor { } fn execute(&self, ctx: &CmdContext) -> Vec> { if let Some(cat_name) = ctx.cat_at_cursor() { - vec![Box::new(effect::CycleAxis(cat_name))] + vec![Box::new(effect::CycleAxis(ViewTarget::Active, cat_name))] } else { vec![effect::set_status( "Move cursor to a category header to change axis".to_string(), @@ -569,10 +569,12 @@ impl Cmd for FilterToItem { }) => { vec![ Box::new(effect::SetAxis { + view: ViewTarget::Active, category: cat_name.clone(), axis: crate::view::Axis::Page, }), Box::new(effect::SetPageSelection { + view: ViewTarget::Active, category: cat_name.clone(), item: item_name.clone(), }), diff --git a/crates/improvise-command/src/command/cmd/tile.rs b/crates/improvise-command/src/command/cmd/tile.rs index d77d133d..2027cf37 100644 --- a/crates/improvise-command/src/command/cmd/tile.rs +++ b/crates/improvise-command/src/command/cmd/tile.rs @@ -1,4 +1,4 @@ -use crate::ui::effect::{self, Effect}; +use crate::ui::effect::{self, Effect, ViewTarget}; use crate::view::Axis; use super::core::{Cmd, CmdContext}; @@ -124,10 +124,11 @@ impl Cmd for TileAxisOp { }; let axis_effect: Box = match self.axis { Some(axis) => Box::new(effect::SetAxis { + view: ViewTarget::Active, category: name.to_string(), axis, }), - None => Box::new(effect::CycleAxis(name.to_string())), + None => Box::new(effect::CycleAxis(ViewTarget::Active, name.to_string())), }; let status = format!("{} → {}", name, new_axis.plain_label()); vec![axis_effect, effect::set_status(status)] diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index 1cba2e88..9dd1f127 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -21,7 +21,7 @@ use crate::format::parse_number_format; use crate::import::wizard::ImportWizard; use crate::model::cell::{CellKey, CellValue, DataStore}; use crate::persistence; -use crate::ui::effect::{EffectKind, ModelChange}; +use crate::ui::effect::{EffectKind, ModelChange, ViewTarget}; use crate::ui::project::Dirty; use crate::ui::viewport::{compute_col_widths, compute_row_header_width, compute_visible_cols}; use crate::view::{GridLayout, View}; @@ -328,6 +328,12 @@ pub struct App { /// The views the **last** batch was observed to REFRAME: different rows or /// columns, in a different order. What invalidates a cursor (improvise-ni6k). last_reframed_views: std::collections::BTreeSet, + /// The frame each **non-active** view had before this batch wrote it, captured + /// at the one door such a write goes through (improvise-lltt). + written_view_frames: std::collections::BTreeMap, + /// The non-active views the last batch reframed. The host's own cursor cannot + /// witness these, so `cursor_epoch` alone is blind to them. + last_reframed_elsewhere: std::collections::BTreeSet, /// **Config**: assembled at startup by `KeymapSet::default_keymaps()`. Read /// every keypress, never mutated during a session. keymap_set: KeymapSet, @@ -382,6 +388,8 @@ impl App { cursor_epoch: 0, last_reshaped_views: std::collections::BTreeSet::new(), last_reframed_views: std::collections::BTreeSet::new(), + written_view_frames: std::collections::BTreeMap::new(), + last_reframed_elsewhere: std::collections::BTreeSet::new(), keymap_set: KeymapSet::default_keymaps(), storage, } @@ -534,11 +542,53 @@ impl App { self.resolve_view(&self.view_state.active_view).1 } - /// The same view, to mutate. View configuration is document state shared by - /// every client on that view, so this is where a geometry effect writes. - pub fn view_mut(&mut self) -> &mut View { - let name = self.view_name().to_string(); - &mut self.model_state.workbook.views[&name] + /// The view a geometry effect names, to mutate. There is **no operandless + /// form**: the ambiguity improvise-lltt closed was exactly that one. + pub fn view_mut_of(&mut self, target: &ViewTarget) -> Option<&mut View> { + // A `Named` that the document has not got is a no-op, never a fallback: + // `resolve_view`'s default would move a view nobody asked about. + let name = match target { + ViewTarget::Active => self.view_name().to_string(), + ViewTarget::Named(name) => name.clone(), + }; + self.remember_elsewhere(&name); + self.model_state.workbook.views.get_mut(&name) + } + + /// Take the opening frame of a view this batch is about to write and the host + /// is not on — the lazy half of `apply_effects`' observation (improvise-lltt). + fn remember_elsewhere(&mut self, name: &str) { + let unseen = name != self.view_name() + && !self.written_view_frames.contains_key(name) + && self.model_state.workbook.views.contains_key(name); + if unseen { + let workbook = &self.model_state.workbook; + let frame = view_frame(&workbook.model, &workbook.views[name]); + self.written_view_frames.insert(name.to_string(), frame); + } + } + + /// Which non-active views the last batch reframed. A **precise** set, unlike + /// [`last_reframed_views`](Self::last_reframed_views) — `context/code/command.md`. + pub fn last_reframed_elsewhere(&self) -> &std::collections::BTreeSet { + &self.last_reframed_elsewhere + } + + /// Diff every frame `remember_elsewhere` took. The audience for a view the + /// host is not on, which no cursor of the host's can witness. + fn observe_written_view_frames(&mut self) { + let before = std::mem::take(&mut self.written_view_frames); + let workbook = &self.model_state.workbook; + self.last_reframed_elsewhere = before + .into_iter() + .filter(|(name, was)| match workbook.views.get(name) { + Some(view) => view_frame(&workbook.model, view) != *was, + None => true, + }) + .map(|(name, _)| name) + .collect(); + let moved = self.last_reframed_elsewhere.clone(); + self.last_reframed_views.extend(moved); } /// Send the active view's cursor to the origin after a shape change. Clients @@ -751,6 +801,8 @@ impl App { let mut frame_basis = moves_geometry.then(|| self.opening_view_frame_keyed()); let mut frames_basis = writes_cells.then(|| view_frames(&self.model_state.workbook)); self.last_reframed_views.clear(); + self.last_reframed_elsewhere.clear(); + self.written_view_frames.clear(); let mut dirty = Dirty::NOTHING; for effect in effects { let kind = effect.kind(); @@ -830,6 +882,7 @@ impl App { if let Some(before) = records_before { self.bump_changed_records(&before); } + self.observe_written_view_frames(); self.observe_reshape(shapes_before); // The audience's half, and over-reporting is the safe direction: this set // is read only behind the epoch gate — `context/code/command.md`. @@ -1268,6 +1321,7 @@ mod tests { fn flip_to_q2(app: &mut App) { app.apply_effects(vec![Box::new(super::super::effect::SetPageSelection { + view: ViewTarget::Active, category: "Quarter".to_string(), item: "Q2".to_string(), })]); @@ -1338,6 +1392,7 @@ mod tests { // A batch that reframes nothing. With the guard removed, the opening // basis is the FROZEN layout and the in-loop hash is the LIVE one. app.apply_effects(vec![Box::new(super::super::effect::SetNumberFormat( + ViewTarget::Active, ",.3f".to_string(), ))]); @@ -1370,7 +1425,10 @@ mod tests { // changes nothing. Comparing across the switch is a near-certain mismatch. app.apply_effects(vec![ Box::new(super::super::effect::SwitchView("Second".to_string())), - Box::new(super::super::effect::SetNumberFormat(",.2f".to_string())), + Box::new(super::super::effect::SetNumberFormat( + ViewTarget::Active, + ",.2f".to_string(), + )), ]); assert_eq!( @@ -1413,7 +1471,7 @@ mod tests { app.apply_effects(vec![ Box::new(super::super::effect::SwitchView("Second".to_string())), - Box::new(super::super::effect::TransposeAxes), + Box::new(super::super::effect::TransposeAxes(ViewTarget::Active)), ]); assert_eq!( @@ -1491,6 +1549,7 @@ mod tests { let epoch = app.cursor_epoch(); app.apply_effects(vec![Box::new(super::super::effect::SetPageSelection { + view: ViewTarget::Active, category: "Quarter".to_string(), item: "Q1".to_string(), })]); @@ -3666,6 +3725,7 @@ mod basis_tiers { assert_eq!(app.view().axis_of("Month"), Axis::Column); app.apply_effects(vec![Box::new(SetAxis { + view: ViewTarget::Active, category: "Month".into(), axis: Axis::Row, })]); @@ -3694,8 +3754,10 @@ mod basis_tiers { // Both arms, because the CellData half never left the loop: a test over // the geometry arm alone cannot say whether the ordering rule is general. - let geometry: Vec> = - vec![Box::new(SetNumberFormat(",.2f".into())), Box::new(Save)]; + let geometry: Vec> = vec![ + Box::new(SetNumberFormat(ViewTarget::Active, ",.2f".into())), + Box::new(Save), + ]; let data: Vec> = vec![ Box::new(SetCell(cell("North", "Revenue"), CellValue::Number(99.0))), Box::new(Save), @@ -3909,6 +3971,7 @@ mod basis_tiers { Box::new(AbortChain), Box::new(SetCell(cell("South", "Revenue"), CellValue::Number(88.0))), Box::new(SetAxis { + view: ViewTarget::Active, category: "Month".into(), axis: Axis::Column, }), diff --git a/crates/improvise-command/src/ui/effect.rs b/crates/improvise-command/src/ui/effect.rs index d79469a9..ebbe03fe 100644 --- a/crates/improvise-command/src/ui/effect.rs +++ b/crates/improvise-command/src/ui/effect.rs @@ -36,6 +36,14 @@ pub enum ModelChange { WholeWorkbook, } +/// Which view a geometry effect writes. The host's own key path means "the one +/// this app is on"; a wire command names one — `context/code/command.md`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ViewTarget { + Active, + Named(String), +} + /// A discrete state change produced by a command. /// Effects know how to apply themselves to the App. pub trait Effect: Debug { @@ -241,11 +249,12 @@ impl Effect for EnterEditAtCursor { } #[derive(Debug)] -pub struct TogglePruneEmpty; +pub struct TogglePruneEmpty(pub ViewTarget); impl Effect for TogglePruneEmpty { fn apply(&self, app: &mut App) { - let v = app.view_mut(); - v.prune_empty = !v.prune_empty; + if let Some(v) = app.view_mut_of(&self.0) { + v.prune_empty = !v.prune_empty; + } } fn kind(&self) -> EffectKind { EffectKind::Model(ModelChange::LayoutGeometry) @@ -382,12 +391,15 @@ impl Effect for ViewForward { #[derive(Debug)] pub struct SetAxis { + pub view: ViewTarget, pub category: String, pub axis: Axis, } impl Effect for SetAxis { fn apply(&self, app: &mut App) { - app.view_mut().set_axis(&self.category, self.axis); + if let Some(v) = app.view_mut_of(&self.view) { + v.set_axis(&self.category, self.axis); + } } fn kind(&self) -> EffectKind { EffectKind::Model(ModelChange::LayoutGeometry) @@ -396,13 +408,15 @@ impl Effect for SetAxis { #[derive(Debug)] pub struct SetPageSelection { + pub view: ViewTarget, pub category: String, pub item: String, } impl Effect for SetPageSelection { fn apply(&self, app: &mut App) { - app.view_mut() - .set_page_selection(&self.category, &self.item); + if let Some(v) = app.view_mut_of(&self.view) { + v.set_page_selection(&self.category, &self.item); + } } fn kind(&self) -> EffectKind { EffectKind::Model(ModelChange::LayoutGeometry) @@ -411,13 +425,15 @@ impl Effect for SetPageSelection { #[derive(Debug)] pub struct ToggleGroup { + pub view: ViewTarget, pub category: String, pub group: String, } impl Effect for ToggleGroup { fn apply(&self, app: &mut App) { - app.view_mut() - .toggle_group_collapse(&self.category, &self.group); + if let Some(v) = app.view_mut_of(&self.view) { + v.toggle_group_collapse(&self.category, &self.group); + } } fn kind(&self) -> EffectKind { EffectKind::Model(ModelChange::LayoutGeometry) @@ -426,12 +442,15 @@ impl Effect for ToggleGroup { #[derive(Debug)] pub struct HideItem { + pub view: ViewTarget, pub category: String, pub item: String, } impl Effect for HideItem { fn apply(&self, app: &mut App) { - app.view_mut().hide_item(&self.category, &self.item); + if let Some(v) = app.view_mut_of(&self.view) { + v.hide_item(&self.category, &self.item); + } } fn kind(&self) -> EffectKind { EffectKind::Model(ModelChange::LayoutGeometry) @@ -440,12 +459,15 @@ impl Effect for HideItem { #[derive(Debug)] pub struct ShowItem { + pub view: ViewTarget, pub category: String, pub item: String, } impl Effect for ShowItem { fn apply(&self, app: &mut App) { - app.view_mut().show_item(&self.category, &self.item); + if let Some(v) = app.view_mut_of(&self.view) { + v.show_item(&self.category, &self.item); + } } fn kind(&self) -> EffectKind { EffectKind::Model(ModelChange::LayoutGeometry) @@ -453,10 +475,12 @@ impl Effect for ShowItem { } #[derive(Debug)] -pub struct TransposeAxes; +pub struct TransposeAxes(pub ViewTarget); impl Effect for TransposeAxes { fn apply(&self, app: &mut App) { - app.view_mut().transpose_axes(); + if let Some(v) = app.view_mut_of(&self.0) { + v.transpose_axes(); + } } fn kind(&self) -> EffectKind { EffectKind::Model(ModelChange::LayoutGeometry) @@ -464,10 +488,13 @@ impl Effect for TransposeAxes { } #[derive(Debug)] -pub struct CycleAxis(pub String); +pub struct CycleAxis(pub ViewTarget, pub String); impl Effect for CycleAxis { fn apply(&self, app: &mut App) { - app.view_mut().cycle_axis(&self.0); + let category = self.1.clone(); + if let Some(v) = app.view_mut_of(&self.0) { + v.cycle_axis(&category); + } } fn kind(&self) -> EffectKind { EffectKind::Model(ModelChange::LayoutGeometry) @@ -475,10 +502,13 @@ impl Effect for CycleAxis { } #[derive(Debug)] -pub struct SetNumberFormat(pub String); +pub struct SetNumberFormat(pub ViewTarget, pub String); impl Effect for SetNumberFormat { fn apply(&self, app: &mut App) { - app.view_mut().number_format = self.0.clone(); + let format = self.1.clone(); + if let Some(v) = app.view_mut_of(&self.0) { + v.number_format = format; + } } fn kind(&self) -> EffectKind { EffectKind::Model(ModelChange::LayoutGeometry) @@ -1362,15 +1392,19 @@ mod tests { ), ( Box::new(SetAxis { + view: ViewTarget::Active, category: "Type".into(), axis: Axis::Page, }), geom, ), - (Box::new(TransposeAxes), geom), + (Box::new(TransposeAxes(ViewTarget::Active)), geom), (Box::new(CreateView("Other".into())), geom), - (Box::new(SetNumberFormat(",.3f".into())), geom), - (Box::new(TogglePruneEmpty), geom), + ( + Box::new(SetNumberFormat(ViewTarget::Active, ",.3f".into())), + geom, + ), + (Box::new(TogglePruneEmpty(ViewTarget::Active)), geom), ]; for (effect, expected) in cases { assert_eq!( @@ -1420,29 +1454,34 @@ mod tests { Box::new(CreateView("Second".into())), Box::new(DeleteView("Default".into())), Box::new(SetAxis { + view: ViewTarget::Active, category: "Type".into(), axis: Axis::Page, }), Box::new(SetPageSelection { + view: ViewTarget::Active, category: "Type".into(), item: "Clothing".into(), }), Box::new(ToggleGroup { + view: ViewTarget::Active, category: "Type".into(), group: "Wearables".into(), }), Box::new(HideItem { + view: ViewTarget::Active, category: "Type".into(), item: "Food".into(), }), Box::new(ShowItem { + view: ViewTarget::Active, category: "Type".into(), item: "Clothing".into(), }), - Box::new(TransposeAxes), - Box::new(CycleAxis("Type".into())), - Box::new(SetNumberFormat(",.3f".into())), - Box::new(TogglePruneEmpty), + Box::new(TransposeAxes(ViewTarget::Active)), + Box::new(CycleAxis(ViewTarget::Active, "Type".into())), + Box::new(SetNumberFormat(ViewTarget::Active, ",.3f".into())), + Box::new(TogglePruneEmpty(ViewTarget::Active)), ]; // A COUNT, not a totality check: nothing forces a fourteenth here, and a // probe variant proved it — improvise-5ubs, `context/code/command.md`. @@ -1708,11 +1747,12 @@ mod tests { // break, so it must still see the axis move. app.apply_effects(vec![ Box::new(SetAxis { + view: ViewTarget::Active, category: "Type".into(), axis: Axis::Page, }), Box::new(AbortChain), - Box::new(TogglePruneEmpty), + Box::new(TogglePruneEmpty(ViewTarget::Active)), ]); assert!(app.last_batch_aborted(), "the fixture must actually abort"); assert_eq!( @@ -1731,6 +1771,7 @@ mod tests { app.apply_effects(vec![ Box::new(AbortChain), Box::new(SetAxis { + view: ViewTarget::Active, category: "Type".into(), axis: Axis::Page, }), @@ -1846,6 +1887,7 @@ mod tests { fn set_axis_effect() { let mut app = test_app(); SetAxis { + view: ViewTarget::Active, category: "Type".to_string(), axis: Axis::Page, } @@ -1875,7 +1917,7 @@ mod tests { .into_iter() .map(String::from) .collect(); - TransposeAxes.apply(&mut app); + TransposeAxes(ViewTarget::Active).apply(&mut app); let row_after: Vec = app .model_state .workbook @@ -2481,12 +2523,14 @@ mod tests { ), ("SetAxis", nothing, || { Box::new(SetAxis { + view: ViewTarget::Active, category: "Type".into(), axis: Axis::Column, }) }), ("SetPageSelection", nothing, || { Box::new(SetPageSelection { + view: ViewTarget::Active, category: "Type".into(), item: "Food".into(), }) @@ -2501,6 +2545,7 @@ mod tests { }, || { Box::new(ToggleGroup { + view: ViewTarget::Active, category: "Type".into(), group: "Baked".into(), }) @@ -2508,6 +2553,7 @@ mod tests { ), ("HideItem", nothing, || { Box::new(HideItem { + view: ViewTarget::Active, category: "Type".into(), item: "Food".into(), }) @@ -2515,10 +2561,13 @@ mod tests { ( "ShowItem", |app| { - app.view_mut().hide_item("Type", "Food"); + app.view_mut_of(&ViewTarget::Active) + .unwrap() + .hide_item("Type", "Food"); }, || { Box::new(ShowItem { + view: ViewTarget::Active, category: "Type".into(), item: "Food".into(), }) @@ -2527,16 +2576,24 @@ mod tests { ( "TransposeAxes", |app| { - app.view_mut().set_axis("Type", Axis::Row); - app.view_mut().set_axis("Month", Axis::Column); + app.view_mut_of(&ViewTarget::Active) + .unwrap() + .set_axis("Type", Axis::Row); + app.view_mut_of(&ViewTarget::Active) + .unwrap() + .set_axis("Month", Axis::Column); }, - || Box::new(TransposeAxes), + || Box::new(TransposeAxes(ViewTarget::Active)), ), - ("CycleAxis", nothing, || Box::new(CycleAxis("Type".into()))), - ("SetNumberFormat", nothing, || { - Box::new(SetNumberFormat(",.4f".into())) + ("CycleAxis", nothing, || { + Box::new(CycleAxis(ViewTarget::Active, "Type".into())) + }), + ("SetNumberFormat", nothing, || { + Box::new(SetNumberFormat(ViewTarget::Active, ",.4f".into())) + }), + ("TogglePruneEmpty", nothing, || { + Box::new(TogglePruneEmpty(ViewTarget::Active)) }), - ("TogglePruneEmpty", nothing, || Box::new(TogglePruneEmpty)), ]; for (label, prepare, make) in cases { @@ -2704,9 +2761,9 @@ mod tests { fn toggle_prune_empty_effect() { let mut app = test_app(); let before = app.model_state.workbook.active_view().prune_empty; - TogglePruneEmpty.apply(&mut app); + TogglePruneEmpty(ViewTarget::Active).apply(&mut app); assert_ne!(app.model_state.workbook.active_view().prune_empty, before); - TogglePruneEmpty.apply(&mut app); + TogglePruneEmpty(ViewTarget::Active).apply(&mut app); assert_eq!(app.model_state.workbook.active_view().prune_empty, before); } @@ -2748,7 +2805,7 @@ mod tests { #[test] fn set_number_format_effect() { let mut app = test_app(); - SetNumberFormat(",.2f".to_string()).apply(&mut app); + SetNumberFormat(ViewTarget::Active, ",.2f".to_string()).apply(&mut app); assert_eq!(app.model_state.workbook.active_view().number_format, ",.2f"); } @@ -2758,6 +2815,7 @@ mod tests { fn set_page_selection_effect() { let mut app = test_app(); SetPageSelection { + view: ViewTarget::Active, category: "Type".to_string(), item: "Food".to_string(), } @@ -2777,6 +2835,7 @@ mod tests { fn hide_and_show_item_effects() { let mut app = test_app(); HideItem { + view: ViewTarget::Active, category: "Type".to_string(), item: "Food".to_string(), } @@ -2789,6 +2848,7 @@ mod tests { ); ShowItem { + view: ViewTarget::Active, category: "Type".to_string(), item: "Food".to_string(), } @@ -2807,6 +2867,7 @@ mod tests { fn toggle_group_effect() { let mut app = test_app(); ToggleGroup { + view: ViewTarget::Active, category: "Type".to_string(), group: "MyGroup".to_string(), } @@ -2818,6 +2879,7 @@ mod tests { .is_group_collapsed("Type", "MyGroup") ); ToggleGroup { + view: ViewTarget::Active, category: "Type".to_string(), group: "MyGroup".to_string(), } @@ -2836,7 +2898,7 @@ mod tests { fn cycle_axis_effect() { let mut app = test_app(); let before = app.model_state.workbook.active_view().axis_of("Type"); - CycleAxis("Type".to_string()).apply(&mut app); + CycleAxis(ViewTarget::Active, "Type".to_string()).apply(&mut app); let after = app.model_state.workbook.active_view().axis_of("Type"); assert_ne!(before, after); } diff --git a/crates/improvise-command/src/ui/host.rs b/crates/improvise-command/src/ui/host.rs index 523e4400..51ff2e03 100644 --- a/crates/improvise-command/src/ui/host.rs +++ b/crates/improvise-command/src/ui/host.rs @@ -890,7 +890,8 @@ mod tests { let (asker, _) = host.connect(10, 10); let (bystander, _) = host.connect(10, 10); - let frames = host.model(asker, ModelCommand::TransposeAxes); + let view = host.app().view_name().to_string(); + let frames = host.model(asker, ModelCommand::TransposeAxes { view }); let reset = ServerFrame::Commands(vec![Command::ResetCursor]); for (who, id) in [("asker", asker), ("bystander", bystander)] { diff --git a/crates/improvise-command/src/ui/project.rs b/crates/improvise-command/src/ui/project.rs index ae9c0bb6..340b6060 100644 --- a/crates/improvise-command/src/ui/project.rs +++ b/crates/improvise-command/src/ui/project.rs @@ -972,12 +972,19 @@ impl Sessions { /// If the batch renumbered the grid, invalidate the mirrors that were looking /// at it **and** say so on the wire; neither half works alone (improvise-gvnb). fn adopt_cursor_epoch(&mut self, app: &App, before: u64) -> Vec { - if app.cursor_epoch() == before { + // The epoch is the HOST's cursor, so it is blind to a view the host is not + // on — which a view-named command reframes (improvise-lltt). + let host_moved = app.cursor_epoch() != before; + let elsewhere = app.last_reframed_elsewhere(); + if !host_moved && elsewhere.is_empty() { return Vec::new(); } // The REFRAME set, not the reshape set: a cursor is invalidated by the // rows moving, which is a different question from the basis's (ni6k). - let reshaped = app.last_reframed_views(); + let reshaped = match host_moved { + true => app.last_reframed_views(), + false => elsewhere, + }; self.sessions .iter_mut() .filter(|(_, session)| cursor_invalidated(reshaped, &session.view.active_view)) @@ -1053,7 +1060,7 @@ fn fresh_view(app: &App) -> ViewState { mod tests { use super::*; use crate::model::cell::{CellKey, CellValue}; - use crate::ui::effect::{AddFormula, Effect, RemoveCategory, SetAxis, SetCell}; + use crate::ui::effect::{AddFormula, Effect, RemoveCategory, SetAxis, SetCell, ViewTarget}; use crate::workbook::Workbook; use improvise_protocol::{ BasisCache, CategoryTreeCache, DocumentCache, FormulaCache, TileBarCache, ViewListCache, @@ -1495,6 +1502,7 @@ mod tests { target_category: "_Measure".to_string(), })], vec![Box::new(SetAxis { + view: ViewTarget::Active, category: "Region".to_string(), axis: Axis::Column, })], @@ -2153,7 +2161,9 @@ mod tests { sessions.apply(&mut app, asker, &Command::SetRowOffset(1)); sessions.apply(&mut app, bystander, &Command::SetRowOffset(2)); - let applied = sessions.apply_caught_up(&mut app, asker, ModelCommand::TransposeAxes); + let view = app.view_name().to_string(); + let applied = + sessions.apply_caught_up(&mut app, asker, ModelCommand::TransposeAxes { view }); assert_eq!( sessions @@ -2542,26 +2552,30 @@ mod tests { /// The commands that renumber the grid. A **shared** table: the cursor audience /// and the two shape counters must answer for the same cases (improvise-pdvm). - fn reshaping_commands() -> Vec { + fn reshaping_commands(view: &str) -> Vec { use improvise_protocol::ModelCommand; vec![ ModelCommand::SetAxis { + view: view.into(), category: "Region".into(), axis: Axis::Column, }, ModelCommand::HideItem { + view: view.into(), category: "Region".into(), item: "North".into(), }, ModelCommand::ShowItem { + view: view.into(), category: "Product".into(), item: "Gadget".into(), }, ModelCommand::ToggleGroup { + view: view.into(), category: "Product".into(), group: "Hardware".into(), }, - ModelCommand::TogglePruneEmpty, + ModelCommand::TogglePruneEmpty { view: view.into() }, ModelCommand::AddCategory("Zone".into()), ModelCommand::RemoveCategory("Region".into()), ModelCommand::AddItem { @@ -2594,7 +2608,8 @@ mod tests { /// the case is only evidence if the grid really moved (improvise-9mo9). #[test] fn every_reshaping_model_command_invalidates_every_cursor() { - for cmd in reshaping_commands() { + let named = app_with_a_reshapeable_grid(); + for cmd in reshaping_commands(named.view_name()) { let label = format!("{cmd:?}"); let mut app = app_with_a_reshapeable_grid(); let before = grid_shape(&app); @@ -2634,18 +2649,21 @@ mod tests { /// The commands the document ACCEPTS while changing nothing. Shared with the /// counter test for the reason `reshaping_commands` is (improvise-pdvm). - fn no_op_commands() -> Vec { + fn no_op_commands(view: &str) -> Vec { use improvise_protocol::ModelCommand; vec![ ModelCommand::SetAxis { + view: view.into(), category: "Region".into(), axis: Axis::Row, }, ModelCommand::HideItem { + view: view.into(), category: "Product".into(), item: "Gadget".into(), }, ModelCommand::ShowItem { + view: view.into(), category: "Product".into(), item: "Widget".into(), }, @@ -2666,18 +2684,22 @@ mod tests { // Review's cases: a name the grid never drew, and a `_Measure` item // that exists only as a formula target (gotcha 7). ModelCommand::SetAxis { + view: view.into(), category: "NoSuchCategory".into(), axis: Axis::Row, }, ModelCommand::HideItem { + view: view.into(), category: "Region".into(), item: "NoSuchItem".into(), }, ModelCommand::ShowItem { + view: view.into(), category: "Region".into(), item: "Ghost".into(), }, ModelCommand::ToggleGroup { + view: view.into(), category: "Product".into(), group: "NoSuchGroup".into(), }, @@ -2692,7 +2714,8 @@ mod tests { /// nothing reshaped nothing either (found in review of improvise-9mo9). #[test] fn an_accepted_no_op_invalidates_no_cursor() { - for cmd in no_op_commands() { + let named = app_with_a_reshapeable_grid(); + for cmd in no_op_commands(named.view_name()) { let label = format!("{cmd:?}"); let mut app = app_with_a_reshapeable_grid(); let before = grid_shape(&app); @@ -2775,10 +2798,12 @@ mod tests { let (bystander, _) = sessions.connect(&app, 100, 100); sessions.apply(&mut app, bystander, &Command::SetRowOffset(1)); + let view = app.view_name().to_string(); let applied = sessions.apply_caught_up( &mut app, asker, ModelCommand::SetPageSelection { + view: view.clone(), category: "Quarter".into(), item: "Q2".into(), }, @@ -2824,10 +2849,12 @@ mod tests { let (bystander, _) = sessions.connect(&app, 100, 100); sessions.apply(&mut app, bystander, &Command::SetRowOffset(1)); + let view = app.view_name().to_string(); let applied = sessions.apply_caught_up( &mut app, asker, ModelCommand::SetPageSelection { + view: view.clone(), category: "Quarter".into(), item: "Q2".into(), }, @@ -3036,6 +3063,132 @@ mod tests { app } + /// improvise-lltt: the command names its view, so it lands on the asker's view + /// and not on the host's. See `context/test-evidence/command.md`. + #[test] + fn a_view_named_command_lands_on_the_view_it_names() { + use improvise_protocol::ModelCommand; + + let mut app = app_with_two_reshapeable_views(); + let mut sessions = Sessions::default(); + let (asker, _) = sessions.connect(&app, 100, 100); + sessions.apply(&mut app, asker, &Command::SwitchView("Second".into())); + assert_eq!( + app.view_name(), + "Default", + "the host stays on its own view: that split is what makes this ambiguous" + ); + + sessions.apply_caught_up( + &mut app, + asker, + ModelCommand::SetAxis { + view: "Second".into(), + category: "Channel".into(), + axis: Axis::Page, + }, + ); + + let views = &app.model_state.workbook.views; + assert_eq!( + views["Second"].axis_of("Channel"), + Axis::Page, + "the asker's own view never moved" + ); + assert_eq!( + views["Default"].axis_of("Channel"), + Axis::None, + "the axis moved on the view the asker is NOT on" + ); + } + + /// improvise-lltt: naming a view the document has not got writes nothing — + /// `resolve_view`'s fallback would move the default instead. + #[test] + fn a_command_naming_a_view_the_document_has_not_got_writes_nothing() { + use improvise_protocol::ModelCommand; + + let mut app = app_with_two_reshapeable_views(); + let before = format!("{:?}", app.model_state.workbook.views); + let mut sessions = Sessions::default(); + let (asker, _) = sessions.connect(&app, 100, 100); + + sessions.apply_caught_up( + &mut app, + asker, + ModelCommand::SetAxis { + view: "NoSuchView".into(), + category: "Channel".into(), + axis: Axis::Page, + }, + ); + + assert_eq!( + format!("{:?}", app.model_state.workbook.views), + before, + "a name the document has not got fell back onto a view that has one" + ); + } + + /// improvise-lltt: `cursor_epoch` is the host's own cursor, so it cannot + /// witness a reframe elsewhere. See `context/test-evidence/command.md`. + #[test] + fn a_reframe_of_a_view_the_host_is_not_on_invalidates_its_clients() { + use improvise_protocol::ModelCommand; + + let mut app = app_with_two_reshapeable_views(); + let mut sessions = Sessions::default(); + let (asker, _) = sessions.connect(&app, 100, 100); + let (bystander, _) = sessions.connect(&app, 100, 100); + let (at_home, _) = sessions.connect(&app, 100, 100); + for who in [asker, bystander] { + sessions.apply(&mut app, who, &Command::SwitchView("Second".into())); + } + sessions.apply(&mut app, bystander, &Command::SetRowOffset(1)); + sessions.apply(&mut app, at_home, &Command::SetRowOffset(2)); + + let applied = sessions.apply_caught_up( + &mut app, + asker, + ModelCommand::HideItem { + view: "Second".into(), + category: "Channel".into(), + item: "Direct".into(), + }, + ); + let broadcast = diff(applied).1; + + assert!( + broadcast.contains(&Broadcast::To(bystander, Command::ResetCursor)), + "a client on the reframed view was never told: {broadcast:?}" + ); + assert_eq!( + sessions + .get_mut(bystander) + .unwrap() + .view() + .cursor() + .row_offset, + 0, + "the mirror kept an offset into a grid that no longer has those rows" + ); + // The negative arm, or "invalidate everyone" passes this test. + assert!( + !broadcast.contains(&Broadcast::To(at_home, Command::ResetCursor)), + "a client on `Default` was invalidated by a reframe of `Second`" + ); + assert_eq!( + sessions + .get_mut(at_home) + .unwrap() + .view() + .cursor() + .row_offset, + 2, + "a client on `Default` lost its place to a reframe of `Second`" + ); + } + /// improvise-nynk: which view a client is on is that client's, so a reshape of /// one view is not addressed to a client sitting on another. #[test] @@ -3056,12 +3209,13 @@ mod tests { "the bystander never left the view the reshape is aimed at" ); - // Aimed at the host's own view, which is `Default`: `HideItem` writes - // `view_mut()`, so `Second`'s shape is untouched by construction. + // Aimed at `Default` BY NAME since improvise-lltt, so `Second`'s shape is + // untouched — the operand, not an ambient one. let applied = sessions.apply_caught_up( &mut app, asker, ModelCommand::HideItem { + view: "Default".into(), category: "Region".into(), item: "North".into(), }, @@ -3120,7 +3274,8 @@ mod tests { /// -- "has the grid's shape moved under you" -- so they answer over one table. #[test] fn both_shape_counters_move_for_every_reshaping_command() { - for cmd in reshaping_commands() { + let named = app_with_a_reshapeable_grid(); + for cmd in reshaping_commands(named.view_name()) { let label = format!("{cmd:?}"); let mut app = app_with_a_reshapeable_grid(); app.model_state.dirty = false; @@ -3165,10 +3320,14 @@ mod tests { // number format is written to the file while changing no row or column. let config_only = vec![ ModelCommand::HideItem { + view: "Default".into(), category: "Region".into(), item: "NoSuchItem".into(), }, - ModelCommand::SetNumberFormat(",.4f".into()), + ModelCommand::SetNumberFormat { + view: "Default".into(), + format: ",.4f".into(), + }, ]; for cmd in config_only { @@ -3204,7 +3363,8 @@ mod tests { /// moves neither counter and does not raise `dirty` (improvise-75ld). #[test] fn neither_shape_counter_moves_for_an_accepted_no_op() { - for cmd in no_op_commands() { + let named = app_with_a_reshapeable_grid(); + for cmd in no_op_commands(named.view_name()) { let label = format!("{cmd:?}"); let mut app = app_with_a_reshapeable_grid(); app.model_state.dirty = false; @@ -3243,10 +3403,14 @@ mod tests { sessions.apply(&mut app, bystander, &Command::SetRowOffset(2)); + let view = app.view_name().to_string(); let applied = sessions.apply_caught_up( &mut app, asker, - ModelCommand::SetNumberFormat(",.2f".into()), + ModelCommand::SetNumberFormat { + view, + format: ",.2f".into(), + }, ); assert_eq!( @@ -3280,7 +3444,9 @@ mod tests { host_dispatch(&mut client, &applied, bystander); assert_eq!(client.cursor().row_offset, 2); - let applied = sessions.apply_caught_up(&mut app, asker, ModelCommand::TransposeAxes); + let view = app.view_name().to_string(); + let applied = + sessions.apply_caught_up(&mut app, asker, ModelCommand::TransposeAxes { view }); host_dispatch(&mut client, &applied, bystander); assert_eq!( @@ -3620,6 +3786,7 @@ mod tests { vec![ Box::new(crate::ui::effect::AbortChain), Box::new(SetAxis { + view: ViewTarget::Active, category: "Region".to_string(), axis: Axis::Column, }), @@ -4405,7 +4572,7 @@ mod cell_identity { mod basis_check { use super::*; use crate::model::cell::{CellKey, CellValue}; - use crate::ui::effect::{RemoveItem, SetAxis, SetCell}; + use crate::ui::effect::{RemoveItem, SetAxis, SetCell, ViewTarget}; use crate::workbook::Workbook; use improvise_core::view::Axis; use improvise_protocol::{Basis, ModelCommand}; @@ -4508,6 +4675,7 @@ mod basis_check { // A reshapes the grid. No record B named is written by this. app.apply_effects(vec![Box::new(SetAxis { + view: ViewTarget::Active, category: "Region".to_string(), axis: Axis::Column, })]); @@ -4531,6 +4699,7 @@ mod basis_check { let (id, _) = sessions.connect(&app, 100, 100); app.apply_effects(vec![Box::new(SetAxis { + view: ViewTarget::Active, category: "Region".to_string(), axis: Axis::Column, })]); diff --git a/crates/improvise-command/src/ui/reduce.rs b/crates/improvise-command/src/ui/reduce.rs index f3716cc6..9c6b5ee4 100644 --- a/crates/improvise-command/src/ui/reduce.rs +++ b/crates/improvise-command/src/ui/reduce.rs @@ -7,7 +7,7 @@ use improvise_protocol::{Basis, Command, ModelCommand, Rejection}; use crate::command::key::KeyEvent; use crate::ui::app::App; -use crate::ui::effect::{self, Effect}; +use crate::ui::effect::{self, Effect, ViewTarget}; use crate::ui::project::{Dirty, document_projections}; /// A wire [`Command`] whose basis the authoritative state has answered for @@ -166,30 +166,65 @@ pub fn model_effects(app: &App, cmd: &ModelCommand) -> Vec> { ModelCommand::AddCategory(name) => Box::new(effect::AddCategory(name.clone())), ModelCommand::RemoveCategory(name) => Box::new(effect::RemoveCategory(name.clone())), - ModelCommand::SetAxis { category, axis } => Box::new(effect::SetAxis { + ModelCommand::SetAxis { + view, + category, + axis, + } => Box::new(effect::SetAxis { + view: ViewTarget::Named(view.clone()), category: category.clone(), axis: *axis, }), - ModelCommand::CycleAxis(category) => Box::new(effect::CycleAxis(category.clone())), - ModelCommand::TransposeAxes => Box::new(effect::TransposeAxes), - ModelCommand::SetPageSelection { category, item } => Box::new(effect::SetPageSelection { + ModelCommand::CycleAxis { view, category } => Box::new(effect::CycleAxis( + ViewTarget::Named(view.clone()), + category.clone(), + )), + ModelCommand::TransposeAxes { view } => { + Box::new(effect::TransposeAxes(ViewTarget::Named(view.clone()))) + } + ModelCommand::SetPageSelection { + view, + category, + item, + } => Box::new(effect::SetPageSelection { + view: ViewTarget::Named(view.clone()), category: category.clone(), item: item.clone(), }), - ModelCommand::HideItem { category, item } => Box::new(effect::HideItem { + ModelCommand::HideItem { + view, + category, + item, + } => Box::new(effect::HideItem { + view: ViewTarget::Named(view.clone()), category: category.clone(), item: item.clone(), }), - ModelCommand::ShowItem { category, item } => Box::new(effect::ShowItem { + ModelCommand::ShowItem { + view, + category, + item, + } => Box::new(effect::ShowItem { + view: ViewTarget::Named(view.clone()), category: category.clone(), item: item.clone(), }), - ModelCommand::ToggleGroup { category, group } => Box::new(effect::ToggleGroup { + ModelCommand::ToggleGroup { + view, + category, + group, + } => Box::new(effect::ToggleGroup { + view: ViewTarget::Named(view.clone()), category: category.clone(), group: group.clone(), }), - ModelCommand::TogglePruneEmpty => Box::new(effect::TogglePruneEmpty), - ModelCommand::SetNumberFormat(fmt) => Box::new(effect::SetNumberFormat(fmt.clone())), + ModelCommand::TogglePruneEmpty { view } => { + Box::new(effect::TogglePruneEmpty(ViewTarget::Named(view.clone()))) + } + ModelCommand::SetNumberFormat { view, format } => Box::new(effect::SetNumberFormat( + ViewTarget::Named(view.clone()), + format.clone(), + )), ModelCommand::CreateView(name) => Box::new(effect::CreateView(name.clone())), ModelCommand::DeleteView(name) => Box::new(effect::DeleteView(name.clone())), @@ -527,29 +562,44 @@ mod tests { ModelCommand::AddCategory("Product".into()), ModelCommand::RemoveCategory("Region".into()), ModelCommand::SetAxis { + view: "Default".into(), category: "Region".into(), axis: Axis::Row, }, - ModelCommand::CycleAxis("Region".into()), - ModelCommand::TransposeAxes, + ModelCommand::CycleAxis { + view: "Default".into(), + category: "Region".into(), + }, + ModelCommand::TransposeAxes { + view: "Default".into(), + }, ModelCommand::SetPageSelection { + view: "Default".into(), category: "Region".into(), item: "North".into(), }, ModelCommand::HideItem { + view: "Default".into(), category: "Region".into(), item: "North".into(), }, ModelCommand::ShowItem { + view: "Default".into(), category: "Region".into(), item: "North".into(), }, ModelCommand::ToggleGroup { + view: "Default".into(), category: "Region".into(), group: "Coastal".into(), }, - ModelCommand::TogglePruneEmpty, - ModelCommand::SetNumberFormat(",.2f".into()), + ModelCommand::TogglePruneEmpty { + view: "Default".into(), + }, + ModelCommand::SetNumberFormat { + view: "Default".into(), + format: ",.2f".into(), + }, ModelCommand::CreateView("Third".into()), ModelCommand::DeleteView("Second".into()), ModelCommand::LoadModel(PathBuf::from("doc.improv")), @@ -576,14 +626,14 @@ mod tests { ModelCommand::AddCategory(_) => "AddCategory", ModelCommand::RemoveCategory(_) => "RemoveCategory", ModelCommand::SetAxis { .. } => "SetAxis", - ModelCommand::CycleAxis(_) => "CycleAxis", - ModelCommand::TransposeAxes => "TransposeAxes", + ModelCommand::CycleAxis { .. } => "CycleAxis", + ModelCommand::TransposeAxes { .. } => "TransposeAxes", ModelCommand::SetPageSelection { .. } => "SetPageSelection", ModelCommand::HideItem { .. } => "HideItem", ModelCommand::ShowItem { .. } => "ShowItem", ModelCommand::ToggleGroup { .. } => "ToggleGroup", - ModelCommand::TogglePruneEmpty => "TogglePruneEmpty", - ModelCommand::SetNumberFormat(_) => "SetNumberFormat", + ModelCommand::TogglePruneEmpty { .. } => "TogglePruneEmpty", + ModelCommand::SetNumberFormat { .. } => "SetNumberFormat", ModelCommand::CreateView(_) => "CreateView", ModelCommand::DeleteView(_) => "DeleteView", ModelCommand::LoadModel(_) => "LoadModel", @@ -670,13 +720,17 @@ mod tests { /// effect of each `Debug` shape — nothing, `(`, and ` {` after the name. #[test] fn the_effect_name_reader_reads_all_three_debug_shapes() { - assert_eq!(effect_type_name(&effect::TransposeAxes), "TransposeAxes"); + assert_eq!( + effect_type_name(&effect::TransposeAxes(ViewTarget::Active)), + "TransposeAxes" + ); assert_eq!( effect_type_name(&effect::AddCategory("Region".into())), "AddCategory" ); assert_eq!( effect_type_name(&effect::HideItem { + view: ViewTarget::Active, category: "Region".into(), item: "North".into(), }), @@ -748,12 +802,22 @@ mod tests { assert_eq!( by_key(char_key('t'), plain), - by_command(ModelCommand::TransposeAxes, plain), + by_command( + ModelCommand::TransposeAxes { + view: "Default".into(), + }, + plain + ), "`t` and Model(TransposeAxes)" ); assert_eq!( by_key(char_key('P'), plain), - by_command(ModelCommand::TogglePruneEmpty, plain), + by_command( + ModelCommand::TogglePruneEmpty { + view: "Default".into(), + }, + plain + ), "`P` and Model(TogglePruneEmpty)" ); @@ -787,7 +851,12 @@ mod tests { // The layout moves under the client's feet, which is the tier // `Sessions::apply` answers with `Rejection::LayoutMoved`. - let bump = caught_up(&app, ModelCommand::TransposeAxes); + let bump = caught_up( + &app, + ModelCommand::TransposeAxes { + view: app.view_name().to_string(), + }, + ); apply_checked(&mut app, &bump); let refused = Checked::against(&app, &wire).err(); @@ -833,7 +902,9 @@ mod tests { value: CellValue::Number(1.0), }, ModelCommand::AddCategory("Product".into()), - ModelCommand::TransposeAxes, + ModelCommand::TransposeAxes { + view: "Default".into(), + }, ] { let mut app = app(); assert!(!app.model_state.dirty, "the fixture starts clean"); diff --git a/crates/improvise-protocol/src/reduce.rs b/crates/improvise-protocol/src/reduce.rs index 21ad2aa5..d821a1f6 100644 --- a/crates/improvise-protocol/src/reduce.rs +++ b/crates/improvise-protocol/src/reduce.rs @@ -67,8 +67,8 @@ pub enum Command { Project(Projection), } -/// A mutation of the shared document: the 26 `EffectKind::Model(..)` effects -/// minus two, leaving **24**. Derived, not invented — see §2. +/// A mutation of the shared document: 24 variants, derived not invented (§2). A +/// view-configuration variant **names its view** (improvise-lltt). #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub enum ModelCommand { // ── ModelChange::CellData ─────────────────────────────────────── @@ -106,29 +106,44 @@ pub enum ModelCommand { AddCategory(String), RemoveCategory(String), SetAxis { + view: String, category: String, axis: Axis, }, - CycleAxis(String), - TransposeAxes, + CycleAxis { + view: String, + category: String, + }, + TransposeAxes { + view: String, + }, SetPageSelection { + view: String, category: String, item: String, }, HideItem { + view: String, category: String, item: String, }, ShowItem { + view: String, category: String, item: String, }, ToggleGroup { + view: String, category: String, group: String, }, - TogglePruneEmpty, - SetNumberFormat(String), + TogglePruneEmpty { + view: String, + }, + SetNumberFormat { + view: String, + format: String, + }, CreateView(String), DeleteView(String), @@ -158,14 +173,14 @@ pub fn records_named(cmd: &ModelCommand) -> Vec { | ModelCommand::AddCategory(_) | ModelCommand::RemoveCategory(_) | ModelCommand::SetAxis { .. } - | ModelCommand::CycleAxis(_) - | ModelCommand::TransposeAxes + | ModelCommand::CycleAxis { .. } + | ModelCommand::TransposeAxes { .. } | ModelCommand::SetPageSelection { .. } | ModelCommand::HideItem { .. } | ModelCommand::ShowItem { .. } | ModelCommand::ToggleGroup { .. } - | ModelCommand::TogglePruneEmpty - | ModelCommand::SetNumberFormat(_) + | ModelCommand::TogglePruneEmpty { .. } + | ModelCommand::SetNumberFormat { .. } | ModelCommand::CreateView(_) | ModelCommand::DeleteView(_) | ModelCommand::LoadModel(_) @@ -369,7 +384,12 @@ mod tests { &mut view, &modelled(ModelCommand::AddCategory("Product".into())), ); - reduce_view(&mut view, &modelled(ModelCommand::TransposeAxes)); + reduce_view( + &mut view, + &modelled(ModelCommand::TransposeAxes { + view: "Default".into(), + }), + ); assert_eq!(before, format!("{view:?}")); } @@ -435,6 +455,7 @@ mod tests { value: CellValue::Text("pending".into()), }), modelled(ModelCommand::SetAxis { + view: "Default".into(), category: "Region".into(), axis: Axis::Filter, }), @@ -449,7 +470,9 @@ mod tests { model_name: Some("Rows".into()), array_path: None, }), - modelled(ModelCommand::TransposeAxes), + modelled(ModelCommand::TransposeAxes { + view: "Default".into(), + }), ]; for cmd in &stream { let json = serde_json::to_string(cmd).expect("serializes"); diff --git a/crates/improvise-protocol/src/resolve.rs b/crates/improvise-protocol/src/resolve.rs index df377090..6f2c0533 100644 --- a/crates/improvise-protocol/src/resolve.rs +++ b/crates/improvise-protocol/src/resolve.rs @@ -330,7 +330,9 @@ fn composed( // this client's to send; both are the host's — clients.md. "transpose" => Some(vec![Command::Model { basis: state.basis.claim([])?, - cmd: ModelCommand::TransposeAxes, + cmd: ModelCommand::TransposeAxes { + view: state.view.active_view.clone(), + }, }]), // ── editing ────────────────────────────────────────────────── @@ -342,7 +344,13 @@ fn composed( // ── the category panel ─────────────────────────────────────── "cycle-axis-at-cursor" => Some(match cat_at_cursor(state) { - Some(cat) => vec![model(state, ModelCommand::CycleAxis(cat))?], + Some(cat) => vec![model( + state, + ModelCommand::CycleAxis { + view: state.view.active_view.clone(), + category: cat, + }, + )?], None => vec![Command::SetStatus( "Move cursor to a category header to change axis".to_string(), )], @@ -653,6 +661,7 @@ fn page_cycle(state: &ClientState, way: Odometer) -> Option> { Some(Command::Model { basis: state.basis.claim([])?, cmd: ModelCommand::SetPageSelection { + view: state.view.active_view.clone(), category: (*cat).to_string(), item: items[indices[i]].clone(), }, @@ -908,6 +917,7 @@ fn filter_to_item_at(state: &ClientState, index: usize) -> Option> model( state, ModelCommand::SetAxis { + view: state.view.active_view.clone(), category: cat_name.clone(), axis: Axis::Page, }, @@ -915,6 +925,7 @@ fn filter_to_item_at(state: &ClientState, index: usize) -> Option> model( state, ModelCommand::SetPageSelection { + view: state.view.active_view.clone(), category: cat_name.clone(), item: item_name.clone(), }, @@ -1002,10 +1013,14 @@ fn tile_axis_at(state: &ClientState, index: usize, axis: Option) -> Option let new_axis = axis.unwrap_or_else(|| tile.axis.next()); let cmd = match axis { Some(axis) => ModelCommand::SetAxis { + view: state.view.active_view.clone(), category: tile.category.clone(), axis, }, - None => ModelCommand::CycleAxis(tile.category.clone()), + None => ModelCommand::CycleAxis { + view: state.view.active_view.clone(), + category: tile.category.clone(), + }, }; Some(vec![ model(state, cmd)?, @@ -1321,7 +1336,10 @@ mod tests { vec![ Command::Model { basis: state.basis.claim([]).unwrap(), - cmd: ModelCommand::CycleAxis("Product".into()), + cmd: ModelCommand::CycleAxis { + view: state.view.active_view.clone(), + category: "Product".into(), + }, }, // `Column.next()` is `Page` — the *new* axis, as `Enter` says. Command::SetStatus("Product → Page".into()), @@ -1404,6 +1422,7 @@ mod tests { Command::Model { basis: basis.clone(), cmd: ModelCommand::SetAxis { + view: state.view.active_view.clone(), category: "Region".into(), axis: Axis::Page }, @@ -1411,6 +1430,7 @@ mod tests { Command::Model { basis, cmd: ModelCommand::SetPageSelection { + view: state.view.active_view.clone(), category: "Region".into(), item: "North".into() }, @@ -2101,7 +2121,9 @@ mod tests { resolve(&client, ev(KeyCode::Char('t'))), KeyOutcome::Local(vec![Command::Model { basis: client.basis.claim([]).unwrap(), - cmd: ModelCommand::TransposeAxes, + cmd: ModelCommand::TransposeAxes { + view: client.view.active_view.clone(), + }, }]) ); } @@ -2316,7 +2338,7 @@ mod tests { .into_iter() .map(|c| match c { Command::Model { - cmd: ModelCommand::SetPageSelection { category, item }, + cmd: ModelCommand::SetPageSelection { category, item, .. }, .. } => (category, item), other => panic!("expected a page selection, got {other:?}"), diff --git a/crates/improvise-wasm-client/tests/lockstep.rs b/crates/improvise-wasm-client/tests/lockstep.rs index ca3b005d..ed3d7df3 100644 --- a/crates/improvise-wasm-client/tests/lockstep.rs +++ b/crates/improvise-wasm-client/tests/lockstep.rs @@ -473,6 +473,7 @@ fn the_fixtures_basis_tracks_a_host_whose_layout_has_moved() { let bump = caught_up( &app, improvise_protocol::ModelCommand::HideItem { + view: app.view_name().to_string(), category: "Region".into(), item: "South".into(), }, diff --git a/crates/improvise-worker-server/src/worker.rs b/crates/improvise-worker-server/src/worker.rs index 10c05907..c3dcff28 100644 --- a/crates/improvise-worker-server/src/worker.rs +++ b/crates/improvise-worker-server/src/worker.rs @@ -358,9 +358,12 @@ mod tests { // The frame's basis is the FIRST command's, answered once, so that is // where the staleness must be injected. - let Command::Model { mut basis, cmd } = - reduce::caught_up(worker.app(), ModelCommand::TransposeAxes) - else { + let Command::Model { mut basis, cmd } = reduce::caught_up( + worker.app(), + ModelCommand::TransposeAxes { + view: worker.app().view_name().to_string(), + }, + ) else { unreachable!("caught_up builds a Command::Model") }; basis.document_generation += 1; @@ -425,7 +428,8 @@ mod tests { // Dirty the document, so the automatic trigger genuinely would fire // if it were consulted. That write fails and is counted. - let wire = reduce::caught_up(worker.app(), ModelCommand::TransposeAxes); + let view = worker.app().view_name().to_string(); + let wire = reduce::caught_up(worker.app(), ModelCommand::TransposeAxes { view }); worker.on_frame(ClientFrame::command(wire)); assert!( worker.app().model_state.dirty, diff --git a/crates/improvise-ws-server/src/registry.rs b/crates/improvise-ws-server/src/registry.rs index e59c659e..7c34b11b 100644 --- a/crates/improvise-ws-server/src/registry.rs +++ b/crates/improvise-ws-server/src/registry.rs @@ -178,6 +178,16 @@ impl Registry { self.handle(who, ClientFrame::command(wire)) } + /// The view this client's document is on, so a view-scoped command can name + /// one the document really has (improvise-lltt). + #[cfg(test)] + fn view_of(&self, who: &ClientId) -> String { + match self.documents.get(&who.document) { + Some(host) => host.app().view_name().to_string(), + None => String::new(), + } + } + /// One frame from one client, and everything that owes as a result — /// within that client's document. An unopen document produces nothing. pub fn handle(&mut self, who: &ClientId, msg: ClientFrame) -> Vec { @@ -453,7 +463,8 @@ mod tests { let (same_doc, _) = registry.connect(Some("alpha"), 10, 10).unwrap(); let (other_doc, _) = registry.connect(Some("beta"), 10, 10).unwrap(); - let frames = registry.model(&asker, ModelCommand::TransposeAxes); + let view = registry.view_of(&asker); + let frames = registry.model(&asker, ModelCommand::TransposeAxes { view }); let broadcast = ServerFrame::Commands(vec![Command::ResetCursor]); assert!( @@ -472,7 +483,8 @@ mod tests { fn the_other_document_would_also_have_a_cursor_to_invalidate() { let mut registry = registry(); let (elsewhere, _) = registry.connect(Some("beta"), 10, 10).unwrap(); - let frames = registry.model(&elsewhere, ModelCommand::TransposeAxes); + let view = registry.view_of(&elsewhere); + let frames = registry.model(&elsewhere, ModelCommand::TransposeAxes { view }); assert!( to(&frames, &elsewhere).contains(&&ServerFrame::Commands(vec![Command::ResetCursor])), "the isolation test would pass for the wrong reason"