From 1b96f8d7141aa26baa44b31bef4c7fe8c2aa83dc Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 15 Oct 2026 16:26:02 +0700 Subject: [PATCH 1/2] fix(datagrid): save NULL when a date or number cell is cleared An emptied cell was staged as an empty string whatever the column type, so MySQL without strict mode stored a cleared DATETIME as the zero date. Typed text is now read for its column: a type with no empty value stages NULL, and a NOT NULL column of such a type refuses the edit. --- CHANGELOG.md | 6 + TablePro/Core/Services/ColumnType.swift | 40 ++- .../PluginCellValue+EnteredText.swift | 20 ++ TablePro/Models/Query/TableRows.swift | 24 ++ TablePro/Models/UI/MultiRowEditState.swift | 27 +- .../MainContentCoordinator+SidebarSave.swift | 11 +- .../MainContentView+EventHandlers.swift | 8 +- TablePro/Views/Results/DataGridRowView.swift | 29 +- .../Extensions/DataGridView+CellCommit.swift | 21 +- .../Extensions/DataGridView+CellPaste.swift | 53 ++- .../Extensions/DataGridView+FillColumn.swift | 25 +- .../FieldEditors/FieldEditorContext.swift | 7 +- .../FieldEditors/FieldMenuContent.swift | 7 +- .../Services/ColumnTypeEmptyValueTests.swift | 126 +++++++ .../MultiRowEditStateClearedFieldTests.swift | 101 ++++++ .../Main/SidebarSaveClearedValueTests.swift | 70 ++++ .../Extensions/CellPasteRoutingTests.swift | 20 ++ .../Extensions/ClearedCellCommitTests.swift | 328 ++++++++++++++++++ .../Results/Extensions/FillColumnTests.swift | 7 +- .../FieldEditorContextPolicyTests.swift | 19 + 20 files changed, 898 insertions(+), 51 deletions(-) create mode 100644 TablePro/Extensions/PluginCellValue+EnteredText.swift create mode 100644 TableProTests/Core/Services/ColumnTypeEmptyValueTests.swift create mode 100644 TableProTests/Models/MultiRowEditStateClearedFieldTests.swift create mode 100644 TableProTests/Views/Main/SidebarSaveClearedValueTests.swift create mode 100644 TableProTests/Views/Results/Extensions/ClearedCellCommitTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index eec8d803cc..469fd66252 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Smaller updates: about 6 MB from any of the last five releases and 21 MB from older ones, down from 34 MB. - Several statements run together on PostgreSQL from iPhone and iPad commit one at a time, and a failure keeps the ones before it. (#3399) - MySQL dumps write TIMESTAMP values in UTC and turn off foreign key checks while they load, as mysqldump does. +- Set Value > Empty and Set EMPTY offered only on columns that can hold an empty value, not on dates, numbers or booleans. +- Clearing a date, number or boolean cell in a NOT NULL column keeps the stored value instead of saving an empty string. +- Clearing a Redis key's TTL in the grid removes its expiry. ### Removed @@ -120,6 +123,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - GitHub Copilot language server pinned to a reviewed version and checksum instead of the latest npm release. - One unreadable setting, or one unknown AI provider, resetting its whole settings group to defaults. +- Clearing a date, number or boolean cell saving an empty string, stored as `0000-00-00 00:00:00` on MySQL, instead of NULL. +- Pasting a copied NULL into a date, number or boolean cell writing the text NULL, stored as a zero date or 0 on MySQL. +- Empty line in pasted cells moving the values below it up a row, or cutting off the columns after it. ### Security diff --git a/TablePro/Core/Services/ColumnType.swift b/TablePro/Core/Services/ColumnType.swift index 5a8523a98b..8415e0b895 100644 --- a/TablePro/Core/Services/ColumnType.swift +++ b/TablePro/Core/Services/ColumnType.swift @@ -2,8 +2,8 @@ // ColumnType.swift // TablePro // -// Column type metadata for type-aware formatting and display. -// Driver-specific type mapping lives in each plugin; this enum is display-only. +// Column type metadata for type-aware formatting, display and editing. +// Driver-specific type mapping lives in each plugin. // import Foundation @@ -163,6 +163,42 @@ enum ColumnType: Equatable, Sendable { } } + /// Whether `''` is a value of this type, so an emptied cell can be written as one. + /// + /// A date, number, boolean, JSON document, geometry or array has no empty value. Sent `''`, + /// MySQL without strict mode stores its zero value (`0000-00-00 00:00:00`, `0`), SQL Server + /// stores `1900-01-01` or `0`, SQLite keeps the text, and a strict MySQL or PostgreSQL refuses + /// the save. An empty SET is the empty set, and an enum has an empty value only when one of + /// its labels is the empty string. + var hasEmptyValue: Bool { + switch self { + case .text(let rawType): + return !Self.nonCharacterTextTypes.contains(Self.baseTypeName(rawType)) + case .set, .blob: + return true + case .enumType(_, let values): + return values?.contains("") ?? false + case .date, .timestamp, .datetime, .integer, .decimal, .boolean, .json, .spatial, .array: + return false + } + } + + /// Types the classifier files under `.text`, either by name or because nothing else matched, + /// that hold no empty string either. + private static let nonCharacterTextTypes: Set = [ + "YEAR", "UUID", "TIMEUUID", "UNIQUEIDENTIFIER", + "INET", "CIDR", "MACADDR", "MACADDR8", "IPV4", "IPV6", "INTERVAL" + ] + + /// `Nullable(UUID)`, `year(4)` and `INTERVAL DAY TO SECOND` all name their type in the first word. + private static func baseTypeName(_ rawType: String?) -> String { + guard var name = rawType?.uppercased() else { return "" } + while let wrapper = ["NULLABLE(", "LOWCARDINALITY("].first(where: { name.hasPrefix($0) }) { + name.removeFirst(wrapper.count) + } + return String(name.prefix { $0.isLetter || $0.isNumber || $0 == "_" }) + } + /// The element type of an array column, if this is one var arrayElement: ColumnType? { switch self { diff --git a/TablePro/Extensions/PluginCellValue+EnteredText.swift b/TablePro/Extensions/PluginCellValue+EnteredText.swift new file mode 100644 index 0000000000..1cdc6350ab --- /dev/null +++ b/TablePro/Extensions/PluginCellValue+EnteredText.swift @@ -0,0 +1,20 @@ +// +// PluginCellValue+EnteredText.swift +// TablePro +// + +import Foundation +import TableProPluginKit + +internal extension PluginCellValue { + /// What text entered for a column means as a value. + /// + /// Emptying a cell whose type has no empty value asks for no value, which is NULL. Writing `''` + /// there turned a cleared `deleted_at` into `0000-00-00 00:00:00` on MySQL, so the row still + /// read as deleted. A text column keeps `''`, which is a real value there. + static func entered(_ text: String?, columnType: ColumnType?) -> PluginCellValue { + guard let text else { return .null } + guard text.isEmpty, let columnType, !columnType.hasEmptyValue else { return .text(text) } + return .null + } +} diff --git a/TablePro/Models/Query/TableRows.swift b/TablePro/Models/Query/TableRows.swift index eb8b782ba7..fef736e0de 100644 --- a/TablePro/Models/Query/TableRows.swift +++ b/TablePro/Models/Query/TableRows.swift @@ -94,6 +94,30 @@ struct TableRows: Sendable { return !ColumnDefaultLiteral.isNull(defaultValue) } + /// The column's type with the labels its catalog declared, which a result set alone never carries. + func editingColumnType(at index: Int) -> ColumnType? { + guard columnTypes.indices.contains(index) else { return nil } + let columnType = columnTypes[index] + guard columns.indices.contains(index), + let labels = columnEnumValues[columns[index]], !labels.isEmpty else { return columnType } + return columnType.withAllowedValues(labels) + } + + /// What text entered for a column stages, or nil when the column holds neither it nor NULL. + /// + /// An emptied date in a NOT NULL column has no value to write. MySQL turns NULL there into the + /// current time when `explicit_defaults_for_timestamp` is off, and `''` into the zero date, so + /// the edit is refused rather than staged. + func enteredValue(_ text: String?, forColumn index: Int) -> PluginCellValue? { + let value = PluginCellValue.entered(text, columnType: editingColumnType(at: index)) + guard value.isNull, refusesNull(columnAt: index) else { return value } + return nil + } + + func refusesNull(columnAt index: Int) -> Bool { + columns.indices.contains(index) && columnNullable[columns[index]] == false + } + func value(at row: Int, column: Int) -> PluginCellValue { guard row >= 0, row < rows.count else { return .null } return rows[row][column] diff --git a/TablePro/Models/UI/MultiRowEditState.swift b/TablePro/Models/UI/MultiRowEditState.swift index 0f683856f2..6c700a9dda 100644 --- a/TablePro/Models/UI/MultiRowEditState.swift +++ b/TablePro/Models/UI/MultiRowEditState.swift @@ -44,6 +44,9 @@ struct FieldEditState: Identifiable { /// Save never writes. var isServerOwned: Bool = false + /// False for a NOT NULL column. Nil from the schema reads as nullable, as the grid's menus do. + var isNullable = true + /// The value already differs from the loaded schema because the edit is recorded elsewhere. var hasCommittedEdit: Bool = false @@ -130,6 +133,7 @@ final class MultiRowEditState: ObservableObject { @Published private(set) var absentCells: [Set] = [] @Published private(set) var columns: [String] = [] @Published private(set) var columnTypes: [ColumnType] = [] + private var nonNullableColumns: Set = [] var hasEdits: Bool { fields.contains { $0.hasEdit } @@ -146,6 +150,7 @@ final class MultiRowEditState: ObservableObject { primaryKeyColumns: Set = [], foreignKeyColumns: Set = [], serverOwnedColumns: Set = [], + nonNullableColumns: Set = [], displayFormats: [ValueDisplayFormat?] = [], binaryColumns: Set = [] ) { @@ -159,6 +164,7 @@ final class MultiRowEditState: ObservableObject { self.absentCells = absentCells self.columns = columns self.columnTypes = columnTypes + self.nonNullableColumns = nonNullableColumns var newFields: [FieldEditState] = [] @@ -228,6 +234,7 @@ final class MultiRowEditState: ObservableObject { isPrimaryKey: primaryKeyColumns.contains(columnName), isForeignKey: foreignKeyColumns.contains(columnName), isServerOwned: serverOwnedColumns.contains(columnName), + isNullable: !nonNullableColumns.contains(columnName), originalValue: originalValue, hasMultipleValues: hasMultipleValues, pendingValue: pendingValue, @@ -337,13 +344,22 @@ final class MultiRowEditState: ObservableObject { guard index < fields.count else { return } let hadPendingEdit = fields[index].hasEdit let original = fields[index].originalValue - let pending = Self.resolvePendingValue(value, original: original, isJson: fields[index].isJson) + var pending = Self.resolvePendingValue(value, original: original, isJson: fields[index].isJson) + var staged = pending.map { PluginCellValue.entered($0, columnType: fields[index].columnTypeEnum) } + /// A NOT NULL column whose type has no empty value holds nothing an emptied field could + /// mean, so the field goes back to its stored value, as the grid's cell does. + if staged == .null, !fields[index].isNullable { + pending = nil + staged = nil + } fields[index].pendingValue = pending fields[index].isPendingNull = false fields[index].isPendingDefault = false fields[index].isPendingRemoval = false - if pending != nil { - onFieldChanged?(index, PluginCellValue.fromOptional(pending), .typing) + /// The field keeps its empty text while the change it stages is NULL: swapping the editor + /// for the pending NULL pill would take the field away in the middle of typing a new value. + if let staged { + onFieldChanged?(index, staged, .typing) } else if hadPendingEdit { /// `originalValue` is nil for three different situations, and only one of them is a /// value: a stored NULL, a selection whose rows do not agree, and a field the rows do @@ -391,7 +407,9 @@ final class MultiRowEditState: ObservableObject { updateField(at: columnIndex, value: value) return } - onDetachedFieldChanged?(columnIndex, PluginCellValue.fromOptional(value), rowIDs) + let staged = PluginCellValue.entered(value, columnType: columnTypes[safe: columnIndex]) + if staged.isNull, columns[safe: columnIndex].map(nonNullableColumns.contains) == true { return } + onDetachedFieldChanged?(columnIndex, staged, rowIDs) } private static func resolvePendingValue(_ value: String?, original: String?, isJson: Bool) -> String? { @@ -497,6 +515,7 @@ final class MultiRowEditState: ObservableObject { absentCells = [] columns = [] columnTypes = [] + nonNullableColumns = [] } /// Get all edited fields with their new values diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+SidebarSave.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+SidebarSave.swift index e921d1c7a8..32fb5df80d 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+SidebarSave.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+SidebarSave.swift @@ -40,6 +40,13 @@ extension MainContentCoordinator { guard !editedFields.isEmpty else { return [] } let tableRows = tabSessionRegistry.tableRows(for: tab.id) + /// Typed text is read for its column, as the grid reads a cell, and text a NOT NULL column can + /// hold neither as itself nor as NULL writes nothing. An explicit NULL stays NULL. + let writes: [(field: InspectorFieldEdit, value: PluginCellValue)] = editedFields.compactMap { field in + guard !field.removesField, let text = field.newValue else { return (field, .null) } + return tableRows.enteredValue(text, forColumn: field.columnIndex).map { (field, $0) } + } + guard !writes.isEmpty else { return [] } let displayIDs = activeGridDisplayIDs let changes: [RowChange] = selectionState.indices.sorted().compactMap { rowIndex -> RowChange? in guard let resolvedRow = DisplayRowMapping.row( @@ -51,7 +58,7 @@ extension MainContentCoordinator { return RowChange( rowID: resolvedRow.id, type: .update, - cellChanges: editedFields.map { field in + cellChanges: writes.map { field, newValue in let oldValue: PluginCellValue = field.columnIndex < originalRow.count ? originalRow[field.columnIndex] : .null @@ -59,7 +66,7 @@ extension MainContentCoordinator { columnIndex: field.columnIndex, columnName: field.columnName, oldValue: oldValue, - newValue: field.removesField ? .null : PluginCellValue.fromOptional(field.newValue), + newValue: newValue, oldIsAbsent: resolvedRow.isAbsent(field.columnIndex), newIsAbsent: field.removesField ) diff --git a/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift b/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift index 52f4c2e89f..a238aa113f 100644 --- a/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift +++ b/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift @@ -180,12 +180,7 @@ extension MainContentView { } let allRows = selectedRows.map { Array($0.values) } - var columnTypes = tableRows.columnTypes - for (i, col) in tableRows.columns.enumerated() where i < columnTypes.count { - if let values = tableRows.columnEnumValues[col], !values.isEmpty { - columnTypes[i] = columnTypes[i].withAllowedValues(values) - } - } + let columnTypes = tableRows.columnTypes.indices.compactMap { tableRows.editingColumnType(at: $0) } if !changeManager.hasChanges { trailingPaneState.inspector.editState.clearEdits() @@ -219,6 +214,7 @@ extension MainContentView { primaryKeyColumns: pkColumns, foreignKeyColumns: fkColumns, serverOwnedColumns: tableRows.generatedColumns.union(changeManager.unwritableColumns(among: tableRows.columns)), + nonNullableColumns: Set(tableRows.columnNullable.compactMap { $0.value ? nil : $0.key }), displayFormats: inspectorDisplayFormats(for: tab, columns: tableRows.columns, types: tableRows.columnTypes), binaryColumns: MultiRowEditState.binaryColumns(in: allRows) ) diff --git a/TablePro/Views/Results/DataGridRowView.swift b/TablePro/Views/Results/DataGridRowView.swift index d08fbb9b49..05190452b7 100644 --- a/TablePro/Views/Results/DataGridRowView.swift +++ b/TablePro/Views/Results/DataGridRowView.swift @@ -5,6 +5,7 @@ import AppKit import Combine +import TableProPluginKit @MainActor class DataGridRowView: NSTableRowView { @@ -690,9 +691,12 @@ class DataGridRowView: NSTableRowView { && coordinator.isColumnWritable(tableRows.columns[dataColumnIndex]) guard coordinator.isEditable, namesWritableColumn else { return } - let setValueItem = NSMenuItem(title: String(localized: "Set Value"), action: nil, keyEquivalent: "") - setValueItem.submenu = buildSetValueMenu(dataColumnIndex: dataColumnIndex, tableRows: tableRows) - menu.addItem(setValueItem) + let setValueMenu = buildSetValueMenu(dataColumnIndex: dataColumnIndex, tableRows: tableRows) + if !setValueMenu.items.isEmpty { + let setValueItem = NSMenuItem(title: String(localized: "Set Value"), action: nil, keyEquivalent: "") + setValueItem.submenu = setValueMenu + menu.addItem(setValueItem) + } guard coordinator.supportsFieldRemoval, coordinator.displayRow(at: rowIndex)?.isAbsent(dataColumnIndex) == false else { return } @@ -703,14 +707,18 @@ class DataGridRowView: NSTableRowView { menu.addItem(removeFieldItem) } - private func buildSetValueMenu(dataColumnIndex: Int, tableRows: TableRows) -> NSMenu { + /// Empty is offered only where `''` is a value of the column's type, the way NULL is offered + /// only where the column takes it. On a date it wrote the server's zero date. + func buildSetValueMenu(dataColumnIndex: Int, tableRows: TableRows) -> NSMenu { let setValueMenu = NSMenu() - let emptyItem = NSMenuItem( - title: String(localized: "Empty"), action: #selector(setEmptyValue(_:)), keyEquivalent: "") - emptyItem.representedObject = dataColumnIndex - emptyItem.target = self - setValueMenu.addItem(emptyItem) + if tableRows.editingColumnType(at: dataColumnIndex)?.hasEmptyValue ?? true { + let emptyItem = NSMenuItem( + title: String(localized: "Empty"), action: #selector(setEmptyValue(_:)), keyEquivalent: "") + emptyItem.representedObject = dataColumnIndex + emptyItem.target = self + setValueMenu.addItem(emptyItem) + } let columnName = dataColumnIndex < tableRows.columns.count ? tableRows.columns[dataColumnIndex] @@ -839,9 +847,10 @@ class DataGridRowView: NSTableRowView { coordinator?.removeField(row: rowIndex, columnIndex: columnIndex) } + /// Written as `''` itself rather than read as typed text, which would turn it into NULL. @objc private func setEmptyValue(_ sender: NSMenuItem) { guard let columnIndex = sender.representedObject as? Int else { return } - coordinator?.setCellValueAtColumn("", at: rowIndex, columnIndex: columnIndex) + coordinator?.commitTypedCellEdit(row: rowIndex, columnIndex: columnIndex, newValue: .text("")) } @objc private func setDefaultValue(_ sender: NSMenuItem) { diff --git a/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift b/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift index 4117050a8a..d9f0ceaac8 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift @@ -10,8 +10,21 @@ import TableProPluginKit private let cellCommitLogger = Logger(subsystem: "com.TablePro", category: "DataGrid") extension TableViewCoordinator { + /// Every caller hands over text a person entered, so it is read for the column it lands in: + /// an emptied date, number or boolean is NULL, an emptied text cell is `''`. A NOT NULL column + /// that can hold neither keeps its value, and the beep says the edit was refused. func commitCellEdit(row: Int, columnIndex: Int, newValue: String?) { - commitTypedCellEdit(row: row, columnIndex: columnIndex, newValue: PluginCellValue.fromOptional(newValue)) + if !commitEnteredText(newValue, row: row, columnIndex: columnIndex) { + NSSound.beep() + } + } + + /// False when the column holds neither the text nor NULL in its place, and nothing was staged. + @discardableResult + func commitEnteredText(_ text: String?, row: Int, columnIndex: Int) -> Bool { + guard let value = tableRowsProvider().enteredValue(text, forColumn: columnIndex) else { return false } + commitTypedCellEdit(row: row, columnIndex: columnIndex, newValue: value) + return true } func commitTypedCellEdit(row: Int, columnIndex: Int, newValue typedNewValue: PluginCellValue) { @@ -42,7 +55,11 @@ extension TableViewCoordinator { /// A detached editor outlives a sort, a value filter and a display-format change, none of which /// close it, and a display row names a different record after any of them. func commitCellEdit(rowID: RowID, columnIndex: Int, newValue: String?) { - commitTypedCellEdit(rowID: rowID, columnIndex: columnIndex, newValue: PluginCellValue.fromOptional(newValue)) + guard let value = tableRowsProvider().enteredValue(newValue, forColumn: columnIndex) else { + NSSound.beep() + return + } + commitTypedCellEdit(rowID: rowID, columnIndex: columnIndex, newValue: value) } func commitTypedCellEdit(rowID: RowID, columnIndex: Int, newValue typedNewValue: PluginCellValue) { diff --git a/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift b/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift index ea274ff522..59d21ed9ad 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift @@ -32,13 +32,15 @@ extension TableViewCoordinator { } let maxRow = min(anchorRow + grid.count, cachedRowCount) - let maxCol = min(anchorPosition + (grid.first?.count ?? 0), presentedColumnCount) + let maxCol = min(anchorPosition + Self.blockWidth(grid), presentedColumnCount) guard anchorRow < maxRow, anchorPosition < maxCol else { return false } let undoManager = tableView?.window?.undoManager undoManager?.beginUndoGrouping() undoManager?.setActionName(String(localized: "Paste Cells")) + let tableRows = tableRowsProvider() + var refusedCell = false for (gridRow, rowValues) in grid.enumerated() { let targetRow = anchorRow + gridRow guard targetRow < maxRow else { break } @@ -48,11 +50,17 @@ extension TableViewCoordinator { let targetPosition = anchorPosition + gridCol guard targetPosition < maxCol else { break } guard let targetCol = dataColumnIndex(atDisplayPosition: targetPosition) else { continue } - commitCellEdit(row: targetRow, columnIndex: targetCol, newValue: cellValue) + let text = Self.pastedText(cellValue, columnType: tableRows.editingColumnType(at: targetCol)) + if !commitEnteredText(text, row: targetRow, columnIndex: targetCol) { + refusedCell = true + } } } undoManager?.endUndoGrouping() + if refusedCell { + NSSound.beep() + } tableView?.reloadData() return true @@ -71,27 +79,56 @@ extension TableViewCoordinator { guard let text = ClipboardService.shared.readText(), !text.isEmpty else { return nil } guard (text as NSString).length <= Self.maxCellPasteLength else { return nil } - let grid = text.components(separatedBy: "\n") - .filter { !$0.isEmpty } - .map { $0.components(separatedBy: "\t") } + let grid = Self.pastedLines(text).map { $0.components(separatedBy: "\t") } guard let firstRow = grid.first else { return nil } /// Whole rows are routed away by the result's own column count, not the presented one. /// `TSVRowParser` fills from data index 0 and knows nothing about which columns are on /// screen, so handing it a block as wide as the *visible* run writes the fields into hidden /// or wrong columns. A block that matches the visible width but not the result's stays on - /// the cell path, where every field is placed by display position. + /// the cell path, where every field is placed by display position. Row paste skips empty + /// lines, so they do not decide the shape. let isSingleValue = grid.count == 1 && firstRow.count == 1 let dataColumnCount = tableRowsProvider().columns.count - if !isSingleValue, dataColumnCount > 0, grid.allSatisfy({ $0.count == dataColumnCount }) { + let rowLines = grid.filter { $0 != [""] } + if !isSingleValue, dataColumnCount > 0, !rowLines.isEmpty, + rowLines.allSatisfy({ $0.count == dataColumnCount }) { return nil } guard let anchorPosition = displayPosition(ofDataColumnIndex: anchorColumn) else { return nil } let maxRow = min(anchorRow + grid.count, cachedRowCount) - let maxCol = min(anchorPosition + firstRow.count, presentedColumnCount) + let maxCol = min(anchorPosition + Self.blockWidth(grid), presentedColumnCount) guard anchorRow < maxRow, anchorPosition < maxCol else { return nil } return grid } + + /// The widest line, since an empty line holds one field and may come first. + static func blockWidth(_ grid: [[String]]) -> Int { + grid.map(\.count).max() ?? 0 + } + + /// The grid copies a NULL cell as the text `NULL`. A date, number or boolean cannot hold those + /// four letters, so there they paste as NULL rather than the server's zero date or an error. A + /// text column keeps them: the same text is what a stored `'NULL'` copies as. + static func pastedText(_ field: String, columnType: ColumnType?) -> String? { + guard field == "NULL", let columnType, !columnType.hasEmptyValue, + columnType.enumValues?.contains(field) != true else { return field } + return nil + } + + /// An empty line inside the block is an empty value and keeps the rows below it in place. + /// Dropping it moved every later value up a row. Empty lines at the end are the line breaks a + /// spreadsheet ends its copy with, not rows. + /// + /// A `\r\n` copy is folded to `\n` first. Split as it is, every row kept a `\r` on its last field, + /// and split on both characters it read as an empty line between every two rows. + static func pastedLines(_ text: String) -> [String] { + var lines = text.replacingOccurrences(of: "\r\n", with: "\n").components(separatedBy: .newlines) + while lines.count > 1, lines.last?.isEmpty == true { + lines.removeLast() + } + return lines + } } diff --git a/TablePro/Views/Results/Extensions/DataGridView+FillColumn.swift b/TablePro/Views/Results/Extensions/DataGridView+FillColumn.swift index dc5f3b56c9..318a8cea5f 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+FillColumn.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+FillColumn.swift @@ -35,8 +35,14 @@ extension TableViewCoordinator { alert.window.initialFirstResponder = accessory.firstResponderView alert.beginSheetModal(for: window) { [weak self] response in - guard response == .alertFirstButtonReturn else { return } - self?.applyFillColumn(columnIndex: columnIndex, value: accessory.resolvedValue) + guard response == .alertFirstButtonReturn, let self else { return } + guard let value = self.fillColumnValue( + text: accessory.enteredText, setNull: accessory.setsNull, columnIndex: columnIndex + ) else { + NSSound.beep() + return + } + self.applyFillColumn(columnIndex: columnIndex, value: value) } } @@ -75,8 +81,10 @@ extension TableViewCoordinator { return (0.. PluginCellValue { - setNull ? .null : .text(text) + /// The value a fill writes, read for the column the way a typed cell is: an empty fill of a date + /// is NULL. Nil when the column holds neither the text nor NULL in its place. + func fillColumnValue(text: String, setNull: Bool, columnIndex: Int) -> PluginCellValue? { + setNull ? .null : tableRowsProvider().enteredValue(text, forColumn: columnIndex) } static func fillImpactDescription(rowCount: Int) -> String { @@ -118,12 +126,9 @@ private final class FillColumnAccessoryView: NSView { super.init(coder: coder) } - var resolvedValue: PluginCellValue { - TableViewCoordinator.fillColumnValue( - text: valueField.stringValue, - setNull: nullCheckbox?.state == .on - ) - } + var enteredText: String { valueField.stringValue } + + var setsNull: Bool { nullCheckbox?.state == .on } var firstResponderView: NSView { valueField } diff --git a/TablePro/Views/RowInspector/FieldEditors/FieldEditorContext.swift b/TablePro/Views/RowInspector/FieldEditors/FieldEditorContext.swift index 8bd9fa3ca2..2fdf5ea681 100644 --- a/TablePro/Views/RowInspector/FieldEditors/FieldEditorContext.swift +++ b/TablePro/Views/RowInspector/FieldEditors/FieldEditorContext.swift @@ -61,9 +61,12 @@ internal struct FieldEditorContext { internal var hasMultipleValues: Bool { valueState == .multipleValues } /// The prompt a text field shows while it is empty. A state with its own name says it; an - /// ordinary value echoes the stored one so the user can see what they are replacing. + /// ordinary value echoes the stored one so the user can see what they are replacing. An emptied + /// date, number or boolean is staged as NULL, so it says that instead. internal var placeholderText: String { - valueState.placeholder ?? originalValue ?? "NULL" + if let placeholder = valueState.placeholder { return placeholder } + if valueState == .value(""), originalValue != "", !columnType.hasEmptyValue { return "NULL" } + return originalValue ?? "NULL" } /// What Copy Value puts on the pasteboard. diff --git a/TablePro/Views/RowInspector/FieldEditors/FieldMenuContent.swift b/TablePro/Views/RowInspector/FieldEditors/FieldMenuContent.swift index 27622d078c..be646dbf8f 100644 --- a/TablePro/Views/RowInspector/FieldEditors/FieldMenuContent.swift +++ b/TablePro/Views/RowInspector/FieldEditors/FieldMenuContent.swift @@ -9,7 +9,8 @@ import SwiftUI /// down and its context menu so both stay in sync. /// /// A read-only field keeps the copy actions and loses the mutating ones. Hiding the whole menu -/// left a value that is neither selectable nor copyable. +/// left a value that is neither selectable nor copyable. Set EMPTY is offered only where `''` is a +/// value of the column's type. internal struct FieldMenuContent: View { let value: String let columnType: ColumnType @@ -31,7 +32,9 @@ internal struct FieldMenuContent: View { if canMutate { Button("Set NULL") { onSetNull() } Button("Set DEFAULT") { onSetDefault() } - Button("Set EMPTY") { onSetEmpty() } + if columnType.hasEmptyValue { + Button("Set EMPTY") { onSetEmpty() } + } if let onRemoveField { Button("Remove Field") { onRemoveField() } } diff --git a/TableProTests/Core/Services/ColumnTypeEmptyValueTests.swift b/TableProTests/Core/Services/ColumnTypeEmptyValueTests.swift new file mode 100644 index 0000000000..6a82d837f8 --- /dev/null +++ b/TableProTests/Core/Services/ColumnTypeEmptyValueTests.swift @@ -0,0 +1,126 @@ +// +// ColumnTypeEmptyValueTests.swift +// TableProTests +// + +import Foundation +import TableProPluginKit +import Testing + +@testable import TablePro + +struct ColumnTypeEmptyValueTests { + @Test( + "A date, number, boolean, JSON, geometry or array has no empty value", + arguments: [ + ColumnType.date(rawType: "DATE"), + .datetime(rawType: "DATETIME"), + .timestamp(rawType: "TIMESTAMP"), + .timestamp(rawType: "TIME"), + .integer(rawType: "INT"), + .integer(rawType: "RedisInt"), + .decimal(rawType: "DECIMAL(10,2)"), + .boolean(rawType: "TINYINT(1)"), + .json(rawType: "JSON"), + .spatial(rawType: "GEOMETRY"), + .array(rawType: "integer[]", element: .integer(rawType: "integer")) + ] + ) + func typedColumnsHaveNoEmptyValue(_ columnType: ColumnType) { + #expect(columnType.hasEmptyValue == false) + } + + @Test( + "Text, SET and binary columns hold the empty string", + arguments: [ + ColumnType.text(rawType: "VARCHAR(255)"), + .text(rawType: "LONGTEXT"), + .text(rawType: nil), + .set(rawType: "SET('a','b')", values: ["a", "b"]), + .blob(rawType: "BLOB") + ] + ) + func characterColumnsHoldEmptyValue(_ columnType: ColumnType) { + #expect(columnType.hasEmptyValue) + } + + @Test( + "Types the classifier files as text that hold no empty string", + arguments: ["YEAR", "year(4)", "uuid", "UNIQUEIDENTIFIER", "inet", "macaddr8", "interval day to second", "Nullable(UUID)"] + ) + func nonCharacterTextTypes(_ rawType: String) { + #expect(ColumnType.text(rawType: rawType).hasEmptyValue == false) + } + + @Test( + "Server spellings the classifier files as text still have no empty value", + arguments: [ + "YEAR", "year(4)", "uuid", "uniqueidentifier", "Nullable(UUID)", + "LowCardinality(Nullable(IPv4))", "inet", "interval day to second" + ] + ) + func classifiedNonCharacterTypes(_ rawTypeName: String) { + #expect(ColumnTypeClassifier().classify(rawTypeName: rawTypeName).hasEmptyValue == false) + } + + @Test("Server spellings of character types hold the empty string", arguments: ["VARCHAR(20)", "character varying", "TEXT", "nvarchar(max)"]) + func classifiedCharacterTypes(_ rawTypeName: String) { + #expect(ColumnTypeClassifier().classify(rawTypeName: rawTypeName).hasEmptyValue) + } + + @Test("An enum has an empty value only when one of its labels is the empty string") + func enumEmptyLabel() { + #expect(ColumnType.enumType(rawType: "ENUM", values: ["a", "b"]).hasEmptyValue == false) + #expect(ColumnType.enumType(rawType: "ENUM", values: nil).hasEmptyValue == false) + #expect(ColumnType.enumType(rawType: "ENUM", values: ["", "a"]).hasEmptyValue) + } + + @Test("Emptied text entered for a date is NULL") + func emptiedDateIsNull() { + #expect(PluginCellValue.entered("", columnType: .datetime(rawType: "DATETIME")) == .null) + #expect(PluginCellValue.entered("", columnType: .integer(rawType: "INT")) == .null) + } + + @Test("Emptied text entered for a text column stays the empty string") + func emptiedTextStaysEmpty() { + #expect(PluginCellValue.entered("", columnType: .text(rawType: "VARCHAR(255)")) == .text("")) + #expect(PluginCellValue.entered("", columnType: nil) == .text("")) + } + + @Test("Entered text and an explicit NULL pass through") + func valuesPassThrough() { + let datetime = ColumnType.datetime(rawType: "DATETIME") + #expect(PluginCellValue.entered("2024-01-01 10:00:00", columnType: datetime) == .text("2024-01-01 10:00:00")) + #expect(PluginCellValue.entered(" ", columnType: datetime) == .text(" ")) + #expect(PluginCellValue.entered(nil, columnType: .text(rawType: nil)) == .null) + } + + @Test("A table's enum labels decide whether its column has an empty value") + func tableRowsMergeEnumLabels() { + let tableRows = TableRows.from( + queryRows: [[.text("a"), .text("x")]], + columns: ["plain", "withEmpty"], + columnTypes: [.enumType(rawType: "ENUM", values: nil), .enumType(rawType: "ENUM", values: nil)], + columnEnumValues: ["plain": ["a", "b"], "withEmpty": ["", "x"]] + ) + + #expect(tableRows.enteredValue("", forColumn: 0) == .null) + #expect(tableRows.enteredValue("", forColumn: 1) == .text("")) + } + + @Test("A NOT NULL column refuses an entry it could only store as NULL, and takes the rest") + func notNullColumnRefusesNull() { + let tableRows = TableRows.from( + queryRows: [[.text("2024-01-01"), .text("a"), .text("2024-01-01")]], + columns: ["created_at", "name", "deleted_at"], + columnTypes: [.timestamp(rawType: "TIMESTAMP"), .text(rawType: "VARCHAR(10)"), .date(rawType: "DATE")], + columnNullable: ["created_at": false, "name": false] + ) + + #expect(tableRows.enteredValue("", forColumn: 0) == nil) + #expect(tableRows.enteredValue(nil, forColumn: 0) == nil) + #expect(tableRows.enteredValue("2024-02-02 00:00:00", forColumn: 0) == .text("2024-02-02 00:00:00")) + #expect(tableRows.enteredValue("", forColumn: 1) == .text("")) + #expect(tableRows.enteredValue("", forColumn: 2) == .null) + } +} diff --git a/TableProTests/Models/MultiRowEditStateClearedFieldTests.swift b/TableProTests/Models/MultiRowEditStateClearedFieldTests.swift new file mode 100644 index 0000000000..2f09f522ff --- /dev/null +++ b/TableProTests/Models/MultiRowEditStateClearedFieldTests.swift @@ -0,0 +1,101 @@ +// +// MultiRowEditStateClearedFieldTests.swift +// TableProTests +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +@MainActor +private final class StagedFieldRecorder { + var values: [Int: PluginCellValue] = [:] + var detachedValues: [Int: PluginCellValue] = [:] +} + +@MainActor +struct MultiRowEditStateClearedFieldTests { + private static let columns = ["id", "deleted_at", "name", "doc", "created_at"] + private static let deletedAt = 1 + private static let name = 2 + private static let doc = 3 + private static let createdAt = 4 + + private func makeState(_ recorder: StagedFieldRecorder) -> MultiRowEditState { + let state = MultiRowEditState() + state.configure( + selectedRowIndices: [0], + rowIDs: [.existing(0)], + allRows: [["1", "2024-01-01 10:00:00", "Alice", "{\"a\":1}", "2023-12-01 09:00:00"]], + columns: Self.columns, + columnTypes: [ + .integer(rawType: "INT"), + .datetime(rawType: "DATETIME"), + .text(rawType: "VARCHAR(255)"), + .json(rawType: "JSON"), + .timestamp(rawType: "TIMESTAMP") + ], + nonNullableColumns: ["created_at"] + ) + state.onFieldChanged = { index, value, _ in recorder.values[index] = value } + state.onDetachedFieldChanged = { index, value, _ in recorder.detachedValues[index] = value } + return state + } + + @Test("Clearing a date field stages NULL and keeps the field's text editable") + func clearedDateFieldStagesNull() { + let recorder = StagedFieldRecorder() + let state = makeState(recorder) + + state.updateField(at: Self.deletedAt, value: "") + + #expect(recorder.values[Self.deletedAt] == .null) + #expect(state.fields[Self.deletedAt].isPendingNull == false) + #expect(state.currentText(at: Self.deletedAt) == "") + } + + @Test("Clearing a text field stages the empty string") + func clearedTextFieldStagesEmpty() { + let recorder = StagedFieldRecorder() + let state = makeState(recorder) + + state.updateField(at: Self.name, value: "") + + #expect(recorder.values[Self.name] == .text("")) + } + + @Test("Clearing a NOT NULL timestamp field stages nothing and keeps the stored value") + func clearedNotNullFieldIsRefused() { + let recorder = StagedFieldRecorder() + let state = makeState(recorder) + + state.updateField(at: Self.createdAt, value: "") + + #expect(recorder.values[Self.createdAt] == nil) + #expect(state.fields[Self.createdAt].hasEdit == false) + #expect(state.currentText(at: Self.createdAt) == "2023-12-01 09:00:00") + } + + @Test("Clearing a NOT NULL timestamp after typing in it takes the typed value back out") + func clearedNotNullFieldRevertsTypedValue() { + let recorder = StagedFieldRecorder() + let state = makeState(recorder) + + state.updateField(at: Self.createdAt, value: "2024-05-05 00:00:00") + state.updateField(at: Self.createdAt, value: "") + + #expect(recorder.values[Self.createdAt] == .text("2023-12-01 09:00:00")) + #expect(state.fields[Self.createdAt].hasEdit == false) + } + + @Test("A detached window's cleared JSON goes to its rows as NULL") + func clearedDetachedJSONStagesNull() { + let recorder = StagedFieldRecorder() + let state = makeState(recorder) + + state.updateDetachedField(columnIndex: Self.doc, rowIDs: [.existing(7)], value: "") + + #expect(recorder.detachedValues[Self.doc] == .null) + } +} diff --git a/TableProTests/Views/Main/SidebarSaveClearedValueTests.swift b/TableProTests/Views/Main/SidebarSaveClearedValueTests.swift new file mode 100644 index 0000000000..ddef0a2b09 --- /dev/null +++ b/TableProTests/Views/Main/SidebarSaveClearedValueTests.swift @@ -0,0 +1,70 @@ +// +// SidebarSaveClearedValueTests.swift +// TableProTests +// + +import Foundation +import TableProPluginKit +import Testing + +@testable import TablePro + +@MainActor +struct SidebarSaveClearedValueTests { + private let fixture = CoordinatorFixture() + + private func makeCoordinator() -> MainContentCoordinator { + let tabManager = fixture.tabManager + let coordinator = fixture.coordinator + var tab = QueryTab(title: "users", query: "SELECT * FROM users", tabType: .table, tableName: "users") + tab.execution.lastExecutedAt = Date() + tab.display.resultsViewMode = .data + tabManager.tabs.append(tab) + tabManager.selectedTabId = tab.id + + coordinator.setActiveTableRows( + TableRows.from( + queryRows: [[.text("1"), .text("2024-01-01 10:00:00"), .text("Alice")]], + columns: ["id", "deleted_at", "name"], + columnTypes: [.integer(rawType: "INT"), .datetime(rawType: "DATETIME"), .text(rawType: "VARCHAR(255)")], + hasAuthoritativeSchema: true + ), + for: tab.id + ) + coordinator.changeManager.configureForTable( + tableName: "users", + columns: ["id", "deleted_at", "name"], + primaryKeyColumns: ["id"], + databaseType: .mysql, + generatedColumns: [] + ) + coordinator.selectionState.indices = [0] + return coordinator + } + + @Test("An inspector save of a cleared date binds NULL") + func clearedDateBindsNull() throws { + let coordinator = makeCoordinator() + + let statements = try coordinator.sidebarEditStatements( + editedFields: [InspectorFieldEdit(columnIndex: 1, columnName: "deleted_at", newValue: "")] + ) + + let statement = try #require(statements.first) + #expect(statement.sql.contains("`deleted_at` = ?")) + let boundValue = try #require(statement.parameters.first) + #expect(boundValue == nil) + } + + @Test("An inspector save of a cleared text field binds the empty string") + func clearedTextBindsEmpty() throws { + let coordinator = makeCoordinator() + + let statements = try coordinator.sidebarEditStatements( + editedFields: [InspectorFieldEdit(columnIndex: 2, columnName: "name", newValue: "")] + ) + + let statement = try #require(statements.first) + #expect(statement.parameters.first.flatMap { $0 as? String } == "") + } +} diff --git a/TableProTests/Views/Results/Extensions/CellPasteRoutingTests.swift b/TableProTests/Views/Results/Extensions/CellPasteRoutingTests.swift index e91b12664d..e73b5d188a 100644 --- a/TableProTests/Views/Results/Extensions/CellPasteRoutingTests.swift +++ b/TableProTests/Views/Results/Extensions/CellPasteRoutingTests.swift @@ -122,6 +122,26 @@ extension GlobalStateSuites { #expect(coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: 0)) } + @Test("Whole rows with an empty line between them still go to row paste") + func defersOnRowShapedTSVWithEmptyLine() throws { + let stub = try #require(RecordingClipboard.current) + stub.text = "x\ty\tz\n\nq\tw\te\n\n" + + let coordinator = makeCoordinator(columns: ["a", "b", "c"], rowCount: 5) + + #expect(coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: 0) == false) + } + + @Test("A value with trailing line breaks still fills only the focused cell in a one-column table") + func singleValueWithTrailingBreaksInSingleColumnTable() throws { + let stub = try #require(RecordingClipboard.current) + stub.text = "hello\n\n" + + let coordinator = makeCoordinator(columns: ["a"], rowCount: 3) + + #expect(coordinator.canPasteCellsFromClipboard(anchorRow: 1, anchorColumn: 0)) + } + @Test("Returns false when not editable") func refusesWhenReadOnly() throws { let stub = try #require(RecordingClipboard.current) diff --git a/TableProTests/Views/Results/Extensions/ClearedCellCommitTests.swift b/TableProTests/Views/Results/Extensions/ClearedCellCommitTests.swift new file mode 100644 index 0000000000..78a185d7a9 --- /dev/null +++ b/TableProTests/Views/Results/Extensions/ClearedCellCommitTests.swift @@ -0,0 +1,328 @@ +// +// ClearedCellCommitTests.swift +// TableProTests +// +// An emptied cell in a column whose type has no empty value is staged as NULL, so a cleared +// MySQL `deleted_at` is written as NULL rather than the server's zero date. A NOT NULL column of +// such a type refuses the edit, since it holds neither value. +// + +import AppKit +import SwiftUI +import TableProPluginKit +import Testing + +@testable import TablePro + +@MainActor +private final class NoopClearedCellLayoutPersister: ColumnLayoutPersisting { + func load(for key: ColumnLayoutTableKey) -> ColumnLayoutState? { nil } + func save(_ layout: ColumnLayoutState, for key: ColumnLayoutTableKey) {} + func clear(for key: ColumnLayoutTableKey) {} +} + +@MainActor +private final class ClearedCellRowStore { + var tableRows: TableRows + + init(_ tableRows: TableRows) { + self.tableRows = tableRows + } +} + +@MainActor +private struct ClearedCellFixture { + static let columns = ["id", "deleted_at", "name", "visits", "created_at", "doc"] + static let deletedAt = 1 + static let name = 2 + static let visits = 3 + static let createdAt = 4 + static let doc = 5 + + let manager: DataChangeManager + let coordinator: TableViewCoordinator + let store: ClearedCellRowStore + + init() { + manager = DataChangeManager() + manager.configureForTable( + tableName: "users", + columns: Self.columns, + primaryKeyColumns: ["id"], + databaseType: .mysql, + generatedColumns: [] + ) + coordinator = TableViewCoordinator( + changeManager: AnyChangeManager(manager), + isEditable: true, + selectedRowIndices: .constant([]), + delegate: nil, + layoutPersister: NoopClearedCellLayoutPersister() + ) + let created: PluginCellValue = .text("2023-12-01 09:00:00") + let doc: PluginCellValue = .text("{\"a\":1}") + store = ClearedCellRowStore(TableRows.from( + queryRows: [ + [.text("1"), .text("2024-01-01 10:00:00"), .text("Alice"), .text("3"), created, doc], + [.text("2"), .text("2024-02-02 10:00:00"), .text("Bob"), .text("4"), created, doc], + [.text("3"), .text("2024-03-03 10:00:00"), .text("Carol"), .text("5"), created, doc] + ], + columns: Self.columns, + columnTypes: [ + .integer(rawType: "INT"), + .datetime(rawType: "DATETIME"), + .text(rawType: "VARCHAR(255)"), + .integer(rawType: "INT"), + .timestamp(rawType: "TIMESTAMP"), + .json(rawType: "JSON") + ], + columnNullable: ["deleted_at": true, "name": false, "created_at": false] + )) + let store = store + coordinator.tableRowsProvider = { store.tableRows } + coordinator.tableRowsMutator = { mutation in mutation(&store.tableRows) } + coordinator.updateCache() + } + + func value(row: Int, column: Int) -> PluginCellValue { + store.tableRows.rows[row].values[column] + } + + func stagedValue(row: Int, column: Int) -> PluginCellValue? { + manager.pending.change(forRow: .existing(row), type: .update)? + .cellChanges.first { $0.columnIndex == column }?.newValue + } +} + +@MainActor +struct ClearedCellCommitTests { + @Test("Clearing a date cell stages NULL and the UPDATE binds NULL, not an empty string") + func clearedDateIsNull() throws { + let fixture = ClearedCellFixture() + + fixture.coordinator.commitCellEdit(row: 0, columnIndex: ClearedCellFixture.deletedAt, newValue: "") + + #expect(fixture.stagedValue(row: 0, column: ClearedCellFixture.deletedAt) == .null) + #expect(fixture.value(row: 0, column: ClearedCellFixture.deletedAt) == .null) + let statement = try #require(try fixture.manager.generateSQL().first) + #expect(statement.sql.contains("`deleted_at` = ?")) + let boundValue = try #require(statement.parameters.first) + #expect(boundValue == nil) + } + + @Test("Clearing a text cell keeps the empty string, which is a value there, NOT NULL or not") + func clearedTextStaysEmpty() { + let fixture = ClearedCellFixture() + + fixture.coordinator.commitCellEdit(row: 0, columnIndex: ClearedCellFixture.name, newValue: "") + + #expect(fixture.stagedValue(row: 0, column: ClearedCellFixture.name) == .text("")) + #expect(fixture.value(row: 0, column: ClearedCellFixture.name) == .text("")) + } + + @Test("Clearing a NOT NULL timestamp stages nothing and keeps the stored value") + func clearedNotNullTimestampIsRefused() { + let fixture = ClearedCellFixture() + + fixture.coordinator.commitCellEdit(row: 0, columnIndex: ClearedCellFixture.createdAt, newValue: "") + + #expect(fixture.manager.hasChanges == false) + #expect(fixture.value(row: 0, column: ClearedCellFixture.createdAt) == .text("2023-12-01 09:00:00")) + } + + @Test("A detached editor's cleared number commits NULL into the row it was opened for") + func clearedNumberByRowIDIsNull() { + let fixture = ClearedCellFixture() + + fixture.coordinator.commitCellEdit(rowID: .existing(2), columnIndex: ClearedCellFixture.visits, newValue: "") + + #expect(fixture.stagedValue(row: 2, column: ClearedCellFixture.visits) == .null) + } + + @Test("The JSON editor's cleared document commits NULL, which a JSON column can hold where '' is refused") + func clearedJSONIsNull() { + let fixture = ClearedCellFixture() + + fixture.coordinator.commitCellEdit( + rowID: .existing(1), fallbackDisplayRow: 1, columnIndex: ClearedCellFixture.doc, newValue: "" + ) + + #expect(fixture.stagedValue(row: 1, column: ClearedCellFixture.doc) == .null) + } + + @Test("An empty Fill Column on a date writes NULL into every row") + func emptyFillOnDateIsNull() throws { + let fixture = ClearedCellFixture() + let value = try #require( + fixture.coordinator.fillColumnValue(text: "", setNull: false, columnIndex: ClearedCellFixture.deletedAt) + ) + + fixture.coordinator.applyFillColumn(columnIndex: ClearedCellFixture.deletedAt, value: value) + + for row in 0..<3 { + #expect(fixture.stagedValue(row: row, column: ClearedCellFixture.deletedAt) == .null) + } + } + + @Test("An empty Fill Column on a NOT NULL timestamp has no value to write") + func emptyFillOnNotNullTimestampIsRefused() { + let fixture = ClearedCellFixture() + + #expect(fixture.coordinator.fillColumnValue(text: "", setNull: false, columnIndex: ClearedCellFixture.createdAt) == nil) + #expect( + fixture.coordinator.fillColumnValue(text: "2024-05-05 00:00:00", setNull: false, columnIndex: ClearedCellFixture.createdAt) + == .text("2024-05-05 00:00:00") + ) + } + + @Test("Set Value offers Empty on a text column and not on a date or a number") + func setValueEmptyFollowsColumnType() { + let fixture = ClearedCellFixture() + let rowView = DataGridRowView() + + let dateTitles = rowView.buildSetValueMenu( + dataColumnIndex: ClearedCellFixture.deletedAt, tableRows: fixture.store.tableRows + ).items.map { $0.title } + let numberTitles = rowView.buildSetValueMenu( + dataColumnIndex: ClearedCellFixture.visits, tableRows: fixture.store.tableRows + ).items.map { $0.title } + let textTitles = rowView.buildSetValueMenu( + dataColumnIndex: ClearedCellFixture.name, tableRows: fixture.store.tableRows + ).items.map { $0.title } + + #expect(!dateTitles.contains("Empty")) + #expect(dateTitles.contains("NULL")) + #expect(!numberTitles.contains("Empty")) + #expect(textTitles.contains("Empty")) + } + + @Test("Set Value offers Empty on an enum only when one of its labels is empty") + func setValueEmptyOnEnumFollowsLabels() { + let tableRows = TableRows.from( + queryRows: [[.text("a"), .text("x")]], + columns: ["plain", "withEmpty"], + columnTypes: [.enumType(rawType: "ENUM", values: nil), .enumType(rawType: "ENUM", values: nil)], + columnEnumValues: ["plain": ["a", "b"], "withEmpty": ["", "x"]] + ) + let rowView = DataGridRowView() + + #expect(!rowView.buildSetValueMenu(dataColumnIndex: 0, tableRows: tableRows).items.map { $0.title }.contains("Empty")) + #expect(rowView.buildSetValueMenu(dataColumnIndex: 1, tableRows: tableRows).items.map { $0.title }.contains("Empty")) + } + + @Test("A NOT NULL number with no default offers no Set Value items at all") + func setValueMenuEmptyForNotNullNumber() { + let tableRows = TableRows.from( + queryRows: [[.text("1")]], + columns: ["visits"], + columnTypes: [.integer(rawType: "INT")], + columnNullable: ["visits": false] + ) + + #expect(DataGridRowView().buildSetValueMenu(dataColumnIndex: 0, tableRows: tableRows).items.isEmpty) + } + + @Test("Pasted lines keep an empty line as a value and drop the closing line breaks") + func pastedLines() { + #expect(TableViewCoordinator.pastedLines("a\n\nc") == ["a", "", "c"]) + #expect(TableViewCoordinator.pastedLines("a\nb\n") == ["a", "b"]) + #expect(TableViewCoordinator.pastedLines("hello\n\n") == ["hello"]) + #expect(TableViewCoordinator.pastedLines("a") == ["a"]) + } + + @Test("A CRLF copy splits into the same lines as a LF copy") + func pastedLinesFoldCRLF() { + #expect(TableViewCoordinator.pastedLines("a\r\n\r\nc\r\n") == ["a", "", "c"]) + #expect(TableViewCoordinator.pastedLines("x\ty\r\nq\tw\r\n") == ["x\ty", "q\tw"]) + } + + @Test("A pasted NULL is NULL where the type cannot hold the text, and text everywhere else") + func pastedText() { + let datetime = ColumnType.datetime(rawType: "DATETIME") + #expect(TableViewCoordinator.pastedText("NULL", columnType: datetime) == nil) + #expect(TableViewCoordinator.pastedText("NULL", columnType: .integer(rawType: "INT")) == nil) + #expect(TableViewCoordinator.pastedText("NULL", columnType: .text(rawType: "VARCHAR(20)")) == "NULL") + #expect(TableViewCoordinator.pastedText("NULL", columnType: .enumType(rawType: "ENUM", values: ["NULL", "a"])) == "NULL") + #expect(TableViewCoordinator.pastedText("NULL", columnType: nil) == "NULL") + #expect(TableViewCoordinator.pastedText("", columnType: datetime) == "") + #expect(TableViewCoordinator.pastedText("null", columnType: datetime) == "null") + } +} + +extension GlobalStateSuites { + @Suite(.recordingClipboard) + @MainActor + struct ClearedCellPasteTests { + @Test("An empty pasted field lands as NULL in a date column and as the empty string in text") + func emptyPastedFieldFollowsColumnType() throws { + let stub = try #require(RecordingClipboard.current) + stub.text = "\t" + let fixture = ClearedCellFixture() + + #expect(fixture.coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: ClearedCellFixture.deletedAt)) + + #expect(fixture.value(row: 0, column: ClearedCellFixture.deletedAt) == .null) + #expect(fixture.value(row: 0, column: ClearedCellFixture.name) == .text("")) + } + + @Test("A copied NULL pasted into a date cell is NULL, not the text NULL") + func pastedNullIsNull() throws { + let stub = try #require(RecordingClipboard.current) + stub.text = "NULL" + let fixture = ClearedCellFixture() + + #expect(fixture.coordinator.pasteCellsFromClipboard(anchorRow: 1, anchorColumn: ClearedCellFixture.deletedAt)) + + #expect(fixture.value(row: 1, column: ClearedCellFixture.deletedAt) == .null) + } + + @Test("The text NULL pasted into a text cell stays text, since a stored 'NULL' copies the same way") + func pastedNullTextIntoTextStaysText() throws { + let stub = try #require(RecordingClipboard.current) + stub.text = "NULL" + let fixture = ClearedCellFixture() + + #expect(fixture.coordinator.pasteCellsFromClipboard(anchorRow: 1, anchorColumn: ClearedCellFixture.name)) + + #expect(fixture.value(row: 1, column: ClearedCellFixture.name) == .text("NULL")) + } + + @Test("A copied NULL pasted into a NOT NULL column leaves the cell as it was") + func pastedNullIntoNotNullIsRefused() throws { + let stub = try #require(RecordingClipboard.current) + stub.text = "NULL" + let fixture = ClearedCellFixture() + + #expect(fixture.coordinator.pasteCellsFromClipboard(anchorRow: 1, anchorColumn: ClearedCellFixture.createdAt)) + + #expect(fixture.value(row: 1, column: ClearedCellFixture.createdAt) == .text("2023-12-01 09:00:00")) + #expect(fixture.manager.hasChanges == false) + } + + @Test("A block that opens on an empty line still fills every column of the lines below it") + func leadingEmptyLineKeepsBlockWidth() throws { + let stub = try #require(RecordingClipboard.current) + stub.text = "\nZed\t9" + let fixture = ClearedCellFixture() + + #expect(fixture.coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: ClearedCellFixture.name)) + + #expect(fixture.value(row: 0, column: ClearedCellFixture.name) == .text("")) + #expect(fixture.value(row: 1, column: ClearedCellFixture.name) == .text("Zed")) + #expect(fixture.value(row: 1, column: ClearedCellFixture.visits) == .text("9")) + } + + @Test("An empty line in a pasted column keeps the values below it on their own rows") + func emptyLineKeepsRowsAligned() throws { + let stub = try #require(RecordingClipboard.current) + stub.text = "Xavier\n\nZoe" + let fixture = ClearedCellFixture() + + #expect(fixture.coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: ClearedCellFixture.name)) + + #expect(fixture.value(row: 0, column: ClearedCellFixture.name) == .text("Xavier")) + #expect(fixture.value(row: 1, column: ClearedCellFixture.name) == .text("")) + #expect(fixture.value(row: 2, column: ClearedCellFixture.name) == .text("Zoe")) + } + } +} diff --git a/TableProTests/Views/Results/Extensions/FillColumnTests.swift b/TableProTests/Views/Results/Extensions/FillColumnTests.swift index e8c45fbb8f..8bd1d1baa1 100644 --- a/TableProTests/Views/Results/Extensions/FillColumnTests.swift +++ b/TableProTests/Views/Results/Extensions/FillColumnTests.swift @@ -95,9 +95,10 @@ struct FillColumnTests { @Test("Resolves NULL distinctly from an empty string") func resolvesNullDistinctFromEmpty() { - #expect(TableViewCoordinator.fillColumnValue(text: "ignored", setNull: true) == .null) - #expect(TableViewCoordinator.fillColumnValue(text: "", setNull: false) == .text("")) - #expect(TableViewCoordinator.fillColumnValue(text: "active", setNull: false) == .text("active")) + let coordinator = makeCoordinator(columns: ["a"], rowCount: 1, manager: DataChangeManager()) + #expect(coordinator.fillColumnValue(text: "ignored", setNull: true, columnIndex: 0) == .null) + #expect(coordinator.fillColumnValue(text: "", setNull: false, columnIndex: 0) == .text("")) + #expect(coordinator.fillColumnValue(text: "active", setNull: false, columnIndex: 0) == .text("active")) } @Test("Impact description reflects singular and plural counts") diff --git a/TableProTests/Views/RowInspector/FieldEditorContextPolicyTests.swift b/TableProTests/Views/RowInspector/FieldEditorContextPolicyTests.swift index c78d47498c..3cee086209 100644 --- a/TableProTests/Views/RowInspector/FieldEditorContextPolicyTests.swift +++ b/TableProTests/Views/RowInspector/FieldEditorContextPolicyTests.swift @@ -54,6 +54,25 @@ struct FieldEditorContextPolicyTests { #expect(makeContext(isReadOnly: false, originalValue: long).placeholderText == long) } + @Test("an emptied date field says NULL, which is what it stages, and a text field echoes the old value") + func emptiedFieldPlaceholderFollowsColumnType() { + func context(_ columnType: ColumnType, original: String?) -> FieldEditorContext { + FieldEditorContext( + columnName: "c", + columnType: columnType, + isLongText: false, + value: .constant(""), + originalValue: original, + valueState: .value(""), + isReadOnly: false + ) + } + + #expect(context(.datetime(rawType: "DATETIME"), original: "2024-01-01 10:00:00").placeholderText == "NULL") + #expect(context(.text(rawType: "VARCHAR(20)"), original: "Alice").placeholderText == "Alice") + #expect(context(.datetime(rawType: "DATETIME"), original: "").placeholderText == "") + } + @Test("a stored empty string is not reported as NULL") func storedEmptyStringIsNotNull() { #expect(makeContext(isReadOnly: true, originalValue: "").emptyStatePlaceholder == nil) From 1a940bcf99cea1417f388f76f701ae6f2577a773 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 15 Oct 2026 17:27:57 +0700 Subject: [PATCH 2/2] fix(datagrid): keep NULL through cell copy and paste A copied NULL and a stored 'NULL' copied as the same text, and a value with a tab or line break split into extra fields on paste. The grid's cell copy now carries its fields beside the text, and both cell and row paste read them while the clipboard still holds that copy. The inspector marks a NULL staged in the grid as a pending NULL instead of an empty string, so its boolean picker no longer reads false. Elasticsearch browse results hint long, short, byte, the float variants and date_nanos to the classifier, so clearing one writes NULL. --- CHANGELOG.md | 4 +- .../ElasticsearchPluginDriver+Execution.swift | 3 +- .../ElasticsearchTypeClassification.swift | 44 ++++++++++++ .../Infrastructure/ClipboardService.swift | 26 +++++++ .../Services/Query/RowOperationsManager.swift | 28 ++++++++ TablePro/Models/UI/MultiRowEditState.swift | 5 +- .../Results/DataGridView+RowActions.swift | 31 ++++---- .../Extensions/DataGridView+CellPaste.swift | 58 +++++++++------ .../Core/Services/ClipboardServiceTests.swift | 19 +++++ .../RowOperationsManagerPasteTests.swift | 26 +++++++ .../Helpers/RecordingClipboard.swift | 14 +++- .../Models/MultiRowEditStateTests.swift | 33 ++++++++- .../Extensions/ClearedCellCommitTests.swift | 72 ++++++++++++++++--- 13 files changed, 310 insertions(+), 53 deletions(-) create mode 100644 Plugins/ElasticsearchDriverPlugin/ElasticsearchTypeClassification.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 469fd66252..85886a9241 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,7 +124,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - GitHub Copilot language server pinned to a reviewed version and checksum instead of the latest npm release. - One unreadable setting, or one unknown AI provider, resetting its whole settings group to defaults. - Clearing a date, number or boolean cell saving an empty string, stored as `0000-00-00 00:00:00` on MySQL, instead of NULL. -- Pasting a copied NULL into a date, number or boolean cell writing the text NULL, stored as a zero date or 0 on MySQL. +- Pasting a copied NULL cell writing the text NULL instead of NULL. +- Inspector's boolean picker showing false for a value set to NULL in the grid. +- Clearing an Elasticsearch long, short or byte field sending an empty string the mapping refuses. - Empty line in pasted cells moving the values below it up a row, or cutting off the columns after it. ### Security diff --git a/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver+Execution.swift b/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver+Execution.swift index 36f5373f33..30365282be 100644 --- a/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver+Execution.swift +++ b/Plugins/ElasticsearchDriverPlugin/ElasticsearchPluginDriver+Execution.swift @@ -361,7 +361,8 @@ extension ElasticsearchPluginDriver { columnTypeNames: typeNames, rows: rows, rowsAffected: 0, - executionTime: Date().timeIntervalSince(startTime) + executionTime: Date().timeIntervalSince(startTime), + columnMeta: ElasticsearchTypeClassification.columnMeta(columns: columns, typeNames: typeNames) ) } diff --git a/Plugins/ElasticsearchDriverPlugin/ElasticsearchTypeClassification.swift b/Plugins/ElasticsearchDriverPlugin/ElasticsearchTypeClassification.swift new file mode 100644 index 0000000000..cd55e3eb51 --- /dev/null +++ b/Plugins/ElasticsearchDriverPlugin/ElasticsearchTypeClassification.swift @@ -0,0 +1,44 @@ +// +// ElasticsearchTypeClassification.swift +// ElasticsearchDriverPlugin +// + +import Foundation +import TableProPluginKit + +enum ElasticsearchTypeClassification { + /// The SQL name the app classifies a mapping type by, where the mapping name is not one it + /// reads. Unhinted, `long`, `short` and `date_nanos` classify as text: a cleared cell was then + /// written as `""`, which the field's mapping refuses, and the grid sorted the numbers as text. + static func classificationName(for mappingType: String) -> String? { + switch mappingType { + case "long", "unsigned_long": return "BIGINT" + case "short": return "SMALLINT" + case "byte": return "TINYINT" + case "token_count": return "INTEGER" + case "half_float", "scaled_float": return "FLOAT" + case "date_nanos": return "TIMESTAMP" + default: return nil + } + } + + /// The result's columns as `fetchColumns` describes them, carrying the hints above. + static func columnMeta(columns: [String], typeNames: [String]) -> [PluginColumnInfo] { + zip(columns, typeNames).map { column, typeName in + let isID = column == ElasticsearchMappingFlattener.idColumn + return PluginColumnInfo( + name: column, + dataType: typeName, + isNullable: !isID, + isPrimaryKey: isID, + generationExpression: nil, + generationKind: nil, + ddlSpelling: nil, + ddlDefault: nil, + ddlGenerationExpression: nil, + ddlCollation: nil, + classificationTypeName: classificationName(for: typeName) + ) + } + } +} diff --git a/TablePro/Core/Services/Infrastructure/ClipboardService.swift b/TablePro/Core/Services/Infrastructure/ClipboardService.swift index 715af7f1c3..d98dae8e28 100644 --- a/TablePro/Core/Services/Infrastructure/ClipboardService.swift +++ b/TablePro/Core/Services/Infrastructure/ClipboardService.swift @@ -33,13 +33,26 @@ extension GridRowsClipboardPayload { } } +/// A cell block the grid copied, each field as the grid held it. +/// +/// The text alone loses two things a paste needs: a NULL copies as the text `NULL`, the same text a +/// stored `'NULL'` copies as, and a value holding a tab or a line break splits into extra fields. +struct GridCellsClipboardPayload: Codable, Equatable { + /// The text the copy wrote. A paste reads the fields only while the clipboard still holds it. + let text: String + /// The fields by line, in the order the copy wrote them, nil for a NULL. + let fields: [[String?]] +} + protocol ClipboardProvider { func readText() -> String? func readGridRows() -> GridRowsClipboardPayload? + func readGridCells() -> GridCellsClipboardPayload? func writeText(_ text: String) func writeCsv(_ csv: String) func writeImage(_ image: NSImage) func writeRows(tsv: String, html: String?, gridRows: GridRowsClipboardPayload) + func writeCells(_ cells: GridCellsClipboardPayload) var hasText: Bool { get } var hasGridRows: Bool { get } } @@ -56,6 +69,7 @@ struct NSPasteboardClipboardProvider: ClipboardProvider { private static let tsvType = NSPasteboard.PasteboardType("public.utf8-tab-separated-values-text") private static let csvType = NSPasteboard.PasteboardType("public.comma-separated-values-text") private static let gridRowsType = NSPasteboard.PasteboardType("com.TablePro.gridRows") + private static let gridCellsType = NSPasteboard.PasteboardType("com.TablePro.gridCells") /// The convention clipboard managers watch for to keep an item out of their history. private static let concealedType = NSPasteboard.PasteboardType("org.nspasteboard.ConcealedType") @@ -72,6 +86,11 @@ struct NSPasteboardClipboardProvider: ClipboardProvider { return try? JSONDecoder().decode(GridRowsClipboardPayload.self, from: data) } + func readGridCells() -> GridCellsClipboardPayload? { + guard let data = NSPasteboard.general.data(forType: Self.gridCellsType) else { return nil } + return try? JSONDecoder().decode(GridCellsClipboardPayload.self, from: data) + } + func writeText(_ text: String) { let pb = NSPasteboard.general pb.clearContents() @@ -116,6 +135,13 @@ struct NSPasteboardClipboardProvider: ClipboardProvider { } } + func writeCells(_ cells: GridCellsClipboardPayload) { + writeText(cells.text) + if let data = try? JSONEncoder().encode(cells) { + NSPasteboard.general.setData(data, forType: Self.gridCellsType) + } + } + var hasText: Bool { PasteboardTextReader.hasText() } diff --git a/TablePro/Core/Services/Query/RowOperationsManager.swift b/TablePro/Core/Services/Query/RowOperationsManager.swift index 45fce4b452..ca6d9ffcf1 100644 --- a/TablePro/Core/Services/Query/RowOperationsManager.swift +++ b/TablePro/Core/Services/Query/RowOperationsManager.swift @@ -320,6 +320,14 @@ final class RowOperationsManager { return PasteRowsResult(pastedRows: [], delta: .none) } + if parser == nil, let cells = clipboardProvider.readGridCells(), cells.text == clipboardText { + let parsedRows = Self.rows(fromCopiedCells: cells, schema: schema, tableRows: tableRows) + return insertParsedRows( + keepsMissingFields ? parsedRows.map(Self.nullsAsMissingFields) : parsedRows, + into: &tableRows + ) + } + let rowParser = parser ?? Self.detectParser(for: clipboardText) let parseResult = rowParser.parse(clipboardText, schema: schema) @@ -366,6 +374,26 @@ final class RowOperationsManager { } } + /// Cells the grid copied, as wide as the table, become rows field by field, so a value holding a + /// tab or a line break stays one value and a NULL stays NULL. An empty field is read for its + /// column, as a typed one is. + private static func rows( + fromCopiedCells cells: GridCellsClipboardPayload, + schema: TableSchema, + tableRows: TableRows + ) -> [ParsedRow] { + cells.fields.enumerated().map { index, fields in + var values: [PluginCellValue] = (0.. ParsedRow { diff --git a/TablePro/Models/UI/MultiRowEditState.swift b/TablePro/Models/UI/MultiRowEditState.swift index 6c700a9dda..7b0b0cda53 100644 --- a/TablePro/Models/UI/MultiRowEditState.swift +++ b/TablePro/Models/UI/MultiRowEditState.swift @@ -212,11 +212,14 @@ final class MultiRowEditState: ObservableObject { } // Mark externally modified columns (e.g., edited in data grid). A modified field that - // is now missing is one an edit removed. + // is now missing is one an edit removed, and one that now reads NULL is a pending NULL: + // written as an empty pending value it read as `''`, and the boolean picker showed false. if externallyModifiedColumns.contains(colIndex), pendingValue == nil, !isPendingNull, !isPendingDefault, !isPendingRemoval { if isAbsent { isPendingRemoval = true + } else if originalValue == nil, !hasMultipleValues { + isPendingNull = true } else { pendingValue = originalValue ?? "" } diff --git a/TablePro/Views/Results/DataGridView+RowActions.swift b/TablePro/Views/Results/DataGridView+RowActions.swift index a8c79b7575..a3724a8135 100644 --- a/TablePro/Views/Results/DataGridView+RowActions.swift +++ b/TablePro/Views/Results/DataGridView+RowActions.swift @@ -63,29 +63,29 @@ extension TableViewCoordinator { let cell = row.values[columnIndex] let columnTypes = tableRows.columnTypes let columnType = columnTypes.indices.contains(columnIndex) ? columnTypes[columnIndex] : nil + let text = cellCopyText(cell, columnIndex: columnIndex, columnType: columnType) + ClipboardService.shared.writeCells(GridCellsClipboardPayload(text: text, fields: [[cell.isNull ? nil : text]])) + } + private func cellCopyText(_ cell: PluginCellValue, columnIndex: Int, columnType: ColumnType?) -> String { if case .bytes(let data) = cell { let format = columnIndex < columnDisplayFormats.count ? columnDisplayFormats[columnIndex] : nil - let value = format.flatMap { format in + return format.flatMap { format in guard format.isApplicable(to: columnType, databaseType: databaseType) else { return nil } return ValueDisplayFormatter.apply(data, format: format, columnType: columnType) } ?? BlobFormattingService.shared.format(data, for: .copy) ?? "" - ClipboardService.shared.writeText(value) - return } let value = cell.asText ?? "NULL" if columnIndex < columnDisplayFormats.count, let format = columnDisplayFormats[columnIndex], format != .raw, let formatted = ValueDisplayFormatter.apply(value, format: format, columnType: columnType) { - ClipboardService.shared.writeText(formatted) - return + return formatted } - let copyValue = BlobFormattingService.shared.formatIfNeeded(value, columnType: columnType, for: .copy) - ClipboardService.shared.writeText(copyValue) + return BlobFormattingService.shared.formatIfNeeded(value, columnType: columnType, for: .copy) } func copyRowsAsInsert(at indices: Set) { @@ -404,14 +404,14 @@ extension TableViewCoordinator { rowActionsLogger.warning("Clipboard copy truncated: \(selectedRowCount) rows selected, capping at \(maxRows)") } - var lines: [String] = [] - lines.reserveCapacity(rowRange.count) + var copiedLines: [[String?]] = [] + copiedLines.reserveCapacity(rowRange.count) for rowIndex in rowRange { guard let row = displayRow(at: rowIndex) else { - lines.append(String(repeating: "\t", count: positions.count - 1)) + copiedLines.append(Array(repeating: "", count: positions.count)) continue } - var fields: [String] = [] + var fields: [String?] = [] fields.reserveCapacity(positions.count) for position in positions { guard selection.contains(row: rowIndex, displayColumn: position), @@ -421,16 +421,15 @@ extension TableViewCoordinator { continue } let columnType = columnTypes.indices.contains(columnIndex) ? columnTypes[columnIndex] : nil - let text = RowValueCopyFormatter.copyText(cell: row.values[columnIndex], columnType: columnType) ?? "NULL" - fields.append(text) + fields.append(RowValueCopyFormatter.copyText(cell: row.values[columnIndex], columnType: columnType)) } - lines.append(fields.joined(separator: "\t")) + copiedLines.append(fields) } - var text = lines.joined(separator: "\n") + var text = copiedLines.map { $0.map { $0 ?? "NULL" }.joined(separator: "\t") }.joined(separator: "\n") if isTruncated { text.append("\n(truncated, showing first \(maxRows) of \(selectedRowCount) rows)") } - ClipboardService.shared.writeText(text) + ClipboardService.shared.writeCells(GridCellsClipboardPayload(text: text, fields: copiedLines)) } } diff --git a/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift b/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift index 59d21ed9ad..86d7588611 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift @@ -16,7 +16,7 @@ extension TableViewCoordinator { /// Whether a paste would land in the focused cell rather than append rows. Answered without /// mutating anything so menu validation can ask it. func canPasteCellsFromClipboard(anchorRow: Int, anchorColumn: Int) -> Bool { - cellPasteGrid(anchorRow: anchorRow, anchorColumn: anchorColumn) != nil + cellPasteBlock(anchorRow: anchorRow, anchorColumn: anchorColumn) != nil } /// Fills cells rightwards from the anchor, in the order they appear on screen. @@ -26,13 +26,13 @@ extension TableViewCoordinator { /// whichever column happens to hold the next slot rather than the one beside it on screen, and a /// hidden column could swallow a value with nothing shown for it. func pasteCellsFromClipboard(anchorRow: Int, anchorColumn: Int) -> Bool { - guard let grid = cellPasteGrid(anchorRow: anchorRow, anchorColumn: anchorColumn), + guard let block = cellPasteBlock(anchorRow: anchorRow, anchorColumn: anchorColumn), let anchorPosition = displayPosition(ofDataColumnIndex: anchorColumn) else { return false } - let maxRow = min(anchorRow + grid.count, cachedRowCount) - let maxCol = min(anchorPosition + Self.blockWidth(grid), presentedColumnCount) + let maxRow = min(anchorRow + block.lines.count, cachedRowCount) + let maxCol = min(anchorPosition + block.width, presentedColumnCount) guard anchorRow < maxRow, anchorPosition < maxCol else { return false } let undoManager = tableView?.window?.undoManager @@ -41,16 +41,18 @@ extension TableViewCoordinator { let tableRows = tableRowsProvider() var refusedCell = false - for (gridRow, rowValues) in grid.enumerated() { + for (gridRow, fields) in block.lines.enumerated() { let targetRow = anchorRow + gridRow guard targetRow < maxRow else { break } guard !isRowDeleted(displayRow: targetRow) else { continue } - for (gridCol, cellValue) in rowValues.enumerated() { + for (gridCol, field) in fields.enumerated() { let targetPosition = anchorPosition + gridCol guard targetPosition < maxCol else { break } guard let targetCol = dataColumnIndex(atDisplayPosition: targetPosition) else { continue } - let text = Self.pastedText(cellValue, columnType: tableRows.editingColumnType(at: targetCol)) + let text = block.isGridCopy + ? field + : field.flatMap { Self.pastedText($0, columnType: tableRows.editingColumnType(at: targetCol)) } if !commitEnteredText(text, row: targetRow, columnIndex: targetCol) { refusedCell = true } @@ -66,6 +68,16 @@ extension TableViewCoordinator { return true } + /// A clipboard read as cells: the fields the grid's own copy carries, or the clipboard's text + /// split into lines and fields. Only the grid's copy can hold a NULL. + struct CellPasteBlock { + let lines: [[String?]] + let isGridCopy: Bool + + /// The widest line, since an empty line holds one field and may come first. + var width: Int { lines.map(\.count).max() ?? 0 } + } + /// The clipboard as a cell block, or nil when it belongs to the row-paste path instead. /// /// A clipboard holding one value fills the focused cell, which is what copying a cell and @@ -73,14 +85,15 @@ extension TableViewCoordinator { /// row paste, which appended a row and left the focused cell untouched. A block as wide as the /// table is still whole rows and stays with row paste, and so is anything TablePro's own row /// copy wrote, which carries its own pasteboard type. - private func cellPasteGrid(anchorRow: Int, anchorColumn: Int) -> [[String]]? { + private func cellPasteBlock(anchorRow: Int, anchorColumn: Int) -> CellPasteBlock? { guard isEditable else { return nil } if ClipboardService.shared.hasGridRows { return nil } guard let text = ClipboardService.shared.readText(), !text.isEmpty else { return nil } guard (text as NSString).length <= Self.maxCellPasteLength else { return nil } - let grid = Self.pastedLines(text).map { $0.components(separatedBy: "\t") } - guard let firstRow = grid.first else { return nil } + let block = Self.gridCopiedBlock(matching: text) + ?? CellPasteBlock(lines: Self.pastedLines(text).map { $0.components(separatedBy: "\t") }, isGridCopy: false) + guard let firstLine = block.lines.first else { return nil } /// Whole rows are routed away by the result's own column count, not the presented one. /// `TSVRowParser` fills from data index 0 and knows nothing about which columns are on @@ -88,30 +101,33 @@ extension TableViewCoordinator { /// or wrong columns. A block that matches the visible width but not the result's stays on /// the cell path, where every field is placed by display position. Row paste skips empty /// lines, so they do not decide the shape. - let isSingleValue = grid.count == 1 && firstRow.count == 1 + let isSingleValue = block.lines.count == 1 && firstLine.count == 1 let dataColumnCount = tableRowsProvider().columns.count - let rowLines = grid.filter { $0 != [""] } + let rowLines = block.lines.filter { $0 != [""] } if !isSingleValue, dataColumnCount > 0, !rowLines.isEmpty, rowLines.allSatisfy({ $0.count == dataColumnCount }) { return nil } guard let anchorPosition = displayPosition(ofDataColumnIndex: anchorColumn) else { return nil } - let maxRow = min(anchorRow + grid.count, cachedRowCount) - let maxCol = min(anchorPosition + Self.blockWidth(grid), presentedColumnCount) + let maxRow = min(anchorRow + block.lines.count, cachedRowCount) + let maxCol = min(anchorPosition + block.width, presentedColumnCount) guard anchorRow < maxRow, anchorPosition < maxCol else { return nil } - return grid + return block } - /// The widest line, since an empty line holds one field and may come first. - static func blockWidth(_ grid: [[String]]) -> Int { - grid.map(\.count).max() ?? 0 + /// The fields of the grid's own copy, while the clipboard still holds the text it wrote. They are + /// taken as they are rather than split from the text, where a value with a tab or a line break + /// in it would become several fields and a NULL would read as the text `NULL`. + private static func gridCopiedBlock(matching text: String) -> CellPasteBlock? { + guard let copied = ClipboardService.shared.readGridCells(), copied.text == text else { return nil } + return CellPasteBlock(lines: copied.fields, isGridCopy: true) } - /// The grid copies a NULL cell as the text `NULL`. A date, number or boolean cannot hold those - /// four letters, so there they paste as NULL rather than the server's zero date or an error. A - /// text column keeps them: the same text is what a stored `'NULL'` copies as. + /// Text from another app has no NULL of its own. A date, number or boolean cannot hold the + /// four letters `NULL`, so there they paste as NULL rather than the server's zero date or an + /// error; a text column keeps them. static func pastedText(_ field: String, columnType: ColumnType?) -> String? { guard field == "NULL", let columnType, !columnType.hasEmptyValue, columnType.enumValues?.contains(field) != true else { return field } diff --git a/TableProTests/Core/Services/ClipboardServiceTests.swift b/TableProTests/Core/Services/ClipboardServiceTests.swift index 8c9f4ff6e8..4408a42e86 100644 --- a/TableProTests/Core/Services/ClipboardServiceTests.swift +++ b/TableProTests/Core/Services/ClipboardServiceTests.swift @@ -57,4 +57,23 @@ struct ClipboardServiceTests { #expect(provider.readGridRows() == payload) } + + @Test("writeCells writes the text and round-trips the fields, NULL included") + func writeCellsRoundTripsFields() { + let provider = NSPasteboardClipboardProvider() + let cells = GridCellsClipboardPayload(text: "NULL\tNULL", fields: [[nil, "NULL"]]) + provider.writeCells(cells) + + #expect(provider.readText() == "NULL\tNULL") + #expect(provider.readGridCells() == cells) + } + + @Test("A later plain write drops the fields with the text they described") + func writeTextClearsFields() { + let provider = NSPasteboardClipboardProvider() + provider.writeCells(GridCellsClipboardPayload(text: "NULL", fields: [[nil]])) + provider.writeText("other") + + #expect(provider.readGridCells() == nil) + } } diff --git a/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift b/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift index 2711bc58c2..80819290f7 100644 --- a/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift +++ b/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift @@ -70,6 +70,32 @@ struct RowOperationsManagerPasteTests { #expect(values[2] == .null) } + @Test("Cells the grid copied as wide as the table paste field by field, NULL and line breaks kept") + func copiedCellsPasteFieldByField() { + let clipboard = RecordingClipboard() + clipboard.writeCells(GridCellsClipboardPayload( + text: "7\tSmith\nJohn\tNULL", + fields: [["7", "Smith\nJohn", nil]] + )) + + let rows = pasteRows(clipboard, manager: makeManager()) + + #expect(rows.count == 1) + #expect(rows.first?[1] == .text("Smith\nJohn")) + #expect(rows.first?[2] == .null) + } + + @Test("Copied cells stale against the clipboard's text are not used") + func staleCopiedCellsAreIgnored() { + let clipboard = RecordingClipboard() + clipboard.text = "7\tAnn\tann@test.com" + clipboard.gridCells = GridCellsClipboardPayload(text: "other", fields: [["7", "Bob", nil]]) + + let values = pasteValues(clipboard, manager: makeManager()) + + #expect(values[1] == .text("Ann")) + } + @Test("Structured payload wins over plain text when both are present") func structuredPreferredOverText() { let clipboard = RecordingClipboard() diff --git a/TableProTests/Helpers/RecordingClipboard.swift b/TableProTests/Helpers/RecordingClipboard.swift index 149a826f1f..a35c7167a7 100644 --- a/TableProTests/Helpers/RecordingClipboard.swift +++ b/TableProTests/Helpers/RecordingClipboard.swift @@ -9,6 +9,7 @@ final class RecordingClipboard: ClipboardProvider { var text: String? var gridRows: GridRowsClipboardPayload? + var gridCells: GridCellsClipboardPayload? private(set) var html: String? private(set) var writes: [String] = [] private(set) var images: [NSImage] = [] @@ -18,6 +19,7 @@ final class RecordingClipboard: ClipboardProvider { func readText() -> String? { text } func readGridRows() -> GridRowsClipboardPayload? { gridRows } + func readGridCells() -> GridCellsClipboardPayload? { gridCells } func writeText(_ text: String) { replace(text: text) } func writeCsv(_ csv: String) { replace(text: csv) } @@ -31,10 +33,20 @@ final class RecordingClipboard: ClipboardProvider { replace(text: tsv, html: html, gridRows: gridRows) } - private func replace(text: String?, html: String? = nil, gridRows: GridRowsClipboardPayload? = nil) { + func writeCells(_ cells: GridCellsClipboardPayload) { + replace(text: cells.text, gridCells: cells) + } + + private func replace( + text: String?, + html: String? = nil, + gridRows: GridRowsClipboardPayload? = nil, + gridCells: GridCellsClipboardPayload? = nil + ) { self.text = text self.html = html self.gridRows = gridRows + self.gridCells = gridCells if let text { writes.append(text) } } } diff --git a/TableProTests/Models/MultiRowEditStateTests.swift b/TableProTests/Models/MultiRowEditStateTests.swift index 92887594da..507414b760 100644 --- a/TableProTests/Models/MultiRowEditStateTests.swift +++ b/TableProTests/Models/MultiRowEditStateTests.swift @@ -917,8 +917,8 @@ struct MultiRowEditStateTests { #expect(sut.fields[1].pendingValue == "Alice") } - @Test("Uses empty string when original value is nil") - func usesEmptyStringWhenOriginalIsNil() { + @Test("A NULL the grid staged reads as a pending NULL, not an empty string") + func gridStagedNullIsPendingNull() { let sut = MultiRowEditState() sut.configure( selectedRowIndices: [0], @@ -928,7 +928,34 @@ struct MultiRowEditStateTests { externallyModifiedColumns: [0] ) #expect(sut.fields[0].hasEdit == true) - #expect(sut.fields[0].pendingValue == "") + #expect(sut.fields[0].pendingValue == nil) + #expect(FieldValueState.resolve(sut.fields[0]) == .pendingNull) + } + + @Test("A boolean set to NULL in the grid shows NULL in the inspector's picker, not false") + func gridStagedNullBooleanIsPendingNull() { + let sut = MultiRowEditState() + sut.configure( + selectedRowIndices: [0], + allRows: [["1", nil]], + columns: ["id", "active"], + columnTypes: [.integer(rawType: "INT"), .boolean(rawType: "TINYINT(1)")], + externallyModifiedColumns: [1] + ) + #expect(FieldValueState.resolve(sut.fields[1]) == .pendingNull) + } + + @Test("Rows that disagree keep showing that they disagree after an edit elsewhere") + func multipleValuesAreNotPendingNull() { + let sut = MultiRowEditState() + sut.configure( + selectedRowIndices: [0, 1], + allRows: [["1", "Alice"], ["2", "Bob"]], + columns: ["id", "name"], + columnTypes: [.text(rawType: nil), .text(rawType: nil)], + externallyModifiedColumns: [1] + ) + #expect(sut.fields[1].isPendingNull == false) } } diff --git a/TableProTests/Views/Results/Extensions/ClearedCellCommitTests.swift b/TableProTests/Views/Results/Extensions/ClearedCellCommitTests.swift index 78a185d7a9..c5b56bcc53 100644 --- a/TableProTests/Views/Results/Extensions/ClearedCellCommitTests.swift +++ b/TableProTests/Views/Results/Extensions/ClearedCellCommitTests.swift @@ -76,7 +76,7 @@ private struct ClearedCellFixture { .timestamp(rawType: "TIMESTAMP"), .json(rawType: "JSON") ], - columnNullable: ["deleted_at": true, "name": false, "created_at": false] + columnNullable: ["deleted_at": true, "name": true, "created_at": false] )) let store = store coordinator.tableRowsProvider = { store.tableRows } @@ -110,7 +110,7 @@ struct ClearedCellCommitTests { #expect(boundValue == nil) } - @Test("Clearing a text cell keeps the empty string, which is a value there, NOT NULL or not") + @Test("Clearing a text cell keeps the empty string, which is a value there") func clearedTextStaysEmpty() { let fixture = ClearedCellFixture() @@ -239,14 +239,19 @@ struct ClearedCellCommitTests { @Test("A pasted NULL is NULL where the type cannot hold the text, and text everywhere else") func pastedText() { let datetime = ColumnType.datetime(rawType: "DATETIME") - #expect(TableViewCoordinator.pastedText("NULL", columnType: datetime) == nil) - #expect(TableViewCoordinator.pastedText("NULL", columnType: .integer(rawType: "INT")) == nil) - #expect(TableViewCoordinator.pastedText("NULL", columnType: .text(rawType: "VARCHAR(20)")) == "NULL") - #expect(TableViewCoordinator.pastedText("NULL", columnType: .enumType(rawType: "ENUM", values: ["NULL", "a"])) == "NULL") - #expect(TableViewCoordinator.pastedText("NULL", columnType: nil) == "NULL") - #expect(TableViewCoordinator.pastedText("", columnType: datetime) == "") - #expect(TableViewCoordinator.pastedText("null", columnType: datetime) == "null") + let varchar = ColumnType.text(rawType: "VARCHAR(20)") + func pasted(_ field: String, _ columnType: ColumnType?) -> String? { + TableViewCoordinator.pastedText(field, columnType: columnType) + } + #expect(pasted("NULL", datetime) == nil) + #expect(pasted("NULL", .integer(rawType: "INT")) == nil) + #expect(pasted("NULL", varchar) == "NULL") + #expect(pasted("NULL", .enumType(rawType: "ENUM", values: ["NULL", "a"])) == "NULL") + #expect(pasted("NULL", nil) == "NULL") + #expect(pasted("", datetime) == "") + #expect(pasted("null", datetime) == "null") } + } extension GlobalStateSuites { @@ -287,6 +292,55 @@ extension GlobalStateSuites { #expect(fixture.value(row: 1, column: ClearedCellFixture.name) == .text("NULL")) } + @Test("A NULL copied from the grid pastes as NULL into a text cell") + func copiedNullRoundTripsIntoText() throws { + let fixture = ClearedCellFixture() + fixture.coordinator.commitCellEdit(row: 0, columnIndex: ClearedCellFixture.name, newValue: nil) + + fixture.coordinator.copyCellValue(at: 0, columnIndex: ClearedCellFixture.name) + let clipboard = try #require(RecordingClipboard.current) + #expect(clipboard.text == "NULL") + #expect(fixture.coordinator.pasteCellsFromClipboard(anchorRow: 1, anchorColumn: ClearedCellFixture.name)) + + #expect(fixture.value(row: 1, column: ClearedCellFixture.name) == .null) + } + + @Test("The grid's copy pastes field by field, so a line break inside a value moves nothing") + func gridCopyKeepsFieldBoundaries() throws { + let clipboard = try #require(RecordingClipboard.current) + clipboard.writeCells(GridCellsClipboardPayload(text: "first\nsecond\nNULL", fields: [["first\nsecond"], [nil]])) + let fixture = ClearedCellFixture() + + #expect(fixture.coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: ClearedCellFixture.name)) + + #expect(fixture.value(row: 0, column: ClearedCellFixture.name) == .text("first\nsecond")) + #expect(fixture.value(row: 1, column: ClearedCellFixture.name) == .null) + #expect(fixture.value(row: 2, column: ClearedCellFixture.name) == .text("Carol")) + } + + @Test("Fields left behind by text another app wrote over the copy are not used") + func staleGridCopyIsIgnored() throws { + let clipboard = try #require(RecordingClipboard.current) + clipboard.text = "NULL" + clipboard.gridCells = GridCellsClipboardPayload(text: "other", fields: [[nil]]) + let fixture = ClearedCellFixture() + + #expect(fixture.coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: ClearedCellFixture.name)) + + #expect(fixture.value(row: 0, column: ClearedCellFixture.name) == .text("NULL")) + } + + @Test("A stored 'NULL' copied from the grid pastes as the same text") + func copiedNullTextRoundTripsAsText() throws { + let fixture = ClearedCellFixture() + fixture.coordinator.commitCellEdit(row: 0, columnIndex: ClearedCellFixture.name, newValue: "NULL") + + fixture.coordinator.copyCellValue(at: 0, columnIndex: ClearedCellFixture.name) + #expect(fixture.coordinator.pasteCellsFromClipboard(anchorRow: 2, anchorColumn: ClearedCellFixture.name)) + + #expect(fixture.value(row: 2, column: ClearedCellFixture.name) == .text("NULL")) + } + @Test("A copied NULL pasted into a NOT NULL column leaves the cell as it was") func pastedNullIntoNotNullIsRefused() throws { let stub = try #require(RecordingClipboard.current)