Skip to content

fix(editor): keep word delete and move on whole characters - #2727

Merged
datlechin merged 1 commit into
mainfrom
fix/word-delete-surrogate-pairs
Sep 11, 2026
Merged

fix(editor): keep word delete and move on whole characters#2727
datlechin merged 1 commit into
mainfrom
fix/word-delete-surrogate-pairs

Conversation

@datlechin

Copy link
Copy Markdown
Member

Found while investigating #2717 (PR #2724).

Option+Delete, Option+Forward Delete and Option+Left/Right in the SQL and JSON editors could split a character in two. Deleting the word éx (e + U+0301) left a lone combining accent. Deleting after '%😀abc removed abc plus half of the emoji, which left an unpaired surrogate that draws as a replacement box. Control+Delete on 💯 deleted one half of the pair. Each of these leaves exactly the kind of invisible or broken character #2717 was about.

Root cause

TextSelectionManager.extendSelectionWord walks the text with NSString.enumerateSubstrings(options: .byCaretPositions), which hands back one grapheme cluster per step, and grew an NSRange by substring.count on each step. NSRange counts UTF-16 code units, and String.count counts grapheme clusters. So every character wider than one UTF-16 unit (a combining sequence, anything outside the BMP, an emoji with a modifier or ZWJ, a flag) made the range one or more units short. Backward, the start landed inside the first character of the word. Forward, the end landed inside the last one. The same range drives word delete, word move and word select, so all six commands were off.

extendSelectionCharacter with decomposeCharacters (Control+Delete, deleteBackwardByDecomposingPreviousCharacter:) returned a single UTF-16 unit. That separates a combining mark correctly, but it also cut a surrogate pair in half. AppKit's NSTextView removes one Unicode scalar here: all of 😀, only the skin tone modifier of 👍🏽, only U+0301 of é (measured with a probe against NSTextView.doCommand(by:)).

What changed

All in LocalPackages/CodeEditTextView, SelectionManipulation+Horizontal.swift:

  • The word range is the union of the ranges the enumerator reports (substringRange), which are already in UTF-16 units. No counting.
  • The decomposing character range takes one Unicode scalar: two units when the unit at the caret is half of a valid surrogate pair, one unit otherwise. The non-decomposing path asks rangeOfComposedCharacterSequence(at:) directly.
  • findBeginningOfLineText stepped the same way (one unit per caret position). It now moves to the end of the enumerator's range instead. I found no input where the old stepping gave a wrong answer, since every horizontal whitespace character is a single unit, but it was the same unit mix.

TextSelectionManagerTests had two expectations that encoded the bug: on "Loren Ipsum 💯", a decomposing step over 💯 expected (13, 1) and (12, 1), which is half of the surrogate pair. They now expect (12, 2), the whole scalar, matching NSTextView.

Verification

  • swift test --package-path LocalPackages/CodeEditTextView (the CI gate), on the committed HEAD: XCTest 52 tests, 0 failures; Swift Testing 269 tests in 26 suites, all passed. Log: scratchpad/word-delete-surrogates-package-tests-publish.log.
  • New suite WordNavigationUnicodeTests: 12 tests, 84 cases. Eight parameterized tests run 10 strings each (combining marks, naïve, Deseret and math-italic letters outside the BMP, emoji with a skin tone, a ZWJ family, emoji before and after an identifier) through the real TextView responder methods: deleteWordBackward(_:), deleteWordForward(_:), moveWordLeft(_:), moveWordRight(_:) and both AndModifySelection variants, plus the raw word range. Four cases cover the decomposing character step. All 84 pass.
  • Baseline: the same tests against the pre-fix SelectionManipulation+Horizontal.swift fail with 83 issues across 11 of the 12 tests. The one that passes on both is decomposingBackwardSeparatesACombiningMark, which guards that the scalar fix still splits é the way AppKit does. TextSelectionManagerTests fails 3 assertions on the old code ({13, 1} and {12, 1}). Log: scratchpad/word-delete-surrogates-baseline-publish.log.
  • App: verify.sh build succeeded (.analysis/fix-word-delete-surrogate-pairs/logs/build-TablePro-154459.log). verify.sh test over the editor suites EditorContextMenuRoutingTests and StatementNavigationCommandTests: 18 cases passed (.analysis/fix-word-delete-surrogate-pairs/logs/test-EditorContextMenuRoutingTests-144530.log).
  • swiftlint lint --strict: clean on the changed source file and the new test file. TextSelectionManagerTests.swift reports 17 xct_specific_matcher violations, the same 17 as on main, none on the edited lines.
  • UI tests: none added. The package tests call the same responder methods the key bindings dispatch to, on a real TextView, so a UI test would only add keyboard driving of astral-plane input.

Notes

Codex review was unavailable (usage limit), so an adversarial reviewer agent read the diff and probed the old and new word logic side by side. What it raised and what I did with it:

  • Emoji next to an identifier. After 😀abc, Option+Delete now removes 😀abc, while NSTextView removes only abc. The editor's word rule treats an emoji as part of a word because it is neither punctuation nor whitespace. That rule predates this change and is a policy question, not the defect: the old code removed abc and half of the emoji. The tests accept either range, so a later change to the word rule does not have to rewrite them.
  • Flags. 🇻🇳abc used to lose abc plus the trailing surrogate of the second regional indicator. It now goes with the word as a whole, by the same rule as above.
  • A caret already inside a character. If the caret sits between e and U+0301, a forward step starts mid-cluster and still separates them. The editor's own movement commands never leave the caret there, so this is not reachable from the keyboard and is left alone.
  • CRLF. \r\n is one caret position of two units; the reviewer confirmed the old and new ranges are identical for word moves across it.
  • Not fixed here, separate path: double-clicking ❤️ (U+2764 U+FE0F) selects U+2764 alone and leaves the variation selector outside the selection. That is findWordBoundary, the double-click word selection, which this PR does not touch. Worth a follow-up since deleting that selection leaves an invisible U+FE0F behind.

https://claude.ai/code/session_011THKc9TRHE8xjcxXidDHXP

@datlechin
datlechin merged commit 0e9f937 into main Sep 11, 2026
7 checks passed
@datlechin
datlechin deleted the fix/word-delete-surrogate-pairs branch September 11, 2026 08:50
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