Refactor Phase 3 (optional): table-drive the menu + shortcut builders

[ten9876]

Follow-up to #3351 (closed). This is the issue's Phase 3, which it explicitly marked "optional, later" — filing it so the idea isn't lost, not because it's load-bearing.

Idea

buildMenuBar() (now in MainWindow_Menus.cpp, ~1,000 lines) and registerShortcutActions() (in MainWindow_Shortcuts.cpp, ~560 lines) are long imperative sequences of near-identical addAction(...) + connect(...) / registerAction(...) calls. Much of that repetition could collapse into a data table (id, label, shortcut, handler, enable-predicate) walked by a small builder loop.

Why it's optional / low priority

• Pure ergonomics — no behavior change, no architectural unlock, no bug.
• The decomposition already solved the actual pain (Refactor: break up the 15k-line MainWindow.cpp monolith (phased, behavior-preserving) #3351's goal): these are now isolated in their own reviewable TUs, off the MainWindow.cpp hot path.
• Table-driving menus is a readability nicety with real risk of subtle behavior drift (action ordering, conditional enables, lambda captures) for modest payoff. Only worth doing if someone's already deep in the menu/shortcut code for another reason.

If pursued

• Start with registerShortcutActions() — it's the more regular of the two (the shortcut registry already has a uniform registerAction(id, name, category, …) shape).
• Menus are messier (submenus, separators, dynamic/conditional items, the contributors-fetch lambda) and may not table-drive cleanly — partial conversion is fine.
• Behavior-preserving; full menu-walk + shortcut QA required.

No urgency. Closeable as wontfix if the team decides the repetition isn't worth touching.

Refs #3351

[aethersdr-agent]

Triage: valid optional refactor — feasible on shortcuts, not worth it on menus

Read both target files. The issue's self-assessment holds up; adding the concrete detail a maintainer needs to make the call.

registerShortcutActions() — MainWindow_Shortcuts.cpp:530

Table-driving is feasible and genuinely low-risk here, because the call site is already uniform:

m_shortcutManager.registerAction(id, name, category, QKeySequence, handler, autoRepeat=false);

~90 calls, all that exact shape, and two sub-groups (bands[] at :619, modes[] at :653) are already loop-driven. A struct ShortcutDef { const char* id; const char* name; const char* category; QKeySequence key; std::function<void()> handler; bool autoRepeat; } array walked by one loop would drop the repeated m_shortcutManager.registerAction( prefix from every entry. Mechanics that all work cleanly:

• Null handlers (ptt_hold :674, the three momentary CW actions :1012–:1017) — registerAction already accepts nullptr, so an empty std::function row is fine.
• Local-helper captures (nudgeFreq, cycleStep, zoomActivePanadapter, stepActivePanRfGain) — captured into the table initializer the same way they're captured today.
• Ordering — a std::array preserves registration order (which drives the ShortcutDialog display order), so no drift there.
• Trailing setup (loadBindings :1068, rebuildShortcuts, the lease-timer + focusChanged connects :1072–:1086) stays outside the table as-is.

The honest caveat on payoff: the handler lambda bodies are what dominate the line count, not the boilerplate prefix. Table-driving collapses the 6 metadata columns but every bespoke body still has to live somewhere (as a row's std::function). So you trade registerAction("af_gain_up", "AF Gain Up", "Audio", QKeySequence(Qt::Key_Up), [this]{...}) for {"af_gain_up", "AF Gain Up", "Audio", Qt::Key_Up, [this]{...}} — real but modest. The win is mostly visual alignment + one obvious place to scan all bindings.

buildMenuBar() — MainWindow_Menus.cpp:64

Recommend leaving this alone. Even the first 120 lines show why it resists a clean table:

• Per-action setMenuRole() that is load-bearing on macOS and varies per item (QuitRole :77, PreferencesRole :89, NoRole :103/:109/:119 — tied to Keyboard Shortcuts menu item does nothing on macOS (precompiled build) ,Preferences open the Shortcuts Window #883/Settings menu contains two separate entries for the same functionality: "Radio Setup" and "Preferences" #1013). That's a mandatory column and the exact kind of conditional the issue warns drifts silently.
• #ifdef HAVE_MQTT conditional items (:117), plus separators, QActionGroup, checkable items, submenus, and the contributors-fetch network lambda noted in the issue.

Any table here would be so studded with per-row exceptions it wouldn't be more readable than the imperative form. Partial conversion would just split the menu definition across two paradigms — worse, not better.

Handoff recommendation

This is a legitimate enhancement but, per the issue's own framing, low priority and optional. My read:

1. If pursued: do only the shortcut half (clean, low-risk, behavior-preserving), and leave menus imperative. Still requires a full keyboard-shortcut QA pass (every binding fires, autoRepeat preserved, null-handler entries still register, ShortcutDialog ordering unchanged).
2. Otherwise: reasonable to close as wontfix — the Refactor: break up the 15k-line MainWindow.cpp monolith (phased, behavior-preserving) #3351 decomposition already delivered the actual value (these are isolated, reviewable TUs off the MainWindow.cpp hot path), and the remaining repetition is cosmetic.

Flagged maintainer-review for the go/no-go call. No code touched in this pass.

---

_{🤖 aethersdr-agent · cost: $1.3267 · model: claude-opus-4-8}

---

_{📍 Citation health check (Foundry Constitution Principle I): 0 resolved, 2 unresolved against /Users/aetherclaude/workspace/AetherSDR at origin/main.}

Unresolved citations (2)

Citation	Reason
MainWindow_Menus.cpp:64	file not found in workspace
MainWindow_Shortcuts.cpp:530	file not found in workspace