Repository navigation
Add iOS custom toolbar macros - #6924
Closed
austinywang wants to merge 13 commits into
Closed
austinywang wants to merge 13 commits into
austinywang wants to merge 13 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds iOS toolbar actions for text, key combos, and macros. It introduces new payload and macro-step types, updates action output encoding, expands the toolbar editor UI to switch modes and edit macro steps, and adds localization and tests. ChangesiOS toolbar macros
Estimated code review effort: 4 (Complex) | ~60 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ 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 |
Contributor
Greptile SummaryThis PR adds iOS custom toolbar macros, extending the existing text-only toolbar action to support three modes: literal text, a single modified key combo, and a multi-step macro that concatenates step outputs. It also fixes an index-out-of-range crash when deleting macro steps (#6087) by switching to element bindings keyed on stable step UUIDs instead of index-backed bindings.
Confidence Score: 5/5Safe to merge — well-validated model layer, correct ForEach deletion fix, and complete localization coverage. The macro model correctly guards against partial output (any unencodable step causes the whole macro to return nil), the ForEach element-binding approach is the right fix for the deletion crash, all 24 new string keys carry both en and ja translations, and the new tests cover concatenation, rejection, Codable round-trips, and persistence. No actor isolation, blocking-primitive, or ambient-global-state issues were found in the new production sources. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User taps Save] --> B{isValid?}
B -- No --> C[Save disabled]
B -- Yes --> D{mode}
D -- .text --> E[ToolbarActionPayload.text]
D -- .keyCombo --> F[ToolbarActionPayload.keyCombo]
D -- .macro --> G[Collect macroSteps]
G --> H{All steps valid?}
H -- No --> I[macroPayload = nil, disabled]
H -- Yes --> J[ToolbarActionPayload.macro steps]
E --> K[payload.output]
F --> K
J --> K
K --> L{switch payload}
L -- .text --> M[normalize newline to CR]
L -- .keyCombo --> N[TerminalKeyEncoder.encode]
L -- .macro --> O{for each step output}
O -- any nil --> P[whole macro = nil]
O -- all non-nil --> Q[Data sequence concat]
M --> R[CustomToolbarAction.output send to terminal]
N --> R
Q --> R
%%{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"}}}%%
flowchart TD
A[User taps Save] --> B{isValid?}
B -- No --> C[Save disabled]
B -- Yes --> D{mode}
D -- .text --> E[ToolbarActionPayload.text]
D -- .keyCombo --> F[ToolbarActionPayload.keyCombo]
D -- .macro --> G[Collect macroSteps]
G --> H{All steps valid?}
H -- No --> I[macroPayload = nil, disabled]
H -- Yes --> J[ToolbarActionPayload.macro steps]
E --> K[payload.output]
F --> K
J --> K
K --> L{switch payload}
L -- .text --> M[normalize newline to CR]
L -- .keyCombo --> N[TerminalKeyEncoder.encode]
L -- .macro --> O{for each step output}
O -- any nil --> P[whole macro = nil]
O -- all non-nil --> Q[Data sequence concat]
M --> R[CustomToolbarAction.output send to terminal]
N --> R
Q --> R
Reviews (13): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
added 3 commits
June 26, 2026 21:40
…-key-combos-macros-in-the-toolbar-e
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CustomToolbarActionEditorView.swift (1)
246-249: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
String.localizedStringWithFormatinstead ofString(format:)for the localized step title.
L10n.stringreturns a plainString, and the format argument (index) is applied separately viaString(format:). Based on prior repo guidance, this overload pattern should useString.localizedStringWithFormatso numeral/positional formatting respects the user's locale, rather than plainString(format:).💬 Proposed fix
private func macroStepTitle(index: Int) -> String { let format = L10n.string("mobile.toolbar.editor.stepHeaderFormat", defaultValue: "Step %d") - return String(format: format, index) + return String.localizedStringWithFormat(format, index) }🤖 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/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CustomToolbarActionEditorView.swift` around lines 246 - 249, The localized step title in macroStepTitle(index:) is using String(format:), which can ignore locale-specific number formatting. Update that helper to use String.localizedStringWithFormat with the format returned by L10n.string so the step label respects the user’s locale while keeping the same stepHeaderFormat behavior.Source: Learnings
🤖 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.
Outside diff comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CustomToolbarActionEditorView.swift`:
- Around line 246-249: The localized step title in macroStepTitle(index:) is
using String(format:), which can ignore locale-specific number formatting.
Update that helper to use String.localizedStringWithFormat with the format
returned by L10n.string so the step label respects the user’s locale while
keeping the same stepHeaderFormat behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 36769dd5-2e55-4213-bdbf-3da860f2a51a
📒 Files selected for processing (2)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/CustomToolbarActionEditorView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalSpecialKey+ToolbarEditor.swift
Iterate the macro editor over element bindings (ForEach($macroSteps)) instead of index-backed $macroSteps[index]. A macro row can delete itself, and an index captured in a binding goes stale the instant the array shrinks; SwiftUI can re-read that binding during the Form diff and crash with an index-out-of-range (most reproducibly when deleting a middle or last step). Element bindings stay valid across removals, and the row now deletes and numbers itself by step id. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
macroStepTitle builds a user-facing 'Step N' header. Use String.localizedStringWithFormat so the index respects the user's locale, matching the app's dominant localized-format idiom. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-key-combos-macros-in-the-toolbar-e
The Vercel preview deployment for the prior merge commit was created but never picked up by a builder (commit status stuck 'pending', never updated), while recent main commits deploy fine. Empty commit to re-trigger a fresh deploy. No source changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-key-combos-macros-in-the-toolbar-e
…-key-combos-macros-in-the-toolbar-e
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
3 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.
Fixes #6087
Summary
Tests
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds iOS custom toolbar macros so a button can send text, a key combo, or a multi-step macro. Validates and encodes actions, rejects unsupported/partial macros, localizes the editor (incl. “Step N”), and fixes a macro-step deletion crash for #6087.
New Features
CmuxMobileTerminalKit: addedToolbarActionMacroStepand.macroinToolbarActionPayload; moved output encoding to payload/steps; normalized newlines to\r; concatenated step outputs;CustomToolbarAction.outputnow delegates to payload.CmuxMobileShellUI: editor adds an Action Type picker (Text, Key Combo, Macro), key-combo fields with Shift/Control/Option toggles and a localized key picker with an “unsupported combo” hint, and a macro step editor with add/remove, per-step kind, stable step IDs, and localized titles including “Step N”.TerminalAccessoryConfigurationpersists and keeps macro actions enabled; tests cover macro concatenation and rejection of unsupported/partial/empty macros; new localized strings and special-key display names.Bug Fixes
Written for commit 4737f62. Summary will update on new commits.
Summary by CodeRabbit