refactor!: route Timeline editing through Beutl.Editor.Services#1845
Merged
Conversation
…lines Add the surface area for editing services that the timeline View and ViewModel will route through, in preparation for moving Scene/Element mutation and history Commit out of the UI layer. - ISceneTimeRangeService / ISceneTimeRangeDragSession - IElementMoveService / IElementMoveDragSession + ElementMoveOutcome - IElementResizeService / IElementResizeDragSession + ElementResizeRequest - IElementClipboardService + ElementPasteOutcome - IElementDuplicateService + DuplicateOutcome - IElementLifecycleService + SplitOutcome / GroupOutcome - IElementNudgeService - ILayerMoveService + LayerMovePlan - IKeyFrameMoveService - IClipboardGateway + ClipboardEntry (Avalonia-free clipboard abstraction) Implementations follow in subsequent commits; nothing is wired into IEditorContext.GetService yet.
First two of the editor services introduced in the previous commit. Both own a single HistoryManager.Commit per user-visible operation, so the timeline View and ViewModel can stop calling history.Commit directly.
SceneTimeRangeService: SetStart/SetEnd plus Begin{Start,End}Drag sessions. Drag sessions own initial-state snapshots and restore them on Cancel.
ElementResizeService: BeginResize returns a drag session whose Commit(IReadOnlyList<ElementResizeRequest>) calls Scene.MoveChild per element and emits a single MoveElement history entry.
Registered via IEditorContext.GetService on EditViewModel; lazy-instantiated to share one instance per editor lifetime.
Covered by 21 NUnit tests in tests/Beutl.UnitTests/Editor/Services/.
…ervices Three more services in the editing pipeline, plus the Avalonia clipboard adapter that lets the service layer stay UI-free. ElementMoveService: BeginMove returns a drag session whose Commit fans out to MoveChildren + history.Commit(MoveElement), or to the duplicate service when Alt+drag is active. WouldOverlap short-circuits Alt+drag when the copy lands on top of a source. ElementDuplicateService: wraps the existing DuplicateHelper. DuplicateAtClickedPosition runs the spiral search (lifted from TimelineTabViewModel.CorrectPosition) bounded by 100k steps so a packed timeline cannot hang the caller. ElementClipboardService: PasteAsync dispatches on clipboard formats (Elements / Element / Files / Bitmap). CopyAsync serializes one or many elements; CutAsync is Copy + Scene.RemoveChild + history.Commit(CutElement). IClipboardGateway is the boundary that keeps Beutl.Editor Avalonia-free. AvaloniaClipboardGateway in Beutl.Editor.Components maps Avalonia DataFormats onto BeutlClipboardFormats string identifiers. Covered by 39 NUnit tests across three test files.
…meMove services Final four services in the editing pipeline. ElementLifecycleService: Exclude / Delete / SetEnabled / SetAccentColor / Split / Group / Ungroup. Split ports the regenerate-and-shift-keyframes logic from ElementViewModel.SplitCore and stays bounded by the project frame rate. ElementNudgeService: System.Threading.Timer-based debounce (300ms) coalesces a burst of keyboard nudges into one MoveElement history entry. Flush is wired to HistoryManager.BeforeMutation in EditViewModel so an Undo / Redo / JumpTo never absorbs a pending nudge into the wrong transaction. Dispose() flushes any outstanding commit. LayerMoveService: PlanMove enumerates the elements that need ZIndex shifts; CommitMove applies them and emits one MoveLayer history entry. KeyFrameMoveService: thin commit boundary for InlineAnimation drag releases (KeyTime is mutated by the View during the drag; the service owns the MoveKeyFrame commit). Covered by 29 NUnit tests across four test files.
Replace the inline editing logic with calls into Beutl.Editor.Services. About 350 lines net come out of the ViewModel. Migrated callsites: - Scene Start / Duration: SetStartTimeCore / SetEndTimeCore -> ISceneTimeRangeService.SetStart / SetEnd. - Paste: PasteCore + PasteElementList + PasteElement + PasteImageElement + PasteFiles -> IElementClipboardService.PasteAsync. The clipboard format dispatch and per-format file IO now live in the service. - Duplicate: DuplicateSelectedElements + RegenerateAndPlaceAtCorrectedPosition + CorrectPosition (spiral search) -> IElementDuplicateService.DuplicateAtClickedPosition. DuplicateElementsAt remains as an internal entry point for the Alt+drag View behavior; it now delegates to DuplicateAtPosition. - Nudge: NudgeSelectedElements + ScheduleNudgeCommit + OnNudgeCommitTick -> IElementNudgeService.Nudge. The DispatcherTimer is gone (the service uses System.Threading.Timer). - BeforeMutation flush: dropped from the VM; EditViewModel.CreateNudgeService wires it directly to the singleton ElementNudgeService. Notification behavior is preserved at the call sites — the project-not-saved check moves out of HandleDuplicateException into the entry points, and the I/O failure notification is still differentiated. BREAKING CHANGE: TimelineTabViewModel no longer exposes the private editing helpers (PasteCore, DuplicateSelectedElements, NudgeSelectedElements, ScheduleNudgeCommit, OnNudgeCommitTick, FlushPendingNudgeCommit, SetStartTimeCore, SetEndTimeCore, RegenerateAndPlaceAtCorrectedPosition, CorrectPosition, HandleDuplicateException). Out-of-tree code that hooked them via reflection must route through IEditorContext.GetService instead.
Replace per-element editing helpers with calls into IElementLifecycleService and IElementClipboardService. Migrated callsites: - OnExclude / OnDelete -> IElementLifecycleService.Exclude / Delete. - OnGroupSelectedElements / OnUngroupSelectedElements -> IElementLifecycleService.Group / Ungroup. The service now always removes the supplied ids from existing groups first so single-id Group calls keep their previous ungroup-by-default behavior. - SplitCore -> IElementLifecycleService.Split. The group-remap logic that tracked _elementGroup per ViewModel moves into the service, using Scene.Groups directly. - AccentColor subscribe -> IElementLifecycleService.SetAccentColor. - Copy / OnCut -> IElementClipboardService.CopyAsync / CutAsync. ClipboardHelper, JsonArray, DataTransfer assembly drops out of ElementViewModel. Dead code removed: ElementViewModel.SetClipboard, RemoveIdsFromElementSets. ~120 lines net leave the ViewModel. BREAKING CHANGE: ElementViewModel no longer exposes SetClipboard / RemoveIdsFromElementSets / the per-method OnExclude / OnDelete / OnSplit / OnGroupSelectedElements / OnUngroupSelectedElements helpers. Out-of-tree code that hooked them via reflection must route through IEditorContext.GetService instead.
Strip the direct Scene mutation and history.Commit calls out of the timeline View layer. Every drag/release path now flows through a service drag session or an IElement*Service command. TimelineTabView (Scene Start / Duration drag): - PointerPressed on a start/end bar opens an ISceneTimeRangeDragSession via ISceneTimeRangeService.BeginDragStart / BeginDragEnd. - PointerMoved calls session.Update(_pointerFrame); direct Scene.Start / Scene.Duration writes are removed. - PointerReleased calls session.Commit and disposes the session. The two history.Commit calls in the view are gone. ElementView: - _MoveBehavior.OnBorderPointerReleased rebuilds onto IElementMoveService.BeginMove / Commit. ElementMoveOutcome drives the View's animation, notification, and visual-restore paths. Alt+drag duplicate, fallback move, and "would overlap source" all live behind the same DragSession contract. - _ResizeBehavior multi-element path uses IElementResizeService.BeginResize plus ElementResizeRequest. SubmitViewModelChanges still owns the single-element resize as before. - EnableElementClick routes through IElementLifecycleService.SetEnabled. LayerHeader: Border_PointerReleased plans + commits through ILayerMoveService. The View keeps its MoveLayerCommand for VM/animation orchestration; the service simply owns the MoveLayer history entry. InlineAnimationLayer: _DragBehavior.OnPointerReleased commits through IKeyFrameMoveService.CommitMove with the affected key frames. ILayerMoveService.CommitMove is intentionally a pure commit boundary now; the caller still drives the ZIndex writes so the LayerHeader VM / element animations can stay in sync. The corresponding test was updated to drive the writes from the test. BREAKING CHANGE: The Timeline view code no longer touches HistoryManager directly. Plugins that subclass these views and override the drag/release paths must obtain an IEditorContext.GetService<I...Service> instead of calling history.Commit. The same applies to the LayerMoveService/CommitMove contract change.
Document the editing service surface (ISceneTimeRangeService / IElementMoveService / IElementResizeService / IElementClipboardService / IElementDuplicateService / IElementLifecycleService / IElementNudgeService / ILayerMoveService / IKeyFrameMoveService / IClipboardGateway) so future AI assistants extending the editor know where to add new services and how each service owns its commit boundary.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “editing pipeline” service layer in Beutl.Editor.Services (interfaces + initial implementations) and refactors Timeline/UI code to route scene/element mutations and history commits through these services, improving separation between UI and editor logic and enabling unit testing without Avalonia dependencies (via IClipboardGateway).
Changes:
- Added multiple editor service interfaces/implementations (scene time range, element move/resize/duplicate/clipboard/lifecycle/nudge, layer move, keyframe move).
- Refactored Timeline Views/ViewModels to use the new services instead of directly mutating models and calling
HistoryManager.Commit. - Added unit tests for the new services and introduced an Avalonia-backed
IClipboardGatewayimplementation.
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Beutl.UnitTests/Editor/Services/SceneTimeRangeServiceTests.cs | Adds unit tests for SceneTimeRangeService start/end editing + drag sessions. |
| tests/Beutl.UnitTests/Editor/Services/LayerMoveServiceTests.cs | Adds unit tests for LayerMoveService plan/commit behavior. |
| tests/Beutl.UnitTests/Editor/Services/KeyFrameMoveServiceTests.cs | Adds unit tests for KeyFrameMoveService commit boundary. |
| tests/Beutl.UnitTests/Editor/Services/ElementResizeServiceTests.cs | Adds unit tests for ElementResizeService drag session commit/cancel behavior. |
| tests/Beutl.UnitTests/Editor/Services/ElementNudgeServiceTests.cs | Adds unit tests for debounced nudge + flush/Dispose semantics. |
| tests/Beutl.UnitTests/Editor/Services/ElementMoveServiceTests.cs | Adds unit tests for move/alt-duplicate outcomes and commit semantics. |
| tests/Beutl.UnitTests/Editor/Services/ElementLifecycleServiceTests.cs | Adds unit tests for exclude/delete/enabled/color/split/group/ungroup. |
| tests/Beutl.UnitTests/Editor/Services/ElementDuplicateServiceTests.cs | Adds unit tests for overlap detection + duplicate placement behavior. |
| tests/Beutl.UnitTests/Editor/Services/ElementClipboardServiceTests.cs | Adds unit tests for copy/cut/paste via IClipboardGateway. |
| src/Beutl/ViewModels/EditViewModel.cs | Wires new services into IEditorContext.GetService, including nudge flush hook and clipboard gateway instantiation. |
| src/Beutl.Editor/Services/SceneTimeRangeService.cs | Implements ISceneTimeRangeService including drag sessions and history commits. |
| src/Beutl.Editor/Services/LayerMoveService.cs | Implements ILayerMoveService plan/commit boundary. |
| src/Beutl.Editor/Services/KeyFrameMoveService.cs | Implements IKeyFrameMoveService commit boundary. |
| src/Beutl.Editor/Services/ISceneTimeRangeService.cs | Introduces scene time range service + drag session interfaces. |
| src/Beutl.Editor/Services/ILayerMoveService.cs | Introduces layer move service interface + LayerMovePlan. |
| src/Beutl.Editor/Services/IKeyFrameMoveService.cs | Introduces keyframe move commit interface. |
| src/Beutl.Editor/Services/IElementResizeService.cs | Introduces element resize service interface + request types. |
| src/Beutl.Editor/Services/IElementNudgeService.cs | Introduces nudge service interface. |
| src/Beutl.Editor/Services/IElementMoveService.cs | Introduces element move service interface + outcome types. |
| src/Beutl.Editor/Services/IElementLifecycleService.cs | Introduces lifecycle service interface + outcomes. |
| src/Beutl.Editor/Services/IElementDuplicateService.cs | Introduces duplicate service interface + outcome. |
| src/Beutl.Editor/Services/IElementClipboardService.cs | Introduces clipboard service interface + paste outcome type. |
| src/Beutl.Editor/Services/IClipboardGateway.cs | Introduces Avalonia-free clipboard abstraction (IClipboardGateway, ClipboardEntry). |
| src/Beutl.Editor/Services/ElementResizeService.cs | Implements resize commit/cancel session. |
| src/Beutl.Editor/Services/ElementNudgeService.cs | Implements debounced nudge using System.Threading.Timer and history commit coalescing. |
| src/Beutl.Editor/Services/ElementMoveService.cs | Implements move/alt-duplicate commit behavior and outcome reporting. |
| src/Beutl.Editor/Services/ElementLifecycleService.cs | Implements lifecycle operations (exclude/delete/split/group/ungroup/etc.) with single commits. |
| src/Beutl.Editor/Services/ElementDuplicateService.cs | Implements duplicate placement with bounded spiral search to avoid UI hangs. |
| src/Beutl.Editor/Services/ElementClipboardService.cs | Implements cut/copy/paste dispatch using IClipboardGateway. |
| src/Beutl.Editor/Services/BeutlClipboardFormats.cs | Adds Avalonia-free clipboard format identifiers for editor services. |
| src/Beutl.Editor/CLAUDE.md | Documents the new editing service layer and intended usage patterns. |
| src/Beutl.Editor.Components/TimelineTab/Views/TimelineTabView.axaml.cs | Switches scene range drag updates/commit to ISceneTimeRangeService sessions. |
| src/Beutl.Editor.Components/TimelineTab/Views/LayerHeader.axaml.cs | Switches layer move commit to ILayerMoveService. |
| src/Beutl.Editor.Components/TimelineTab/Views/InlineAnimationLayer.axaml.cs | Switches keyframe drag release commit to IKeyFrameMoveService. |
| src/Beutl.Editor.Components/TimelineTab/Views/ElementView.axaml.cs | Switches enable/move/resize behaviors to lifecycle/move/resize services. |
| src/Beutl.Editor.Components/TimelineTab/ViewModels/TimelineTabViewModel.cs | Routes duplicate/paste/nudge flows through new services and removes old inline clipboard/spiral logic. |
| src/Beutl.Editor.Components/TimelineTab/ViewModels/ElementViewModel.cs | Routes copy/cut/exclude/delete/group/ungroup/split/color through services. |
| src/Beutl.Editor.Components/Services/AvaloniaClipboardGateway.cs | Adds Avalonia-backed implementation of IClipboardGateway. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address design review finding #1 (drag-session shape divergence) by removing the `IXxxDragSession` ceremony from `ISceneTimeRangeService`, `IElementMoveService`, and `IElementResizeService`. All three are now plain-method surfaces. `ISceneTimeRangeService` is the only one that genuinely needed a per-pointer-frame update phase: it now exposes `UpdateStartDrag` / `UpdateEndDrag` (mutate without commit) plus `CommitStartChange` / `CommitEndChange` (close the transaction). The View re-drives Update with the initial values to cancel — no Cancel method to leak. `IElementMoveService` collapses to `Move(scene, elements, deltaStart, deltaZIndex)` (returns `Moved` / `None`) and `DuplicateOrMove(...)` (returns `Duplicated` / `DuplicateOverlapsSource` / `FellBackToMove`). The Alt+drag fallback to a plain move stays inside the service. `IElementResizeService` collapses to `Resize(scene, requests)`. The `ResizeEdge` enum and `ClampToOriginalDuration` flag were View hints the service never consumed — both are gone from the public surface; the View keeps its own representation of edge and clamp. Updates `TimelineTabView` and `ElementView._{Move,Resize}Behavior` to the new shape, drops the local `_timeRangeDragSession` field, and refreshes the SceneTimeRange / Move / Resize NUnit fixtures. BREAKING CHANGE: `ISceneTimeRangeDragSession`, `IElementMoveDragSession`, `IElementResizeDragSession`, the `BeginDragStart` / `BeginDragEnd` / `BeginMove` / `BeginResize` factory methods, and the `ResizeEdge` enum are removed. Plugins that wired into the session shape must switch to the plain-method surface: scene drag callers split their per-frame `session.Update(...)` calls into `UpdateXxxDrag(...)` and the single `session.Commit()` into `CommitXxxChange()`; element move / resize callers replace `BeginMove(...).Commit(d, dz)` with `Move(scene, elems, d, dz)` (or `DuplicateOrMove(...)` for Alt+drag) and `BeginResize(...).Commit(requests)` with `Resize(scene, requests)`.
`.editorconfig` requires utf-8-bom for `.cs`, but the service files added earlier on this branch landed without the BOM. `dotnet format whitespace` flags every one of them. Apply the fix as a separate commit so the diff in the surrounding refactor stays focused on behavior.
Consolidate fixes for findings raised in the multi-agent review of the service-layer refactor: - design #3 / `ElementPasteOutcome`: convert to `sealed record` so it matches `DuplicateOutcome` / `SplitOutcome` / `GroupOutcome` and gets structural equality. `Empty` is preserved as a static sentinel. - design #4 / `ElementClipboardService.PasteBitmapAsync`: `imageAccentColorFactory` was optional and defaulted to `Colors.Teal` — a silent behavior regression vs. the in-VM code, which used `ColorGenerator.GenerateColor(typeof(SourceImage).FullName!)`. The factory is now required; `EditViewModel` passes the deterministic factory. - design #5 / `EditorConstants.ElementFileExtension`: deduplicate the `"belm"` literal previously held in three service files plus `Beutl.Editor.Components.Constants`. New `Beutl.Editor.EditorConstants` owns the source value; `Beutl.Editor.Components.Constants` delegates. - design #6 (drag-session Cancel) is resolved by the preceding commit collapsing `ISceneTimeRangeService` to plain methods — there is no Cancel surface left to leak. - pr-review #1 / `KeyFrameMoveServiceTests`: the previous `CommitMove_WithKeyFrames_CommitsOnce` test used a detached KeyFrame whose property changes were never observed, so the assertion `GreaterThanOrEqualTo(before)` was vacuously true. The fixture now attaches a dedicated `CoreObjectOperationObserver` to the KeyFrame and pins the contract with `EqualTo(before + 1)` + an Undo round-trip. - pr-review #2 / `ElementNudgeService` dispose race: mark `_disposed` `volatile`, take `_gate` inside `Dispose` to set the flag, and re-check `_disposed` inside `Schedule` so a concurrent `Nudge` cannot reach `_timer.Change` after `_timer.Dispose`. `Flush` also skips `_timer.Change` when already disposed. - pr-review #3 / `ElementLifecycleService.Group` single-id contract: the test now asserts `_history.UndoCount` and `_scene.Groups.Count` are unchanged, pinning the documented no-op behavior even if a future observer were to surface a property change.
Two real bugs Copilot caught in the multi-element paste + clipboard plumbing introduced by the service-layer refactor: - `ElementClipboardService.PasteAsync` threaded `clickedFrame` / `clickedLayer` into `PasteSingleElementAsync`, but `PasteElementsAsync` ignored both and called `DuplicateAtClickedPosition(scene, oldElements, TimeSpan.Zero, 0)`, silently placing the duplicates at (0s, layer 0) regardless of the user's right-click target. This matches a regression vs. the pre-refactor `TimelineTabViewModel.PasteElementList` which used `ClickedFrame` / `CalculateClickedLayer` for the spiral search anchor. Pass the click position through. - `CopyAsync` published a `"text/plain"` clipboard entry, but `AvaloniaClipboardGateway.SetAsync` only routed entries through `MapFromString`, which did not recognize `"text/plain"` and silently dropped it. The result was no human-pasteable text on Cmd/Ctrl-V in other apps, even though the previous in-VM code added it via `DataTransferItem.CreateText`. Introduce `BeutlClipboardFormats.Text` and special-case it in the gateway so the platform's native text slot receives the payload. Adds `PasteAsync_ElementsFormat_RespectsClickedPosition` to pin the paste-position contract and `CopyAsync_ExposesPlainTextAlongsideElementJson` to pin the text/plain output. Copilot also flagged the multi-element `Select(JsonNode (e) => ...)` syntax as invalid C# — that is a C# 10+ explicit-return-type lambda which is valid under `LangVersion: preview`, so no change. The `clampToOriginalDuration` flag Copilot flagged on `IElementResizeService` is already gone — the preceding refactor removed the entire `BeginResize` surface.
yuto-trd
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63cb0c163c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Three real bugs Codex caught after the multi-element refactor: - P1 / `CutAsync` data loss when clipboard unavailable: `CutAsync` used to call `CopyAsync` and then unconditionally delete the source elements. If the platform clipboard is unavailable (no top-level window — `ClipboardHelper.GetClipboard()` returns null in `AvaloniaClipboardGateway`), the clipboard write silently no-oped and the elements vanished from the scene with no way to paste them back. Change `IClipboardGateway.SetAsync` and `IElementClipboardService.CopyAsync` to return `Task<bool>` so the Cut path can verify the clipboard write before mutating the scene. `CutAsync` now returns `false`, preserves the elements, and logs a warning when the clipboard is unavailable. - P2 / `PasteElementsAsync` silent failure: when `DuplicateAtClickedPosition` fails (unsaved scene, I/O failure staging duplicates) the multi-element paste returned `ElementPasteOutcome.Empty` with no diagnostic surface. The single- element and bitmap paths already `LogWarning` on the same kind of failure. Match that behavior here so the user can tell paste was attempted instead of silently nothing-ing. - P2 / multi-element drag zero-delta visual: when a multi-element drag releases with sub-frame movement that rounds to `(deltaStart, deltaZIndex) = (0, 0)`, `IElementMoveService.Move` correctly returns `None` and skips the history commit, but the `case ElementMoveOutcome.None: return;` arm in `_MoveBehavior.OnBorderPointerReleased` skipped the visual restore too. The clips stayed visibly offset at the dragged `Margin`/`BorderMargin` until some later model/scale change rebound them. Call `ForceRestoreVisualToModel` in that arm. Adds `CutAsync_ClipboardUnavailable_PreservesScene` and a `SimulateUnavailable` toggle on the in-memory clipboard fixture to pin the cut-safety contract. BREAKING CHANGE: `IClipboardGateway.SetAsync` now returns `Task<bool>` (was `Task`). `IElementClipboardService.CopyAsync` now returns `Task<bool>` (was `Task`). Out-of-tree gateway / clipboard implementations must update their signatures. Returning `true` preserves the previous "always succeeded" assumption.
The beutl-test-runner subagent (and any other agent that uses worktree isolation) materializes a git worktree under .claude/worktrees/agent-<id>/. Track only the agent contract surface (settings.json / agents/ / skills/ / etc.), not the per-run worktrees.
`ElementLifecycleService.Group` called `_historyManager.Commit` unconditionally even when no group was created and `RemoveIdsFromGroups` was a no-op. With nothing to commit on its own the call would have been silent — but if the current `HistoryTransaction` already held pending operations from an upstream caller (e.g. the test fixture's `Scene.Children.Add(element)` before calling `Group(scene, [id])`), Commit closed the transaction and materialized a phantom undo entry that bundled the unrelated work. The `Group_SingleId_DoesNotCreateGroup` test added to pin the no-op contract fired exactly this path: the fixture's `AddElement` left an uncommitted `Scene.Children.Add` op in the transaction, and the follow-up `Group(scene, [element.Id])` flushed it under the `GroupElements` history label. Fix: make `RemoveIdsFromGroups` return whether it changed anything, and only commit in `Group` when `removed || created` is true. `Ungroup` gets the matching guard: commit only when the call removed something. Adds `Ungroup_IdsNotInAnyGroup_IsNoOp` to lock the symmetric contract on `Ungroup`. This was diagnosed by the `beutl-test-runner` subagent, which caught the failure in CI build #26205117772 (commit 63cb0c1) after the stronger `UndoCount` assertion landed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95bb5ec0da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`UpdateEndDrag` re-used `ApplyEnd`, which is the one-shot keyboard / menu code path: when the requested end is less than `Scene.Start`, both `Scene.Start` and `Scene.Duration` shift backward. That behavior is correct for "Set End Time to Current Time" (a deliberate, single operation), but during a live drag the absolute time range of the scene must not jerk backward just because the user crossed `Scene.Start` mid-pointer-move (Codex review #r3278970042). The pre-refactor drag loop only clamped duration to >= 1 frame and never touched `Scene.Start`. The symmetric regression exists on `UpdateStartDrag`: the one-shot `SetStart` path shifts the end forward when the requested start is past the current end; the drag loop must clamp the start to `(initialEnd - 1 frame)` instead so the absolute end stays pinned. Split the apply helpers into one-shot (`ApplyStart` / `ApplyEnd`, unchanged behavior — still used by `SetStart` / `SetEnd`) and drag (`ApplyStartDrag` / `ApplyEndDrag`, clamp-only). Existing tests that exercise `UpdateXxxDrag` happen inside the clamp range, so they still pass on the new helpers. Adds `UpdateEndDrag_PointerBeforeStart_KeepsStartPinned` and `UpdateStartDrag_PointerAfterInitialEnd_ClampsToOneFrameBeforeEnd` to pin both clamp contracts.
…dels
`Services/` is documented in `Beutl.Editor/CLAUDE.md` as the editing
pipeline directory, but it has accumulated payload types that have
nothing to do with the pipeline: `AudioFrameSnapshot` (audio-preview
PCM record), `CacheBlock` (frame-cache display block),
`ElementDescription` (placement parameter record). All three are
referenced from `Beutl.Editor.Components` views and the app
ViewModels, not from any service implementation.
Introduce `Beutl.Editor/Models/` and move the three records there with
namespace `Beutl.Editor.Models`. Service interfaces that previously
exposed them (`IBufferStatus.CacheBlocks`, `IPreviewPlayer.PlayAudio`,
`IElementAdder.AddElement`, `IElementClipboardService` paste-files
path via `ElementAdderImpl`) take a `using Beutl.Editor.Models;`
directive; test fixtures for the three types follow.
The split makes the `Services/` directory contract honest again — what
remains is interfaces, implementations, and the stateless helpers /
constants those services dispatch on (`DuplicateHelper`,
`ObjectRegenerator`, `BeutlClipboardFormats`, `IClipboardGateway` +
its `ClipboardEntry` record). The latter group is the working state
of the editing pipeline, not generic editor models, so it stays put.
BREAKING CHANGE: `Beutl.Editor.Services.AudioFrameSnapshot`,
`Beutl.Editor.Services.CacheBlock`, and
`Beutl.Editor.Services.ElementDescription` are renamed to
`Beutl.Editor.Models.{AudioFrameSnapshot|CacheBlock|ElementDescription}`.
Out-of-tree consumers add a `using Beutl.Editor.Models;` directive
(or fully qualify), nothing else changes.
Move the remaining five constants (`SceneFileExtension`, `ProjectFileExtension`, `BeutlFolder`, `ViewStateFolder`, `ProjectPackageExtension`) from `Beutl.Editor.Components.Constants` into `Beutl.Editor.EditorConstants` and delete the now-empty `Constants` class. `ElementFileExtension` already lived in `EditorConstants` via a delegating const declaration; the partial delegation pattern is gone — there is now a single source of truth. Updates 11 call sites across `Beutl.Editor.Components`, the app ViewModels (`EditViewModel`, `CreateNewSceneViewModel`, `ElementAdderImpl`), and the app services (`ProjectService`, `OutputService`, `SceneWorkspaceItemExtension`, `EditorHostFallback`, `MainView.InitializeMenuBar`). BREAKING CHANGE: `Beutl.Editor.Components.Constants` is removed. Use `Beutl.Editor.EditorConstants` instead — the new type lives in `Beutl.Editor` so non-UI consumers (e.g. project-system services) no longer need to take a `Beutl.Editor.Components` reference just to read a file extension.
`BeutlDataFormats.Element` / `.Elements` previously re-declared the clipboard format strings (`"BeutlElementJson"`, `"BeutlElementsJson"`) as private consts inside the Avalonia-typed wrapper. The Avalonia-free `BeutlClipboardFormats` already owned the same literals — keeping two copies meant a future format-string change had to update both files. Derive the Avalonia-typed `DataFormat<string>` values from the `BeutlClipboardFormats` constants directly. Single source of truth, zero behavior change.
The previous interface mixed structural commands (Exclude, Delete, Split, Group, Ungroup — mutate `Scene.Children` / `Scene.Groups`, touch disk via `CoreSerializer.StoreToUri` and `ObjectRegenerator`) with attribute writes (SetEnabled, SetAccentColor — single-property mutation on one `Element`, no file I/O). The two groups commit under disjoint command names and have entirely different dependency surfaces. A plugin author wanting to override the structural piece inherited responsibility for the attribute piece (and vice versa). Split the service: - `IElementStructureService` — Exclude / Delete / Split / Group / Ungroup. Implementation is the existing `ElementLifecycleService` with attribute methods removed; renamed to `ElementStructureService`. - `IElementAttributeService` — SetEnabled / SetAccentColor. New service, ~30 lines. Two call sites switch interface (Color subscribe in `ElementViewModel`, `EnableElementClick` in `ElementView`); the three structural callers keep the same shape under the new interface name. `EditViewModel` registers both services independently. Existing `ElementLifecycleServiceTests` is renamed to `ElementStructureServiceTests` with attribute tests extracted to a new `ElementAttributeServiceTests` fixture. BREAKING CHANGE: `Beutl.Editor.Services.IElementLifecycleService` and `Beutl.Editor.Services.ElementLifecycleService` are removed. Migrate to `IElementStructureService` (structural calls) and `IElementAttributeService` (SetEnabled / SetAccentColor). The `SplitOutcome` / `GroupOutcome` records keep their names and move with the structure service.
The two-step `PlanMove` + `CommitMove` boundary left the model in an open-transaction window: `LayerHeader.axaml.cs:106` ran `MoveLayerCommand.Do()` to write every `Element.ZIndex` value, then `service.CommitMove(plan)` stamped history afterward. Any exception between the writes and the commit would leave the scene mutated with no entry on the history stack. The same enumeration of shifted elements was also computed in two places (service `PlanMove` + View's nested `MoveLayerCommand`). Collapse to a single `ApplyMove(scene, oldLayer, newLayer, directElements)` that: - enumerates the shift range against the pre-write state, - writes `Element.ZIndex = newLayer` on the direct set and `e.ZIndex += shiftDelta` on the shifted set, - commits one `MoveLayer` entry, - returns the `LayerMovePlan` so the View can drive animations. The View (`LayerHeader.axaml.cs`) is rewritten without the nested `MoveLayerCommand`. After `ApplyMove` returns, it updates VM-side state (`LayerHeaderViewModel.Number.Value`, `LayerHeaders.Move(...)`) and triggers element animations with `affectModel: false` so the animation hook does not double-write `Element.ZIndex`. Tests: `LayerMoveServiceTests` swap from "two-step + caller writes" to "service writes + commits", and add `ApplyMove_AfterUndo_...` to pin the round-trip. BREAKING CHANGE: `ILayerMoveService.PlanMove` and `ILayerMoveService.CommitMove` are removed; replaced by a single `ApplyMove(scene, oldLayer, newLayer, directElements)` that owns both the writes and the commit. Out-of-tree implementations migrate to the new single method; out-of-tree callers replace `PlanMove(...)` + caller-driven `ZIndex` writes + `CommitMove(...)` with one call to `ApplyMove(...)` and use the returned `LayerMovePlan` to drive any post-move side effects.
The service body is a single line: `_historyManager.Commit(CommandNames.MoveKeyFrame)`. The only possible test was "Commit was called", which is a HistoryManager contract test, not a service test. The InlineAnimation View already calls `history.Commit` directly for Remove and Paste; routing only the multi-keyframe drag-release through a thin service-shaped wrapper bought no test surface and added two layers of indirection. `InlineAnimationLayer._DragBehavior.OnPointerReleased` now calls `historyManager.Commit(CommandNames.MoveKeyFrame)` inline, mirroring the surrounding KeyFrame ops. The `ReflectModelKeyTime()` calls still sit before the commit, so the per-item KeyTime writes batch into the same history transaction the way the service did. BREAKING CHANGE: `Beutl.Editor.Services.IKeyFrameMoveService` and `Beutl.Editor.Services.KeyFrameMoveService` are removed. Out-of-tree inline-animation views inline the same one-line `historyManager.Commit(CommandNames.MoveKeyFrame)` directly — the service was load-bearing for nothing.
`LayerHeaderViewModel.SwitchEnabledCommand` iterated `scene.Children`, filtered by ZIndex, conditionally toggled `Element.IsEnabled`, and committed `ChangeLayerEnabled`. `LayerHeaderViewModel.SetColor` lazy-created a `TimelineLayer` model in `scene.Layers` if none existed yet and committed `ChangeLayerColor`. Both bypassed the editor service layer entirely. Extract into `ILayerAttributeService` with two methods that return `bool` indicating whether a commit actually happened (idempotency): - `SetEnabled(scene, zIndex, newEnabled)` — flips only elements that differ from the target state. No-op + no commit when all elements already match. - `SetColor(scene, zIndex, color, defaultName)` — creates the model on demand with `defaultName` when no `TimelineLayer` exists at `zIndex`; otherwise updates in-place. No-op + no commit when the existing color already matches. The VM keeps the `_skipSubscription` guard and the local `IsEnabled` toggle so the UI updates immediately; the service handles the model side and the commit decision. Tests cover the five branches (flip-some, all-match-noop, create-from-empty, update-in-place, color-match-noop). This is the testable surface that didn't exist before — the lazy-create branch in particular was previously invisible to NUnit.
`SceneSettingsTabViewModel.Apply` had three-field change detection, three-property writes, and a single Commit inlined into a `ReactiveCommand` subscribe lambda — none of which was testable without spinning up an Avalonia view-model. Extract `ISceneSettingsService.Apply(scene, frameSize, start, duration)` that: - skips commit when all three already match (idempotency contract), - writes all three fields then commits one `ChangeSceneSettings` entry, - returns bool so the caller can tell whether anything changed. The tab VM keeps the input validation, the `CanApply` reactive combinator, and the LayerCount option update — those are UI concerns. The model write + commit moves into the service. Tests cover the four meaningful branches: no-change-no-commit, single-field-change, three-fields-collapse-into-one-entry, undo round-trip.
`GraphEditorViewModel.PasteAnimation/PasteKeyFrame` and `InlineAnimationLayerViewModel.PasteAnimation/PasteKeyFrame` carried nearly-identical paste logic — JSON parse, $type discriminator check, generic-type-arg validation against the target's ValueType, and (for keyframe paste) a key-time collision branch that updates the existing key in place. Two copies, each ~50 lines, with subtly different error messages and the same five validation branches. Extract into `IKeyFrameClipboardService`: - `PasteAnimation(target, json)` returns a flat enum (InvalidJson / MissingType / TypeIsNotKeyFrameAnimation / GenericTypeMismatch / UnexpectedError / Pasted). The Id-preservation and per-keyframe Id regeneration are kept inside the service. - `PasteKeyFrame(target, json, keyTime)` returns (Outcome, EasingForFallback?) so the GenericTypeMismatch caller can still invoke its own typed `InsertKeyFrame` path with the parsed easing — the service cannot create a new keyframe of the target type without the View's value accessor. Both VM call sites become a switch over the outcome enum. Notification strings stay in the VM (they are UI-side and reference per-view label keys); the service handles the model writes, the commit decision, and the validation branches. Tests cover the seven meaningful branches (invalid JSON, missing type, generic mismatch on both methods, animation paste success, keyframe insert, keyframe replace-existing) — each verifying both the outcome enum and the commit count, including the GenericTypeMismatch no-commit contract on PasteKeyFrame.
The NodeGraph tab's three ViewModels (`GraphNodeViewModel`, `NodePortViewModel`, `NodeGraphViewModel`) carried eleven distinct `HistoryManager.Commit` sites with non-trivial validation interleaved: - `AddNodePort` rejected duplicate `GroupInput` / `GroupOutput` in a `GraphGroup` only at the call site, not through any shared guard. - `Delete` walked the node's items, classified each by port-direction (output / list / input), collected the matching `Connection`s from `graph.AllConnections`, and disconnected them before removing the node. Same logic was duplicated for `Remove` on dynamic ports. - `TryConnect` had a three-way branch (dynamic-port add vs. plain connect vs. no-op) plus a port-direction sort helper inline. - `DisconnectAll` switched on port type to walk the right connection collection. Extract into `INodeGraphMutationService`: - AddNode / RemoveNode / MoveNodes / RenameNode - RemovePort / MoveConnectionSlot / RenamePort - TryConnect (returns `NodeConnectOutcome` to distinguish PortAdded from Connected), TryDisconnect, DisconnectAll, DisconnectConnection Each method commits at most one history entry. Rename methods skip the commit when the name already matches (idempotency contract). `TryConnect` keeps the port-direction sort logic that used to live as a private helper on `NodePortViewModel`. The three view-models become thin routers: each operation is one or two lines calling the service. ~150 lines of repeated validation disappear across the three files. Tests cover the highest-value branches (GroupInput duplicate guard, GroupOutput duplicate guard, RenameNode idempotency, MoveNodes collapse-into-one-entry, RemoveNode commit, MoveNodes empty no-op). The port and connection paths require richer fixture setup with real `IInputPort` / `IOutputPort` instances — those tests are kept for a follow-up rather than bundled with this scope.
`Element.Objects` mutations (Add / Insert / Move / Remove / paste-over / toggle-enabled) lived in five places across the ElementPropertyTab — `ElementPropertyTabView.Drop` (Add), `EngineObjectPropertyView.Drop` (InsertAt), `EngineObjectPropertyView.Remove_Click` (Remove), `_DragBehavior.OnMoveDraggedItem` (Move), `EngineObjectPropertyViewModel.SetJsonString` (PasteOver), and `EngineObjectPropertyViewModel.IsEnabled.Skip(1).Subscribe` (SetEnabled). No shared validation: Move did not guard against identical indices, InsertAt did not clamp out-of-range drop targets, PasteOver threw the same `Exception` for both invalid JSON and a missing `$type`. Extract into `IElementObjectService`. The high-value test surface is: - InsertAt clamps the index to `[0, Count]` rather than throwing. - Move returns false and commits nothing when oldIndex == newIndex. - Remove returns false when the object is not in the list (the View could previously commit a no-op RemoveObject in racing scenarios). - PasteOver distinguishes InvalidJson, MissingType, UnexpectedError, and Pasted outcomes — the only one that commits is Pasted. - SetEnabled is idempotent. Drop handlers, the `_DragBehavior`, and the ViewModel subscribe blocks become one-line routers to the service. ~50 lines of branching logic disappear from the three files. Tests cover the eleven branches above.
`MenuBarViewModel.Scene.OnExcludeElement / OnCutElement` and `MainView.InitializeMenuBar` (delete-from-project confirm flow) all hand-rolled the same "scene mutation + history.Commit" code that `IElementStructureService` and `IElementClipboardService` already own: - `OnExcludeElement` called `scene.RemoveChild + Commit(RemoveElement)` → route through `IElementStructureService.Exclude`. - `OnCutElement` built a `DataTransfer`, set it on the Avalonia clipboard, called `scene.RemoveChild + Commit(CutElement)` → route through `IElementClipboardService.CutAsync`. As a bonus, this picks up the clipboard-unavailable safety check the service added in 98f30b2 (no longer loses data when the platform clipboard is null). - `OnRemoveFromProject` (after the confirmation dialog) called `scene.DeleteChild + Commit(DeleteElement)` → route through `IElementStructureService.Delete`. No new service, no new test surface — just removes three bypasses that duplicated logic the service layer was supposed to own.
The service-extraction commits did not compile and had never been tested (Beutl.UnitTests references Beutl.Editor, so the whole suite was unbuildable). Fixing the compile errors then surfaced latent undo, threading, and error-handling defects. Build: - NodeGraphMutationService: add missing `using Beutl.NodeGraph.Nodes.Group` for GroupInput/GroupOutput and drop the bogus explicit type arg `SelectMany<INodeItem, ...>` (INodeItem does not exist; the element type is INodeMember, inferred as before the refactor). - LayerHeader.axaml.cs: add missing `using Beutl.ProjectSystem` for Element. - Clear branch-introduced warnings: [NotNullWhen(true)] on SortPortDirection out params (CS8602/CS8604), and remove the redundant `using Beutl.Editor.Services` (already a global using) and unused `using Beutl.Animation.Easings`. Correctness: - LayerMoveService.ApplyMove now writes TimelineLayer.ZIndex for the direct and shifted layers inside the committed MoveLayer transaction. Previously the View wrote these recorded properties after the service committed, leaking them into the next history entry and desyncing header color/name from elements on undo. LayerHeaderViewModel.UpdateZIndex no longer writes the model (the reactive Number binding reflects the service write) to avoid double-applying the shift. - ElementNudgeService commits via an injected UI-thread post so the debounce timer's HistoryManager.Commit and reactive fan-out no longer run on a thread-pool thread (was a DispatcherTimer before extraction). - ElementDuplicateService.DuplicateAtClickedPosition logs the swallowed exception instead of silently dropping I/O failures. - ClipboardJson.TryParse guards JsonNode.Parse against malformed clipboard input across the five paste sites (KeyFrame x2, ElementObject, ElementClipboard x2), which previously threw instead of returning the designed InvalidJson/Empty outcome. Tests: - Add LayerMoveService regression tests covering TimelineLayer shift and the undo round-trip (the no-leak guarantee). Verified: build 0 errors / 0 warnings on touched files; Beutl.UnitTests 3079 passed / 0 failed; dotnet format clean.
Member
Author
|
Pushed A local review surfaced that the branch did not compile (so the test suite, which references Build
Correctness
Tests: added Verified: build 0 errors / 0 warnings on touched files; |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f89404aac6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- ElementDuplicateService.DuplicateAtClickedPosition: move ObjectRegenerator.Regenerate (and the placement computation) inside the guarded try so a corrupt / unknown-plugin element returns DuplicateOutcome.Failed instead of letting the exception escape the paste command. Matches the existing guard in DuplicateAtPosition. (Codex P1) - AvaloniaClipboardGateway.SetAsync: dispose the DataTransfer via `using var` so the IDisposable is released after SetDataAsync. (github-code-quality) Verified: build 0 errors / 0 warnings on touched files; Beutl.UnitTests Editor area 847 passed / 0 failed; dotnet format clean.
Contributor
|
No TODO comments were found. |
Contributor
Minimum allowed line rate is |
yuto-trd
added a commit
that referenced
this pull request
Jun 5, 2026
- Module boundary map: split Beutl.Editor out as non-UI editor logic (no Avalonia) from the Beutl.Editor.Components / Beutl.Controls UI row, matching the #1845 routing of editing through Beutl.Editor.Services. - Document the /beutl-pre-pr skill alongside the build/test/format/coverage skills. - Add the install-dotnet.sh SessionStart hook (wired since #1844) to the hooks table.
yuto-trd
added a commit
that referenced
this pull request
Jun 5, 2026
- Module boundary map: split Beutl.Editor out as non-UI editor logic (no Avalonia) from the Beutl.Editor.Components / Beutl.Controls UI row, matching the #1845 routing of editing through Beutl.Editor.Services. - Document the /beutl-pre-pr skill alongside the build/test/format/coverage skills. - Add the install-dotnet.sh SessionStart hook (wired since #1844) to the hooks table.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Move scene / element mutation and
HistoryManager.Commitout of the timeline View and ViewModel layers and into a dedicated editing service layer underBeutl.Editor.Services. The View becomes input → service routing; the ViewModel becomes bindings → service routing.Scope (whole-feature, one PR)
This PR is the full implementation, not just the interface scaffolding — the original commit description ("nothing is wired into
IEditorContext.GetServiceyet") is from the first commit and no longer reflects the final state.Services added
All public, reachable via
IEditorContext.GetRequiredService<T>():ISceneTimeRangeService—Scene.Start/Scene.Durationediting.SetStart/SetEnd(one-shot for keyboard / menu) +UpdateStartDrag/UpdateEndDrag/CommitStartChange/CommitEndChange(plain-method drag path).IElementMoveService—Move(...)andDuplicateOrMove(...). ReturnsElementMoveOutcome(Moved/Duplicated/FellBackToMove/DuplicateOverlapsSource/None).IElementResizeService—Resize(scene, IReadOnlyList<ElementResizeRequest>). Multi-element resize in one transaction.IElementClipboardService—Copy/Cut/Paste. Dispatches onIClipboardGatewayformats soBeutl.Editorstays Avalonia-free.IElementDuplicateService— selection duplicate + Alt+drag position duplicate. WrapsDuplicateHelper;DuplicateAtClickedPositionruns a bounded spiral search (≤ 100 000steps) so a packed timeline cannot hang the caller.IElementLifecycleService—Exclude/Delete/SetEnabled/SetAccentColor/Split/Group/Ungroup.IElementNudgeService— debounced keyboard nudge.System.Threading.Timer(notDispatcherTimer) keeps it Avalonia-free;Flushis wired toHistoryManager.BeforeMutationso Undo / Redo never absorbs a pending nudge.ILayerMoveService—PlanMoveenumerates affected elements;CommitMovecloses the transaction.IKeyFrameMoveService— commit boundary for InlineAnimation drag releases.IClipboardGateway+ClipboardEntry+BeutlClipboardFormats— Avalonia-free clipboard abstraction.AvaloniaClipboardGatewaylives inBeutl.Editor.Components/Services/.EditorConstants.ElementFileExtension— single source of truth for"belm".View / ViewModel impact
TimelineTabView.axaml.cs— scene Start / Duration drag now routes throughUpdateXxxDrag+CommitXxxChange; noHistoryManager.Commitcall left in the View.ElementView.axaml.cs—_MoveBehaviorcallsMove(...)/DuplicateOrMove(...);_ResizeBehaviorcallsResize(...);EnableElementClickcallsSetEnabled. Theusing sessionblocks and theElementMoveOutcomeswitch shrunk significantly.LayerHeader.axaml.cs/InlineAnimationLayer.axaml.cs— commit boundaries moved intoILayerMoveService/IKeyFrameMoveService.TimelineTabViewModel.cs— ~350 net lines removed (paste, duplicate, nudge timer, spiral search).ElementViewModel.cs— ~120 net lines removed (cut/copy/exclude/delete/group/ungroup/split/color helpers).Multi-agent review cycle (applied)
After the initial implementation we ran
beutl-design-reviewer,beutl-reviewer,beutl-source-generator-impact, then took Copilot's PR review. The following follow-up commits land in this PR:refactor!: collapse drag-session services into plain methods—ISceneTimeRangeService,IElementMoveService,IElementResizeServiceno longer exposeIXxxDragSessionceremony. Drag callers use plain methods directly.style: add utf-8-bom to remaining Editor.Services files—.editorconfigcharset enforcement.fix: address design / pr review findings on editor services—ElementPasteOutcome→sealed record,imageAccentColorFactoryrequired (was silently defaulting toColors.Teal),EditorConstantsdeduplicates"belm",ElementNudgeServicedispose race fixed,KeyFrameMoveServiceTestsno longer vacuous,ElementLifecycleServiceTests.Group_SingleIdpins UndoCount.fix: address copilot review on element clipboard service—PasteElementsAsyncnow respects clicked frame / layer (was regressing to(0s, 0)),BeutlClipboardFormats.Textintroduced and routed to the native clipboard text slot inAvaloniaClipboardGateway.BREAKING CHANGE
IXxxDragSessioninterfaces and the correspondingBeginXxx/BeginMove/BeginResizefactory methods are removed. Plugins that wired into the session shape migrate to:UpdateStartDrag/UpdateEndDrag+ finalCommitStartChange/CommitEndChangeMove(scene, elems, deltaStart, deltaZIndex)orDuplicateOrMove(...)for Alt+dragResize(scene, requests)IElementResizeServiceno longer exposesResizeEdgeorClampToOriginalDuration(View hints the service never consumed).TimelineTabViewModelno longer exposes the inline editing helpers (PasteCore,DuplicateSelectedElements,NudgeSelectedElements,ScheduleNudgeCommit,OnNudgeCommitTick,FlushPendingNudgeCommit,SetStartTimeCore,SetEndTimeCore,RegenerateAndPlaceAtCorrectedPosition,CorrectPosition,HandleDuplicateException). Out-of-tree code that hooked them via reflection must route throughIEditorContext.GetServiceinstead.ElementViewModelno longer exposesSetClipboard,RemoveIdsFromElementSets, or the per-methodOnExclude/OnDelete/OnSplit/OnGroupSelectedElements/OnUngroupSelectedElementshelpers. Same migration path.TimelineTabView,ElementView,LayerHeader,InlineAnimationLayer) no longer touchesHistoryManagerdirectly. Plugins that subclass these views and override drag / release paths obtain anIEditorContext.GetService<I...Service>()instead of callinghistory.Commit.Test coverage
New NUnit fixtures under
tests/Beutl.UnitTests/Editor/Services/— one per service. Coverage includes single-commit boundaries, Apply / Revert throughHistoryManager.Undo, dispose / race semantics forElementNudgeService, paste-position regression, and text/plain output. ExistingDuplicateHelperTests/ObjectRegeneratorTestsare untouched.Verification
dotnet test Beutl.slnx -f net10.0 --filter "FullyQualifiedName~Editor.Services"— exercises all new services.