Repository navigation
Add empty workspace group entrypoints - #7061
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:
📝 WalkthroughWalkthroughAdds a new ChangesWorkspace Group Creation and Sidebar UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
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 |
Greptile SummaryThis PR introduces a "New Empty Workspace Group" entrypoint wired to a new
Confidence Score: 4/5Safe to merge with one UX fix: the File menu 'New Workspace Group' item should be disabled on remote tmux mirror tabs the same way the workspace-row context menu entry is. The Sources/cmuxApp.swift — the new File menu entry needs a Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User triggers newWorkspaceGroup] --> B{Entrypoint}
B --> C[⌃⌘G Shortcut\nAppDelegate shortcut handler]
B --> D[File menu\ncmuxApp.swift]
B --> E[Workspace-row\ncontext menu]
B --> F[Blank sidebar\ncontext menu]
C --> G[createEmptyWorkspaceGroup\npreferredWindow: event.window]
D --> G2[createEmptyWorkspaceGroup\ntabManager: activeTabManager]
E --> H{canCreateEmptyWorkspaceGroup\n= !isRemoteTmuxMirror}
H -- false --> I[Button disabled ✓]
H -- true --> G3[createEmptyWorkspaceGroup\ntabManager: tabManager]
F --> G4[createEmptyWorkspaceGroup\ntabManager: tabManager\nNo disabled guard ⚠️]
G --> K{resolvedTabs.selectedTab.isRemoteTmuxMirror?}
G2 --> K
G3 --> K
G4 --> K
K -- true --> L[return false\nsilent no-op]
K -- false --> M[tabs.createWorkspaceGroup\nname: empty string]
M --> N[Auto-named Group N\nanchor workspace inserted\nselection updated]
D -. no disabled guard ⚠️ .-> G2
%%{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 triggers newWorkspaceGroup] --> B{Entrypoint}
B --> C[⌃⌘G Shortcut\nAppDelegate shortcut handler]
B --> D[File menu\ncmuxApp.swift]
B --> E[Workspace-row\ncontext menu]
B --> F[Blank sidebar\ncontext menu]
C --> G[createEmptyWorkspaceGroup\npreferredWindow: event.window]
D --> G2[createEmptyWorkspaceGroup\ntabManager: activeTabManager]
E --> H{canCreateEmptyWorkspaceGroup\n= !isRemoteTmuxMirror}
H -- false --> I[Button disabled ✓]
H -- true --> G3[createEmptyWorkspaceGroup\ntabManager: tabManager]
F --> G4[createEmptyWorkspaceGroup\ntabManager: tabManager\nNo disabled guard ⚠️]
G --> K{resolvedTabs.selectedTab.isRemoteTmuxMirror?}
G2 --> K
G3 --> K
G4 --> K
K -- true --> L[return false\nsilent no-op]
K -- false --> M[tabs.createWorkspaceGroup\nname: empty string]
M --> N[Auto-named Group N\nanchor workspace inserted\nselection updated]
D -. no disabled guard ⚠️ .-> G2
Reviews (11): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![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/ContentView.swift`:
- Around line 13133-13149: The new empty-area context menu action in
sidebarEmptyAreaWorkspaceGroupContextMenu bypasses the remote-tmux guard and can
create a local orphan workspace via createEmptyWorkspaceGroup(tabManager:), so
update this path to respect the same remote-aware invariant used by the existing
empty-area new-workspace flow. Reuse the remote-aware helper or conditionally
disable/hide the menu item when the window is in remote-mirror/remote-tmux mode,
and avoid calling AppDelegate.shared?.createEmptyWorkspaceGroup(tabManager:)
unconditionally from the Button action.
🪄 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: 2cad1adc-0257-4d35-a396-1e22edc803fc
📒 Files selected for processing (15)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/TabItemView+WorkspaceGroups.swiftSources/cmuxApp.swiftcmuxTests/WorkspaceGroupTests.swiftdocs/workspace-groups.mdskills/cmux-settings/references/shortcut-actions.mdweb/data/cmux-shortcuts.tsweb/data/cmux.schema.jsonweb/messages/en.jsonweb/messages/ja.json
db41553 to
ba7c610
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "contextMenu.close": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Close" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "閉じる" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
All 12 new strings added to Localizable.xcstrings only carry en and ja translations, but the catalog already supports ar, bs, da, de, es, fr, it, ko, nb, pl, pt-BR, and others (confirmed by the adjacent contextMenu.closeOtherWorkspaces and contextMenu.workspaceGroup.newFromSelection entries, which both have a full locale set). Users on any of those locales will see raw English text in the new New Empty Workspace Group menu item, the New Workspace Group File-menu entry, the Pinned group tooltip, and the reorganized Close / Move / Copy / Notifications / Remote Connection submenus.
The affected keys are: contextMenu.close, contextMenu.copy, contextMenu.move, contextMenu.notifications, contextMenu.remoteConnection, contextMenu.workspaceGroup.newEmpty, contextMenu.workspaceGroup.title, menu.file.newWorkspaceGroup, shortcut.newWorkspaceGroup.label, workspaceGroup.contextMenu.configuration, workspaceGroup.contextMenu.notifications, and workspaceGroup.pinned.tooltip.
Rule Used: Flag production user-facing text that is not fully... (source)
…group # Conflicts: # .github/swift-file-length-budget.tsv
709e6fd to
23c8d92
Compare
23c8d92 to
66d8414
Compare
| if let key = newWorkspaceGroupShortcut.keyEquivalent { | ||
| Button(newWorkspaceGroupLabel) { | ||
| _ = AppDelegate.shared?.createEmptyWorkspaceGroup(tabManager: tabManager) | ||
| } | ||
| .keyboardShortcut(key, modifiers: newWorkspaceGroupShortcut.eventModifiers) | ||
| } else { | ||
| Button(newWorkspaceGroupLabel) { | ||
| _ = AppDelegate.shared?.createEmptyWorkspaceGroup(tabManager: tabManager) | ||
| } | ||
| } |
There was a problem hiding this comment.
isRemoteTmuxMirror guard that the workspace-row context menu section (workspaceGroupContextMenuSection) applies via canCreateEmptyWorkspaceGroup. On a remote tmux mirror tab the button appears enabled; clicking it silently returns false with no user feedback, unlike the workspace row entry which correctly shows the item disabled.
| if let key = newWorkspaceGroupShortcut.keyEquivalent { | |
| Button(newWorkspaceGroupLabel) { | |
| _ = AppDelegate.shared?.createEmptyWorkspaceGroup(tabManager: tabManager) | |
| } | |
| .keyboardShortcut(key, modifiers: newWorkspaceGroupShortcut.eventModifiers) | |
| } else { | |
| Button(newWorkspaceGroupLabel) { | |
| _ = AppDelegate.shared?.createEmptyWorkspaceGroup(tabManager: tabManager) | |
| } | |
| } | |
| let canCreate = tabManager.selectedTab?.isRemoteTmuxMirror != true | |
| if let key = newWorkspaceGroupShortcut.keyEquivalent { | |
| Button(newWorkspaceGroupLabel) { | |
| _ = AppDelegate.shared?.createEmptyWorkspaceGroup(tabManager: tabManager) | |
| } | |
| .keyboardShortcut(key, modifiers: newWorkspaceGroupShortcut.eventModifiers) | |
| .disabled(!canCreate) | |
| } else { | |
| Button(newWorkspaceGroupLabel) { | |
| _ = AppDelegate.shared?.createEmptyWorkspaceGroup(tabManager: tabManager) | |
| } | |
| .disabled(!canCreate) | |
| } |
66d8414 to
7b31812
Compare
…group # Conflicts: # .github/swift-file-length-budget.tsv
| } | ||
| } | ||
|
|
||
| splitCommandButton(title: String(localized: "menu.file.newWorkspaceGroup", defaultValue: "New Workspace Group"), shortcut: menuShortcut(for: .newWorkspaceGroup)) { | ||
| _ = AppDelegate.shared?.createEmptyWorkspaceGroup( | ||
| tabManager: activeTabManager, | ||
| preferredWindow: NSApp.keyWindow ?? NSApp.mainWindow | ||
| ) | ||
| } | ||
|
|
||
| splitCommandButton(title: String(localized: "menu.file.openFolder", defaultValue: "Open Folder…"), shortcut: menuShortcut(for: .openFolder)) { |
There was a problem hiding this comment.
The new splitCommandButton for menu.file.newWorkspaceGroup carries no .disabled(...) modifier. When the focused tab is a remote tmux mirror, createEmptyWorkspaceGroup silently returns false with no user feedback, but the menu item remains visually active and clickable — unlike the workspace row context menu entry, which correctly uses .disabled(!canCreateEmptyWorkspaceGroup) (evaluated as tabManager.selectedTab?.isRemoteTmuxMirror != true).
The intent described in the PR is that this action is disabled on remote tmux mirror tabs; applying that guard at the File menu level makes the affordance consistent across all four entrypoints.
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!
…empty-workspace-group entrypoints (menu/shortcut/handler) reverted by refactor's AppDelegate/cmuxApp winning the merge
This branch was successfully deployed
1 participant
Summary
newWorkspaceGroupshortcut to app/settings catalogs, schema/docs, and shortcut docs with default⌃⌘GVerification
git diff --checkjq empty Resources/Localizable.xcstrings web/messages/en.json web/messages/ja.json web/data/cmux.schema.jsonswift test --package-path Packages/macOS/CmuxSettingsxcodebuild test -project cmux.xcodeproj -scheme cmux-unit -configuration Debug -destination platform=macOS -derivedDataPath /tmp/cmux-emptygrp -only-testing:cmuxTests/WorkspaceGroupTests(newcreateEmptyGroupInsertsAnchorOnlyGroup()passed; existing workspace group ordering tests still fail when the whole suite runs together)emptyg, preflightedworkspace.group.createwithchild_workspace_ids: []; it createdGroup 1with a single anchor workspace and rendered in the sidebarNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a “New Empty Workspace Group” entrypoint with a default ⌃⌘G shortcut across the app. It creates an anchor‑only, auto‑named group and is available from the keyboard, File menu, workspace row context menu, and the empty sidebar area.
newWorkspaceGroupshortcut and handler; routes to the active window/tab manager and is disabled on remote tmux mirror tabs.menu.file.newWorkspaceGroup,contextMenu.workspaceGroup.newEmpty,shortcut.newWorkspaceGroup.label,workspaceGroup.pinned.tooltip; updatedweb/data/cmux.schema.json,web/data/cmux-shortcuts.ts; added a unit test for anchor‑only group creation.Written for commit 76fab67. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation