Skip to content

refactor(runtime): type menu navigation directions - #500

Merged
eval-exec merged 2 commits into
mainfrom
refactor/menu-navigation-direction
Oct 7, 2026
Merged

eval-exec merged 2 commits into
mainfrom
refactor/menu-navigation-direction

Conversation

@eval-exec

@eval-exec eval-exec commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

Menu hover navigation accepted any i32, although every caller intended a single forward or backward step. Replace the argument with MenuDirection::{Next, Previous} so unsupported steps such as zero or two cannot be passed.

Update keyboard handlers, session tests, and the opt-in native menu test to use named variants. Keep the internal traversal, wrapping, disabled-item skipping, and submenu behavior unchanged. Restrict the navigation method and direction type to the menu module. Derive num_enum::IntoPrimitive with #[repr(i32)] and explicit 1 / -1 discriminants, then convert using direction.into(). Reuse the existing workspace num_enum dependency; the lockfile adds only the runtime crate dependency edge, with no package version changes.

Validation: all 59 focused menu session tests pass. Formatting and diff checks pass. Tests retain the related tests/ location and _test.rs suffix.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 17:37
@coderabbitai

coderabbitai Bot commented Oct 7, 2026

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 610295b7-08ac-4271-a439-7ac2548cf3b1
📥 Commits

Reviewing files that changed from the base of the PR and between d842a7a and 3d71663.

📒 Files selected for processing (4)
  • crates/neomacs-display-runtime/src/menus/interaction.rs
  • crates/neomacs-display-runtime/src/menus/session.rs
  • crates/neomacs-display-runtime/src/menus/session/tests/session_test.rs
  • crates/neomacs-display-runtime/src/menus/tests/native_test.rs
 ___________________________________________________________
< Did you mean to handle errors, or just *experience* them? >
 -----------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@eval-exec
eval-exec merged commit e30732e into main Oct 7, 2026
25 of 37 checks passed

Copilot AI 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.

🟢 Approval recommended

It is a small, mechanical, behavior-preserving type-safety refactor fully contained in the menu module, with all callers updated, visibility correctly narrowed, and existing test coverage retained.

0 open findings

What changed in this PR

This PR replaces the untyped i32 argument of the menu move_hover navigation method with a dedicated MenuDirection::{Next, Previous} enum, making invalid steps (0, 2, etc.) unrepresentable. It is a type-safety refactor internal to the neomacs-display-runtime menu module, preserving all existing traversal, wrapping, separator/disabled-item skipping, and submenu behavior.

Changes:

  • Introduce pub(super) enum MenuDirection (Next = 1, Previous = -1) deriving num_enum::IntoPrimitive with #[repr(i32)], and retype move_hover to accept it (now pub(super)), converting to the internal step via direction.into().
  • Update keyboard handlers and all tests (session + opt-in native) to pass named variants.
  • Add the existing workspace num_enum dependency to the crate (reflected in Cargo.lock).
File Description
crates/​neomacs-display-runtime/​src/​menus/​session.rs Adds MenuDirection enum; retypes/narrows move_hover and converts the enum to the internal step.
crates/​neomacs-display-runtime/​src/​menus/​interaction.rs Updates arrow/Home/End key handlers to pass MenuDirection variants.
crates/​neomacs-display-runtime/​src/​menus/​session/​tests/​session_test.rs Migrates hover tests to named variants; refreshes two now-stale comments.
crates/​neomacs-display-runtime/​src/​menus/​tests/​native_test.rs Imports and uses MenuDirection::Next in the opt-in native menu test.
crates/​neomacs-display-runtime/​Cargo.toml Adds num_enum.workspace = true.
Cargo.lock Records the new num_enum dependency edge for the runtime crate.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

2 participants