iOS: restore hold-to-repeat Backspace on the terminal keyboard - #6288
lawrencecchen wants to merge 2 commits into
Conversation
The iOS terminal input view (TerminalInputTextView) is a UITextView with a perpetually-empty document. The software keyboard only keeps auto-repeating deleteBackward() while the first responder reports hasText == true; the inherited empty-document hasText reads false, so holding Backspace deletes exactly one character and then stops. This test asserts hasText is a constant true (the keyboard's repeat gate) and that each repeat tick reaches the Mac. It is red on current main: the inherited UITextView.hasText returns false for the empty document. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Holding Backspace on the iOS terminal keyboard deleted one character and then stopped instead of repeat-deleting while held. TerminalInputTextView is a UITextView that keeps a perpetually-empty local document (every keystroke is forwarded to the Mac, then the buffer is cleared). The software keyboard's auto-repeat timer only keeps firing deleteBackward() while the first responder reports hasText == true, and the inherited empty-document hasText read false, so the repeat died after the first delete. The working fix was developed in the unmerged native-input PRs (#5573 / #5596, documentless variant in commit 2577f23) but never landed on main, so main has the plain UITextView with no hasText override. This applies the minimal, principled piece of that work that fits a UITextView (iSH's technique): - Override hasText -> true so the keyboard keeps auto-repeating Backspace ("it's always ok to send a delete"). - Re-key deleteBackward() off markedTextRange (IME composition in progress) instead of hasText. hasText is now a forced constant, so the armed-modifier guards drop their !hasText condition and the local-edit branch routes on markedTextRange != nil only. While composing, the delete edits the marked text locally; otherwise it is a real backspace forwarded to the Mac on every repeat tick. Makes the red test from the prior commit pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesHold-to-repeat Backspace contract and implementation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryFixes hold-to-repeat Backspace on the iOS terminal keyboard by overriding
Confidence Score: 4/5Safe to merge; the change is confined to one view, the logic inside deleteBackward() is semantically identical to the pre-fix code for every execution path, and the new tests lock the repeat contract. The only behavioral change outside the fix itself is that UIKit's hasText contract is intentionally violated — the document is always empty but hasText always returns true. That could enable Select All and similar editing-menu items via canPerformAction on an always-empty document. Given the view's invisible text/cursor and the terminal surface's gesture handling, this is unlikely to be user-visible, but it hasn't been explicitly guarded against. Packages/iOS/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift — the hasText override at line 72 and its interaction with UIKit's canPerformAction editing-menu path. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant KB as iOS Keyboard Timer
participant TITV as TerminalInputTextView
participant Mac as Mac Terminal
Note over KB: User holds Backspace key
KB->>TITV: hasText? → true (allows repeat)
KB->>TITV: deleteBackward() [tick 1]
alt "Modifier armed (e.g. ⌘) and markedTextRange == nil"
TITV->>Mac: onEscapeSequence (e.g. Ctrl+U)
else "IME composing (markedTextRange != nil)"
TITV->>TITV: super.deleteBackward()
else Normal (no modifier, not composing)
TITV->>Mac: onBackspace()
end
KB->>TITV: hasText? → true (keyboard keeps repeating)
KB->>TITV: deleteBackward() [tick 2…N]
TITV->>Mac: onBackspace() (every tick)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant KB as iOS Keyboard Timer
participant TITV as TerminalInputTextView
participant Mac as Mac Terminal
Note over KB: User holds Backspace key
KB->>TITV: hasText? → true (allows repeat)
KB->>TITV: deleteBackward() [tick 1]
alt "Modifier armed (e.g. ⌘) and markedTextRange == nil"
TITV->>Mac: onEscapeSequence (e.g. Ctrl+U)
else "IME composing (markedTextRange != nil)"
TITV->>TITV: super.deleteBackward()
else Normal (no modifier, not composing)
TITV->>Mac: onBackspace()
end
KB->>TITV: hasText? → true (keyboard keeps repeating)
KB->>TITV: deleteBackward() [tick 2…N]
TITV->>Mac: onBackspace() (every tick)
Reviews (1): Last reviewed commit: "iOS terminal input: restore hold-to-repe..." | Re-trigger Greptile |
| /// Because `hasText` is now a forced constant, internal byte-routing must NOT | ||
| /// key off it; ``deleteBackward()`` and its modifier guards key off | ||
| /// ``markedTextRange`` (IME composition in progress) instead. | ||
| override var hasText: Bool { true } |
There was a problem hiding this comment.
hasText == true enables UIKit editing-menu items on an always-empty document
UITextView.canPerformAction(_:withSender:) uses hasText to gate "Select All" (and in some UIKit versions "Cut"/"Copy") in the long-press editing menu. With hasText forced to true, a user long-pressing on the terminal surface could see "Select All" and related menu items at all times, even though the document is always empty and any selection would be over zero characters.
In practice the risk is low because the terminal surface captures long-press gestures before the UITextView magnifier fires, textColor and tintColor are both .clear, and autocorrectionType/spellCheckingType are off. Worth verifying that a long-press on the soft-keyboard input area doesn't accidentally surface the editing menu, since UIKit's hasText contract is now intentionally violated here.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…hor (#6299) * ios: sustain hold-to-repeat Backspace via a virtual delete-repeat anchor Hold-to-repeat Backspace on the iOS soft keyboard never repeated. Device debugging confirmed the cause: with an EMPTY virtual document, UIKit no-ops the keyboard's software delete and never calls deleteBackward() while the key is held, so backspace did not reach the Mac. Forcing hasText == true alone (the earlier #6288 attempt) does not change this. Fix: convert TerminalInputTextView from a UITextView subclass to a bare UIView conforming to UIKeyInput + UITextInput that exposes a one-character virtual document. When not composing it shows a hidden zero-width "delete-repeat anchor" (toggling \u{200B}/\u{2060}); each empty-buffer deleteBackward() forwards a real backspace via onBackspace and brackets the anchor toggle in inputDelegate.textWillChange/textDidChange so UIKit re-arms its document-driven key-repeat timer. IME composition (markedText) suppresses the anchor; a delete during composition cancels the composition instead of forwarding a stray backspace. Supersedes the hasText-only approach in #6288 (can be closed in favor of this); related to #6238. Single commit (not red/green): the fix is a whole-view rewrite from UITextView to UIView/UIKeyInput/UITextInput, so the failing test and the view it tests are inseparable. The new TerminalInputBackspaceRepeatTests asserts the observable invariants that sustain the repeat (non-empty 1-char document when idle, N deletes => N backspaces with the document re-armed non-empty after each, the textWillChange/textDidChange re-arm firing per delete, the anchor char alternating, and composing suppressing both the anchor and any stray backspace), so reverting to UITextView, dropping the anchor, or removing the re-arm all fail it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Refresh Swift file-length budget for documentless input rewrite TerminalInputTextView grew with the hand-rolled UIKeyInput/UITextInput conformance (the proven delete-repeat fix). Accepting as known debt; splitting the conformance into an extension file is a follow-up (file-org). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ios: shorten IME composition on composing-delete instead of cancelling While composing CJK/Japanese/Korean/pinyin text the documentless input view cancelled the entire marked composition on the first Backspace and emitted nothing, so mid-composition correction was impossible (en+ja are supported locales). Restore the prior UITextView behavior: deleteBackward() during composition now drops the last grapheme (Character, not a UTF-16 unit, so multi-scalar glyphs are never split) and re-presents the shortened candidate via setMarkedText, which brackets the change in textWillChange/textDidChange and derives markedTextRange/selectedTextRange from the new string so UIKit and the IME stay in sync. Removing the last unit clears/unmarks the composition. Still emits zero bytes to the Mac while composing; the non-composing forward-DEL + anchor re-arm path is unchanged. Update the regression test: a multi-char composition shortens by one per delete (still composing, zero onBackspace), clears on the last unit, and a subsequent non-composing delete forwards a backspace again. Adds a grapheme-boundary case (flag emoji = one grapheme, multiple UTF-16 units). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Refresh Swift file-length budget after IME composing-delete fix Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: conform mock UITextInputDelegate to the iOS 18.4 SDK (conversationContext) main's bump to the iOS 26 SDK made conversationContext(_:didChange:) a required UITextInputDelegate method; the backspace-repeat test's mock delegate didn't implement it, failing the cmuxFeatureTests build. Add the unused stub. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hor (#6299)
* ios: sustain hold-to-repeat Backspace via a virtual delete-repeat anchor
Hold-to-repeat Backspace on the iOS soft keyboard never repeated. Device
debugging confirmed the cause: with an EMPTY virtual document, UIKit no-ops
the keyboard's software delete and never calls deleteBackward() while the key
is held, so backspace did not reach the Mac. Forcing hasText == true alone
(the earlier #6288 attempt) does not change this.
Fix: convert TerminalInputTextView from a UITextView subclass to a bare
UIView conforming to UIKeyInput + UITextInput that exposes a one-character
virtual document. When not composing it shows a hidden zero-width
"delete-repeat anchor" (toggling \u{200B}/\u{2060}); each empty-buffer
deleteBackward() forwards a real backspace via onBackspace and brackets the
anchor toggle in inputDelegate.textWillChange/textDidChange so UIKit re-arms
its document-driven key-repeat timer. IME composition (markedText) suppresses
the anchor; a delete during composition cancels the composition instead of
forwarding a stray backspace.
Supersedes the hasText-only approach in
manaflow-ai/cmux#6288 (can be closed in favor of
this); related to manaflow-ai/cmux#6238.
Single commit (not red/green): the fix is a whole-view rewrite from
UITextView to UIView/UIKeyInput/UITextInput, so the failing test and the view
it tests are inseparable. The new TerminalInputBackspaceRepeatTests asserts
the observable invariants that sustain the repeat (non-empty 1-char document
when idle, N deletes => N backspaces with the document re-armed non-empty
after each, the textWillChange/textDidChange re-arm firing per delete, the
anchor char alternating, and composing suppressing both the anchor and any
stray backspace), so reverting to UITextView, dropping the anchor, or removing
the re-arm all fail it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Refresh Swift file-length budget for documentless input rewrite
TerminalInputTextView grew with the hand-rolled UIKeyInput/UITextInput
conformance (the proven delete-repeat fix). Accepting as known debt; splitting the
conformance into an extension file is a follow-up (file-org).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* ios: shorten IME composition on composing-delete instead of cancelling
While composing CJK/Japanese/Korean/pinyin text the documentless input
view cancelled the entire marked composition on the first Backspace and
emitted nothing, so mid-composition correction was impossible (en+ja are
supported locales). Restore the prior UITextView behavior: deleteBackward()
during composition now drops the last grapheme (Character, not a UTF-16
unit, so multi-scalar glyphs are never split) and re-presents the shortened
candidate via setMarkedText, which brackets the change in
textWillChange/textDidChange and derives markedTextRange/selectedTextRange
from the new string so UIKit and the IME stay in sync. Removing the last
unit clears/unmarks the composition. Still emits zero bytes to the Mac while
composing; the non-composing forward-DEL + anchor re-arm path is unchanged.
Update the regression test: a multi-char composition shortens by one per
delete (still composing, zero onBackspace), clears on the last unit, and a
subsequent non-composing delete forwards a backspace again. Adds a
grapheme-boundary case (flag emoji = one grapheme, multiple UTF-16 units).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Refresh Swift file-length budget after IME composing-delete fix
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: conform mock UITextInputDelegate to the iOS 18.4 SDK (conversationContext)
main's bump to the iOS 26 SDK made conversationContext(_:didChange:) a required
UITextInputDelegate method; the backspace-repeat test's mock delegate didn't
implement it, failing the cmuxFeatureTests build. Add the unused stub.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirror of PR #6288. Force hasText=true so the keyboard's auto-repeat timer keeps firing deleteBackward while Backspace is held (empty UITextView reports hasText false, stopping repeat after one delete). Re-key deleteBackward branches off markedTextRange (IME composition) instead of hasText. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Holding Backspace on the iOS terminal keyboard deleted one character and then stopped, instead of repeat-deleting continuously while held.
Root cause
TerminalInputTextViewis aUITextViewthat keeps a perpetually-empty local document: every committed keystroke is forwarded to the Mac, then the local buffer is cleared. The software keyboard's auto-repeat timer for Backspace only keeps firingdeleteBackward()while the first responder reportshasText == true. With the inherited empty-documenthasTextreadingfalse, the repeat died after the first delete.The working hold-to-repeat mechanism was developed in the unmerged native-input PRs (#5573, #5596, documentless variant in commit
2577f2312) but never landed onmain. Somainstill has the plainUITextViewwith nohasTextoverride and Backspace routing that gates onhasText. This is the regression.Fix
Applies the minimal, principled piece of that prior work that fits a
UITextView(iSH's technique):hasText -> trueso the keyboard keeps auto-repeating Backspace ("it's always ok to send a delete").deleteBackward()offmarkedTextRange(IME composition in progress) instead ofhasText. SincehasTextis now a forced constant, the armed-modifier guards drop their!hasTextcondition and the local-edit branch routes onmarkedTextRange != nilonly. While composing, the delete edits the marked text locally; otherwise it is a real backspace forwarded to the Mac on every repeat tick.Principled, not a hack: it restores the documented mechanism the prior fix used and removes the now-meaningless
hasTextbyte-routing rather than papering over it. The IME composition path (Japanese/CJK) is preserved by keying local edits offmarkedTextRange.Tests
Two-commit red/green structure in
ios/cmuxPackage/Tests/cmuxFeatureTests/TerminalInputBackspaceRepeatTests.swift:hasText == true, the keyboard's repeat gate). Red onmainbecause the inherited empty-documenthasTextisfalse.Residual risk
Low. The change is confined to one view. The armed-modifier Backspace ladders (⌘/⌥/⌃/⇧ + Backspace) are exercised by the existing
TerminalInputAccessoryShiftTests; those guards now key offmarkedTextRangeonly, which is the same condition they already required (they previously also required!hasText, which was always true for the empty document, so behavior is unchanged for the non-composing case). Dictation/autocorrect committed blocks are unaffected (they rideinsertText/textChange, notdeleteBackward).Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Single-view input routing change; non-composing behavior matches the prior empty-document case, with IME guarded via marked text only.
Overview
Holding Backspace on the iOS terminal software keyboard only deleted once because UIKit stops auto-repeating
deleteBackward()whenhasTextis false, and the proxy’s localUITextViewstays empty after each keystroke is forwarded to the Mac.TerminalInputTextViewnow overrideshasTextto always returntrue(same approach as iSH) so the keyboard keeps firing repeat deletes.deleteBackward()no longer useshasTextfor routing: accessory-modifier branches drop their!hasTextchecks, and local vs Mac backspace is decided only bymarkedTextRange(IME composition still edits marked text locally).New
TerminalInputBackspaceRepeatTestsasserthasTextis always true and that five successivedeleteBackward()calls each invokeonBackspacewhen not composing.Reviewed by Cursor Bugbot for commit 318abaa. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Restores hold-to-repeat Backspace on the iOS terminal keyboard so holding the key continuously deletes characters as expected. Forces the input view to always report
hasText == trueand updates delete handling to respect IME composition.trueforhasTextto keep the keyboard’s Backspace auto-repeat active.deleteBackwardbymarkedTextRangeto preserve IME; removehasTextchecks and forward real backspace on each repeat when not composing.hasText == trueand that repeated deletes reach the terminal.Written for commit 318abaa. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests