Support RTL direction in native text composers#5008
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds an NSTextView extension to configure and apply ChangesNatural Writing Direction for Composed Text
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 16 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (16 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 |
![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 561bee0. Configure here.
| textView.textContainerInset = TextBoxLayout.textInset | ||
| textView.textContainer?.lineFragmentPadding = 0 | ||
| textView.registerForDraggedTypes([.fileURL]) | ||
| textView.configureCmuxNaturalWritingDirectionForComposedText() |
There was a problem hiding this comment.
Redundant triple-configuration of writing direction on creation
Low Severity
configureCmuxNaturalWritingDirectionForComposedText() is called three times during TextBoxInputTextView creation: once in init (line 3894), once in makeNSView right after init (line 3693), and once in updateTextView called from makeNSView (line 3755). The call at line 3693 is completely redundant with the init. Additionally, the call at line 3755 in updateTextView runs on every SwiftUI update cycle and is redundant with refreshInlineAttachmentCells one line above, which already sets typingAttributes including .paragraphStyle via currentTextAttributes().
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 561bee0. Configure here.
There was a problem hiding this comment.
Fixed by relying on TextBoxInputTextView init for creation-time setup and removing the redundant make/update configure calls; refreshInlineAttachmentCells still preserves the natural paragraph style through currentTextAttributes().
— Claude Code
Greptile SummaryAdds natural writing direction (RTL/LTR auto-resolution) to all cmux-owned AppKit text composers. A new
Confidence Score: 5/5Safe to merge — changes are confined to AppKit text layout properties with no effect on data, auth, or terminal rendering. All five changed files touch only AppKit paragraph/writing-direction attributes on NSTextView instances. The new helpers are small, well-scoped, and correctly applied: configure is called once at view creation and apply is called only when text is programmatically mutated. The change does not touch concurrency, data persistence, or any shared mutable state. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant SwiftUI as SwiftUI Binding
participant Rep as NSViewRepresentable
participant ContainerView as Container NSView
participant TV as NSTextView
Note over ContainerView,TV: makeNSView / init path
SwiftUI->>Rep: makeNSView
Rep->>ContainerView: init
ContainerView->>TV: configureCmuxNaturalWritingDirection()
Rep->>TV: "string = text"
Rep->>TV: applyCmuxNaturalWritingDirection()
Note over SwiftUI,TV: updateNSView path (text changed)
SwiftUI->>Rep: updateNSView (new text)
Rep->>TV: "string = text"
Rep->>TV: applyCmuxNaturalWritingDirection()
Note over TV: TextBoxInputTextView internal paths
TV->>TV: clearContent → configure
TV->>TV: installAttributedContent → apply
Reviews (2): Last reviewed commit: "Address composer RTL review feedback" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| textView.refreshInlineAttachmentCells(font: font, foregroundColor: foregroundColor) | ||
| textView.configureCmuxNaturalWritingDirectionForComposedText() | ||
| textView.recenterSingleLineTextContainer() |
There was a problem hiding this comment.
configureCmuxNaturalWritingDirectionForComposedText() is now called inside updateTextView, which runs on every updateNSView invocation — font changes, color refreshes, and any unrelated binding update all trigger it. Each call allocates an NSMutableParagraphStyle, copies the typingAttributes dictionary, and writes both defaultParagraphStyle and typingAttributes back. TextBoxInputTextView.init already calls this in the designated initializer, and the makeNSView setup block calls it once on creation, so direction state is established before any update cycle. Placing it here adds continuous allocation pressure on the main thread for no change in behavior. Compare FeedbackComposerMessageEditor, which correctly limits the configure call to makeNSView and only calls the apply variant when text actually changes.
| textView.refreshInlineAttachmentCells(font: font, foregroundColor: foregroundColor) | |
| textView.configureCmuxNaturalWritingDirectionForComposedText() | |
| textView.recenterSingleLineTextContainer() | |
| textView.refreshInlineAttachmentCells(font: font, foregroundColor: foregroundColor) | |
| textView.recenterSingleLineTextContainer() |
Rule Used: Flag SwiftUI changes that can cause stale state, b... (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!
There was a problem hiding this comment.
Fixed by removing the hot-path configure call from updateNSView. The text view is configured when created, and refreshInlineAttachmentCells keeps the same natural paragraph style in typing attributes.
— Claude Code
| ?? NSMutableParagraphStyle() | ||
| paragraphStyle.alignment = .natural | ||
| paragraphStyle.baseWritingDirection = .natural | ||
| return paragraphStyle.copy() as? NSParagraphStyle ?? paragraphStyle |
There was a problem hiding this comment.
NSMutableParagraphStyle.copy() always returns NSParagraphStyle (the immutable type), so as? NSParagraphStyle never fails and the ?? paragraphStyle branch is dead code. The fallback would silently return the mutable copy (an NSMutableParagraphStyle upcast) in an impossible code path, which also defeats the immutability guarantee the copy was meant to provide. The trailing fallback can be removed.
| return paragraphStyle.copy() as? NSParagraphStyle ?? paragraphStyle | |
| return paragraphStyle.copy() as! NSParagraphStyle |
There was a problem hiding this comment.
Fixed by returning the immutable paragraph-style copy directly with the AppKit-guaranteed cast.
— Claude Code
![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
🤖 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 `@Sources/NaturalTextCompositionDirection.swift`:
- Line 31: Replace the force cast on paragraphStyle.copy() with a safe cast and
fallback: call paragraphStyle.copy(), attempt to cast to NSParagraphStyle using
as?, and return the result or fall back to the original paragraphStyle;
reference the paragraphStyle variable, its copy() call, and NSParagraphStyle in
this change to satisfy SwiftLint's force_cast rule.
🪄 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: a96995f5-1181-4cb4-876f-2af97ed817af
📒 Files selected for processing (2)
Sources/NaturalTextCompositionDirection.swiftSources/TextBoxInput.swift
💤 Files with no reviewable changes (1)
- Sources/TextBoxInput.swift
| ?? NSMutableParagraphStyle() | ||
| paragraphStyle.alignment = .natural | ||
| paragraphStyle.baseWritingDirection = .natural | ||
| return paragraphStyle.copy() as! NSParagraphStyle |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Avoid the force cast flagged by SwiftLint.
copy() returns Any; while the cast is safe here, the force cast trips the force_cast rule. Use a safe cast with a fallback (an NSMutableParagraphStyle is itself an NSParagraphStyle, so it satisfies the return type).
♻️ Proposed fix
- return paragraphStyle.copy() as! NSParagraphStyle
+ return paragraphStyle.copy() as? NSParagraphStyle ?? paragraphStyle📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return paragraphStyle.copy() as! NSParagraphStyle | |
| return paragraphStyle.copy() as? NSParagraphStyle ?? paragraphStyle |
🧰 Tools
🪛 SwiftLint (0.63.2)
[Error] 31-31: Force casts should be avoided
(force_cast)
🤖 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 `@Sources/NaturalTextCompositionDirection.swift` at line 31, Replace the force
cast on paragraphStyle.copy() with a safe cast and fallback: call
paragraphStyle.copy(), attempt to cast to NSParagraphStyle using as?, and return
the result or fall back to the original paragraphStyle; reference the
paragraphStyle variable, its copy() call, and NSParagraphStyle in this change to
satisfy SwiftLint's force_cast rule.
1 participant
Summary
Scope
This PR delivers Part 1 of #5007: cmux native prompt/message composition uses natural writing direction so Hebrew/Arabic paragraphs can resolve RTL while Latin remains LTR.
Part 2 is intentionally out of scope: the terminal pane is Ghostty's cell grid, and BiDi reordering belongs upstream in Ghostty. This PR does not attempt to patch the embedded terminal grid.
Verification
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Low Risk
Localized AppKit text-view configuration and call sites; no auth, data, or terminal rendering changes.
Overview
Adds
NaturalTextCompositionDirection.swift, anNSTextViewextension that sets natural base writing direction, alignment, default paragraph style, and typing attributes, plusapplyCmuxNaturalWritingDirectionToComposedText()to re-apply those settings across existingtextStoragewhen text is set from SwiftUI.That setup is wired into cmux-owned native composers: command palette and feedback editors in
ContentView.swift, feed inline editors inFeedPanelView.swift, and the mainTextBoxInputpath (TextBoxInputTextViewinit, clear, draft install, sync, andcurrentTextAttributes). The Xcode project registers the new source file.This is Part 1 of #5007 (RTL/LTR in native composition); the embedded terminal grid is unchanged.
Reviewed by Cursor Bugbot for commit 3aa84a1. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Enable natural RTL/LTR composition in native text composers so Hebrew/Arabic render RTL while Latin stays LTR. Adds a shared
NSTextViewextension and applies it across the agent prompt, feedback editor, inline feed editor, and main input.NaturalTextCompositionDirection.swiftwithNSTextViewhelpers to set.naturalbase writing direction and alignment, update paragraph style/typing attributes, and reapply to existing text storage.TextBoxInputTextViewconfigures ininit, on clear, and aftersetContent, andcurrentTextAttributesnow includes the natural paragraph style so new typing inherits direction.Written for commit 3aa84a1. Summary will update on new commits.
Summary by CodeRabbit