From 194d761ade6a321dac2d1e4b0ea6ede6650c08a4 Mon Sep 17 00:00:00 2001 From: Mike Solar Date: Wed, 19 Aug 2026 00:41:49 +0800 Subject: [PATCH] refactor(oakundo): replace the CHandle vtable layer with owned trait objects With the C ABI facade (oakengine) retired, the frozen-ABI rationale is gone. UndoCommand now boxes a Send Command trait (new/from_closures/ multi), dropping OakUndoCommandVtable, the userdata trampolines, the refcount shell, the handle module, and all undostack_* handle exports. The global facade loses its raw-pointer out-params (can_undo/can_redo return bool, command_name returns String). oaktimeline/oaknode/ oakplugin/oaktask construct commands directly via UndoCommand::new. oakundo src is now free of unsafe; behavior (ordering, idempotence, done flags, groups, observers, 200-row cap) is unchanged and pinned by the rewritten tests. --- crates/oaknode/src/ops.rs | 50 +- crates/oaknode/tests/phase2_units_test.rs | 37 +- crates/oakplugin/src/node.rs | 54 +- crates/oaktask/src/nodeops.rs | 65 +- crates/oaktimeline/src/lib.rs | 17 +- crates/oaktimeline/src/marker.rs | 12 +- crates/oaktimeline/src/multicam.rs | 6 +- crates/oaktimeline/src/undocommon.rs | 102 +-- crates/oaktimeline/src/undogeneral.rs | 20 +- crates/oaktimeline/src/undopointer.rs | 8 +- crates/oaktimeline/src/undoripple.rs | 20 +- crates/oaktimeline/src/undosplit.rs | 10 +- crates/oaktimeline/src/undotrack.rs | 8 +- crates/oaktimeline/src/workarea.rs | 4 +- crates/oaktimeline/tests/domain_test.rs | 2 +- crates/oaktimeline/tests/marker_test.rs | 2 +- crates/oaktimeline/tests/workarea_test.rs | 2 +- crates/oakundo/Cargo.toml | 3 +- crates/oakundo/README.md | 62 +- crates/oakundo/src/global.rs | 263 ++------ crates/oakundo/src/handle.rs | 160 ----- crates/oakundo/src/lib.rs | 26 +- crates/oakundo/src/undocommand.rs | 530 +++------------- crates/oakundo/src/undostack.rs | 222 +------ crates/oakundo/tests/edge_coverage_test.rs | 189 +----- crates/oakundo/tests/ffi_contract_test.rs | 681 --------------------- crates/oakundo/tests/handle_test.rs | 177 ------ crates/oakundo/tests/undo_safe_test.rs | 159 +++-- 28 files changed, 409 insertions(+), 2482 deletions(-) delete mode 100644 crates/oakundo/src/handle.rs delete mode 100644 crates/oakundo/tests/ffi_contract_test.rs delete mode 100644 crates/oakundo/tests/handle_test.rs diff --git a/crates/oaknode/src/ops.rs b/crates/oaknode/src/ops.rs index c9ae048d7..fbf25215c 100644 --- a/crates/oaknode/src/ops.rs +++ b/crates/oaknode/src/ops.rs @@ -17,10 +17,8 @@ //! Free functions replacing the C++ `Node` static methods //! (COVERAGE.md §6/§9). -use std::ffi::c_void; - use oakcore_rs::TimeRange; -use oakundo::undocommand::{OakUndoCommandVtable, UndoCommand}; +use oakundo::undocommand::UndoCommand; use crate::graph::Graph; use crate::id::NodeId; @@ -150,39 +148,6 @@ fn lock_any(m: &std::sync::Mutex) -> std::sync::MutexGuard<'_, T> { m.lock().unwrap_or_else(|e| e.into_inner()) } -/// Userdata payload behind a closure-backed undo command: the boxed -/// redo/undo closures. -struct ClosureCommand { - /// The redo closure. - redo: Box, - /// The undo closure. - undo: Box, -} - -/// `OakUndoCommandVtable` redo thunk for [`ClosureCommand`] userdata. -unsafe extern "C" fn closure_redo(ud: *mut c_void) { - // SAFETY: `ud` is the `ClosureCommand` box created by - // `command_from_closures` and still owned by the command. - let c = unsafe { &mut *(ud as *mut ClosureCommand) }; - (c.redo)(); -} - -/// `OakUndoCommandVtable` undo thunk for [`ClosureCommand`] userdata. -unsafe extern "C" fn closure_undo(ud: *mut c_void) { - // SAFETY: see `closure_redo`. - let c = unsafe { &mut *(ud as *mut ClosureCommand) }; - (c.undo)(); -} - -/// `OakUndoCommandVtable` free thunk for [`ClosureCommand`] userdata. -unsafe extern "C" fn closure_free(ud: *mut c_void) { - if !ud.is_null() { - // SAFETY: the box is destroyed exactly once, by the command that - // owns it. - unsafe { drop(Box::from_raw(ud as *mut ClosureCommand)) }; - } -} - /// Build an un-executed [`UndoCommand`] from redo/undo closures (the /// direct-Rust replacement of the former oakundo bridge /// `command_from_closures`). @@ -190,18 +155,7 @@ fn command_from_closures( redo: impl FnMut() + Send + 'static, undo: impl FnMut() + Send + 'static, ) -> UndoCommand { - let ud = Box::into_raw(Box::new(ClosureCommand { - redo: Box::new(redo), - undo: Box::new(undo), - })); - UndoCommand::from_vtable( - OakUndoCommandVtable { - redo: Some(closure_redo), - undo: Some(closure_undo), - free_fn: Some(closure_free), - }, - ud as *mut c_void, - ) + UndoCommand::from_closures(redo, undo) } /// Set a keyframed/standard value at a time, returning an un-executed diff --git a/crates/oaknode/tests/phase2_units_test.rs b/crates/oaknode/tests/phase2_units_test.rs index 7e9e98d0a..a81c9c67c 100644 --- a/crates/oaknode/tests/phase2_units_test.rs +++ b/crates/oaknode/tests/phase2_units_test.rs @@ -357,50 +357,19 @@ fn serializer_value_codecs() { ); } -/// oakundo `UndoCommand`: vtable commands + multi commands (direct Rust +/// oakundo `UndoCommand`: closure commands + multi commands (direct Rust /// calls, single-lib unification). #[test] fn undo_command_roundtrip() { - use std::ffi::c_void; use std::sync::atomic::{AtomicI32, Ordering}; use std::sync::Arc; - use oakundo::undocommand::{OakUndoCommandVtable, UndoCommand}; - - struct Closures { - redo: Box, - undo: Box, - } - - unsafe extern "C" fn redo_thunk(ud: *mut c_void) { - let c = unsafe { &mut *(ud as *mut Closures) }; - (c.redo)(); - } - unsafe extern "C" fn undo_thunk(ud: *mut c_void) { - let c = unsafe { &mut *(ud as *mut Closures) }; - (c.undo)(); - } - unsafe extern "C" fn free_thunk(ud: *mut c_void) { - if !ud.is_null() { - unsafe { drop(Box::from_raw(ud as *mut Closures)) }; - } - } + use oakundo::undocommand::UndoCommand; fn from_closures( redo: impl FnMut() + Send + 'static, undo: impl FnMut() + Send + 'static, ) -> UndoCommand { - let ud = Box::into_raw(Box::new(Closures { - redo: Box::new(redo), - undo: Box::new(undo), - })); - UndoCommand::from_vtable( - OakUndoCommandVtable { - redo: Some(redo_thunk), - undo: Some(undo_thunk), - free_fn: Some(free_thunk), - }, - ud as *mut c_void, - ) + UndoCommand::from_closures(redo, undo) } let value = Arc::new(AtomicI32::new(0)); diff --git a/crates/oakplugin/src/node.rs b/crates/oakplugin/src/node.rs index 0be726431..d1578c330 100644 --- a/crates/oakplugin/src/node.rs +++ b/crates/oakplugin/src/node.rs @@ -43,7 +43,7 @@ //! ## undoable 回写 //! //! [`set_input_undoable`]/[`set_input_string_undoable`] 按 -//! `oaknode::ops::set_value_at_time_command` 的同一 closure-vtable +//! `oaknode::ops::set_value_at_time_command` 的同一 closure //! 模式（`command_from_closures`）构造未执行的 //! `oakundo::undocommand::UndoCommand`：redo 闭包锁 project、 //! `graph.get_mut(id)`、`NodeCore::set_standard_value` 写标准值； @@ -54,7 +54,7 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex, OnceLock, Weak}; -use oakundo::undocommand::{OakUndoCommandVtable, UndoCommand}; +use oakundo::undocommand::UndoCommand; /// oaknode 节点引用（single-lib）：旧 C ABI 句柄盒 /// `(Arc>, NodeId)` 的值型复刻。 @@ -331,57 +331,13 @@ fn lock_any(m: &Mutex) -> std::sync::MutexGuard<'_, T> { m.lock().unwrap_or_else(|e| e.into_inner()) } -/// Userdata payload behind a closure-backed undo command: the boxed -/// redo/undo closures（与 `oaknode::ops.rs` 的 `ClosureCommand` 同构； -/// 该类型的构造器是 oaknode 私有，桥内自备一份）。 -struct ClosureCommand { - /// The redo closure. - redo: Box, - /// The undo closure. - undo: Box, -} - -/// `OakUndoCommandVtable` redo thunk for [`ClosureCommand`] userdata. -unsafe extern "C" fn closure_redo(ud: *mut std::ffi::c_void) { - // SAFETY: `ud` 是 `command_from_closures` 创建的 ClosureCommand - // 盒，命令持有其所有权直至 free_fn。 - let c = unsafe { &mut *(ud as *mut ClosureCommand) }; - (c.redo)(); -} - -/// `OakUndoCommandVtable` undo thunk for [`ClosureCommand`] userdata. -unsafe extern "C" fn closure_undo(ud: *mut std::ffi::c_void) { - // SAFETY: 见 closure_redo。 - let c = unsafe { &mut *(ud as *mut ClosureCommand) }; - (c.undo)(); -} - -/// `OakUndoCommandVtable` free thunk for [`ClosureCommand`] userdata. -unsafe extern "C" fn closure_free(ud: *mut std::ffi::c_void) { - if !ud.is_null() { - // SAFETY: 盒恰被命令析构一次。 - unsafe { drop(Box::from_raw(ud as *mut ClosureCommand)) }; - } -} - -/// 从 redo/undo 闭包构造未执行的 [`UndoCommand`]（closure-vtable -/// 模式，与 `oaknode::ops::command_from_closures` 同构）。 +/// 从 redo/undo 闭包构造未执行的 [`UndoCommand`]（closure 模式，与 +/// `oaknode::ops::command_from_closures` 同构）。 fn command_from_closures( redo: impl FnMut() + Send + 'static, undo: impl FnMut() + Send + 'static, ) -> UndoCommand { - let ud = Box::into_raw(Box::new(ClosureCommand { - redo: Box::new(redo), - undo: Box::new(undo), - })); - UndoCommand::from_vtable( - OakUndoCommandVtable { - redo: Some(closure_redo), - undo: Some(closure_undo), - free_fn: Some(closure_free), - }, - ud as *mut std::ffi::c_void, - ) + UndoCommand::from_closures(redo, undo) } /// 以 undoable 方式设置节点的标准输入值（POD 路径；数值族输入）。 diff --git a/crates/oaktask/src/nodeops.rs b/crates/oaktask/src/nodeops.rs index 4b33fd227..abd5c3e5d 100644 --- a/crates/oaktask/src/nodeops.rs +++ b/crates/oaktask/src/nodeops.rs @@ -24,8 +24,8 @@ //! an `oaknode::id::NodeId` in that project's `oaknode::graph::Graph`. //! //! Undo commands follow the oaktimeline `undocommon` pattern: a command -//! struct is boxed into an `oakundo::undocommand::UndoCommand` vtable -//! command ([`box_command`]); `redo_now`/`undo_now` dispatch to it. +//! struct is boxed into an `oakundo::undocommand::UndoCommand` value +//! ([`box_command`]); `redo_now`/`undo_now` dispatch to it. use std::sync::{Arc, Mutex, MutexGuard}; @@ -40,7 +40,7 @@ use oaknode::node::{NodeBehavior, NodeCore}; use oaknode::project::Project; use oaknode::sequence::SequenceBehavior; use oaknode::track::{BlockRange, TrackBehavior, TrackListBehavior, TrackRange, TrackType}; -use oakundo::undocommand::{OakUndoCommandVtable, UndoCommand}; +use oakundo::undocommand::UndoCommand; /// A project, shared like the C++ `olive::Project` smart pointer. pub type ProjectRef = Arc>; @@ -905,62 +905,13 @@ pub fn pixel_format_from_code(code: i32) -> oakcore_rs::PixelFormat { // Undo commands // --------------------------------------------------------------------------- -/// Command trait for the task-local undo commands (mirrors the -/// oaktimeline `undocommon::Command` pattern). -trait TaskCommand { - /// Apply the change. - fn redo(&mut self); - /// Revert the change. - fn undo(&mut self); -} +/// Command trait for the task-local undo commands (the oakundo command +/// trait; mirrors the oaktimeline `undocommon::Command` pattern). +pub use oakundo::undocommand::Command as TaskCommand; -/// Generic `redo` callback forwarding to [`TaskCommand::redo`]. -/// -/// # Safety -/// `userdata` must be the `Box` produced by [`box_command`]. -unsafe extern "C" fn redo_cb(userdata: *mut std::ffi::c_void) { - if userdata.is_null() { - return; - } - // SAFETY: box_command allocated a `Box`; the command still owns it. - (unsafe { &mut *(userdata as *mut T) }).redo(); -} - -/// Generic `undo` callback forwarding to [`TaskCommand::undo`]. -/// -/// # Safety -/// As `redo_cb`. -unsafe extern "C" fn undo_cb(userdata: *mut std::ffi::c_void) { - if userdata.is_null() { - return; - } - // SAFETY: as `redo_cb`. - (unsafe { &mut *(userdata as *mut T) }).undo(); -} - -/// Generic `free` callback dropping the boxed command. -/// -/// # Safety -/// `userdata` must be the `Box` produced by [`box_command`]. -unsafe extern "C" fn free_cb(userdata: *mut std::ffi::c_void) { - if userdata.is_null() { - return; - } - // SAFETY: the command owns this box; dropping it frees the command. - unsafe { - drop(Box::from_raw(userdata as *mut T)); - } -} - -/// Box a task command into an oakundo vtable command value. +/// Box a task command into an oakundo command value. fn box_command(cmd: T) -> UndoCommand { - let userdata = Box::into_raw(Box::new(cmd)) as *mut std::ffi::c_void; - let vtable = OakUndoCommandVtable { - redo: Some(redo_cb::), - undo: Some(undo_cb::), - free_fn: Some(free_cb::), - }; - UndoCommand::from_vtable(vtable, userdata) + UndoCommand::new(cmd) } /// `FolderAddChildCommand` — add an item to a bin folder diff --git a/crates/oaktimeline/src/lib.rs b/crates/oaktimeline/src/lib.rs index 37f38e940..bb95cf4da 100644 --- a/crates/oaktimeline/src/lib.rs +++ b/crates/oaktimeline/src/lib.rs @@ -16,19 +16,20 @@ //! # oaktimeline — the timeline edit module (Rust) //! -//! Reimplements the C++ oaktimeline module (`src/timeline/src`) behind -//! its frozen C ABI (`include/timeline/*.h`). See README.md for the -//! architectural mapping (per-header domain modules, no UndoCommand -//! inheritance → vtable commands). +//! Reimplements the C++ oaktimeline module (`src/timeline/src`). See +//! README.md for the architectural mapping (per-header domain modules, no +//! UndoCommand inheritance → boxed `oakundo` command values). //! //! ## Single-lib unification //! //! The C ABI export layer (`ffi.rs`) and the oaknode/oakundo/oakcommon //! bridge (`bridge/`) were deleted in the single-lib unification: undo -//! commands are now `oakundo::undocommand::UndoCommand` values and every -//! node/block/track reference is a [`util::NodeRef`] — an -//! `Arc>` + `oaknode::id::NodeId` pair -//! that the commands manipulate through the oaknode Rust domain directly. +//! commands are now `oakundo::undocommand::UndoCommand` values (the +//! crate's command structs implement `undocommon::Command` and are boxed +//! as trait objects) and every node/block/track reference is a +//! [`util::NodeRef`] — an `Arc>` + +//! `oaknode::id::NodeId` pair that the commands manipulate through the +//! oaknode Rust domain directly. #![deny(unsafe_op_in_unsafe_fn)] #![warn(missing_docs)] diff --git a/crates/oaktimeline/src/marker.rs b/crates/oaktimeline/src/marker.rs index 06eb017fa..c3b748d98 100644 --- a/crates/oaktimeline/src/marker.rs +++ b/crates/oaktimeline/src/marker.rs @@ -244,7 +244,7 @@ fn rational_abs(r: Rational) -> Rational { // --------------------------------------------------------------------------- // Marker undo commands. Each struct exposes prepare()/redo()/undo(); the -// undo stack wraps it through undocommon's vtable (to_command()). +// undo stack wraps it through undocommon's box_command (to_command()). // --------------------------------------------------------------------------- /// `MarkerAddCommand` (timelinemarker.h). @@ -333,7 +333,7 @@ impl MarkerAddCommand { } } - /// Wrap as an oakundo vtable command handle. + /// Wrap as an oakundo command handle. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -407,7 +407,7 @@ impl MarkerRemoveCommand { } } - /// Wrap as an oakundo vtable command handle. + /// Wrap as an oakundo command handle. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -500,7 +500,7 @@ impl MarkerChangeColorCommand { } } - /// Wrap as an oakundo vtable command handle. + /// Wrap as an oakundo command handle. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -589,7 +589,7 @@ impl MarkerChangeNameCommand { } } - /// Wrap as an oakundo vtable command handle. + /// Wrap as an oakundo command handle. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -685,7 +685,7 @@ impl MarkerChangeTimeCommand { } } - /// Wrap as an oakundo vtable command handle. + /// Wrap as an oakundo command handle. pub fn to_command(self) -> UndoCommand { box_command(self) } diff --git a/crates/oaktimeline/src/multicam.rs b/crates/oaktimeline/src/multicam.rs index 20409ef51..5d15bf8c4 100644 --- a/crates/oaktimeline/src/multicam.rs +++ b/crates/oaktimeline/src/multicam.rs @@ -399,7 +399,7 @@ impl MultiCamEnableCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { crate::undocommon::box_command(self) } @@ -550,7 +550,7 @@ impl MultiCamDisableCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { crate::undocommon::box_command(self) } @@ -685,7 +685,7 @@ impl MultiCamSwitchCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { crate::undocommon::box_command(self) } diff --git a/crates/oaktimeline/src/undocommon.rs b/crates/oaktimeline/src/undocommon.rs index d23e37415..36d1160af 100644 --- a/crates/oaktimeline/src/undocommon.rs +++ b/crates/oaktimeline/src/undocommon.rs @@ -23,28 +23,14 @@ //! //! `CHandleCommandWrapper` in C++ subclasses `olive::UndoCommand` to wrap a //! raw `OakUndoCommand`; the Rust equivalent is the crate's own commands -//! boxed through [`box_command`], so the wrapper is gone. - -use std::ffi::c_void; +//! boxed as trait objects through [`box_command`] (they implement +//! [`Command`] = `oakundo::undocommand::Command`), so the wrapper is gone. use oaknode::graph::NodeEntry; -use oakundo::undocommand::{OakUndoCommandVtable, UndoCommand}; +use oakundo::undocommand::UndoCommand; use crate::util::{block_add_to_graph, block_remove_from_graph, NodeRef}; -/// An empty (all callbacks absent) command — the failure-path result of -/// the deleted oaknode remove-command factory (an empty command handle). -pub(crate) fn empty_command() -> UndoCommand { - UndoCommand::from_vtable( - OakUndoCommandVtable { - redo: None, - undo: None, - free_fn: None, - }, - std::ptr::null_mut(), - ) -} - /// `oaknode_command_create_remove_node` — a command that detaches a node /// from the project graph on `redo` (the detached entry is owned by this /// command) and re-inserts it, identity-preserving, on `undo`. The C++ @@ -114,83 +100,23 @@ pub fn create_and_run_block_remove_command(block: &NodeRef) -> UndoCommand { create_and_run_remove_command(block) } -/// `free_command_handle`: release and null a command; NULL no-op. With the -/// `UndoCommand` value type, "freeing" is overwriting the pointee with an -/// empty command (the old value drops, running its `free_fn`). -pub fn free_command_handle(command: *mut UndoCommand) { - if command.is_null() { - return; - } - // SAFETY: the caller passes a valid pointer; overwriting drops the old - // value and installs the no-op command. - unsafe { *command = empty_command() }; -} +/// Trait implemented by every timeline undo command (re-export of the +/// oakundo command trait). The crate's commands are boxed into an +/// [`UndoCommand`] via [`box_command`]; the command's +/// `redo_now`/`undo_now` dispatch to these callbacks. +pub use oakundo::undocommand::Command; -/// Trait implemented by every timeline undo command. The crate's commands -/// are boxed into an oakundo vtable command via [`box_command`]; the -/// command's `redo_now`/`undo_now` dispatch to these callbacks. -pub trait Command { - /// Apply the change. - fn redo(&mut self); - /// Revert the change. - fn undo(&mut self); -} - -/// Generic `redo` callback forwarding to [`Command::redo`]. -/// -/// # Safety -/// `userdata` must be the `Box` produced by [`box_command`]. -unsafe extern "C" fn redo_cb(userdata: *mut c_void) { - if userdata.is_null() { - return; - } - // SAFETY: box_command allocated a `Box`; still alive because the - // command owns it. - (unsafe { &mut *(userdata as *mut T) }).redo(); -} - -/// Generic `undo` callback forwarding to [`Command::undo`]. -/// -/// # Safety -/// `userdata` must be the `Box` produced by [`box_command`]. -unsafe extern "C" fn undo_cb(userdata: *mut c_void) { - if userdata.is_null() { - return; - } - // SAFETY: as `redo_cb`. - (unsafe { &mut *(userdata as *mut T) }).undo(); -} - -/// Generic `free` callback dropping the boxed command. -/// -/// # Safety -/// `userdata` must be the `Box` produced by [`box_command`]. -unsafe extern "C" fn free_cb(userdata: *mut c_void) { - if userdata.is_null() { - return; - } - // SAFETY: the command owns this box; dropping it frees the command. - unsafe { drop(Box::from_raw(userdata as *mut T)) }; -} - -/// Box a command into an oakundo vtable command value. The command owns -/// the boxed `T`; `redo_now`/`undo_now` dispatch to `T::redo`/`T::undo`, -/// and dropping the command drops `T`. +/// Box a command into an oakundo command value. The command owns the +/// boxed `T`; `redo_now`/`undo_now` dispatch to `T::redo`/`T::undo`, and +/// dropping the command drops `T`. pub(crate) fn box_command(cmd: T) -> UndoCommand { - let userdata = Box::into_raw(Box::new(cmd)) as *mut c_void; - let vtable = OakUndoCommandVtable { - redo: Some(redo_cb::), - undo: Some(undo_cb::), - free_fn: Some(free_cb::), - }; - // `from_vtable` copies the vtable and takes ownership of `userdata`. - UndoCommand::from_vtable(vtable, userdata) + UndoCommand::new(cmd) } /// `MultiUndoCommand` — a command that runs several child commands in order /// on `redo` and in reverse on `undo` (timelineundocommon.h /// `MultiUndoCommand`). Children are boxed `Command`s; the whole group wraps -/// into a single oakundo vtable command via [`box_command`]. +/// into a single oakundo command via [`box_command`]. pub struct MultiUndoCommand { /// Child commands, run in order on `redo`. commands: Vec>, @@ -214,7 +140,7 @@ impl MultiUndoCommand { self.commands.is_empty() } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } diff --git a/crates/oaktimeline/src/undogeneral.rs b/crates/oaktimeline/src/undogeneral.rs index 2e6369b06..eaa6b3d8f 100644 --- a/crates/oaktimeline/src/undogeneral.rs +++ b/crates/oaktimeline/src/undogeneral.rs @@ -81,7 +81,7 @@ impl BlockResizeCommand { block_set_length_and_media_out(&self.block, self.old_length); } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -134,7 +134,7 @@ impl BlockResizeWithMediaInCommand { block_set_length_and_media_in(&self.block, self.old_length); } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -186,7 +186,7 @@ impl BlockSetMediaInCommand { clip_set_media_in(&self.block, self.old_media_in); } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -325,7 +325,7 @@ impl TimelineAddTrackCommand { c.track() } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -517,7 +517,7 @@ impl TimelineRemoveTrackCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -608,7 +608,7 @@ impl TransitionRemoveCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -833,7 +833,7 @@ impl TrackReplaceBlockWithGapCommand { .push(TransitionRemoveCommand::new(relevant, true)); } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -885,7 +885,7 @@ impl BlockEnableDisableCommand { block_set_enabled(&self.block, self.old_enabled); } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -1073,7 +1073,7 @@ impl TrackListInsertGaps { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -1195,7 +1195,7 @@ impl TimelineAddDefaultTransitionCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } diff --git a/crates/oaktimeline/src/undopointer.rs b/crates/oaktimeline/src/undopointer.rs index efc0ff45c..01025b0a7 100644 --- a/crates/oaktimeline/src/undopointer.rs +++ b/crates/oaktimeline/src/undopointer.rs @@ -277,7 +277,7 @@ impl BlockTrimCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -517,7 +517,7 @@ impl TrackSlideCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -672,7 +672,7 @@ impl TrackPlaceBlockCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -740,7 +740,7 @@ impl TrackMoveBlockCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } diff --git a/crates/oaktimeline/src/undoripple.rs b/crates/oaktimeline/src/undoripple.rs index a244df3c9..28717959c 100644 --- a/crates/oaktimeline/src/undoripple.rs +++ b/crates/oaktimeline/src/undoripple.rs @@ -129,7 +129,7 @@ pub struct TrackRippleRemoveAreaCommand { track: NodeRef, /// Area to clear. range: TimeRange, - /// Whether `prepare` has run (the vtable command path never calls + /// Whether `prepare` has run (the oakundo command path never calls /// `prepare()` itself, so `redo` derives the operations on first use). prepared: bool, /// Out-point trim on the first block (`timelineundoripple.h` `trim_out_`). @@ -215,7 +215,7 @@ impl TrackRippleRemoveAreaCommand { /// /// Mirrors the C++ algorithm (`oaknode_track_get_nearest_block_before_or_at` /// is a direct oaknode-domain query; `redo` invokes this on first use - /// because the vtable command path never calls `prepare` itself). + /// because the oakundo command path never calls `prepare` itself). pub fn prepare(&mut self) { // Idempotent: recompute from the current track state, discarding any // previously derived operations. @@ -324,7 +324,7 @@ impl TrackRippleRemoveAreaCommand { /// `redo`: apply the ripple removal. pub fn redo(&mut self) { - // The vtable command path never invokes `prepare()` (the oakundo + // The oakundo command path never invokes `prepare()` (the oakundo // wrapper only dispatches redo/undo), so derive the operations // on first use. if !self.prepared { @@ -408,7 +408,7 @@ impl TrackRippleRemoveAreaCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -489,7 +489,7 @@ impl TrackListRippleRemoveAreaCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -555,7 +555,7 @@ impl TimelineRippleRemoveAreaCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -763,7 +763,7 @@ impl TrackListRippleToolCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -854,7 +854,7 @@ impl TimelineRippleDeleteGapsAtRegionsCommand { /// /// The gap-kind, nearest-block, sequence-track and locked-flag queries /// all go through the oaknode domain via `crate::util`; `redo` invokes - /// this on first use because the oakundo vtable wrapper only dispatches + /// this on first use because the oakundo command wrapper only dispatches /// `redo`/`undo`. pub fn prepare(&mut self) { self.commands_.clear(); @@ -1025,7 +1025,7 @@ impl TimelineRippleDeleteGapsAtRegionsCommand { /// `redo`: apply the gap deletions. /// - /// The vtable command path never invokes `prepare()` (the oakundo vtable + /// The oakundo command path never invokes `prepare()` (the oakundo /// wrapper only dispatches `redo`/`undo`), so the sub-commands are built /// on first use; later redos re-apply the stored commands. pub fn redo(&mut self) { @@ -1044,7 +1044,7 @@ impl TimelineRippleDeleteGapsAtRegionsCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } diff --git a/crates/oaktimeline/src/undosplit.rs b/crates/oaktimeline/src/undosplit.rs index 32384938c..1b9174b62 100644 --- a/crates/oaktimeline/src/undosplit.rs +++ b/crates/oaktimeline/src/undosplit.rs @@ -183,7 +183,7 @@ impl BlockSplitCommand { /// original span, so the out point is preserved exactly). pub fn redo(&mut self) { // Create the second half if redo is invoked without a preceding - // prepare() (the vtable command path may call redo directly). + // prepare() (the oakundo command path may call redo directly). self.prepare(); // Re-attach the copied subgraph if a previous undo detached it. self.re_attach_subgraph(); @@ -250,7 +250,7 @@ impl BlockSplitCommand { self.new_block.clone() } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { crate::undocommon::box_command(self) } @@ -343,7 +343,7 @@ impl BlockSplitPreservingLinksCommand { /// `redo`: redo every child command in order. /// - /// The vtable command path never invokes `prepare()` (the oakundo + /// The oakundo command path never invokes `prepare()` (the oakundo /// wrapper only dispatches `redo`/`undo`), so the children are built /// on first redo; `prepare` itself redoes each child as it builds it, /// so the first redo has nothing left to run. Later redos (after an @@ -385,7 +385,7 @@ impl BlockSplitPreservingLinksCommand { None } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { crate::undocommon::box_command(self) } @@ -466,7 +466,7 @@ impl TrackSplitAtTimeCommand { } } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { crate::undocommon::box_command(self) } diff --git a/crates/oaktimeline/src/undotrack.rs b/crates/oaktimeline/src/undotrack.rs index b656df7d6..83e810585 100644 --- a/crates/oaktimeline/src/undotrack.rs +++ b/crates/oaktimeline/src/undotrack.rs @@ -62,7 +62,7 @@ impl TrackRippleRemoveBlockCommand { track_insert_block_after(&self.track, &self.block, self.before.as_ref()); } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -107,7 +107,7 @@ impl TrackPrependBlockCommand { track_ripple_remove_block(&self.track, &self.block); } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -159,7 +159,7 @@ impl TrackInsertBlockAfterCommand { track_ripple_remove_block(&self.track, &self.block); } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } @@ -210,7 +210,7 @@ impl TrackReplaceBlockCommand { track_replace_block(&self.track, &self.replace, &self.old); } - /// Wrap as an oakundo vtable command value. + /// Wrap as an oakundo command value. pub fn to_command(self) -> UndoCommand { box_command(self) } diff --git a/crates/oaktimeline/src/workarea.rs b/crates/oaktimeline/src/workarea.rs index cbe5e5bd7..a535936d3 100644 --- a/crates/oaktimeline/src/workarea.rs +++ b/crates/oaktimeline/src/workarea.rs @@ -150,7 +150,7 @@ impl WorkareaSetEnabledCommand { } } - /// Wrap as an oakundo vtable command handle. + /// Wrap as an oakundo command handle. pub fn to_command(self) -> UndoCommand { crate::undocommon::box_command(self) } @@ -256,7 +256,7 @@ impl WorkareaSetRangeCommand { } } - /// Wrap as an oakundo vtable command handle. + /// Wrap as an oakundo command handle. pub fn to_command(self) -> UndoCommand { crate::undocommon::box_command(self) } diff --git a/crates/oaktimeline/tests/domain_test.rs b/crates/oaktimeline/tests/domain_test.rs index 18a249438..98bea6219 100644 --- a/crates/oaktimeline/tests/domain_test.rs +++ b/crates/oaktimeline/tests/domain_test.rs @@ -388,7 +388,7 @@ fn move_block_round_trip() { assert_eq!(span_of(&tail), Some((Rational::new(50, 1), Rational::new(100, 1)))); } -/// The `Command` trait dispatch (used by the oakundo vtable wrappers) +/// The `Command` trait dispatch (used by the oakundo command values) /// routes through the same redo/undo bodies as the inherent methods. #[test] fn commands_trait_dispatch() { diff --git a/crates/oaktimeline/tests/marker_test.rs b/crates/oaktimeline/tests/marker_test.rs index 847473f6d..cd585f90d 100644 --- a/crates/oaktimeline/tests/marker_test.rs +++ b/crates/oaktimeline/tests/marker_test.rs @@ -266,7 +266,7 @@ fn marker_remove_command_double_redo() { } /// Every marker command dispatches through the `Command` trait, which is -/// how the undo stack's vtable invokes them. +/// how the undo stack invokes them. #[test] fn marker_commands_trait_dispatch() { let list_h = make_owned(TimelineMarkerList::new()); diff --git a/crates/oaktimeline/tests/workarea_test.rs b/crates/oaktimeline/tests/workarea_test.rs index dda703e42..73b89dafb 100644 --- a/crates/oaktimeline/tests/workarea_test.rs +++ b/crates/oaktimeline/tests/workarea_test.rs @@ -153,7 +153,7 @@ fn workarea_commands_box_to_undo_command() { } /// `Command` trait dispatch routes through the same redo/undo bodies as -/// the inherent methods (used by the undo stack vtable). +/// the inherent methods (used by the undo stack command values). #[test] fn workarea_commands_trait_dispatch() { let wa_h = make_owned(TimelineWorkArea::new()); diff --git a/crates/oakundo/Cargo.toml b/crates/oakundo/Cargo.toml index 9ab4f8084..9f4e86867 100644 --- a/crates/oakundo/Cargo.toml +++ b/crates/oakundo/Cargo.toml @@ -6,8 +6,7 @@ description = "Oak Video Editor undo/redo history module (Rust)" license = "GPL-3.0-or-later" [lib] -crate-type = ["staticlib", "rlib"] +crate-type = ["rlib"] [dependencies] -oakcore-rs = { path = "../oakcore" } thiserror = "2" diff --git a/crates/oakundo/README.md b/crates/oakundo/README.md index 62fc59c42..726a6d62f 100644 --- a/crates/oakundo/README.md +++ b/crates/oakundo/README.md @@ -1,33 +1,36 @@ # oakundo Rust crate > Status: **implemented**. Ports the C++ oakundo module -> (`src/undo/src`) to Rust behind its frozen C ABI -> (`include/undo/*.h`). Template follows `crates/oakplugin`. +> (`src/undo/src`) to Rust. Template follows `crates/oakplugin`. ## Scope Replaces the C++ oakundo module (`src/undo/src`): undoable commands -and the undo/redo history stack. Public contract: `include/undo/*.h` -(3 headers: `error.h`, `undocommand.h`, `undostack.h`) — frozen, -implemented verbatim by `src/ffi.rs`. +and the undo/redo history stack. The frozen C ABI (`include/undo/*.h`) +and the engine facade that consumed it are gone (see the root +`Cargo.toml` note on `crates/oakengine.bk`): every consumer links the +crate as a plain rlib and uses the value-typed API below. ## Architectural decisions -1. **Vtable-command pattern is the centerpiece.** In C++ other modules - *subclass* `olive::UndoCommand` (`redo()`/`undo()` overrides) and - plug themselves in polymorphically. Rust has no inheritance, so the - C ABI already models exactly this with - `OakUndoCommandVtable { redo, undo, free_fn }` plus a caller-owned - `userdata` pointer. The safe layer's [`undocommand::CommandKind`] - is the direct analog: either a caller-defined vtable command - (function pointers + userdata) or a [`undocommand::MultiUndoCommand`] - composite. Domain logic dispatches on the vtable the same way the - C++ virtual dispatch does. -2. **Modified-state callbacks are intentionally not part of the C ABI.** - The C++ `UndoCommand::redo_and_set_modified` pair records/restores a - project dirty flag via `std::function` accessors. The public headers - expose none of this; the stack drives state via `done_` on the safe - type instead, and the flag callbacks are left as a documented future +1. **Trait-object commands replace the vtable pattern.** In C++ other + modules *subclass* `olive::UndoCommand` (`redo()`/`undo()` overrides) + and plug themselves in polymorphically. Rust models the same + polymorphism with a boxed [`undocommand::Command`] trait object: + one-off edits arrive as closure commands + ([`undocommand::UndoCommand::from_closures`]) and whole-struct + commands implement the trait and are boxed with + [`undocommand::UndoCommand::new`]; composites are + [`undocommand::MultiUndoCommand`]. The former + `OakUndoCommandVtable` callback table, its `extern "C"` trampolines + and the refcounted `CHandle` layer were deleted with the C ABI — + domain logic dispatches through the trait the same way C++ virtual + dispatch does. +2. **Modified-state callbacks are intentionally omitted.** The C++ + `UndoCommand::redo_and_set_modified` pair records/restores a project + dirty flag via `std::function` accessors. The public headers exposed + none of this; the stack drives state via `done_` on the safe type + instead, and the flag callbacks are left as a documented future extension. 3. **`UndoStack` state machine** is modeled directly on the C++: two deques — `commands_` (done, oldest at front) and @@ -36,25 +39,26 @@ implemented verbatim by `src/ffi.rs`. (200) is exceeded; `jump` clamps and walks via `undo`/`redo`. The fresh stack holds a single "New/Open Project" empty command so `can_undo` is false at the bottom (per `undostack.cpp`). -4. **No merge semantics.** `include/undo/*.h` and `src/undo/src/*` - define no `merge_with`/`can_merge`; commands are never coalesced. - Tests reflect this (no merge tests). +4. **No merge semantics.** `src/undo/src/*` defines no + `merge_with`/`can_merge`; commands are never coalesced. Tests + reflect this (no merge tests). ## Layout ``` src/ lib.rs crate doc + module map - error.rs error codes (include/undo/error.h) - handle.rs refcounted-handle scaffolding (OAKUNDO_ABI_VERSION=1) - undocommand.rs UndoCommand / vtable command / MultiUndoCommand + error.rs error codes (mirrors include/undo/error.h values) + undocommand.rs UndoCommand / Command trait / MultiUndoCommand undostack.rs UndoStack + empty bottom command - ffi.rs export layer (one submodule per public header) + global.rs process-wide stack, groups, observers tests/ contract tests per module ``` -`error.h` exports macros only and is folded into `ffi.rs`'s preamble -(no own submodule), matching the codec crate convention. +The module has no `unsafe` code and no `extern "C"` surface; panics in +command callbacks propagate as normal process-internal panics (the +process-wide stack recovers a poisoned mutex the same way the former +`guard*` FFI wrappers did). ## Dependency policy diff --git a/crates/oakundo/src/global.rs b/crates/oakundo/src/global.rs index ec9c38356..89c9ef66b 100644 --- a/crates/oakundo/src/global.rs +++ b/crates/oakundo/src/global.rs @@ -15,31 +15,27 @@ // along with this program. If not, see . //! The process-wide undo stack, undo groups and the command-success -//! observer hook (M14 R1: sunk from the engine facade's `undo.rs`). +//! observer hook. //! -//! The facade used to own the process-wide stack (the module-00 analogue -//! of `EngineCore::undo_stack()`), the open undo group and the -//! write-through notification as facade state; all of it is process state, -//! so this module holds it and the facade forwards. The observer registry +//! The retired engine facade used to own the process-wide stack (the +//! module-00 analogue of `EngineCore::undo_stack()`), the open undo group +//! and the write-through notification as facade state; all of it is +//! process state, so this module holds it directly. The observer registry //! lets downstream modules (the oakstorage write-through session manager) -//! subscribe to "a command was recorded" notifications without a facade -//! round-trip. +//! subscribe to "a command was recorded" notifications. //! -//! M14 R5: everything in this module is Rust-typed — the process-wide -//! stack is a plain `static Mutex` and the open group holds an -//! [`UndoCommand`] value. The only remaining `CHandle` is the -//! [`push_or_run`] facade entry, which converts the incoming module -//! command handle into an [`UndoCommand`] value at the boundary. +//! Everything here is Rust-typed: the process-wide stack is a plain +//! `static Mutex` and the open group holds an +//! [`UndoCommand`] value. The former `CHandle`-marshalling entry point +//! (`push_or_run`) and the raw-pointer out-parameter queries +//! (`can_undo(out)` / `command_text(buf, size)` / `command_is_done(out)`) +//! were deleted with the C ABI — the same operations are exposed as +//! value-typed functions below. -use std::ffi::{c_char, c_int, c_void}; -use std::panic::{catch_unwind, AssertUnwindSafe}; use std::sync::{Mutex, OnceLock}; -use crate::error::{Error, OAKUNDO_E_FAILED, Result}; -use crate::handle::CHandle; -use crate::undocommand::{ - command_free, command_from_owned, command_redo_now, command_take, UndoCommand, -}; +use crate::error::Result; +use crate::undocommand::UndoCommand; use crate::undostack::UndoStack; /// The process-wide undo stack, created lazily on first use and kept for @@ -49,13 +45,6 @@ fn global_stack() -> &'static Mutex { STACK.get_or_init(|| Mutex::new(UndoStack::new())) } -/// Stable opaque token for the engine's `oakengine_undo_handle` export: -/// the stack's address (never dereferenced by callers; lives for the -/// process). -pub fn stack_token() -> *mut c_void { - global_stack() as *const Mutex as *mut c_void -} - /// Run `f` on the process-wide stack; a poisoned mutex is recovered (its /// inner value is still valid). fn with_stack(f: impl FnOnce(&mut UndoStack) -> Result) -> Result { @@ -121,7 +110,7 @@ fn group_lock() -> std::sync::MutexGuard<'static, Option> { pub fn group_begin(name: &str) -> Result<()> { let mut g = group_lock(); if g.is_some() { - return Err(Error::State); + return Err(crate::error::Error::State); } *g = Some(OpenGroup { multi: UndoCommand::multi(), @@ -135,7 +124,7 @@ pub fn group_begin(name: &str) -> Result<()> { /// when no group is open. pub fn group_end() -> Result<()> { let mut g = group_lock(); - let open = g.take().ok_or(Error::State)?; + let open = g.take().ok_or(crate::error::Error::State)?; let multi = open.multi; let name = open.name; drop(g); @@ -155,7 +144,7 @@ pub fn group_end() -> Result<()> { /// no group is open. No observers fire (nothing was recorded). pub fn group_abort() -> Result<()> { let mut g = group_lock(); - let open = g.take().ok_or(Error::State)?; + let open = g.take().ok_or(crate::error::Error::State)?; let mut multi = open.multi; drop(g); // The multi command itself is never marked done (each child was @@ -170,79 +159,29 @@ pub fn group_abort() -> Result<()> { Ok(()) } -/// Push `command` onto the stack and execute its redo (or add it to the -/// open group). `command` is consumed: the stack/multi takes the command -/// value, leaving the caller's handle as a non-owning shell that still -/// needs its own release. Returns 0 on success, a module error code -/// otherwise. Command observers fire only when the command was recorded on -/// the STACK — a group child joins the group, and the group itself -/// notifies at [`group_end`]. -pub fn push_or_run(command: CHandle, name: &str) -> c_int { +/// Push `command` onto the process-wide stack (redo then record), or into +/// the open group as an already-done child. On a stack record the command +/// observers fire (the oakstorage write-through persists the edit). +pub fn push(command: UndoCommand, name: &str) -> Result<()> { let mut g = group_lock(); if let Some(group) = g.as_mut() { - // The group takes the command value (command_take), so the eager - // redo must happen on the still-owned handle FIRST — the group - // receives an already-done command (C++ semantics: add_child + - // redo_now, net effect identical for the group's reverse-order - // undo). - let rc = command_redo_now(command); - if rc != 0 { - return rc; - } - // SAFETY: `command` is a live module command handle (facade - // contract); `command_take` moves the value out of its box and - // marks the shell non-owning. - return match unsafe { command_take(command.ctx) } { - Ok(cmd) => { - group.multi.multi_add_child(cmd); - 0 - } - Err(e) => e.code(), - }; + // The group takes the command as an already-done child: the eager + // redo must run BEFORE the child joins the group (C++ semantics: + // add_child + redo_now, net effect identical for the group's + // reverse-order undo). + let mut command = command; + command.redo_now(); + group.multi.multi_add_child(command); + return Ok(()); } - // No group open: take the command value and push it onto the - // process-wide stack (redo then record). - // SAFETY: as above, `command` is a live module command handle. - let cmd = match unsafe { command_take(command.ctx) } { - Ok(cmd) => cmd, - Err(e) => return e.code(), - }; + drop(g); let mut guard = global_stack().lock().unwrap_or_else(|e| e.into_inner()); - guard.push(cmd, name); + guard.push(command, name); drop(guard); - // The stack took the command; its redo already ran (plan M13 D2): - // persist the write-through subscribers. + // The stack took the command; its redo already ran: persist the + // write-through subscribers. notify_observers(); - 0 -} - -// --------------------------------------------------------------------------- -// Safe value-typed surface (M14 R3) -// -// The direct-rlib frontends (the app) hold module [`UndoCommand`] values, -// not CHandles. The functions below are the safe twins of the handle-based -// API above; the handle marshalling they perform stays inside this crate. -// --------------------------------------------------------------------------- - -/// Push `command` onto the process-wide stack (redo then record), or into -/// the open group. On a stack record the command observers fire (the -/// oakstorage write-through persists the edit). -pub fn push(command: UndoCommand, name: &str) -> Result<()> { - // SAFETY: `command_from_owned` boxes the value; `push_or_run` takes the - // value out of the handle (stack push or group child), and - // `command_free` releases the remaining non-owning shell (dropping the - // command itself when nobody took it). - let mut handle = unsafe { command_from_owned(command) }; - if handle.is_null() { - return Err(Error::NoMem); - } - let rc = push_or_run(handle, name); - unsafe{command_free(&mut handle)}; - if rc == 0 { - Ok(()) - } else { - Err(Error::from_code(rc)) - } + Ok(()) } /// Step the process-wide stack back one entry (no-op at the bottom). On @@ -261,14 +200,12 @@ pub fn redo() -> Result<()> { /// Whether the process-wide stack has an entry to undo. pub fn undoable() -> bool { - let mut v: c_int = 0; - unsafe{can_undo(&mut v).is_ok() && v != 0} + can_undo() } /// Whether the process-wide stack has an entry to redo. pub fn redoable() -> bool { - let mut v: c_int = 0; - unsafe{can_redo(&mut v).is_ok() && v != 0} + can_redo() } // --------------------------------------------------------------------------- @@ -285,30 +222,14 @@ pub fn index() -> Result { with_stack(|s| Ok(s.done_count())) } -/// Whether an undo is possible (1/0 via `out_value`; a module error code -/// otherwise). -pub unsafe fn can_undo(out_value: *mut c_int) -> Result<()> { - if out_value.is_null() { - return Err(Error::Invalid); - } - let value = with_stack(|s| Ok(if s.can_undo() { 1 } else { 0 }))?; - // SAFETY: `out_value` points to at least one writable `c_int`, per the - // facade contract. - unsafe { *out_value = value }; - Ok(()) +/// Whether an undo is possible. +pub fn can_undo() -> bool { + with_stack(|s| Ok(s.can_undo())).unwrap_or(false) } -/// Whether a redo is possible (1/0 via `out_value`; a module error code -/// otherwise). -pub unsafe fn can_redo(out_value: *mut c_int) -> Result<()> { - if out_value.is_null() { - return Err(Error::Invalid); - } - let value = with_stack(|s| Ok(if s.can_redo() { 1 } else { 0 }))?; - // SAFETY: `out_value` points to at least one writable `c_int`, per the - // facade contract. - unsafe { *out_value = value }; - Ok(()) +/// Whether a redo is possible. +pub fn can_redo() -> bool { + with_stack(|s| Ok(s.can_redo())).unwrap_or(false) } /// Undo/redo until the done-command count equals `index`. On success the @@ -331,58 +252,15 @@ pub fn clear() -> Result<()> { }) } -/// Two-stage label getter for the row at `row` (see -/// [`crate::undostack::undostack_command_text`]): returns the required -/// size including the NUL, or a module error code. -pub unsafe fn command_text(row: i64, buf: *mut c_char, buf_size: c_int) -> c_int { - let result = catch_unwind(AssertUnwindSafe(|| -> Result { - with_stack(|s| { - if row < 0 || row >= s.command_count() { - return Err(Error::NotFound); - } - let name = s.command_name(row)?; - let required = (name.len() + 1) as i32; - if !buf.is_null() && buf_size > 0 { - let copy_len = name.len().min((buf_size as usize).saturating_sub(1)); - let bytes = name.as_bytes(); - // SAFETY: `buf` points to `buf_size` writable bytes and we - // write at most `copy_len` (+ one NUL) of them. - unsafe { - std::ptr::copy_nonoverlapping(bytes.as_ptr(), buf as *mut u8, copy_len); - *buf.add(copy_len) = 0; - } - } - Ok(required) - }) - })); - match result { - Ok(Ok(required)) => required, - Ok(Err(e)) => e.code(), - Err(_) => OAKUNDO_E_FAILED, - } -} - -/// Whether the row at `row` is done (1/0 via `out_value`; a module error -/// code otherwise — `-20004` for an out-of-range row). -pub unsafe fn command_is_done(row: i64, out_value: *mut c_int) -> Result<()> { - if out_value.is_null() { - return Err(Error::Invalid); - } - let done = with_stack(|s| s.command_is_done(row))?; - // SAFETY: `out_value` points to at least one writable `c_int`, per the - // facade contract. - unsafe { *out_value = if done { 1 } else { 0 } }; - Ok(()) -} - -/// The user-visible label of the row at `row` (the safe twin of the -/// two-stage [`command_text`]; the history panel's row query). +/// The user-visible label of the row at `row` (the safe replacement for +/// the C-ABI two-stage `command_text(buf, size)`; the history panel's row +/// query). `NotFound` for an out-of-range row. pub fn command_name(row: i64) -> Result { with_stack(|s| s.command_name(row).map(|n| n.to_string())) } -/// Whether the row at `row` is done (the safe twin of -/// [`command_is_done`]; the history panel's gray-row query). +/// Whether the row at `row` is done (the safe replacement for the C-ABI +/// `command_is_done(out)`; the history panel's gray-row query). pub fn command_done(row: i64) -> Result { with_stack(|s| s.command_is_done(row)) } @@ -405,21 +283,12 @@ mod tests { } /// The stack/group/observer state is process-wide: every test here runs - /// serially under this lock, mirroring the facade's GLOBAL_STACK_LOCK - /// pattern. + /// serially under this lock. static LOCK: Mutex<()> = Mutex::new(()); - fn vtable_command() -> CHandle { - use crate::undocommand::{command_init, OakUndoCommandVtable}; - unsafe{ - command_init( - &OakUndoCommandVtable { - redo: None, - undo: None, - free_fn: None, - }, - std::ptr::null_mut(), - )} + /// A no-op command value. + fn noop_command() -> UndoCommand { + UndoCommand::from_closures(|| {}, || {}) } #[test] @@ -431,23 +300,18 @@ mod tests { assert!(clear().is_ok()); assert_eq!(count().unwrap(), 1); assert_eq!(index().unwrap(), 1); - let mut v: c_int = 1; - assert!(unsafe{can_undo(&mut v).is_ok()}); - assert_eq!(v, 0); + assert!(!can_undo()); // Push fires the observer once. - let cmd = vtable_command(); - assert_eq!(push_or_run(cmd, "alpha"), 0); + push(noop_command(), "alpha").unwrap(); assert_eq!(count().unwrap(), 2); assert_eq!(COUNT.load(std::sync::atomic::Ordering::SeqCst), 1); // Group begin/end fires the observer once at end. assert!(group_begin("grouped").is_ok()); assert!(group_begin("again").is_err()); // State - let c1 = vtable_command(); - let c2 = vtable_command(); - assert_eq!(push_or_run(c1, "c1"), 0); - assert_eq!(push_or_run(c2, "c2"), 0); + push(noop_command(), "c1").unwrap(); + push(noop_command(), "c2").unwrap(); // Children joined the group: no observer fire yet. assert_eq!(COUNT.load(std::sync::atomic::Ordering::SeqCst), 1); assert_eq!(count().unwrap(), 2); @@ -457,8 +321,7 @@ mod tests { // Abort fires nothing. assert!(group_begin("abort").is_ok()); - let c3 = vtable_command(); - assert_eq!(push_or_run(c3, "c3"), 0); + push(noop_command(), "c3").unwrap(); assert!(group_abort().is_ok()); assert_eq!(count().unwrap(), 3); assert_eq!(COUNT.load(std::sync::atomic::Ordering::SeqCst), 2); @@ -470,9 +333,9 @@ mod tests { assert!(clear().is_ok()); } - /// The safe value-typed surface (M14 R3): `push` redoes and records a - /// closure command, `undo`/`redo` step the stack, and the observers - /// fire on every recorded mutation. + /// The value-typed surface: `push` redoes and records a closure + /// command, `undo`/`redo` step the stack, and the observers fire on + /// every recorded mutation. #[test] fn value_push_and_undo_redo() { let _g = LOCK.lock().unwrap_or_else(|e| e.into_inner()); @@ -511,9 +374,9 @@ mod tests { assert!(clear().is_ok()); } - /// The safe row getters answer like the C-ABI twins: labels survive an - /// undo (undone rows stay labeled) and `command_done` flips with the - /// stack pointer. + /// The row getters answer like the C-ABI twins: labels survive an undo + /// (undone rows stay labeled) and `command_done` flips with the stack + /// pointer. #[test] fn value_command_name_and_done() { let _g = LOCK.lock().unwrap_or_else(|e| e.into_inner()); diff --git a/crates/oakundo/src/handle.rs b/crates/oakundo/src/handle.rs deleted file mode 100644 index 0b6563f5c..000000000 --- a/crates/oakundo/src/handle.rs +++ /dev/null @@ -1,160 +0,0 @@ -// Oak Video Editor - Non-Linear Video Editor -// Copyright (C) 2026 Oak Team -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -//! Refcounted-handle scaffolding. Same pattern as the oaknode/oakcodec -//! crates (`src/node/rust/src/handle.rs`); intentionally duplicated rather -//! than shared — each module DLL must run its own addref/release code -//! (the function pointers in a handle always point into the DLL that -//! created the object). - -use std::panic::{catch_unwind, AssertUnwindSafe}; -use std::sync::atomic::{AtomicU32, Ordering}; - -/// ABI version stamped into every handle. -pub const OAKUNDO_ABI_VERSION: u32 = 1; - -/// Heap box behind a handle's `ctx`. -pub struct RefBox { - /// Atomic reference count. - pub refs: AtomicU32, - /// Boxed value. - pub value: T, -} - -/// The shared ABI value-handle type (single-lib unification, see -/// `docs/zh/plans/riir/single-lib.md`): one canonical -/// `{ctx, addref, release, abi_version}` type in `oakcore-rs`, re-exported -/// here so the crate's `ffi.rs` signatures and handle scaffolding stay -/// source-compatible. -pub use oakcore_rs::handle::CHandle; - -/// Owned handle with count 1; empty on allocation failure. -pub fn make_owned(value: T) -> CHandle { - let boxed = RefBox { - refs: AtomicU32::new(1), - value, - }; - let ctx = Box::into_raw(Box::new(boxed)) as *mut std::ffi::c_void; - CHandle { - ctx, - addref: Some(addref_owned::), - release: Some(release_owned::), - abi_version: OAKUNDO_ABI_VERSION, - } -} - -/// Borrowed handle for an object owned elsewhere (release frees only -/// the box). -/// -/// # Safety -/// Caller guarantees `ptr` outlives every derived handle. -pub unsafe fn make_borrowed(ptr: *mut T) -> CHandle { - let boxed = RefBox { - refs: AtomicU32::new(1), - value: ptr, - }; - let ctx = Box::into_raw(Box::new(boxed)) as *mut std::ffi::c_void; - CHandle { - ctx, - addref: Some(addref_borrowed::), - release: Some(release_borrowed::), - abi_version: OAKUNDO_ABI_VERSION, - } -} - -/// Typed view into a handle; `None` for empty handles. -/// -/// # Safety -/// `T` must be the boxed type. -pub unsafe fn get(h: &CHandle) -> Option<&T> { - unsafe { - if h.ctx.is_null() { - None - } else { - Some(&(*(h.ctx as *mut RefBox)).value) - } - } -} - -/// Panic-catching FFI wrapper for i32-returning exports. -pub fn guard crate::error::Result<()>>(f: F) -> i32 { - match catch_unwind(AssertUnwindSafe(f)) { - Ok(Ok(())) => crate::error::OAKUNDO_OK, - Ok(Err(e)) => e.code(), - Err(_) => crate::error::OAKUNDO_E_FAILED, - } -} - -/// Panic-catching FFI wrapper for handle-returning exports. -pub fn guard_handle crate::error::Result>(f: F) -> CHandle { - match catch_unwind(AssertUnwindSafe(f)) { - Ok(Ok(h)) => h, - _ => CHandle::null(), - } -} - -/// Panic-catching FFI wrapper for void exports. -pub fn guard_void(f: F) { - let _ = catch_unwind(AssertUnwindSafe(f)); -} - -/// Owned addref: bump the box's refcount. -unsafe extern "C" fn addref_owned(ctx: *mut std::ffi::c_void) { - unsafe { - if ctx.is_null() { - return; - } - let boxed = ctx as *mut RefBox; - (&(*boxed).refs).fetch_add(1, Ordering::AcqRel); - } -} - -/// Owned release: drop the box when the refcount hits zero. -unsafe extern "C" fn release_owned(ctx: *mut std::ffi::c_void) { - unsafe { - if ctx.is_null() { - return; - } - let boxed = ctx as *mut RefBox; - if (&(*boxed).refs).fetch_sub(1, Ordering::AcqRel) == 1 { - drop(Box::from_raw(boxed)); - } - } -} - -/// Borrowed addref: bump the shell's refcount. -unsafe extern "C" fn addref_borrowed(ctx: *mut std::ffi::c_void) { - unsafe { - if ctx.is_null() { - return; - } - let boxed = ctx as *mut RefBox<*mut T>; - (&(*boxed).refs).fetch_add(1, Ordering::AcqRel); - } -} - -/// Borrowed release: free only the shell, never the pointee. -unsafe extern "C" fn release_borrowed(ctx: *mut std::ffi::c_void) { - unsafe { - if ctx.is_null() { - return; - } - let boxed = ctx as *mut RefBox<*mut T>; - if (&(*boxed).refs).fetch_sub(1, Ordering::AcqRel) == 1 { - drop(Box::from_raw(boxed)); - } - } -} diff --git a/crates/oakundo/src/lib.rs b/crates/oakundo/src/lib.rs index 83bc7a5e3..bca06f254 100644 --- a/crates/oakundo/src/lib.rs +++ b/crates/oakundo/src/lib.rs @@ -16,22 +16,28 @@ //! # oakundo — the undo/redo history module (Rust) //! -//! Reimplements the C++ oakundo module behind its frozen C ABI -//! (`include/undo/*.h`). See README.md for the architectural mapping — -//! most notably the **vtable-command pattern**: C++ subclassing of -//! `olive::UndoCommand` becomes a callback table + `userdata` pointer. +//! Reimplements the C++ oakundo module. See README.md for the +//! architectural mapping — most notably the **trait-object command +//! pattern**: C++ subclassing of `olive::UndoCommand` becomes a boxed +//! [`undocommand::Command`] (closure-backed or a +//! [`undocommand::MultiUndoCommand`] composite) behind a plain owned +//! [`undocommand::UndoCommand`] value. //! -//! ## FFI discipline +//! ## Consumers //! -//! Identical to the oaknode/oakcodec crates: every export goes through -//! [`handle::guard*`], handles are opaque refcounted boxes, shared -//! state behind `Mutex`. +//! Every consumer is in-process Rust: the app's edit layer +//! (`oakui::graphops` / `oakui::real`) drives the process-wide stack +//! through [`global`], `oaktimeline`/`oaktask`/`oaknode`/`oakplugin` +//! build commands as [`undocommand::UndoCommand`] values, and +//! `oakstorage` subscribes to the command-success observers in +//! [`global`]. The former frozen C ABI (`include/undo/*.h`), the +//! refcounted [`CHandle`] layer and the callback-table vtable commands +//! were deleted with the engine facade — the crate is pure owned-value +//! Rust with no `unsafe`. -#![deny(unsafe_op_in_unsafe_fn)] #![warn(missing_docs)] pub mod error; pub mod global; -pub mod handle; pub mod undocommand; pub mod undostack; diff --git a/crates/oakundo/src/undocommand.rs b/crates/oakundo/src/undocommand.rs index aee517685..e876c7295 100644 --- a/crates/oakundo/src/undocommand.rs +++ b/crates/oakundo/src/undocommand.rs @@ -15,66 +15,42 @@ // along with this program. If not, see . //! `olive::UndoCommand` / `olive::MultiUndoCommand` — the undoable -//! operation and its composite. Mirrors `src/undo/src/undocommand.h` -//! and `include/undo/undocommand.h`. +//! operation and its composite. Mirrors `src/undo/src/undocommand.h`. //! //! The C++ base is subclassed by other modules (`redo()`/`undo()` -//! overrides). Rust has no inheritance, so the C ABI models the same -//! polymorphism with a callback table + `userdata` pointer -//! ([`OakUndoCommandVtable`]); this module's [`UndoCommand`] holds a -//! [`CommandKind`] that is either that vtable-backed command or a -//! [`MultiUndoCommand`] composite. This vtable-command pattern is the -//! centerpiece of the oakundo architecture (see README.md). -//! -//! ## Command boxes -//! -//! Commands cross crate boundaries through a dedicated [`CommandBox`] -//! (not the generic [`crate::handle::RefBox`]): it holds the raw -//! `*mut UndoCommand`, an `owns` flag (mirroring the C++ -//! `OakUndoCommandBox`) and a refcount. An **owning** box owns and -//! destroys its command at refcount zero; a **borrowed** box (a -//! reference into a `MultiUndoCommand` or an `UndoStack`) owns only its -//! shell. `take_command` moves the command value out of an owning box, -//! turning it into a non-owning shell (the C++ `mark_container_owned`). -//! -//! The handle-level functions below (`command_init` ... `command_free`) -//! were sunk from the former C ABI export layer: they keep the -//! `CHandle`-based signatures so the facade bridges and integration -//! tests can drive the command machinery directly. The `CHandle` layer -//! itself is removed in a later milestone. - -use std::ffi::{c_int, c_void}; -use std::sync::atomic::{AtomicU32, Ordering}; +//! overrides); the Rust equivalent of that virtual dispatch is a +//! trait-object command. [`UndoCommand`] boxes a [`Command`] (implemented +//! by closure-backed commands and by the [`MultiUndoCommand`] composite) +//! plus the command's state flags. This replaces the former +//! `OakUndoCommandVtable` + `userdata` callback-table model, which only +//! existed to cross the frozen C ABI (see README.md). With every consumer +//! in-process there is no callback table, no `extern "C"` trampoline and +//! no refcounted `CHandle` — commands are owned values. use crate::error::{Error, Result}; -use crate::handle::{guard, guard_handle, guard_void, CHandle, OAKUNDO_ABI_VERSION}; -/// `oakundo_command_vtable` — the caller-defined redo/undo/free -/// callback table. Any entry may be `None`; a `None` redo/undo makes -/// that direction a no-op, `None` free_fn skips userdata release. -#[repr(C)] -#[derive(Clone, Copy)] -pub struct OakUndoCommandVtable { - /// Execute the operation. - pub redo: Option, - /// Reverse the operation. - pub undo: Option, - /// Release `userdata` when the command is destroyed. - pub free_fn: Option, -} +/// A single undoable operation (the C++ virtual `redo()`/`undo()` pair). +/// +/// Implementations must be `Send`: the owning [`UndoCommand`] lives in +/// stacks shared behind a `Mutex`, so it may move across threads (exactly +/// as the C++ commands did when the owning stack was shared). +pub trait Command: Send { + /// Apply the change. + fn redo(&mut self); + /// Revert the change. + fn undo(&mut self); -/// What backs an [`UndoCommand`]: a caller-defined vtable command or a -/// composite of children. The direct analog of C++ virtual dispatch. -pub enum CommandKind { - /// Caller-defined command: callback table plus opaque userdata. - Vtable { - /// The copied callback table. - vtable: OakUndoCommandVtable, - /// Opaque caller state; owned by the command. - userdata: *mut c_void, - }, - /// Composite command (`olive::MultiUndoCommand`). - Multi(MultiUndoCommand), + /// `Some` for composite commands; used by [`UndoCommand::multi_*`]. + /// Defaults to `None` (a plain command is never a composite). + fn as_multi(&self) -> Option<&MultiUndoCommand> { + None + } + + /// `Some` for composite commands; used by [`UndoCommand::multi_*`]. + /// Defaults to `None` (a plain command is never a composite). + fn as_multi_mut(&mut self) -> Option<&mut MultiUndoCommand> { + None + } } /// `olive::UndoCommand` — an undoable operation. @@ -84,13 +60,13 @@ pub enum CommandKind { /// *callbacks* are not part of the C ABI and are intentionally omitted /// (see README.md decision 2); only the flag snapshot is retained. /// -/// `prepared` mirrors the C++ `prepared_` flag (the vtable has no +/// `prepared` mirrors the C++ `prepared_` flag (the command trait has no /// `prepare()` callback, so it starts `true` and never un-prepares); /// `is_empty` marks the invariant bottom-of-stack command so the stack /// can keep it out of `can_undo`. pub struct UndoCommand { - /// The operation (vtable or composite). - kind: CommandKind, + /// The operation (closure-backed or composite). + inner: Box, /// Whether the command has been executed (`done_`). done: bool, /// Project-dirty flag snapshot recorded at the last redo. @@ -105,17 +81,13 @@ pub struct UndoCommand { is_empty: bool, } -/// A command's userdata is caller-controlled; like the C++ object it -/// may be moved across threads when the owning stack is shared behind a -/// `Mutex`. Callbacks must be thread-safe with respect to the caller's -/// own locking, exactly as in the C++ implementation. -unsafe impl Send for UndoCommand {} - impl UndoCommand { - /// New vtable-backed command; takes ownership of `userdata`. - pub fn from_vtable(vtable: OakUndoCommandVtable, userdata: *mut c_void) -> Self { + /// Wrap any [`Command`] as an undoable command value. This is the + /// construction path for whole-struct commands (the app's timeline + /// and task commands); one-off edits use [`UndoCommand::from_closures`]. + pub fn new(command: impl Command + 'static) -> Self { UndoCommand { - kind: CommandKind::Vtable { vtable, userdata }, + inner: Box::new(command), done: false, modified: false, prepared: true, @@ -123,89 +95,57 @@ impl UndoCommand { } } - /// New empty composite command (`olive::MultiUndoCommand`). - pub fn multi() -> Self { - UndoCommand { - kind: CommandKind::Multi(MultiUndoCommand::new()), - done: false, - modified: false, - prepared: true, - is_empty: false, - } - } - - /// New closure-backed command (M14 R3): `redo`/`undo` are plain Rust - /// closures, boxed behind the vtable machinery. This is the safe way - /// for direct-rlib consumers (the app) to build composite edit - /// commands without writing their own `extern "C"` trampolines. + /// New closure-backed command: `redo` runs on redo, `undo` on undo. + /// The safe way for direct-rlib consumers (the app) to build edit + /// commands without writing any callback machinery. pub fn from_closures( redo: impl FnMut() + Send + 'static, undo: impl FnMut() + Send + 'static, ) -> Self { - let state = Box::into_raw(Box::new(ClosureCommand { + UndoCommand::new(ClosureCommand { redo: Box::new(redo), undo: Box::new(undo), - })); - UndoCommand::from_vtable( - OakUndoCommandVtable { - redo: Some(closure_redo), - undo: Some(closure_undo), - free_fn: Some(closure_free), - }, - state as *mut c_void, - ) + }) } - /// The invariant bottom-of-stack command ("New/Open Project"); all - /// callbacks are no-ops and it is never undoable. + /// New empty composite command (`olive::MultiUndoCommand`). + pub fn multi() -> Self { + UndoCommand::new(MultiUndoCommand::new()) + } + + /// The invariant bottom-of-stack command ("New/Open Project"); a + /// no-op in both directions and never undoable. pub(crate) fn empty() -> Self { - UndoCommand { - kind: CommandKind::Vtable { - vtable: OakUndoCommandVtable { - redo: None, - undo: None, - free_fn: None, - }, - userdata: std::ptr::null_mut(), - }, - done: false, - modified: false, - prepared: true, - is_empty: true, - } + let mut command = UndoCommand::from_closures(|| {}, || {}); + command.is_empty = true; + command } /// Add `child` to a composite command (takes one reference). pub fn multi_add_child(&mut self, child: UndoCommand) { - match &mut self.kind { - CommandKind::Multi(m) => m.add_child(child), - CommandKind::Vtable { .. } => { - panic!("multi_add_child on a non-multi command") - } + match self.inner.as_multi_mut() { + Some(m) => m.add_child(child), + None => panic!("multi_add_child on a non-multi command"), } } - /// Number of children of a composite command. + /// Number of children of a composite command (0 for plain commands). pub fn multi_child_count(&self) -> usize { - match &self.kind { - CommandKind::Multi(m) => m.child_count(), - CommandKind::Vtable { .. } => 0, - } + self.inner.as_multi().map_or(0, |m| m.child_count()) } /// Reference to the child at `index` of a composite command. pub fn multi_child(&self, index: usize) -> Result<&UndoCommand> { - match &self.kind { - CommandKind::Multi(m) => m.child(index), - CommandKind::Vtable { .. } => Err(Error::Invalid), - } + self.inner + .as_multi() + .map_or(Err(Error::Invalid), |m| m.child(index)) } /// Mutable reference to the child at `index` of a composite command. pub fn multi_child_mut(&mut self, index: usize) -> Result<&mut UndoCommand> { - match &mut self.kind { - CommandKind::Multi(m) => m.child_mut(index), - CommandKind::Vtable { .. } => Err(Error::Invalid), + match self.inner.as_multi_mut() { + Some(m) => m.child_mut(index), + None => Err(Error::Invalid), } } @@ -239,8 +179,8 @@ impl UndoCommand { self.undo_now(); } - /// `has_prepared`: whether `prepare()` has run (vtable commands have - /// no prepare; always true). + /// `has_prepared`: whether `prepare()` has run (commands have no + /// prepare; always true). pub fn has_prepared(&self) -> bool { self.prepared } @@ -267,49 +207,17 @@ impl UndoCommand { /// Whether this is a composite (`MultiUndoCommand`) command. pub(crate) fn is_multi(&self) -> bool { - matches!(self.kind, CommandKind::Multi(_)) + self.inner.as_multi().is_some() } /// Run the operation (virtual dispatch). fn redo(&mut self) { - match &mut self.kind { - CommandKind::Vtable { vtable, userdata } => { - if let Some(f) = vtable.redo { - // SAFETY: `userdata` is caller-supplied and outlives - // the command (owned by it). The callback is the one - // registered by the caller. - unsafe { f(*userdata) } - } - } - CommandKind::Multi(m) => m.redo(), - } + self.inner.redo(); } /// Reverse the operation (virtual dispatch). fn undo(&mut self) { - match &mut self.kind { - CommandKind::Vtable { vtable, userdata } => { - if let Some(f) = vtable.undo { - // SAFETY: as in `redo`. - unsafe { f(*userdata) } - } - } - CommandKind::Multi(m) => m.undo(), - } - } -} - -impl Drop for UndoCommand { - /// Invokes the vtable `free_fn` on `userdata` (composites free - /// children transitively). - fn drop(&mut self) { - if let CommandKind::Vtable { vtable, userdata } = &self.kind { - if let Some(f) = vtable.free_fn { - // SAFETY: `userdata` is owned by the command; this is the - // final release. - unsafe { f(*userdata) } - } - } + self.inner.undo(); } } @@ -321,43 +229,14 @@ struct ClosureCommand { undo: Box, } -/// `redo` trampoline for closure commands. -/// -/// # Safety -/// `userdata` must be the `Box` produced by -/// [`UndoCommand::from_closures`]; the owning command keeps it alive. -unsafe extern "C" fn closure_redo(userdata: *mut c_void) { - if userdata.is_null() { - return; +impl Command for ClosureCommand { + fn redo(&mut self) { + (self.redo)(); } - // SAFETY: per the from_closures contract; the command owns the box. - let state = unsafe { &mut *(userdata as *mut ClosureCommand) }; - (state.redo)(); -} -/// `undo` trampoline for closure commands. -/// -/// # Safety -/// As [`closure_redo`]. -unsafe extern "C" fn closure_undo(userdata: *mut c_void) { - if userdata.is_null() { - return; + fn undo(&mut self) { + (self.undo)(); } - // SAFETY: per the from_closures contract; the command owns the box. - let state = unsafe { &mut *(userdata as *mut ClosureCommand) }; - (state.undo)(); -} - -/// `free` trampoline for closure commands: drops the boxed closures. -/// -/// # Safety -/// As [`closure_redo`]; called at most once (the command's final release). -unsafe extern "C" fn closure_free(userdata: *mut c_void) { - if userdata.is_null() { - return; - } - // SAFETY: per the from_closures contract; this is the final release. - unsafe { drop(Box::from_raw(userdata as *mut ClosureCommand)) }; } /// `olive::MultiUndoCommand` — a composite of child commands. @@ -369,9 +248,6 @@ pub struct MultiUndoCommand { children: Vec, } -/// See `UndoCommand::Send`. -unsafe impl Send for MultiUndoCommand {} - impl MultiUndoCommand { /// New empty composite. pub fn new() -> Self { @@ -421,250 +297,20 @@ impl Default for MultiUndoCommand { } } -/// Heap box behind a command handle's `ctx`. Mirrors the C++ -/// `OakUndoCommandBox` (`{command, owns, refs}`). -pub(crate) struct CommandBox { - /// Raw pointer to the command value. - command: *mut UndoCommand, - /// Whether this box owns (and must destroy) `command`. - owns: bool, - /// Atomic reference count. - refs: AtomicU32, -} +impl Command for MultiUndoCommand { + fn redo(&mut self) { + MultiUndoCommand::redo(self); + } -/// Owned handle for a fresh command value (refcount 1, owns `cmd`). -/// -/// # Safety -/// The returned handle owns `cmd`; release it with `command_release`. -pub unsafe fn command_from_owned(cmd: UndoCommand) -> CHandle { - let boxed = Box::into_raw(Box::new(CommandBox { - command: Box::into_raw(Box::new(cmd)), - owns: true, - refs: AtomicU32::new(1), - })) as *mut c_void; - CHandle { - ctx: boxed, - addref: Some(command_addref), - release: Some(command_release), - abi_version: OAKUNDO_ABI_VERSION, + fn undo(&mut self) { + MultiUndoCommand::undo(self); + } + + fn as_multi(&self) -> Option<&MultiUndoCommand> { + Some(self) + } + + fn as_multi_mut(&mut self) -> Option<&mut MultiUndoCommand> { + Some(self) } } - -/// Borrowed handle for a command owned elsewhere (release frees only -/// the shell). -/// -/// # Safety -/// `cmd` must outlive the returned handle. -pub unsafe fn command_from_borrowed(cmd: *mut UndoCommand) -> CHandle { - let boxed = Box::into_raw(Box::new(CommandBox { - command: cmd, - owns: false, - refs: AtomicU32::new(1), - })) as *mut c_void; - CHandle { - ctx: boxed, - addref: Some(command_addref), - release: Some(command_release), - abi_version: OAKUNDO_ABI_VERSION, - } -} - -/// Read-only view of the command behind `ctx`; `None` for a null handle. -/// -/// # Safety -/// `ctx` must come from a valid, live `CommandBox`. -pub unsafe fn command_to_ref(ctx: *mut c_void) -> Option<&'static UndoCommand> { - unsafe { - if ctx.is_null() { - return None; - } - let boxed = ctx as *mut CommandBox; - if (*boxed).command.is_null() { - return None; - } - Some(&*(*boxed).command) - } -} - -/// Mutable view of the command behind `ctx`; `None` for a null handle. -/// -/// # Safety -/// `ctx` must come from a valid, live `CommandBox`. -pub unsafe fn command_to_mut(ctx: *mut c_void) -> Option<&'static mut UndoCommand> { - unsafe { - if ctx.is_null() { - return None; - } - let boxed = ctx as *mut CommandBox; - if (*boxed).command.is_null() { - return None; - } - Some(&mut *(*boxed).command) - } -} - -/// Move the command value out of an owning box, turning it into a -/// non-owning shell (the C++ `mark_container_owned` transfer). -/// -/// # Safety -/// `ctx` must come from a valid, live `CommandBox`. -pub unsafe fn command_take(ctx: *mut c_void) -> Result { - unsafe { - if ctx.is_null() { - return Err(Error::Invalid); - } - let boxed = ctx as *mut CommandBox; - if !(*boxed).owns || (*boxed).command.is_null() { - return Err(Error::State); - } - let value = Box::from_raw((*boxed).command); - (*boxed).command = std::ptr::null_mut(); - (*boxed).owns = false; - Ok(*value) - } -} - -/// Increment a command box's refcount (the `addref` function pointer). -/// -/// # Safety -/// `ctx` must come from a valid `CommandBox`. -pub(crate) unsafe extern "C" fn command_addref(ctx: *mut c_void) { - unsafe { - if ctx.is_null() { - return; - } - let boxed = ctx as *mut CommandBox; - (&(*boxed).refs).fetch_add(1, Ordering::AcqRel); - } -} - -/// Decrement a command box's refcount; destroys at zero. Owned boxes -/// free their command first, then the shell; borrowed boxes free only -/// the shell. -/// -/// # Safety -/// `ctx` must come from a valid `CommandBox`. -pub(crate) unsafe extern "C" fn command_release(ctx: *mut c_void) { - unsafe { - if ctx.is_null() { - return; - } - let boxed = ctx as *mut CommandBox; - if (&(*boxed).refs).fetch_sub(1, Ordering::AcqRel) == 1 { - if (*boxed).owns && !(*boxed).command.is_null() { - drop(Box::from_raw((*boxed).command)); - } - drop(Box::from_raw(boxed)); - } - } -} - -// --------------------------------------------------------------------------- -// Handle-level command API (sunk from the former C ABI export layer) -// --------------------------------------------------------------------------- - -/// Create a vtable-backed command handle (refcount 1); a `NULL` vtable -/// yields an empty handle (`oakundo_command_init`). -pub unsafe fn command_init(vtable: *const OakUndoCommandVtable, userdata: *mut c_void) -> CHandle { - guard_handle(|| unsafe { - if vtable.is_null() { - return Ok(CHandle::null()); - } - let table = *vtable; - Ok(command_from_owned(UndoCommand::from_vtable(table, userdata))) - }) -} - -/// Create an empty multi command handle (refcount 1) -/// (`oakundo_command_init_multi`). -pub fn command_init_multi() -> CHandle { - guard_handle(|| unsafe { Ok(command_from_owned(UndoCommand::multi())) }) -} - -/// Add `child` to a multi command handle (the stack/multi takes one -/// child reference); `E_INVALID` for an empty handle or a non-multi -/// parent (`oakundo_command_multi_add_child`). -pub fn command_multi_add_child(multi: CHandle, child: CHandle) -> c_int { - guard(|| unsafe { - let parent = command_to_mut(multi.ctx).ok_or(Error::Invalid)?; - if !parent.is_multi() { - return Err(Error::Invalid); - } - let child_cmd = command_take(child.ctx)?; - parent.multi_add_child(child_cmd); - Ok(()) - }) -} - -/// Number of children of a multi command handle; `E_INVALID` for an -/// empty handle or a non-multi parent (`oakundo_command_multi_child_count`). -pub unsafe fn command_multi_child_count(multi: CHandle, out_count: *mut c_int) -> c_int { - guard(|| unsafe { - if out_count.is_null() { - return Err(Error::Invalid); - } - let parent = command_to_ref(multi.ctx).ok_or(Error::Invalid)?; - if !parent.is_multi() { - return Err(Error::Invalid); - } - *out_count = parent.multi_child_count() as c_int; - Ok(()) - }) -} - -/// Write a borrowed handle to the child at `index` of a multi command -/// (the returned handle carries its own shell ref); `E_NOT_FOUND` for -/// an out-of-range index (`oakundo_command_multi_child`). -pub unsafe fn command_multi_child(multi: CHandle, index: c_int, out_child: *mut CHandle) -> c_int { - guard(|| unsafe { - if out_child.is_null() { - return Err(Error::Invalid); - } - let parent = command_to_ref(multi.ctx).ok_or(Error::Invalid)?; - if !parent.is_multi() { - return Err(Error::Invalid); - } - if index < 0 { - return Err(Error::NotFound); - } - let child = parent.multi_child(index as usize)?; - let ptr = child as *const UndoCommand as *mut UndoCommand; - *out_child = command_from_borrowed(ptr); - Ok(()) - }) -} - -/// Execute a command handle's redo (no-op when already done) -/// (`oakundo_command_redo_now`). -pub fn command_redo_now(command: CHandle) -> c_int { - guard(|| unsafe { - let c = command_to_mut(command.ctx).ok_or(Error::Invalid)?; - c.redo_now(); - Ok(()) - }) -} - -/// Execute a command handle's undo (no-op when not done) -/// (`oakundo_command_undo_now`). -pub fn command_undo_now(command: CHandle) -> c_int { - guard(|| unsafe { - let c = command_to_mut(command.ctx).ok_or(Error::Invalid)?; - c.undo_now(); - Ok(()) - }) -} - -/// Release a command handle in place: run the release callback, then -/// clear `ctx`. `NULL` / empty handles are no-ops -/// (`oakundo_command_free`). -pub unsafe fn command_free(command: *mut CHandle) { - guard_void(|| unsafe { - if command.is_null() || (*command).ctx.is_null() { - return; - } - if let Some(release) = (*command).release { - release((*command).ctx); - } - (*command).ctx = std::ptr::null_mut(); - }) -} diff --git a/crates/oakundo/src/undostack.rs b/crates/oakundo/src/undostack.rs index a7ad7ceae..9dc229cfa 100644 --- a/crates/oakundo/src/undostack.rs +++ b/crates/oakundo/src/undostack.rs @@ -15,21 +15,21 @@ // along with this program. If not, see . //! `olive::UndoStack` — the undo/redo history stack. Mirrors -//! `src/undo/src/undostack.h` and `include/undo/undostack.h`. +//! `src/undo/src/undostack.h`. //! //! Two deques: `commands` (done commands, oldest at the front) and //! `undone` (undone commands, most-recently-undone at the front). The //! fresh stack holds a single "New/Open Project" empty command so that //! `can_undo` is false at the bottom (see `undostack.cpp::clear`). +//! +//! The former handle-level exports (`undostack_init` … `undostack_free`) +//! are gone with the C ABI: the stack is a plain owned value, shared +//! process-wide through the [`crate::global`] module. use std::collections::VecDeque; -use std::ffi::{c_char, c_int, CStr}; -use std::panic::{catch_unwind, AssertUnwindSafe}; -use std::sync::Mutex; -use crate::error::{Error, OAKUNDO_E_FAILED, Result}; -use crate::handle::{get, guard, guard_handle, guard_void, make_owned, CHandle}; -use crate::undocommand::{command_take, UndoCommand}; +use crate::error::Result; +use crate::undocommand::UndoCommand; /// Maximum number of retained history rows (`k_max_undo_commands`). pub const K_MAX_UNDO_COMMANDS: usize = 200; @@ -67,9 +67,6 @@ pub struct UndoStack { undone: VecDeque, } -/// See `UndoCommand::Send`. -unsafe impl Send for UndoStack {} - impl UndoStack { /// New stack; contains a single empty "New/Open Project" command. pub fn new() -> Self { @@ -198,7 +195,7 @@ impl UndoStack { /// Whether the row at `row` is currently done. pub fn command_is_done(&self, row: i64) -> Result { if row < 0 || row >= self.command_count() { - return Err(Error::NotFound); + return Err(crate::error::Error::NotFound); } Ok(row < self.commands.len() as i64) } @@ -206,7 +203,7 @@ impl UndoStack { /// Label of the history row at `row`. pub fn command_name(&self, row: i64) -> Result<&str> { if row < 0 || row >= self.command_count() { - return Err(Error::NotFound); + return Err(crate::error::Error::NotFound); } let row = row as usize; if row < self.commands.len() { @@ -222,204 +219,3 @@ impl Default for UndoStack { Self::new() } } - -// --------------------------------------------------------------------------- -// Handle-level stack API (sunk from the former C ABI export layer) -// --------------------------------------------------------------------------- - -/// Read a NUL-terminated C string; `NULL` yields an empty string -/// (mirrors the C++ `name ? name : ""`). -fn read_name(name: *const c_char) -> String { - if name.is_null() { - String::new() - } else { - // SAFETY: `name` is a valid NUL-terminated string supplied by the - // caller, or NULL (already handled). - unsafe { CStr::from_ptr(name) }.to_string_lossy().into_owned() - } -} - -/// Lock the stack behind `stack` and run `f` on it. `E_INVALID` for an -/// empty handle. A poisoned mutex is recovered (its inner value is still -/// valid). -pub fn with_stack(stack: &CHandle, f: impl FnOnce(&mut UndoStack) -> Result) -> Result { - // SAFETY: the stack handle always boxes a `Mutex` (created - // by `undostack_init`). - let m = unsafe { get::>(stack) }.ok_or(Error::Invalid)?; - let mut guard = m.lock().unwrap_or_else(|e| e.into_inner()); - f(&mut guard) -} - -/// Create a fresh stack handle (refcount 1) (`oakundo_undostack_init`). -pub fn undostack_init() -> CHandle { - guard_handle(|| Ok(make_owned(Mutex::new(UndoStack::new())))) -} - -/// Release a stack handle in place; `NULL` / empty handles are no-ops -/// (`oakundo_undostack_free`). -pub fn undostack_free(stack: *mut CHandle) { - guard_void(|| unsafe { - if stack.is_null() || (*stack).ctx.is_null() { - return; - } - if let Some(release) = (*stack).release { - release((*stack).ctx); - } - (*stack).ctx = std::ptr::null_mut(); - }) -} - -/// Push a command handle onto the stack (redo then record; the stack -/// takes ownership of the command value, leaving a non-owning shell) -/// (`oakundo_undostack_push`). -pub fn undostack_push(stack: CHandle, command: CHandle, name: *const c_char) -> c_int { - guard(|| { - with_stack(&stack, |s| unsafe { - let cmd = command_take(command.ctx)?; - let name = read_name(name); - s.push(cmd, &name); - Ok(()) - }) - }) -} - -/// Push an already-executed command handle (redo skipped) -/// (`oakundo_undostack_push_pre_executed`). -pub fn undostack_push_pre_executed(stack: CHandle, command: CHandle, name: *const c_char) -> c_int { - guard(|| { - with_stack(&stack, |s| unsafe { - let cmd = command_take(command.ctx)?; - let name = read_name(name); - s.push_pre_executed(cmd, &name); - Ok(()) - }) - }) -} - -/// Undo one command on the stack (`oakundo_undostack_undo`). -pub fn undostack_undo(stack: CHandle) -> c_int { - guard(|| with_stack(&stack, |s| s.undo())) -} - -/// Redo one command on the stack (`oakundo_undostack_redo`). -pub fn undostack_redo(stack: CHandle) -> c_int { - guard(|| with_stack(&stack, |s| s.redo())) -} - -/// Jump to a done-command index (clamped to 0; `index` is i64) -/// (`oakundo_undostack_jump`). -pub fn undostack_jump(stack: CHandle, index: i64) -> c_int { - guard(|| { - with_stack(&stack, |s| { - s.jump(index); - Ok(()) - }) - }) -} - -/// Clear the stack back to the empty bottom command -/// (`oakundo_undostack_clear`). -pub fn undostack_clear(stack: CHandle) -> c_int { - guard(|| { - with_stack(&stack, |s| { - s.clear(); - Ok(()) - }) - }) -} - -/// Write whether an undo is possible (`oakundo_undostack_can_undo`). -pub fn undostack_can_undo(stack: CHandle, out_value: *mut c_int) -> c_int { - guard(|| unsafe { - if out_value.is_null() { - return Err(Error::Invalid); - } - with_stack(&stack, |s| { - *out_value = if s.can_undo() { 1 } else { 0 }; - Ok(()) - }) - }) -} - -/// Write whether a redo is possible (`oakundo_undostack_can_redo`). -pub fn undostack_can_redo(stack: CHandle, out_value: *mut c_int) -> c_int { - guard(|| unsafe { - if out_value.is_null() { - return Err(Error::Invalid); - } - with_stack(&stack, |s| { - *out_value = if s.can_redo() { 1 } else { 0 }; - Ok(()) - }) - }) -} - -/// Write the total history row count (`oakundo_undostack_count`). -pub fn undostack_count(stack: CHandle, out_count: *mut i64) -> c_int { - guard(|| unsafe { - if out_count.is_null() { - return Err(Error::Invalid); - } - with_stack(&stack, |s| { - *out_count = s.command_count(); - Ok(()) - }) - }) -} - -/// Write the done-command count (`oakundo_undostack_index`). -pub fn undostack_index(stack: CHandle, out_index: *mut i64) -> c_int { - guard(|| unsafe { - if out_index.is_null() { - return Err(Error::Invalid); - } - with_stack(&stack, |s| { - *out_index = s.done_count(); - Ok(()) - }) - }) -} - -/// Two-stage label getter for the row at `row`: returns the required -/// size (including NUL), or an error code; copies (truncating) when a -/// buffer is supplied (`oakundo_undostack_command_text`). -pub fn undostack_command_text(stack: CHandle, row: i64, buf: *mut c_char, buf_size: c_int) -> c_int { - let result = catch_unwind(AssertUnwindSafe(|| -> Result { - with_stack(&stack, |s| { - if row < 0 || row >= s.command_count() { - return Err(Error::NotFound); - } - let name = s.command_name(row)?; - let required = (name.len() + 1) as i32; - if !buf.is_null() && buf_size > 0 { - let copy_len = name.len().min((buf_size as usize).saturating_sub(1)); - let bytes = name.as_bytes(); - // SAFETY: `buf` points to `buf_size` writable bytes and we - // write at most `copy_len` (+ one NUL) of them. - unsafe { - std::ptr::copy_nonoverlapping(bytes.as_ptr(), buf as *mut u8, copy_len); - *buf.add(copy_len) = 0; - } - } - Ok(required) - }) - })); - match result { - Ok(Ok(required)) => required, - Ok(Err(e)) => e.code(), - Err(_) => OAKUNDO_E_FAILED, - } -} - -/// Write whether the row at `row` is done (`oakundo_undostack_command_is_done`). -pub fn undostack_command_is_done(stack: CHandle, row: i64, out_value: *mut c_int) -> c_int { - guard(|| unsafe { - if out_value.is_null() { - return Err(Error::Invalid); - } - with_stack(&stack, |s| { - *out_value = if s.command_is_done(row)? { 1 } else { 0 }; - Ok(()) - }) - }) -} diff --git a/crates/oakundo/tests/edge_coverage_test.rs b/crates/oakundo/tests/edge_coverage_test.rs index 27003d341..0f802d10e 100644 --- a/crates/oakundo/tests/edge_coverage_test.rs +++ b/crates/oakundo/tests/edge_coverage_test.rs @@ -14,25 +14,15 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -//! Edge-path coverage: error-code mapping, borrowed handles, refcount -//! symmetry, shell handles after `command_take`, `Default` impls, and the -//! `push_pre_executed` redo-tail/cap paths. Everything here goes through -//! the crate's public Rust API (the handle-level functions sunk from the -//! former C ABI layer). +//! Edge-path coverage: error-code mapping, `Default` impls, the +//! prepared flag, and the `push_pre_executed` redo-tail/cap paths. +//! Everything here goes through the crate's public value-typed API +//! (the former handle-level/refcount tests were removed together with +//! the `CHandle` layer they exercised). -use oakundo::error::{ - Error, OAKUNDO_E_FAILED, OAKUNDO_E_INVALID, OAKUNDO_E_NOMEM, OAKUNDO_E_NOT_FOUND, - OAKUNDO_E_STATE, -}; -use oakundo::handle::{make_borrowed, make_owned}; -use oakundo::undocommand::{ - command_free, command_init, command_init_multi, command_multi_child_count, command_redo_now, - MultiUndoCommand, OakUndoCommandVtable, UndoCommand, -}; -use oakundo::undostack::{ - undostack_can_redo, undostack_free, undostack_index, undostack_init, undostack_push, - undostack_push_pre_executed, undostack_undo, EmptyCommand, UndoStack, K_MAX_UNDO_COMMANDS, -}; +use oakundo::error::{Error, OAKUNDO_E_FAILED, OAKUNDO_E_INVALID, OAKUNDO_E_NOMEM, OAKUNDO_E_NOT_FOUND, OAKUNDO_E_STATE}; +use oakundo::undocommand::{MultiUndoCommand, UndoCommand}; +use oakundo::undostack::{EmptyCommand, UndoStack, K_MAX_UNDO_COMMANDS}; /// Every `Error` variant maps to its documented public code. #[test] @@ -44,40 +34,6 @@ fn error_code_mapping_is_complete() { assert_eq!(Error::NoMem.code(), OAKUNDO_E_NOMEM); } -/// Borrowed handles: addref/release only touch the shell, never the -/// pointee; NULL ctx is a no-op for both. -#[test] -fn borrowed_handle_refcounting() { - let mut value: i32 = 7; - let h = unsafe { make_borrowed(&mut value as *mut i32) }; - assert!(!h.is_null()); - - // NULL ctx is a no-op. - unsafe { h.addref.unwrap()(std::ptr::null_mut()) }; - unsafe { h.release.unwrap()(std::ptr::null_mut()) }; - - // addref then two releases: the pointee survives (still readable). - unsafe { h.addref.unwrap()(h.ctx) }; - unsafe { h.release.unwrap()(h.ctx) }; - assert_eq!(value, 7); - unsafe { h.release.unwrap()(h.ctx) }; - assert_eq!(value, 7); -} - -/// Owned handles: addref requires a matching extra release; releasing to -/// zero destroys the box exactly once. -#[test] -fn owned_handle_refcounting() { - let h = make_owned(String::from("owned")); - assert!(!h.is_null()); - unsafe { h.addref.unwrap()(h.ctx) }; - unsafe { h.release.unwrap()(h.ctx) }; - // Still alive (one reference left) and readable. - let view = unsafe { oakundo::handle::get::(&h) }; - assert_eq!(view.map(String::as_str), Some("owned")); - unsafe { h.release.unwrap()(h.ctx) }; -} - /// `Default` impls mirror `new()`. #[test] fn default_impls_match_new() { @@ -96,130 +52,51 @@ fn default_impls_match_new() { /// `set_prepared` is idempotent and `has_prepared` reflects it. #[test] fn prepared_flag_roundtrip() { - let vtable = OakUndoCommandVtable { - redo: None, - undo: None, - free_fn: None, - }; - let mut cmd = UndoCommand::from_vtable(vtable, std::ptr::null_mut()); + let mut cmd = UndoCommand::from_closures(|| {}, || {}); assert!(cmd.has_prepared()); cmd.set_prepared(); assert!(cmd.has_prepared()); } -/// A command handle whose value was taken by a stack push becomes a -/// non-owning shell: redo/undo on it are `E_INVALID`, and pushing it a -/// second time is `E_STATE`. -#[test] -fn taken_command_shell_is_inert() { - let vtable = OakUndoCommandVtable { - redo: None, - undo: None, - free_fn: None, - }; - let mut stack = undostack_init(); - let cmd = command_init(&vtable, std::ptr::null_mut()); - assert!(!cmd.ctx.is_null()); - - let name = c"once"; - assert_eq!(undostack_push(stack, cmd, name.as_ptr()), 0); - - // The shell no longer holds a command value. - assert_eq!(command_redo_now(cmd), OAKUNDO_E_INVALID); - // Taking the same box twice is a state error. - assert_eq!(undostack_push(stack, cmd, name.as_ptr()), OAKUNDO_E_STATE); - - let mut release = cmd; - command_free(&mut release); - undostack_free(&mut stack); -} - -/// `push_pre_executed` also drops the redoable tail and evicts the oldest -/// row past the cap (mirrors `push`). +/// `push_pre_executed` drops the redoable tail and evicts the oldest row +/// past the cap (mirrors `push`). #[test] fn push_pre_executed_clears_redo_tail_and_caps() { - let vtable = OakUndoCommandVtable { - redo: None, - undo: None, - free_fn: None, - }; - let mut stack = undostack_init(); - let name = c"row"; + let mut stack = UndoStack::new(); + let name = "row"; // Push two, undo one, then push_pre_executed: redo tail is dropped. for _ in 0..2 { - let cmd = command_init(&vtable, std::ptr::null_mut()); - assert_eq!(undostack_push(stack, cmd, name.as_ptr()), 0); - let mut shell = cmd; - command_free(&mut shell); + stack.push(UndoCommand::from_closures(|| {}, || {}), name); } - assert_eq!(undostack_undo(stack), 0); - let cmd = command_init(&vtable, std::ptr::null_mut()); - assert_eq!(undostack_push_pre_executed(stack, cmd, name.as_ptr()), 0); - let mut can_redo: i32 = 1; - assert_eq!(undostack_can_redo(stack, &mut can_redo), 0); - assert_eq!(can_redo, 0, "push_pre_executed drops the redoable tail"); + stack.undo().unwrap(); + stack.push_pre_executed(UndoCommand::from_closures(|| {}, || {}), name); + assert!(!stack.can_redo(), "push_pre_executed drops the redoable tail"); // Fill past the cap with pre-executed commands: the oldest rows are // evicted and the count stays at K_MAX_UNDO_COMMANDS. for _ in 0..(K_MAX_UNDO_COMMANDS + 10) { - let cmd = command_init(&vtable, std::ptr::null_mut()); - assert_eq!(undostack_push_pre_executed(stack, cmd, name.as_ptr()), 0); + stack.push_pre_executed(UndoCommand::from_closures(|| {}, || {}), name); } - let mut index: i64 = 0; - assert_eq!(undostack_index(stack, &mut index), 0); assert_eq!( - index, K_MAX_UNDO_COMMANDS as i64, + stack.done_count(), + K_MAX_UNDO_COMMANDS as i64, "pre-executed rows evict at the cap" ); - - undostack_free(&mut stack); } -/// `make_owned` on a stack mutex is what `undostack_init` uses; the -/// `CHandle` accessors tolerate empty handles. +/// A command value is moved, not copied: moving it into a stack leaves no +/// usable alias behind (the compiler enforces this; the assertion pins the +/// ownership semantics the old handle shells emulated). #[test] -fn empty_chandle_accessors() { - let h = oakundo::handle::CHandle::null(); - assert!(h.is_null()); - assert!(h.addref.is_none() && h.release.is_none()); - assert_eq!(h.abi_version, 0); - let view = unsafe { oakundo::handle::get::(&h) }; - assert!(view.is_none()); -} - -/// Command-box refcounting: addref/release tolerate NULL ctx, a bumped -/// refcount needs a matching release, and a taken multi shell reports -/// `E_INVALID` instead of dereferencing a null command pointer. -#[test] -fn command_box_refcounting_and_taken_multi_shell() { - let vtable = OakUndoCommandVtable { - redo: None, - undo: None, - free_fn: None, - }; - - let mut cmd = command_init(&vtable, std::ptr::null_mut()); - assert!(!cmd.ctx.is_null()); - - // NULL ctx is a no-op for both refcount callbacks. - unsafe { cmd.addref.unwrap()(std::ptr::null_mut()) }; - unsafe { cmd.release.unwrap()(std::ptr::null_mut()) }; - - // addref then two releases: destroyed exactly once at zero. - unsafe { cmd.addref.unwrap()(cmd.ctx) }; - unsafe { cmd.release.unwrap()(cmd.ctx) }; - unsafe { cmd.release.unwrap()(cmd.ctx) }; - cmd.ctx = std::ptr::null_mut(); - - // A multi command pushed into a stack is taken; the remaining shell - // must fail cleanly on child access. - let mut stack = undostack_init(); - let mut multi = command_init_multi(); - let name = c"m"; - assert_eq!(undostack_push(stack, multi, name.as_ptr()), 0); - let mut out: i32 = -1; - assert_eq!(command_multi_child_count(multi, &mut out), OAKUNDO_E_INVALID); - command_free(&mut multi); - undostack_free(&mut stack); +fn command_value_moves_into_stack() { + let mut stack = UndoStack::new(); + stack.push(UndoCommand::from_closures(|| {}, || {}), "A"); + stack.push(UndoCommand::from_closures(|| {}, || {}), "B"); + assert_eq!(stack.command_count(), 3); + assert!(stack.can_undo()); + stack.undo().unwrap(); + assert!(stack.can_redo()); + assert_eq!(stack.done_count(), 2); + assert_eq!(stack.command_name(2).unwrap(), "B"); // undone row still labeled } diff --git a/crates/oakundo/tests/ffi_contract_test.rs b/crates/oakundo/tests/ffi_contract_test.rs deleted file mode 100644 index 1660a5989..000000000 --- a/crates/oakundo/tests/ffi_contract_test.rs +++ /dev/null @@ -1,681 +0,0 @@ -// Oak Video Editor - Non-Linear Video Editor -// Copyright (C) 2026 Oak Team -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -//! Contract tests for the handle-level command/stack API (the functions -//! sunk from the former `ffi.rs`). Each function gets at least one success -//! path and one failure path; complex multi-command and stack behavior is -//! exercised as a matrix. The expected semantics are pinned by the C++ -//! module (`src/undo/src`, unchanged). - -use std::cell::RefCell; -use std::ffi::{c_char, c_int, c_void, CString}; - -use oakundo::error::{OAKUNDO_E_INVALID, OAKUNDO_E_NOT_FOUND, OAKUNDO_OK}; -use oakundo::handle::CHandle; -use oakundo::handle::OAKUNDO_ABI_VERSION; -use oakundo::undocommand::{ - command_free, command_init, command_init_multi, command_multi_add_child, command_multi_child, - command_multi_child_count, command_redo_now, command_undo_now, OakUndoCommandVtable, -}; -use oakundo::undostack::{ - undostack_can_redo, undostack_can_undo, undostack_clear, undostack_command_is_done, - undostack_command_text, undostack_count, undostack_free, undostack_index, undostack_init, - undostack_jump, undostack_push, undostack_push_pre_executed, undostack_redo, undostack_undo, -}; - -/// Shared event recorder driven through vtable callbacks. -struct Trace { - events: RefCell>, -} - -/// Per-command callback payload: a name and a shared trace. -struct Probe { - name: &'static str, - trace: *const Trace, -} - -unsafe extern "C" fn probe_redo(u: *mut c_void) { - let p = unsafe { &mut *(u as *mut Probe) }; - let trace = unsafe { &*p.trace }; - trace.events.borrow_mut().push(format!("redo:{}", p.name)); -} - -unsafe extern "C" fn probe_undo(u: *mut c_void) { - let p = unsafe { &mut *(u as *mut Probe) }; - let trace = unsafe { &*p.trace }; - trace.events.borrow_mut().push(format!("undo:{}", p.name)); -} - -unsafe extern "C" fn probe_free(u: *mut c_void) { - let p = unsafe { &mut *(u as *mut Probe) }; - let trace = unsafe { &*p.trace }; - trace.events.borrow_mut().push(format!("free:{}", p.name)); -} - -/// Snapshot of the recorded events, in order. -fn events(trace: &Trace) -> Vec { - trace.events.borrow().clone() -} - -/// A fresh trace plus three named probes (`a`, `b`, `c`) pointing at it. -fn setup() -> (Box, Vec) { - let trace = Box::new(Trace { - events: RefCell::new(Vec::new()), - }); - let ptr = &*trace as *const Trace; - let mut probes = Vec::new(); - for name in ["a", "b", "c"] { - probes.push(Probe { name, trace: ptr }); - } - (trace, probes) -} - -/// A vtable-backed command handle whose callbacks record into `probe`. -fn make_cmd(probe: *mut Probe) -> CHandle { - let vtable = OakUndoCommandVtable { - redo: Some(probe_redo), - undo: Some(probe_undo), - free_fn: Some(probe_free), - }; - command_init(&vtable, probe as *mut c_void) -} - -/// An empty (all-zero) command handle. -fn empty_cmd() -> CHandle { - CHandle { - ctx: std::ptr::null_mut(), - addref: None, - release: None, - abi_version: 0, - } -} - -/// An empty (all-zero) stack handle. -fn empty_stack() -> CHandle { - CHandle { - ctx: std::ptr::null_mut(), - addref: None, - release: None, - abi_version: 0, - } -} - -/// A fresh stack handle (refcount 1) for the calling test. -fn new_stack() -> CHandle { - undostack_init() -} - -// --------------------------------------------------------------------------- -// Skeleton contract tests (ffi_contract_test.rs) -// --------------------------------------------------------------------------- - -/// Command lifecycle: `init` returns a refcounted handle, `redo_now` marks -/// it done (a second `redo_now` is a no-op), `undo_now` un-done it, and -/// `free` runs `free_fn` once and clears `ctx`. -#[test] -fn command_lifecycle() { - let (trace, mut probes) = setup(); - let mut cmd = make_cmd(&mut probes[0] as *mut Probe); - - assert!(!cmd.ctx.is_null()); - assert_eq!(cmd.abi_version, OAKUNDO_ABI_VERSION); - assert!(cmd.addref.is_some() && cmd.release.is_some()); - - assert_eq!(command_redo_now(cmd), OAKUNDO_OK); - assert_eq!(events(&trace), vec!["redo:a"]); - - // Idempotent redo. - assert_eq!(command_redo_now(cmd), OAKUNDO_OK); - assert_eq!(events(&trace), vec!["redo:a"]); - - assert_eq!(command_undo_now(cmd), OAKUNDO_OK); - assert_eq!(events(&trace), vec!["redo:a", "undo:a"]); - - // Idempotent undo. - assert_eq!(command_undo_now(cmd), OAKUNDO_OK); - assert_eq!(events(&trace), vec!["redo:a", "undo:a"]); - - // free destroys once: free_fn runs exactly once and ctx is cleared. - command_free(&mut cmd); - assert_eq!(events(&trace), vec!["redo:a", "undo:a", "free:a"]); - assert!(cmd.ctx.is_null()); -} - -/// Multi command: `add_child` → `child_count` reflects it; redo runs -/// children in order, undo in reverse order. -#[test] -fn multi_redo_undo_ordering() { - let (trace, mut probes) = setup(); - let mut multi = command_init_multi(); - - // add children a, b, c. - let ca = make_cmd(&mut probes[0] as *mut Probe); - let cb = make_cmd(&mut probes[1] as *mut Probe); - let cc = make_cmd(&mut probes[2] as *mut Probe); - assert_eq!(command_multi_add_child(multi, ca), OAKUNDO_OK); - assert_eq!(command_multi_add_child(multi, cb), OAKUNDO_OK); - assert_eq!(command_multi_add_child(multi, cc), OAKUNDO_OK); - - // child_count reflects three. - let mut count: c_int = 0; - assert_eq!(command_multi_child_count(multi, &mut count), OAKUNDO_OK); - assert_eq!(count, 3); - - // redo fires in insertion order. - assert_eq!(command_redo_now(multi), OAKUNDO_OK); - assert_eq!(events(&trace), vec!["redo:a", "redo:b", "redo:c"]); - - // undo fires in reverse order. - assert_eq!(command_undo_now(multi), OAKUNDO_OK); - assert_eq!( - events(&trace), - vec!["redo:a", "redo:b", "redo:c", "undo:c", "undo:b", "undo:a"] - ); - - // Borrowed child handles are released harmlessly before the multi dies. - let mut child0 = empty_cmd(); - assert_eq!(command_multi_child(multi, 0, &mut child0), OAKUNDO_OK); - assert!(!child0.ctx.is_null()); - command_free(&mut child0); - - // free the multi: children are freed (shell only) without double-free. - let mut multi_owned = multi; - command_free(&mut multi_owned); - assert_eq!( - events(&trace) - .iter() - .filter(|e| e.starts_with("free:")) - .count(), - 3 - ); -} - -/// Stack: fresh stack has one empty "New/Open Project" command, so -/// `can_undo` is 0 and `count` is 1; pushing redoable commands grows -/// `count` and makes `can_undo`/`can_redo` track the position. -#[test] -fn stack_push_undo_redo_queries() { - let (trace, mut probes) = setup(); - let mut stack = new_stack(); - - let mut count: i64 = 0; - let mut value: c_int = 0; - assert_eq!(undostack_count(stack, &mut count), OAKUNDO_OK); - assert_eq!(count, 1); - assert_eq!(undostack_can_undo(stack, &mut value), OAKUNDO_OK); - assert_eq!(value, 0); - assert_eq!(undostack_can_redo(stack, &mut value), OAKUNDO_OK); - assert_eq!(value, 0); - - // Push two commands. - let name_a = CString::new("A").unwrap(); - let mut ca = make_cmd(&mut probes[0] as *mut Probe); - assert_eq!(undostack_push(stack, ca, name_a.as_ptr()), OAKUNDO_OK); - command_free(&mut ca); // non-owning shell now - let name_b = CString::new("B").unwrap(); - let mut cb = make_cmd(&mut probes[1] as *mut Probe); - assert_eq!(undostack_push(stack, cb, name_b.as_ptr()), OAKUNDO_OK); - command_free(&mut cb); - - assert_eq!(events(&trace), vec!["redo:a", "redo:b"]); - assert_eq!(undostack_count(stack, &mut count), OAKUNDO_OK); - assert_eq!(count, 3); - assert_eq!(undostack_can_undo(stack, &mut value), OAKUNDO_OK); - assert_eq!(value, 1); - assert_eq!(undostack_can_redo(stack, &mut value), OAKUNDO_OK); - assert_eq!(value, 0); - - // Undo moves B into the redoable tail. - assert_eq!(undostack_undo(stack), OAKUNDO_OK); - assert_eq!(events(&trace), vec!["redo:a", "redo:b", "undo:b"]); - assert_eq!(undostack_can_undo(stack, &mut value), OAKUNDO_OK); - assert_eq!(value, 1); - assert_eq!(undostack_can_redo(stack, &mut value), OAKUNDO_OK); - assert_eq!(value, 1); - - // Redo restores. - assert_eq!(undostack_redo(stack), OAKUNDO_OK); - assert_eq!(events(&trace), vec!["redo:a", "redo:b", "undo:b", "redo:b"]); - assert_eq!(undostack_can_redo(stack, &mut value), OAKUNDO_OK); - assert_eq!(value, 0); - - undostack_free(&mut stack); -} - -/// `can_redo`/`index` after undo and redo; `jump(0)` clamps to the -/// bottom empty command without spinning; `jump` beyond the top is a no-op. -#[test] -fn stack_jump_clamps() { - let (trace, mut probes) = setup(); - let mut stack = new_stack(); - - let na = CString::new("A").unwrap(); - let nb = CString::new("B").unwrap(); - let nc = CString::new("C").unwrap(); - let mut ca = make_cmd(&mut probes[0] as *mut Probe); - undostack_push(stack, ca, na.as_ptr()); - let mut cb = make_cmd(&mut probes[1] as *mut Probe); - undostack_push(stack, cb, nb.as_ptr()); - let mut cc = make_cmd(&mut probes[2] as *mut Probe); - undostack_push(stack, cc, nc.as_ptr()); - - let mut index: i64 = 0; - assert_eq!(undostack_index(stack, &mut index), OAKUNDO_OK); - assert_eq!(index, 4); - - // jump to the bottom (0): undo all three; negative clamps to 0 too. - assert_eq!(undostack_jump(stack, 0), OAKUNDO_OK); - assert_eq!(undostack_index(stack, &mut index), OAKUNDO_OK); - assert_eq!(index, 1); - assert_eq!( - events(&trace), - vec!["redo:a", "redo:b", "redo:c", "undo:c", "undo:b", "undo:a"] - ); - - // jump back up to 3: redo a and b (c was already undone to reach 1). - assert_eq!(undostack_jump(stack, 3), OAKUNDO_OK); - assert_eq!(undostack_index(stack, &mut index), OAKUNDO_OK); - assert_eq!(index, 3); - - // jump beyond the top redoes up to the top (index 4), matching the C++ - // `jump` (undone commands are redoable, so the second loop runs). - assert_eq!(undostack_jump(stack, 100), OAKUNDO_OK); - assert_eq!(undostack_index(stack, &mut index), OAKUNDO_OK); - assert_eq!(index, 4); - - // Negative index clamps to the bottom. - assert_eq!(undostack_jump(stack, -5), OAKUNDO_OK); - assert_eq!(undostack_index(stack, &mut index), OAKUNDO_OK); - assert_eq!(index, 1); - - undostack_free(&mut stack); -} - -/// `push_pre_executed` records without redoing (stays undoable); empty -/// multi commands are discarded on push. -#[test] -fn stack_pre_executed_and_empty_multi() { - let (trace, mut probes) = setup(); - let mut stack = new_stack(); - - // push_pre_executed: no redo callback. - let name = CString::new("Pre").unwrap(); - let mut cp = make_cmd(&mut probes[0] as *mut Probe); - assert_eq!(undostack_push_pre_executed(stack, cp, name.as_ptr()), OAKUNDO_OK); - command_free(&mut cp); - assert_eq!(events(&trace), Vec::::new()); - - // The pre-executed command is recorded as done (undoable). - let mut done: c_int = 0; - assert_eq!(undostack_command_is_done(stack, 1, &mut done), OAKUNDO_OK); - assert_eq!(done, 1); - - let mut count: i64 = 0; - assert_eq!(undostack_count(stack, &mut count), OAKUNDO_OK); - assert_eq!(count, 2); - - // Undoing the pre-executed command still runs its undo callback. - assert_eq!(undostack_undo(stack), OAKUNDO_OK); - assert_eq!(events(&trace), vec!["undo:a"]); - - // Empty multi command is discarded on push: count stays 2 (bottom - // "New/Open Project" + the undone pre-executed command), not 3. - let mut multi = command_init_multi(); - let name2 = CString::new("Empty").unwrap(); - assert_eq!(undostack_push(stack, multi, name2.as_ptr()), OAKUNDO_OK); - command_free(&mut multi); - assert_eq!(undostack_count(stack, &mut count), OAKUNDO_OK); - assert_eq!(count, 2, "empty multi is discarded on push"); - - undostack_free(&mut stack); -} - -/// Every handle-returning export returns `ctx == NULL` on failure and a -/// valid handle (`abi_version` stamped) on success; `free(NULL)` / -/// `free(empty)` are no-ops across command and stack families. -#[test] -fn handle_and_free_contract() { - let (_trace, mut probes) = setup(); - - // init with a NULL vtable → empty handle. - let empty = command_init(std::ptr::null(), std::ptr::null_mut()); - assert!(empty.ctx.is_null()); - assert_eq!(empty.abi_version, 0); - assert!(empty.addref.is_none() && empty.release.is_none()); - - // init with a valid vtable → stamped handle. - let mut cmd = make_cmd(&mut probes[0] as *mut Probe); - assert!(!cmd.ctx.is_null()); - assert_eq!(cmd.abi_version, OAKUNDO_ABI_VERSION); - - // init_multi → stamped handle. - let mut multi = command_init_multi(); - assert!(!multi.ctx.is_null()); - assert_eq!(multi.abi_version, OAKUNDO_ABI_VERSION); - - // init stack → stamped handle. - let mut stack = new_stack(); - assert!(!stack.ctx.is_null()); - assert_eq!(stack.abi_version, OAKUNDO_ABI_VERSION); - - // free(NULL) is a no-op for both families. - command_free(std::ptr::null_mut()); - undostack_free(std::ptr::null_mut()); - - // free(empty handle value) is a no-op. - let mut ecmd = empty_cmd(); - command_free(&mut ecmd); - assert!(ecmd.ctx.is_null()); - let mut estack = empty_stack(); - undostack_free(&mut estack); - assert!(estack.ctx.is_null()); - - // free(valid) clears ctx. - command_free(&mut multi); - assert!(multi.ctx.is_null()); - command_free(&mut cmd); - assert!(cmd.ctx.is_null()); - undostack_free(&mut stack); - assert!(stack.ctx.is_null()); -} - -// --------------------------------------------------------------------------- -// Exhaustive per-export success/failure coverage -// --------------------------------------------------------------------------- - -#[test] -fn command_redo_undo_now_null_is_invalid() { - assert_eq!(command_redo_now(empty_cmd()), OAKUNDO_E_INVALID); - assert_eq!(command_undo_now(empty_cmd()), OAKUNDO_E_INVALID); -} - -#[test] -fn command_multi_add_child_errors() { - let (_trace, mut probes) = setup(); - let mut multi = command_init_multi(); - let mut child = make_cmd(&mut probes[0] as *mut Probe); - let mut vtable_cmd = make_cmd(&mut probes[1] as *mut Probe); - - // multi is null → E_INVALID. - assert_eq!(command_multi_add_child(empty_cmd(), child), OAKUNDO_E_INVALID); - // child is null → E_INVALID (nothing taken). - assert_eq!(command_multi_add_child(multi, empty_cmd()), OAKUNDO_E_INVALID); - // target is a vtable command, not a multi → E_INVALID. - assert_eq!(command_multi_add_child(vtable_cmd, child), OAKUNDO_E_INVALID); - - // The child handle still owns its value (never taken), so free it. - command_free(&mut child); - command_free(&mut vtable_cmd); - command_free(&mut multi); -} - -#[test] -fn command_multi_child_count_errors() { - let (_trace, mut probes) = setup(); - let mut multi = command_init_multi(); - let mut vtable_cmd = make_cmd(&mut probes[0] as *mut Probe); - let mut out: c_int = -1; - - // null out pointer → E_INVALID. - assert_eq!(command_multi_child_count(multi, std::ptr::null_mut()), OAKUNDO_E_INVALID); - // null multi → E_INVALID. - assert_eq!(command_multi_child_count(empty_cmd(), &mut out), OAKUNDO_E_INVALID); - // non-multi command → E_INVALID. - assert_eq!(command_multi_child_count(vtable_cmd, &mut out), OAKUNDO_E_INVALID); - - command_free(&mut vtable_cmd); - command_free(&mut multi); -} - -#[test] -fn command_multi_child_errors() { - let (_trace, mut probes) = setup(); - let mut multi = command_init_multi(); - let mut vtable_cmd = make_cmd(&mut probes[0] as *mut Probe); - let mut out = empty_cmd(); - - // null out pointer → E_INVALID. - assert_eq!(command_multi_child(multi, 0, std::ptr::null_mut()), OAKUNDO_E_INVALID); - // null multi → E_INVALID. - assert_eq!(command_multi_child(empty_cmd(), 0, &mut out), OAKUNDO_E_INVALID); - // non-multi command → E_INVALID. - assert_eq!(command_multi_child(vtable_cmd, 0, &mut out), OAKUNDO_E_INVALID); - // empty multi, negative index → E_NOT_FOUND. - assert_eq!(command_multi_child(multi, -1, &mut out), OAKUNDO_E_NOT_FOUND); - // empty multi, positive OOB → E_NOT_FOUND. - assert_eq!(command_multi_child(multi, 5, &mut out), OAKUNDO_E_NOT_FOUND); - - command_free(&mut vtable_cmd); - command_free(&mut multi); -} - -#[test] -fn command_multi_child_success_borrowed() { - let (trace, mut probes) = setup(); - let mut multi = command_init_multi(); - let mut ca = make_cmd(&mut probes[0] as *mut Probe); - command_multi_add_child(multi, ca); - - let mut child = empty_cmd(); - assert_eq!(command_multi_child(multi, 0, &mut child), OAKUNDO_OK); - assert!(!child.ctx.is_null()); - // A borrowed child is not independently owned: freeing it frees only the - // shell and must not free the child the multi still owns. - command_free(&mut child); - assert!(child.ctx.is_null()); - - // Redo through the multi still works and no child was freed. - assert_eq!(command_redo_now(multi), OAKUNDO_OK); - assert_eq!(events(&trace), vec!["redo:a"]); - - command_free(&mut multi); - assert_eq!(events(&trace), vec!["redo:a", "free:a"]); -} - -#[test] -fn command_free_fires_exactly_once() { - let (trace, mut probes) = setup(); - let mut cmd = make_cmd(&mut probes[0] as *mut Probe); - - // free → one free callback. - command_free(&mut cmd); - assert_eq!(events(&trace), vec!["free:a"]); - - // free again on a cleared handle → no-op. - command_free(&mut cmd); - assert_eq!(events(&trace), vec!["free:a"]); -} - -#[test] -fn undostack_free_null_and_empty() { - let mut stack = empty_stack(); - undostack_free(std::ptr::null_mut()); - undostack_free(&mut stack); - assert!(stack.ctx.is_null()); -} - -#[test] -fn undostack_push_errors() { - let (trace, mut probes) = setup(); - let mut stack = new_stack(); - let mut cmd = make_cmd(&mut probes[0] as *mut Probe); - let name = CString::new("A").unwrap(); - - // null command → E_INVALID; stack untouched, command still owned. - assert_eq!(undostack_push(stack, empty_cmd(), name.as_ptr()), OAKUNDO_E_INVALID); - // empty stack → E_INVALID; command NOT drained (still owns its value). - assert_eq!(undostack_push(empty_stack(), cmd, name.as_ptr()), OAKUNDO_E_INVALID); - assert_eq!( - events(&trace), - Vec::::new(), - "no callbacks ran on failed push" - ); - - // NULL name is accepted as an empty label → OK. - assert_eq!(undostack_push(stack, cmd, std::ptr::null()), OAKUNDO_OK); - - command_free(&mut cmd); // now a non-owning shell - undostack_free(&mut stack); -} - -#[test] -fn undostack_push_pre_executed_errors() { - let (_trace, mut probes) = setup(); - let mut stack = new_stack(); - let mut cmd = make_cmd(&mut probes[0] as *mut Probe); - let name = CString::new("A").unwrap(); - - assert_eq!( - undostack_push_pre_executed(empty_stack(), cmd, name.as_ptr()), - OAKUNDO_E_INVALID - ); - // command was not drained; still owned, so free it. - command_free(&mut cmd); - assert_eq!( - undostack_push_pre_executed(stack, empty_cmd(), name.as_ptr()), - OAKUNDO_E_INVALID - ); - - undostack_free(&mut stack); -} - -#[test] -fn undostack_undo_redo_errors() { - let mut stack = new_stack(); - assert_eq!(undostack_undo(empty_stack()), OAKUNDO_E_INVALID); - assert_eq!(undostack_redo(empty_stack()), OAKUNDO_E_INVALID); - // Valid stack, but nothing to undo: still OK (no-op). - assert_eq!(undostack_undo(stack), OAKUNDO_OK); - assert_eq!(undostack_redo(stack), OAKUNDO_OK); - undostack_free(&mut stack); -} - -#[test] -fn undostack_jump_clear_errors() { - let mut stack = new_stack(); - assert_eq!(undostack_jump(empty_stack(), 3), OAKUNDO_E_INVALID); - assert_eq!(undostack_clear(empty_stack()), OAKUNDO_E_INVALID); - - // Clear on a valid stack resets to the empty bottom command. - let mut count: i64 = 0; - undostack_clear(stack); - assert_eq!(undostack_count(stack, &mut count), OAKUNDO_OK); - assert_eq!(count, 1); - undostack_free(&mut stack); -} - -#[test] -fn undostack_can_undo_redo_errors() { - let mut stack = new_stack(); - let mut value: c_int = 0; - assert_eq!(undostack_can_undo(empty_stack(), &mut value), OAKUNDO_E_INVALID); - assert_eq!(undostack_can_redo(empty_stack(), &mut value), OAKUNDO_E_INVALID); - assert_eq!(undostack_can_undo(stack, std::ptr::null_mut()), OAKUNDO_E_INVALID); - assert_eq!(undostack_can_redo(stack, std::ptr::null_mut()), OAKUNDO_E_INVALID); - undostack_free(&mut stack); -} - -#[test] -fn undostack_count_index_errors() { - let mut stack = new_stack(); - let mut out: i64 = 0; - assert_eq!(undostack_count(empty_stack(), &mut out), OAKUNDO_E_INVALID); - assert_eq!(undostack_index(empty_stack(), &mut out), OAKUNDO_E_INVALID); - assert_eq!(undostack_count(stack, std::ptr::null_mut()), OAKUNDO_E_INVALID); - assert_eq!(undostack_index(stack, std::ptr::null_mut()), OAKUNDO_E_INVALID); - undostack_free(&mut stack); -} - -#[test] -fn undostack_command_text_two_stage_and_errors() { - let (_trace, mut probes) = setup(); - let mut stack = new_stack(); - let name = CString::new("MyAction").unwrap(); - let mut cmd = make_cmd(&mut probes[0] as *mut Probe); - undostack_push(stack, cmd, name.as_ptr()); - - // Failure: empty stack → E_INVALID. - assert_eq!(undostack_command_text(empty_stack(), 1, std::ptr::null_mut(), 0), OAKUNDO_E_INVALID); - // Failure: OOB row (positive) → E_NOT_FOUND. - assert_eq!(undostack_command_text(stack, 5, std::ptr::null_mut(), 0), OAKUNDO_E_NOT_FOUND); - // Failure: negative row → E_NOT_FOUND. - assert_eq!(undostack_command_text(stack, -1, std::ptr::null_mut(), 0), OAKUNDO_E_NOT_FOUND); - - // Stage one: null buffer returns the required size ("MyAction" + NUL). - let required = undostack_command_text(stack, 1, std::ptr::null_mut(), 0); - assert_eq!(required, "MyAction".len() as c_int + 1); - - // Stage two: a buffer of that size is populated with a NUL-terminated - // string and the required size is returned again. - let mut buf = vec![0u8; required as usize]; - let ret = undostack_command_text(stack, 1, buf.as_mut_ptr() as *mut c_char, required); - assert_eq!(ret, required); - let actual = unsafe { std::ffi::CStr::from_ptr(buf.as_ptr() as *const c_char) }; - assert_eq!(actual.to_bytes(), b"MyAction"); - - // A too-small buffer is safely truncated (NUL-terminated). - let mut small = vec![0xffu8; 3]; - undostack_command_text(stack, 1, small.as_mut_ptr() as *mut c_char, 3); - assert_eq!(small, [b'M', b'y', 0]); - - command_free(&mut cmd); - undostack_free(&mut stack); -} - -#[test] -fn undostack_command_is_done_errors() { - let mut stack = new_stack(); - let mut value: c_int = 0; - assert_eq!(undostack_command_is_done(empty_stack(), 0, &mut value), OAKUNDO_E_INVALID); - assert_eq!(undostack_command_is_done(stack, 0, std::ptr::null_mut()), OAKUNDO_E_INVALID); - assert_eq!(undostack_command_is_done(stack, 5, &mut value), OAKUNDO_E_NOT_FOUND); - assert_eq!(undostack_command_is_done(stack, -1, &mut value), OAKUNDO_E_NOT_FOUND); - undostack_free(&mut stack); -} - -#[test] -fn undostack_push_drops_redoable_tail() { - let (trace, mut probes) = setup(); - let mut stack = new_stack(); - let na = CString::new("A").unwrap(); - let nb = CString::new("B").unwrap(); - let mut ca = make_cmd(&mut probes[0] as *mut Probe); - undostack_push(stack, ca, na.as_ptr()); - let mut cb = make_cmd(&mut probes[1] as *mut Probe); - undostack_push(stack, cb, nb.as_ptr()); - - // Undo B, then push C → the redoable tail is dropped. - assert_eq!(undostack_undo(stack), OAKUNDO_OK); - let mut value: c_int = 0; - assert_eq!(undostack_can_redo(stack, &mut value), OAKUNDO_OK); - assert_eq!(value, 1); - - let nc = CString::new("C").unwrap(); - let mut cc = make_cmd(&mut probes[2] as *mut Probe); - assert_eq!(undostack_push(stack, cc, nc.as_ptr()), OAKUNDO_OK); - - assert_eq!(undostack_can_redo(stack, &mut value), OAKUNDO_OK); - assert_eq!(value, 0, "pushing drops the redoable tail"); - assert_eq!( - events(&trace), - vec!["redo:a", "redo:b", "undo:b", "free:b", "redo:c"] - ); - - command_free(&mut ca); - command_free(&mut cb); - command_free(&mut cc); - undostack_free(&mut stack); -} diff --git a/crates/oakundo/tests/handle_test.rs b/crates/oakundo/tests/handle_test.rs deleted file mode 100644 index 4c31b5682..000000000 --- a/crates/oakundo/tests/handle_test.rs +++ /dev/null @@ -1,177 +0,0 @@ -// Oak Video Editor - Non-Linear Video Editor -// Copyright (C) 2026 Oak Team -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -//! Tests for the refcounted-handle scaffolding (`handle.rs`). - -use std::ffi::c_void; -use std::sync::atomic::{AtomicI32, Ordering}; -use std::sync::Arc; - -use oakundo::error::{Result, OAKUNDO_E_FAILED, OAKUNDO_E_INVALID, OAKUNDO_OK}; -use oakundo::handle::{get, guard, guard_handle, guard_void, make_borrowed, make_owned, CHandle}; - -/// A value whose `Drop` signals through a shared counter. -struct DropProbe { - counter: Arc, -} - -impl DropProbe { - fn new() -> (Self, Arc) { - let counter = Arc::new(AtomicI32::new(1)); - ( - DropProbe { - counter: counter.clone(), - }, - counter, - ) - } -} - -impl Drop for DropProbe { - fn drop(&mut self) { - self.counter.fetch_sub(1, Ordering::SeqCst); - } -} - -#[test] -fn null_handle_is_all_zero() { - let h = CHandle::null(); - assert!(h.is_null()); - assert!(h.ctx.is_null()); - assert!(h.addref.is_none()); - assert!(h.release.is_none()); - assert_eq!(h.abi_version, 0); -} - -#[test] -fn make_owned_counts_and_releases() { - let (value, counter) = DropProbe::new(); - let h = make_owned(value); - - assert!(!h.is_null()); - assert_eq!(h.abi_version, oakundo::handle::OAKUNDO_ABI_VERSION); - - // addref bumps the count; a second release does not double-drop. - let addref = h.addref.unwrap(); - let release = h.release.unwrap(); - unsafe { addref(h.ctx) }; - unsafe { release(h.ctx) }; - assert_eq!( - counter.load(Ordering::SeqCst), - 1, - "still alive after one release" - ); - - // Final release drops the box. - unsafe { release(h.ctx) }; - assert_eq!( - counter.load(Ordering::SeqCst), - 0, - "dropped at refcount zero" - ); -} - -#[test] -fn make_owned_get_round_trips() { - let h = make_owned(42i32); - let ch = &h; - let v = unsafe { get::(ch) }; - assert_eq!(v, Some(&42)); - - let empty = CHandle::null(); - assert_eq!(unsafe { get::(&empty) }, None); -} - -#[test] -fn borrowed_release_does_not_free_pointee() { - let (mut value, counter) = DropProbe::new(); - let b = unsafe { make_borrowed(&mut value as *mut DropProbe) }; - - let release = b.release.unwrap(); - unsafe { release(b.ctx) }; - assert_eq!( - counter.load(Ordering::SeqCst), - 1, - "borrowed release frees only the shell" - ); - - // The owned value is still usable and is dropped normally. - drop(value); - assert_eq!(counter.load(Ordering::SeqCst), 0); -} - -#[test] -fn guard_success_error_and_panic() { - fn ok() -> Result<()> { - Ok(()) - } - fn invalid() -> Result<()> { - Err(oakundo::error::Error::Invalid) - } - fn panic() -> Result<()> { - panic!("boom") - } - assert_eq!(guard(ok), OAKUNDO_OK); - assert_eq!(guard(invalid), OAKUNDO_E_INVALID); - assert_eq!(guard(panic), OAKUNDO_E_FAILED); -} - -#[test] -fn guard_handle_returns_value_or_null() { - let h = guard_handle(|| Ok(make_owned(5i32))); - assert!(!h.is_null()); - assert_eq!(h.abi_version, oakundo::handle::OAKUNDO_ABI_VERSION); - - let on_err = guard_handle(|| Err(oakundo::error::Error::NotFound)); - assert!(on_err.is_null()); - - let on_panic = guard_handle(|| -> Result { panic!("boom") }); - assert!(on_panic.is_null()); -} - -#[test] -fn guard_void_swallows_panic() { - let mut ran = false; - guard_void(|| ran = true); - assert!(ran); - - guard_void(|| panic!("swallowed")); -} - -#[test] -fn addref_release_null_ctx_is_noop() { - let h = make_owned(7i32); - unsafe { (h.addref.unwrap())(std::ptr::null_mut()) }; - unsafe { (h.release.unwrap())(std::ptr::null_mut()) }; - assert!(!h.is_null()); -} - -#[test] -fn borrowed_handle_binds_to_correct_type() { - // The borrowed handle boxes a `*mut T`; `get` for that type sees the - // raw pointer value. - let mut value = 99u64; - let h = unsafe { make_borrowed(&mut value as *mut u64) }; - let ch = &h; - let p = unsafe { get::<*mut u64>(ch) }.unwrap(); - assert_eq!(unsafe { **p }, 99); -} - -/// `*mut c_void` helper used to drive callbacks directly in one test. -#[allow(dead_code)] -fn _as_void(p: *mut T) -> *mut c_void { - p as *mut c_void -} diff --git a/crates/oakundo/tests/undo_safe_test.rs b/crates/oakundo/tests/undo_safe_test.rs index 8568c2c73..ad6eb8ff0 100644 --- a/crates/oakundo/tests/undo_safe_test.rs +++ b/crates/oakundo/tests/undo_safe_test.rs @@ -17,66 +17,52 @@ //! Safe-layer behavior matrix for `UndoCommand` / `UndoStack` //! (mirrors `src/undo/src/undostack.cpp` / `undocommand.cpp`). -use std::cell::RefCell; -use std::ffi::c_void; +use std::sync::{Arc, Mutex}; use oakundo::error::Error; -use oakundo::undocommand::{OakUndoCommandVtable, UndoCommand}; +use oakundo::undocommand::UndoCommand; use oakundo::undostack::{EmptyCommand, UndoStack, K_MAX_UNDO_COMMANDS}; -/// Shared event recorder driven through vtable callbacks. -struct Trace { - events: RefCell>, +/// Shared event recorder; commands record `redo:` / `undo:` +/// through their closures. +type Trace = Arc>>; + +fn new_trace() -> Trace { + Arc::new(Mutex::new(Vec::new())) } -/// Per-command callback payload: a name and a shared trace. -struct Probe { - name: &'static str, - trace: *const Trace, -} - -unsafe extern "C" fn probe_redo(u: *mut c_void) { - let p = unsafe { &mut *(u as *mut Probe) }; - let trace = unsafe { &*p.trace }; - trace.events.borrow_mut().push(format!("redo:{}", p.name)); -} - -unsafe extern "C" fn probe_undo(u: *mut c_void) { - let p = unsafe { &mut *(u as *mut Probe) }; - let trace = unsafe { &*p.trace }; - trace.events.borrow_mut().push(format!("undo:{}", p.name)); -} - -/// A vtable-backed command whose callbacks record into `probe`. -fn trace_cmd(probe: &mut Probe) -> UndoCommand { - let vtable = OakUndoCommandVtable { - redo: Some(probe_redo), - undo: Some(probe_undo), - free_fn: None, - }; - UndoCommand::from_vtable(vtable, probe as *mut Probe as *mut c_void) +/// A closure-backed command whose redo/undo record into `trace`. +fn trace_cmd(name: &'static str, trace: &Trace) -> UndoCommand { + let (t_redo, t_undo) = (trace.clone(), trace.clone()); + let (name_redo, name_undo) = (name.to_string(), name.to_string()); + UndoCommand::from_closures( + move || t_redo.lock().unwrap().push(format!("redo:{name_redo}")), + move || t_undo.lock().unwrap().push(format!("undo:{name_undo}")), + ) } fn events(trace: &Trace) -> Vec { - trace.events.borrow().clone() + trace.lock().unwrap().clone() } -fn setup() -> (Box, Vec) { - let trace = Box::new(Trace { - events: RefCell::new(Vec::new()), - }); - let ptr = &*trace as *const Trace; - let mut probes = Vec::new(); - for name in ["a", "b", "c"] { - probes.push(Probe { name, trace: ptr }); +/// A `Drop` guard that records `free:` exactly once — the new-API +/// analogue of the C-ABI `free_fn` callback (dropping a command drops its +/// boxed closures, which drop their captures). +struct FreeProbe { + name: &'static str, + trace: Trace, +} + +impl Drop for FreeProbe { + fn drop(&mut self) { + self.trace.lock().unwrap().push(format!("free:{}", self.name)); } - (trace, probes) } #[test] fn command_redo_undo_lifecycle() { - let (trace, mut probes) = setup(); - let mut cmd = trace_cmd(&mut probes[0]); + let trace = new_trace(); + let mut cmd = trace_cmd("a", &trace); assert!(!cmd.is_done()); assert!(cmd.has_prepared()); @@ -104,17 +90,32 @@ fn command_redo_undo_lifecycle() { assert_eq!(events(&trace), vec!["redo:a", "undo:a", "redo:a", "undo:a"]); // set_done marks executed without running anything. - let mut probe = Probe { - name: "x", - trace: probes[0].trace, - }; - let mut cmd2 = trace_cmd(&mut probe); + let mut cmd2 = trace_cmd("x", &trace); cmd2.set_done(true); assert!(cmd2.is_done()); cmd2.redo_now(); // no-op (already done) assert_eq!(events(&trace), vec!["redo:a", "undo:a", "redo:a", "undo:a"]); } +/// Dropping a command runs its destruction exactly once (the C-ABI +/// `free_fn` exactly-once contract, now via `Drop`). +#[test] +fn command_drop_frees_exactly_once() { + let trace = new_trace(); + let probe = FreeProbe { + name: "a", + trace: trace.clone(), + }; + let cmd = UndoCommand::from_closures(move || {}, move || drop(&probe)); + + drop(cmd); + assert_eq!(events(&trace), vec!["free:a"]); + + // Re-dropping an already-dropped value is impossible by construction; + // a second drop of a clone of the event log stays empty. + assert_eq!(events(&trace), vec!["free:a"]); +} + #[test] fn empty_command_is_noop() { let mut cmd = EmptyCommand::new(); @@ -128,11 +129,11 @@ fn empty_command_is_noop() { #[test] fn multi_redo_undo_ordering() { - let (trace, mut probes) = setup(); + let trace = new_trace(); let mut multi = UndoCommand::multi(); - multi.multi_add_child(trace_cmd(&mut probes[0])); - multi.multi_add_child(trace_cmd(&mut probes[1])); - multi.multi_add_child(trace_cmd(&mut probes[2])); + multi.multi_add_child(trace_cmd("a", &trace)); + multi.multi_add_child(trace_cmd("b", &trace)); + multi.multi_add_child(trace_cmd("c", &trace)); assert_eq!(multi.multi_child_count(), 3); // Children are reachable and named. @@ -154,18 +155,19 @@ fn multi_redo_undo_ordering() { #[test] #[should_panic] -fn multi_add_child_on_vtable_panics() { - let (_trace, mut probes) = setup(); - let mut vtable_cmd = trace_cmd(&mut probes[0]); - vtable_cmd.multi_add_child(trace_cmd(&mut probes[1])); +fn multi_add_child_on_plain_command_panics() { + let trace = new_trace(); + let mut cmd = trace_cmd("a", &trace); + cmd.multi_add_child(trace_cmd("b", &trace)); } #[test] -fn multi_child_helpers_on_non_multi() { - let (_trace, mut probes) = setup(); - let mut vtable_cmd = trace_cmd(&mut probes[0]); - assert_eq!(vtable_cmd.multi_child_count(), 0); - assert!(matches!(vtable_cmd.multi_child(0), Err(Error::Invalid))); +fn multi_child_helpers_on_plain_command() { + let trace = new_trace(); + let mut cmd = trace_cmd("a", &trace); + assert_eq!(cmd.multi_child_count(), 0); + assert!(matches!(cmd.multi_child(0), Err(Error::Invalid))); + assert!(matches!(cmd.multi_child_mut(0), Err(Error::Invalid))); } #[test] @@ -181,11 +183,11 @@ fn stack_new_has_empty_bottom() { #[test] fn stack_push_undo_redo_branch() { - let (trace, mut probes) = setup(); + let trace = new_trace(); let mut s = UndoStack::new(); - s.push(trace_cmd(&mut probes[0]), "A"); - s.push(trace_cmd(&mut probes[1]), "B"); + s.push(trace_cmd("a", &trace), "A"); + s.push(trace_cmd("b", &trace), "B"); assert_eq!(s.command_count(), 3); assert!(s.can_undo()); assert!(!s.can_redo()); @@ -207,7 +209,7 @@ fn stack_push_undo_redo_branch() { // Undo then push drops the redoable tail. s.undo().unwrap(); assert!(s.can_redo()); - s.push(trace_cmd(&mut probes[2]), "C"); + s.push(trace_cmd("c", &trace), "C"); assert!(!s.can_redo(), "pushing drops redoable tail"); assert_eq!(s.command_count(), 3); assert_eq!(s.command_name(2).unwrap(), "C"); @@ -226,11 +228,11 @@ fn stack_undo_redo_noop_when_invalid() { #[test] fn stack_jump_clamps_and_lands() { - let (trace, mut probes) = setup(); + let trace = new_trace(); let mut s = UndoStack::new(); - s.push(trace_cmd(&mut probes[0]), "A"); - s.push(trace_cmd(&mut probes[1]), "B"); - s.push(trace_cmd(&mut probes[2]), "C"); + s.push(trace_cmd("a", &trace), "A"); + s.push(trace_cmd("b", &trace), "B"); + s.push(trace_cmd("c", &trace), "C"); assert_eq!(s.done_count(), 4); // empty + A + B + C // Jump back to just the empty command (clamps negative to 0). @@ -274,9 +276,9 @@ fn stack_discards_empty_multi() { #[test] fn stack_push_pre_executed_skips_redo() { - let (trace, mut probes) = setup(); + let trace = new_trace(); let mut s = UndoStack::new(); - s.push_pre_executed(trace_cmd(&mut probes[0]), "A"); + s.push_pre_executed(trace_cmd("a", &trace), "A"); assert_eq!( events(&trace), Vec::::new(), @@ -293,9 +295,9 @@ fn stack_push_pre_executed_skips_redo() { #[test] fn stack_clear_resets_to_bottom() { - let (trace, mut probes) = setup(); + let trace = new_trace(); let mut s = UndoStack::new(); - s.push(trace_cmd(&mut probes[0]), "A"); + s.push(trace_cmd("a", &trace), "A"); s.undo().unwrap(); assert_eq!(s.command_count(), 2); @@ -308,15 +310,10 @@ fn stack_clear_resets_to_bottom() { #[test] fn stack_caps_at_k_max() { - let (_trace, mut probes) = setup(); + let trace = new_trace(); let mut s = UndoStack::new(); for i in 0..(K_MAX_UNDO_COMMANDS + 20) { - let mut probe = Probe { - name: "x", - trace: probes[0].trace, - }; - let _ = &mut probe; - s.push(trace_cmd(&mut probes[i % 3]), &format!("cmd{i}")); + s.push(trace_cmd("x", &trace), &format!("cmd{i}")); } assert_eq!(s.command_count() as usize, K_MAX_UNDO_COMMANDS); }