Repository navigation
Conversation
|
@mrzv is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
To use Codex here, create a Codex account and connect to 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:
📝 WalkthroughWalkthroughAdds four configurable terminal scrollback actions (page/line up/down): key parsing/matching, UI display, runtime routing to TabManager, schema/web catalog updates, docs/localization, and tests. ChangesScrollback Navigation Shortcuts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 18 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (18 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 four configurable terminal scrollback actions (
Confidence Score: 5/5Safe to merge; the feature is well-isolated, fully tested, and does not touch any shared state paths that could regress existing shortcuts. The new shortcut actions are correctly guarded behind web/app/[locale]/docs/changelog/page.tsx — the changelog localization lookup is coupled to the English prose in CHANGELOG.md. Important Files Changed
Sequence DiagramsequenceDiagram
participant OS as macOS Event System
participant AD as AppDelegate (shortcut monitor)
participant SC as ShortcutStroke.matches(event:)
participant TM as TabManager
participant Term as Terminal Surface
OS->>AD: NSEvent (e.g. Shift+PageUp)
AD->>AD: matchConfiguredShortcut(event:, action: .scrollbackPageUp)
AD->>SC: matches(event:)
SC->>SC: recordableKey(from: event) → "pageup" via NSEvent.SpecialKey
SC->>SC: isPageShortcutKey? → early-return true
SC-->>AD: true
AD->>AD: preferredMainWindowContextForShortcutRouting
AD->>TM: scrollFocusedTerminalScrollbackPage(up: true)
TM->>TM: "captureFocusIntent == .terminal(.surface)?"
alt Terminal surface focused
TM->>Term: performBindingAction("scroll_page_up")
Term-->>TM: true
TM-->>AD: true (event consumed)
else TextBox or browser focused
TM-->>AD: false (event passed through)
end
Reviews (9): Last reviewed commit: "Match page shortcuts from Fn arrow keys" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swift`:
- Around line 213-216: The displayName for the new ShortcutAction cases
(.scrollbackPageUp, .scrollbackPageDown, .scrollbackLineUp, .scrollbackLineDown)
is currently hardcoded in ShortcutAction; replace those literal English strings
with localized lookups (e.g., NSLocalizedString or your project's string catalog
API) using unique keys (e.g., "shortcut.scrollbackPageUp.displayName", etc.) and
update the Localizable.strings (or catalog) with corresponding translations;
ensure the ShortcutAction.displayName computed property returns the localized
string for each of the four cases.
In `@Resources/Localizable.xcstrings`:
- Around line 128677-128693: Update the Japanese localizations for the
scrollback page labels so they are proper translations instead of copied
English: replace the "ja" value for the key shortcut.scrollbackPageDown.label
with "スクロールバックを1ページ下へ" and the "ja" value for shortcut.scrollbackPageUp.label
with "スクロールバックを1ページ上へ"; ensure the "stringUnit.state" remains "translated" for
both entries to match the pattern used by the line scrollback labels.
- Around line 125442-125475: Replace the English placeholders in the Japanese
localizations for the shortcut keys: update the "ja" stringUnit.value for
"shortcut.key.pageDown" and "shortcut.key.pageUp" to proper Japanese
translations (e.g., "ページダウン" and "ページアップ") so they are not identical to the
English values; modify the entries for the keys "shortcut.key.pageDown" and
"shortcut.key.pageUp" in Resources/Localizable.xcstrings to set the ja
localization to the suggested Japanese strings.
🪄 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: 4419f4d7-2393-4cb4-ab9e-5fe2f09152a2
📒 Files selected for processing (11)
Packages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swiftPackages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/GlobalHotkeySection.swiftPackages/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/KeyboardShortcutsSection.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/KeyboardShortcutContext.swiftSources/KeyboardShortcutSettings.swiftSources/TabManager.swiftweb/data/cmux-shortcuts.tsweb/data/cmux.schema.json
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 `@CHANGELOG.md`:
- Line 9: The new English-only changelog line in CHANGELOG.md must be localized
for all supported docs locales: update CHANGELOG.md (or move the entry into the
localized changelog files) so that the same changelog copy exists for every
locale listed in web/i18n/routing.ts; identify the locales from
web/i18n/routing.ts and add translated versions of the "Configurable terminal
scrollback shortcuts..." entry for each locale key, ensuring the localized copy
is used by the docs renderer (match filenames/locale suffixes used by existing
localized changelog entries and follow the existing localization pattern).
🪄 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: 5766f29d-53f8-455a-8889-e10202d032df
📒 Files selected for processing (6)
CHANGELOG.mddocs/configuration.mdskills/cmux-settings/references/shortcut-actions.mdweb/app/[locale]/docs/configuration/page.tsxweb/app/[locale]/docs/keyboard-shortcuts/page.tsxweb/data/cmux-shortcuts.ts
Author
|
@codex review |
@mrzv I can't start this review because your workspace has reached its free monthly review limit. Reviews resume at the start of your next billing cycle. Paid plans include much higher monthly review limits. Upgrade now to resume reviews. |
|
To use Codex here, create a Codex account and connect to github. |
1 similar comment
|
To use Codex here, create a Codex account and connect to github. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Inspect the TabManager scrollback methods
rg -n "scrollFocusedTerminal" Sources/TabManager.swift -A 5
echo "---"
# Check AppDelegate handling
rg -n "scrollbackPage\|scrollbackLine\|scrollback" Sources/AppDelegate.swift -C 3
echo "---"
# Check KeyboardShortcutSettings for the new actions and arrow key handling
rg -n "scrollback\|isArrowShortcut\|usesDirectKeyCode\|pageup\|pagedown" Sources/KeyboardShortcutSettings.swift -C 2
echo "---"
# Check KeyboardShortcutContext
rg -n "scrollback\|nonBrowserPanel" Sources/KeyboardShortcutContext.swift -C 3
echo "---"
# Check for existing Shift+Up / Shift+Down bindings that could conflict
rg -n "shift.*up\|shift.*down\|\.up\|\.down" Sources/KeyboardShortcutSettings.swift -i -C 2 | head -60
echo "---"
# Check the scroll_page_lines action format
rg -n "scroll_page_lines\|scroll_page_up\|scroll_page_down\|performBindingAction" Sources/TabManager.swift -C 3Length of output: 1810 🏁 Script executed: #!/bin/bash
# Check cmux-shortcuts.ts for the new entries
rg -n "scrollback" web/data/cmux-shortcuts.ts -C 3
echo "---"
# Check for existing shift+up/shift+down shortcuts to look for conflicts
rg -n "shift.*up\|shift.*down" web/data/cmux-shortcuts.ts -i -C 2
echo "---"
# Check docs/configuration.md new content
cat docs/configuration.md | tail -60
echo "---"
# Check schema updates
rg -n "scrollback" web/data/cmux.schema.jsonLength of output: 4605 🏁 Script executed: #!/bin/bash
# Get full KeyboardShortcutSettings scrollback section
rg -n "scrollback\|scrollBack\|isArrow\|usesDirectKey\|pageup\|pagedown\|pgup\|pg_up\|page.up" Sources/KeyboardShortcutSettings.swift -i -C 2
echo "==="
# Check performBindingAction signature/usage to understand the format
rg -n "performBindingAction" Sources/ -C 3 | head -60
echo "==="
# Check scroll_page_lines format — is it used elsewhere?
rg -rn "scroll_page_lines\|scroll_page_up\|scroll_page_down" Sources/ --type swift
echo "==="
# Check for existing shift+up / shift+down conflicts in defaults
rg -n "\.up\b\|\.down\b\|shiftUp\|shiftDown\|shift.*arrow\|arrowUp\|arrowDown" Sources/KeyboardShortcutSettings.swift -i -C 3 | head -80
echo "==="
# Inspect ShortcutAction+Defaults.swift for the new entries
fd "ShortcutAction+Defaults.swift" --exec cat {}Length of output: 4466 🏁 Script executed: #!/bin/bash
# Find ShortcutAction files
fd "ShortcutAction" --type f
echo "==="
# Get scrollback-related lines from KeyboardShortcutSettings
fd "KeyboardShortcutSettings.swift" --type f --exec rg -n "scrollback\|pageup\|pagedown\|isArrow\|usesDirectKey" {} -i -C 3
echo "==="
# Check if configValue is used for line shortcuts anywhere
rg -n "configValue\|scrollbackLine" web/data/cmux-shortcuts.ts -C 2
echo "==="
# Find the ShortcutAction+Defaults.swift
fd "ShortcutAction" --type f --exec cat {}Length of output: 19586
Review of PR
|
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f875855185
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@web/app/`[locale]/docs/changelog/page.tsx:
- Line 42: The current strict full-string match for the changelog entry
(variable scrollbackShortcutsChangelogItem) is brittle and will miss small copy
edits; update the lookup/locale-matching logic to use a stable key or a tolerant
comparison instead — e.g., map the entry by a fixed identifier/slug (preferred)
or match using a normalized substring/startsWith/includes check against
scrollbackShortcutsChangelogItem so minor wording/punctuation changes in
CHANGELOG.md won't break the locale lookup; update all similar checks (including
the other changelog items referenced) to use the same stable-key or tolerant
comparison approach.
In `@web/data/cmux-shortcuts.ts`:
- Around line 138-155: The entries for the keyboard shortcuts with id
"scrollbackLineUp" and "scrollbackLineDown" are missing their configValue fields
so the docs UI cannot generate copy-ready snippets; add configValue: "shift+up"
to the scrollbackLineUp object and configValue: "shift+down" to the
scrollbackLineDown object in web/data/cmux-shortcuts.ts so those objects match
the page variants and provide the copy-ready configuration string for the UI.
🪄 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: 377c51c0-fd8b-45e7-b2c6-366460e56231
📒 Files selected for processing (32)
CHANGELOG.mdPackages/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swiftSources/KeyboardShortcutSettings.swiftSources/TabManager.swiftcmuxTests/AppDelegateShortcutRoutingTests.swiftcmuxTests/KeyboardShortcutContextTests.swiftcmuxTests/WorkspaceUnitTests.swiftdocs/configuration.mdweb/app/[locale]/docs/changelog/page.tsxweb/app/[locale]/docs/configuration/page.tsxweb/app/[locale]/docs/keyboard-shortcuts/page.tsxweb/data/cmux-shortcuts.tsweb/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/en.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/ja.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/uk.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
Author
|
@codex review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check current state of cmux-shortcuts.ts for scrollback entries
rg -n "scrollback" web/data/cmux-shortcuts.ts -C 4
echo "==="
# Check TabManager scrollback methods + comment on sign convention
rg -n "scrollFocusedTerminal\|scroll_page_lines\|scroll_page_up\|scroll_page_down" Sources/TabManager.swift -C 4
echo "==="
# Check ShortcutAction+Defaults for scrollback
fd "ShortcutAction+Defaults.swift" --exec rg -n "scrollback" {} -C 3
echo "==="
# Check the test coverage
rg -n "scrollback\|Scrollback" cmuxTests/ -l
echo "==="
# Verify nonBrowserPanel context
rg -n "scrollback\|nonBrowserPanel" Sources/KeyboardShortcutContext.swift -C 3Length of output: 2292 🏁 Script executed: #!/bin/bash
# Find TabManager and check scrollback methods
fd "TabManager.swift" --type f | head -5
echo "==="
fd "TabManager.swift" --type f --exec rg -n "scrollFocused\|scroll_page\|performBinding" {} -C 3
echo "==="
# Find KeyboardShortcutContext
fd "KeyboardShortcutContext.swift" --type f --exec cat {}
echo "==="
# Check test files
fd "WorkspaceUnitTests.swift" --type f --exec rg -n "scrollback" {} -C 2
fd "KeyboardShortcutContextTests.swift" --type f --exec rg -n "scrollback" {} -C 2Length of output: 15063
✅ All previous concerns addressed
🟡 One remaining minor nitIn Overall the PR is well-implemented and ready to merge. 🧠 Learnings used |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc16e15446
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Author
|
@codex review |
|
Based on the prior analysis, the implementation is in great shape — all previously flagged items have been resolved (missing 🧠 Learnings used✅ Action performedReview finished.
|
teamleaderleo
added
S3: minor
This branch has not been 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.
Summary
Add configurable shortcuts for scrolling.
The defaults are Shift+PgUp/PgDown.
In
~/.config/cmux/cmux.json:scrollbackLineUp / scrollbackLineDown are intentionally unbound by default so shells and terminal apps can receive Shift+Up / Shift+Down. The user can bind them, for example:
Testing
Verified manually that the default shortcuts work.
Review Trigger (Copy/Paste as PR comment)
Checklist
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds configurable terminal scrollback shortcuts for page and line scrolling (fixes #4645). Defaults to Shift+Page Up/Down and also matches Fn+Up/Down; line scrolling is unbound by default so shells can receive Shift+Up/Down.
New Features
shortcuts.bindingsin~/.config/cmux/cmux.json; supportspageup/pagedownand aliasespgup/pgdn.Bug Fixes
Written for commit 4009e61. Summary will update on new commits.
Summary by CodeRabbit
New Features
UX
Localization
Documentation
Bug Fix
Tests