refactor(gui): split buildMenuBar into MainWindow_Menus.cpp (#3351 Phase 1b)#3520
Conversation
…ase 1b) Third PR of the #3351 monolith-decomposition series. Pure code motion — no behavior change, no MainWindow.h changes. Moves buildMenuBar() (1,048 lines: every QMenu/QAction, their enable/disable wiring, and ~70 inline connect lambdas) out of MainWindow.cpp into the new MainWindow_Menus.cpp. One supporting promotion, called out for review: - The keyboard-shortcut mutable state (s_keyboardShortcutsEnabled, s_sliderShortcutLeaseActive) and its accessor helpers (textInputCaptured, shortcutInputCaptured, shortcutGuard, leaseHolderBusy) gain external linkage, declared in a new internal header MainWindowShortcutState.h. The View-menu shortcut toggle — which writes the flag and hands shortcutGuard to ShortcutManager::rebuildShortcuts — moved with buildMenuBar, so the state now has writers in two TUs. Definitions stay in MainWindow.cpp; the header documents why file-scope state exists at all (the ShortcutManager guard is a receiver-less std::function that cannot read MainWindow members). The Shortcuts TU (Phase 1c) will take ownership of the definitions. Breadcrumb comment at the excision site points to the new file. MainWindow.cpp: 17,308 → 16,261 lines (series total: 19,474 → 16,261, −16.5%). Verified: full build clean, all 32 ctest suites pass, a11y lint clean. Refs #3351. Principle XI.
jensenpat
left a comment
jensenpat
left a comment
There was a problem hiding this comment.
Reviewed: verified verbatim code motion (multiset diff of removed vs added lines — only deltas are the new TU's include block and the documented static→extern promotion of shortcut state). MainWindowShortcutState.h is well-scoped and documented; CMake updated; all CI green on Linux/macOS/Windows + CodeQL + a11y.
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed with focus on the stated review surface (the shortcut-state linkage promotion) and verbatim-motion verification.
Verified:
buildMenuBar()is byte-identical — I extracted the old body frommainand the new body fromMainWindow_Menus.cppand diffed them: 1,048 lines, zero differences. The "pure code motion" claim holds.- Linkage is consistent. The header declares everything in
namespace AetherSDR, and the definitions inMainWindow.cppsit inside the namespace block that opens at line 246 — so theexterndeclarations and definitions match. Grepped the whole tree for the six promoted names: no colliding definitions in any other TU. - MainWindow.cpp changes are exactly the minimum: 8 added lines total (the new include,
staticdropped from the two flags and four accessors, and the breadcrumb at the excision site). Nothing else touched. - Thread-safety of the now-cross-TU writers: both write sites (the View-menu toggle in the Menus TU, the lease lifecycle in MainWindow.cpp) and the guard's invocation via
ShortcutManagerall run on the GUI thread, so plainbools remain fine. - CMakeLists addition is correct; all four files are in scope; AppSettings usage is pre-existing moved code.
One optional note, no action needed: the s_ prefix is now a slight misnomer on external-linkage globals, but the header already documents that the Shortcuts TU (Phase 1c) takes ownership of the definitions next — renaming there, when the state actually moves, is the right time.
The header comment explaining why file-scope state exists (the receiver-less std::function<bool()> guard) is exactly the kind of documentation that makes these splits reviewable later. Nice work — this is a clean Phase 1b. The QA bench checklist in the description covers the right surfaces (shortcut toggle round-trip especially).
🤖 aethersdr-agent · cost: $10.7221 · model: claude-fable-5
2 participants
Summary
Third PR of the #3351 series. Pure code motion — no behavior change, no MainWindow.h changes.
Moves
buildMenuBar()— 1,048 lines: every QMenu/QAction, enable/disable wiring, ~70 inline connect lambdas — into the newMainWindow_Menus.cpp.Not-pure-motion exception (the review surface)
One supporting promotion, forced by the split:
The keyboard-shortcut mutable state gains external linkage —
s_keyboardShortcutsEnabled,s_sliderShortcutLeaseActive, and their accessors (textInputCaptured,shortcutInputCaptured,shortcutGuard,leaseHolderBusy) are now declared in a new internal headerMainWindowShortcutState.h. The View-menu shortcut toggle writes the flag and handsshortcutGuardtoShortcutManager::rebuildShortcuts, and it moved withbuildMenuBar— so the state now has writers in two TUs. Definitions stay in MainWindow.cpp.The header documents why file-scope state exists at all (the ShortcutManager guard is a receiver-less
std::function<bool()>that can't read MainWindow members) and that the Shortcuts TU (Phase 1c) takes ownership of the definitions next.Everything else is verbatim. Breadcrumb at the excision site.
Numbers
MainWindow.cpp: 17,308 → 16,261 (−1,047)MainWindow_Menus.cpp: 1,113 lines; newMainWindowShortcutState.h: 44 linesVerification
QA checklist for the bench
This phase is menu construction only — the lowest-risk split in the series:
Refs #3351