Skip to content

fix(editor): recheck query diagnostics whenever the document is replaced and show their messages - #2728

Merged
datlechin merged 1 commit into
mainfrom
fix/query-diagnostics-refresh
Sep 11, 2026
Merged

fix(editor): recheck query diagnostics whenever the document is replaced and show their messages#2728
datlechin merged 1 commit into
mainfrom
fix/query-diagnostics-refresh

Conversation

@datlechin

Copy link
Copy Markdown
Member

Found while investigating #2717 (PR #2724).

The SQL editor underlines structural mistakes (an unclosed bracket, a stray quote) 500ms after typing stops. Three things were wrong with those underlines:

  • Switching query tabs left the previous tab's underlines in place, now sitting under unrelated text at the old offsets. The incoming text, or any query pushed into the editor through its text binding, was not checked at all until the first keystroke.
  • An underline on a line below the visible area was never drawn, even after scrolling to it.
  • An underline had no message anywhere. QueryDiagnostic.message was produced for every problem and never shown, and QueryDiagnosticsController.diagnostic(at:) had no caller.

Root cause

Refresh. Diagnostics were scheduled only from textView(_:didReplaceContentsIn:with:), which fires on edits. A tab switch or a loaded query replaces the whole document through TextViewController.setText (the app's only route, via TextBindingSync), and that method rebuilt the text storage, folds and highlighter without telling any TextViewCoordinator. So nothing re-ran the check and nothing removed the old emphasis layers, which live on the text view's layer and survive a storage swap. The statement run controls had the same gap and were only kept current because repointFolds(to:) happened to refresh them as a side effect.

Drawing. EmphasisManager.createEmphasisLayer built the shape layer only when the range already had line fragments. For a range on a line not yet laid out it returned a bare CAShapeLayer() that was never inserted into the view and had no stroke colour or line width. updateLayerBackgrounds() later gave that layer a path but never attached it, so the underline stayed invisible for good.

Messages. Nothing carried the message from the diagnostic to the view.

What changed

  • TextViewCoordinator gains textViewDidReplaceDocument(controller:) with an empty default, and TextViewController.setText calls it on every coordinator after the new document is in place. This is the vendored CodeEditSourceEditor, not PluginKit, so no ABI concern.
  • SQLEditorCoordinator.textViewDidReplaceDocument drops the old underlines at once, schedules a check of the new text (skipped past the same 100,000 character limit edits use), refreshes the run controls and current-statement highlight, and resets the Vim line cache, fold preview and inline suggestion. The run-control refresh moved here out of repointFolds(to:). A large document now also clears its underlines on edit instead of keeping stale ones, and teardown clears them too.
  • EmphasisManager: the per-emphasis record is a class, so a layer that could not be drawn yet is created with its real style and attached on the first updateLayerBackgrounds() that finds its line fragments.
  • Emphasis gains an optional toolTip. EmphasisManager registers native NSView.addToolTip(_:owner:userData:) rects for it through a new EmphasisToolTips owner, moves them when layout moves the text, and removes them when the emphasis is removed, flashed away, or its text is gone. Query diagnostics pass their message, so resting the pointer on an underline shows it.
  • QueryDiagnosticsRotorSearch adds an NSAccessibilityCustomRotor named "Query Issues" to the editor's text view. VoiceOver walks the problems in document order, reads each message, and can filter by message text. install(on:) is idempotent.
  • QueryDiagnosticsController.clear(in:) returns early when nothing is underlined, because removeEmphases(for:) forces a layer layout and clear now runs on every document replacement.
  • New string "Query Issues" in the catalog with ko, tr, vi, zh-Hans and zh-Hant. docs/features/sql-editor.mdx describes the tab-switch check, the tooltip and the rotor. One CHANGELOG line under Fixed.

Verification

  • verify.sh build: succeeded (.analysis/fix-query-diagnostics-refresh/logs/build-TablePro-154839.log).
  • verify.sh test over 13 suites (QueryDiagnosticsRefreshTests, QueryDiagnosticMessageTests, SQLEditorCoordinatorTests, SQLEditorCoordinatorCleanupTests, SQLEditorCoordinatorEscapeMenuTests, EditorLifecycleTeardownTests, RemoveInvisibleCharactersCommandTests, StatementRunControllerTests, StatementNavigationCommandTests, GutterHighlightTests, StatementRunPerformanceGuardTests, QueryCompletionAdapterLifecycleTests, StringCatalogIntegrityTests): 99 cases executed, 99 passed (.analysis/fix-query-diagnostics-refresh/logs/test-QueryDiagnosticsRefreshTests-154956.log).
  • The two new suites on the final source: 13 cases (6 in QueryDiagnosticsRefreshTests, 7 in QueryDiagnosticMessageTests), all passed (.analysis/fix-query-diagnostics-refresh/logs/test-QueryDiagnosticsRefreshTests-155454.log).
  • Fails without the fix: with the setText notification removed, all 5 refresh cases that existed then failed (test-QueryDiagnosticsRefreshTests-145858.log, -145954.log). With the lazy attach and the early-returning clear reverted, underlineBelowTheViewportIsDrawn and clearWithoutUnderlinesDoesNotForceLayout failed (test-QueryDiagnosticsRefreshTests-155225.log).
  • swift test --package-path LocalPackages/CodeEditTextView (the CI gate), rerun on the committed HEAD: XCTest 52 tests, 0 failures; Swift Testing 263 tests in 25 suites, all passed. That includes 6 new EmphasisManagerTests cases: the tooltip covers its text and nothing else, no tooltip without text, removal drops it, a deleted range drops it, it follows a layout change, and an underline on a line not yet laid out is drawn with its stroke once the line lays out.
  • swiftlint lint --strict on every changed Swift file: no violations on added lines. The 12 it reports in the vendored LocalPackages files are all on lines that predate this branch.
  • python3 scripts/localization.py verify and the docs checks (check-writing-style.sh, check-docs-against-source.py) passed.
  • UI tests: none added. A tooltip needs the pointer to rest over a point for the system hover delay, and the rotor needs VoiceOver, so neither runs deterministically under XCUITest. The unit tests drive the real TextViewController.setText and TextBindingSync paths and ask the tooltip owner and the rotor delegate directly.

Notes

Codex review was unavailable (usage limit), so an adversarial reviewer agent read the diff before the commit. On a final read for this PR, these were left as they are:

  • Two problems at the same offset. The rotor steps by start offset, so a second problem starting exactly where another starts is skipped by next and previous. This cannot happen today: the SQL producer emits at most an unmatched closer and an unterminated comment, which never start on the same character, and the MongoDB producer emits at most one diagnostic.
  • Overlapping underlines. The tooltip owner answers with the first registration whose rect contains the point, so where two underlines overlap one message wins. The producers do not emit overlapping ranges, for the same reason.
  • setTextStorage path. TextView.setTextStorage still does not notify coordinators. Nothing in the app replaces the storage that way; every document replacement goes through TextViewController.setText.
  • EmphasisLayer identity. Changing it from a struct to a class makes == identity. The only comparison is the flash removal lookup, which already holds the same instance.

https://claude.ai/code/session_011THKc9TRHE8xjcxXidDHXP

@mintlify

mintlify Bot commented Sep 11, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 11, 2026, 9:24 AM

💡 Tip: Enable Automations to automatically generate PRs for you.

@datlechin
datlechin merged commit 5bf4f6b into main Sep 11, 2026
12 of 14 checks passed
@datlechin
datlechin deleted the fix/query-diagnostics-refresh branch September 11, 2026 13:03
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.

1 participant