Skip to content

fix(gui): preserve the FlexControl menu ampersand. Principle VIII. - #6569

Merged
NF0T merged 2 commits into
aethersdr:mainfrom
w5jwp:fix/flexcontrol-menu-ampersand
Oct 19, 2026
Merged

NF0T merged 2 commits into
aethersdr:mainfrom
w5jwp:fix/flexcontrol-menu-ampersand

Conversation

@w5jwp

@w5jwp w5jwp commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #6568.

Settings → Controllers currently displays “FlexControl Knob Buttons...” because Qt consumes the unescaped ampersand as a mnemonic marker. Escape it as && so the menu displays “FlexControl Knob & Buttons...”, matching the documented name, and clarify the shortcut in the menu reference.

The menu audit found no other hardcoded label with this ampersand defect. The separate EQ “Slope” double space is outside this fix.

Constitution principle honored

Principle VIII — Evidence Over Assertion: identify the exact QAction string and record the checks actually performed, including platform and build limits.

Test plan

  • Local build passes: cmake --build /tmp/aethersdr-menu-build --target AetherSDR --parallel 4 (pinned Qt 6.12.0)
  • Behavior verified on a real radio if applicable — not applicable to this host menu label
  • Existing tests pass (CI) — pending PR creation
  • Reproduction steps documented if user-reported bug

Offscreen smoke check passed with QT_QPA_PLATFORM=offscreen, isolated settings, and transmit automation disabled. The live menu list reports “FlexControl Knob & Buttons...”; triggering that action opens a visible AetherSDR::RadioSetupDialog. Artifacts: menus.json, settings-tree.json, and offscreen.log in /tmp/aethersdr-menu-prep. The first sandboxed launch was blocked by audio permissions; the successful check ran outside the command sandbox. Windows/macOS visual checks have not been run. The docs build is pending: Node/npm are unavailable in this environment.

Checklist

  • Commits are signed — GitHub-signed commit bb40bcc; API reports VALID, isValid=true
  • No new flat-key AppSettings calls — no settings changes
  • Code is clean-room
  • All meter UI uses MeterSmoother — no meter changes
  • Documentation updated if user-visible behavior changed — menu-reference.md
  • Security-sensitive changes reference a GHSA if applicable — not applicable

No CHANGELOG.md change. Self-assignment is skipped per explicit user instruction. Duplicate searches were repeated before filing. The fix contains only MainWindow_Menus.cpp and menu-reference.md; setup-script artifacts are excluded.

Escape the literal ampersand so Qt displays FlexControl Knob & Buttons... instead of dropping the character. Clarify the menu reference.

Prepared by Justin Pauler (W5JWP) <justin@w5jwp.radio> with Codex. Local Qt 6.12 compilation and offscreen menu/action smoke check passed.
@w5jwp
w5jwp requested a review from a team as a code owner October 18, 2026 16:18

@NF0T NF0T left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @w5jwp, the code fix is exactly right and I'd merge the MainWindow_Menus.cpp hunk as-is. One small docs change first.

What's correct

  • The diagnosis holds. QAction/QMenu treat a single & as a mnemonic marker, so "Knob & Buttons..." renders as Knob␣␣Buttons... with the following space as the "shortcut". && is Qt's documented escape for a literal ampersand. macOS's native menu bar strips mnemonics the same way and also turns && into &, so all three platforms are fixed by one literal.
  • Nothing breaks by label. I grepped src/, tests/ and tools/ for the old string. Its only code consumer is the automation bridge, and actionDisplayText() (src/core/AutomationServer.cpp:131) already folds && into & when it matches menu paths. A menu verb targeting FlexControl Knob & Buttons... therefore resolves after this change and did not resolve before it, because the old display text had two spaces. That's consistent with your offscreen smoke check.
  • The audit claim holds. I ran my own regex sweep for unescaped & in addAction/addMenu/QAction(/setText/setTitle literals across src/. This line is the only hit; the neighbouring "Sync Kiwi Audio && Display" is already escaped.
  • Signed (GitHub valid), CI is green on the pull_request merge result, and there's no CHANGELOG entry. Good.

Scope

File Claimed by body Verdict
src/gui/MainWindow_Menus.cpp (1 line) yes in scope, correct
docs/user/docs/menu-reference.md (+2) yes in scope, but see below

Preference-vs-fix: a fix. The source now matches the label the user guide already documents in five places (menu-reference.md:31, flexcontrol-tuning-knob.md, radio-setup.md, usb-control-surfaces.md and the generated flexcontrol-actions.md).

The one change: drop the added docs paragraph

The user guide was already correct. It already says FlexControl Knob & Buttons... everywhere, so the source change brings the app into line with the docs and no page needs editing. The new paragraph adds no information:

  • "In builds with serial-port support … opens the FlexControl Tuning Knob group directly" repeats the Controllers row at menu-reference.md:31 and the build-feature note two lines above it.
  • "The menu label includes the & character on Linux, macOS and Windows" only makes sense to someone who saw the bug. AGENTS.md § User documentation says "State current behaviour, never history", and the Docs Style Guide says "Describe current behavior". To a reader who never saw the double space, the sentence is puzzling.

AGENTS.md's "same PR" docs rule is satisfied here by the existing pages already matching. It doesn't need a new paragraph. A one-click suggestion is inline.

What I tried to break

  • Lookups by label: the bridge matcher, tests, gen_reference.py and docs. All either unaffected or now consistent (above).
  • Sibling defects: no other unescaped menu/action literal in src/.
  • Platform split: the HAVE_SERIALPORT guard is unchanged, and the action is still setVisible(true) / NoRole, so macOS won't move it into the app menu.
  • Not driven in the running app. This is a one-literal change whose outcome Qt's documented && semantics decide, and your offscreen check already exercised the action. I'm stating that as reasoned from code and Qt docs, not reproduced. I also didn't visually check Windows or macOS.

Next step

Drop the paragraph (accept the suggestion) and I'll approve and merge. CODEOWNERS puts both files in Tier 3 (* and docs/ → @aethersdr/reviewers).

73,
Ryan NF0T


👨🏼‍💻 Co-authored by Claude Opus 5.5

Comment thread docs/user/docs/menu-reference.md Outdated
@w5jwp

w5jwp commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

@NF0T Removed the redundant paragraph from menu-reference.md in e7d8d58, as requested. The commit contains only that paragraph removal; the existing documentation and source fix remain in place. Could you re-review? Thanks!

@NF0T NF0T left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @w5jwp, e7d8d58b is exactly the requested change and nothing else. I checked the compare diff bb40bcca...e7d8d58b: it only removes the two lines from menu-reference.md. Against main the PR is now the single & → && line in MainWindow_Menus.cpp.

Re-checked on the current head:

  • Diff: one line, one file. The user guide already documents FlexControl Knob & Buttons... (menu-reference.md:31, among others), so with this fix the app matches the docs.
  • CI: all five checks are green on the pull_request run for e7d8d58b, and it merges cleanly onto current main.
  • Signing: both commits are valid.
  • CODEOWNERS: the only file left matches * → @aethersdr/reviewers (Tier 3).

My earlier findings still hold: the bridge's menu matcher already treats && as &, and no other menu/action literal in src/ has an unescaped ampersand. Not driven in the running app. The result is decided by Qt's documented && escape, and your offscreen check already exercised the action.

For future reference only: the PR body still says menu-reference.md was updated. The squash commit body will describe the final diff.

Approving and merging. Thanks for the careful audit and the clean, minimal fix.

73,
Ryan NF0T


👨🏼‍💻 Co-authored by Claude Opus 5.5

@NF0T
NF0T merged commit 20f7e15 into aethersdr:main Oct 19, 2026
5 checks passed
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.

FlexControl controller menu drops its ampersand and displays an extra gap

2 participants