From 1dd5da873ba7b0d9c50cb83b4b39e9cb76ae8e65 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 10 Apr 2026 19:09:48 +0700 Subject: [PATCH 1/2] fix: context menu Format SQL not working due to stale closure capture (#659) Root cause: formatQuery() passed as onFormatSQL closure captures QueryEditorView struct by value. After SwiftUI recreates the view, the captured @Binding queryText becomes stale (empty). Toolbar button works because it runs in current render context. Fix: add fallback in context menu that formats directly via the text view when the callback closure is stale. Also lazily recreate context menu after destroy(). --- .../Views/Editor/SQLEditorCoordinator.swift | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/TablePro/Views/Editor/SQLEditorCoordinator.swift b/TablePro/Views/Editor/SQLEditorCoordinator.swift index 13902b88..e1ab0592 100644 --- a/TablePro/Views/Editor/SQLEditorCoordinator.swift +++ b/TablePro/Views/Editor/SQLEditorCoordinator.swift @@ -206,12 +206,36 @@ final class SQLEditorCoordinator: TextViewCoordinator { menu.onExplainWithAI = { [weak self] text in self?.onAIExplain?(text) } menu.onOptimizeWithAI = { [weak self] text in self?.onAIOptimize?(text) } menu.onSaveAsFavorite = { [weak self] text in self?.onSaveAsFavorite?(text) } - menu.onFormatSQL = { [weak self] in self?.onFormatSQL?() } + menu.onFormatSQL = { [weak self, weak controller] in + guard let self else { return } + if let onFormat = self.onFormatSQL { + onFormat() + } else if let textView = controller?.textView { + // Fallback: format directly via text view if callback is stale + self.formatSQLInTextView(textView) + } + } contextMenu = menu } + private func formatSQLInTextView(_ textView: TextView) { + let sql = textView.string + guard !sql.isEmpty else { return } + let formatter = SQLFormatterService() + let cursorOffset = textView.selectedRange().location + do { + let result = try formatter.format(sql, dialect: .mysql, cursorOffset: cursorOffset) + textView.replaceCharacters(in: NSRange(location: 0, length: (textView.string as NSString).length), with: result.formattedSQL) + } catch { + Self.logger.error("Context menu format error: \(error.localizedDescription)") + } + } + /// Called by EditorEventRouter when a right-click is detected in this editor's text view. func showContextMenu(for event: NSEvent, in textView: TextView) { + if contextMenu == nil, let controller { + installAIContextMenu(controller: controller) + } guard let menu = contextMenu else { return } NSMenu.popUpContextMenu(menu, with: event, for: textView) } From 27dc18417ed981864730ccc1eaeeeb34f9684c98 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 10 Apr 2026 19:22:12 +0700 Subject: [PATCH 2/2] fix: root cause fix for stale editor callbacks + 3 formatter bugs (#659) Coordinator lifecycle: - Move callback wiring from .onAppear to view body (fresh on every re-render) - Add revive() method to re-register with EditorEventRouter after tab switch - Add onFormatSQL = nil to destroy() cleanup - Add databaseType property to coordinator - Remove hacky formatSQLInTextView fallback (was hardcoding .mysql dialect) SQL formatter: - Fix comment placeholder order: single-pass extraction by source position - Fix SQL-standard '' escape not handled in string literal regex - Fix addLineBreaks ignoring uppercaseKeywords option --- .../Formatting/SQLFormatterService.swift | 52 ++++++------------- .../Views/Editor/SQLEditorCoordinator.swift | 36 +++++-------- TablePro/Views/Editor/SQLEditorView.swift | 35 +++++++------ 3 files changed, 49 insertions(+), 74 deletions(-) diff --git a/TablePro/Core/Services/Formatting/SQLFormatterService.swift b/TablePro/Core/Services/Formatting/SQLFormatterService.swift index d6d31fc6..49c8ccd0 100644 --- a/TablePro/Core/Services/Formatting/SQLFormatterService.swift +++ b/TablePro/Core/Services/Formatting/SQLFormatterService.swift @@ -40,26 +40,17 @@ struct SQLFormatterService: SQLFormatterProtocol { // MARK: - Cached Regex Patterns (CPU-3, CPU-9, CPU-10) /// String literal extraction patterns — one per quote character + /// Handles both backslash escapes (\'') and SQL-standard doubled-quote escapes ('') private static let stringLiteralRegexes: [String: NSRegularExpression] = { var result: [String: NSRegularExpression] = [:] for quoteChar in ["'", "\"", "`"] { let escaped = NSRegularExpression.escapedPattern(for: quoteChar) - let pattern = "\(escaped)((?:\\\\\\\\\(quoteChar)|[^\(quoteChar)])*?)\(escaped)" + let pattern = "\(escaped)((?:\\\\\\\\\(quoteChar)|\(escaped)\(escaped)|[^\(quoteChar)])*)\(escaped)" result[quoteChar] = regex(pattern) } return result }() - /// Line comment pattern: --[^\n]* - private static let lineCommentRegex: NSRegularExpression = { - regex("--[^\\n]*") - }() - - /// Block comment pattern: /* ... */ - private static let blockCommentRegex: NSRegularExpression = { - regex("/\\*.*?\\*/", options: .dotMatchesLineSeparators) - }() - /// Line break keyword patterns — pre-compiled for all 16 keywords (CPU-9) /// Sorted by keyword length (longest first) to handle multi-word keywords correctly private static let lineBreakRegexes: [(keyword: String, regex: NSRegularExpression)] = { @@ -207,7 +198,7 @@ struct SQLFormatterService: SQLFormatterProtocol { result = restoreStringLiterals(result, literals: stringLiterals) // Step 5: Add line breaks before major keywords - result = addLineBreaks(result) + result = addLineBreaks(result, options: options) // Step 6: Add indentation based on nesting if options.indentSize > 0 { @@ -288,38 +279,28 @@ struct SQLFormatterService: SQLFormatterProtocol { // MARK: - Comment Handling (Fix #6: UUID placeholders) - /// Extract comments with UUID-based placeholders (prevents collisions) + /// Combined pattern matching both line comments (--...) and block comments (/*...*/) + private static let combinedCommentRegex: NSRegularExpression = { + regex("--[^\\n]*|/\\*.*?\\*/", options: .dotMatchesLineSeparators) + }() + + /// Extract all comments in a single pass, ordered by position in the source SQL. + /// This ensures __COMMENT_0__ is always the first comment, __COMMENT_1__ the second, etc. private func extractComments(from sql: String) -> (String, [(placeholder: String, content: String)]) { var result = sql var comments: [(String, String)] = [] - var counter = 0 - // Extract line comments (-- ...) using cached regex - let lineMatches = Self.lineCommentRegex.matches( + let allMatches = Self.combinedCommentRegex.matches( in: result, range: NSRange(result.startIndex..., in: result) ) - for match in lineMatches.reversed() { - if let range = safeRange(from: match.range, in: result) { - let comment = String(result[range]) - let placeholder = "__COMMENT_\(counter)__" - counter += 1 - comments.insert((placeholder, comment), at: 0) - result.replaceSubrange(range, with: placeholder) - } - } - // Extract block comments (/* ... */) using cached regex - // Note: This doesn't handle nested block comments (SQL doesn't officially support them) - let blockMatches = Self.blockCommentRegex.matches( - in: result, - range: NSRange(result.startIndex..., in: result) - ) - for match in blockMatches.reversed() { + // Process in reverse to maintain valid indices; assign counters by source position + for (reverseIndex, match) in allMatches.reversed().enumerated() { if let range = safeRange(from: match.range, in: result) { let comment = String(result[range]) + let counter = allMatches.count - 1 - reverseIndex let placeholder = "__COMMENT_\(counter)__" - counter += 1 comments.insert((placeholder, comment), at: 0) result.replaceSubrange(range, with: placeholder) } @@ -363,15 +344,16 @@ struct SQLFormatterService: SQLFormatterProtocol { // MARK: - Line Breaks - private func addLineBreaks(_ sql: String) -> String { + private func addLineBreaks(_ sql: String, options: SQLFormatterOptions) -> String { var result = sql // Use pre-compiled regex patterns for all line break keywords (CPU-9) for (keyword, regex) in Self.lineBreakRegexes { + let replacement = options.uppercaseKeywords ? keyword.uppercased() : keyword result = regex.stringByReplacingMatches( in: result, range: NSRange(result.startIndex..., in: result), - withTemplate: "\n\(keyword.uppercased())" + withTemplate: "\n\(replacement)" ) } diff --git a/TablePro/Views/Editor/SQLEditorCoordinator.swift b/TablePro/Views/Editor/SQLEditorCoordinator.swift index e1ab0592..568fd4bc 100644 --- a/TablePro/Views/Editor/SQLEditorCoordinator.swift +++ b/TablePro/Views/Editor/SQLEditorCoordinator.swift @@ -50,6 +50,7 @@ final class SQLEditorCoordinator: TextViewCoordinator { @ObservationIgnored var onAIOptimize: ((String) -> Void)? @ObservationIgnored var onSaveAsFavorite: ((String) -> Void)? @ObservationIgnored var onFormatSQL: (() -> Void)? + @ObservationIgnored var databaseType: DatabaseType? /// Whether the editor text view is currently the first responder. /// Used to guard cursor propagation — when the find panel highlights @@ -172,6 +173,7 @@ final class SQLEditorCoordinator: TextViewCoordinator { onAIExplain = nil onAIOptimize = nil onSaveAsFavorite = nil + onFormatSQL = nil schemaProvider = nil contextMenu = nil vimEngine = nil @@ -185,6 +187,17 @@ final class SQLEditorCoordinator: TextViewCoordinator { cleanupMonitors() } + func revive() { + guard didDestroy else { return } + didDestroy = false + if let controller, let textView = controller.textView { + EditorEventRouter.shared.register(self, textView: textView) + } + if contextMenu == nil, let controller { + installAIContextMenu(controller: controller) + } + } + // MARK: - AI Context Menu private func installAIContextMenu(controller: TextViewController) { @@ -206,31 +219,10 @@ final class SQLEditorCoordinator: TextViewCoordinator { menu.onExplainWithAI = { [weak self] text in self?.onAIExplain?(text) } menu.onOptimizeWithAI = { [weak self] text in self?.onAIOptimize?(text) } menu.onSaveAsFavorite = { [weak self] text in self?.onSaveAsFavorite?(text) } - menu.onFormatSQL = { [weak self, weak controller] in - guard let self else { return } - if let onFormat = self.onFormatSQL { - onFormat() - } else if let textView = controller?.textView { - // Fallback: format directly via text view if callback is stale - self.formatSQLInTextView(textView) - } - } + menu.onFormatSQL = { [weak self] in self?.onFormatSQL?() } contextMenu = menu } - private func formatSQLInTextView(_ textView: TextView) { - let sql = textView.string - guard !sql.isEmpty else { return } - let formatter = SQLFormatterService() - let cursorOffset = textView.selectedRange().location - do { - let result = try formatter.format(sql, dialect: .mysql, cursorOffset: cursorOffset) - textView.replaceCharacters(in: NSRange(location: 0, length: (textView.string as NSString).length), with: result.formattedSQL) - } catch { - Self.logger.error("Context menu format error: \(error.localizedDescription)") - } - } - /// Called by EditorEventRouter when a right-click is detected in this editor's text view. func showContextMenu(for event: NSEvent, in textView: TextView) { if contextMenu == nil, let controller { diff --git a/TablePro/Views/Editor/SQLEditorView.swift b/TablePro/Views/Editor/SQLEditorView.swift index cb22e1c5..ad0e0b9e 100644 --- a/TablePro/Views/Editor/SQLEditorView.swift +++ b/TablePro/Views/Editor/SQLEditorView.swift @@ -38,7 +38,18 @@ struct SQLEditorView: View { @Environment(\.colorScheme) private var colorScheme var body: some View { - Group { + // Keep callbacks fresh on every parent re-render + coordinator.onCloseTab = onCloseTab + coordinator.onExecuteQuery = onExecuteQuery + coordinator.onAIExplain = onAIExplain + coordinator.onAIOptimize = onAIOptimize + coordinator.onSaveAsFavorite = onSaveAsFavorite + coordinator.onFormatSQL = onFormatSQL + coordinator.schemaProvider = schemaProvider + coordinator.connectionAIPolicy = connectionAIPolicy + coordinator.databaseType = databaseType + + return Group { if editorReady { SourceEditor( $text, @@ -97,33 +108,23 @@ struct SQLEditorView: View { editorConfiguration = Self.makeConfiguration() } .onAppear { + if coordinator.isDestroyed { + coordinator.revive() + } if completionAdapter == nil { completionAdapter = SQLCompletionAdapter(schemaProvider: schemaProvider, databaseType: databaseType) } - coordinator.schemaProvider = schemaProvider - coordinator.connectionAIPolicy = connectionAIPolicy - coordinator.onCloseTab = onCloseTab - coordinator.onExecuteQuery = onExecuteQuery - coordinator.onAIExplain = onAIExplain - coordinator.onAIOptimize = onAIOptimize - coordinator.onSaveAsFavorite = onSaveAsFavorite - coordinator.onFormatSQL = onFormatSQL setupFavoritesObserver() } } else { Color(nsColor: .textBackgroundColor) .onAppear { + if coordinator.isDestroyed { + coordinator.revive() + } if completionAdapter == nil { completionAdapter = SQLCompletionAdapter(schemaProvider: schemaProvider, databaseType: databaseType) } - coordinator.schemaProvider = schemaProvider - coordinator.connectionAIPolicy = connectionAIPolicy - coordinator.onCloseTab = onCloseTab - coordinator.onExecuteQuery = onExecuteQuery - coordinator.onAIExplain = onAIExplain - coordinator.onAIOptimize = onAIOptimize - coordinator.onSaveAsFavorite = onSaveAsFavorite - coordinator.onFormatSQL = onFormatSQL setupFavoritesObserver() editorReady = true }