Skip to content

refactor(runtime): derive frame demand metadata with strum - #507

Merged
eval-exec merged 3 commits into
mainfrom
refactor/demand-reason-strum
Oct 8, 2026
Merged

eval-exec merged 3 commits into
mainfrom
refactor/demand-reason-strum

Conversation

@eval-exec

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

Copy link
Copy Markdown
Owner

Frame-demand reasons were declared through a custom macro that generated the enum, variant list, count, and diagnostic-name match. Replace it with a normal enum using strum::EnumCount, VariantArray, and VariantNames.

Derive diagnostic names with enum-level snake_case and preserve declaration order. Rename the internal WebKit variant to Webkit so its existing webkit diagnostic spelling needs no override. Small constant adapters retain DemandReason::ALL as a fixed-size array, COUNT, and the const fn name API, including use in diagnostic constant tables. Existing enumset and enum-map derives remain in place. No dependency or lockfile changes are needed.

Validation: all 46 scheduler tests pass with cargo nextest run -p neomacs-display-runtime --lib -E 'test(render_thread::frame_sched::frame_sched_test::)'. This includes the independent golden diagnostic names, dense indices, enum-set iteration order, and all per-window counters. Rustfmt and diff checks pass. Existing unit tests remain in render_thread/tests/frame_sched_test.rs.

Copilot AI balanced review requested due to automatic review settings October 8, 2026 02:14
@coderabbitai

coderabbitai Bot commented Oct 8, 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: 8f89f8a1-91ce-406d-bc59-5dc5ac90621a
📥 Commits

Reviewing files that changed from the base of the PR and between 8af179f and dd4d6f0.

📒 Files selected for processing (1)
  • crates/neomacs-display-runtime/src/render_thread/frame_sched.rs
 ______________________________
< torvalds@linux:~$ git review >
 ------------------------------
  \
   \   \
        \ /\
        ( )
      .( 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 f46f7b9 into main Oct 8, 2026
26 of 32 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 mechanical, behavior-preserving refactor whose diagnostic names, ordering, variant rename, and const APIs are fully covered by existing golden/density tests that I verified remain consistent.

0 open findings

What changed in this PR

This PR refactors the DemandReason frame-demand enum in neomacs-display-runtime from a hand-rolled demand_reasons! macro (which generated the enum, ALL, COUNT, and the name match table) to a plain enum that derives its metadata via strum::EnumCount, VariantArray, and VariantNames. The goal is to reduce the custom-macro surface while preserving the existing ALL/COUNT/name APIs and the stable snake_case diagnostic vocabulary used by frame metrics.

Changes:

  • Replace the demand_reasons! macro with a normal enum using strum derives plus #[strum(serialize_all = "snake_case")], deriving diagnostic names from variant identifiers.
  • Rename the WebKit variant to Webkit so its webkit diagnostic name needs no per-variant override; update the two call sites.
  • Reimplement COUNT, ALL, and const fn name as thin adapters over the derived strum traits, keeping the fixed-size-array and const-expression API intact.
File Description
crates/​neomacs-display-runtime/​src/​render_thread/​frame_sched.rs Replaces the macro with a strum-derived enum and const adapters for COUNT/ALL/name.
crates/​neomacs-display-runtime/​src/​render_thread/​lifecycle.rs Updates the WebKit → Webkit variant reference at the webkit-active call site.
crates/​neomacs-display-runtime/​src/​render_thread/​tests/​frame_sched_test.rs Updates the test assertion to the renamed Webkit variant.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

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