Skip to content

Commit bfcbf58

Browse files
authored
refactor(editor): rebuild the query tab around pure presentation models (#2892)
1 parent 1593910 commit bfcbf58

51 files changed

Lines changed: 2637 additions & 931 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3030
- Row grid for data Compare & Sync with every column shown and each differing value marked. (#2537)
3131
- Acknowledgements entries for the four tree-sitter grammars the SQL editor ships.
3232
- **Edit > Find > Find and Replace…** (`Cmd+Option+F`) and **Use Selection for Find** (`Cmd+E`) in the SQL editor.
33+
- **Run** split button in the query editor, with Run All Statements, Run Without Limit, Clear Query and Clear Results on its menu.
34+
- **Stop** in the query editor while a query is running.
35+
- **Query > Clear Query** and **Query > Clear Results**.
36+
- Result chooser in the status bar, naming the result on screen and offering Pin, Unpin, Close and Close Others.
37+
- A reason on a dimmed Run, Explain, Format or Favorite saying why it cannot run.
3338

3439
### Changed
3540

@@ -50,14 +55,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5055
- Connection rows without colored dots, on the Mac and on iOS.
5156
- SQL Server sessions open with the ANSI SET profile the server requires, matching every other client.
5257
- Compared columns in data Compare & Sync chosen per table, and saved with each table's key, filter and row limit. (#2537)
58+
- Query history opens in the trailing pane beside the inspector and the assistant, with its entry list above its detail.
59+
- Query editor command bar with one control size, the container picker leading and the commands trailing.
5360

5461
### Removed
5562

5663
- CodeEditSymbols, a dependency the editor linked and never called, from the app and from Acknowledgements.
5764
- `Ctrl+Cmd+J` from the editor's reserved shortcuts, so it can be bound in Settings > Keyboard.
65+
- Result tab strip above the query results, and the "Query" heading above the editor.
66+
- Trash button that cleared the query and the results under one name.
67+
- Query history drawer under the editor and results.
5868

5969
### Fixed
6070

71+
- Stale error banner over a pinned result after clearing the results of a failed query.
6172
- Imported connections pointing at an SSH profile that is not on the importing Mac.
6273
- Syntax highlighting falling a second or two behind while typing quickly in the SQL editor.
6374
- Beep and a question-mark badge when pressing `Ctrl+Cmd+J` in the SQL editor.

TablePro/Core/Menu/QueryMenuBuilder.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,19 @@ enum QueryMenuBuilder {
3434
keyboard: keyboard
3535
),
3636
MenuItemFactory.separator,
37+
/// Two commands because they are two effects. The editor's trash button did both under
38+
/// the single name "Clear Query", so neither was announced for what it was and neither
39+
/// had a menu-bar route, which is what the HIG requires before a toolbar item may carry
40+
/// it. They take no default shortcut: both are destructive and infrequent.
41+
MenuItemFactory.item(
42+
String(localized: "Clear Query"),
43+
action: #selector(MainSplitViewController.clearQuery(_:))
44+
),
45+
MenuItemFactory.item(
46+
String(localized: "Clear Results"),
47+
action: #selector(MainSplitViewController.clearResults(_:))
48+
),
49+
MenuItemFactory.separator,
3750
MenuItemFactory.item(
3851
String(localized: "Explain Query"),
3952
action: #selector(MainSplitViewController.explainQuery(_:)),

TablePro/Core/Services/Infrastructure/MainSplitViewController+QueryMenuActions.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,14 @@ extension MainSplitViewController {
6666
commandActions?.saveAsFavorite()
6767
}
6868

69+
@objc func clearQuery(_ sender: Any?) {
70+
commandActions?.clearQuery()
71+
}
72+
73+
@objc func clearResults(_ sender: Any?) {
74+
commandActions?.clearResults()
75+
}
76+
6977
@objc func previewFKReference(_ sender: Any?) {
7078
commandActions?.previewFKReference()
7179
}

TablePro/Core/Services/Infrastructure/MainSplitViewController.swift

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,7 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan
717717
workspace.panes.detail.rootView = AnyView(buildDetailView(for: workspace))
718718
workspace.panes.inspector.rootView = AnyView(buildInspectorView(for: workspace))
719719
workspace.panes.assistant.rootView = AnyView(buildAssistantView(for: workspace))
720+
workspace.panes.history.rootView = AnyView(buildHistoryView(for: workspace))
720721
refreshTabStripPane(of: workspace)
721722
workspace.panes.markRendered(workspace.paneRenderKey)
722723
guard isShowing(workspace) else { return }
@@ -897,6 +898,16 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan
897898
}
898899
}
899900

901+
@ViewBuilder
902+
private func buildHistoryView(for workspace: ConnectionWorkspace) -> some View {
903+
if workspace.resolvedPane == .content,
904+
let coordinator = workspace.sessionState?.coordinator {
905+
HistoryPanelView(coordinator: coordinator)
906+
} else {
907+
TrailingPaneUnavailableView(surface: .history)
908+
}
909+
}
910+
900911
/// Rebuilds the trailing surfaces alone. `commandActions` is read eagerly by both, and it only
901912
/// exists once the detail pane has appeared, which is after `rebuildPanes()` has already built
902913
/// them against a nil value. Rebuilding the detail pane too would remount the very view that
@@ -905,6 +916,7 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan
905916
guard let selected = workspaces.selected else { return }
906917
selected.panes.inspector.rootView = AnyView(buildInspectorView(for: selected))
907918
selected.panes.assistant.rootView = AnyView(buildAssistantView(for: selected))
919+
selected.panes.history.rootView = AnyView(buildHistoryView(for: selected))
908920
}
909921

910922
/// Parents whichever surface the selected workspace is showing.
@@ -1006,6 +1018,10 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan
10061018
isTrailingPaneOpen && resolvedTrailingSurface == .assistant
10071019
}
10081020

1021+
var isHistoryVisible: Bool {
1022+
isTrailingPaneOpen && resolvedTrailingSurface == .history
1023+
}
1024+
10091025
func showInspector() {
10101026
reveal(.inspector)
10111027
}
@@ -1015,6 +1031,10 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan
10151031
reveal(.assistant)
10161032
}
10171033

1034+
func showHistory() {
1035+
reveal(.history)
1036+
}
1037+
10181038
/// Auto-show follows a grid click, which is not a request for a different surface. Revealing
10191039
/// the inspector unconditionally swapped the assistant out from under a half-typed question and
10201040
/// persisted the inspector as that connection's surface, on every row the user clicked.
@@ -1025,9 +1045,25 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan
10251045

10261046
func hideTrailingPane() {
10271047
inspectorSplitItem?.animator().isCollapsed = true
1048+
syncHistoryPanelVisibility()
10281049
recomputeWindowMinSize()
10291050
}
10301051

1052+
/// Keeps `HistoryPanelState.isVisible` saying what the window is actually showing.
1053+
///
1054+
/// The flag is not redundant with the surface. It is what `HistoryPanelView`'s `.task(id:)`
1055+
/// keys its activation on, so the view model only builds and starts querying while history is
1056+
/// on screen, and it is what the Find Past Queries tip reads to know it has been answered.
1057+
/// Leaving it behind when history moved into the trailing pane would have left the panel
1058+
/// mounted and inert, which is the shape of a pane that renders nothing forever.
1059+
private func syncHistoryPanelVisibility() {
1060+
guard let connectionId = workspaces.selected?.connectionId else { return }
1061+
let showing = isTrailingPaneOpen && resolvedTrailingSurface == .history
1062+
let state = HistoryPanelState.forConnection(connectionId)
1063+
guard state.isVisible != showing else { return }
1064+
state.isVisible = showing
1065+
}
1066+
10311067
/// Puts the hosted child back in step with what the settings now allow.
10321068
///
10331069
/// The stored surface is left alone: a user who turns the assistant off and on again gets it
@@ -1044,6 +1080,7 @@ internal final class MainSplitViewController: NSSplitViewController, TrailingPan
10441080
rebuildTrailingPanes()
10451081
showSelectedTrailingPane()
10461082
inspectorSplitItem?.animator().isCollapsed = false
1083+
syncHistoryPanelVisibility()
10471084
recomputeWindowMinSize()
10481085
}
10491086

TablePro/Core/Services/Infrastructure/MainWindowToolbar.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate {
233233
_ = self?.coordinator?.toolbarState.hasDataPendingChanges
234234
_ = self?.coordinator?.toolbarState.safeModeLevel
235235
_ = self?.coordinator?.toolbarState.currentDatabase
236+
_ = self?.coordinator?.toolbarState.isQueryTab
236237
} onChange: { [weak self] in
237238
Task { @MainActor [weak self] in
238239
guard let self,
@@ -365,6 +366,7 @@ internal final class MainWindowToolbar: NSObject, NSToolbarDelegate {
365366
/// `addRow`, `restorePreviousValues`, `quickSwitcher` and `newTab` are absent on purpose: they
366367
/// ride a group as subitems and the delegate vends no standalone item for any of them, so
367368
/// listing one here would offer the customization palette a tile it cannot build.
369+
///
368370
internal static let allowedItemIdentifiers: [NSToolbarItem.Identifier] = defaultItemIdentifiers + [
369371
previewSQL,
370372
results,

TablePro/Core/Services/Infrastructure/TrailingPaneProxy.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ import Foundation
1717
internal protocol TrailingPaneProxy: AnyObject {
1818
var isInspectorVisible: Bool { get }
1919
var isAssistantVisible: Bool { get }
20+
var isHistoryVisible: Bool { get }
2021
func showInspector()
2122
func showAssistant()
23+
func showHistory()
2224
func hideTrailingPane()
2325

2426
/// Reveals the inspector for a selection the user made somewhere else, and only if that does
@@ -37,4 +39,8 @@ internal extension TrailingPaneProxy {
3739
func toggleAssistant() {
3840
if isAssistantVisible { hideTrailingPane() } else { showAssistant() }
3941
}
42+
43+
func toggleHistory() {
44+
if isHistoryVisible { hideTrailingPane() } else { showHistory() }
45+
}
4046
}

TablePro/Core/Services/Infrastructure/WorkspacePanes.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ internal final class WorkspacePanes {
5353
/// only way it gets the `sizingOptions` firewall below, which is applied here and nowhere else.
5454
internal let assistant: NSHostingController<AnyView>
5555

56+
/// Query history, on the same terms as the assistant: its own controller so the list's scroll
57+
/// position, the selected entry and a half-typed search survive the reader looking at a row.
58+
internal let history: NSHostingController<AnyView>
59+
5660
internal let sidebar: NSHostingController<AnyView>
5761
/// The editor tab strip. It is a pane like the other three, built and kept alive per
5862
/// connection, even though the window shows it in the titlebar accessory rather than in a
@@ -72,6 +76,7 @@ internal final class WorkspacePanes {
7276
detail = NSHostingController(rootView: AnyView(Color.clear))
7377
inspector = NSHostingController(rootView: AnyView(Color.clear))
7478
assistant = NSHostingController(rootView: AnyView(Color.clear))
79+
history = NSHostingController(rootView: AnyView(Color.clear))
7580
sidebar = NSHostingController(rootView: AnyView(Color.clear))
7681
tabStrip = EditorTabStripPaneController()
7782
for pane in panes {
@@ -80,7 +85,7 @@ internal final class WorkspacePanes {
8085
}
8186

8287
private var panes: [NSHostingController<AnyView>] {
83-
[detail, inspector, assistant, sidebar]
88+
[detail, inspector, assistant, history, sidebar]
8489
}
8590

8691
/// The controller a trailing surface is drawn by. One split item hosts whichever of these the
@@ -89,6 +94,7 @@ internal final class WorkspacePanes {
8994
switch surface {
9095
case .inspector: inspector
9196
case .assistant: assistant
97+
case .history: history
9298
}
9399
}
94100

TablePro/Models/Connection/ConnectionToolbarState.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ final class ConnectionToolbarState {
129129
/// Whether the current editor has non-empty query text
130130
var hasQueryText: Bool = false
131131

132+
/// Whether the selected tab is a query tab. `isTableTab` cannot answer this: a structure,
133+
/// dashboard or diagram tab is neither, and the Run item has to be disabled on all of them.
134+
var isQueryTab: Bool = false
135+
132136
/// SQL statements rendered in the SQL preview sheet
133137
var previewStatements: [String] = []
134138

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
//
2+
// QueryCommandAvailability.swift
3+
// TablePro
4+
//
5+
6+
import Foundation
7+
import TableProPluginKit
8+
9+
/// What the query tab's command bar can do right now.
10+
///
11+
/// Pure, the way `ResultStatusModel` and `QueryResultPresentation` are, so the whole enable matrix
12+
/// is decidable without mounting a view. The bar used to answer this inline: `disabled(!hasQuery)`
13+
/// written out at four call sites, each with its own idea of what "has a query" meant, and Format
14+
/// with no gate at all.
15+
struct QueryCommandAvailability {
16+
let canRun: Bool
17+
let canStop: Bool
18+
let canExplain: Bool
19+
let canFormat: Bool
20+
let canSaveAsFavorite: Bool
21+
let canClearQuery: Bool
22+
let canClearResults: Bool
23+
let explainVariants: [ExplainVariant]
24+
25+
/// Every hint the bar shows, resolved here so a disabled control can say why rather than just
26+
/// dimming. A control that dims without explaining is the one thing a reader cannot act on.
27+
let runHint: String
28+
let stopHint: String
29+
let explainHint: String
30+
let formatHint: String
31+
let favoriteHint: String
32+
33+
init(
34+
isConnected: Bool,
35+
hasQueryText: Bool,
36+
isExecuting: Bool,
37+
hasResults: Bool,
38+
explainVariants: [ExplainVariant],
39+
shortcutHint: (String, ShortcutAction) -> String
40+
) {
41+
self.explainVariants = explainVariants
42+
canRun = isConnected && hasQueryText && !isExecuting
43+
canStop = isExecuting
44+
canExplain = isConnected && hasQueryText && !isExecuting && !explainVariants.isEmpty
45+
/// Formatting rewrites text the reader already has, so it does not wait for a server.
46+
canFormat = hasQueryText
47+
canSaveAsFavorite = hasQueryText
48+
canClearQuery = hasQueryText
49+
canClearResults = hasResults
50+
51+
runHint = Self.hint(
52+
base: shortcutHint(String(localized: "Run"), .executeQuery),
53+
reason: Self.blockedReason(isConnected: isConnected, hasQueryText: hasQueryText, isExecuting: isExecuting)
54+
)
55+
stopHint = shortcutHint(String(localized: "Stop"), .cancelQuery)
56+
explainHint = Self.hint(
57+
base: shortcutHint(String(localized: "Explain"), .explainQuery),
58+
reason: explainVariants.isEmpty
59+
? String(localized: "This database does not explain statements.")
60+
: Self.blockedReason(isConnected: isConnected, hasQueryText: hasQueryText, isExecuting: isExecuting)
61+
)
62+
formatHint = Self.hint(
63+
base: shortcutHint(String(localized: "Format"), .formatQuery),
64+
reason: hasQueryText ? nil : String(localized: "There is nothing to format yet.")
65+
)
66+
favoriteHint = Self.hint(
67+
base: shortcutHint(String(localized: "Save as Favorite"), .saveAsFavorite),
68+
reason: hasQueryText ? nil : String(localized: "There is nothing to save yet.")
69+
)
70+
}
71+
72+
private static func blockedReason(isConnected: Bool, hasQueryText: Bool, isExecuting: Bool) -> String? {
73+
if isExecuting { return String(localized: "A query is already running.") }
74+
if !hasQueryText { return String(localized: "There is nothing to run yet.") }
75+
if !isConnected { return String(localized: "This connection is not available.") }
76+
return nil
77+
}
78+
79+
private static func hint(base: String, reason: String?) -> String {
80+
guard let reason else { return base }
81+
return "\(base)\n\(reason)"
82+
}
83+
}
84+
85+
/// What the editor bar's leading control names: the container this tab's SQL runs in.
86+
struct QueryScopeBarModel {
87+
let containers: [DatabaseMetadata]
88+
let selectedName: String
89+
let entityName: String
90+
let isReadOnly: Bool
91+
let schemaName: String?
92+
}

0 commit comments

Comments
 (0)