Skip to content

Group cmux TUI context menu actions - #8225

Merged
lawrencecchen merged 4 commits into
mainfrom
feat-rust-tui-context-menu-grouping
Jul 16, 2026
Merged

lawrencecchen merged 4 commits into
mainfrom
feat-rust-tui-context-menu-grouping

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • group pane actions into current tab, creation, browser, pane layout, and ID sections
  • render non-clickable separators and skip them during keyboard and mouse navigation
  • update Rust TUI docs and smoke coverage for the new row layout

Testing

  • ZIG=/opt/homebrew/Cellar/zig/0.15.2_1/bin/zig cargo test -p cmux-tui (223 passed)
  • ZIG=/opt/homebrew/Cellar/zig/0.15.2_1/bin/zig cargo clippy -p cmux-tui --all-targets -- -D warnings
  • isolated smoke-tui runtime replay through context-menu actions (SMOKE OK)

Scope

  • Rust cmux-tui only

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Grouped context-menu actions in cmux-tui into sections with visible, non-clickable separators. When space is tight, the menu drops only extra separators so all actions remain visible.

  • New Features

    • Pane menu groups: current tab, creation, browser (when available, adds “Activate” for external), layout, and IDs.
    • Workspace menu groups rename/close and separates copy ID; screen menu groups rename/close.
    • Separators draw as ├─┤; Up/Down and hit-testing skip them; Enter and clicks only activate actions.
  • Refactors

    • Added MenuItem; ContextMenu now holds Vec<MenuItem> and builds from grouped actions.
    • Introduced pane_context_menu_groups(...), selection helpers that skip separators, and fit_to_rows(...) to manage separator collapse and bounds-safe selection.

Written for commit 40abcbf. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Context menus are now grouped into sections with visible separator rows.
    • Keyboard and mouse interactions treat separator rows as non-activating, ensuring Enter/right-click actions only trigger real items.
  • Bug Fixes
    • Improved rendering for separator rows and refined label drawing to match grouped layout.
  • Documentation
    • Expanded context-menu documentation with the grouped pane menu contents and described ID-copy actions.
  • Tests
    • Strengthened smoke/UI snapshot assertions and added unit coverage for grouped composition and navigation behavior.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Context menus now use grouped action lists with separator rows. Construction, rendering, mouse activation, keyboard navigation, documentation, unit tests, and smoke tests were updated for the new menu layout.

Changes

Grouped context menu flow

Layer / File(s) Summary
Menu model and separator-aware navigation
cmux-tui/crates/cmux-tui/src/app.rs
Introduces MenuItem, stores menu entries as actions or separators, and updates hit-testing, keyboard navigation, and activation to skip separators.
Grouped menu construction and rendering
cmux-tui/crates/cmux-tui/src/app.rs, cmux-tui/crates/cmux-tui/src/ui/overlay.rs
Builds pane, workspace, and screen menus from action groups and renders separator rows with border glyphs.
Menu behavior validation and documentation
cmux-tui/crates/cmux-tui/src/app.rs, cmux-tui/docs/mouse.md, cmux-tui/scripts/smoke-tui.py
Adds grouping and separator tests, documents the updated pane menu, and verifies rendered rows and corrected click coordinates in smoke tests.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant App
  participant ContextMenu
  participant Overlay
  User->>App: Open pane context menu
  App->>App: Build grouped pane actions
  App->>ContextMenu: Create grouped menu rows
  ContextMenu-->>Overlay: Provide MenuItem rows
  Overlay-->>User: Render actions and separators
  User->>App: Select or activate a row
  App->>ContextMenu: Resolve actionable item
  ContextMenu-->>App: MenuAction or no action
Loading
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed PR only touches Rust TUI code/docs/tests; no Swift files or actor-isolation-sensitive declarations were changed.
Cmux Swift Blocking Runtime ✅ Passed PR changes only touch Rust (cmux-tui/src/app.rs); no Swift files or blocking-runtime primitives were introduced.
Cmux Browser Automation Off-Main ✅ Passed HEAD^..HEAD changes only cmux-tui/src/app.rs; no browser automation router, mainActor, or WebKit/AppKit code was touched.
Cmux Expensive Synchronous Load ✅ Passed PASS: The PR diff only changes Rust cmux-tui code (app.rs); no Swift files or synchronous agent-history load paths were touched, so the Swift-specific rule is not applicable.
Cmux Cache Substitution Correctness ✅ Passed PASS: PR only changes Rust TUI/menu code, docs, and a Python smoke test; no Swift/TS/JS cache substitution or persistence/history/snapshot path is touched.
Cmux No Hacky Sleeps ✅ Passed The PR adds no new fixed sleeps or timers; smoke-tui.py only changes assertions and click coordinates, and its existing timeout loop/sleep is unchanged.
Cmux Algorithmic Complexity ✅ Passed Menu logic only scans a fixed handful of context-menu rows/groups; no scalable user-data collection is rescanned or re-sorted.
Cmux Swift Concurrency ✅ Passed The PR only changes Rust cmux-tui files; no Swift-owned code or Swift concurrency patterns were introduced.
Cmux Swift @Concurrent ✅ Passed Diff vs origin/main touches only Rust, docs, and a smoke test; no .swift files changed, so the Swift concurrency rule is not applicable.
Cmux Swift Package Boundaries ✅ Passed No Swift files or package-manifest changes are in the diff; the PR only touches Rust cmux-tui code, so the Swift boundary rule is not applicable.
Cmux Swiftpm Lockfiles ✅ Passed Diff only changes Rust TUI docs/tests; no SwiftPM, Xcode, .gitignore, workflow, or Package.resolved files were touched.
Cmux Swift Logging ✅ Passed The diff only changes Rust cmux-tui/src/app.rs; no Swift files or Swift runtime logging changes are present.
Cmux User-Facing Error Privacy ✅ Passed Changed files add menu grouping/rendering and test/docs updates only; no user-facing error, alert, or recovery copy exposes forbidden internal details.
Cmux Full Internationalization ✅ Passed PASS: this commit only hardens menu selection bounds and tests; it adds no new Swift/web user-facing text or locale assets.
Cmux Swiftui State Layout ✅ Passed HEAD only changes cmux-tui/crates/cmux-tui/src/app.rs; no Swift/SwiftUI files were touched, so the SwiftUI state-layout rule is not applicable.
Cmux Architecture Rethink ✅ Passed Diff only changes Rust cmux-tui app.rs; no Swift files or Swift lifecycle/architecture changes are touched, so the Swift rethink rule doesn’t apply.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR changes only Rust TUI code; no Swift window code was added or modified, so the auxiliary-window shortcut rule doesn’t apply.
Cmux Source Artifacts ✅ Passed Only changed path is hand-written Rust source/tests; no logs, caches, temp dirs, or other source-control artifacts were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The commit only changes cmux-tui/crates/cmux-tui/src/app.rs; no Swift production Sources/ files were touched, so the rule doesn’t apply.
Cmux No Ambient Global State ✅ Passed PASS: The PR only touches Rust/doc/test files, not Swift, and it adds file-scoped Rust helpers/enums rather than ambient global state or singletons.
Title check ✅ Passed The title clearly summarizes the main change: grouping cmux TUI context menu actions.
Description check ✅ Passed The description covers the summary, testing, and scope, and is mostly complete despite a few template sections being omitted.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-rust-tui-context-menu-grouping

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR groups the cmux TUI right-click context menu into labelled sections (current-tab, creation, browser, pane-layout, and IDs) separated by non-interactive ├─┤ divider rows. Keyboard and mouse navigation skip separators, fit_to_rows drops only excess separators when the terminal is too short, and the selection index is preserved across resizes.

  • MenuItem enum + ContextMenu refactor (app.rs): ContextMenu::at now accepts Vec<Vec<MenuAction>> groups and inserts MenuItem::Separator between non-empty groups; select_previous/select_next/item_at/selected_action all guard against activating separators; fit_to_rows rebuilds from all_items each draw frame, dropping bottom separators first when the screen is constrained.
  • Rendering (overlay.rs): draw_menu calls fit_to_rows once per frame before clamping the rect, then renders separators as ├─┤ in the border style while continuing to highlight action rows by their position in menu.items.
  • Tests and smoke script: four new unit tests cover grouped composition, navigation edge cases (including usize::MAX sentinel), browser-group contents, and separator-dropping/restoring; smoke test row-layout assertions and mouse-click coordinates updated to match the new layout.

Confidence Score: 5/5

Safe to merge — purely additive UI grouping in the Rust TUI crate with no shared state, no new unsafe code, and no cross-language interfaces touched.

Every separator-skipping path (keyboard, mouse press, mouse drag-release, Enter key) correctly guards against activating a separator. fit_to_rows is idempotent under repeated frame calls and correctly preserves the selected action across resize. The smoke test coordinates were updated consistently with the new layout and the inline unit tests cover the key navigation edge cases including out-of-bounds sentinel values.

No files require special attention.

Important Files Changed

Filename Overview
cmux-tui/crates/cmux-tui/src/app.rs Introduces MenuItem enum, rewrites ContextMenu to use Vec with separator support; fit_to_rows correctly drops excess separators; select_previous/select_next skip separators; item_at/selected_action guard separators on every activation path; pane_context_menu_groups cleanly centralizes grouped layout; thorough unit tests added inline.
cmux-tui/crates/cmux-tui/src/ui/overlay.rs draw_menu now calls fit_to_rows once per frame before clamping, renders separator rows as ├─┤ using the border style, and correctly uses the item index for selection highlight; no regressions in the label-drawing path.
cmux-tui/scripts/smoke-tui.py Adds separator assertions and row-by-row layout assertions for the pane menu; updates OSC-52 copy click from row 7 to row 16 to match 'Copy tab id' new position; updates 'Close tab' drag-click from row 13 to row 7; coordinates are consistent with the new grouped layout.
cmux-tui/docs/mouse.md Documentation updated to describe the new grouped pane menu, separator collapse behavior, and workspace/screen copy-ID action; accurate and complete.

Reviews (2): Last reviewed commit: "Keep grouped TUI menus visible" | Re-trigger Greptile

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@cmux-tui/crates/cmux-tui/src/app.rs`:
- Around line 1702-1718: Update select_previous and select_next to obtain their
sliced ranges through self.items.get(...) before iterating, so empty menus or
out-of-range selected indices produce no match instead of panicking. Preserve
the existing selection movement behavior for valid ranges.
🪄 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: fb68c140-48a0-43ef-bfaf-8fd27422811e

📥 Commits

Reviewing files that changed from the base of the PR and between b6cb50d and b485ae0.

📒 Files selected for processing (4)
  • cmux-tui/crates/cmux-tui/src/app.rs
  • cmux-tui/crates/cmux-tui/src/ui/overlay.rs
  • cmux-tui/docs/mouse.md
  • cmux-tui/scripts/smoke-tui.py

Comment thread cmux-tui/crates/cmux-tui/src/app.rs
@lawrencecchen
lawrencecchen merged commit 5b91793 into main Jul 16, 2026
6 checks passed
@lawrencecchen
lawrencecchen deleted the feat-rust-tui-context-menu-grouping branch July 16, 2026 03:58
@lawrencecchen
lawrencecchen restored the feat-rust-tui-context-menu-grouping branch July 18, 2026 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant