Skip to content

Commit 7c1d844

Browse files
authored
fix(datagrid): route Cmd+V to row paste when clipboard holds copied rows (#935)
* fix(datagrid): route Cmd+V to row paste when clipboard holds copied rows Cmd+C on a row selection writes TSV via ClipboardService and Cmd+V then routed through pasteCellsFromClipboard, which overwrote cells starting at the focused cell with the source row's values, instead of inserting a new row. The asymmetry was: Cmd+C always operated on selectedRowIndexes (row level), Cmd+V chose cell paste whenever any cell was focused, so copying then pasting in the same selection looked like the row's tail columns silently mutated. Two signals now defer cell paste to dataGridPasteRows (which inserts): 1. The clipboard was written by this app's row-copy path. ClipboardProvider gains a writeRows(tsv:html:?) method that tags the pasteboard with com.TablePro.gridRows. pasteCellsFromClipboard checks hasGridRows first and bails when set. 2. The clipboard's TSV shape is full-row (every line has exactly numberOfDataColumns values). Catches external row-shaped data the user clearly intended as a row insert. Migrates RowOperationsManager.copySelectedRowsToClipboard and TableViewCoordinator.copyRows / copyRowsWithHeaders from writeText / writeTabular to writeRows. Drops the now-unused writeTabular method from the protocol. Cell paste continues to work for external single-column or shape-mismatched TSV (clearly cell-shaped data from spreadsheets etc.). Right-click row > Paste and Cmd+V with no cell focus already went straight to row paste; unchanged. * test(datagrid): add regression tests for pasteCellsFromClipboard routing Locks the contract that pasteCellsFromClipboard returns false (deferring to dataGridPasteRows) on the two new signals: clipboard carries the gridRows tag, or every TSV line matches the table's column count. Without these tests, the bug from this same PR could regress silently if someone changes the pasteboard tag string or removes the shape check. Also covers the cell-paste happy path (shape-mismatched TSV) and the read-only short-circuit.
1 parent 39e1905 commit 7c1d844

6 files changed

Lines changed: 130 additions & 8 deletions

File tree

TablePro/Core/Services/Infrastructure/ClipboardService.swift

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,14 @@ import UniformTypeIdentifiers
1212
protocol ClipboardProvider {
1313
func readText() -> String?
1414
func writeText(_ text: String)
15-
func writeTabular(tsv: String, html: String)
15+
func writeRows(tsv: String, html: String?)
1616
var hasText: Bool { get }
17+
var hasGridRows: Bool { get }
1718
}
1819

1920
struct NSPasteboardClipboardProvider: ClipboardProvider {
2021
private static let tsvType = NSPasteboard.PasteboardType("public.utf8-tab-separated-values-text")
22+
private static let gridRowsType = NSPasteboard.PasteboardType("com.TablePro.gridRows")
2123

2224
func readText() -> String? {
2325
NSPasteboard.general.string(forType: .string)
@@ -30,17 +32,24 @@ struct NSPasteboardClipboardProvider: ClipboardProvider {
3032
pb.setString(text, forType: NSPasteboard.PasteboardType(UTType.utf8PlainText.identifier))
3133
}
3234

33-
func writeTabular(tsv: String, html: String) {
35+
func writeRows(tsv: String, html: String?) {
3436
let pb = NSPasteboard.general
3537
pb.clearContents()
3638
pb.setString(tsv, forType: .string)
3739
pb.setString(tsv, forType: Self.tsvType)
38-
pb.setString(html, forType: .html)
40+
if let html {
41+
pb.setString(html, forType: .html)
42+
}
43+
pb.setString("1", forType: Self.gridRowsType)
3944
}
4045

4146
var hasText: Bool {
4247
NSPasteboard.general.string(forType: .string) != nil
4348
}
49+
50+
var hasGridRows: Bool {
51+
NSPasteboard.general.types?.contains(Self.gridRowsType) == true
52+
}
4453
}
4554

4655
@MainActor

TablePro/Core/Services/Query/RowOperationsManager.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ final class RowOperationsManager {
281281
result.append("\n(truncated, showing first \(Self.maxClipboardRows) of \(totalSelected) rows)")
282282
}
283283

284-
ClipboardService.shared.writeText(result)
284+
ClipboardService.shared.writeRows(tsv: result, html: nil)
285285
}
286286

287287
func pasteRowsFromClipboard(

TablePro/Views/Results/DataGridView+RowActions.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ extension TableViewCoordinator {
4949

5050
let tsv = tsvRows.joined(separator: "\n")
5151
let html = HtmlTableEncoder.encode(rows: htmlRows)
52-
ClipboardService.shared.writeTabular(tsv: tsv, html: html)
52+
ClipboardService.shared.writeRows(tsv: tsv, html: html)
5353
}
5454

5555
func copyRowsWithHeaders(at indices: Set<Int>) {
@@ -69,7 +69,7 @@ extension TableViewCoordinator {
6969

7070
let tsv = tsvRows.joined(separator: "\n")
7171
let html = HtmlTableEncoder.encode(rows: htmlRows, headers: columns)
72-
ClipboardService.shared.writeTabular(tsv: tsv, html: html)
72+
ClipboardService.shared.writeRows(tsv: tsv, html: html)
7373
}
7474

7575
@MainActor

TablePro/Views/Results/Extensions/DataGridView+CellPaste.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,21 @@ import AppKit
88
extension TableViewCoordinator {
99
func pasteCellsFromClipboard(anchorRow: Int, anchorColumn: Int) -> Bool {
1010
guard isEditable else { return false }
11+
if ClipboardService.shared.hasGridRows { return false }
1112
guard let text = ClipboardService.shared.readText(), !text.isEmpty else { return false }
1213

1314
let grid = text.components(separatedBy: "\n")
1415
.filter { !$0.isEmpty }
1516
.map { $0.components(separatedBy: "\t") }
1617
guard !grid.isEmpty, grid[0].count > 1 || grid.count > 1 else { return false }
1718

19+
let dataColumnCount = tableRowsProvider().columns.count
20+
if dataColumnCount > 0, grid.allSatisfy({ $0.count == dataColumnCount }) {
21+
return false
22+
}
23+
1824
let maxRow = min(anchorRow + grid.count, cachedRowCount)
19-
let maxCol = min(anchorColumn + (grid.first?.count ?? 0), tableRowsProvider().columns.count)
25+
let maxCol = min(anchorColumn + (grid.first?.count ?? 0), dataColumnCount)
2026
guard anchorRow < maxRow, anchorColumn < maxCol else { return false }
2127

2228
let undoManager = tableView?.window?.undoManager

TableProTests/Core/Services/RowOperationsManagerCopyTests.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,22 @@ import Testing
55
private final class MockClipboardProvider: ClipboardProvider {
66
var lastWrittenText: String?
77
var textToRead: String?
8+
var lastWasGridRows = false
89

910
func readText() -> String? { textToRead }
1011

1112
func writeText(_ text: String) {
1213
lastWrittenText = text
14+
lastWasGridRows = false
1315
}
1416

15-
func writeTabular(tsv: String, html: String) {
17+
func writeRows(tsv: String, html: String?) {
1618
lastWrittenText = tsv
19+
lastWasGridRows = true
1720
}
1821

1922
var hasText: Bool { textToRead != nil }
23+
var hasGridRows: Bool { lastWasGridRows }
2024
}
2125

2226
@MainActor
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
//
2+
// CellPasteRoutingTests.swift
3+
// TableProTests
4+
//
5+
// Locks the contract that pasteCellsFromClipboard defers to row paste
6+
// when the clipboard carries the in-app gridRows tag or has a row-shaped
7+
// TSV. Without these checks, Cmd+V on a focused cell after Cmd+C on a row
8+
// silently overwrites the row's tail columns.
9+
//
10+
11+
import AppKit
12+
import Foundation
13+
import SwiftUI
14+
import Testing
15+
@testable import TablePro
16+
17+
@MainActor
18+
private final class StubClipboard: ClipboardProvider {
19+
var text: String?
20+
var hasGridRowsValue = false
21+
22+
func readText() -> String? { text }
23+
func writeText(_ text: String) { self.text = text; hasGridRowsValue = false }
24+
func writeRows(tsv: String, html: String?) { self.text = tsv; hasGridRowsValue = true }
25+
var hasText: Bool { text != nil }
26+
var hasGridRows: Bool { hasGridRowsValue }
27+
}
28+
29+
@Suite("pasteCellsFromClipboard routing")
30+
@MainActor
31+
struct CellPasteRoutingTests {
32+
private func makeCoordinator(columns: [String], rowCount: Int) -> TableViewCoordinator {
33+
let coordinator = TableViewCoordinator(
34+
changeManager: AnyChangeManager(DataChangeManager()),
35+
isEditable: true,
36+
selectedRowIndices: .constant([]),
37+
delegate: nil
38+
)
39+
let columnTypes: [ColumnType] = Array(repeating: .text(rawType: nil), count: columns.count)
40+
let rows = (0..<rowCount).map { i in (0..<columns.count).map { c in "r\(i)c\(c)" } }
41+
let tableRows = TableRows.from(queryRows: rows, columns: columns, columnTypes: columnTypes)
42+
coordinator.tableRowsProvider = { tableRows }
43+
coordinator.updateCache()
44+
return coordinator
45+
}
46+
47+
@Test("Defers to row paste when clipboard has gridRows tag")
48+
func defersOnGridRowsTag() {
49+
let stub = StubClipboard()
50+
stub.text = "anything\twith\ttabs"
51+
stub.hasGridRowsValue = true
52+
ClipboardService.shared = stub
53+
54+
let coordinator = makeCoordinator(columns: ["a", "b", "c"], rowCount: 5)
55+
let result = coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: 0)
56+
57+
#expect(result == false)
58+
}
59+
60+
@Test("Defers to row paste when every TSV line matches column count")
61+
func defersOnRowShapedTSV() {
62+
let stub = StubClipboard()
63+
stub.text = "x\ty\tz\nq\tw\te"
64+
stub.hasGridRowsValue = false
65+
ClipboardService.shared = stub
66+
67+
let coordinator = makeCoordinator(columns: ["a", "b", "c"], rowCount: 5)
68+
let result = coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: 0)
69+
70+
#expect(result == false)
71+
}
72+
73+
@Test("Cell pastes shape-mismatched TSV into focused range")
74+
func cellPastesShapeMismatchedTSV() {
75+
let stub = StubClipboard()
76+
stub.text = "x\ty"
77+
stub.hasGridRowsValue = false
78+
ClipboardService.shared = stub
79+
80+
let coordinator = makeCoordinator(columns: ["a", "b", "c", "d", "e"], rowCount: 5)
81+
let result = coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: 0)
82+
83+
#expect(result == true)
84+
}
85+
86+
@Test("Returns false when not editable")
87+
func refusesWhenReadOnly() {
88+
let stub = StubClipboard()
89+
stub.text = "x\ty"
90+
ClipboardService.shared = stub
91+
92+
let coordinator = TableViewCoordinator(
93+
changeManager: AnyChangeManager(DataChangeManager()),
94+
isEditable: false,
95+
selectedRowIndices: .constant([]),
96+
delegate: nil
97+
)
98+
99+
let result = coordinator.pasteCellsFromClipboard(anchorRow: 0, anchorColumn: 0)
100+
101+
#expect(result == false)
102+
}
103+
}

0 commit comments

Comments
 (0)