Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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()
}

Expand Down Expand Up @@ -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
Expand All @@ -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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh highlighting after binding text replacement

When a loaded editor receives an external binding change, this swaps the TextView storage directly, but it bypasses TextViewController.setText(_:), which is the path that calls setUpHighlighter() after replacing the text. TextView.setTextStorage reuses the existing storage delegate without emitting a full-character edit, so the current highlighter/style container remains sized and initialized for the old document; externally loaded text can render with stale or missing syntax highlighting until some later highlighter reset/edit happens.

Useful? React with 👍 / 👎.

isUpdatingFromRepresentable = false
lastSyncedText = newValue
}

@objc func textControllerCursorsDidUpdate(_ notification: Notification) {
guard let controller = notification.object as? TextViewController else {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ final class HighlightProviderStateTest: XCTestCase {
highlightProvider: mockProvider,
textView: textView,
visibleRangeProvider: rangeProvider,
language: .swift
language: .sql
)

wait(for: [setUpExpectation], timeout: 1.0)
Expand All @@ -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)")
Expand All @@ -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)
}
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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")
}

Expand All @@ -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.")
Expand Down Expand Up @@ -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.")
Expand All @@ -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()
Expand All @@ -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.")
}

Expand All @@ -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
Expand Down
9 changes: 9 additions & 0 deletions TablePro/Views/Results/ResultsJsonView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,18 @@ internal struct ResultsJsonView: View {
}
.buttonStyle(.borderless)
.controlSize(.small)
.disabled(isInitialComputePending)
}
.padding(.horizontal, 10)
.padding(.vertical, 6)
}

// MARK: - Content

private var isInitialComputePending: Bool {
prettyText.isEmpty
}

@ViewBuilder
private var content: some View {
if tableRows.rows.isEmpty {
Expand All @@ -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:
Expand Down
Loading