Skip to content

fix(editor): leave keys to the input method while the SQL editor has marked text - #2730

Merged
datlechin merged 1 commit into
mainfrom
fix/ime-composition-key-monitors
Sep 11, 2026
Merged

fix(editor): leave keys to the input method while the SQL editor has marked text#2730
datlechin merged 1 commit into
mainfrom
fix/ime-composition-key-monitors

Conversation

@datlechin

Copy link
Copy Markdown
Member

Found while investigating #2717 (PR #2724).

Root cause

The SQL editor handles several keys in local NSEvent monitors. A local monitor sees a keyDown before the window hands it to the first responder, so it runs before NSTextInputContext gets a chance to give the key to the input method. None of the monitors asked whether a composition was in progress:

  • TextViewController.handleEvent took Escape (show completions or close the find panel), Control-Space, Shift-Tab, Option-Up and Option-Down, and ran its Command chords (Command-/, Command-[, Command-], Command-Shift-D, Command-Shift-K, Command-Control-J) over the text around the marked range.
  • The completion list (SuggestionController) took Escape, Up, Down, Return and Tab.
  • The find panel (FindPanelHostingView) closed on Escape, even when a text field or the editor was composing.
  • InlineSuggestionManager accepted the AI suggestion on Tab, and requested and showed suggestions while text was still marked.

So with Pinyin, Kana, Hangul or a dead key, Escape could not cancel a composition, Tab went to the editor or the AI suggestion, and with the completion list open, Return, Tab and the arrows moved or applied the list instead of the candidates. The editor acted on the key and the input method never saw it.

A second defect sat under the first. When an input method empties its composition (Backspace over the last marked character, or Escape), it calls setMarkedText(""). CodeEditTextView.TextView kept a zero-length marked range after that call, so hasMarkedText() stayed true. NSTextView ends the composition instead; measured with a probe on this machine:

after le: true {7, 2} "SELECT le"
after empty: false {7, 0} "SELECT " {7, 0}

Left alone, that stale range would have made the new guards withhold the editor's keys until the next committed character.

What changed

  • Every editor key monitor now returns the event untouched while the text view has marked text, so the input method gets Escape, Tab, Return, the arrows and Control-Space.
  • The editor's own Command chords are consumed and run nothing mid-composition. They are not passed on, because Command-[ and Command-] are also Previous Page and Next Page in the Query menu, and letting them through would page the grid while the user is composing. Command chords the editor does not own (Command-S, Command-C, Command-Option-[) still reach the menu bar.
  • The editor's chord table moved into EditorKeyCommand, so the normal path and the composition path read one list instead of two copies that could drift.
  • The find panel's Escape monitor leaves the key alone when the first responder is composing, whether that is the editor or the panel's own search field.
  • InlineSuggestionManager requests nothing while text is marked, drops a reply that arrives after a composition began, and on Tab mid-composition dismisses the suggestion unaccepted and lets the input method have the key.
  • TextView.setMarkedText with an empty string now clears the marked text at every cursor, matching NSTextView.

Verification

TableProTests, through verify.sh --no-wait --root <worktree>:

  • Build: .analysis/fix-ime-composition-key-monitors/logs/build-TablePro-161007.log, BUILD SUCCEEDED.
  • Final run on the committed tree: .analysis/fix-ime-composition-key-monitors/logs/test-EditorKeyMonitorCompositionTests-161741.log, 55 cases passed, 0 failed (EditorKeyMonitorCompositionTests 49, InlineSuggestionManagerCompositionTests 6).
  • Neighbouring suites: test-EditorKeyMonitorCompositionTests-161600.log, 68 cases passed, 0 failed, adding InlineSuggestionManagerFocusTests 6 and CellOverlayEditorMovementTests 7.
  • The tests fail without the fix. With the first-round changes reverted (test-EditorKeyMonitorCompositionTests-152650.log), 16 cases failed across 7 tests while the settled-text controls passed. With the second-round changes reverted (test-EditorKeyMonitorCompositionTests-161444.log), 19 cases failed: the 6 withheld Command chords, 11 emptied-composition cases, plus the inline suggestion's re-request after an emptied composition and its Tab mid-composition.

The editor tests drive the real TextViewController, SuggestionController and FindPanelHostingView in a window: they begin a composition with setMarkedText and send real keyDown events through each monitor's handler, then check the event's fate, the text and the marked range. Each key is also checked with nothing composing, to prove the editor still claims it.

Packages:

  • swift test --package-path LocalPackages/CodeEditTextView (the CI gate): 260 tests in 25 suites and 52 XCTest cases passed, including the 3 new IMEInputTests cases for an emptied composition (single cursor, a composition begun after an emptied one, and several cursors).
  • swift test --package-path LocalPackages/CodeEditSourceEditor --filter over the four suggestion suites (SuggestionApplyTests, SuggestionControllerPlacementTests, SuggestionPanelFocusTests, TextViewControllerSuggestionsTests): 17 tests, 1 skipped, 0 failures.

SwiftLint --strict on every changed Swift file: no violation on an added line. The ones it reports in the vendored packages (implicit_return, identifier_name on MAX_VISIBLE_ROWS, sorted_imports in IMEInputTests) are on lines this PR does not touch.

UI tests: none. A composition needs a system input source selected, and switching the input source from the test runner changes the keyboard state of the whole machine, so it cannot run deterministically on CI. The unit tests drive the same NSTextInputClient calls an input method makes.

Notes

Codex was unavailable (usage limit), so an adversarial reviewer agent read the diff instead. It raised two defects in the first version, both fixed here:

  • An emptied composition left hasMarkedText() true for good, so the new guards would have withheld Escape and Tab until the next committed character. Fixed in CodeEditTextView itself rather than worked around in each monitor.
  • The first version passed the editor's Command chords on mid-composition, which let Command-[ and Command-] fire Previous Page and Next Page. They are now consumed.

The SwiftLint findings above are left as they are: they sit in the vendored forks, which stay close to upstream so they can take its changes.

https://claude.ai/code/session_011THKc9TRHE8xjcxXidDHXP

@datlechin
datlechin merged commit abfc700 into main Sep 11, 2026
11 of 13 checks passed
@datlechin
datlechin deleted the fix/ime-composition-key-monitors branch September 11, 2026 13:04
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