From 9f95836905b34d7f2a28cf1f8069b590d2c4793f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 22 Jul 2026 00:56:07 +0700 Subject: [PATCH 1/2] feat(inspector): insert rows and confirm data-bearing deletes in the CSV editor --- CHANGELOG.md | 1 + Plugins/CSVInspectorPlugin/CSVDocument.swift | 12 ++++ TablePro/TableProApp.swift | 17 ++++++ .../InspectorDeleteConfirmation.swift | 52 ++++++++++++++++ .../Inspector/InspectorRowMenuBuilder.swift | 31 ++++++++++ .../Inspector/InspectorViewController.swift | 59 ++++++++++++++++++- TablePro/Views/Results/DataGridRowView.swift | 8 +++ .../Views/Results/DataGridViewDelegate.swift | 2 + TableProTests/Plugins/CSVInspectorTests.swift | 33 +++++++++++ .../InspectorDeleteConfirmationTests.swift | 33 +++++++++++ .../Views/InspectorRowMenuBuilderTests.swift | 31 ++++++++++ docs/features/csv-inspector.mdx | 4 +- 12 files changed, 279 insertions(+), 4 deletions(-) create mode 100644 TablePro/Views/Inspector/InspectorDeleteConfirmation.swift create mode 100644 TablePro/Views/Inspector/InspectorRowMenuBuilder.swift create mode 100644 TableProTests/Views/InspectorDeleteConfirmationTests.swift create mode 100644 TableProTests/Views/InspectorRowMenuBuilderTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index cb076c6ad..952eec64e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Connect to PGlite through its socket server (`pglite-server`), with schema browsing, SQL editing, and row editing like PostgreSQL. Defaults to a loopback host with no TLS, and treats the connection as single-use to match PGlite's one-connection limit. (#1911) - In the CSV editor, right-click a column header to rename, insert, delete, or change the type of that column. (#1913) +- In the CSV editor, insert a row above or below any row from the row's right-click menu or the Edit menu. Deleting rows that contain data now asks you to confirm first. (#1469) - Add llama.cpp and MLX as local AI providers. Each preset points at the server's default local endpoint and needs no API key, alongside the existing Ollama and custom OpenAI-compatible options. (#1777) ### Fixed diff --git a/Plugins/CSVInspectorPlugin/CSVDocument.swift b/Plugins/CSVInspectorPlugin/CSVDocument.swift index 36ada45db..911765ffa 100644 --- a/Plugins/CSVInspectorPlugin/CSVDocument.swift +++ b/Plugins/CSVInspectorPlugin/CSVDocument.swift @@ -191,6 +191,7 @@ public final class CSVDocument: NSDocument, InspectorDocument { registerUndo { document in document.removeRow(at: index, suppressUndo: false) } + setUndoActionName(String(localized: "Add Row")) onChange?() } @@ -199,11 +200,13 @@ public final class CSVDocument: NSDocument, InspectorDocument { registerUndo { document in document.removeRow(at: index, suppressUndo: false) } + setUndoActionName(String(localized: "Insert Row")) onChange?() } public func removeRow(at index: Int) { removeRow(at: index, suppressUndo: false) + setUndoActionName(String(localized: "Delete Row")) } private func removeRow(at index: Int, suppressUndo: Bool) { @@ -230,6 +233,11 @@ public final class CSVDocument: NSDocument, InspectorDocument { registerUndo { document in document.reinsertRows(removed) } + setUndoActionName( + removed.count == 1 + ? String(localized: "Delete Row") + : String(localized: "Delete Rows") + ) onChange?() } @@ -318,6 +326,10 @@ public final class CSVDocument: NSDocument, InspectorDocument { undoManager?.registerUndo(withTarget: self, handler: action) } + private func setUndoActionName(_ name: String) { + undoManager?.setActionName(name) + } + private func shiftTypeOverrides(insertingAt index: Int) { typeOverrides = typeOverrides.reduce(into: [Int: InspectorColumnType]()) { result, entry in result[entry.key >= index ? entry.key + 1 : entry.key] = entry.value diff --git a/TablePro/TableProApp.swift b/TablePro/TableProApp.swift index 2b2cab867..3937172ae 100644 --- a/TablePro/TableProApp.swift +++ b/TablePro/TableProApp.swift @@ -605,6 +605,23 @@ struct AppMenuCommands: Commands { Divider() + Button(String(localized: "Insert Row Above")) { + NSApp.sendAction(#selector(InspectorViewController.inspectorInsertRowAbove(_:)), to: nil, from: nil) + } + .disabled(!keyWindowIsInspector) + + Button(String(localized: "Insert Row Below")) { + NSApp.sendAction(#selector(InspectorViewController.inspectorInsertRowBelow(_:)), to: nil, from: nil) + } + .disabled(!keyWindowIsInspector) + + Button(String(localized: "Delete Rows")) { + NSApp.sendAction(#selector(InspectorViewController.inspectorDeleteSelectedRows(_:)), to: nil, from: nil) + } + .disabled(!keyWindowIsInspector) + + Divider() + // Table operations (work when tables selected in sidebar) Button("Truncate Table") { actions?.truncateTables() diff --git a/TablePro/Views/Inspector/InspectorDeleteConfirmation.swift b/TablePro/Views/Inspector/InspectorDeleteConfirmation.swift new file mode 100644 index 000000000..4cb087c3f --- /dev/null +++ b/TablePro/Views/Inspector/InspectorDeleteConfirmation.swift @@ -0,0 +1,52 @@ +// +// InspectorDeleteConfirmation.swift +// TablePro +// + +import AppKit + +@MainActor +enum InspectorDeleteConfirmation { + static func rowsContainData(_ rowsCells: [[String]]) -> Bool { + rowsCells.contains { cells in cells.contains { !$0.isEmpty } } + } + + static func rowDeleteTitle(count: Int) -> String { + count == 1 + ? String(localized: "Delete this row?") + : String(format: String(localized: "Delete %lld rows?"), count) + } + + static func confirmDeleteRowsIfNeeded( + rowsCells: [[String]], + window: NSWindow?, + proceed: @escaping @MainActor () -> Void + ) { + guard rowsContainData(rowsCells) else { + proceed() + return + } + present(messageText: rowDeleteTitle(count: rowsCells.count), window: window, proceed: proceed) + } + + private static func present( + messageText: String, + window: NSWindow?, + proceed: @escaping @MainActor () -> Void + ) { + guard let window else { + proceed() + return + } + let alert = NSAlert() + alert.messageText = messageText + alert.alertStyle = .warning + let deleteButton = alert.addButton(withTitle: String(localized: "Delete")) + deleteButton.hasDestructiveAction = true + alert.addButton(withTitle: String(localized: "Cancel")) + alert.beginSheetModal(for: window) { response in + guard response == .alertFirstButtonReturn else { return } + proceed() + } + } +} diff --git a/TablePro/Views/Inspector/InspectorRowMenuBuilder.swift b/TablePro/Views/Inspector/InspectorRowMenuBuilder.swift new file mode 100644 index 000000000..5997525fa --- /dev/null +++ b/TablePro/Views/Inspector/InspectorRowMenuBuilder.swift @@ -0,0 +1,31 @@ +// +// InspectorRowMenuBuilder.swift +// TablePro +// + +import AppKit + +@MainActor +enum InspectorRowMenuBuilder { + static func structureItems(forRow displayRow: Int) -> [NSMenuItem] { + [ + actionItem( + title: String(localized: "Insert Row Above"), + action: #selector(InspectorViewController.inspectorInsertRowAbove(_:)), + row: displayRow + ), + actionItem( + title: String(localized: "Insert Row Below"), + action: #selector(InspectorViewController.inspectorInsertRowBelow(_:)), + row: displayRow + ), + ] + } + + private static func actionItem(title: String, action: Selector, row: Int) -> NSMenuItem { + let item = NSMenuItem(title: title, action: action, keyEquivalent: "") + item.tag = row + item.target = nil + return item + } +} diff --git a/TablePro/Views/Inspector/InspectorViewController.swift b/TablePro/Views/Inspector/InspectorViewController.swift index 25f4d9d97..b29dbab9b 100644 --- a/TablePro/Views/Inspector/InspectorViewController.swift +++ b/TablePro/Views/Inspector/InspectorViewController.swift @@ -162,8 +162,19 @@ final class InspectorViewController: NSViewController, NSUserInterfaceValidation return displayToStore[index] } guard !storeIndices.isEmpty else { return } - pendingPostRefresh = .selectClamped(displayRow: sortedDisplay.first ?? 0) - inspectorDocument.removeRows(at: IndexSet(storeIndices)) + let columnCount = inspectorDocument.columnNames.count + let rowsCells = storeIndices.map { storeRow in + (0.. Int { + let rowCount = inspectorDocument?.rowCount ?? 0 + let anchorDisplayRow: Int? = if let item = sender as? NSMenuItem { + item.tag + } else if below { + state.selectedRowIndices.max() + } else { + state.selectedRowIndices.min() + } + guard let displayRow = anchorDisplayRow, + displayRow >= 0, displayRow < displayToStore.count else { + return below ? rowCount : 0 + } + let storeRow = displayToStore[displayRow] + return below ? storeRow + 1 : storeRow + } + @objc func inspectorAddColumn(_ sender: Any?) { promptForColumnName(title: String(localized: "Add Column"), initial: "") { [weak self] name in guard let self, let name, !name.isEmpty else { return } @@ -278,6 +321,11 @@ final class InspectorViewController: NSViewController, NSUserInterfaceValidation ) } + fileprivate func rowStructureMenuItems(forRow displayRow: Int) -> [NSMenuItem] { + guard inspectorDocument != nil else { return [] } + return InspectorRowMenuBuilder.structureItems(forRow: displayRow) + } + private func renameLayoutKey(from oldName: String, to newName: String) { if state.columnLayout.columnOrder != nil { state.columnLayout.columnOrder = state.columnLayout.columnOrder?.map { $0 == oldName ? newName : $0 } @@ -350,7 +398,8 @@ final class InspectorViewController: NSViewController, NSUserInterfaceValidation case #selector(redo(_:)): return nsDocument?.undoManager?.canRedo ?? false case #selector(saveDocument(_:)), #selector(saveDocumentAs(_:)), - #selector(toggleInspectorFilter(_:)), #selector(inspectorAddRow(_:)): + #selector(toggleInspectorFilter(_:)), #selector(inspectorAddRow(_:)), + #selector(inspectorInsertRowAbove(_:)), #selector(inspectorInsertRowBelow(_:)): return nsDocument != nil case #selector(inspectorDeleteSelectedRows(_:)): return !state.selectedRowIndices.isEmpty @@ -701,6 +750,10 @@ private final class InspectorGridDelegate: DataGridViewDelegate { owner?.columnStructureMenuItems(forColumn: dataColumnIndex) ?? [] } + func dataGridRowStructureMenuItems(forRow displayRow: Int) -> [NSMenuItem] { + owner?.rowStructureMenuItems(forRow: displayRow) ?? [] + } + func dataGridUndo() { owner?.handleUndo() } diff --git a/TablePro/Views/Results/DataGridRowView.swift b/TablePro/Views/Results/DataGridRowView.swift index d0e0f5e81..5a0b0acfc 100644 --- a/TablePro/Views/Results/DataGridRowView.swift +++ b/TablePro/Views/Results/DataGridRowView.swift @@ -320,6 +320,14 @@ class DataGridRowView: NSTableRowView { } if coordinator.isEditable { + let rowStructureItems = coordinator.delegate?.dataGridRowStructureMenuItems(forRow: rowIndex) ?? [] + if !rowStructureItems.isEmpty { + menu.addItem(NSMenuItem.separator()) + for item in rowStructureItems { + menu.addItem(item) + } + } + let duplicateItem = NSMenuItem( title: String(localized: "Duplicate"), action: #selector(duplicateRow), keyEquivalent: "") duplicateItem.target = self diff --git a/TablePro/Views/Results/DataGridViewDelegate.swift b/TablePro/Views/Results/DataGridViewDelegate.swift index 5be7bf038..2bfc8ffaa 100644 --- a/TablePro/Views/Results/DataGridViewDelegate.swift +++ b/TablePro/Views/Results/DataGridViewDelegate.swift @@ -28,6 +28,7 @@ protocol DataGridViewDelegate: AnyObject { func dataGridHideColumn(_ columnName: String) func dataGridShowAllColumns() func dataGridColumnStructureMenuItems(forColumn dataColumnIndex: Int) -> [NSMenuItem] + func dataGridRowStructureMenuItems(forRow displayRow: Int) -> [NSMenuItem] func dataGridVisualState(forRow row: Int) -> RowVisualState? func dataGridRowView(for tableView: NSTableView, row: Int, coordinator: TableViewCoordinator) -> NSTableRowView? func dataGridEmptySpaceMenu() -> NSMenu? @@ -57,6 +58,7 @@ extension DataGridViewDelegate { func dataGridHideColumn(_ columnName: String) {} func dataGridShowAllColumns() {} func dataGridColumnStructureMenuItems(forColumn dataColumnIndex: Int) -> [NSMenuItem] { [] } + func dataGridRowStructureMenuItems(forRow displayRow: Int) -> [NSMenuItem] { [] } func dataGridVisualState(forRow row: Int) -> RowVisualState? { nil } func dataGridRowView(for tableView: NSTableView, row: Int, coordinator: TableViewCoordinator) -> NSTableRowView? { nil } func dataGridEmptySpaceMenu() -> NSMenu? { nil } diff --git a/TableProTests/Plugins/CSVInspectorTests.swift b/TableProTests/Plugins/CSVInspectorTests.swift index 934a23b1d..0b282c647 100644 --- a/TableProTests/Plugins/CSVInspectorTests.swift +++ b/TableProTests/Plugins/CSVInspectorTests.swift @@ -265,6 +265,39 @@ struct CSVRowStoreTests { #expect(store.cells(forRow: 4) == ["5", "5"]) } + @Test("insertRow places a row at the given index and shifts the rest down") + func insertRowAtIndex() { + let store = makeStore("a,b\n1,1\n3,3\n") + store.insertRow(["2", "2"], at: 1) + #expect(store.rowCount == 3) + #expect(store.cells(forRow: 0) == ["1", "1"]) + #expect(store.cells(forRow: 1) == ["2", "2"]) + #expect(store.cells(forRow: 2) == ["3", "3"]) + } + + @Test("insertRow at 0 inserts above the first row") + func insertRowAtTop() { + let store = makeStore("a,b\n1,1\n2,2\n") + store.insertRow(["0", "0"], at: 0) + #expect(store.cells(forRow: 0) == ["0", "0"]) + #expect(store.cells(forRow: 1) == ["1", "1"]) + } + + @Test("insertRow past the end clamps to an append") + func insertRowClampsToEnd() { + let store = makeStore("a,b\n1,1\n") + store.insertRow(["9", "9"], at: 99) + #expect(store.rowCount == 2) + #expect(store.cells(forRow: 1) == ["9", "9"]) + } + + @Test("insertRow pads a short row to the column count") + func insertRowPadsShortRow() { + let store = makeStore("a,b,c\n1,2,3\n") + store.insertRow(["x"], at: 0) + #expect(store.cells(forRow: 0) == ["x", "", ""]) + } + @Test("renameColumn updates columnNames in place") func renameColumnInPlace() { let store = makeStore("a,b,c\n1,2,3\n") diff --git a/TableProTests/Views/InspectorDeleteConfirmationTests.swift b/TableProTests/Views/InspectorDeleteConfirmationTests.swift new file mode 100644 index 000000000..5cd3f7298 --- /dev/null +++ b/TableProTests/Views/InspectorDeleteConfirmationTests.swift @@ -0,0 +1,33 @@ +// +// InspectorDeleteConfirmationTests.swift +// TableProTests +// + +import AppKit +@testable import TablePro +import Testing + +@MainActor +@Suite("InspectorDeleteConfirmation") +struct InspectorDeleteConfirmationTests { + @Test("An all-empty selection contains no data") + func emptyRowsHaveNoData() { + #expect(InspectorDeleteConfirmation.rowsContainData([["", ""], ["", ""]]) == false) + } + + @Test("An empty selection contains no data") + func noRowsHaveNoData() { + #expect(InspectorDeleteConfirmation.rowsContainData([]) == false) + } + + @Test("A selection with any non-empty cell contains data") + func nonEmptyRowsHaveData() { + #expect(InspectorDeleteConfirmation.rowsContainData([["", ""], ["", "x"]]) == true) + } + + @Test("Delete title reflects the row count") + func rowDeleteTitles() { + #expect(InspectorDeleteConfirmation.rowDeleteTitle(count: 1) == "Delete this row?") + #expect(InspectorDeleteConfirmation.rowDeleteTitle(count: 3).contains("3")) + } +} diff --git a/TableProTests/Views/InspectorRowMenuBuilderTests.swift b/TableProTests/Views/InspectorRowMenuBuilderTests.swift new file mode 100644 index 000000000..537ee4e7d --- /dev/null +++ b/TableProTests/Views/InspectorRowMenuBuilderTests.swift @@ -0,0 +1,31 @@ +// +// InspectorRowMenuBuilderTests.swift +// TableProTests +// + +import AppKit +@testable import TablePro +import Testing + +@MainActor +@Suite("InspectorRowMenuBuilder") +struct InspectorRowMenuBuilderTests { + @Test("Structure items are Insert Row Above then Insert Row Below") + func structureItemsOrder() { + let items = InspectorRowMenuBuilder.structureItems(forRow: 4) + + #expect(items.count == 2) + #expect(items[0].action == #selector(InspectorViewController.inspectorInsertRowAbove(_:))) + #expect(items[1].action == #selector(InspectorViewController.inspectorInsertRowBelow(_:))) + } + + @Test("Each item carries the clicked row and no explicit target") + func structureItemsWiring() { + let items = InspectorRowMenuBuilder.structureItems(forRow: 7) + + for item in items { + #expect(item.tag == 7) + #expect(item.target == nil) + } + } +} diff --git a/docs/features/csv-inspector.mdx b/docs/features/csv-inspector.mdx index a54c82403..aaeda09a9 100644 --- a/docs/features/csv-inspector.mdx +++ b/docs/features/csv-inspector.mdx @@ -34,8 +34,10 @@ Rows load in pages. The page size comes from the Default page size setting in [S - Double-click a cell (or press Return on it) to edit. Return commits and closes the editor. Tab commits and moves to the next cell, wrapping to the next row. - The toolbar `Add Row` button appends a new row, scrolls to it, and selects it. +- Right-click a row and choose **Insert Row Above** or **Insert Row Below** to add a blank row next to it. Both also appear in the Edit menu, where they act on the selected row. - Select rows and press `Delete` (the toolbar `Delete` button, or right-click a row and choose Delete) to remove them. The next row takes selection so arrow keys keep working from where you were. -- Cmd+Z and Cmd+Shift+Z undo and redo every change. A bulk delete or a paste is a single undo step. +- Deleting rows that hold data asks you to confirm first. Deleting blank rows skips the prompt. +- Cmd+Z and Cmd+Shift+Z undo and redo every change. Each insert, a bulk delete, or a paste is a single undo step. ## Column operations From f1f8e5a5ff2af53bfb5f3a4fc1dc688500f7936a Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 22 Jul 2026 10:21:21 +0700 Subject: [PATCH 2/2] fix(inspector): name CSV row undo/redo consistently and preserve the Paste label --- Plugins/CSVInspectorPlugin/CSVDocument.swift | 37 +++++++------ TablePro/TableProApp.swift | 6 +-- .../Inspector/InspectorRowInsertion.swift | 17 ++++++ .../Inspector/InspectorViewController.swift | 15 +++--- .../PluginTestSources/CSVDocument.swift | 1 + .../Plugins/CSVDocumentUndoTests.swift | 53 +++++++++++++++++++ .../Views/InspectorRowInsertionTests.swift | 44 +++++++++++++++ docs/features/csv-inspector.mdx | 2 +- 8 files changed, 146 insertions(+), 29 deletions(-) create mode 100644 TablePro/Views/Inspector/InspectorRowInsertion.swift create mode 120000 TableProTests/PluginTestSources/CSVDocument.swift create mode 100644 TableProTests/Plugins/CSVDocumentUndoTests.swift create mode 100644 TableProTests/Views/InspectorRowInsertionTests.swift diff --git a/Plugins/CSVInspectorPlugin/CSVDocument.swift b/Plugins/CSVInspectorPlugin/CSVDocument.swift index 911765ffa..71849eb20 100644 --- a/Plugins/CSVInspectorPlugin/CSVDocument.swift +++ b/Plugins/CSVInspectorPlugin/CSVDocument.swift @@ -187,61 +187,63 @@ public final class CSVDocument: NSDocument, InspectorDocument { } public func appendRow() { + let name = String(localized: "Add Row") let index = store.appendRow(values: []) registerUndo { document in - document.removeRow(at: index, suppressUndo: false) + document.removeRow(at: index, suppressUndo: false, actionName: name) } - setUndoActionName(String(localized: "Add Row")) + setUndoActionName(name) onChange?() } public func insertRow(at index: Int) { + let name = String(localized: "Insert Row") store.insertRow([], at: index) registerUndo { document in - document.removeRow(at: index, suppressUndo: false) + document.removeRow(at: index, suppressUndo: false, actionName: name) } - setUndoActionName(String(localized: "Insert Row")) + setUndoActionName(name) onChange?() } public func removeRow(at index: Int) { - removeRow(at: index, suppressUndo: false) - setUndoActionName(String(localized: "Delete Row")) + removeRow(at: index, suppressUndo: false, actionName: String(localized: "Delete Row")) } - private func removeRow(at index: Int, suppressUndo: Bool) { + private func removeRow(at index: Int, suppressUndo: Bool, actionName: String) { guard let removed = store.removeRow(at: index) else { return } if !suppressUndo { registerUndo { document in - document.reinsertRow(removed, at: index) + document.reinsertRow(removed, at: index, actionName: actionName) } + setUndoActionName(actionName) } onChange?() } - private func reinsertRow(_ values: [String], at index: Int) { + private func reinsertRow(_ values: [String], at index: Int, actionName: String) { store.insertRow(values, at: index) registerUndo { document in - document.removeRow(at: index, suppressUndo: false) + document.removeRow(at: index, suppressUndo: false, actionName: actionName) } + setUndoActionName(actionName) onChange?() } public func removeRows(at indices: IndexSet) { let removed = store.removeRows(at: indices) guard !removed.isEmpty else { return } + let name = removed.count == 1 + ? String(localized: "Delete Row") + : String(localized: "Delete Rows") registerUndo { document in - document.reinsertRows(removed) + document.reinsertRows(removed, actionName: name) } - setUndoActionName( - removed.count == 1 - ? String(localized: "Delete Row") - : String(localized: "Delete Rows") - ) + setUndoActionName(name) onChange?() } - private func reinsertRows(_ rows: [(index: Int, cells: [String])]) { + private func reinsertRows(_ rows: [(index: Int, cells: [String])], actionName: String) { for entry in rows.sorted(by: { $0.index < $1.index }) { store.insertRow(entry.cells, at: entry.index) } @@ -249,6 +251,7 @@ public final class CSVDocument: NSDocument, InspectorDocument { registerUndo { document in document.removeRows(at: originalIndices) } + setUndoActionName(actionName) onChange?() } diff --git a/TablePro/TableProApp.swift b/TablePro/TableProApp.swift index 3937172ae..50694ac72 100644 --- a/TablePro/TableProApp.swift +++ b/TablePro/TableProApp.swift @@ -605,17 +605,17 @@ struct AppMenuCommands: Commands { Divider() - Button(String(localized: "Insert Row Above")) { + Button("Insert Row Above") { NSApp.sendAction(#selector(InspectorViewController.inspectorInsertRowAbove(_:)), to: nil, from: nil) } .disabled(!keyWindowIsInspector) - Button(String(localized: "Insert Row Below")) { + Button("Insert Row Below") { NSApp.sendAction(#selector(InspectorViewController.inspectorInsertRowBelow(_:)), to: nil, from: nil) } .disabled(!keyWindowIsInspector) - Button(String(localized: "Delete Rows")) { + Button("Delete Rows") { NSApp.sendAction(#selector(InspectorViewController.inspectorDeleteSelectedRows(_:)), to: nil, from: nil) } .disabled(!keyWindowIsInspector) diff --git a/TablePro/Views/Inspector/InspectorRowInsertion.swift b/TablePro/Views/Inspector/InspectorRowInsertion.swift new file mode 100644 index 000000000..cb7bc9077 --- /dev/null +++ b/TablePro/Views/Inspector/InspectorRowInsertion.swift @@ -0,0 +1,17 @@ +// +// InspectorRowInsertion.swift +// TablePro +// + +import Foundation + +enum InspectorRowInsertion { + static func storeIndex(anchorDisplayRow: Int?, below: Bool, displayToStore: [Int], rowCount: Int) -> Int { + guard let displayRow = anchorDisplayRow, + displayRow >= 0, displayRow < displayToStore.count else { + return below ? rowCount : 0 + } + let storeRow = displayToStore[displayRow] + return below ? storeRow + 1 : storeRow + } +} diff --git a/TablePro/Views/Inspector/InspectorViewController.swift b/TablePro/Views/Inspector/InspectorViewController.swift index b29dbab9b..776956232 100644 --- a/TablePro/Views/Inspector/InspectorViewController.swift +++ b/TablePro/Views/Inspector/InspectorViewController.swift @@ -143,7 +143,6 @@ final class InspectorViewController: NSViewController, NSUserInterfaceValidation let undoManager = nsDocument?.undoManager undoManager?.beginUndoGrouping() - undoManager?.setActionName(String(localized: "Paste")) for row in rows { let newRowIndex = inspectorDocument.rowCount inspectorDocument.appendRow() @@ -151,6 +150,7 @@ final class InspectorViewController: NSViewController, NSUserInterfaceValidation inspectorDocument.setCell(row: newRowIndex, column: column, to: value) } } + undoManager?.setActionName(String(localized: "Paste")) undoManager?.endUndoGrouping() } @@ -233,7 +233,6 @@ final class InspectorViewController: NSViewController, NSUserInterfaceValidation } private func insertStoreIndex(anchoredBy sender: Any?, below: Bool) -> Int { - let rowCount = inspectorDocument?.rowCount ?? 0 let anchorDisplayRow: Int? = if let item = sender as? NSMenuItem { item.tag } else if below { @@ -241,12 +240,12 @@ final class InspectorViewController: NSViewController, NSUserInterfaceValidation } else { state.selectedRowIndices.min() } - guard let displayRow = anchorDisplayRow, - displayRow >= 0, displayRow < displayToStore.count else { - return below ? rowCount : 0 - } - let storeRow = displayToStore[displayRow] - return below ? storeRow + 1 : storeRow + return InspectorRowInsertion.storeIndex( + anchorDisplayRow: anchorDisplayRow, + below: below, + displayToStore: displayToStore, + rowCount: inspectorDocument?.rowCount ?? 0 + ) } @objc func inspectorAddColumn(_ sender: Any?) { diff --git a/TableProTests/PluginTestSources/CSVDocument.swift b/TableProTests/PluginTestSources/CSVDocument.swift new file mode 120000 index 000000000..d3da4ca58 --- /dev/null +++ b/TableProTests/PluginTestSources/CSVDocument.swift @@ -0,0 +1 @@ +../../Plugins/CSVInspectorPlugin/CSVDocument.swift \ No newline at end of file diff --git a/TableProTests/Plugins/CSVDocumentUndoTests.swift b/TableProTests/Plugins/CSVDocumentUndoTests.swift new file mode 100644 index 000000000..bfb5e7367 --- /dev/null +++ b/TableProTests/Plugins/CSVDocumentUndoTests.swift @@ -0,0 +1,53 @@ +// +// CSVDocumentUndoTests.swift +// TableProTests +// +// Tests CSVDocument (compiled via symlink from Plugins/CSVInspectorPlugin/). +// + +import AppKit +import Foundation +import TableProPluginKit +import Testing + +@MainActor +@Suite("CSVDocument undo naming") +struct CSVDocumentUndoTests { + private func makeDocument(_ contents: String) throws -> CSVDocument { + let url = FileManager.default.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).csv") + try contents.data(using: .utf8)!.write(to: url) + let document = CSVDocument() + try document.read(from: url, ofType: "public.comma-separated-values-text") + try? FileManager.default.removeItem(at: url) + return document + } + + @Test("Insert Row keeps its name through undo and redo") + func insertRowNaming() throws { + let document = try makeDocument("a,b\n1,2\n") + document.insertRow(at: 0) + #expect(document.undoManager?.undoActionName == "Insert Row") + document.undoManager?.undo() + #expect(document.undoManager?.redoActionName == "Insert Row") + document.undoManager?.redo() + #expect(document.undoManager?.undoActionName == "Insert Row") + } + + @Test("Delete Rows keeps its name through undo") + func deleteRowsNaming() throws { + let document = try makeDocument("a,b\n1,2\n3,4\n5,6\n") + document.removeRows(at: IndexSet([0, 1])) + #expect(document.undoManager?.undoActionName == "Delete Rows") + document.undoManager?.undo() + #expect(document.undoManager?.redoActionName == "Delete Rows") + } + + @Test("Add Row keeps its name through undo and redo") + func addRowNaming() throws { + let document = try makeDocument("a,b\n1,2\n") + document.appendRow() + #expect(document.undoManager?.undoActionName == "Add Row") + document.undoManager?.undo() + #expect(document.undoManager?.redoActionName == "Add Row") + } +} diff --git a/TableProTests/Views/InspectorRowInsertionTests.swift b/TableProTests/Views/InspectorRowInsertionTests.swift new file mode 100644 index 000000000..42bd7c2ad --- /dev/null +++ b/TableProTests/Views/InspectorRowInsertionTests.swift @@ -0,0 +1,44 @@ +// +// InspectorRowInsertionTests.swift +// TableProTests +// + +@testable import TablePro +import Testing + +@Suite("InspectorRowInsertion") +struct InspectorRowInsertionTests { + @Test("A clicked row maps through displayToStore for above and below") + func mapsClickedRow() { + let displayToStore = [10, 11, 12] + #expect(InspectorRowInsertion.storeIndex( + anchorDisplayRow: 1, below: false, displayToStore: displayToStore, rowCount: 3) == 11) + #expect(InspectorRowInsertion.storeIndex( + anchorDisplayRow: 1, below: true, displayToStore: displayToStore, rowCount: 3) == 12) + } + + @Test("A non-identity displayToStore (filtered or sorted view) resolves the store row") + func nonIdentityMapping() { + let displayToStore = [4, 0, 9] + #expect(InspectorRowInsertion.storeIndex( + anchorDisplayRow: 0, below: false, displayToStore: displayToStore, rowCount: 10) == 4) + #expect(InspectorRowInsertion.storeIndex( + anchorDisplayRow: 2, below: true, displayToStore: displayToStore, rowCount: 10) == 10) + } + + @Test("No anchor inserts at the top for above and appends for below") + func emptySelectionFallback() { + #expect(InspectorRowInsertion.storeIndex( + anchorDisplayRow: nil, below: false, displayToStore: [0, 1], rowCount: 2) == 0) + #expect(InspectorRowInsertion.storeIndex( + anchorDisplayRow: nil, below: true, displayToStore: [0, 1], rowCount: 2) == 2) + } + + @Test("An out-of-range anchor falls back to top or append") + func outOfRangeFallback() { + #expect(InspectorRowInsertion.storeIndex( + anchorDisplayRow: 5, below: false, displayToStore: [0, 1], rowCount: 2) == 0) + #expect(InspectorRowInsertion.storeIndex( + anchorDisplayRow: -1, below: true, displayToStore: [0, 1], rowCount: 2) == 2) + } +} diff --git a/docs/features/csv-inspector.mdx b/docs/features/csv-inspector.mdx index aaeda09a9..84bba5395 100644 --- a/docs/features/csv-inspector.mdx +++ b/docs/features/csv-inspector.mdx @@ -34,7 +34,7 @@ Rows load in pages. The page size comes from the Default page size setting in [S - Double-click a cell (or press Return on it) to edit. Return commits and closes the editor. Tab commits and moves to the next cell, wrapping to the next row. - The toolbar `Add Row` button appends a new row, scrolls to it, and selects it. -- Right-click a row and choose **Insert Row Above** or **Insert Row Below** to add a blank row next to it. Both also appear in the Edit menu, where they act on the selected row. +- Right-click a row and choose **Insert Row Above** or **Insert Row Below** to add a blank row next to it. Both also appear in the Edit menu, where they insert next to the selected row, or at the top or bottom when nothing is selected. - Select rows and press `Delete` (the toolbar `Delete` button, or right-click a row and choose Delete) to remove them. The next row takes selection so arrow keys keep working from where you were. - Deleting rows that hold data asks you to confirm first. Deleting blank rows skips the prompt. - Cmd+Z and Cmd+Shift+Z undo and redo every change. Each insert, a bulk delete, or a paste is a single undo step.