From 57fc1326697b3a8683c433ab8183cc51cfe5f99e Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 27 May 2026 18:55:07 +0700 Subject: [PATCH] fix(datagrid): keep pasted rows aligned when a cell value contains a comma --- CHANGELOG.md | 1 + .../Infrastructure/ClipboardService.swift | 20 +- .../Services/Query/RowOperationsManager.swift | 90 +++++---- .../Results/DataGridView+RowActions.swift | 17 +- .../Core/Services/ClipboardServiceTests.swift | 16 ++ .../RowOperationsManagerCopyTests.swift | 35 +++- .../RowOperationsManagerPasteTests.swift | 191 ++++++++++++++++++ .../Extensions/CellPasteRoutingTests.swift | 10 +- 8 files changed, 332 insertions(+), 48 deletions(-) create mode 100644 TableProTests/Core/Services/RowOperationsManagerPasteTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b1628910..bf06ab8ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Pasting copied rows no longer misplaces values when a cell contains a comma (such as a user agent string); each value stays in its own column, and a real NULL is kept distinct from the literal text "NULL". - BigQuery: switching to another table now loads its data right away, instead of leaving the grid empty until you close and reopen the tab. - Custom and OpenAI-compatible AI providers now work when the base URL already ends in `/v1`, instead of building a doubled `/v1/v1/` path that failed. (#1400) - MongoDB: opening a collection no longer crashes when a document contains a NaN or infinite number. (#1418) diff --git a/TablePro/Core/Services/Infrastructure/ClipboardService.swift b/TablePro/Core/Services/Infrastructure/ClipboardService.swift index 2a82bc0a2..1c9e3cf96 100644 --- a/TablePro/Core/Services/Infrastructure/ClipboardService.swift +++ b/TablePro/Core/Services/Infrastructure/ClipboardService.swift @@ -4,13 +4,20 @@ // import AppKit +import TableProPluginKit import UniformTypeIdentifiers +struct GridRowsClipboardPayload: Codable, Equatable { + let columns: [String] + let rows: [[PluginCellValue]] +} + protocol ClipboardProvider { func readText() -> String? + func readGridRows() -> GridRowsClipboardPayload? func writeText(_ text: String) func writeCsv(_ csv: String) - func writeRows(tsv: String, html: String?) + func writeRows(tsv: String, html: String?, gridRows: GridRowsClipboardPayload) var hasText: Bool { get } var hasGridRows: Bool { get } } @@ -24,6 +31,11 @@ struct NSPasteboardClipboardProvider: ClipboardProvider { NSPasteboard.general.string(forType: .string) } + func readGridRows() -> GridRowsClipboardPayload? { + guard let data = NSPasteboard.general.data(forType: Self.gridRowsType) else { return nil } + return try? JSONDecoder().decode(GridRowsClipboardPayload.self, from: data) + } + func writeText(_ text: String) { let pb = NSPasteboard.general pb.clearContents() @@ -39,7 +51,7 @@ struct NSPasteboardClipboardProvider: ClipboardProvider { pb.setString(csv, forType: Self.csvType) } - func writeRows(tsv: String, html: String?) { + func writeRows(tsv: String, html: String?, gridRows: GridRowsClipboardPayload) { let pb = NSPasteboard.general pb.clearContents() pb.setString(tsv, forType: .string) @@ -47,7 +59,9 @@ struct NSPasteboardClipboardProvider: ClipboardProvider { if let html { pb.setString(html, forType: .html) } - pb.setString("1", forType: Self.gridRowsType) + if let data = try? JSONEncoder().encode(gridRows) { + pb.setData(data, forType: Self.gridRowsType) + } } var hasText: Bool { diff --git a/TablePro/Core/Services/Query/RowOperationsManager.swift b/TablePro/Core/Services/Query/RowOperationsManager.swift index 1957eb4d4..3f84af036 100644 --- a/TablePro/Core/Services/Query/RowOperationsManager.swift +++ b/TablePro/Core/Services/Query/RowOperationsManager.swift @@ -253,6 +253,8 @@ final class RowOperationsManager { let estimatedRowLength = max(columns.count, 1) * 12 var result = "" result.reserveCapacity(indicesToCopy.count * estimatedRowLength) + var structuredRows: [[PluginCellValue]] = [] + structuredRows.reserveCapacity(indicesToCopy.count) if includeHeaders, !columns.isEmpty { for (colIdx, col) in columns.enumerated() { @@ -265,6 +267,7 @@ final class RowOperationsManager { guard rowIndex < tableRows.count else { continue } if !result.isEmpty { result.append("\n") } let cells = projection.values(Array(tableRows.rows[rowIndex].values)) + structuredRows.append(cells) for (colIdx, cell) in cells.enumerated() { if colIdx > 0 { result.append("\t") } switch cell { @@ -282,7 +285,8 @@ final class RowOperationsManager { result.append("\n(truncated, showing first \(Self.maxClipboardRows) of \(totalSelected) rows)") } - ClipboardService.shared.writeRows(tsv: result, html: nil) + let payload = GridRowsClipboardPayload(columns: columns, rows: structuredRows) + ClipboardService.shared.writeRows(tsv: result, html: nil, gridRows: payload) } func pasteRowsFromClipboard( @@ -293,15 +297,20 @@ final class RowOperationsManager { parser: RowDataParser? = nil ) -> PasteRowsResult { let clipboardProvider = clipboard ?? ClipboardService.shared - guard let clipboardText = clipboardProvider.readText() else { - return PasteRowsResult(pastedRows: [], delta: .none) - } - let schema = TableSchema( columns: columns, primaryKeyColumns: primaryKeyColumns ) + if parser == nil, let payload = clipboardProvider.readGridRows() { + let parsedRows = Self.reconcileStructuredRows(payload, schema: schema) + return insertParsedRows(parsedRows, into: &tableRows) + } + + guard let clipboardText = clipboardProvider.readText() else { + return PasteRowsResult(pastedRows: [], delta: .none) + } + let rowParser = parser ?? Self.detectParser(for: clipboardText) let parseResult = rowParser.parse(clipboardText, schema: schema) @@ -315,47 +324,54 @@ final class RowOperationsManager { } } - static func detectParser(for text: String) -> RowDataParser { - var tabLines = 0 - var commaLines = 0 - var nonEmptyLines = 0 - var lineHasTab = false - var lineHasComma = false - var lineIsEmpty = true + private static func reconcileStructuredRows( + _ payload: GridRowsClipboardPayload, + schema: TableSchema + ) -> [ParsedRow] { + let sourceForDestination = sourceColumnIndices(from: payload.columns, to: schema.columns) - for char in text { - if char.isNewline { - if !lineIsEmpty { - nonEmptyLines += 1 - if lineHasTab { tabLines += 1 } - if lineHasComma { commaLines += 1 } - } - lineHasTab = false - lineHasComma = false - lineIsEmpty = true - } else { - if !char.isWhitespace { lineIsEmpty = false } - if char == "\t" { lineHasTab = true } - if char == "," { lineHasComma = true } + return payload.rows.enumerated().map { index, row in + var values: [PluginCellValue] = sourceForDestination.map { sourceIndex in + guard let sourceIndex, sourceIndex < row.count else { return .null } + return row[sourceIndex] + } + + if let pkIndex = schema.primaryKeyIndex, pkIndex < values.count { + values[pkIndex] = .text("__DEFAULT__") } + + return ParsedRow(values: values, sourceLineNumber: index + 1) } - if !lineIsEmpty { - nonEmptyLines += 1 - if lineHasTab { tabLines += 1 } - if lineHasComma { commaLines += 1 } + } + + private static func sourceColumnIndices(from source: [String], to destination: [String]) -> [Int?] { + var sourceIndexByName: [String: Int] = [:] + for (index, name) in source.enumerated() where sourceIndexByName[name] == nil { + sourceIndexByName[name] = index } - guard nonEmptyLines > 0 else { return TSVRowParser() } + let byName = destination.map { sourceIndexByName[$0] } + guard byName.allSatisfy({ $0 == nil }) else { return byName } - let tabCount = tabLines - let commaCount = commaLines + return destination.indices.map { $0 < source.count ? $0 : nil } + } + + static func detectParser(for text: String) -> RowDataParser { + var containsTab = false + var containsComma = false + + for char in text { + if char == "\t" { + containsTab = true + break + } + if char == "," { containsComma = true } + } - if tabCount > commaCount { + if containsTab { return TSVRowParser() - } else if commaCount > 0 { - return CSVRowParser() } - return TSVRowParser() + return containsComma ? CSVRowParser() : TSVRowParser() } private func insertParsedRows( diff --git a/TablePro/Views/Results/DataGridView+RowActions.swift b/TablePro/Views/Results/DataGridView+RowActions.swift index d9028e737..bfa91dca8 100644 --- a/TablePro/Views/Results/DataGridView+RowActions.swift +++ b/TablePro/Views/Results/DataGridView+RowActions.swift @@ -42,19 +42,24 @@ extension TableViewCoordinator { let tableRows = tableRowsProvider() let projection = visibleColumnProjection let columnTypes = projection.columnTypes(tableRows.columnTypes) + let columns = projection.columns(tableRows.columns) var tsvRows: [String] = [] var htmlRows: [[String]] = [] + var structuredRows: [[PluginCellValue]] = [] for index in sortedIndices { guard let values = displayRow(at: index)?.values else { continue } - let formatted = formatRowValues(values: projection.values(Array(values)), columnTypes: columnTypes) + let projected = projection.values(Array(values)) + let formatted = formatRowValues(values: projected, columnTypes: columnTypes) tsvRows.append(formatted.joined(separator: "\t")) htmlRows.append(formatted) + structuredRows.append(projected) } let tsv = tsvRows.joined(separator: "\n") let html = HtmlTableEncoder.encode(rows: htmlRows) - ClipboardService.shared.writeRows(tsv: tsv, html: html) + let payload = GridRowsClipboardPayload(columns: columns, rows: structuredRows) + ClipboardService.shared.writeRows(tsv: tsv, html: html, gridRows: payload) } func copyRowsWithHeaders(at indices: Set) { @@ -65,17 +70,21 @@ extension TableViewCoordinator { let columns = projection.columns(tableRows.columns) var tsvRows: [String] = [columns.joined(separator: "\t")] var htmlRows: [[String]] = [] + var structuredRows: [[PluginCellValue]] = [] for index in sortedIndices { guard let values = displayRow(at: index)?.values else { continue } - let formatted = formatRowValues(values: projection.values(Array(values)), columnTypes: columnTypes) + let projected = projection.values(Array(values)) + let formatted = formatRowValues(values: projected, columnTypes: columnTypes) tsvRows.append(formatted.joined(separator: "\t")) htmlRows.append(formatted) + structuredRows.append(projected) } let tsv = tsvRows.joined(separator: "\n") let html = HtmlTableEncoder.encode(rows: htmlRows, headers: columns) - ClipboardService.shared.writeRows(tsv: tsv, html: html) + let payload = GridRowsClipboardPayload(columns: columns, rows: structuredRows) + ClipboardService.shared.writeRows(tsv: tsv, html: html, gridRows: payload) } @MainActor diff --git a/TableProTests/Core/Services/ClipboardServiceTests.swift b/TableProTests/Core/Services/ClipboardServiceTests.swift index 7a3cba2db..c3ee7d7a9 100644 --- a/TableProTests/Core/Services/ClipboardServiceTests.swift +++ b/TableProTests/Core/Services/ClipboardServiceTests.swift @@ -5,6 +5,7 @@ import AppKit @testable import TablePro +import TableProPluginKit import Testing import UniformTypeIdentifiers @@ -42,4 +43,19 @@ struct ClipboardServiceTests { let pb = NSPasteboard.general #expect(pb.string(forType: Self.tsvType) == nil) } + + @Test("writeRows round-trips structured grid rows through readGridRows losslessly") + func writeRowsRoundTripsStructuredRows() { + let provider = NSPasteboardClipboardProvider() + let payload = GridRowsClipboardPayload( + columns: ["id", "name", "blob"], + rows: [ + [.text("1"), .text("Smith, John"), .null], + [.text("2"), .text("NULL"), .bytes(Data([0x01, 0x02, 0xFF]))], + ] + ) + provider.writeRows(tsv: "1\tSmith, John\tNULL", html: nil, gridRows: payload) + + #expect(provider.readGridRows() == payload) + } } diff --git a/TableProTests/Core/Services/RowOperationsManagerCopyTests.swift b/TableProTests/Core/Services/RowOperationsManagerCopyTests.swift index 35ff79447..c3bf4bac2 100644 --- a/TableProTests/Core/Services/RowOperationsManagerCopyTests.swift +++ b/TableProTests/Core/Services/RowOperationsManagerCopyTests.swift @@ -5,11 +5,15 @@ import Testing private final class MockClipboardProvider: ClipboardProvider { var lastWrittenText: String? + var lastWrittenGridRows: GridRowsClipboardPayload? var textToRead: String? + var gridRowsToRead: GridRowsClipboardPayload? var lastWasGridRows = false func readText() -> String? { textToRead } + func readGridRows() -> GridRowsClipboardPayload? { gridRowsToRead } + func writeText(_ text: String) { lastWrittenText = text lastWasGridRows = false @@ -20,8 +24,9 @@ private final class MockClipboardProvider: ClipboardProvider { lastWasGridRows = false } - func writeRows(tsv: String, html: String?) { + func writeRows(tsv: String, html: String?, gridRows: GridRowsClipboardPayload) { lastWrittenText = tsv + lastWrittenGridRows = gridRows lastWasGridRows = true } @@ -266,4 +271,32 @@ struct RowOperationsManagerCopyTests { #expect(result == "1\tAlice\talice@test.com") } + + @Test("Copy writes structured grid rows with column names and raw cell values") + func copyWritesStructuredGridRows() { + let (manager, _) = makeManager() + let rows: [[String?]] = [["1", "Smith, John", nil]] + let clipboard = MockClipboardProvider() + ClipboardService.shared = clipboard + let tableRows = makeTableRows(rows: rows) + + manager.copySelectedRowsToClipboard(selectedIndices: [0], tableRows: tableRows) + + #expect(clipboard.lastWrittenGridRows?.columns == ["id", "name", "email"]) + #expect(clipboard.lastWrittenGridRows?.rows == [[.text("1"), .text("Smith, John"), .null]]) + } + + @Test("Structured grid rows follow visible column projection and visual order") + func structuredGridRowsFollowProjection() { + let (manager, _) = makeManager() + let rows: [[String?]] = [["1", "Alice", "alice@test.com"]] + let clipboard = MockClipboardProvider() + ClipboardService.shared = clipboard + let tableRows = makeTableRows(rows: rows) + + manager.copySelectedRowsToClipboard(selectedIndices: [0], tableRows: tableRows, visibleColumnIndices: [2, 0]) + + #expect(clipboard.lastWrittenGridRows?.columns == ["email", "id"]) + #expect(clipboard.lastWrittenGridRows?.rows == [[.text("alice@test.com"), .text("1")]]) + } } diff --git a/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift b/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift new file mode 100644 index 000000000..d0243091d --- /dev/null +++ b/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift @@ -0,0 +1,191 @@ +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +private final class PasteMockClipboard: ClipboardProvider { + var textToRead: String? + var gridRowsToRead: GridRowsClipboardPayload? + + func readText() -> String? { textToRead } + func readGridRows() -> GridRowsClipboardPayload? { gridRowsToRead } + func writeText(_ text: String) {} + func writeCsv(_ csv: String) {} + func writeRows(tsv: String, html: String?, gridRows: GridRowsClipboardPayload) {} + var hasText: Bool { textToRead != nil } + var hasGridRows: Bool { gridRowsToRead != nil } +} + +@MainActor +@Suite("RowOperationsManager Paste") +struct RowOperationsManagerPasteTests { + private static let columns = ["id", "name", "email"] + + private func makeManager() -> RowOperationsManager { + let changeManager = DataChangeManager() + changeManager.configureForTable( + tableName: "users", + columns: Self.columns, + primaryKeyColumns: ["id"], + databaseType: .mysql + ) + return RowOperationsManager(changeManager: changeManager) + } + + private func emptyRows() -> TableRows { + let types: [ColumnType] = Array(repeating: .text(rawType: nil), count: Self.columns.count) + return TableRows.from(queryRows: [], columns: Self.columns, columnTypes: types) + } + + private func pasteRows(_ clipboard: PasteMockClipboard, manager: RowOperationsManager) -> [[PluginCellValue]] { + var rows = emptyRows() + let result = manager.pasteRowsFromClipboard( + columns: Self.columns, + primaryKeyColumns: ["id"], + tableRows: &rows, + clipboard: clipboard + ) + return result.pastedRows.map { $0.values } + } + + private func pasteValues(_ clipboard: PasteMockClipboard, manager: RowOperationsManager) -> [PluginCellValue] { + pasteRows(clipboard, manager: manager).first ?? [] + } + + @Test("Structured grid rows preserve comma-containing values") + func structuredPreservesCommas() { + let clipboard = PasteMockClipboard() + clipboard.gridRowsToRead = GridRowsClipboardPayload( + columns: Self.columns, + rows: [[.text("7"), .text("Smith, John"), .text("a@test.com")]] + ) + + let values = pasteValues(clipboard, manager: makeManager()) + + #expect(values.count == 3) + #expect(values[0] == .text("__DEFAULT__")) + #expect(values[1] == .text("Smith, John")) + #expect(values[2] == .text("a@test.com")) + } + + @Test("Structured grid rows distinguish a real NULL from the literal string NULL") + func structuredDistinguishesNull() { + let clipboard = PasteMockClipboard() + clipboard.gridRowsToRead = GridRowsClipboardPayload( + columns: Self.columns, + rows: [[.text("7"), .text("NULL"), .null]] + ) + + let values = pasteValues(clipboard, manager: makeManager()) + + #expect(values[1] == .text("NULL")) + #expect(values[2] == .null) + } + + @Test("Structured payload wins over plain text when both are present") + func structuredPreferredOverText() { + let clipboard = PasteMockClipboard() + clipboard.gridRowsToRead = GridRowsClipboardPayload( + columns: Self.columns, + rows: [[.text("7"), .text("structured"), .text("x@test.com")]] + ) + clipboard.textToRead = "7\twrong\twrong@test.com" + + let values = pasteValues(clipboard, manager: makeManager()) + + #expect(values[1] == .text("structured")) + } + + @Test("Structured grid rows map source columns to destination by name when reordered") + func structuredMapsByColumnName() { + let clipboard = PasteMockClipboard() + clipboard.gridRowsToRead = GridRowsClipboardPayload( + columns: ["email", "id", "name"], + rows: [[.text("e@x.com"), .text("9"), .text("Jo")]] + ) + + let values = pasteValues(clipboard, manager: makeManager()) + + #expect(values[0] == .text("__DEFAULT__")) + #expect(values[1] == .text("Jo")) + #expect(values[2] == .text("e@x.com")) + } + + @Test("Destination columns absent from the source fill with NULL (e.g. a hidden column copy)") + func structuredFillsMissingColumnsWithNull() { + let clipboard = PasteMockClipboard() + clipboard.gridRowsToRead = GridRowsClipboardPayload( + columns: ["name", "email"], + rows: [[.text("Jo"), .text("e@x.com")]] + ) + + let values = pasteValues(clipboard, manager: makeManager()) + + #expect(values.count == 3) + #expect(values[0] == .text("__DEFAULT__")) + #expect(values[1] == .text("Jo")) + #expect(values[2] == .text("e@x.com")) + } + + @Test("Unrelated source columns fall back to positional mapping") + func structuredFallsBackToPositional() { + let clipboard = PasteMockClipboard() + clipboard.gridRowsToRead = GridRowsClipboardPayload( + columns: ["a", "b", "c"], + rows: [[.text("1"), .text("2"), .text("3")]] + ) + + let values = pasteValues(clipboard, manager: makeManager()) + + #expect(values[0] == .text("__DEFAULT__")) + #expect(values[1] == .text("2")) + #expect(values[2] == .text("3")) + } + + @Test("Structured payload pastes multiple rows") + func structuredPastesMultipleRows() { + let clipboard = PasteMockClipboard() + clipboard.gridRowsToRead = GridRowsClipboardPayload( + columns: Self.columns, + rows: [ + [.text("1"), .text("Alice"), .text("a@test.com")], + [.text("2"), .text("Bob"), .null], + ] + ) + + let rows = pasteRows(clipboard, manager: makeManager()) + + #expect(rows.count == 2) + #expect(rows[0] == [.text("__DEFAULT__"), .text("Alice"), .text("a@test.com")]) + #expect(rows[1] == [.text("__DEFAULT__"), .text("Bob"), .null]) + } + + @Test("Plain-text TSV with commas inside a value parses as TSV, not CSV") + func textTSVWithCommasParsesAsTSV() { + let clipboard = PasteMockClipboard() + clipboard.textToRead = "7\tMozilla/5.0 (KHTML, like Gecko) Safari\ta@test.com" + + let values = pasteValues(clipboard, manager: makeManager()) + + #expect(values.count == 3) + #expect(values[0] == .text("__DEFAULT__")) + #expect(values[1] == .text("Mozilla/5.0 (KHTML, like Gecko) Safari")) + #expect(values[2] == .text("a@test.com")) + } + + @Test("detectParser picks TSV when a tab is present even alongside commas") + func detectParserTabWins() { + #expect(RowOperationsManager.detectParser(for: "a\tb,c") is TSVRowParser) + #expect(RowOperationsManager.detectParser(for: "a,b\tc") is TSVRowParser) + } + + @Test("detectParser picks CSV only when commas appear without tabs") + func detectParserCsvWhenNoTabs() { + #expect(RowOperationsManager.detectParser(for: "a,b,c") is CSVRowParser) + } + + @Test("detectParser defaults to TSV for single-column text") + func detectParserDefaultsToTSV() { + #expect(RowOperationsManager.detectParser(for: "single") is TSVRowParser) + } +} diff --git a/TableProTests/Views/Results/Extensions/CellPasteRoutingTests.swift b/TableProTests/Views/Results/Extensions/CellPasteRoutingTests.swift index fc83a5aa9..11f330cc0 100644 --- a/TableProTests/Views/Results/Extensions/CellPasteRoutingTests.swift +++ b/TableProTests/Views/Results/Extensions/CellPasteRoutingTests.swift @@ -10,10 +10,10 @@ import AppKit import Foundation -import TableProPluginKit import SwiftUI -import Testing @testable import TablePro +import TableProPluginKit +import Testing @MainActor private final class NoopColumnLayoutPersister: ColumnLayoutPersisting { @@ -28,9 +28,13 @@ private final class StubClipboard: ClipboardProvider { var hasGridRowsValue = false func readText() -> String? { text } + func readGridRows() -> GridRowsClipboardPayload? { nil } func writeText(_ text: String) { self.text = text; hasGridRowsValue = false } func writeCsv(_ csv: String) { self.text = csv; hasGridRowsValue = false } - func writeRows(tsv: String, html: String?) { self.text = tsv; hasGridRowsValue = true } + func writeRows(tsv: String, html: String?, gridRows: GridRowsClipboardPayload) { + self.text = tsv + hasGridRowsValue = true + } var hasText: Bool { text != nil } var hasGridRows: Bool { hasGridRowsValue } }