Skip to content

refactor(datagrid): collapse cachedTableRows mirror into tableRowsProvider() reads #934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
datlechin merged 1 commit into from
Apr 29, 2026
Merged

refactor(datagrid): collapse cachedTableRows mirror into tableRowsProvider() reads #934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions TablePro/Views/Results/DataGridCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]?
Expand All @@ -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 }
Comment on lines +32 to +33
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist column layout without depending on live row provider

Using tableRowsProvider() as the source of column names makes layout persistence fail during teardown paths where row data is removed before SwiftUI dismantles the grid. In performClose, removeTableRows(for:) runs before tabs are removed, so dismantleNSView can call persistColumnLayoutToStorage() after the provider already returns an empty TableRows; this new guard then exits early and skips saving widths/order for the current table. Previously the coordinator’s cached snapshot allowed persistence even after store teardown.

Useful? React with 👍 / 👎.


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)
}
Expand Down Expand Up @@ -170,7 +170,6 @@ final class TableViewCoordinator: NSObject, NSTableViewDelegate, NSTableViewData

private func releaseData() {
overlayEditor?.dismiss(commit: false)
cachedTableRows = TableRows()
rowVisualStateCache.removeAll()
displayCache.removeAll()
columnDisplayFormats = []
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion TablePro/Views/Results/DataGridView+RowActions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ extension TableViewCoordinator {
tableRowsMutator { rows in
_ = rows.remove(at: IndexSet(integer: index))
}
cachedTableRows = tableRowsProvider()
updateCache()
tableView?.reloadData()
}
Expand Down
3 changes: 0 additions & 3 deletions TablePro/Views/Results/DataGridView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -226,7 +225,6 @@ struct DataGridView: NSViewRepresentable {
}

let latestRows = tableRowsProvider()
coordinator.cachedTableRows = latestRows
let rowDisplayCount = sortedIDs?.count ?? latestRows.count
let columnCount = latestRows.columns.count

Expand Down Expand Up @@ -627,7 +625,6 @@ struct DataGridView: NSViewRepresentable {
coordinator.themeObserver = nil
}
coordinator.tableRowsController.detach()
coordinator.cachedTableRows = TableRows()
}

func makeCoordinator() -> TableViewCoordinator {
Expand Down
2 changes: 1 addition & 1 deletion TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 5 additions & 6 deletions TablePro/Views/Results/Extensions/DataGridView+Sort.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}()
Expand Down Expand Up @@ -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()
Expand Down
Loading