From 227e5f11883f615aa0e11659e044a8ce4fa670e1 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 5 Jun 2026 22:43:04 +0700 Subject: [PATCH 1/2] fix(editor): sync external text binding changes into the source editor (#1576) --- CHANGELOG.md | 1 + .../SourceEditor+Coordinator.swift | 24 ++++- .../SourceEditor/SourceEditor.swift | 5 + .../HighlightProviderStateTest.swift | 12 +-- .../Highlighting/HighlighterTests.swift | 2 +- .../SourceEditorBindingSyncTests.swift | 102 ++++++++++++++++++ .../TreeSitterClientTests.swift | 16 +-- TablePro/Views/Results/ResultsJsonView.swift | 8 ++ 8 files changed, 152 insertions(+), 18 deletions(-) create mode 100644 LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/SourceEditorBindingSyncTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fceaa727..5512607a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- The JSON results view shows row data right away instead of staying blank until you switch between Tree and Text, and it updates when the row selection changes. A spinner shows while large results are being formatted. (#1576) - Double-clicking or pressing Enter on a JSON cell now edits its value inline, like other cells; on a blob cell it opens the hex editor. The chevron still opens the tree or hex editor. Neither cell responded to double-click before. (#1588) - Query results appear as soon as the database returns rows. Column defaults, foreign keys, and row counts now load in the background instead of holding up the grid, which removes a multi-second wait on remote databases whose system tables are slow to query. (#1574) - MySQL and MariaDB queries are ready to edit right away. Primary key and nullability come back with the rows, so an editable query no longer waits on a separate metadata query. (#1574) diff --git a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor+Coordinator.swift b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor+Coordinator.swift index cd008290e..2aaca9167 100644 --- a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor+Coordinator.swift +++ b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor+Coordinator.swift @@ -17,6 +17,7 @@ extension SourceEditor { var isUpdatingFromRepresentable: Bool = false var isUpdateFromTextView: Bool = false var text: TextAPI + var lastSyncedText: String? @Binding var editorState: SourceEditorState private(set) var highlightProviders: [any HighlightProviding] @@ -125,7 +126,7 @@ extension SourceEditor { guard let textView = notification.object as? TextView else { return } - // A plain string binding is one-way (from this view, up the hierarchy) so it's not in the state binding + guard !isUpdatingFromRepresentable else { return } guard case .binding(let binding) = text else { return } // For large documents, debounce the binding writeback to avoid @@ -139,13 +140,30 @@ extension SourceEditor { textBindingTask = Task { @MainActor [weak self, weak textView] in try? await Task.sleep(for: .milliseconds(150)) guard !Task.isCancelled, let self, let textView else { return } - binding.wrappedValue = textView.string + let newText = textView.string + self.lastSyncedText = newText + binding.wrappedValue = newText } } else { - binding.wrappedValue = textView.string + let newText = textView.string + lastSyncedText = newText + binding.wrappedValue = newText } } + /// Pushes an external binding change down into the text view. The text view's + /// content wins while one of its own edits is still in flight (`lastSyncedText` + /// only trails the binding during the debounce window, when both hold the same + /// pre-edit value), so user typing is never clobbered by a stale binding. + func syncBindingText(_ newValue: String, controller: TextViewController) { + guard newValue != lastSyncedText else { return } + textBindingTask?.cancel() + isUpdatingFromRepresentable = true + controller.textView.setText(newValue) + isUpdatingFromRepresentable = false + lastSyncedText = newValue + } + @objc func textControllerCursorsDidUpdate(_ notification: Notification) { guard let controller = notification.object as? TextViewController else { return diff --git a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift index 54b56778c..5d98ba223 100644 --- a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift +++ b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift @@ -110,6 +110,7 @@ public struct SourceEditor: NSViewControllerRepresentable { switch text { case .binding(let binding): controller.textView.setText(binding.wrappedValue) + context.coordinator.lastSyncedText = binding.wrappedValue case .storage(let textStorage): controller.textView.setTextStorage(textStorage) } @@ -134,6 +135,10 @@ public struct SourceEditor: NSViewControllerRepresentable { context.coordinator.updateHighlightProviders(highlightProviders) + if case .binding(let binding) = text { + context.coordinator.syncBindingText(binding.wrappedValue, controller: controller) + } + // Prevent infinite loop of update notifications if context.coordinator.isUpdateFromTextView { context.coordinator.isUpdateFromTextView = false diff --git a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Highlighting/HighlightProviderStateTest.swift b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Highlighting/HighlightProviderStateTest.swift index 61334cd14..accd532fb 100644 --- a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Highlighting/HighlightProviderStateTest.swift +++ b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Highlighting/HighlightProviderStateTest.swift @@ -52,7 +52,7 @@ final class HighlightProviderStateTest: XCTestCase { highlightProvider: mockProvider, textView: textView, visibleRangeProvider: rangeProvider, - language: .swift + language: .sql ) wait(for: [setUpExpectation], timeout: 1.0) @@ -66,9 +66,9 @@ final class HighlightProviderStateTest: XCTestCase { let mockProvider = Mock.highlightProvider( onSetUp: { language in switch language { - case .c: + case .json: firstSetUpExpectation.fulfill() - case .swift: + case .sql: secondSetUpExpectation.fulfill() default: XCTFail("Unexpected language: \(language)") @@ -84,12 +84,12 @@ final class HighlightProviderStateTest: XCTestCase { highlightProvider: mockProvider, textView: textView, visibleRangeProvider: rangeProvider, - language: .c + language: .json ) wait(for: [firstSetUpExpectation], timeout: 1.0) - state.setLanguage(language: .swift) + state.setLanguage(language: .sql) wait(for: [secondSetUpExpectation], timeout: 1.0) } @@ -113,7 +113,7 @@ final class HighlightProviderStateTest: XCTestCase { highlightProvider: mockProvider, textView: textView, visibleRangeProvider: rangeProvider, - language: .swift + language: .sql ) // These reflect values like `NSTextStorage` outputs, and differ from ranges used in other tests. diff --git a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Highlighting/HighlighterTests.swift b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Highlighting/HighlighterTests.swift index 90a4a0132..b6fe4f9ab 100644 --- a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Highlighting/HighlighterTests.swift +++ b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Highlighting/HighlighterTests.swift @@ -247,7 +247,7 @@ final class HighlighterTests: XCTestCase { attributeProvider: attributeProvider ) textView.addStorageDelegate(highlighter) - highlighter.setLanguage(language: .swift) + highlighter.setLanguage(language: .sql) highlighter.invalidate() // Delete Characters diff --git a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/SourceEditorBindingSyncTests.swift b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/SourceEditorBindingSyncTests.swift new file mode 100644 index 000000000..8adf7d7c0 --- /dev/null +++ b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/SourceEditorBindingSyncTests.swift @@ -0,0 +1,102 @@ +import AppKit +import SwiftUI +import XCTest + +@testable import CodeEditSourceEditor +import CodeEditTextView + +final class SourceEditorBindingSyncTests: XCTestCase { + var controller: TextViewController! + + override func setUpWithError() throws { + controller = Mock.textViewController(theme: Mock.theme()) + controller.loadView() + controller.view.frame = NSRect(x: 0, y: 0, width: 1_000, height: 1_000) + controller.view.layoutSubtreeIfNeeded() + } + + @MainActor + private func makeCoordinator( + get: @escaping () -> String, + set: @escaping (String) -> Void + ) -> SourceEditor.Coordinator { + SourceEditor.Coordinator( + text: .binding(Binding(get: get, set: { set($0) })), + editorState: .constant(SourceEditorState()), + highlightProviders: nil + ) + } + + @MainActor + func test_externalBindingChangeUpdatesTextView() { + var bound = "initial" + let coordinator = makeCoordinator(get: { bound }, set: { bound = $0 }) + controller.textView.setText("initial") + coordinator.lastSyncedText = "initial" + + bound = "updated" + coordinator.syncBindingText(bound, controller: controller) + + XCTAssertEqual(controller.textView.string, "updated") + XCTAssertEqual(coordinator.lastSyncedText, "updated") + } + + @MainActor + func test_syncSkipsWhenBindingMatchesLastSyncedText() { + var bound = "before edit" + let coordinator = makeCoordinator(get: { bound }, set: { bound = $0 }) + controller.textView.setText("in-flight user edit") + coordinator.lastSyncedText = "before edit" + + coordinator.syncBindingText(bound, controller: controller) + + XCTAssertEqual(controller.textView.string, "in-flight user edit") + XCTAssertEqual(coordinator.lastSyncedText, "before edit") + } + + @MainActor + func test_textViewEditWritesBackAndRecordsLastSyncedText() { + var bound = "" + let coordinator = makeCoordinator(get: { bound }, set: { bound = $0 }) + controller.textView.setText("typed text") + + coordinator.textViewDidChangeText( + Notification(name: TextView.textDidChangeNotification, object: controller.textView) + ) + + XCTAssertEqual(bound, "typed text") + XCTAssertEqual(coordinator.lastSyncedText, "typed text") + XCTAssertTrue(coordinator.isUpdateFromTextView) + } + + @MainActor + func test_editNotificationDuringProgrammaticSyncDoesNotWriteBack() { + var bound = "external" + var setterCallCount = 0 + let coordinator = makeCoordinator(get: { bound }, set: { bound = $0; setterCallCount += 1 }) + controller.textView.setText("stale") + coordinator.isUpdatingFromRepresentable = true + + coordinator.textViewDidChangeText( + Notification(name: TextView.textDidChangeNotification, object: controller.textView) + ) + + XCTAssertEqual(setterCallCount, 0) + XCTAssertFalse(coordinator.isUpdateFromTextView) + XCTAssertEqual(bound, "external") + } + + @MainActor + func test_repeatedSyncWithSameValueLeavesTextViewUntouched() { + var bound = "stable" + let coordinator = makeCoordinator(get: { bound }, set: { bound = $0 }) + controller.textView.setText("stable") + coordinator.lastSyncedText = "stable" + let storageBefore = controller.textView.textStorage + + coordinator.syncBindingText(bound, controller: controller) + + XCTAssertIdentical(controller.textView.textStorage, storageBefore) + XCTAssertEqual(controller.textView.string, "stable") + } +} diff --git a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/TreeSitterClientTests.swift b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/TreeSitterClientTests.swift index d2446cc9a..17aca5842 100644 --- a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/TreeSitterClientTests.swift +++ b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/TreeSitterClientTests.swift @@ -34,7 +34,7 @@ final class TreeSitterClientTests: XCTestCase { func test_clientSetup() async { let client = Mock.treeSitterClient() let textView = Mock.textView() - client.setUp(textView: textView, codeLanguage: .swift) + client.setUp(textView: textView, codeLanguage: .sql) let expectation = XCTestExpectation(description: "Setup occurs") @@ -49,7 +49,7 @@ final class TreeSitterClientTests: XCTestCase { let primaryLanguage = client.state?.primaryLayer.id let layerCount = client.state?.layers.count - XCTAssertEqual(primaryLanguage, .swift, "Client set up incorrect language") + XCTAssertEqual(primaryLanguage, .sql, "Client set up incorrect language") XCTAssertEqual(layerCount, 1, "Client set up too many layers") } @@ -67,7 +67,7 @@ final class TreeSitterClientTests: XCTestCase { let client = Mock.treeSitterClient() let textView = Mock.textView() - client.setUp(textView: textView, codeLanguage: .swift) + client.setUp(textView: textView, codeLanguage: .sql) // Perform a highlight query let cancelledQuery = XCTestExpectation(description: "Highlight query should be cancelled by edits.") @@ -109,7 +109,7 @@ final class TreeSitterClientTests: XCTestCase { let textView = Mock.textView() // First setup, wrong language - client.setUp(textView: textView, codeLanguage: .c) + client.setUp(textView: textView, codeLanguage: .json) // Perform a highlight query let cancelledQuery = XCTestExpectation(description: "Highlight query should be cancelled by second setup.") @@ -132,12 +132,12 @@ final class TreeSitterClientTests: XCTestCase { } // Second setup, which should cancel all previous operations - client.setUp(textView: textView, codeLanguage: .swift) + client.setUp(textView: textView, codeLanguage: .sql) let finalSetupExpectation = XCTestExpectation(description: "Final setup should complete successfully.") Task.detached { - while client.state?.primaryLayer.id != .swift { + while client.state?.primaryLayer.id != .sql { try await Task.sleep(for: .seconds(0.5)) } finalSetupExpectation.fulfill() @@ -148,7 +148,7 @@ final class TreeSitterClientTests: XCTestCase { // Ensure only the final setup's language is active let primaryLanguage = client.state?.primaryLayer.id let layerCount = client.state?.layers.count - XCTAssertEqual(primaryLanguage, .swift, "Client set up incorrect language after re-setup.") + XCTAssertEqual(primaryLanguage, .sql, "Client set up incorrect language after re-setup.") XCTAssertEqual(layerCount, 1, "Client set up too many layers after re-setup.") } @@ -158,7 +158,7 @@ final class TreeSitterClientTests: XCTestCase { let textView = Mock.textView() textView.setText("asadajkfijio;amfjamc;aoijaoajkvarpfjo;sdjlkj") - client.setUp(textView: textView, codeLanguage: .swift) + client.setUp(textView: textView, codeLanguage: .sql) // Set up random edits let editExpectations = (0..<10).map { index -> XCTestExpectation in diff --git a/TablePro/Views/Results/ResultsJsonView.swift b/TablePro/Views/Results/ResultsJsonView.swift index b8b9ac07a..0d7cf35ba 100644 --- a/TablePro/Views/Results/ResultsJsonView.swift +++ b/TablePro/Views/Results/ResultsJsonView.swift @@ -100,6 +100,10 @@ internal struct ResultsJsonView: View { // MARK: - Content + private var isInitialComputePending: Bool { + prettyText.isEmpty + } + @ViewBuilder private var content: some View { if tableRows.rows.isEmpty { @@ -108,6 +112,10 @@ internal struct ResultsJsonView: View { systemImage: "curlybraces", description: Text(String(localized: "Execute a query to view results as JSON")) ) + } else if isInitialComputePending { + ProgressView() + .controlSize(.small) + .frame(maxWidth: .infinity, maxHeight: .infinity) } else { switch viewMode { case .text: From 337b3798065255c026e1e537c97d345e6da77bc1 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 5 Jun 2026 22:50:59 +0700 Subject: [PATCH 2/2] refactor(editor): capture binding state in coordinator init and refresh writeback target (#1576) --- .../SourceEditor+Coordinator.swift | 12 +++++- .../SourceEditor/SourceEditor.swift | 2 +- .../SourceEditorBindingSyncTests.swift | 41 +++++++++++++++++-- TablePro/Views/Results/ResultsJsonView.swift | 1 + 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor+Coordinator.swift b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor+Coordinator.swift index 2aaca9167..7f84975a0 100644 --- a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor+Coordinator.swift +++ b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor+Coordinator.swift @@ -28,6 +28,9 @@ extension SourceEditor { self.text = text self._editorState = editorState self.highlightProviders = highlightProviders ?? [TreeSitterClient()] + if case .binding(let binding) = text { + self.lastSyncedText = binding.wrappedValue + } super.init() } @@ -140,9 +143,10 @@ extension SourceEditor { textBindingTask = Task { @MainActor [weak self, weak textView] in try? await Task.sleep(for: .milliseconds(150)) guard !Task.isCancelled, let self, let textView else { return } + guard case .binding(let currentBinding) = self.text else { return } let newText = textView.string self.lastSyncedText = newText - binding.wrappedValue = newText + currentBinding.wrappedValue = newText } } else { let newText = textView.string @@ -155,6 +159,12 @@ extension SourceEditor { /// content wins while one of its own edits is still in flight (`lastSyncedText` /// only trails the binding during the debounce window, when both hold the same /// pre-edit value), so user typing is never clobbered by a stale binding. + /// + /// Uses `setText` rather than `replaceCharacters` on purpose: `replaceCharacters` + /// is the user-edit path. It is gated on `isEditable`, runs mutation filters, and + /// fires suggestion triggers, none of which should happen for a programmatic + /// whole-document replacement. `setText` clearing the undo stack matches the + /// new-document semantics of that replacement. func syncBindingText(_ newValue: String, controller: TextViewController) { guard newValue != lastSyncedText else { return } textBindingTask?.cancel() diff --git a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift index 5d98ba223..29a081439 100644 --- a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift +++ b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift @@ -110,7 +110,6 @@ public struct SourceEditor: NSViewControllerRepresentable { switch text { case .binding(let binding): controller.textView.setText(binding.wrappedValue) - context.coordinator.lastSyncedText = binding.wrappedValue case .storage(let textStorage): controller.textView.setTextStorage(textStorage) } @@ -135,6 +134,7 @@ public struct SourceEditor: NSViewControllerRepresentable { context.coordinator.updateHighlightProviders(highlightProviders) + context.coordinator.text = text if case .binding(let binding) = text { context.coordinator.syncBindingText(binding.wrappedValue, controller: controller) } diff --git a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/SourceEditorBindingSyncTests.swift b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/SourceEditorBindingSyncTests.swift index 8adf7d7c0..92eb52f3e 100644 --- a/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/SourceEditorBindingSyncTests.swift +++ b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/SourceEditorBindingSyncTests.swift @@ -32,7 +32,6 @@ final class SourceEditorBindingSyncTests: XCTestCase { var bound = "initial" let coordinator = makeCoordinator(get: { bound }, set: { bound = $0 }) controller.textView.setText("initial") - coordinator.lastSyncedText = "initial" bound = "updated" coordinator.syncBindingText(bound, controller: controller) @@ -46,7 +45,6 @@ final class SourceEditorBindingSyncTests: XCTestCase { var bound = "before edit" let coordinator = makeCoordinator(get: { bound }, set: { bound = $0 }) controller.textView.setText("in-flight user edit") - coordinator.lastSyncedText = "before edit" coordinator.syncBindingText(bound, controller: controller) @@ -86,12 +84,49 @@ final class SourceEditorBindingSyncTests: XCTestCase { XCTAssertEqual(bound, "external") } + @MainActor + func test_largeDocumentEditDebouncesWritebackWithoutClobbering() async throws { + var bound = "" + let coordinator = makeCoordinator(get: { bound }, set: { bound = $0 }) + let largeText = String(repeating: "a", count: 500_001) + controller.textView.setText(largeText) + + coordinator.textViewDidChangeText( + Notification(name: TextView.textDidChangeNotification, object: controller.textView) + ) + + XCTAssertEqual(bound, "") + XCTAssertTrue(coordinator.isUpdateFromTextView) + + coordinator.syncBindingText(bound, controller: controller) + XCTAssertEqual(controller.textView.string, largeText) + + try await Task.sleep(for: .milliseconds(300)) + XCTAssertEqual(bound, largeText) + XCTAssertEqual(coordinator.lastSyncedText, largeText) + } + + @MainActor + func test_syncShorterTextDropsOutOfBoundsSelection() { + var bound = "select * from table" + let coordinator = makeCoordinator(get: { bound }, set: { bound = $0 }) + controller.textView.setText("select * from table") + controller.textView.selectionManager.setSelectedRange(NSRange(location: 19, length: 0)) + + bound = "ab" + coordinator.syncBindingText(bound, controller: controller) + + XCTAssertEqual(controller.textView.string, "ab") + for selection in controller.textView.selectionManager.textSelections { + XCTAssertLessThanOrEqual(selection.range.max, 2) + } + } + @MainActor func test_repeatedSyncWithSameValueLeavesTextViewUntouched() { var bound = "stable" let coordinator = makeCoordinator(get: { bound }, set: { bound = $0 }) controller.textView.setText("stable") - coordinator.lastSyncedText = "stable" let storageBefore = controller.textView.textStorage coordinator.syncBindingText(bound, controller: controller) diff --git a/TablePro/Views/Results/ResultsJsonView.swift b/TablePro/Views/Results/ResultsJsonView.swift index 0d7cf35ba..ab08cff4b 100644 --- a/TablePro/Views/Results/ResultsJsonView.swift +++ b/TablePro/Views/Results/ResultsJsonView.swift @@ -93,6 +93,7 @@ internal struct ResultsJsonView: View { } .buttonStyle(.borderless) .controlSize(.small) + .disabled(isInitialComputePending) } .padding(.horizontal, 10) .padding(.vertical, 6)