From f4e8e62ec09e004af8becb921bb0eec94df6337f Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Fri, 3 Jul 2026 22:32:13 +0800 Subject: [PATCH] style: rustfmt drift in op-cli design routing and native widget host --- crates/op-cli/src/main.rs | 6 +- .../src/command_app_state_tests.rs | 79 +++++++++++++++++++ crates/op-editor-core/src/command_batch.rs | 5 ++ crates/op-editor-core/src/history.rs | 7 ++ crates/op-editor-core/src/mutators.rs | 2 + crates/op-editor-core/src/state.rs | 1 + crates/op-host-native/src/widget_host.rs | 12 +-- .../op-host-native/src/widget_host/scroll.rs | 13 ++- 8 files changed, 116 insertions(+), 9 deletions(-) diff --git a/crates/op-cli/src/main.rs b/crates/op-cli/src/main.rs index 0ca787c62..0ade19886 100644 --- a/crates/op-cli/src/main.rs +++ b/crates/op-cli/src/main.rs @@ -464,9 +464,9 @@ fn map_design_like( let raw = resolve_arg(payload.map(String::as_str))?; let trimmed = raw.trim(); let script_requested = flags.contains_key("script") - || payload.map(String::as_str).is_some_and(|p| { - p.starts_with('@') && (p.ends_with(".js") || p.ends_with(".mjs")) - }); + || payload + .map(String::as_str) + .is_some_and(|p| p.starts_with('@') && (p.ends_with(".js") || p.ends_with(".mjs"))); let mut pairs = Vec::new(); if script_requested { pairs.push(pair("script", trimmed)); diff --git a/crates/op-editor-core/src/command_app_state_tests.rs b/crates/op-editor-core/src/command_app_state_tests.rs index 38bc28962..770c16169 100644 --- a/crates/op-editor-core/src/command_app_state_tests.rs +++ b/crates/op-editor-core/src/command_app_state_tests.rs @@ -105,3 +105,82 @@ fn merge_app_state_empty_incoming_is_noop() { })); assert!(s.doc.state.is_none()); } + +// (e) A rolled-back batch must not leave stale ownership: the failed +// batch's MergeAppState never landed in doc.state, so a later merge of +// the same key (any plan_idx) must land instead of being skipped +// against a phantom owner. +#[test] +fn rolled_back_batch_leaves_no_stale_app_state_ownership() { + let mut s = EditorState::new(); + let mut m = BTreeMap::new(); + m.insert("cart".into(), entry(1)); + let failed = s.apply(EditorCommand::Batch { + commands: vec![ + EditorCommand::MergeAppState { + plan_idx: 0, + state: m, + }, + // Batchable but fails: no node with this id exists. + EditorCommand::SetNodeText { + node_id: crate::node_id::NodeId::new("no-such-node".to_string()), + text: "x".into(), + }, + ], + }); + assert!(!failed, "batch with a failing sub-command must report false"); + assert!( + s.doc.state.as_ref().is_none_or(|st| !st.contains_key("cart")), + "rolled-back merge must not survive in doc.state" + ); + + let mut retry = BTreeMap::new(); + retry.insert("cart".into(), entry(7)); + assert!( + s.apply(EditorCommand::MergeAppState { + plan_idx: 9, + state: retry, + }), + "post-rollback merge must land — stale ownership would skip it" + ); + assert_eq!( + s.doc.state.as_ref().unwrap().get("cart").unwrap().default, + Some(serde_json::json!(7)) + ); +} + +// (f) Undoing a batch restores the ownership map alongside doc.state, +// so a re-generation after undo starts from a clean slate. +#[test] +fn undo_restores_app_state_ownership_with_the_document() { + let mut s = EditorState::new(); + let mut m = BTreeMap::new(); + m.insert("tab".into(), entry(3)); + assert!(s.apply(EditorCommand::Batch { + commands: vec![EditorCommand::MergeAppState { + plan_idx: 2, + state: m, + }], + })); + assert!(s.doc.state.as_ref().unwrap().contains_key("tab")); + + assert!(s.undo(), "batch lands as one undo step"); + assert!( + s.doc.state.as_ref().is_none_or(|st| !st.contains_key("tab")), + "undo must remove the merged key from doc.state" + ); + + let mut again = BTreeMap::new(); + again.insert("tab".into(), entry(8)); + assert!( + s.apply(EditorCommand::MergeAppState { + plan_idx: 5, + state: again, + }), + "post-undo merge must land — ownership must have been restored" + ); + assert_eq!( + s.doc.state.as_ref().unwrap().get("tab").unwrap().default, + Some(serde_json::json!(8)) + ); +} diff --git a/crates/op-editor-core/src/command_batch.rs b/crates/op-editor-core/src/command_batch.rs index 5c426f5d2..0d4ed0c9c 100644 --- a/crates/op-editor-core/src/command_batch.rs +++ b/crates/op-editor-core/src/command_batch.rs @@ -162,6 +162,11 @@ impl EditorState { self.selection = pre.selection; self.ui.active_page_index = pre.active_page_index; self.components = pre.components; + // Ownership must roll back with doc.state — a stale + // entry would mark a key generation-owned that the + // restored document no longer carries, silently + // skipping later merges of that key. + self.app_state_owner = pre.app_state_owner; return false; } } diff --git a/crates/op-editor-core/src/history.rs b/crates/op-editor-core/src/history.rs index cbe2eb4fe..abe38eba6 100644 --- a/crates/op-editor-core/src/history.rs +++ b/crates/op-editor-core/src/history.rs @@ -40,6 +40,13 @@ pub struct EditorSnapshot { /// Runtime component registry mirrored from reusable document nodes /// and explicit component commands. pub components: ComponentLibrary, + /// `MergeAppState` ownership map (`key → owning plan_idx`) at + /// snapshot time. Must travel with the snapshot: doc.state is + /// restored on undo / batch rollback, so ownership left behind + /// would mark keys as generation-owned that the restored document + /// no longer carries — later merges would be silently skipped or + /// mis-resolved against a stale owner. + pub app_state_owner: std::collections::BTreeMap, } /// Editor undo / redo stacks. `VecDeque` so the over-cap eviction is an diff --git a/crates/op-editor-core/src/mutators.rs b/crates/op-editor-core/src/mutators.rs index 14840b344..4a9c6ae0d 100644 --- a/crates/op-editor-core/src/mutators.rs +++ b/crates/op-editor-core/src/mutators.rs @@ -264,6 +264,7 @@ impl EditorState { selection: self.selection.clone(), active_page_index: self.ui.active_page_index, components: self.components.clone(), + app_state_owner: self.app_state_owner.clone(), } } @@ -289,6 +290,7 @@ impl EditorState { self.selection = snap.selection; self.ui.active_page_index = snap.active_page_index; self.components = snap.components; + self.app_state_owner = snap.app_state_owner; } /// Undo the last change. False when the undo stack is empty. diff --git a/crates/op-editor-core/src/state.rs b/crates/op-editor-core/src/state.rs index 2187c1088..b3e5a3a98 100644 --- a/crates/op-editor-core/src/state.rs +++ b/crates/op-editor-core/src/state.rs @@ -407,6 +407,7 @@ mod tests { selection: SelectionState::empty(), active_page_index: 0, components: ComponentLibrary::default(), + app_state_owner: std::collections::BTreeMap::new(), }); s.ui.pen_in_progress = Some(crate::NodeId::new("n7")); s.ui.property_input.set_text("stale"); diff --git a/crates/op-host-native/src/widget_host.rs b/crates/op-host-native/src/widget_host.rs index d45b26d0a..c144b760b 100644 --- a/crates/op-host-native/src/widget_host.rs +++ b/crates/op-host-native/src/widget_host.rs @@ -807,14 +807,16 @@ impl WidgetHostNative { true } - /// Route a wheel into the preview runtime; `false` (not consumed — - /// no `onScroll` node under the cursor) lets the caller fall back - /// to canvas pan/zoom so the user can still navigate while - /// previewing. + /// Route a wheel / trackpad-pan scroll into the preview runtime; + /// `false` (not consumed — no `onScroll` node under the cursor) + /// lets the caller fall back to canvas pan/zoom so the user can + /// still navigate while previewing. Mouse wheels carry only + /// `delta_y`; two-finger trackpad pans carry both axes. pub fn preview_dispatch_wheel( &mut self, screen_x: f32, screen_y: f32, + delta_x: f32, delta_y: f32, viewport_w: f32, viewport_h: f32, @@ -825,7 +827,7 @@ impl WidgetHostNative { let consumed = self .preview .as_mut() - .is_some_and(|p| p.dispatch_wheel(doc.x, doc.y, 0.0, delta_y)); + .is_some_and(|p| p.dispatch_wheel(doc.x, doc.y, delta_x, delta_y)); if consumed { self.mark_dirty(); } diff --git a/crates/op-host-native/src/widget_host/scroll.rs b/crates/op-host-native/src/widget_host/scroll.rs index 2f7fcdf41..6284725db 100644 --- a/crates/op-host-native/src/widget_host/scroll.rs +++ b/crates/op-host-native/src/widget_host/scroll.rs @@ -545,7 +545,7 @@ impl WidgetHostNative { // still pan/zoom the canvas while previewing. Runs below the // panel/picker guards — they own the wheel over their rects. if self.preview.is_some() - && self.preview_dispatch_wheel(x, y, delta_y, viewport_width, viewport_height) + && self.preview_dispatch_wheel(x, y, 0.0, delta_y, viewport_width, viewport_height) { return true; } @@ -699,6 +699,17 @@ impl WidgetHostNative { if self.try_scroll_layer_panel(x, y, dx, dy, viewport_height) { return true; } + // Live preview: a two-finger trackpad pan over a node carrying + // `events.onScroll` goes to the runtime (same contract as the + // `apply_wheel` branch — the desktop runner routes PixelDelta + // scrolls here, never through `apply_wheel`); otherwise fall + // through so trackpad panning still navigates the canvas + // while previewing. + if self.preview.is_some() + && self.preview_dispatch_wheel(x, y, dx, dy, viewport_width, viewport_height) + { + return true; + } if !self.over_canvas(x, y, viewport_width, viewport_height) { return false; }