Skip to content

refactor(coordinator): delete dead AppCommands subjects, PasteboardActionRouter, and unused selectors#1340

Merged
datlechin merged 1 commit into
mainfrom
refactor/menu-action-cleanup
May 19, 2026
Merged

refactor(coordinator): delete dead AppCommands subjects, PasteboardActionRouter, and unused selectors#1340
datlechin merged 1 commit into
mainfrom
refactor/menu-action-cleanup

Conversation

@datlechin

Copy link
Copy Markdown
Member

Summary

Internal refactor with zero user-facing behavior change. Net: 12 insertions, 478 deletions across 7 files.

Two parallel code-explorer audits identified dead code and non-native indirection in the menu-action subsystem after PRs #1337 #1338. This PR removes both.

What's removed

Dead AppCommands PassthroughSubjects (zero senders, observers never fire)

  • AppCommands.shared.addNewRow
  • AppCommands.shared.deleteSelectedRows
  • AppCommands.shared.duplicateRow
  • AppCommands.shared.copySelectedRows
  • AppCommands.shared.pasteRows

Verified zero .send() callers across the codebase. The observers in MainContentCommandActions.setupNonMenuNotificationObservers were silently inert; deleting them clears a misleading code path that suggested AppKit views could trigger row operations via the bus (they can't, and don't need to).

PasteboardActionRouter

The router classified Cmd+C/Cmd+V into 3 cases (text / rows / table-names), but after recent refactors both .textCopy and .copyRows dispatch identically via NSApp.sendAction(_:to:nil:from:). The router became a 3-line guard sitting in its own file + enum + 132-line test suite.

Inlined into PasteboardCommands:

Button("Copy") {
    if NSApp.sendAction(#selector(NSText.copy(_:)), to: nil, from: nil) {
        return
    }
    if actions?.hasRowSelection == true {
        actions?.copySelectedRows()
    } else if actions?.hasTableSelection == true {
        actions?.copyTableNames()
    }
}

The native AppKit responder chain handles text view, KeyHandlingTableView, and any other responder that implements copy:. The two-line fallback handles the sidebar case (table names selected but sidebar isn't in the responder chain).

DataTabGridDelegate.dataGridUndo() and dataGridRedo()

Empty overrides of DataGridViewDelegate protocol methods that already had {} default implementations. Pure noise.

TableProResponderActions protocol bloat + comment dump

The file declared 20 @objc optional func selectors plus 100+ lines of documentation comments (violating CLAUDE.md's "no comments" rule). Only undo, redo were actually referenced. Trimmed to the 3 selectors the menu uses (undo, redo, and the new copyRowsAsTSV).

What changes (selector decoupling)

PasteboardCommands used to dispatch the explicit-rows shortcut via #selector(KeyHandlingTableView.copyRowsAsTSV(_:)), naming the concrete NSTableView subclass. Replaced with #selector(TableProResponderActions.copyRowsAsTSV(_:)), an @objc optional selector on a shared protocol. Same dispatch behavior; menu is no longer coupled to a concrete class.

What's intentionally left alone

  • CommandActionsRegistry — documented workaround for NSHostingController scene-focus fragmentation when toolbar buttons take focus. Real Apple SwiftUI/AppKit interop limitation, not a hack.
  • StructureViewActionHandler (closure bag) — correct pattern for SwiftUI View struct -> AppKit class dispatch (a View struct cannot be weakly referenced).
  • Live AppCommands subjects (refreshData, exportQueryResults, openSQLFiles, exportConnections, importConnections, importConnectionsFromApp, presentDatabaseTypeChooser) — all have verified senders.

Test plan

  • xcodebuild -project TablePro.xcodeproj -scheme TablePro test -skipPackagePluginValidation
  • Cmd+C in data grid with single row + cell focused -> cell value
  • Cmd+C with multi-row selection -> rows TSV
  • Cmd+Shift+C -> rows TSV regardless of cell focus
  • Cmd+C in SQL editor with selected text -> standard text copy
  • Cmd+V in data grid with editable tab -> row paste
  • Cmd+V in SQL editor -> text paste
  • Sidebar tables selected, Cmd+C -> table names
  • No regressions in undo (Cmd+Z) / redo (Cmd+Shift+Z)

@datlechin datlechin merged commit 81cf9eb into main May 19, 2026
2 checks passed
@datlechin datlechin deleted the refactor/menu-action-cleanup branch May 19, 2026 17:19
datlechin added a commit that referenced this pull request May 20, 2026
…cher (#1341)

* feat(datagrid): read-only cell viewer with unified interaction dispatcher (#1336)

* refactor(datagrid): route chevron action through cell interaction resolver (#1336)

* fix(coordinator): restore CodeEditTextView import dropped in #1340

* fix(datagrid): keep double-click in edit mode, pickers only via chevron (#1336)

* fix(datagrid): edit FK cell inline on double-click, remove FK picker popover (#1336)

* fix(datagrid): allow inline editing of foreign key cells (#1336)

* refactor(datagrid): trim CellContext to decision-relevant fields (#1336)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant