From ec9ee9495b418d74f729854c4f813d1dade55f69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Nam=20Long?= <26674308+J2TeamNNL@users.noreply.github.com> Date: Sun, 1 Nov 2026 03:15:24 +0700 Subject: [PATCH 1/6] feat(connections): shortcuts and strip menu for dashboard and insights Shift-Option-Command-D opens Server Dashboard and Shift-Option-Command-I opens Query Insights, listed under Connections in Settings > Keyboard. Both commands are also in a live connection's context menu on the connection strip. --- .../Core/Menu/DatabaseMenuBuilder.swift | 8 +- .../Infrastructure/ConnectionMenuPolicy.swift | 8 ++ .../WorkspaceRailViewController.swift | 59 ++++++++++++++- .../Models/UI/KeyboardShortcutModels.swift | 9 ++- .../Core/Menu/ChromeSnapshot.json | 4 +- .../Core/Menu/ServerToolsCommandTests.swift | 73 +++++++++++++++++++ .../Services/ConnectionMenuPolicyTests.swift | 15 ++++ .../MenuActionSelectorCollisionTests.swift | 4 +- 8 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 apple/TableProTests/Core/Menu/ServerToolsCommandTests.swift diff --git a/apple/TablePro/Core/Menu/DatabaseMenuBuilder.swift b/apple/TablePro/Core/Menu/DatabaseMenuBuilder.swift index 44c9fe5afa..e76e29c4fd 100644 --- a/apple/TablePro/Core/Menu/DatabaseMenuBuilder.swift +++ b/apple/TablePro/Core/Menu/DatabaseMenuBuilder.swift @@ -133,7 +133,9 @@ enum DatabaseMenuBuilder { ), MenuItemFactory.item( String(localized: "Server Dashboard"), - action: #selector(MainSplitViewController.showServerDashboard(_:)) + action: #selector(MainSplitViewController.showServerDashboard(_:)), + shortcut: .showServerDashboard, + keyboard: keyboard ), MenuItemFactory.item( String(localized: "Users & Roles"), @@ -141,7 +143,9 @@ enum DatabaseMenuBuilder { ), MenuItemFactory.item( String(localized: "Query Insights"), - action: #selector(MainSplitViewController.showQueryInsights(_:)) + action: #selector(MainSplitViewController.showQueryInsights(_:)), + shortcut: .showQueryInsights, + keyboard: keyboard ), MenuItemFactory.separator, /// No ellipsis: the HIG reserves one for an action that needs more information before diff --git a/apple/TablePro/Core/Services/Infrastructure/ConnectionMenuPolicy.swift b/apple/TablePro/Core/Services/Infrastructure/ConnectionMenuPolicy.swift index 45fd8b3f4f..5162184912 100644 --- a/apple/TablePro/Core/Services/Infrastructure/ConnectionMenuPolicy.swift +++ b/apple/TablePro/Core/Services/Infrastructure/ConnectionMenuPolicy.swift @@ -12,4 +12,12 @@ internal enum ConnectionMenuPolicy { internal static func showsDisconnect(status: ConnectionStatus) -> Bool { status.isConnected } + + internal static func showsServerDashboard(status: ConnectionStatus, type: DatabaseType) -> Bool { + status.isConnected && ServerDashboardQueryProviderFactory.supportsDashboard(for: type) + } + + internal static func showsQueryInsights(status: ConnectionStatus) -> Bool { + status.isConnected + } } diff --git a/apple/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift b/apple/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift index 681ef0ce62..865f991eb1 100644 --- a/apple/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift +++ b/apple/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift @@ -618,6 +618,30 @@ internal final class WorkspaceRailViewController: NSViewController { WindowManager.shared.moveToNewWindow(connectionId: workspace.connectionId) } + /// Named for the rail, not `showServerDashboard(_:)`: that is the menu bar's nil-target action, + /// and the rail sits ahead of `MainSplitViewController` in the responder chain. + @objc + private func showWorkspaceServerDashboard(_ sender: NSMenuItem) { + guard let workspace = sender.representedObject as? WorkspaceID else { return } + showTool(of: workspace) { $0.showServerDashboard() } + } + + @objc + private func showWorkspaceQueryInsights(_ sender: NSMenuItem) { + guard let workspace = sender.representedObject as? WorkspaceID else { return } + showTool(of: workspace) { $0.showQueryInsights() } + } + + /// Switches to the clicked row first, so the tool opens beside the work it belongs to. + private func showTool(of workspace: WorkspaceID, _ open: (MainContentCoordinator) -> Void) { + if let row = entries.firstIndex(where: { $0.workspace == workspace }) { + tableView.selectRowIndexes(IndexSet(integer: row), byExtendingSelection: false) + commit(row: row) + } + guard let coordinator = WindowManager.shared.coordinator(for: workspace.connectionId) else { return } + open(coordinator) + } + @objc private func closeWorkspace(_ sender: NSMenuItem) { guard let workspace = sender.representedObject as? WorkspaceID else { return } @@ -746,6 +770,8 @@ extension WorkspaceRailViewController: NSMenuDelegate { menu.addItem(.separator()) } + addToolItems(to: menu, for: entry) + if let releaseTitle = ConnectionFileLockAction.commandTitle( connectionId: entry.workspace.connectionId ) { @@ -784,11 +810,42 @@ extension WorkspaceRailViewController: NSMenuDelegate { ) } - private func addItem(to menu: NSMenu, title: String, action: Selector, workspace: WorkspaceID) { + /// Spelled as the Database menu spells them, and showing the same shortcut it does. + private func addToolItems(to menu: NSMenu, for entry: WorkspaceRailEntry) { + let keyboard = AppSettingsManager.shared.keyboard + var added = false + if ConnectionMenuPolicy.showsServerDashboard(status: entry.status, type: entry.connection.type) { + let item = addItem( + to: menu, + title: String(localized: "Server Dashboard"), + action: #selector(showWorkspaceServerDashboard(_:)), + workspace: entry.workspace + ) + MenuItemFactory.apply(shortcut: .showServerDashboard, keyboard: keyboard, to: item) + added = true + } + if ConnectionMenuPolicy.showsQueryInsights(status: entry.status) { + let item = addItem( + to: menu, + title: String(localized: "Query Insights"), + action: #selector(showWorkspaceQueryInsights(_:)), + workspace: entry.workspace + ) + MenuItemFactory.apply(shortcut: .showQueryInsights, keyboard: keyboard, to: item) + added = true + } + if added { + menu.addItem(.separator()) + } + } + + @discardableResult + private func addItem(to menu: NSMenu, title: String, action: Selector, workspace: WorkspaceID) -> NSMenuItem { let item = NSMenuItem(title: title, action: action, keyEquivalent: "") item.target = self item.representedObject = workspace menu.addItem(item) + return item } } diff --git a/apple/TablePro/Models/UI/KeyboardShortcutModels.swift b/apple/TablePro/Models/UI/KeyboardShortcutModels.swift index 79b0e75513..3d8544ddcd 100644 --- a/apple/TablePro/Models/UI/KeyboardShortcutModels.swift +++ b/apple/TablePro/Models/UI/KeyboardShortcutModels.swift @@ -67,6 +67,8 @@ enum ShortcutAction: String, Codable, CaseIterable, Identifiable { case openDatabase case switchConnection case moveTabToConnection + case showServerDashboard + case showQueryInsights // Editor & Query case openFile @@ -194,7 +196,8 @@ enum ShortcutAction: String, Codable, CaseIterable, Identifiable { var category: ShortcutCategory { switch self { - case .manageConnections, .newConnection, .openDatabase, .switchConnection, .moveTabToConnection: + case .manageConnections, .newConnection, .openDatabase, .switchConnection, .moveTabToConnection, + .showServerDashboard, .showQueryInsights: return .connections case .openFile, .saveChanges, .saveAs, .copyPath, .copyRelativePath, .executeQuery, .executeAllStatements, .executeQueryWithoutLimit, .cancelQuery, .explainQuery, .formatQuery, @@ -302,6 +305,8 @@ enum ShortcutAction: String, Codable, CaseIterable, Identifiable { case .openFile: return String(localized: "Open File") case .switchConnection: return String(localized: "Switch Connection") case .moveTabToConnection: return String(localized: "Move Tab to Connection") + case .showServerDashboard: return String(localized: "Server Dashboard") + case .showQueryInsights: return String(localized: "Query Insights") case .saveChanges: return String(localized: "Save Changes") case .saveAs: return String(localized: "Save As") case .copyPath: return String(localized: "Copy Path") @@ -662,6 +667,8 @@ struct KeyboardSettings: Codable, Equatable { .openDatabase: .character("k", command: true), .switchConnection: .character("c", command: true, control: true), .moveTabToConnection: .character("c", command: true, option: true, control: true), + .showServerDashboard: .character("d", command: true, shift: true, option: true), + .showQueryInsights: .character("i", command: true, shift: true, option: true), // Editor & Query .openFile: .character("o", command: true), diff --git a/apple/TableProTests/Core/Menu/ChromeSnapshot.json b/apple/TableProTests/Core/Menu/ChromeSnapshot.json index 538e3869b2..23e6ab856e 100644 --- a/apple/TableProTests/Core/Menu/ChromeSnapshot.json +++ b/apple/TableProTests/Core/Menu/ChromeSnapshot.json @@ -220,9 +220,9 @@ {"path": "Database", "separator": true}, {"path": "Database > View ER Diagram", "action": "showERDiagram:"}, {"path": "Database > Build Join Query…", "action": "buildJoinQuery:"}, - {"path": "Database > Server Dashboard", "action": "showServerDashboard:"}, + {"path": "Database > Server Dashboard", "identifier": "shortcut.showServerDashboard", "action": "showServerDashboard:", "key": "d", "modifiers": ["option", "shift", "command"]}, {"path": "Database > Users & Roles", "action": "showUsersAndRoles:"}, - {"path": "Database > Query Insights", "action": "showQueryInsights:"}, + {"path": "Database > Query Insights", "identifier": "shortcut.showQueryInsights", "action": "showQueryInsights:", "key": "i", "modifiers": ["option", "shift", "command"]}, {"path": "Database", "separator": true}, {"path": "Database > Release File Lock", "action": "releaseFileLock:"}, {"path": "Database > Disconnect", "action": "requestDisconnect"}, diff --git a/apple/TableProTests/Core/Menu/ServerToolsCommandTests.swift b/apple/TableProTests/Core/Menu/ServerToolsCommandTests.swift new file mode 100644 index 0000000000..cf17ac7a35 --- /dev/null +++ b/apple/TableProTests/Core/Menu/ServerToolsCommandTests.swift @@ -0,0 +1,73 @@ +// +// ServerToolsCommandTests.swift +// TableProTests +// + +import AppKit +@testable import TablePro +import Testing + +struct ServerToolsShortcutTests { + @Test( + "Each chord belongs to its command alone and shadows no reserved command", + arguments: [ + (ShortcutAction.showServerDashboard, BoundKey.character("d", command: true, shift: true, option: true)), + (ShortcutAction.showQueryInsights, BoundKey.character("i", command: true, shift: true, option: true)) + ] + ) + func chordIsUnique(action: ShortcutAction, chord: BoundKey) { + #expect(KeyboardSettings.defaultShortcuts[action] == chord) + let owners = KeyboardSettings.defaultShortcuts.filter { $0.value == chord }.map(\.key) + #expect(owners == [action]) + #expect(ShortcutAction.reservedConflict(for: chord, context: action.context) == nil) + } + + @Test("Both are listed under Connections by the names the Database menu uses") + func listedUnderConnections() { + #expect(ShortcutAction.showServerDashboard.category == .connections) + #expect(ShortcutAction.showQueryInsights.category == .connections) + #expect(ShortcutAction.showServerDashboard.context == .global) + #expect(ShortcutAction.showQueryInsights.context == .global) + #expect(ShortcutAction.showServerDashboard.displayName == String(localized: "Server Dashboard")) + #expect(ShortcutAction.showQueryInsights.displayName == String(localized: "Query Insights")) + } +} + +@MainActor +struct ServerToolsMenuTests { + private func databaseItem(_ title: String) -> NSMenuItem? { + MainMenuBuilder.build(keyboard: KeyboardSettings()).bar + .items.first { $0.title == String(localized: "Database") }?.submenu? + .items.first { $0.title == title } + } + + @Test("Database > Server Dashboard carries its shortcut") + func serverDashboardItem() throws { + let item = try #require(databaseItem(String(localized: "Server Dashboard"))) + #expect(item.action == #selector(MainSplitViewController.showServerDashboard(_:))) + #expect(item.identifier == MenuItemFactory.identifier(for: .showServerDashboard)) + #expect(item.keyEquivalent == "d") + #expect(item.keyEquivalentModifierMask == [.command, .shift, .option]) + } + + @Test("Database > Query Insights carries its shortcut") + func queryInsightsItem() throws { + let item = try #require(databaseItem(String(localized: "Query Insights"))) + #expect(item.action == #selector(MainSplitViewController.showQueryInsights(_:))) + #expect(item.identifier == MenuItemFactory.identifier(for: .showQueryInsights)) + #expect(item.keyEquivalent == "i") + #expect(item.keyEquivalentModifierMask == [.command, .shift, .option]) + } + + @Test("A cleared binding leaves the item without a key equivalent") + func clearedBindingLeavesNoKey() throws { + var keyboard = KeyboardSettings() + keyboard.clearShortcut(for: .showServerDashboard) + let item = try #require( + MainMenuBuilder.build(keyboard: keyboard).bar + .items.first { $0.title == String(localized: "Database") }?.submenu? + .items.first { $0.title == String(localized: "Server Dashboard") } + ) + #expect(item.keyEquivalent.isEmpty) + } +} diff --git a/apple/TableProTests/Core/Services/ConnectionMenuPolicyTests.swift b/apple/TableProTests/Core/Services/ConnectionMenuPolicyTests.swift index cb86d88d66..c2da6ea2c0 100644 --- a/apple/TableProTests/Core/Services/ConnectionMenuPolicyTests.swift +++ b/apple/TableProTests/Core/Services/ConnectionMenuPolicyTests.swift @@ -22,4 +22,19 @@ struct ConnectionMenuPolicyTests { #expect(!ConnectionMenuPolicy.showsDisconnect(status: .connecting)) #expect(!ConnectionMenuPolicy.showsDisconnect(status: .error("refused"))) } + + @Test("Server Dashboard is offered for a live session on an engine that has one") + func serverDashboardNeedsLiveSessionAndEngine() { + #expect(ConnectionMenuPolicy.showsServerDashboard(status: .connected, type: .postgresql)) + #expect(!ConnectionMenuPolicy.showsServerDashboard(status: .connected, type: .sqlite)) + #expect(!ConnectionMenuPolicy.showsServerDashboard(status: .disconnected, type: .postgresql)) + #expect(!ConnectionMenuPolicy.showsServerDashboard(status: .error("refused"), type: .postgresql)) + } + + @Test("Query Insights is offered for any live session") + func queryInsightsNeedsLiveSession() { + #expect(ConnectionMenuPolicy.showsQueryInsights(status: .connected)) + #expect(!ConnectionMenuPolicy.showsQueryInsights(status: .connecting)) + #expect(!ConnectionMenuPolicy.showsQueryInsights(status: .disconnected)) + } } diff --git a/apple/TableProTests/Core/Services/Infrastructure/MenuActionSelectorCollisionTests.swift b/apple/TableProTests/Core/Services/Infrastructure/MenuActionSelectorCollisionTests.swift index 72ea8d9a74..621aa50020 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/MenuActionSelectorCollisionTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/MenuActionSelectorCollisionTests.swift @@ -28,7 +28,9 @@ struct MenuActionSelectorCollisionTests { #selector(MainSplitViewController.exportTables(_:)), #selector(MainSplitViewController.exportQueryResults(_:)), #selector(MainSplitViewController.backupDatabase(_:)), - #selector(MainSplitViewController.restoreDatabase(_:)) + #selector(MainSplitViewController.restoreDatabase(_:)), + #selector(MainSplitViewController.showServerDashboard(_:)), + #selector(MainSplitViewController.showQueryInsights(_:)) ] ) func railDoesNotShadowMenuBarCommands(command: Selector) { From d8cafaabf4070f84ad1fca5f41107f00fdd7545e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Nam=20Long?= <26674308+J2TeamNNL@users.noreply.github.com> Date: Sun, 1 Nov 2026 03:17:02 +0700 Subject: [PATCH 2/6] docs(changelog): add the entry for #3957 --- apple/changelog.d/3957.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 apple/changelog.d/3957.added.md diff --git a/apple/changelog.d/3957.added.md b/apple/changelog.d/3957.added.md new file mode 100644 index 0000000000..9b52776ef1 --- /dev/null +++ b/apple/changelog.d/3957.added.md @@ -0,0 +1 @@ +Server Dashboard and Query Insights open with Shift-Option-Command-D and -I, or from a connection's menu on the strip. From 17a42fa2fe0b6cc02a108090af0d538479b77c73 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 1 Nov 2026 12:18:44 +0700 Subject: [PATCH 3/6] refactor(connections): run strip tools through the window and gate Agent mode by allowlist --- .../Core/Menu/DatabaseMenuBuilder.swift | 17 +- .../Infrastructure/ConnectionMenuPolicy.swift | 8 - ...nSplitViewController+EditMenuActions.swift | 23 +++ ...nSplitViewController+FileMenuActions.swift | 45 ++---- ...inSplitViewController+MenuValidation.swift | 91 ++++++----- .../ConnectionActionsMenuResolver.swift | 12 +- .../Toolbar/ToolbarContextResolver.swift | 14 +- .../WorkspaceRailViewController.swift | 147 ++++++++---------- .../Models/UI/KeyboardShortcutModels.swift | 4 +- .../Views/AIChat/ChatComposerTextView.swift | 34 ++++ .../Main/MainContentCommandActions.swift | 9 ++ .../Core/Menu/MainMenuBuilderTests.swift | 20 +++ .../Core/Menu/ServerToolsCommandTests.swift | 73 --------- .../Services/ConnectionMenuPolicyTests.swift | 15 -- .../Infrastructure/AgentModeWindowTests.swift | 51 ++++++ .../ConnectionActionsMenuResolverTests.swift | 34 ++++ .../MenuContentModeParityTests.swift | 51 ++++++ .../WorkspaceRailServerToolTests.swift | 101 ++++++++++++ .../Models/ShortcutUniquenessTests.swift | 1 + .../AIChat/ChatComposerScrollViewTests.swift | 30 ++++ apple/changelog.d/3957.added.md | 2 +- apple/changelog.d/3957.fixed.1.md | 1 + apple/changelog.d/3957.fixed.md | 1 + 23 files changed, 503 insertions(+), 281 deletions(-) delete mode 100644 apple/TableProTests/Core/Menu/ServerToolsCommandTests.swift create mode 100644 apple/TableProTests/Core/Services/Infrastructure/WorkspaceRailServerToolTests.swift create mode 100644 apple/changelog.d/3957.fixed.1.md create mode 100644 apple/changelog.d/3957.fixed.md diff --git a/apple/TablePro/Core/Menu/DatabaseMenuBuilder.swift b/apple/TablePro/Core/Menu/DatabaseMenuBuilder.swift index e76e29c4fd..e8a52960cb 100644 --- a/apple/TablePro/Core/Menu/DatabaseMenuBuilder.swift +++ b/apple/TablePro/Core/Menu/DatabaseMenuBuilder.swift @@ -5,10 +5,7 @@ import AppKit -/// "Open Database" and "Close Tabs for Other Databases" read Database or Schema -/// depending on what the driver switches between. That is a closed two-string set -/// driven by `ContainerSwitchTarget`, not an interpolated driver name, because -/// System Settings binds an App Shortcut to a menu item's exact literal title. +// Database-or-Schema titles are a closed set, never interpolated: System Settings binds an App Shortcut to the exact title. @MainActor enum DatabaseMenuBuilder { static func build(keyboard: KeyboardSettings) -> NSMenuItem { @@ -64,10 +61,7 @@ enum DatabaseMenuBuilder { String(localized: "Search Database…"), action: #selector(MainSplitViewController.searchDatabase(_:)) ), - /// The sidebar's own Copy To and Duplicate Database, mirrored so both are reachable - /// from the keyboard. The menu acts on the database being browsed, which is what a - /// command with no clicked row can mean. Spelled exactly as the sidebar and the sheet - /// spell it: one command carrying two names reads as two commands. + // Mirrors the sidebar, acting on the browsed database; one command with two names reads as two. MenuItemFactory.item( String(localized: "Copy To…"), action: #selector(MainSplitViewController.copyObjectsToDatabase(_:)) @@ -85,8 +79,7 @@ enum DatabaseMenuBuilder { String(localized: "Edit View Definition…"), action: #selector(MainSplitViewController.editViewDefinition(_:)) ), - /// The sidebar's own per-object commands, mirrored so each is reachable from the menu - /// bar and the keyboard, and spelled exactly as the sidebar spells them. + // The sidebar's per-object commands, spelled as the sidebar spells them. MenuItemFactory.item( String(localized: "Show DDL"), action: #selector(MainSplitViewController.showObjectDDL(_:)) @@ -148,9 +141,7 @@ enum DatabaseMenuBuilder { keyboard: keyboard ), MenuItemFactory.separator, - /// No ellipsis: the HIG reserves one for an action that needs more information before - /// it can complete, and this needs none. It validates to disabled for every connection - /// whose driver holds no file, which is all of them but the embedded engines. + // No ellipsis: the HIG keeps one for a command that asks for more input, and this asks for none. MenuItemFactory.item( String(localized: "Release File Lock"), action: #selector(MainSplitViewController.releaseFileLock(_:)) diff --git a/apple/TablePro/Core/Services/Infrastructure/ConnectionMenuPolicy.swift b/apple/TablePro/Core/Services/Infrastructure/ConnectionMenuPolicy.swift index 5162184912..45fd8b3f4f 100644 --- a/apple/TablePro/Core/Services/Infrastructure/ConnectionMenuPolicy.swift +++ b/apple/TablePro/Core/Services/Infrastructure/ConnectionMenuPolicy.swift @@ -12,12 +12,4 @@ internal enum ConnectionMenuPolicy { internal static func showsDisconnect(status: ConnectionStatus) -> Bool { status.isConnected } - - internal static func showsServerDashboard(status: ConnectionStatus, type: DatabaseType) -> Bool { - status.isConnected && ServerDashboardQueryProviderFactory.supportsDashboard(for: type) - } - - internal static func showsQueryInsights(status: ConnectionStatus) -> Bool { - status.isConnected - } } diff --git a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+EditMenuActions.swift b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+EditMenuActions.swift index a44b67e21c..261c0833e3 100644 --- a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+EditMenuActions.swift +++ b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+EditMenuActions.swift @@ -10,14 +10,37 @@ extension MainSplitViewController: GridRowCopyCommandResponding {} // Fallback responders; paste: must be here, or AppKit disables Paste and the disabled item swallows Cmd+V window-wide. extension MainSplitViewController { + // Agent mode draws no tab history, so Undo runs the window's own manager, as `NSWindow.undo(_:)` would. @objc func undo(_ sender: Any?) { + guard contentMode == .browse else { + view.window?.undoManager?.undo() + return + } commandActions?.undoChange() } @objc func redo(_ sender: Any?) { + guard contentMode == .browse else { + view.window?.undoManager?.redo() + return + } commandActions?.redoChange() } + var undoTitle: String { + guard contentMode == .browse else { + return view.window?.undoManager?.undoMenuItemTitle ?? String(localized: "Undo") + } + return commandActions?.resolvedUndoTitle ?? String(localized: "Undo") + } + + var redoTitle: String { + guard contentMode == .browse else { + return view.window?.undoManager?.redoMenuItemTitle ?? String(localized: "Redo") + } + return commandActions?.resolvedRedoTitle ?? String(localized: "Redo") + } + @objc func copy(_ sender: Any?) { guard let actions = commandActions else { return } if actions.hasRowSelection { diff --git a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift index 879cd2c8c3..2d23bcea46 100644 --- a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift +++ b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+FileMenuActions.swift @@ -25,23 +25,9 @@ extension MainSplitViewController { commandActions?.newTab() } - /// The editor's meaning of Close, reached through `EditorWindow.performClose(_:)`. - /// - /// It is not an action itself, because the window already is one: `performClose:` resolves to - /// the nearest responder that claims it, so the connections strip still takes Close while it - /// holds the keyboard, and everything else in the window falls through to the window. - /// - /// A connecting or failed pane has no command surface, so Close ends the connection instead. - /// Leaving that to `commandActions` made the command inert on exactly the pane a user most - /// wants to dismiss. - /// Returns false when the window hosts nothing to close, so the window falls back to closing - /// itself rather than letting the command resolve to nothing, which is the failure this whole - /// route exists to make impossible. - /// What Close does here, in the words the connections strip already uses for the same command. - /// A window with no tab left closes the connection, so naming it "Close Tab" would describe an - /// action the command does not take. + // The strip's words for the same command. Agent mode shows no tab, so it names what a window with none would close. var closeCommandTitle: String? { - if commandActions?.hasOpenTab == true { return String(localized: "Close Tab") } + if contentMode == .browse, commandActions?.hasOpenTab == true { return String(localized: "Close Tab") } if let workspace = workspaces.selected, let entry = WorkspaceCloseAction.closableEntry( connectionId: workspace.connectionId, @@ -53,22 +39,18 @@ extension MainSplitViewController { return String(format: String(localized: "Close “%@”"), name) } + // Reached through `EditorWindow.performClose(_:)`; false lets the window close itself when it hosts nothing. func closeFrontmostTab() -> Bool { guard let actions = commandActions else { guard let connectionId = workspaces.selectedConnectionId else { return false } WindowManager.shared.closeWindow(for: connectionId) return true } - /// A window that exists only to hold a tab moved out of another one closes with that tab. - /// `closeTab` deliberately leaves a window standing when its last tab goes, because the - /// connection is still open in it and its object browser is still useful; that is the right - /// answer for the window a connection lives in and the wrong one here, where Close would - /// empty the window and pressing Close again would take the connection down in both. - /// - /// It still goes through `closeTabAwaiting`, which is the primitive Cmd+W is required to - /// keep. Skipping to `super.performClose` closed the window over a save prompt that was - /// never shown and never reached Recently Closed Tabs. The window closes only once the tab - /// actually went, so Cancel at the prompt leaves both standing. + guard contentMode == .browse else { + actions.closeWorkspace() + return true + } + // A window holding only a moved tab closes with it, after `closeTabAwaiting` so a cancelled save prompt keeps both. if isDetachedSingleTabWindow, let selected = workspaces.selected?.sessionState?.tabManager.selectedTab { Task { @MainActor [weak self] in await actions.closeTabAwaiting(id: selected.id) @@ -76,18 +58,11 @@ extension MainSplitViewController { self.workspaces.selected?.sessionState?.tabManager.tabs.isEmpty == true, let workspace = self.workspaces.selected else { return } - /// Asked again after the await. The save sheet can stand for as long as the user - /// likes, and the window this tab was moved out of can close underneath it: closing - /// then takes the connection's last window with it, which disconnects the session - /// and skips the whole-window confirmation that close would otherwise raise. + // Asked again: the source window can close during the prompt, and this one would then be the connection's last. guard WindowManager.shared.workspaces(for: workspace.connectionId).count > 1 else { return } - /// `closeWindowAwaiting`, not a raw `close()`. An inspector edit is connection - /// scoped, so `closeTabAwaiting` deliberately does not ask about it; closing the - /// window directly then tore down the per-window `RightPanelState` and dropped it - /// with no prompt. With the tab already gone this window has none left, which is - /// the branch that closes it. + // Not a raw `close()`, which dropped an unsaved inspector edit with no prompt. await actions.closeWindowAwaiting() } return true diff --git a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift index f480747647..660d2c6a9e 100644 --- a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift +++ b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift @@ -145,7 +145,12 @@ struct MenuValidationContext: Equatable { extension MainSplitViewController: NSMenuItemValidation { // Nil: not this window's call. An unhandled selector stays lit (Clear Selection did); MenuValidationCoverageTests catches it. static func resolvedEnablement(_ selector: Selector, context: MenuValidationContext) -> Bool? { - if context.isAgentMode, browseContentSelectors.contains(selector) { return false } + guard let answer = ownEnablement(selector, context: context) else { return nil } + guard context.isAgentMode else { return answer } + return answer && agentModeSelectors.contains(selector) + } + + private static func ownEnablement(_ selector: Selector, context: MenuValidationContext) -> Bool? { if let find = isFindCommandEnabled(selector, context: context) { return find } if let query = isQueryCommandEnabled(selector, context: context) { return query } if let chooser = isContainerCommandEnabled(selector, context: context) { return chooser } @@ -381,6 +386,9 @@ extension MainSplitViewController: NSMenuItemValidation { return context.isConnected && !context.isReadOnly && context.canCreateSchema case #selector(createTableFolder(_:)): return context.isConnected && context.canCreateTableFolder + // The schema submenu adds this only for an editable schema; reaching here still needs a writable session. + case #selector(editCurrentSchema(_:)): + return context.isConnected && !context.isReadOnly default: return nil } @@ -442,36 +450,36 @@ extension MainSplitViewController: NSMenuItemValidation { } } - // Agent mode does not mount browse content. MenuContentModeParityTests derives this set from the toolbar. - private static let browseContentSelectors: Set = [ - #selector(refreshDatabase(_:)), - #selector(saveDocument(_:)), - #selector(discardChanges(_:)), - #selector(addRow(_:)), - #selector(restorePreviousValues(_:)), - #selector(editDocument(_:)), - #selector(insertDocument(_:)), - #selector(previewSQL(_:)), - #selector(toggleResults(_:)), - #selector(toggleQueryHistory(_:)), - #selector(newEditorTab(_:)), - #selector(openQuickSwitcher(_:)), - #selector(exportTables(_:)), - #selector(exportDataDictionary(_:)), - // Both spellings of one command; one without the other leaves the format list live over a dim leaf. - #selector(importData(_:)), - #selector(importDataFormat(_:)), - #selector(showServerDashboard(_:)), - #selector(navigateBack(_:)), - #selector(navigateForward(_:)), - #selector(moveTabToConnection(_:)), - #selector(searchDatabase(_:)), - #selector(buildJoinQuery(_:)), - #selector(pinResult(_:)), - #selector(compareResult(_:)), - #selector(showPreviousResult(_:)), - #selector(showNextResult(_:)), - #selector(closeResultTab(_:)), + // Agent mode mounts no browse content, so the window answers there only for the window, the connection and the conversation. + static let agentModeSelectors: Set = [ + #selector(switchConnection(_:)), + #selector(closeConnection(_:)), + #selector(setSafeModeLevel(_:)), + #selector(releaseFileLock(_:)), + #selector(openContainerSwitcher(_:)), + #selector(openSchemaSwitcher(_:)), + #selector(switchToSchema(_:)), + #selector(switchSessionContext(_:)), + #selector(setFavoriteDatabaseEnvironment(_:)), + #selector(removeFavoriteDatabase(_:)), + #selector(setContentModeFromMenu(_:)), + #selector(toggleContentModeFromMenu(_:)), + #selector(newAgentSession(_:)), + #selector(openAgentSession(_:)), + #selector(closeAgentSession(_:)), + #selector(deleteAgentSession(_:)), + #selector(newAIConversation(_:)), + #selector(switchAIConversation(_:)), + #selector(clearAIConversations(_:)), + #selector(focusAssistant(_:)), + #selector(focusInspector(_:)), + #selector(toggleWorkspaceRail(_:)), + #selector(showPreviousWorkspace(_:)), + #selector(showNextWorkspace(_:)), + #selector(switchToRecentTab(_:)), + #selector(switchToLeastRecentTab(_:)), + #selector(undo(_:)), + #selector(redo(_:)), ] // A command this window does not own stays enabled, which keeps `performClose:` and the system's own items working. @@ -557,6 +565,18 @@ extension MainSplitViewController: NSMenuItemValidation { } } + // Only the connection-level facts, read the way `menuValidationContext` reads them, so a connection off screen can be asked. + func menuValidationContext(forHosted connectionId: UUID) -> MenuValidationContext? { + guard let workspace = workspaces.workspace(for: connectionId) else { return nil } + var context = MenuValidationContext() + context.hasSelectedWorkspace = true + context.isConnected = Self.isConnected(phase: workspace.phase, pane: workspace.resolvedPane) + context.isAgentMode = workspace.resolvedContentMode == .agent + context.supportsServerDashboard = workspace.sessionState?.coordinator.commandActions? + .supportsServerDashboard ?? false + return context + } + // Rail facts and Focus Assistant come from the window: off the connection they dimmed with no coordinator or in Agent mode. var menuValidationContext: MenuValidationContext { let conversations = assistantConversationModel @@ -653,8 +673,8 @@ extension MainSplitViewController: NSMenuItemValidation { canSearchDatabase: actions.canSearchDatabase, canBuildJoinQuery: actions.canBuildJoinQuery, hasMaintenanceOperations: !actions.maintenanceOperations.isEmpty, - canUndo: actions.canUndo, - canRedo: actions.canRedo, + canUndo: contentMode == .browse ? actions.canUndo : view.window?.undoManager?.canUndo == true, + canRedo: contentMode == .browse ? actions.canRedo : view.window?.undoManager?.canRedo == true, hasEditorForFind: EditorEventRouter.shared.keyWindowHasEditor, hasSelectionForFind: EditorEventRouter.shared.keyWindowEditorHasSelectionForFind, hasActiveGridFind: actions.hasActiveGridFind, @@ -735,9 +755,9 @@ extension MainSplitViewController: NSMenuItemValidation { case #selector(toggleWorkspaceRail(_:)): setTitle(isWorkspaceRailEnabled ? "Hide Connections" : "Show Connections", on: menuItem) case #selector(undo(_:)): - setResolvedTitle(commandActions?.resolvedUndoTitle ?? String(localized: "Undo"), on: menuItem) + setResolvedTitle(undoTitle, on: menuItem) case #selector(redo(_:)): - setResolvedTitle(commandActions?.resolvedRedoTitle ?? String(localized: "Redo"), on: menuItem) + setResolvedTitle(redoTitle, on: menuItem) case #selector(toggleFilterBar(_:)): setTitle(commandActions?.isFilterBarVisible == true ? "Hide Filter Bar" : "Show Filter Bar", on: menuItem) case #selector(toggleQueryHistory(_:)): @@ -791,6 +811,7 @@ extension MainSplitViewController: NSMenuItemValidation { // The mode is in `representedObject`, so enablement needs the item, not the selector the shared table keys on. private func canShowResultView(_ menuItem: NSMenuItem) -> Bool { + guard contentMode == .browse else { return false } guard let raw = menuItem.representedObject as? String, let mode = ResultsViewMode(rawValue: raw) else { return false } return commandActions?.availableResultsViewModes.contains(mode) ?? false diff --git a/apple/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift b/apple/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift index d61ea01cf8..4d62d841bf 100644 --- a/apple/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift +++ b/apple/TablePro/Core/Services/Infrastructure/Toolbar/ConnectionActionsMenuResolver.swift @@ -97,7 +97,8 @@ internal enum ConnectionActionsMenuResolver { entries.append( ActionsMenuEntry( title: String(localized: "Restore Previous Values…"), - selector: NSSelectorFromString("restorePreviousValues:") + selector: NSSelectorFromString("restorePreviousValues:"), + shortcut: .restorePreviousValues ) ) } @@ -200,8 +201,7 @@ internal enum ConnectionActionsMenuResolver { ]) } - /// The places in this connection the window can go. Each of these opens a tab or a drawer, so - /// they belong together and away from the commands that change data. + // Each opens a tab or a drawer, so they sit apart from the commands that change data. private static func windowSection(_ context: ToolbarContext) -> ActionsMenuSection? { var entries: [ActionsMenuEntry] = [ ActionsMenuEntry( @@ -215,14 +215,16 @@ internal enum ConnectionActionsMenuResolver { ), ActionsMenuEntry( title: String(localized: "Query Insights"), - selector: NSSelectorFromString("showQueryInsights:") + selector: NSSelectorFromString("showQueryInsights:"), + shortcut: .showQueryInsights ), ] if context.supportsServerDashboard { entries.append( ActionsMenuEntry( title: String(localized: "Server Dashboard"), - selector: NSSelectorFromString("showServerDashboard:") + selector: NSSelectorFromString("showServerDashboard:"), + shortcut: .showServerDashboard ) ) } diff --git a/apple/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift b/apple/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift index 5fb912893c..2a61bb46ed 100644 --- a/apple/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift +++ b/apple/TablePro/Core/Services/Infrastructure/Toolbar/ToolbarContextResolver.swift @@ -121,19 +121,7 @@ internal enum ToolbarContextResolver { } } - /// The items that act on the browse content, which Agent mode does not mount. - /// - /// Two of them leave the titlebar there; the rest are opt-in from the customization palette, so a - /// user who put one back would otherwise have a live button over a surface that is not on screen: - /// Refresh with no grid to reload, Query History flipping a persisted flag for a drawer that is - /// not mounted and that then sprang open on the way back to browsing, New Tab opening a tab - /// behind the conversation, and the commit control over a gate frozen at the moment the mode - /// changed. - /// - /// Stated once rather than as a term in thirteen arms, because it is one rule. The menu bar's - /// `browseContentSelectors` is the same list in its own vocabulary, and - /// `MenuContentModeParityTests` derives this one back out of the toolbar and holds the two - /// together, so an item added here without a menu twin fails rather than ships. + // Agent mode mounts no browse content. `MenuContentModeParityTests` holds these to the menu bar's `agentModeSelectors`. private static let browseContentIdentifiers: Set = [ MainWindowToolbar.refresh, MainWindowToolbar.saveChanges, diff --git a/apple/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift b/apple/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift index 865f991eb1..4d5d4a64ec 100644 --- a/apple/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift +++ b/apple/TablePro/Core/Services/Infrastructure/WorkspaceRailViewController.swift @@ -87,6 +87,12 @@ internal final class WorkspaceRailTableView: NSTableView { } } +internal struct WorkspaceRailServerTool: Equatable { + internal let title: String + internal let command: Selector + internal let railAction: Selector +} + @MainActor internal final class WorkspaceRailViewController: NSViewController { private var rowSizeObservation: AnyCancellable? @@ -509,42 +515,32 @@ internal final class WorkspaceRailViewController: NSViewController { activate(workspace) } - /// Every rail entry is backed by a live window, so the window is raised directly rather - /// than routed through `TabRouter.openConnection`, which resolves the id against - /// `ConnectionStorage` and would fail for a connection opened from a URL that was never - /// saved. Moving between two containers of the same connection stays in one window and - /// only moves that window's browse cursor. - /// - /// One path, whichever window owns the entry. Selecting the connection in its own window comes - /// first and always: the cross-window case used to raise the target window and move the target - /// connection's browse cursor without ever telling that window to show it, so it came forward - /// still showing the connection it had, its tabs and toolbar named that one, and the work the - /// user clicked moved silently behind it. - /// - /// It ends at `applySelection`, which reads this window's own selection back: a workspace this - /// window hosts leaves the highlight on the row the user picked; one belonging to another window - /// leaves it where it was, because this window did not move. + // `applySelection` reads the window back, so a row another window hosts leaves this rail's highlight where it was. private func activate(_ workspace: WorkspaceID) { + guard let shown = show(workspace) else { return } + moveBrowseCursor(of: shown.window, to: workspace) + applySelection() + } + + // Raised directly: `TabRouter.openConnection` resolves through `ConnectionStorage` and fails for an unsaved URL connection. + private func show(_ workspace: WorkspaceID) -> (window: NSWindow, host: MainSplitViewController)? { guard let window = WindowManager.shared.window(for: workspace.connectionId), - let target = window.contentViewController as? MainSplitViewController else { + let host = window.contentViewController as? MainSplitViewController else { Self.logger.error( - "activate has no window target=\(Self.describe(workspace), privacy: .public)" + "show has no window target=\(Self.describe(workspace), privacy: .public)" ) - return + return nil } - target.selectHostedConnection(workspace.connectionId) + // Before the raise, or the window comes forward still showing the connection it had. + host.selectHostedConnection(workspace.connectionId) - guard window !== view.window else { - moveBrowseCursor(of: window, to: workspace) - applySelection() - return - } + guard window !== view.window else { return (window, host) } let group = window.tabGroup Self.logger.info( """ - activate rail=\(self.railName, privacy: .public) \ + show rail=\(self.railName, privacy: .public) \ target=\(Self.describe(workspace), privacy: .public) \ window=\(window.windowNumber, privacy: .public) visible=\(window.isVisible, privacy: .public) \ tabs=\(group?.windows.count ?? 0, privacy: .public) \ @@ -552,16 +548,13 @@ internal final class WorkspaceRailViewController: NSViewController { """ ) - /// A background tab is a window AppKit will happily make key without bringing to the - /// front of its group, which reads as the click doing nothing. Selecting it in the - /// group first is the documented way to raise a specific tab. + // AppKit makes a background tab key without bringing it to the front of its group. if let group, group.selectedWindow !== window { group.selectedWindow = window } window.makeKeyAndOrderFront(nil) AppActivationPolicyController.shared.activate() - moveBrowseCursor(of: window, to: workspace) - applySelection() + return (window, host) } /// Going to an entry of a connection: its tabs come forward, and the object tree follows to its @@ -618,28 +611,46 @@ internal final class WorkspaceRailViewController: NSViewController { WindowManager.shared.moveToNewWindow(connectionId: workspace.connectionId) } - /// Named for the rail, not `showServerDashboard(_:)`: that is the menu bar's nil-target action, - /// and the rail sits ahead of `MainSplitViewController` in the responder chain. + // Not `showServerDashboard(_:)`: the rail sits ahead of the window in the responder chain and would take the menu bar's command. @objc private func showWorkspaceServerDashboard(_ sender: NSMenuItem) { - guard let workspace = sender.representedObject as? WorkspaceID else { return } - showTool(of: workspace) { $0.showServerDashboard() } + runServerTool(#selector(MainSplitViewController.showServerDashboard(_:)), from: sender) } @objc private func showWorkspaceQueryInsights(_ sender: NSMenuItem) { - guard let workspace = sender.representedObject as? WorkspaceID else { return } - showTool(of: workspace) { $0.showQueryInsights() } + runServerTool(#selector(MainSplitViewController.showQueryInsights(_:)), from: sender) } - /// Switches to the clicked row first, so the tool opens beside the work it belongs to. - private func showTool(of workspace: WorkspaceID, _ open: (MainContentCoordinator) -> Void) { - if let row = entries.firstIndex(where: { $0.workspace == workspace }) { - tableView.selectRowIndexes(IndexSet(integer: row), byExtendingSelection: false) - commit(row: row) - } - guard let coordinator = WindowManager.shared.coordinator(for: workspace.connectionId) else { return } - open(coordinator) + // Both tools are server-wide, so the browse cursor stays: going to the row's container would switch databases for nothing. + private func runServerTool(_ command: Selector, from sender: NSMenuItem) { + guard let workspace = sender.representedObject as? WorkspaceID, + let shown = show(workspace) else { return } + applySelection() + guard MainSplitViewController.isEnabled(command, context: shown.host.menuValidationContext) else { return } + NSApp.sendAction(command, to: shown.host, from: sender) + } + + // The Database menu's titles, order and commands, offered on the menu bar's own answer. + internal static func serverTools(offeredWith context: MenuValidationContext) -> [WorkspaceRailServerTool] { + [ + WorkspaceRailServerTool( + title: String(localized: "Server Dashboard"), + command: #selector(MainSplitViewController.showServerDashboard(_:)), + railAction: #selector(showWorkspaceServerDashboard(_:)) + ), + WorkspaceRailServerTool( + title: String(localized: "Query Insights"), + command: #selector(MainSplitViewController.showQueryInsights(_:)), + railAction: #selector(showWorkspaceQueryInsights(_:)) + ), + ].filter { MainSplitViewController.isEnabled($0.command, context: context) } + } + + private static func serverTools(for connectionId: UUID) -> [WorkspaceRailServerTool] { + let host = WindowManager.shared.window(for: connectionId)?.contentViewController as? MainSplitViewController + guard let context = host?.menuValidationContext(forHosted: connectionId) else { return [] } + return serverTools(offeredWith: context) } @objc @@ -751,15 +762,16 @@ extension WorkspaceRailViewController: CloseCommandNaming { // MARK: - NSMenuDelegate extension WorkspaceRailViewController: NSMenuDelegate { - /// Lighter action first, the one that ends the connection last, which is the order Finder uses - /// on a Locations row and Mail on an account. Close carries the connection's own name because - /// a row can be one of several a connection has open, and the command takes all of them. internal func menuNeedsUpdate(_ menu: NSMenu) { menu.removeAllItems() let row = tableView.clickedRow guard entries.indices.contains(row) else { return } let entry = entries[row] + populate(menu, for: entry, serverTools: Self.serverTools(for: entry.workspace.connectionId)) + } + // Lighter actions first and the one that ends the connection last, the order Finder and Mail keep. + internal func populate(_ menu: NSMenu, for entry: WorkspaceRailEntry, serverTools: [WorkspaceRailServerTool]) { if WindowManager.shared.canMoveToNewWindow(connectionId: entry.workspace.connectionId) { addItem( to: menu, @@ -770,7 +782,13 @@ extension WorkspaceRailViewController: NSMenuDelegate { menu.addItem(.separator()) } - addToolItems(to: menu, for: entry) + // No key equivalent: a context item's chord would run on the window's connection, not the row's. + for tool in serverTools { + addItem(to: menu, title: tool.title, action: tool.railAction, workspace: entry.workspace) + } + if !serverTools.isEmpty { + menu.addItem(.separator()) + } if let releaseTitle = ConnectionFileLockAction.commandTitle( connectionId: entry.workspace.connectionId @@ -810,42 +828,11 @@ extension WorkspaceRailViewController: NSMenuDelegate { ) } - /// Spelled as the Database menu spells them, and showing the same shortcut it does. - private func addToolItems(to menu: NSMenu, for entry: WorkspaceRailEntry) { - let keyboard = AppSettingsManager.shared.keyboard - var added = false - if ConnectionMenuPolicy.showsServerDashboard(status: entry.status, type: entry.connection.type) { - let item = addItem( - to: menu, - title: String(localized: "Server Dashboard"), - action: #selector(showWorkspaceServerDashboard(_:)), - workspace: entry.workspace - ) - MenuItemFactory.apply(shortcut: .showServerDashboard, keyboard: keyboard, to: item) - added = true - } - if ConnectionMenuPolicy.showsQueryInsights(status: entry.status) { - let item = addItem( - to: menu, - title: String(localized: "Query Insights"), - action: #selector(showWorkspaceQueryInsights(_:)), - workspace: entry.workspace - ) - MenuItemFactory.apply(shortcut: .showQueryInsights, keyboard: keyboard, to: item) - added = true - } - if added { - menu.addItem(.separator()) - } - } - - @discardableResult - private func addItem(to menu: NSMenu, title: String, action: Selector, workspace: WorkspaceID) -> NSMenuItem { + private func addItem(to menu: NSMenu, title: String, action: Selector, workspace: WorkspaceID) { let item = NSMenuItem(title: title, action: action, keyEquivalent: "") item.target = self item.representedObject = workspace menu.addItem(item) - return item } } diff --git a/apple/TablePro/Models/UI/KeyboardShortcutModels.swift b/apple/TablePro/Models/UI/KeyboardShortcutModels.swift index 3d8544ddcd..675283e65d 100644 --- a/apple/TablePro/Models/UI/KeyboardShortcutModels.swift +++ b/apple/TablePro/Models/UI/KeyboardShortcutModels.swift @@ -59,7 +59,6 @@ enum ShortcutCategory: String, Codable, CaseIterable, Identifiable { // MARK: - Shortcut Action -/// All customizable keyboard shortcut actions enum ShortcutAction: String, Codable, CaseIterable, Identifiable { // Connections case manageConnections @@ -659,8 +658,7 @@ struct KeyboardSettings: Codable, Equatable { // MARK: - Default Shortcuts - /// Default shortcuts, applied when the user has no override. An action absent - /// from this map has no default and shows as unassigned until the user binds it. + // An action absent here shows as unassigned until the user binds it. static let defaultShortcuts: [ShortcutAction: BoundKey] = [ // Connections .newConnection: .character("n", command: true), diff --git a/apple/TablePro/Views/AIChat/ChatComposerTextView.swift b/apple/TablePro/Views/AIChat/ChatComposerTextView.swift index abbb44c972..9be8004b76 100644 --- a/apple/TablePro/Views/AIChat/ChatComposerTextView.swift +++ b/apple/TablePro/Views/AIChat/ChatComposerTextView.swift @@ -236,6 +236,40 @@ final class ChatComposerNSTextView: NSTextView { return textView } + // Its own typing history: the window's is the connection's, and the window's Undo routes into tab histories. + private let typingUndoManager = UndoManager() + + override var undoManager: UndoManager? { + typingUndoManager + } + + @objc func undo(_ sender: Any?) { + typingUndoManager.undo() + } + + @objc func redo(_ sender: Any?) { + typingUndoManager.redo() + } + + override func validateMenuItem(_ menuItem: NSMenuItem) -> Bool { + switch menuItem.action { + case #selector(undo(_:)): + Self.setTitle(typingUndoManager.undoMenuItemTitle, on: menuItem) + return typingUndoManager.canUndo + case #selector(redo(_:)): + Self.setTitle(typingUndoManager.redoMenuItemTitle, on: menuItem) + return typingUndoManager.canRedo + default: + return super.validateMenuItem(menuItem) + } + } + + // A title write re-lays out an open menu and cancels its tracking. + private static func setTitle(_ title: String, on menuItem: NSMenuItem) { + guard menuItem.title != title else { return } + menuItem.title = title + } + override func becomeFirstResponder() -> Bool { let became = super.becomeFirstResponder() if became { onFocusChange?(true) } diff --git a/apple/TablePro/Views/Main/MainContentCommandActions.swift b/apple/TablePro/Views/Main/MainContentCommandActions.swift index 7622d46b33..e20a3b09c9 100644 --- a/apple/TablePro/Views/Main/MainContentCommandActions.swift +++ b/apple/TablePro/Views/Main/MainContentCommandActions.swift @@ -667,6 +667,15 @@ final class MainContentCommandActions: ObservableObject { closeTab(id: selected.id) return } + closeWorkspace() + } + + // What Close does with no tab on screen: the container, else the connection. + func closeWorkspace() { + guard let coordinator else { + Task { await closeWindowAwaiting() } + return + } if let entry = WorkspaceCloseAction.closableEntry( connectionId: connectionId, container: coordinator.tabManager.visibleContainer diff --git a/apple/TableProTests/Core/Menu/MainMenuBuilderTests.swift b/apple/TableProTests/Core/Menu/MainMenuBuilderTests.swift index 2a01ca20ff..c5a56baff6 100644 --- a/apple/TableProTests/Core/Menu/MainMenuBuilderTests.swift +++ b/apple/TableProTests/Core/Menu/MainMenuBuilderTests.swift @@ -883,6 +883,26 @@ struct MainMenuValidationTests { } } + // Each opens a tab, and Agent mode does not mount the tab strip, so the tab would wait unseen behind the conversation. + @Test("Agent mode dims the commands that open a database tool's tab") + func databaseToolsDimInAgentMode() { + var context = capableContext() + context.isConnected = true + let toolCommands = [ + #selector(MainSplitViewController.showServerDashboard(_:)), + #selector(MainSplitViewController.showQueryInsights(_:)), + #selector(MainSplitViewController.showUsersAndRoles(_:)), + #selector(MainSplitViewController.showERDiagram(_:)) + ] + for selector in toolCommands { + #expect(enabled(selector, context), "\(selector) is dim while browsing") + } + context.isAgentMode = true + for selector in toolCommands { + #expect(!enabled(selector, context), "\(selector) stayed lit in Agent mode") + } + } + @Test("A stale selection does not keep content commands enabled without a connection") func contentCommandsNeedTheConnection() { var context = capableContext() diff --git a/apple/TableProTests/Core/Menu/ServerToolsCommandTests.swift b/apple/TableProTests/Core/Menu/ServerToolsCommandTests.swift deleted file mode 100644 index cf17ac7a35..0000000000 --- a/apple/TableProTests/Core/Menu/ServerToolsCommandTests.swift +++ /dev/null @@ -1,73 +0,0 @@ -// -// ServerToolsCommandTests.swift -// TableProTests -// - -import AppKit -@testable import TablePro -import Testing - -struct ServerToolsShortcutTests { - @Test( - "Each chord belongs to its command alone and shadows no reserved command", - arguments: [ - (ShortcutAction.showServerDashboard, BoundKey.character("d", command: true, shift: true, option: true)), - (ShortcutAction.showQueryInsights, BoundKey.character("i", command: true, shift: true, option: true)) - ] - ) - func chordIsUnique(action: ShortcutAction, chord: BoundKey) { - #expect(KeyboardSettings.defaultShortcuts[action] == chord) - let owners = KeyboardSettings.defaultShortcuts.filter { $0.value == chord }.map(\.key) - #expect(owners == [action]) - #expect(ShortcutAction.reservedConflict(for: chord, context: action.context) == nil) - } - - @Test("Both are listed under Connections by the names the Database menu uses") - func listedUnderConnections() { - #expect(ShortcutAction.showServerDashboard.category == .connections) - #expect(ShortcutAction.showQueryInsights.category == .connections) - #expect(ShortcutAction.showServerDashboard.context == .global) - #expect(ShortcutAction.showQueryInsights.context == .global) - #expect(ShortcutAction.showServerDashboard.displayName == String(localized: "Server Dashboard")) - #expect(ShortcutAction.showQueryInsights.displayName == String(localized: "Query Insights")) - } -} - -@MainActor -struct ServerToolsMenuTests { - private func databaseItem(_ title: String) -> NSMenuItem? { - MainMenuBuilder.build(keyboard: KeyboardSettings()).bar - .items.first { $0.title == String(localized: "Database") }?.submenu? - .items.first { $0.title == title } - } - - @Test("Database > Server Dashboard carries its shortcut") - func serverDashboardItem() throws { - let item = try #require(databaseItem(String(localized: "Server Dashboard"))) - #expect(item.action == #selector(MainSplitViewController.showServerDashboard(_:))) - #expect(item.identifier == MenuItemFactory.identifier(for: .showServerDashboard)) - #expect(item.keyEquivalent == "d") - #expect(item.keyEquivalentModifierMask == [.command, .shift, .option]) - } - - @Test("Database > Query Insights carries its shortcut") - func queryInsightsItem() throws { - let item = try #require(databaseItem(String(localized: "Query Insights"))) - #expect(item.action == #selector(MainSplitViewController.showQueryInsights(_:))) - #expect(item.identifier == MenuItemFactory.identifier(for: .showQueryInsights)) - #expect(item.keyEquivalent == "i") - #expect(item.keyEquivalentModifierMask == [.command, .shift, .option]) - } - - @Test("A cleared binding leaves the item without a key equivalent") - func clearedBindingLeavesNoKey() throws { - var keyboard = KeyboardSettings() - keyboard.clearShortcut(for: .showServerDashboard) - let item = try #require( - MainMenuBuilder.build(keyboard: keyboard).bar - .items.first { $0.title == String(localized: "Database") }?.submenu? - .items.first { $0.title == String(localized: "Server Dashboard") } - ) - #expect(item.keyEquivalent.isEmpty) - } -} diff --git a/apple/TableProTests/Core/Services/ConnectionMenuPolicyTests.swift b/apple/TableProTests/Core/Services/ConnectionMenuPolicyTests.swift index c2da6ea2c0..cb86d88d66 100644 --- a/apple/TableProTests/Core/Services/ConnectionMenuPolicyTests.swift +++ b/apple/TableProTests/Core/Services/ConnectionMenuPolicyTests.swift @@ -22,19 +22,4 @@ struct ConnectionMenuPolicyTests { #expect(!ConnectionMenuPolicy.showsDisconnect(status: .connecting)) #expect(!ConnectionMenuPolicy.showsDisconnect(status: .error("refused"))) } - - @Test("Server Dashboard is offered for a live session on an engine that has one") - func serverDashboardNeedsLiveSessionAndEngine() { - #expect(ConnectionMenuPolicy.showsServerDashboard(status: .connected, type: .postgresql)) - #expect(!ConnectionMenuPolicy.showsServerDashboard(status: .connected, type: .sqlite)) - #expect(!ConnectionMenuPolicy.showsServerDashboard(status: .disconnected, type: .postgresql)) - #expect(!ConnectionMenuPolicy.showsServerDashboard(status: .error("refused"), type: .postgresql)) - } - - @Test("Query Insights is offered for any live session") - func queryInsightsNeedsLiveSession() { - #expect(ConnectionMenuPolicy.showsQueryInsights(status: .connected)) - #expect(!ConnectionMenuPolicy.showsQueryInsights(status: .connecting)) - #expect(!ConnectionMenuPolicy.showsQueryInsights(status: .disconnected)) - } } diff --git a/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift b/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift index 0aebd89ab2..e619785188 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift @@ -217,6 +217,57 @@ struct AgentModeWindowTests { #expect(harness.detailItem.minimumThickness == UsersRolesLayoutMetrics.tabMinimumWidth) } + // The browse tab stays selected behind the conversation, so Close and Undo must not reach it. + @Test("In Agent mode Close names the connection and Undo is the window's, not the hidden tab's") + func agentModeCloseAndUndoSkipTheHiddenTab() throws { + let harness = try Harness() + defer { harness.tearDown() } + try harness.requireContent() + let tabManager = try #require(harness.selected.sessionState?.tabManager) + tabManager.adoptTab(QueryTab(id: UUID(), title: "Users & Roles", query: "", tabType: .usersRoles)) + let undoManager = try #require(harness.window.undoManager) + undoManager.groupsByEvent = false + var undone = false + undoManager.beginUndoGrouping() + undoManager.registerUndo(withTarget: harness.window) { _ in undone = true } + undoManager.endUndoGrouping() + + #expect(harness.controller.closeCommandTitle == String(localized: "Close Tab")) + #expect(!harness.controller.menuValidationContext.canUndo, "Browsing asks the Users & Roles tab") + + harness.controller.setContentMode(.agent) + let title = try #require(harness.controller.closeCommandTitle) + #expect(title != String(localized: "Close Tab")) + #expect(harness.controller.menuValidationContext.canUndo) + #expect(harness.controller.undoTitle == undoManager.undoMenuItemTitle) + harness.controller.undo(nil) + #expect(undone) + } + + // The strip asks this of a row's connection before offering a command, so it has to read what the window reads. + @Test("A hosted connection's command facts match the window's own, and Agent mode offers no server tool") + func hostedContextMatchesTheWindow() throws { + let harness = try Harness(type: .postgresql) + defer { harness.tearDown() } + try harness.requireContent() + + for mode in ConnectionWorkspaceContentMode.allCases { + harness.controller.setContentMode(mode) + let own = harness.controller.menuValidationContext + let hosted = try #require(harness.controller.menuValidationContext(forHosted: harness.connection.id)) + #expect(hosted.isConnected == own.isConnected, "\(mode.rawValue)") + #expect(hosted.isAgentMode == own.isAgentMode, "\(mode.rawValue)") + #expect(hosted.supportsServerDashboard == own.supportsServerDashboard, "\(mode.rawValue)") + + let offered = WorkspaceRailViewController.serverTools(offeredWith: hosted).map(\.title) + let expected = mode == .browse + ? [String(localized: "Server Dashboard"), String(localized: "Query Insights")] + : [] + #expect(offered == expected, "\(mode.rawValue)") + } + #expect(harness.controller.menuValidationContext(forHosted: UUID()) == nil) + } + /// The error, Retry and Manage Connections live on the browse side's unavailable screen, and a /// composer with none of them is a dead end. A reconnect hands the column back. @Test("A connection that drops in Agent mode shows the unavailable screen, and a reconnect the conversation") diff --git a/apple/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift b/apple/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift index d760189a91..0c90fd6701 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/ConnectionActionsMenuResolverTests.swift @@ -108,6 +108,40 @@ struct ConnectionActionsMenuResolverTests { } } + // The delegate draws a row's chord only when the entry declares one. + @Test("Every command entry declares the shortcut its menu-bar twin carries") + @MainActor + func entriesCarryTheirTwinsShortcut() { + var twinShortcuts: [String: ShortcutAction] = [:] + collectShortcuts(from: MainMenuBuilder.build(keyboard: KeyboardSettings()).bar, into: &twinShortcuts) + + #expect(twinShortcuts.count > 20, "Only \(twinShortcuts.count) shortcuts collected; the walk missed the menus") + for tabKind in Self.tabKinds + [nil] { + for contentMode in ConnectionWorkspaceContentMode.allCases { + for isConnected in [true, false] { + let context = Self.context(tabKind: tabKind, contentMode: contentMode, isConnected: isConnected) + for entry in Self.entries(context) { + guard case let .command(_, shortcut) = entry.role else { continue } + #expect(shortcut == twinShortcuts[entry.title], "\(entry.title)") + } + } + } + } + } + + @MainActor + private func collectShortcuts(from menu: NSMenu, into shortcuts: inout [String: ShortcutAction]) { + let actions = Dictionary( + uniqueKeysWithValues: ShortcutAction.allCases.map { (MenuItemFactory.identifier(for: $0), $0) } + ) + for item in menu.items where !item.isSeparatorItem { + if let identifier = item.identifier, let action = actions[identifier] { + shortcuts[item.title] = action + } + if let submenu = item.submenu { collectShortcuts(from: submenu, into: &shortcuts) } + } + } + @MainActor private func collectTitles(from menu: NSMenu, into titles: inout Set) { for item in menu.items where !item.isSeparatorItem { diff --git a/apple/TableProTests/Core/Services/Infrastructure/MenuContentModeParityTests.swift b/apple/TableProTests/Core/Services/Infrastructure/MenuContentModeParityTests.swift index b982c55944..6f9f6df0fe 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/MenuContentModeParityTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/MenuContentModeParityTests.swift @@ -221,6 +221,57 @@ struct MenuContentModeParityTests { } } + // Each was lit in Agent mode and acted on the browse tab, grid, sidebar selection or sheet the mode hides. + @Test("Agent mode dims the commands that act on hidden browse content", arguments: [ + #selector(MainSplitViewController.createNewTable(_:)), + #selector(MainSplitViewController.showTableStructure(_:)), + #selector(MainSplitViewController.executeQuery(_:)), + #selector(MainSplitViewController.duplicateRow(_:)), + #selector(MainSplitViewController.copy(_:)), + #selector(MainSplitViewController.paste(_:)), + #selector(MainSplitViewController.delete(_:)), + #selector(MainSplitViewController.closeOtherTabs(_:)), + #selector(MainSplitViewController.closeAllTabs(_:)), + #selector(MainSplitViewController.goToNextPage(_:)), + #selector(MainSplitViewController.toggleFilterBar(_:)), + #selector(MainSplitViewController.focusResults(_:)), + #selector(MainSplitViewController.exportQueryResults(_:)), + #selector(MainSplitViewController.backupDatabase(_:)), + #selector(MainSplitViewController.createNewDatabase(_:)), + #selector(MainSplitViewController.editCurrentSchema(_:)), + #selector(MainSplitViewController.showQueryInsights(_:)), + ]) + func agentModeDimsHiddenContentCommands(command: Selector) { + var browse = Self.capableMenuContext(.browse) + var agent = Self.capableMenuContext(.agent) + browse.isAgentMode = false + agent.isAgentMode = true + #expect(MainSplitViewController.isEnabled(command, context: browse), "\(NSStringFromSelector(command)) is dim while browsing") + #expect(!MainSplitViewController.isEnabled(command, context: agent), "\(NSStringFromSelector(command)) stayed lit in Agent mode") + } + + // Everything a browse command could ask for, so only the mode can dim it. + private static func capableMenuContext(_ contentMode: ConnectionWorkspaceContentMode) -> MenuValidationContext { + var context = menuContext(contentMode) + context.canCreateTable = true + context.canCreateDatabase = true + context.canShowTableStructure = true + context.hasQueryText = true + context.hasRowSelection = true + context.hasTableSelection = true + context.hasResultRows = true + context.canPasteRows = true + context.canDropSelectedTables = true + context.canCloseOtherTabs = true + context.canCloseAllTabs = true + context.canNavigatePages = true + context.canUseTableResultCommands = true + context.canFocusResults = true + context.supportsBackup = true + context.canCreateSchema = true + return context + } + /// Named rather than left to the equality above, which two disabled answers would also satisfy. @Test("The window's own commands still answer in Agent mode") func theWindowsCommandsAnswerInAgentMode() { diff --git a/apple/TableProTests/Core/Services/Infrastructure/WorkspaceRailServerToolTests.swift b/apple/TableProTests/Core/Services/Infrastructure/WorkspaceRailServerToolTests.swift new file mode 100644 index 0000000000..b689db04d5 --- /dev/null +++ b/apple/TableProTests/Core/Services/Infrastructure/WorkspaceRailServerToolTests.swift @@ -0,0 +1,101 @@ +// +// WorkspaceRailServerToolTests.swift +// TableProTests +// + +import AppKit +@testable import TablePro +import Testing + +@MainActor +struct WorkspaceRailServerToolTests { + private static let dashboard = String(localized: "Server Dashboard") + private static let insights = String(localized: "Query Insights") + + private static func context( + isConnected: Bool = true, + isAgentMode: Bool = false, + supportsServerDashboard: Bool = true + ) -> MenuValidationContext { + var context = MenuValidationContext() + context.hasSelectedWorkspace = true + context.isConnected = isConnected + context.isAgentMode = isAgentMode + context.supportsServerDashboard = supportsServerDashboard + return context + } + + private static func offered(_ context: MenuValidationContext) -> [String] { + WorkspaceRailViewController.serverTools(offeredWith: context).map(\.title) + } + + @Test("A browsing connection gets both tools in the Database menu's order") + func browsingOffersBoth() { + #expect(Self.offered(Self.context()) == [Self.dashboard, Self.insights]) + } + + @Test("An engine without a dashboard gets Query Insights only") + func dashboardFollowsTheEngine() { + #expect(Self.offered(Self.context(supportsServerDashboard: false)) == [Self.insights]) + } + + @Test("Agent mode and a connection that is not live get neither") + func neitherWithoutBrowseContent() { + #expect(Self.offered(Self.context(isAgentMode: true)).isEmpty) + #expect(Self.offered(Self.context(isConnected: false)).isEmpty) + } + + @Test("Each tool runs the menu bar's own command") + func toolsRunTheMenuBarCommands() { + let commands = WorkspaceRailViewController.serverTools(offeredWith: Self.context()).map(\.command) + #expect(commands == [ + #selector(MainSplitViewController.showServerDashboard(_:)), + #selector(MainSplitViewController.showQueryInsights(_:)), + ]) + } + + private static func entry() -> WorkspaceRailEntry { + WorkspaceRailEntry( + workspace: WorkspaceID(connectionId: UUID(), container: ""), + connection: TestFixtures.makeConnection(name: "Primary", type: .postgresql), + status: .connected + ) + } + + private static func layout(of menu: NSMenu) -> [String] { + menu.items.map { $0.isSeparatorItem ? "---" : $0.title } + } + + @Test("The tools sit in their own group, targeted at the rail and naming the row, with no chord") + func toolItemsAreGroupedAndNameTheRow() { + let rail = WorkspaceRailViewController() + let entry = Self.entry() + let tools = WorkspaceRailViewController.serverTools(offeredWith: Self.context()) + let menu = NSMenu() + + rail.populate(menu, for: entry, serverTools: tools) + + let close = String(format: String(localized: "Close Connection “%@”"), entry.connection.name) + #expect(Self.layout(of: menu) == [ + Self.dashboard, Self.insights, "---", String(localized: "Disconnect"), "---", close, + ]) + for tool in tools { + let item = menu.items.first { $0.title == tool.title } + #expect(item?.action == tool.railAction) + #expect(item?.target === rail) + #expect(item?.representedObject as? WorkspaceID == entry.workspace) + #expect(item?.keyEquivalent.isEmpty == true, "\(tool.title) draws a chord the row cannot run") + } + } + + @Test("No tools leaves no empty group behind") + func noToolsNoSeparator() { + let entry = Self.entry() + let menu = NSMenu() + + WorkspaceRailViewController().populate(menu, for: entry, serverTools: []) + + let close = String(format: String(localized: "Close Connection “%@”"), entry.connection.name) + #expect(Self.layout(of: menu) == [String(localized: "Disconnect"), "---", close]) + } +} diff --git a/apple/TableProTests/Models/ShortcutUniquenessTests.swift b/apple/TableProTests/Models/ShortcutUniquenessTests.swift index 2b49c46e85..3bc64bbb25 100644 --- a/apple/TableProTests/Models/ShortcutUniquenessTests.swift +++ b/apple/TableProTests/Models/ShortcutUniquenessTests.swift @@ -76,6 +76,7 @@ struct ShortcutUniquenessTests { ShortcutAction.showTablesList, .showFavoritesList, .restorePreviousValues, .newAgentSession, .openAgentSession, .closeAgentSession, .deleteAgentSession, .newAIConversation, .copyPath, .copyRelativePath, .setCellsToValue, .pinTab, + .showServerDashboard, .showQueryInsights, ]) func newlyRebindableCommandsAreListable(action: ShortcutAction) { #expect(!action.displayName.isEmpty) diff --git a/apple/TableProTests/Views/AIChat/ChatComposerScrollViewTests.swift b/apple/TableProTests/Views/AIChat/ChatComposerScrollViewTests.swift index 1373f53339..d52199c44c 100644 --- a/apple/TableProTests/Views/AIChat/ChatComposerScrollViewTests.swift +++ b/apple/TableProTests/Views/AIChat/ChatComposerScrollViewTests.swift @@ -17,6 +17,36 @@ struct ChatComposerScrollViewTests { return scrollView } + // The window's Undo routes into tab histories, so the composer answers Undo with its own typing. + @Test("Typing undoes in the composer's own history, never the window's") + func typingUndoStaysInTheComposer() { + let textView = ChatComposerNSTextView.make() + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 300, height: 100), + styleMask: [.titled], + backing: .buffered, + defer: false + ) + window.isReleasedWhenClosed = false + window.contentView = textView + defer { window.orderOut(nil) } + + #expect(textView.undoManager !== window.undoManager) + textView.insertText("select 1", replacementRange: NSRange(location: 0, length: 0)) + textView.breakUndoCoalescing() + #expect(window.undoManager?.canUndo != true, "Typing landed in the window's history") + + let undo = NSMenuItem(title: "Undo", action: #selector(ChatComposerNSTextView.undo(_:)), keyEquivalent: "z") + #expect(textView.validateMenuItem(undo)) + textView.undo(nil) + #expect(textView.string.isEmpty) + + let redo = NSMenuItem(title: "Redo", action: #selector(ChatComposerNSTextView.redo(_:)), keyEquivalent: "Z") + #expect(textView.validateMenuItem(redo)) + textView.redo(nil) + #expect(textView.string == "select 1") + } + @Test("The text view width follows the scroll view width") func documentWidthFollowsScrollView() throws { let scrollView = makeComposer(width: 420) diff --git a/apple/changelog.d/3957.added.md b/apple/changelog.d/3957.added.md index 9b52776ef1..d7147478dc 100644 --- a/apple/changelog.d/3957.added.md +++ b/apple/changelog.d/3957.added.md @@ -1 +1 @@ -Server Dashboard and Query Insights open with Shift-Option-Command-D and -I, or from a connection's menu on the strip. +Server Dashboard and Query Insights open with ⌥⇧⌘D and ⌥⇧⌘I, or from a connection's menu on the connections strip. diff --git a/apple/changelog.d/3957.fixed.1.md b/apple/changelog.d/3957.fixed.1.md new file mode 100644 index 0000000000..cfa4997869 --- /dev/null +++ b/apple/changelog.d/3957.fixed.1.md @@ -0,0 +1 @@ +Undo in the AI chat composer undid a Structure, Users & Roles or ER diagram edit instead of the typing. diff --git a/apple/changelog.d/3957.fixed.md b/apple/changelog.d/3957.fixed.md new file mode 100644 index 0000000000..235620511c --- /dev/null +++ b/apple/changelog.d/3957.fixed.md @@ -0,0 +1 @@ +In Agent mode, menu commands and shortcuts acted on the hidden browse tabs, grid and sidebar. From 4a4d95dc85e3ef17438f54207c76a7ae4fd7402d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 1 Nov 2026 12:22:27 +0700 Subject: [PATCH 4/6] fix(agent-mode): give Agent mode its own undo history --- .../Infrastructure/ConnectionWorkspace.swift | 11 +++++-- ...nSplitViewController+EditMenuActions.swift | 14 ++++++--- ...inSplitViewController+MenuValidation.swift | 4 +-- .../Infrastructure/TabWindowController.swift | 7 ++--- .../Infrastructure/AgentModeWindowTests.swift | 31 ++++++++++++------- 5 files changed, 41 insertions(+), 26 deletions(-) diff --git a/apple/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift b/apple/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift index c0daa0a738..4f922750c2 100644 --- a/apple/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift +++ b/apple/TablePro/Core/Services/Infrastructure/ConnectionWorkspace.swift @@ -56,10 +56,14 @@ internal final class ConnectionWorkspace { /// left the mode with the second window's layout. internal var browseCollapseState: (sidebar: Bool, inspector: Bool)? - /// Each workspace owns its undo stack. Routing through `NSWindow.undoManager` was correct - /// while a window meant one connection; sharing one window between several would let an - /// undo in one connection roll back an edit made in another. + // Per workspace: one window hosts several connections, and a shared stack undid one connection's edit from another. internal let undoManager: UndoManager + // Agent mode's own history: the browse one holds edits to a grid the mode hides. + internal let agentUndoManager = UndoManager() + + internal var activeUndoManager: UndoManager { + resolvedContentMode == .agent ? agentUndoManager : undoManager + } /// This connection's own view tree, built once and kept. The window shows one workspace's panes /// at a time by swapping which of these is the split items' child. @@ -401,6 +405,7 @@ internal final class ConnectionWorkspace { sessionState = nil session = nil undoManager.removeAllActions() + agentUndoManager.removeAllActions() /// Once nothing hosts this connection any more, its agent sessions stop: a stream, a tool /// loop or a card waiting for an answer would otherwise keep running with nothing on screen. /// Deferred so this workspace has already left the window's registry when the check runs. diff --git a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+EditMenuActions.swift b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+EditMenuActions.swift index 261c0833e3..2ffdce2b29 100644 --- a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+EditMenuActions.swift +++ b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+EditMenuActions.swift @@ -10,10 +10,14 @@ extension MainSplitViewController: GridRowCopyCommandResponding {} // Fallback responders; paste: must be here, or AppKit disables Paste and the disabled item swallows Cmd+V window-wide. extension MainSplitViewController { - // Agent mode draws no tab history, so Undo runs the window's own manager, as `NSWindow.undo(_:)` would. + // Agent mode draws no tab or grid, so Undo runs the mode's own history, never the browse one behind it. + var agentUndoManager: UndoManager? { + contentMode == .agent ? workspaces.selected?.agentUndoManager : nil + } + @objc func undo(_ sender: Any?) { guard contentMode == .browse else { - view.window?.undoManager?.undo() + agentUndoManager?.undo() return } commandActions?.undoChange() @@ -21,7 +25,7 @@ extension MainSplitViewController { @objc func redo(_ sender: Any?) { guard contentMode == .browse else { - view.window?.undoManager?.redo() + agentUndoManager?.redo() return } commandActions?.redoChange() @@ -29,14 +33,14 @@ extension MainSplitViewController { var undoTitle: String { guard contentMode == .browse else { - return view.window?.undoManager?.undoMenuItemTitle ?? String(localized: "Undo") + return agentUndoManager?.undoMenuItemTitle ?? String(localized: "Undo") } return commandActions?.resolvedUndoTitle ?? String(localized: "Undo") } var redoTitle: String { guard contentMode == .browse else { - return view.window?.undoManager?.redoMenuItemTitle ?? String(localized: "Redo") + return agentUndoManager?.redoMenuItemTitle ?? String(localized: "Redo") } return commandActions?.resolvedRedoTitle ?? String(localized: "Redo") } diff --git a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift index 660d2c6a9e..3e1ab0a6c9 100644 --- a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift +++ b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift @@ -673,8 +673,8 @@ extension MainSplitViewController: NSMenuItemValidation { canSearchDatabase: actions.canSearchDatabase, canBuildJoinQuery: actions.canBuildJoinQuery, hasMaintenanceOperations: !actions.maintenanceOperations.isEmpty, - canUndo: contentMode == .browse ? actions.canUndo : view.window?.undoManager?.canUndo == true, - canRedo: contentMode == .browse ? actions.canRedo : view.window?.undoManager?.canRedo == true, + canUndo: contentMode == .browse ? actions.canUndo : agentUndoManager?.canUndo == true, + canRedo: contentMode == .browse ? actions.canRedo : agentUndoManager?.canRedo == true, hasEditorForFind: EditorEventRouter.shared.keyWindowHasEditor, hasSelectionForFind: EditorEventRouter.shared.keyWindowEditorHasSelectionForFind, hasActiveGridFind: actions.hasActiveGridFind, diff --git a/apple/TablePro/Core/Services/Infrastructure/TabWindowController.swift b/apple/TablePro/Core/Services/Infrastructure/TabWindowController.swift index c465b7fdcb..9a451ad053 100644 --- a/apple/TablePro/Core/Services/Infrastructure/TabWindowController.swift +++ b/apple/TablePro/Core/Services/Infrastructure/TabWindowController.swift @@ -237,13 +237,10 @@ internal final class TabWindowController: NSWindowController, NSWindowDelegate { Self.lifecycleLogger.debug("[switch] windowDidBecomeKey seq=\(seq) total ms=\(Int(Date().timeIntervalSince(t0) * 1_000))") } - /// `NSWindow.undoManager` resolves through here, so every undo domain that registers on the - /// window (the data grid's change manager, and the query editor's text view) lands in the - /// selected connection's own history. One window hosts every connection now, so sharing the - /// window's manager let Cmd+Z in one connection roll back an edit made in another. + // Every undo domain registering on the window lands in the selected connection's history for the mode on screen. internal func windowWillReturnUndoManager(_ window: NSWindow) -> UndoManager? { guard let host = window.contentViewController as? MainSplitViewController else { return nil } - return host.workspaces.selected?.undoManager + return host.workspaces.selected?.activeUndoManager } internal func windowDidResignKey(_ notification: Notification) { diff --git a/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift b/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift index e619785188..0828d5799f 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift @@ -217,31 +217,40 @@ struct AgentModeWindowTests { #expect(harness.detailItem.minimumThickness == UsersRolesLayoutMetrics.tabMinimumWidth) } - // The browse tab stays selected behind the conversation, so Close and Undo must not reach it. - @Test("In Agent mode Close names the connection and Undo is the window's, not the hidden tab's") + // The browse tab and grid stay behind the conversation, so Close and Undo must not reach them. + @Test("In Agent mode Close names the connection and Undo runs the mode's own history") func agentModeCloseAndUndoSkipTheHiddenTab() throws { let harness = try Harness() defer { harness.tearDown() } try harness.requireContent() let tabManager = try #require(harness.selected.sessionState?.tabManager) tabManager.adoptTab(QueryTab(id: UUID(), title: "Users & Roles", query: "", tabType: .usersRoles)) - let undoManager = try #require(harness.window.undoManager) - undoManager.groupsByEvent = false - var undone = false - undoManager.beginUndoGrouping() - undoManager.registerUndo(withTarget: harness.window) { _ in undone = true } - undoManager.endUndoGrouping() + var undoneBrowse = false + var undoneAgent = false + Self.register(on: harness.selected.undoManager) { undoneBrowse = true } #expect(harness.controller.closeCommandTitle == String(localized: "Close Tab")) - #expect(!harness.controller.menuValidationContext.canUndo, "Browsing asks the Users & Roles tab") + #expect(harness.selected.activeUndoManager === harness.selected.undoManager) harness.controller.setContentMode(.agent) let title = try #require(harness.controller.closeCommandTitle) #expect(title != String(localized: "Close Tab")) + #expect(harness.selected.activeUndoManager === harness.selected.agentUndoManager) + #expect(!harness.controller.menuValidationContext.canUndo, "A hidden browse edit lit Undo") + + Self.register(on: harness.selected.agentUndoManager) { undoneAgent = true } #expect(harness.controller.menuValidationContext.canUndo) - #expect(harness.controller.undoTitle == undoManager.undoMenuItemTitle) + #expect(harness.controller.undoTitle == harness.selected.agentUndoManager.undoMenuItemTitle) harness.controller.undo(nil) - #expect(undone) + #expect(undoneAgent) + #expect(!undoneBrowse) + } + + private static func register(on manager: UndoManager, _ action: @escaping @MainActor () -> Void) { + manager.groupsByEvent = false + manager.beginUndoGrouping() + manager.registerUndo(withTarget: manager) { _ in action() } + manager.endUndoGrouping() } // The strip asks this of a row's connection before offering a command, so it has to read what the window reads. From 6b47e8c004f633bfe0d41d5eec960cfccae66eee Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 1 Nov 2026 12:31:17 +0700 Subject: [PATCH 5/6] fix(connections): save every tab before closing a connection, and keep grid undo in its own history --- .../ConnectionCloseAction.swift | 18 ++++++----- .../MainContentCommandActions+BulkClose.swift | 22 +++++++++++-- .../MainContentCommandActions+UndoState.swift | 5 ++- .../Views/Main/MainContentCoordinator.swift | 8 ++++- .../Main/CommandActionsBulkCloseTests.swift | 31 +++++++++++++++++++ apple/changelog.d/3957.fixed.2.md | 1 + 6 files changed, 71 insertions(+), 14 deletions(-) create mode 100644 apple/changelog.d/3957.fixed.2.md diff --git a/apple/TablePro/Core/Services/Infrastructure/ConnectionCloseAction.swift b/apple/TablePro/Core/Services/Infrastructure/ConnectionCloseAction.swift index f7ebfbab3f..392900b057 100644 --- a/apple/TablePro/Core/Services/Infrastructure/ConnectionCloseAction.swift +++ b/apple/TablePro/Core/Services/Infrastructure/ConnectionCloseAction.swift @@ -25,22 +25,25 @@ internal enum ConnectionCloseAction { case confirmUnsavedWork } - /// Pure so the case that used to fail silently is pinned by a test: a connection with no session - /// has nothing to lose, and asking about it produced an alert nobody could answer. - /// Saves in every window hosting the connection, not only the one that answered first. Each - /// coordinator can save just its own selected tab's live work, so a Save that reached one of - /// them left the other window's grid edits behind and closed over them. + // Every window hosting the connection, every tab in each: saving only the selected tab closed over the rest. private static func saveEveryWindowsWork( across coordinators: [MainContentCoordinator], fallback: MainContentCoordinator? ) async -> Bool { let targets = coordinators.isEmpty ? [fallback].compactMap { $0 } : coordinators for coordinator in targets { - guard await coordinator.commandActions?.saveSelectedTabWork() == true else { return false } + guard await coordinator.commandActions?.saveEveryTabsWork() == true else { return false } } return !targets.isEmpty } + private static func unsavedWorkMessage(across coordinators: [MainContentCoordinator]) -> String { + let keepsTabsOpen = coordinators.contains { coordinator in + coordinator.commandActions?.holdsWorkOnlyItsTabCanSave(among: coordinator.tabManager.tabs) == true + } + return MainContentCommandActions.unsavedWorkMessage(keepsTabsOpen: keepsTabsOpen) + } + /// The transaction is asked about before unsaved work, whose Save writes at once: a Cancel on the /// transaction after it would keep the connection open with that write already made. internal static func decision(hasSession: Bool, holdsTransaction: Bool, hasUnsavedWork: Bool) -> Decision { @@ -106,8 +109,9 @@ internal enum ConnectionCloseAction { } let presentingWindow = reveal(connectionId: connectionId) + let targets = coordinators.isEmpty ? [coordinator].compactMap { $0 } : coordinators switch await AlertHelper.confirmSaveChanges( - message: String(localized: "Your changes will be lost if you don't save them."), + message: unsavedWorkMessage(across: targets), window: presentingWindow ) { case .save: diff --git a/apple/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift b/apple/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift index daf9947399..abda4f5937 100644 --- a/apple/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift +++ b/apple/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift @@ -96,9 +96,16 @@ extension MainContentCommandActions { // Save cannot reach off-screen grid edits, staged principals or table drafts; those tabs stay open and the alert says so. private func unsavedWorkMessage(for victims: [QueryTab]) -> String { - guard let coordinator, - victims.contains(where: { coordinator.savability(of: $0) == .mountedOnly }) - else { + Self.unsavedWorkMessage(keepsTabsOpen: holdsWorkOnlyItsTabCanSave(among: victims)) + } + + func holdsWorkOnlyItsTabCanSave(among victims: [QueryTab]) -> Bool { + guard let coordinator else { return false } + return victims.contains { coordinator.savability(of: $0) == .mountedOnly } + } + + static func unsavedWorkMessage(keepsTabsOpen: Bool) -> String { + guard keepsTabsOpen else { return String(localized: "Your changes will be lost if you don't save them.") } return String( @@ -109,6 +116,15 @@ extension MainContentCommandActions { ) } + // Closing the connection takes every tab, so Save has to reach each one; false keeps the connection open. + func saveEveryTabsWork() async -> Bool { + guard let coordinator else { return true } + coordinator.commitActiveGridEdit() + let tabs = coordinator.tabManager.tabs + let saved = await saveVictims(tabs) + return tabs.allSatisfy { saved.contains($0.id) } && !coordinator.hasAnyUnsavedWork() + } + // A victim it cannot save keeps its tab, a file conflict too: the conflict sheet is one window-level slot with no queue. private func saveVictims(_ victims: [QueryTab]) async -> Set { guard let coordinator else { return Set(victims.map(\.id)) } diff --git a/apple/TablePro/Views/Main/MainContentCommandActions+UndoState.swift b/apple/TablePro/Views/Main/MainContentCommandActions+UndoState.swift index f35e03c609..b3f291f407 100644 --- a/apple/TablePro/Views/Main/MainContentCommandActions+UndoState.swift +++ b/apple/TablePro/Views/Main/MainContentCommandActions+UndoState.swift @@ -20,11 +20,10 @@ extension StructureEditingSession: StructureUndoHistory {} extension CreateTableDraft: StructureUndoHistory {} -// Users & Roles, the diagram and the structure editor keep their own histories; everything else registers on the -// window's undo manager. Availability, title and command resolve through the same branch so they cannot disagree. +// Users & Roles, the diagram and the structure editor keep their own histories; everything else uses the browse one. extension MainContentCommandActions { private var windowUndoManager: UndoManager? { - coordinator?.contentWindow?.undoManager + coordinator?.browseUndoManager } // A Create Table tab keeps its view mode at Data, so it is asked for by tab type. diff --git a/apple/TablePro/Views/Main/MainContentCoordinator.swift b/apple/TablePro/Views/Main/MainContentCoordinator.swift index f00eefa556..673b4f9378 100644 --- a/apple/TablePro/Views/Main/MainContentCoordinator.swift +++ b/apple/TablePro/Views/Main/MainContentCoordinator.swift @@ -240,6 +240,12 @@ final class MainContentCoordinator: ObservableObject { // For alerts instead of NSApp.keyWindow, which may return a sheet window and leave dialogs stuck. weak var contentWindow: NSWindow? + // Its own workspace's browse history: the window's answer follows the connection and mode on screen. + var browseUndoManager: UndoManager? { + let host = contentWindow?.contentViewController as? MainSplitViewController + return host?.workspaces.workspace(for: connectionId)?.undoManager ?? contentWindow?.undoManager + } + // No SwiftUI `Scene` in the AppKit lifecycle, so a focused value can't resolve; this reaches AppKit and SwiftUI callers. weak var commandActions: MainContentCommandActions? @@ -602,7 +608,7 @@ final class MainContentCoordinator: ObservableObject { self.persistence = TabPersistenceCoordinator.forConnection(connection.id) ConnectionDataCache.shared(for: connection.id).ensureLoaded() - changeManager.undoManagerProvider = { [weak self] in self?.contentWindow?.undoManager } + changeManager.undoManagerProvider = { [weak self] in self?.browseUndoManager } changeManager.onUndoApplied = { [weak self] result in self?.handleUndoResult(result) } tabManager.onVisibleContainerReconciled = { [weak self] in self?.browseCursorCoordinator.followVisibleContainer() diff --git a/apple/TableProTests/Views/Main/CommandActionsBulkCloseTests.swift b/apple/TableProTests/Views/Main/CommandActionsBulkCloseTests.swift index 313f4300ca..762910ee39 100644 --- a/apple/TableProTests/Views/Main/CommandActionsBulkCloseTests.swift +++ b/apple/TableProTests/Views/Main/CommandActionsBulkCloseTests.swift @@ -289,6 +289,37 @@ struct CommandActionsBulkCloseTests { #expect(current.coordinator.savability(of: selected) == .nothingAtRisk) } + // Closing the connection used to save only the selected tab, then close over a background tab's edits. + @Test("closing the connection keeps it open when a background tab holds work only that tab can save") + func connectionSaveRefusesUnreachableBackgroundWork() async throws { + let connection = TestFixtures.makeConnection(database: "db_a") + let current = makeWindow(connection: connection) + defer { current.coordinator.teardown() } + + current.coordinator.tabManager.addTab(initialQuery: "SELECT 1", databaseName: "db_a") + current.coordinator.tabManager.addTab(initialQuery: "SELECT 2", databaseName: "db_a") + let background = try #require(current.coordinator.tabManager.tabs.first) + current.coordinator.tabManager.mutate(tabId: background.id) { tab in + tab.pendingChanges.deletedRowIDs = [.existing(0)] + } + + #expect(current.actions.holdsWorkOnlyItsTabCanSave(among: current.coordinator.tabManager.tabs)) + #expect(await current.actions.saveEveryTabsWork() == false) + } + + @Test("closing a clean connection saves nothing and lets it close") + func connectionSaveOfCleanTabsCloses() async { + let connection = TestFixtures.makeConnection(database: "db_a") + let current = makeWindow(connection: connection) + defer { current.coordinator.teardown() } + + current.coordinator.tabManager.addTab(initialQuery: "SELECT 1", databaseName: "db_a") + current.coordinator.tabManager.addTab(initialQuery: "SELECT 2", databaseName: "db_a") + + #expect(!current.actions.holdsWorkOnlyItsTabCanSave(among: current.coordinator.tabManager.tabs)) + #expect(await current.actions.saveEveryTabsWork()) + } + @Test("a clean batch closes without asking") func cleanBatchNeedsNoPrompt() async { let connection = TestFixtures.makeConnection(database: "db_a") diff --git a/apple/changelog.d/3957.fixed.2.md b/apple/changelog.d/3957.fixed.2.md new file mode 100644 index 0000000000..cc1c86e51b --- /dev/null +++ b/apple/changelog.d/3957.fixed.2.md @@ -0,0 +1 @@ +Close Connection with Save closed over unsaved edits in tabs other than the one in front. From 4333eac01ee4935fb99db46208cb525d2088ff91 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sun, 1 Nov 2026 12:39:02 +0700 Subject: [PATCH 6/6] fix(connections): wait for inspector edits to save before closing --- .../MainContentCommandActions+BulkClose.swift | 19 ++++++- .../Main/MainContentCommandActions.swift | 49 +++++++------------ apple/changelog.d/3957.fixed.3.md | 1 + 3 files changed, 38 insertions(+), 31 deletions(-) create mode 100644 apple/changelog.d/3957.fixed.3.md diff --git a/apple/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift b/apple/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift index abda4f5937..30d27f936c 100644 --- a/apple/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift +++ b/apple/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift @@ -61,6 +61,22 @@ extension MainContentCommandActions { ) } + // What Close does with no tab on screen: the container, else the connection. + func closeWorkspace() { + guard let coordinator else { + Task { await closeWindowAwaiting() } + return + } + if let entry = WorkspaceCloseAction.closableEntry( + connectionId: connectionId, + container: coordinator.tabManager.visibleContainer + ) { + Task { await WorkspaceCloseAction.close(entry.workspace) } + return + } + Task { await ConnectionCloseAction.close(connectionId: connectionId) } + } + // A list edit, not a window close: the window hosts every connection, so closing all tabs leaves this one empty. private func runBatchClose(kind: BatchCloseKind) async { guard let coordinator else { return } @@ -121,8 +137,9 @@ extension MainContentCommandActions { guard let coordinator else { return true } coordinator.commitActiveGridEdit() let tabs = coordinator.tabManager.tabs + guard !tabs.isEmpty else { return await saveSelectedTabWork() } let saved = await saveVictims(tabs) - return tabs.allSatisfy { saved.contains($0.id) } && !coordinator.hasAnyUnsavedWork() + return tabs.allSatisfy { saved.contains($0.id) } } // A victim it cannot save keeps its tab, a file conflict too: the conflict sheet is one window-level slot with no queue. diff --git a/apple/TablePro/Views/Main/MainContentCommandActions.swift b/apple/TablePro/Views/Main/MainContentCommandActions.swift index e20a3b09c9..2b94e7f5ac 100644 --- a/apple/TablePro/Views/Main/MainContentCommandActions.swift +++ b/apple/TablePro/Views/Main/MainContentCommandActions.swift @@ -139,18 +139,25 @@ final class MainContentCommandActions: ObservableObject { // MARK: - Save Action private func saveInspectorEdits() { - let editState = trailingPaneState.inspector.editState Task { [weak self] in - guard let self else { return } - do { - try await self.coordinator?.saveSidebarEdits(editState: editState) - } catch { - AlertHelper.showErrorSheet( - title: String(localized: "Failed to Save Changes"), - message: error.localizedDescription, - window: self.window - ) - } + await self?.saveInspectorEditsAwaiting() + } + } + + // A close waits on this: started and left, the write could land after the window it reports to closed. + @discardableResult + private func saveInspectorEditsAwaiting() async -> Bool { + let editState = trailingPaneState.inspector.editState + do { + try await coordinator?.saveSidebarEdits(editState: editState) + return true + } catch { + AlertHelper.showErrorSheet( + title: String(localized: "Failed to Save Changes"), + message: error.localizedDescription, + window: window + ) + return false } } @@ -670,22 +677,6 @@ final class MainContentCommandActions: ObservableObject { closeWorkspace() } - // What Close does with no tab on screen: the container, else the connection. - func closeWorkspace() { - guard let coordinator else { - Task { await closeWindowAwaiting() } - return - } - if let entry = WorkspaceCloseAction.closableEntry( - connectionId: connectionId, - container: coordinator.tabManager.visibleContainer - ) { - Task { await WorkspaceCloseAction.close(entry.workspace) } - return - } - Task { await ConnectionCloseAction.close(connectionId: connectionId) } - } - // asBatchSurvivor: nil for a lone close (the window decides), true for the window a batch keeps blank, false otherwise. @discardableResult func closeWindowAwaiting(asBatchSurvivor: Bool? = nil) async -> WindowCloseOutcome { @@ -803,10 +794,8 @@ final class MainContentCommandActions: ObservableObject { } } - // Sidebar-only edits (made directly in the inspector panel) if trailingPaneState.inspector.editState.hasEdits { - saveInspectorEdits() - return true + return await saveInspectorEditsAwaiting() } // File save (query editor with source file) diff --git a/apple/changelog.d/3957.fixed.3.md b/apple/changelog.d/3957.fixed.3.md new file mode 100644 index 0000000000..ef45f88f84 --- /dev/null +++ b/apple/changelog.d/3957.fixed.3.md @@ -0,0 +1 @@ +Save at the close prompt closed before inspector edits were written, so a failed write went unreported.