diff --git a/apple/TablePro/Core/Menu/DatabaseMenuBuilder.swift b/apple/TablePro/Core/Menu/DatabaseMenuBuilder.swift index e165f2b8f7..fa759c6bfe 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(_:)) @@ -133,7 +126,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,16 +136,16 @@ enum DatabaseMenuBuilder { ), MenuItemFactory.item( String(localized: "Query Insights"), - action: #selector(MainSplitViewController.showQueryInsights(_:)) + action: #selector(MainSplitViewController.showQueryInsights(_:)), + shortcut: .showQueryInsights, + keyboard: keyboard ), MenuItemFactory.item( String(localized: "Table Health"), action: #selector(MainSplitViewController.showTableHealth(_:)) ), 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/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/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 a44b67e21c..2ffdce2b29 100644 --- a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+EditMenuActions.swift +++ b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+EditMenuActions.swift @@ -10,14 +10,41 @@ 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 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 { + agentUndoManager?.undo() + return + } commandActions?.undoChange() } @objc func redo(_ sender: Any?) { + guard contentMode == .browse else { + agentUndoManager?.redo() + return + } commandActions?.redoChange() } + var undoTitle: String { + guard contentMode == .browse else { + return agentUndoManager?.undoMenuItemTitle ?? String(localized: "Undo") + } + return commandActions?.resolvedUndoTitle ?? String(localized: "Undo") + } + + var redoTitle: String { + guard contentMode == .browse else { + return agentUndoManager?.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 a6ee87fe59..40071b509b 100644 --- a/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift +++ b/apple/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift @@ -147,7 +147,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 } @@ -385,6 +390,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 } @@ -446,39 +454,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(showUsersAndRoles(_:)), - #selector(showQueryInsights(_:)), - #selector(showTableHealth(_:)), - #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. @@ -564,6 +569,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 @@ -661,8 +678,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 : agentUndoManager?.canUndo == true, + canRedo: contentMode == .browse ? actions.canRedo : agentUndoManager?.canRedo == true, hasEditorForFind: EditorEventRouter.shared.keyWindowHasEditor, hasSelectionForFind: EditorEventRouter.shared.keyWindowEditorHasSelectionForFind, hasActiveGridFind: actions.hasActiveGridFind, @@ -744,9 +761,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(_:)): @@ -800,6 +817,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/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/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 24da7edd8b..8c30c37b00 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 681ef0ce62..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,6 +611,48 @@ internal final class WorkspaceRailViewController: NSViewController { WindowManager.shared.moveToNewWindow(connectionId: workspace.connectionId) } + // 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) { + runServerTool(#selector(MainSplitViewController.showServerDashboard(_:)), from: sender) + } + + @objc + private func showWorkspaceQueryInsights(_ sender: NSMenuItem) { + runServerTool(#selector(MainSplitViewController.showQueryInsights(_:)), from: sender) + } + + // 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 private func closeWorkspace(_ sender: NSMenuItem) { guard let workspace = sender.representedObject as? WorkspaceID else { return } @@ -727,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, @@ -746,6 +782,14 @@ extension WorkspaceRailViewController: NSMenuDelegate { menu.addItem(.separator()) } + // 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 ) { diff --git a/apple/TablePro/Models/UI/KeyboardShortcutModels.swift b/apple/TablePro/Models/UI/KeyboardShortcutModels.swift index 79b0e75513..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 @@ -67,6 +66,8 @@ enum ShortcutAction: String, Codable, CaseIterable, Identifiable { case openDatabase case switchConnection case moveTabToConnection + case showServerDashboard + case showQueryInsights // Editor & Query case openFile @@ -194,7 +195,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 +304,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") @@ -654,14 +658,15 @@ 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), .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/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+BulkClose.swift b/apple/TablePro/Views/Main/MainContentCommandActions+BulkClose.swift index daf9947399..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 } @@ -96,9 +112,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 +132,16 @@ 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 + guard !tabs.isEmpty else { return await saveSelectedTabWork() } + let saved = await saveVictims(tabs) + 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. 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/MainContentCommandActions.swift b/apple/TablePro/Views/Main/MainContentCommandActions.swift index 9db904f8a0..ff81dffedd 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 } } @@ -677,14 +684,7 @@ final class MainContentCommandActions: ObservableObject { closeTab(id: selected.id) 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) } + closeWorkspace() } // asBatchSurvivor: nil for a lone close (the window decides), true for the window a batch keeps blank, false otherwise. @@ -804,10 +804,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/TablePro/Views/Main/MainContentCoordinator.swift b/apple/TablePro/Views/Main/MainContentCoordinator.swift index ed69bb516c..5c95425d6b 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/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/MainMenuBuilderTests.swift b/apple/TableProTests/Core/Menu/MainMenuBuilderTests.swift index a05e9c6725..cf97f14665 100644 --- a/apple/TableProTests/Core/Menu/MainMenuBuilderTests.swift +++ b/apple/TableProTests/Core/Menu/MainMenuBuilderTests.swift @@ -786,7 +786,6 @@ struct MainMenuValidationTests { context.canInsertRows = true context.canDeleteRows = true context.supportsTableHealth = true - context.isCurrentTabEditable = true context.isCurrentTabSchemaResolved = true context.hasTableSelection = true context.hasRowSelection = true @@ -911,6 +910,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/Services/Infrastructure/AgentModeWindowTests.swift b/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift index 0aebd89ab2..0828d5799f 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/AgentModeWindowTests.swift @@ -217,6 +217,66 @@ struct AgentModeWindowTests { #expect(harness.detailItem.minimumThickness == UsersRolesLayoutMetrics.tabMinimumWidth) } + // 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)) + var undoneBrowse = false + var undoneAgent = false + Self.register(on: harness.selected.undoManager) { undoneBrowse = true } + + #expect(harness.controller.closeCommandTitle == String(localized: "Close 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 == harness.selected.agentUndoManager.undoMenuItemTitle) + harness.controller.undo(nil) + #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. + @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/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) { diff --git a/apple/TableProTests/Core/Services/Infrastructure/MenuContentModeParityTests.swift b/apple/TableProTests/Core/Services/Infrastructure/MenuContentModeParityTests.swift index 95af34205e..0c9871d298 100644 --- a/apple/TableProTests/Core/Services/Infrastructure/MenuContentModeParityTests.swift +++ b/apple/TableProTests/Core/Services/Infrastructure/MenuContentModeParityTests.swift @@ -222,6 +222,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/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.added.md b/apple/changelog.d/3957.added.md new file mode 100644 index 0000000000..d7147478dc --- /dev/null +++ b/apple/changelog.d/3957.added.md @@ -0,0 +1 @@ +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.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. 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. 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.