Proposed Bonsplit hooks for cmux issue 1362#26
Proposed Bonsplit hooks for cmux issue 1362#26ebrardushullovcii wants to merge 2 commits intomanaflow-ai:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR promotes TabBarColors and TabBarMetrics to public, adds a public configuration flag Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Companion cmux proposal is here: manaflow-ai/cmux#1437\n\nThat PR now rebases cleanly on current |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/Bonsplit/Internal/Styling/TabBarMetrics.swift (1)
4-52: LGTM!Making these constants public enables external consumers (like the cmux prototype) to reuse Bonsplit's tab-bar metrics consistently.
Minor nit: The file resides under
Internal/Styling/but is now a public API. Consider moving it toPublic/Styling/for consistency, or alternatively renaming the folder if more types become public.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Bonsplit/Internal/Styling/TabBarMetrics.swift` around lines 4 - 52, TabBarMetrics is declared public but lives under Internal/Styling which mismatches its visibility; move the TabBarMetrics type (and any other public styling types) out of the Internal/Styling folder into a Public/Styling area (or rename the folder from Internal to Public) so the filesystem layout reflects the API surface, update any module export or target include if needed, and ensure the symbol TabBarMetrics and its constants keep their public access after the move.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/Bonsplit/Internal/Views/TabItemView.swift`:
- Around line 369-387: The four contextButton calls that create localized labels
for the split actions (contextButton(..., action: .splitRight),
contextButton(..., action: .splitDown), contextButton(..., action:
.splitBrowserRight), contextButton(..., action: .splitBrowserDown) in
TabItemView.swift) are using bundle: .main; change each localized initializer to
use bundle: .module instead so the strings are resolved from the package module
bundle rather than the main app bundle.
- Around line 326-335: The two contextButton calls use bundle: .main which will
miss package-localized strings; change them to use the package bundle (use
bundle: .module) or replace the calls with the existing localizedContextButton
helper used later; update the contextButton invocations around
contextMenuState.hasCustomTitle (and the preceding Rename Tab call) to use
Bundle.module or call localizedContextButton so localization resolves from the
package instead of .main.
---
Nitpick comments:
In `@Sources/Bonsplit/Internal/Styling/TabBarMetrics.swift`:
- Around line 4-52: TabBarMetrics is declared public but lives under
Internal/Styling which mismatches its visibility; move the TabBarMetrics type
(and any other public styling types) out of the Internal/Styling folder into a
Public/Styling area (or rename the folder from Internal to Public) so the
filesystem layout reflects the API surface, update any module export or target
include if needed, and ensure the symbol TabBarMetrics and its constants keep
their public access after the move.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a5a85e71-feca-4c7a-b0b5-2927ae4ee861
📒 Files selected for processing (6)
Sources/Bonsplit/Internal/Styling/TabBarColors.swiftSources/Bonsplit/Internal/Styling/TabBarMetrics.swiftSources/Bonsplit/Internal/Views/TabBarView.swiftSources/Bonsplit/Internal/Views/TabItemView.swiftSources/Bonsplit/Public/BonsplitConfiguration.swiftSources/Bonsplit/Public/Types/TabContextAction.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/Bonsplit/Internal/Views/TabItemView.swift">
<violation number="1" location="Sources/Bonsplit/Internal/Views/TabItemView.swift:327">
P2: These new localized menu labels are resolved from `.main` instead of `Bundle.module`, so Bonsplit package localizations are skipped and labels can stay untranslated unless the host app duplicates the keys.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Pushed to address the actionable AI review item on the Bonsplit side.\n\nThe new localized tab context-menu labels now resolve from instead of , so the package-localized strings come from Bonsplit itself rather than depending on the host app to duplicate those keys. |
|
Pushed The new localized tab context-menu labels now resolve from |
1 participant
Summary
This is a proposed supporting change for
manaflow-ai/cmux#1362.It adds the small Bonsplit hooks needed for cmux to prototype Ghostty-style workspace top tabs while keeping the visual style aligned with the existing Bonsplit tab bar.
Current scope:
Proposal note
This is just a try to help with the cmux proposal.
If this direction is not useful, feel free to ignore or close it.
Summary by cubic
Expose tab bar styling and metrics as public hooks and add small config and context menu actions to support
cmux’s top tabs prototype (issue #1362). Defaults preserve current Bonsplit behavior and look.TabBarColorsandTabBarMetricspublic, with Color/NSColorhelpers for reuse in top‑level tab strips.appearance.showPaneNewTabButtons(default true) to optionally hide pane‑level new‑tab buttons.splitRight,splitDown,splitBrowserRight,splitBrowserDown.Written for commit bda0482. Summary will update on new commits.
Summary by CodeRabbit
New Features
Localization