iOS: hidden native text input for working hold-to-repeat backspace + dictation (supersedes the hasText approach) - #5596
Closed
lawrencecchen wants to merge 5 commits into
Closed
lawrencecchen wants to merge 5 commits into
lawrencecchen wants to merge 5 commits into
Conversation
…xt-input conformance)
Make the iOS terminal input view behave like a real text input so the system
software keyboard drives backspace auto-repeat and dictation natively, the way
iSH's terminal does, without introducing local-echo drift (keystrokes still
forward to the Mac).
iSH technique (app/TerminalView.m): its `hasText` returns YES unconditionally
("it's always ok to send a delete"), so the keyboard's auto-repeat timer keeps
firing `deleteBackward` while backspace is held. cmux kept a perpetually-empty
document, so the inherited `UITextView.hasText` was false and the repeat stopped
after one delete.
- Override `hasText` -> true so the keyboard auto-repeats backspace.
- Decouple `deleteBackward` routing from `hasText` (now a constant): key off
`markedTextRange` (IME composition) instead, so backspace and the
Ctrl/Alt/Cmd+Backspace ladders still reach the Mac.
- Route committed multi-character blocks (dictation, autocorrect, keyboard
clipboard insert) through a new bracketed-paste path so embedded newlines are
not CR-fragmented. Single chars and Return stay on per-key `terminal.input`.
- Add `terminal.paste` text RPC (Mac side routes to `sendText` ->
`ghostty_surface_text`, the bracketed-paste path) wired through every gate:
TerminalController dispatch + handler, MobileHostService ticket auth,
capabilities, and the iOS RPC client allowlist + MobileShellComposite.
- Extract a pure `TerminalCommitRouter` (input-vs-paste) into
CmuxMobileTerminalKit with unit tests.
Dictation note: iSH hand-rolls `insertDictationResultPlaceholder` /
`removeDictationResultPlaceholder` only because it is a raw UIView conforming to
UITextInput. This view is a UITextView, which already satisfies those UITextInput
requirements as framework witnesses that are not exposed for override (the
compiler rejects `override` on them). So we inherit iSH's placeholder plumbing
for free; recognized dictation text arrives via insertText/textViewDidChange and
routes through the bracketed-paste sink.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…through it Address two autoreview P1 findings: - Consume the new `terminal.paste.v1` capability from `mobile.host.status` in MobileShellComposite (supportsTerminalPaste) and fall back to per-key `terminal.input` (CR-normalized) when a new client pairs with an older Mac, so dictation/predictive/clipboard text is never dropped with method_not_found. - Route the visible accessory Paste button's clipboard text through the same bracketed-paste sink (onPasteText) instead of onText, so multi-line clipboard text no longer fragments into per-line Returns. One shared paste entrypoint. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address autoreview P2: resetTerminalOutputTracking() (fired from remoteClient.didSet when the client clears, on every reconnect) now also clears supportsTerminalPaste, so reconnecting from a paste-capable host to an older one cannot send terminal.paste on a stale true before the next status probe lands. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address Aziz documentation policy: expand the default GhosttySurfaceViewDelegate didPasteText extension to a proper Swift-DocC comment (summary + parameters). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… working hold-to-repeat backspace + dictation Supersedes the hasText-override approach from PR #5573, which shipped but did not deliver native hold-to-repeat backspace or system dictation on device. Keeps #5573's terminal.paste RPC and bracketed-paste routing intact. Root cause of the prior failure: TerminalInputTextView was a UITextView that forced hasText=true but cleared its document to "" after every keystroke. A UITextView drives backspace auto-repeat and dictation off its internal text storage/selection, not off UIKeyInput.hasText, so: - hold-to-repeat stopped after one delete (empty doc, collapsed selection at offset 0 = nothing to repeat-delete; UITextView never consults the overridden hasText for repeat), - dictation never landed (the per-keystroke text="" clear nuked the dictation placeholder mid-recognition). Fix: make the view a bare UIView conforming to UIKeyInput + UITextInput directly with no document, the same construction iSH's TerminalView ships for this exact purpose. A raw responder honors hasText for repeat, and the explicit dictation placeholder methods on a real (non-cleared) UITextInput conformer let recognized text arrive via insertText as one block. - TerminalInputTextView: UITextView -> UIView, UIKeyInput, UITextInput. - hasText -> true; insertText routes straight to the terminal (single char via onText, multi-char block via the existing bracketed-paste onPasteText); deleteBackward keeps the Ctrl/Alt/Cmd ladder + onBackspace (0x7F). - Hand-rolled marked-text storage (markedText string) with setMarkedText/unmarkText/markedTextRange; IME routing keys off markedText. - Full UITextInput stub conformance (neutral offsets/rects) + dictation placeholder hooks; keyboard traits as computed UITextInputTraits. - Opaque TerminalInputTextPosition / TerminalInputTextRange sentinels (one file each per file-organization policy); identity-compared by UIKit. - Removed the buffer-mirror/TerminalTextInputPipeline round-trip and the UITextViewDelegate; adapted the test seam. - Autocorrect/predictive stay disabled (incompatible with per-keystroke remote forwarding). Emoji and IME composition still work via insertText. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR implements bracketed-paste support for terminal text input across the iOS and Mac RPC layers. The change refactors ChangesTerminal bracketed-paste flow
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning, 1 inconclusive)
✅ Passed checks (15 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 98e54b7. Configure here.
| guard let composing = markedText else { return } | ||
| withMarkedTextChange { markedText = nil } | ||
| emitCommittedText(composing, source: "unmarkText") | ||
| } |
There was a problem hiding this comment.
unmarkText emits duplicate IME
High Severity
unmarkText() forwards the marked composition to the remote terminal via emitCommittedText, but UIKit treats unmarkText as clearing IME state only; committed text is delivered separately through insertText or replace. That can duplicate characters on commit or send in-progress syllables when the user cancels composition.
Reviewed by Cursor Bugbot for commit 98e54b7. Configure here.
Contributor
Greptile SummaryThis PR replaces the
Confidence Score: 4/5The core input-responder refactor is architecturally sound and closely follows the iSH pattern. All new RPC paths have capability-gated fallbacks for older Mac hosts. The two findings are non-blocking: the file-size overage is a house-rule cleanup, and the composition-not-cleared edge in replace(_:withText:) requires two mutually exclusive input modes to be active simultaneously and is practically unreachable given the disabled autocorrect/predictive-text configuration. The documentless responder is internally coherent — hasText, markedText, sentinel ranges, and the delegate callbacks are wired consistently. The RPC capability flag is correctly reset on disconnect. The replace(_:withText:) gap is a theoretical path (autocorrect off, predictive off, simultaneous IME + accessibility text injection) that won't surface in normal use. The only concrete rule violation is the file size, which is addressable post-merge. TerminalInputTextView.swift warrants a second look for the replace(_:withText:) composition-clearing gap and for the 800-line rule overage; all other files are straightforward additions or small wiring changes. Important Files Changed
Sequence DiagramsequenceDiagram
participant KB as iOS Keyboard
participant TV as TerminalInputTextView
participant GS as GhosttySurfaceView
participant MSC as MobileShellComposite
participant RPC as MobileCoreRPCClient
participant TC as TerminalController (Mac)
Note over KB,TV: Single character typed
KB->>TV: insertText("a")
TV->>GS: onText("a") → didProduceInput
GS->>MSC: submitTerminalRawInput
MSC->>RPC: terminal.input
Note over KB,TV: Hold backspace (auto-repeat)
KB->>TV: "deleteBackward() [repeating, hasText==true]"
TV->>GS: onBackspace → didProduceInput(0x7F)
GS->>MSC: submitTerminalRawInput
Note over KB,TV: Dictation / multi-char block
KB->>TV: insertDictationResultPlaceholder()
TV-->>KB: "" (token)
KB->>TV: insertText("recognized phrase")
TV->>GS: onPasteText → didPasteText
GS->>MSC: submitTerminalPasteText
alt supportsTerminalPaste
MSC->>RPC: terminal.paste
RPC->>TC: v2MobileTerminalPaste
TC->>TC: sendText (ghostty_surface_text)
else fallback (old Mac host)
MSC->>RPC: terminal.input (newlines→CR)
end
Note over KB,TV: IME composition (CJK)
KB->>TV: setMarkedText("한", ...)
TV->>TV: "markedText = "한""
KB->>TV: insertText("한")
TV->>TV: clear markedText
TV->>GS: onText("한") [single char]
Reviews (1): Last reviewed commit: "iOS terminal input: documentless UIKeyIn..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+1068
to
1181
| // MARK: - UITextInput (documentless conformance) | ||
|
|
||
| // This view owns no editable document. It implements `UITextInput` to unlock two | ||
| // keyboard features that a bare `UIKeyInput` view does not get — system | ||
| // dictation (the mic key) and IME marked-text composition — exactly the way | ||
| // iSH's `TerminalView` does. Recognized dictation text and committed IME | ||
| // candidates both arrive through ``insertText(_:)`` and route to the terminal; | ||
| // the geometry/offset methods return neutral values because there is nothing to | ||
| // measure. UIKit compares the marked/selected ranges by object identity. | ||
| extension TerminalInputTextView { | ||
| var markedTextRange: UITextRange? { markedText != nil ? markedTextRangeSentinel : nil } | ||
|
|
||
| var selectedTextRange: UITextRange? { | ||
| get { selectedTextRangeSentinel } | ||
| set {} | ||
| } | ||
|
|
||
| func textViewDidChange(_ textView: UITextView) { | ||
| handleTextChange( | ||
| currentText: textView.text ?? "", | ||
| isComposing: textView.markedTextRange != nil | ||
| ) | ||
| var markedTextStyle: [NSAttributedString.Key: Any]? { | ||
| get { nil } | ||
| set {} | ||
| } | ||
|
|
||
| var beginningOfDocument: UITextPosition { TerminalInputTextPosition() } | ||
| var endOfDocument: UITextPosition { TerminalInputTextPosition() } | ||
|
|
||
| /// The IME hands a candidate string in; hold it as the marked composition so | ||
| /// ``markedTextRange`` reports active composition. Nothing is sent to the | ||
| /// terminal until the candidate commits via ``insertText(_:)`` (driven by | ||
| /// the keyboard) or ``unmarkText()``. | ||
| /// | ||
| /// Mutating ``markedText`` changes the string the view exposes through | ||
| /// ``text(in:)``/``markedTextRange``, so it is a *text* change in the | ||
| /// ``UITextInputDelegate`` contract: it is bracketed with | ||
| /// `textWillChange`/`textDidChange` (via ``withMarkedTextChange(_:)``) so the | ||
| /// IME and dictation machinery keep their composition state synchronized. | ||
| func setMarkedText(_ markedText: String?, selectedRange: NSRange) { | ||
| TerminalInputDebugLog.log("proxy.setMarkedText text=\(TerminalInputDebugLog.textSummary(markedText ?? "")) ") | ||
| withMarkedTextChange { | ||
| self.markedText = (markedText?.isEmpty == true) ? nil : markedText | ||
| } | ||
| } | ||
|
|
||
| /// Brackets a mutation of ``markedText`` with the `UITextInputDelegate` | ||
| /// text-change callbacks. | ||
| /// | ||
| /// The marked composition is the only text this view exposes, so any change | ||
| /// to it (set by the IME, committed by ``insertText(_:)``/``unmarkText()``, | ||
| /// or canceled by ``replace(_:withText:)``) is a text change UIKit must be | ||
| /// told about with `textWillChange`/`textDidChange`. Selection-only callbacks | ||
| /// would leave the keyboard observing stale composition state. | ||
| private func withMarkedTextChange(_ mutate: () -> Void) { | ||
| inputDelegate?.textWillChange(self) | ||
| mutate() | ||
| inputDelegate?.textDidChange(self) | ||
| } | ||
|
|
||
| /// Commit the in-progress IME composition. Forwards the held candidate to the | ||
| /// terminal as one block (multi-character commits route to bracketed paste). | ||
| func unmarkText() { | ||
| guard let composing = markedText else { return } | ||
| withMarkedTextChange { markedText = nil } | ||
| emitCommittedText(composing, source: "unmarkText") | ||
| } | ||
|
|
||
| func text(in range: UITextRange) -> String? { | ||
| if range === markedTextRangeSentinel { return markedText } | ||
| if range === selectedTextRangeSentinel { return "" } | ||
| return nil | ||
| } | ||
|
|
||
| /// Commit text delivered through a range replacement. | ||
| /// | ||
| /// Most committed input arrives via ``insertText(_:)``, but some system | ||
| /// paths (text replacement, certain dictation/suggestion commits) deliver it | ||
| /// by replacing ``selectedTextRange`` or ``markedTextRange`` instead. The | ||
| /// view holds no addressable document, so the range itself is ignored, but | ||
| /// the *text* must still reach the terminal — route it through the same | ||
| /// commit path as ``insertText(_:)`` rather than dropping it. A replacement | ||
| /// of the marked region also supersedes the in-progress IME composition, so | ||
| /// clear it first. An empty replacement is a pure deletion of the marked | ||
| /// composition (no committed text to send). | ||
| func replace(_ range: UITextRange, withText text: String) { | ||
| TerminalInputDebugLog.log("proxy.replace text=\(TerminalInputDebugLog.textSummary(text)) marked=\(range === markedTextRangeSentinel)") | ||
| if range === markedTextRangeSentinel, markedText != nil { | ||
| withMarkedTextChange { markedText = nil } | ||
| } | ||
| guard !text.isEmpty else { return } | ||
| emitCommittedText(text, source: "replace") | ||
| } | ||
| func textRange(from fromPosition: UITextPosition, to toPosition: UITextPosition) -> UITextRange? { nil } | ||
| func position(from position: UITextPosition, offset: Int) -> UITextPosition? { nil } | ||
| func position(from position: UITextPosition, in direction: UITextLayoutDirection, offset: Int) -> UITextPosition? { nil } | ||
| func compare(_ position: UITextPosition, to other: UITextPosition) -> ComparisonResult { .orderedSame } | ||
| func offset(from: UITextPosition, to toPosition: UITextPosition) -> Int { 0 } | ||
| func position(within range: UITextRange, farthestIn direction: UITextLayoutDirection) -> UITextPosition? { nil } | ||
| func characterRange(byExtending position: UITextPosition, in direction: UITextLayoutDirection) -> UITextRange? { nil } | ||
| func baseWritingDirection(for position: UITextPosition, in direction: UITextStorageDirection) -> NSWritingDirection { .leftToRight } | ||
| func setBaseWritingDirection(_ writingDirection: NSWritingDirection, for range: UITextRange) {} | ||
| func firstRect(for range: UITextRange) -> CGRect { .zero } | ||
| func caretRect(for position: UITextPosition) -> CGRect { .zero } | ||
| func selectionRects(for range: UITextRange) -> [UITextSelectionRect] { [] } | ||
| func closestPosition(to point: CGPoint) -> UITextPosition? { nil } | ||
| func closestPosition(to point: CGPoint, within range: UITextRange) -> UITextPosition? { nil } | ||
| func characterRange(at point: CGPoint) -> UITextRange? { nil } | ||
|
|
||
| // MARK: Dictation placeholder hooks | ||
| // | ||
| // UIKit calls these when the mic is tapped. Returning a placeholder (an | ||
| // empty token; iSH does the same) is what tells the framework this view | ||
| // accepts dictation; the recognized text then arrives via `insertText`. The | ||
| // remove hook is a no-op because there is no document placeholder to strip. | ||
| func insertDictationResultPlaceholder() -> Any { "" } | ||
| func removeDictationResultPlaceholder(_ placeholder: Any, willInsertResult: Bool) {} | ||
| } |
Contributor
There was a problem hiding this comment.
TerminalInputTextView.swift is now 1181 lines, against a prior baseline of ~908 lines, and both are over the 800-line threshold flagged by the file-package-boundaries rule. The UITextInput documentless conformance section (lines 1068–1181) and the UITextInputTraits extension (lines 1050–1066) are independently comprehensible units with no private member access that can't be exposed through internal. Splitting them into TerminalInputTextView+UITextInput.swift and TerminalInputTextView+UITextInputTraits.swift would bring the core responder file under the limit without any logic change.
Rule Used: Flag Swift changes that add too much unrelated res... (source)
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!
Comment on lines
+1149
to
1156
| func replace(_ range: UITextRange, withText text: String) { | ||
| TerminalInputDebugLog.log("proxy.replace text=\(TerminalInputDebugLog.textSummary(text)) marked=\(range === markedTextRangeSentinel)") | ||
| if range === markedTextRangeSentinel, markedText != nil { | ||
| withMarkedTextChange { markedText = nil } | ||
| } | ||
| guard !text.isEmpty else { return } | ||
| emitCommittedText(text, source: "replace") | ||
| } |
Contributor
There was a problem hiding this comment.
selectedTextRange during IME
replace(_:withText:) correctly clears markedText when range === markedTextRangeSentinel, but not when range === selectedTextRangeSentinel while markedText != nil. If a system path (e.g., an accessibility "type text" action) calls replace(selectedTextRange, withText:) during active IME composition, emitCommittedText fires immediately while markedText remains set. The IME will later commit its candidate a second time via insertText or unmarkText, producing a double-send to the terminal. In practice this requires simultaneous use of two input modes and is very unlikely given autocorrectionType = .no, but a defensive if markedText != nil { withMarkedTextChange { markedText = nil } } before the emitCommittedText call would make the behavior symmetric with the markedTextRangeSentinel branch.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift (1)
5-1181: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftSplit
TerminalInputTextView.swift; the touched production file violates the repository size policy.This file is now well past the hard 800-line threshold and carries multiple responsibilities (input transport policy, toolbar UI wiring, modifier-state application, paste/image flows, and
UITextInputprotocol surface) in one unit, which materially increases regression risk and review cost.As per coding guidelines, "Flag Swift production files that exceed 400 lines without a clear single responsibility, or exceed 800 lines even with mostly coherent responsibility."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift` around lines 5 - 1181, The file TerminalInputTextView.swift is >800 lines and mixes UI, input protocol conformance, accessory toolbar, modifier handling, and paste logic; split it into focused files: keep the core responder class TerminalInputTextView with only high-level wiring and methods like insertText(_:), deleteBackward(), emitCommittedText(_:source:), and modifier toggles, then move accessory toolbar construction (terminalAccessoryToolbar, populateAccessoryActions(), makeAccessoryButton(...), applyModifierPresentation(), updateAccessoryLayoutInsets()) into a new TerminalAccessoryToolbar (or TerminalInputToolbar) file/class, move paste handling (handlePasteAction()) into TerminalInputPasteHandler, and extract UITextInput conformance plumbing (markedText, setMarkedText(_:selectedRange:), withMarkedTextChange(_:), unmarkText(), replace(_:withText:), text(in:) and dictation hooks) into TerminalInputTextView+UITextInput.swift; ensure shared helpers (AccessoryActionButton usage, onPasteText/onPasteImage/onEscapeSequence callbacks, modifierState references like modifierState.tap/consumeIfNotSticky and methods toggleControlModifier/toggleAlternateModifier) remain accessible (adjust visibility or add internal APIs) and update Notification observer registration to the initializer in the core class. Keep all symbol names unchanged so callers (e.g. handleAccessoryButton(_:), handleAccessoryAction(_:), applyModifierPresentation(), emitCommittedText(_:source:), insertText(_:), setKeyboardShown(_:)) still link; run a build and fix access control/imports after the refactor.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`:
- Around line 1462-1464: The current fallback branch in MobileShellComposite
(guard supportsTerminalPaste -> submitTerminalRawInput) naively replaces "\n"
with "\r" causing Windows CRLF ("\r\n") to become "\r\r"; first normalize CRLF
by replacing occurrences of "\r\n" with a single "\r", then replace any
remaining "\n" with "\r" before calling submitTerminalRawInput (refer to
symbols: supportsTerminalPaste, text, normalized, submitTerminalRawInput,
workspace.id, terminalID).
---
Outside diff comments:
In
`@Packages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swift`:
- Around line 5-1181: The file TerminalInputTextView.swift is >800 lines and
mixes UI, input protocol conformance, accessory toolbar, modifier handling, and
paste logic; split it into focused files: keep the core responder class
TerminalInputTextView with only high-level wiring and methods like
insertText(_:), deleteBackward(), emitCommittedText(_:source:), and modifier
toggles, then move accessory toolbar construction (terminalAccessoryToolbar,
populateAccessoryActions(), makeAccessoryButton(...),
applyModifierPresentation(), updateAccessoryLayoutInsets()) into a new
TerminalAccessoryToolbar (or TerminalInputToolbar) file/class, move paste
handling (handlePasteAction()) into TerminalInputPasteHandler, and extract
UITextInput conformance plumbing (markedText, setMarkedText(_:selectedRange:),
withMarkedTextChange(_:), unmarkText(), replace(_:withText:), text(in:) and
dictation hooks) into TerminalInputTextView+UITextInput.swift; ensure shared
helpers (AccessoryActionButton usage, onPasteText/onPasteImage/onEscapeSequence
callbacks, modifierState references like modifierState.tap/consumeIfNotSticky
and methods toggleControlModifier/toggleAlternateModifier) remain accessible
(adjust visibility or add internal APIs) and update Notification observer
registration to the initializer in the core class. Keep all symbol names
unchanged so callers (e.g. handleAccessoryButton(_:), handleAccessoryAction(_:),
applyModifierPresentation(), emitCommittedText(_:source:), insertText(_:),
setKeyboardShown(_:)) still link; run a build and fix access control/imports
after the refactor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b2762df3-39d7-41ca-9d8c-14f3a1be5872
📒 Files selected for processing (12)
Packages/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileCoreRPCClient.swiftPackages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/GhosttySurfaceRepresentable.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/GhosttySurfaceView.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextPosition.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextRange.swiftPackages/CmuxMobileTerminal/Sources/CmuxMobileTerminal/TerminalInputTextView.swiftPackages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalCommitRoute.swiftPackages/CmuxMobileTerminalKit/Sources/CmuxMobileTerminalKit/TerminalCommitRouter.swiftPackages/CmuxMobileTerminalKit/Tests/CmuxMobileTerminalKitTests/TerminalCommitRouterTests.swiftSources/Mobile/MobileHostService.swiftSources/TerminalController.swift
Comment on lines
+1462
to
+1464
| guard supportsTerminalPaste else { | ||
| let normalized = text.replacingOccurrences(of: "\n", with: "\r") | ||
| await submitTerminalRawInput(normalized, workspaceID: workspace.id, terminalID: terminalID) |
There was a problem hiding this comment.
Normalize CRLF before CR fallback conversion.
Line 1463 replaces \n with \r directly, so Windows-style \r\n becomes \r\r on old-host fallback and can inject extra Returns.
Suggested fix
- let normalized = text.replacingOccurrences(of: "\n", with: "\r")
+ let normalized = text
+ .replacingOccurrences(of: "\r\n", with: "\n")
+ .replacingOccurrences(of: "\r", with: "\n")
+ .replacingOccurrences(of: "\n", with: "\r")
await submitTerminalRawInput(normalized, workspaceID: workspace.id, terminalID: terminalID)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Packages/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift`
around lines 1462 - 1464, The current fallback branch in MobileShellComposite
(guard supportsTerminalPaste -> submitTerminalRawInput) naively replaces "\n"
with "\r" causing Windows CRLF ("\r\n") to become "\r\r"; first normalize CRLF
by replacing occurrences of "\r\n" with a single "\r", then replace any
remaining "\n" with "\r" before calling submitTerminalRawInput (refer to
symbols: supportsTerminalPaste, text, normalized, submitTerminalRawInput,
workspace.id, terminalID).
lawrencecchen
added a commit
that referenced
this pull request
Jun 8, 2026
…extInput input, supersedes #5573 hasText) into dog bundle
lawrencecchen
added a commit
that referenced
this pull request
Jun 11, 2026
Reset to origin/main (composer #5876 landed), then merge old dog HEAD a300868 to preserve every feature not yet on main: notifications dismiss-sync (#5568), multi-Mac switcher hardening (#5545), foreground repaint (#5571), image paste too-large toast (#5572), hidden native input (#5596), workspace groups (#5625), wslist round-10 snapshot, scroll-to-bottom hysteresis, DEV dogfood pane, attachments button, arrow toolbar keys, terminal.paste capability gating. Conflict policy: main's reviewed composer-land form wins for composer core (keyed focus handshake, draft FIFO coalescing, paste submit partial-success), dog wins for unlanded feature surface. ghostty pinned to dog 34cbf18 (descendant of main's e5c962a). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 12, 2026
Carries unlanded: multi-Mac #5545, notif-sync #5568, foreground-repaint #5571, image-paste toast #5572, hidden-input #5596, groups (iOS side) #5625, scroll-hysteresis, dogfood pane, capabilities superset. Main's reviewed forms win: TerminalController decomposition (Control*Context), notif-tap-deeplink #5927, mobile.terminal.* routing.
lawrencecchen
added a commit
that referenced
this pull request
Jun 13, 2026
…#5596/#5625/#5628) over current main Beta queue (#5876/#5872/#5869/#5875/#5927/#5912/#5726/#5776/#5916) is now on main; conflicts resolved by taking main as authoritative for the merged workspace-list/notifications/read-state/close surface, while preserving the carry-set: terminal.paste capability (#5572), hidden-input strings (#5596), smooth-scroll/scroll-to-bottom (#5628), and the live notifications feed (notificationsStore + mobile.notifications.list/mark_read dispatch). Dropped the superseded mute design. Capability flags unified onto main's computed supportedHostCapabilities set (added computed supportsTerminalPaste + DEBUG supportsDogfoodChecklist). xcstrings merged (HEAD-precedence union, mute keys dropped). pbxproj took HEAD consistently; budget regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
lawrencecchen
added a commit
that referenced
this pull request
Jun 15, 2026
Reconcile the branch's per-Bool capability flags onto main's supportedHostCapabilities Set model: add a computed supportsTerminalPaste property + terminalPasteCapability constant, advertise terminal.paste.v1 from the Mac capabilities extension, and keep main's identity-recovery on status-probe failure. deleteBackward keeps both the shift one-shot consumption (markedText == nil) and the branch's marked-text composition cancel.
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
PR #5573 tried to give the iOS terminal native hold-to-repeat backspace and system dictation by overriding
hasTextto returntrueon the existingTerminalInputTextView(aUITextViewsubclass), copying iSH's "it's always ok to send a delete" trick. It shipped but on a real device neither hold-to-repeat backspace nor dictation worked. (Its other half, multi-line paste viaterminal.paste, does work and is kept.)Real reason
hasText = truefailedTerminalInputTextViewwas aUITextViewthat forcedhasText = truebut cleared its document to""after every committed keystroke (the buffer-mirror /TerminalTextInputPipelineround-trip). AUITextViewdrives backspace auto-repeat and dictation off its internal text storage and selection, not offUIKeyInput.hasText:UITextViewnever consults the overriddenhasTextfor repeat. (ThehasTextoverride is only honored by a rawUIKeyInputresponder, which is what iSH actually is.)text = ""clear nuked the placeholder mid-recognition.iSH's
TerminalViewis not aUITextViewat all: it is a bareUIViewconforming toUIKeyInput+UITextInputwith no real document. That is why both features work there.Fix: documentless
UIKeyInput+UITextInputresponderTerminalInputTextViewis nowUIView, UIKeyInput, UITextInputwith no editable document, the same construction iSH ships. On a raw responder the keyboard honorshasTextfor repeat, and the explicit dictation-placeholder methods on a real (non-cleared)UITextInputconformer let recognized text arrive throughinsertTextas one block.Why this produces native repeat + dictation on device where the old approach did not: the auto-repeat timer fires
deleteBackwardas long as a raw first responder reportshasText == true(now a constant), and there is no document/selection state machine fighting it; dictation's placeholder hook returns a token and the recognized text comes back throughinsertTextwith nothing to clear it mid-flight.How input is forwarded to the terminal (byte semantics unchanged from #5573):
insertText->onText->terminal.input(CR for Return, control bytes preserved).deleteBackward->onBackspace->\u{7F}. Ctrl/Alt/Cmd+Backspace ladders intact (Cmd+Backspace = Ctrl+U0x15, etc.).insertText(orreplace(_:withText:)) ->onPasteText-> the existingterminal.pastebracketed-paste RPC, so embedded newlines do not CR-fragment.UIViewdoes not inheritUITextView's paste), routed through the same clipboard handling as the toolbar Paste button.0x09, Shift-TabESC [ Z, arrows, control/alt/cmd ladders, and the toolbar/modifier state machine are unchanged.Marked text (IME composition) is hand-rolled (
markedTextstring + opaqueUITextRange/UITextPositionsentinels). Marked-text mutations are bracketed with theUITextInputDelegatetextWillChange/textDidChangecallbacks so the keyboard/dictation machinery stays synchronized.Known limitation (documented, by design)
A documentless view cannot decompose a CJK syllable itself. Composition edits are normally driven by the IME through
setMarkedText(the IME owns its floating candidate bar; this invisible view'smarkedTextjust mirrors it). In the rare case where an IME routes composition-backspace throughdeleteBackwardinstead, this view cancels the in-progress composition rather than corrupting it (modeling한as a SwiftCharacterand dropping it would nuke the whole syllable). Autocorrect/predictive text stay disabled and cannot be enabled: they require retaining the in-progress word, which is incompatible with forwarding every keystroke to a remote terminal. Emoji and IME composition still work.Relationship to #5573
This supersedes #5573's input-conformance part (the
hasTextoverride) and keeps #5573'sterminal.pasteRPC and bracketed-paste routing. It is built on #5573's branch, so this PR carries those commits too; merging this delivers working repeat + dictation + paste together.Verification
iOS simulator Debug build is clean (arm64). The simulator cannot exercise hold-to-repeat or dictation, so those are device-verified by the dogfood checklist below. Autoreview converged clean (documentless responder is internally coherent; paste routes restored; marked-text delegate callbacks corrected).
Device dogfood checklist
setMarkedTextit decomposes; if it routes throughdeleteBackwardthe composition cancels. Report if cancel is disruptive.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes the core remote terminal input path and adds a new Mac RPC handler; risk is mitigated by capability detection, per-key fallback on old hosts, and unchanged single-character/modifier semantics.
Overview
Replaces the iOS terminal keyboard proxy
TerminalInputTextViewfrom aUITextViewthat cleared its document after every keystroke with a documentlessUIViewconforming toUIKeyInput+UITextInput(iSH-style). That change is what makes hold-to-repeat backspace honorhasTextand lets dictation complete via placeholder/insertTextwithout mid-flight clears. IME composition is tracked only in transientmarkedTextwith sentinelUITextPosition/UITextRangetypes; Cmd+V and toolbar paste are re-wired on the bare view.Multi-character commits (dictation, clipboard paste, etc.) are classified by new
TerminalCommitRouter(>1Character→ paste) and forwarded throughonPasteText→submitTerminalPasteText, which calls the Macterminal.pasteRPC whenterminal.paste.v1is advertised (otherwise newline-normalizedterminal.input). The Mac addsv2MobileTerminalPaste, advertises the capability inmobile.host.status, and includes paste in attach-ticket terminal auth; RPC client auth routing treats paste like other terminal methods.Per-keystroke typing, modifiers, and image paste paths are unchanged aside from routing clipboard text through bracketed paste instead of per-character input.
Reviewed by Cursor Bugbot for commit 98e54b7. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Enables native hold-to-repeat backspace and system dictation on iOS by replacing the terminal input with a hidden, documentless
UIViewthat implementsUIKeyInput/UITextInput. Multi-character commits (dictation, autocorrect, keyboard paste) now go through a newterminal.pasteRPC with capability gating and safe fallback.New Features
hasText = trueon a raw responder.UITextInputplaceholder hooks and commits as one block.terminal.pasteRPC; capabilityterminal.paste.v1is consumed. Falls back toterminal.inputwith newline→CR when unsupported.GhosttySurfaceViewaddsdidPasteText; accessory Paste and Cmd+V route through the same path.Refactors
TerminalInputTextView→ documentlessUIViewconforming toUIKeyInput/UITextInput; no local buffer; IME marked text stored; customTerminalInputTextRange/TerminalInputTextPosition.onText; multi-char blocks viaonPasteTextusingTerminalCommitRouter(inCmuxMobileTerminalKit) with tests.terminal.paste: RPC allowlist inMobileCoreRPCClient/MobileHostService, handler inTerminalController, capability reset on reconnect.Written for commit 98e54b7. Summary will update on new commits.
Summary by CodeRabbit
New Features