Skip to content

refactor(protocol): derive cursor animation numeric conversions - #501

Merged
eval-exec merged 1 commit into
mainfrom
refactor/cursor-animation-conversion
Oct 7, 2026
Merged

eval-exec merged 1 commit into
mainfrom
refactor/cursor-animation-conversion

Conversation

@eval-exec

Copy link
Copy Markdown
Owner

CursorAnimStyle::from_u8 duplicated the enum’s numeric discriminants in a handwritten decoder. Derive num_enum::FromPrimitive and IntoPrimitive so the enum declaration defines both conversions.

Keep the existing from_u8 API and numeric values. Mark Exponential as the numeric fallback, preserving behavior for unknown bytes. Existing serde names and strum variant names remain unchanged. Reuse the existing protocol crate dependency.

Validation: all 851 protocol library tests pass. Conversion tests cover all eight known discriminants in both directions and every unknown byte from 8 through 255. Formatting and diff checks pass; tests remain in types/tests/types_test.rs.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 17:44
@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: 56b22279-d73f-4df4-bfbb-c42d4c435929
📥 Commits

Reviewing files that changed from the base of the PR and between e30732e and 1834287.

📒 Files selected for processing (2)
  • crates/neomacs-display-protocol/src/types.rs
  • crates/neomacs-display-protocol/src/types/tests/types_test.rs
 ____________________________________________________
< Your logging is so chatty it needs a podcast deal. >
 ----------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ 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 5a97f2f into main Oct 7, 2026
27 of 34 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.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

1 open finding
What changed in this PR

Refactors CursorAnimStyle numeric encoding/decoding to rely on derived num_enum conversions instead of a handwritten decoder, while preserving the existing from_u8 API and the “unknown => Exponential” behavior.

Changes:

  • Derive num_enum::FromPrimitive and num_enum::IntoPrimitive for CursorAnimStyle and mark Exponential as the default fallback.
  • Re-implement CursorAnimStyle::from_u8 by delegating to the derived From<u8> conversion.
  • Expand tests to validate round-trip conversions for known discriminants and fallback behavior for all unknown bytes.
File Description
crates/​neomacs-display-protocol/​src/​types.rs Adds derived numeric conversions and default fallback; simplifies from_u8 implementation.
crates/​neomacs-display-protocol/​src/​types/​tests/​types_test.rs Updates tests to cover derived conversions and exhaustive unknown-byte fallback behavior.

🧠 Review effort: Lite


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

fn test_cursor_anim_style_unknown_defaults_to_exponential() {
assert_eq!(CursorAnimStyle::from_u8(8), CursorAnimStyle::Exponential);
assert_eq!(CursorAnimStyle::from_u8(255), CursorAnimStyle::Exponential);
for raw in 8..=u8::MAX {
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