From 5d6e954fed13718a300044eb3c691c6cf0acafd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ng=C3=B4=20Qu=E1=BB=91c=20=C4=90=E1=BA=A1t?= Date: Wed, 29 Apr 2026 11:27:29 +0700 Subject: [PATCH] refactor(datagrid): collapse cachedTableRows mirror into tableRowsProvider() reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the cachedTableRows stored property on TableViewCoordinator. It mirrored what tableRowsProvider() returns and was kept in sync via four writers (initial load, updateNSView, updateCache, releaseData). The two-sources-of-truth setup was a parking-lot item from PR #931 — under it, every reader had to trust the cache had been refreshed, and forgetting to refresh produced stale-value bugs. Each reader now captures rows once at the top of its scope: - persistColumnLayoutToStorage - updateCache (the only true cache update; cachedRowCount/Count still derived) - releaseData (just drops the now-removed field) - DataGridView+CellPaste (anchor column count check) - DataGridView+Sort (sortDescriptorsDidChange + menuNeedsUpdate header context menu) - DataGridView+RowActions (undoInsertRow no longer needs the explicit refresh) Net -6 LOC, one fewer field on the coordinator, and no possibility of cache/source drift. Smoke-tested: column header context menu, click-to-sort, cell paste, undo-insert all behave as before. --- TablePro/Views/Results/DataGridCoordinator.swift | 15 +++++++-------- .../Views/Results/DataGridView+RowActions.swift | 1 - TablePro/Views/Results/DataGridView.swift | 3 --- .../Extensions/DataGridView+CellPaste.swift | 2 +- .../Results/Extensions/DataGridView+Sort.swift | 11 +++++------ 5 files changed, 13 insertions(+), 19 deletions(-) diff --git a/TablePro/Views/Results/DataGridCoordinator.swift b/TablePro/Views/Results/DataGridCoordinator.swift index e8d37f36b..06665fc74 100644 --- a/TablePro/Views/Results/DataGridCoordinator.swift +++ b/TablePro/Views/Results/DataGridCoordinator.swift @@ -9,7 +9,6 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData { var tableRowsProvider: @MainActor () -> TableRows = { TableRows() } var tableRowsMutator: @MainActor (@MainActor (inout TableRows) -> Void) -> Void = { _ in } - var cachedTableRows: TableRows = TableRows() var changeManager: AnyChangeManager var isEditable: Bool var sortedIDs: [RowID]? @@ -30,14 +29,15 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData func persistColumnLayoutToStorage() { guard tabType == .table else { return } guard let tableView, let connectionId, let tableName, !tableName.isEmpty else { return } - guard !cachedTableRows.columns.isEmpty else { return } + let tableRows = tableRowsProvider() + guard !tableRows.columns.isEmpty else { return } var widths: [String: CGFloat] = [:] var order: [String] = [] for column in tableView.tableColumns where column.identifier.rawValue != "__rowNumber__" { guard let colIndex = DataGridView.dataColumnIndex(from: column.identifier), - colIndex < cachedTableRows.columns.count else { continue } - let name = cachedTableRows.columns[colIndex] + colIndex < tableRows.columns.count else { continue } + let name = tableRows.columns[colIndex] widths[name] = column.width order.append(name) } @@ -170,7 +170,6 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData private func releaseData() { overlayEditor?.dismiss(commit: false) - cachedTableRows = TableRows() rowVisualStateCache.removeAll() displayCache.removeAll() columnDisplayFormats = [] @@ -204,9 +203,9 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData } func updateCache() { - cachedTableRows = tableRowsProvider() - cachedRowCount = sortedIDs?.count ?? cachedTableRows.count - cachedColumnCount = cachedTableRows.columns.count + let tableRows = tableRowsProvider() + cachedRowCount = sortedIDs?.count ?? tableRows.count + cachedColumnCount = tableRows.columns.count } func applyInsertedRows(_ indices: IndexSet) { diff --git a/TablePro/Views/Results/DataGridView+RowActions.swift b/TablePro/Views/Results/DataGridView+RowActions.swift index 7a70d8235..87cd7b5d0 100644 --- a/TablePro/Views/Results/DataGridView+RowActions.swift +++ b/TablePro/Views/Results/DataGridView+RowActions.swift @@ -29,7 +29,6 @@ extension TableViewCoordinator { tableRowsMutator { rows in _ = rows.remove(at: IndexSet(integer: index)) } - cachedTableRows = tableRowsProvider() updateCache() tableView?.reloadData() } diff --git a/TablePro/Views/Results/DataGridView.swift b/TablePro/Views/Results/DataGridView.swift index 4b0c877b1..8eeba0714 100644 --- a/TablePro/Views/Results/DataGridView.swift +++ b/TablePro/Views/Results/DataGridView.swift @@ -108,7 +108,6 @@ struct DataGridView: NSViewRepresentable { rowNumberColumn.isHidden = !configuration.showRowNumbers let initialRows = tableRowsProvider() - context.coordinator.cachedTableRows = initialRows context.coordinator.isRebuildingColumns = true for (index, columnName) in initialRows.columns.enumerated() { @@ -226,7 +225,6 @@ struct DataGridView: NSViewRepresentable { } let latestRows = tableRowsProvider() - coordinator.cachedTableRows = latestRows let rowDisplayCount = sortedIDs?.count ?? latestRows.count let columnCount = latestRows.columns.count @@ -627,7 +625,6 @@ struct DataGridView: NSViewRepresentable { coordinator.themeObserver = nil } coordinator.tableRowsController.detach() - coordinator.cachedTableRows = TableRows() } func makeCoordinator() -> TableViewCoordinator { diff --git a/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift b/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift index edcc86102..c209dfd97 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift @@ -16,7 +16,7 @@ extension TableViewCoordinator { guard !grid.isEmpty, grid[0].count > 1 || grid.count > 1 else { return false } let maxRow = min(anchorRow + grid.count, cachedRowCount) - let maxCol = min(anchorColumn + (grid.first?.count ?? 0), cachedTableRows.columns.count) + let maxCol = min(anchorColumn + (grid.first?.count ?? 0), tableRowsProvider().columns.count) guard anchorRow < maxRow, anchorColumn < maxCol else { return false } let undoManager = tableView?.window?.undoManager diff --git a/TablePro/Views/Results/Extensions/DataGridView+Sort.swift b/TablePro/Views/Results/Extensions/DataGridView+Sort.swift index f70f163e6..c9026d896 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Sort.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Sort.swift @@ -15,7 +15,7 @@ extension TableViewCoordinator { guard let sortDescriptor = tableView.sortDescriptors.first, let key = sortDescriptor.key, let columnIndex = DataGridView.dataColumnIndex(from: NSUserInterfaceItemIdentifier(key)), - columnIndex >= 0 && columnIndex < cachedTableRows.columns.count else { + columnIndex >= 0 && columnIndex < tableRowsProvider().columns.count else { return } @@ -62,11 +62,11 @@ extension TableViewCoordinator { let column = tableView.tableColumns[columnIndex] if column.identifier.rawValue == "__rowNumber__" { return } - // Derive base column name from stable identifier (avoids sort indicator in title) + let tableRows = tableRowsProvider() let baseName: String = { if let idx = DataGridView.dataColumnIndex(from: column.identifier), - idx < cachedTableRows.columns.count { - return cachedTableRows.columns[idx] + idx < tableRows.columns.count { + return tableRows.columns[idx] } return column.title }() @@ -103,9 +103,8 @@ extension TableViewCoordinator { filterItem.target = self menu.addItem(filterItem) - // "Display As" submenu for value display format overrides if let dataColumnIndex = DataGridView.dataColumnIndex(from: column.identifier) { - let columnType = dataColumnIndex < cachedTableRows.columnTypes.count ? cachedTableRows.columnTypes[dataColumnIndex] : nil + let columnType = dataColumnIndex < tableRows.columnTypes.count ? tableRows.columnTypes[dataColumnIndex] : nil let applicableFormats = ValueDisplayFormat.applicableFormats(for: columnType) if applicableFormats.count > 1 { let displaySubmenu = NSMenu()