refactor: migrate undo system from custom stacks to NSUndoManager#599
Merged
refactor: migrate undo system from custom stacks to NSUndoManager#599
Conversation
- Fix primaryKeyChange undo closure capturing stale workingPrimaryKey - Fix batch insertion undo leaking insertedRowData entries - Clear undo history in loadSchema to prevent stale actions on table switch - Make undoManager private to prevent bypass of lastUndoResult contract
…ivity crash NSHostingView wrapping MainContentView creates a separate SwiftUI evaluation context. When updateNSView sets rootView, the inner hosting view re-evaluates MainContentView.body, which re-entrantly accesses @observable properties (QueryTabManager.tabs) already being read by the outer tree — triggering a Swift exclusivity violation crash. The PanelResizeHandle approach keeps MainContentView in the same SwiftUI tree, avoiding re-entrant access.
With groupsByEvent=false, NSUndoManager requires explicit beginUndoGrouping() before registerUndo(). Without it, registerUndo throws an exception that AppKit catches silently, causing all undo registration to be no-ops. This broke cell edit commit — changes were recorded in the tracking arrays but never registered with the undo manager. The default groupsByEvent=true auto-creates groups per run loop pass, which is correct for individual cell edits.
- rowInsertion: capture savedValues BEFORE undoRowInsertion clears them, register reverse AFTER redo repopulates insertedRowData - batchRowInsertion redo: restore insertedRowData for each re-inserted row so SQL generation produces correct INSERT values
StructureChangeManager: - S1: Distinguish undo-of-new-add vs redo-of-existing-delete in columnAdd/Delete, indexAdd/Delete, foreignKeyAdd/Delete by checking currentColumns/Indexes/FKs - S2: Update pendingChanges for newly-added items on undo of edit (add else branches) - S3: Call validate() after every undo/redo to keep validationErrors fresh DataChangeManager: - D1: Populate insertedRowData in batchRowInsertion redo path - D2: Shift existing row indices up when redoing row insertion (new shiftRowIndicesUp helper) - D3: Shift insertedRowData and modifiedCells keys in undoRowInsertion - D4: Use rows.contains instead of rows.first for batch deletion undo/redo detection - D5: Set groupsByEvent=false with explicit beginUndoGrouping/endUndoGrouping via registerUndoAction helper to prevent run-loop auto-grouping
…registration failures
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
1 participant
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.
Summary
Replaces two custom LIFO undo stack classes (
DataChangeUndoManager,StructureUndoManager) with Apple'sNSUndoManager. This is a full migration — the custom classes are deleted.What changes:
isRedoingflag)levelsOfUndo = 100preserves the existing stack depth limitWhat stays the same:
Phase 1 — StructureChangeManager (-196 lines):
StructureUndoManager.swiftSchemaUndoActionenum into StructureChangeManagerapplySchemaUndo(_:)— single method that applies inverse + registers reversepush()calls →registerUndo(withTarget:)+setActionName()Phase 2 — DataChangeManager (-274 lines):
DataChangeUndoManager.swiftapplyDataUndo(_:)for all 5 UndoAction casesundoLastChange()/redoLastChange()withUndoResultreturn typegroupsByEvent = falseprevents auto-grouping of independent editsbeginUndoGrouping()/endUndoGrouping()Net: -635 lines of custom undo code deleted
Test plan