Repository navigation
Show more notification content in the sidebar - #7965
Merged
Merged
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:
📝 WalkthroughWalkthroughA new sidebar setting controls the maximum lines shown for the latest notification preview. The setting is validated, persisted, exposed through the settings UI and configuration schema, indexed for navigation, and applied to workspace subtitle rendering. ChangesNotification preview line limit
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsUI
participant SettingsStore
participant ContentView
SettingsUI->>SettingsStore: update notificationMessageLineLimit
SettingsStore->>ContentView: provide persisted setting
ContentView->>ContentView: clamp limit and select subtitle
ContentView->>SettingsUI: render bounded notification preview
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 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 |
…otification-content
Contributor
Greptile SummaryThis PR adds configurable notification preview length to the workspace sidebar. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (9): Last reviewed commit: "Label notification preview line stepper" | Re-trigger Greptile |
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
`@Packages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/SidebarSection.swift`:
- Around line 13-20: The non-private `@State` properties in SidebarSection
conflict with SwiftLint’s private_swiftui_state rule. Keep state private in
SidebarSection and update the SidebarSection+NotificationPreview row builder to
receive the required values as parameters, preserving the existing notification
preview behavior.
In `@web/data/cmux.schema.json`:
- Around line 989-995: Add a stable descriptionKey of
schemaDescriptions.sidebar.notificationMessageLineLimit to the
notificationMessageLineLimit schema entry, then add matching English and
Japanese translations to the schema-description catalogs in web/messages/en.json
and web/messages/ja.json. Keep the existing description and schema behavior
unchanged, and do not modify Resources/Localizable.xcstrings.
🪄 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: 8b332aba-643a-4ab5-bb9e-98891888e8ea
📒 Files selected for processing (13)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Keys/SidebarCatalogSection.swiftPackages/macOS/CmuxSettings/Tests/CmuxSettingsTests/UserDefaultsSettingsClientTests.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Navigation/CuratedSettingEntry+Default.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/SidebarSection+NotificationPreview.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/SidebarSection.swiftPackages/macOS/CmuxSettingsUI/Tests/CmuxSettingsUITests/SettingsRowAnchorResolutionTests.swiftResources/Localizable.xcstringsSources/CmuxSettingsJSONPathSupport.swiftSources/ContentView.swiftSources/KeyboardShortcutSettingsFileStore+Template.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/SettingsNavigation.swiftweb/data/cmux.schema.json
Comment on lines
+13
to
+20
| @State var hideAll: DefaultsValueModel<Bool> | ||
| @State private var wrapTitles: DefaultsValueModel<Bool> | ||
| @State private var showDesc: DefaultsValueModel<Bool> | ||
| @State private var branchVerticalLayout: DefaultsValueModel<Bool> | ||
| @State private var stackBranchDir: DefaultsValueModel<Bool> | ||
| @State private var pathLastOnly: DefaultsValueModel<Bool> | ||
| @State private var showNotification: DefaultsValueModel<Bool> | ||
| @State var showNotification: DefaultsValueModel<Bool> | ||
| @State var notificationMessageLineLimit: DefaultsValueModel<Int> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
SwiftLint private_swiftui_state warnings on non-private @State properties.
Lines 13, 19, and 20 dropped private from @State to allow access from the SidebarSection+NotificationPreview extension. This follows the established pattern already present at lines 31–33 (showAgentActivity, loadingSpinnerPosition, notificationBadgePosition). The trade-off is acceptable given the extension-based row extraction, but consider whether these properties could instead be passed as parameters to the row builder to keep @State private.
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 13-13: SwiftUI state properties should be private
(private_swiftui_state)
[Warning] 19-19: SwiftUI state properties should be private
(private_swiftui_state)
[Warning] 20-20: SwiftUI state properties should be private
(private_swiftui_state)
🤖 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/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/SidebarSection.swift`
around lines 13 - 20, The non-private `@State` properties in SidebarSection
conflict with SwiftLint’s private_swiftui_state rule. Keep state private in
SidebarSection and update the SidebarSection+NotificationPreview row builder to
receive the required values as parameters, preserving the existing notification
preview behavior.
Source: Linters/SAST tools
…otification-content
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
hhsw2015
pushed a commit
to hhsw2015/cmux
that referenced
this pull request
Jul 16, 2026
* Show more notification body content * Expand inline notification previews * Keep notification preview change within file budget * Test notification preview line setting * Make notification preview lines configurable * Fix notification preview review gates * Document notification preview settings API * Test notification schema locale coverage * Localize notification schema description * Label notification preview line stepper (cherry picked from commit e8c6ca9)
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
1 participant
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
sidebar.notificationMessageLineLimitincmux.jsonand its schemaTesting
swift testinPackages/macOS/CmuxSettings(251 passed)swift testinPackages/macOS/CmuxSettingsUI(96 passed)python3 scripts/swift_file_length_budget.py --base-ref origin/mainnotif12at9f3cbd2a86Resources/Localizable.xcstringsandweb/data/cmux.schema.json; new Settings copy includes English and JapaneseConfiguration
{ "sidebar": { "notificationMessageLineLimit": 12 } }Note
Low Risk
Sidebar display and preferences only; values are clamped and bounded, with catalog/UI tests and schema coverage. No auth, security, or data-path changes.
Overview
Adds
sidebar.notificationMessageLineLimit(default 12, range 1–50) so workspace notification previews below titles are no longer capped at a fixed two lines.Settings & config: A new Sidebar Notification Preview Lines stepper (disabled when details are hidden or notification text is off) wires through UserDefaults, settings search,
cmux.jsonparsing/template, and the web schema. English and Japanese strings are included.Runtime: Sidebar rows read the clamped limit from settings, apply it to the latest notification subtitle via dynamic
lineLimit, and pre-shape text withsidebarBoundedDisplayString(up to 4,096 characters) before render. iMessage conversation fallbacks still use two lines when there is no notification text.Reviewed by Cursor Bugbot for commit 9b5ec3f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit