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..7f84975a0 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] @@ -27,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() } @@ -125,7 +129,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 +143,37 @@ 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 + guard case .binding(let currentBinding) = self.text else { return } + let newText = textView.string + self.lastSyncedText = newText + currentBinding.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. + /// + /// 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() + 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..29a081439 100644 --- a/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift +++ b/LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift @@ -134,6 +134,11 @@ 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) + } + // 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..92eb52f3e --- /dev/null +++ b/LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/SourceEditorBindingSyncTests.swift @@ -0,0 +1,137 @@ +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") + + 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.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_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") + 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..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) @@ -100,6 +101,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 +113,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: