Skip to content

fix(editor): sync external text binding changes into the source editor#1592

Merged
datlechin merged 2 commits into
mainfrom
fix/json-view-reclick-1576
Jun 5, 2026
Merged

fix(editor): sync external text binding changes into the source editor#1592
datlechin merged 2 commits into
mainfrom
fix/json-view-reclick-1576

Conversation

@datlechin
Copy link
Copy Markdown
Member

Fixes #1576

Problem

Selecting rows and switching the results view to JSON showed a blank editor. The data only appeared after clicking Tree and then Text, which forces SwiftUI to recreate the editor.

Root cause

SourceEditor set the text binding's value once in makeNSViewController and never again. updateNSViewController only handled cursor, scroll, find panel, and configuration, so a binding that changed after creation never reached the text view. ResultsJsonView computes its JSON asynchronously, so the editor was always created with an empty string and stayed empty.

The same gap made DDLTextView's .onChange(of: ddl) writeback a no-op, and stale JSON stayed on screen when the row selection changed while in text mode.

Fix

  • The coordinator tracks lastSyncedText, the last value synced in either direction. updateNSViewController now pushes the binding value into the text view when it differs. The comparison is O(1) when nothing changed because the unchanged binding returns the same string instance.
  • User typing is never clobbered: textViewDidChangeText records lastSyncedText on both the immediate and the debounced (>500k chars) writeback paths, and a pending debounce is cancelled before an external value is applied. Programmatic setText runs under isUpdatingFromRepresentable, which the change notification handler now respects.
  • ResultsJsonView shows a small spinner until the first JSON formatting pass finishes, instead of an empty editor in text mode or an "Invalid JSON" flash in tree mode. Recomputes triggered by selection changes keep the previous content visible until the new result is ready.

Tests

  • New SourceEditorBindingSyncTests (5 tests, all passing): external change syncs down, in-flight user edits are not clobbered, typing writes back and records the synced value, notifications during programmatic sync do not write back, and same-value syncs leave the text storage untouched.
  • Repaired the package test target, which did not compile: TreeSitterClientTests, HighlighterTests, and HighlightProviderStateTest referenced CodeLanguage.swift and .c, which the vendored CodeEditLanguages no longer ships. They now use .sql and .json.
  • No UI automation: the flow needs a live database connection with result rows, which does not run deterministically in TableProUITests.

Verification

swiftlint lint --strict passes on the changed app file; the package's SwiftLint build plugin passes. Manual check: select rows, switch to JSON, data renders immediately; changing the selection updates the text view in place.

@datlechin
Copy link
Copy Markdown
Member Author

Follow-up to the self-review notes, in 337b379:

  • Coordinator.init now captures lastSyncedText from the binding, so there is no nil window between coordinator creation and makeNSViewController.
  • updateNSViewController refreshes coordinator.text, and the debounced writeback reads the binding at fire time instead of the one captured when typing started.
  • Copy JSON is disabled until the first formatting pass finishes, so it can no longer copy an empty string.
  • Two new tests: the >500k debounce path (no writeback before the delay, no clobber from an interim sync, writeback lands after) and out-of-bounds selection dropping when a shorter external text arrives. 7 tests total, all passing.
  • Kept setText over replaceCharacters on purpose: replaceCharacters is the user-edit path (gated on isEditable, runs mutation filters, fires suggestion triggers). The doc comment on syncBindingText records this.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 337b379806

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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 👍 / 👎.

@datlechin datlechin merged commit d8f9856 into main Jun 5, 2026
4 checks passed
@datlechin datlechin deleted the fix/json-view-reclick-1576 branch June 5, 2026 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSON View needs a re-click

1 participant