Skip to content

fix(settings): button bar layout never persisted due to slash in AppSettings key (#3358)#3409

Merged
NF0T merged 1 commit into
aethersdr:mainfrom
M7HNF-Ian:fix/3358-buttonbar-keys
Jun 5, 2026
Merged

fix(settings): button bar layout never persisted due to slash in AppSettings key (#3358)#3409
NF0T merged 1 commit into
aethersdr:mainfrom
M7HNF-Ian:fix/3358-buttonbar-keys

Conversation

@M7HNF-Ian

@M7HNF-Ian M7HNF-Ian commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Problem

`ButtonBar/Layout`, `ButtonBar/DrawerOpen`, and `ButtonBar/Favorites` all contain a `/`. `AppSettings::save()` silently drops any key not matching `^[A-Za-z_][A-Za-z0-9_]*$` — the validator was tightened in #985 to prevent invalid XML element names. The customisable button bar shipped in #3150 after that tightening, so its layout, drawer state, and favourites have never round-tripped to disk for any user.

Fix

Rename the three keys to `ButtonBarLayout`, `ButtonBarDrawerOpen`, `ButtonBarFavorites`. No migration needed — nothing was ever written to the old slash-containing keys, so there is no on-disk state to convert.

Testing

Verified locally on macOS (Apple Silicon, dev build off `main`):

  • Before fix: `ButtonBar/Layout` and `ButtonBar/DrawerOpen` absent from `AetherSDR.settings` after every save (silently discarded)
  • After fix: `ButtonBarLayout` and `ButtonBarDrawerOpen` present in `AetherSDR.settings` with correct JSON values
  • Button bar order and drawer state survive app restart

Fixes #3358

…ettings key (aethersdr#3358)

ButtonBar/Layout, ButtonBar/DrawerOpen, and ButtonBar/Favorites all
contain a slash. AppSettings::save() silently drops any key not
matching ^[A-Za-z_][A-Za-z0-9_]*$ — the validator was tightened in
aethersdr#985 to prevent invalid XML element names. The customisable button
bar shipped in aethersdr#3150 after that tightening, so its layout, drawer
state, and favourites have never round-tripped to disk for any user.

Fix: rename the three keys to ButtonBarLayout, ButtonBarDrawerOpen,
ButtonBarFavorites. No migration needed — nothing was ever written
to the old keys so there is no on-disk state to convert.

Verified locally: confirmed ButtonBarLayout and ButtonBarDrawerOpen
are present in AetherSDR.settings after the fix; previously they
were absent (silently discarded on every save).

Fixes aethersdr#3358
@M7HNF-Ian M7HNF-Ian requested a review from a team as a code owner June 5, 2026 16:52

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the careful diagnosis and clean fix, @M7HNF-Ian.

Verified

  • src/core/AppSettings.cpp:209 does enforce ^[A-Za-z_][A-Za-z0-9_]*$ and silently drops non-conforming keys (only a qWarning is emitted, line 212). Your description is exactly right.
  • Grepped for other slashed AppSettings keys — these three are the only offenders, and all five call-sites are covered (AppletPanel.cpp:892, :1372, :1391, :1399, :1420).
  • The "no migration needed" reasoning is sound: since save() drops invalid keys before they reach disk, no user can have ButtonBar/Layout (etc.) in their settings file — there is nothing to convert.
  • Scope is appropriately surgical: single file, behavior-preserving rename.

One small observation (non-blocking)

After this rename, the legacy-favourites migration block at AppletPanel.cpp:1389-1402 (ButtonBarFavoritesButtonBarLayout) is dead code for the same reason your fix is needed: that key also contained a slash in its previous life, so no user has it on disk either. It's harmless to keep as a defensive no-op, but a follow-up to delete it would tighten the file. Up to you / the maintainers — not worth gating this PR.

LGTM as a clean root-cause fix for #3358.


🤖 aethersdr-agent · cost: $8.0788 · model: claude-opus-4-7

@NF0T NF0T self-assigned this Jun 5, 2026
@NF0T NF0T enabled auto-merge (squash) June 5, 2026 20:45

@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.

Clean root-cause fix, @M7HNF-Ian. Traced the validator at AppSettings.cpp:209^[A-Za-z_][A-Za-z0-9_]*$ confirmed, and your "no migration needed" reasoning is exactly right: since save() silently drops slash-containing keys before they reach disk, no user has ever had ButtonBar/Layout or its siblings written to their settings file. Nothing to convert.

Verified all 6 call sites against the PR commit — zero remaining slash-keys in AppletPanel.cpp. The dead migration block at line 1386 is a harmless pre-existing no-op for the same reason; leaving it is the right call for this PR.

All 5 CI checks green.

73,
Ryan NF0T

@NF0T NF0T merged commit 8f3af87 into aethersdr:main Jun 5, 2026
5 checks passed
@M7HNF-Ian M7HNF-Ian deleted the fix/3358-buttonbar-keys branch June 7, 2026 14:58
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
…ettings key (aethersdr#3358) (aethersdr#3409)

## Problem

\`ButtonBar/Layout\`, \`ButtonBar/DrawerOpen\`, and
\`ButtonBar/Favorites\` all contain a \`/\`. \`AppSettings::save()\`
silently drops any key not matching \`^[A-Za-z_][A-Za-z0-9_]*$\` — the
validator was tightened in aethersdr#985 to prevent invalid XML element names.
The customisable button bar shipped in aethersdr#3150 *after* that tightening, so
its layout, drawer state, and favourites have **never round-tripped to
disk for any user**.

## Fix

Rename the three keys to \`ButtonBarLayout\`, \`ButtonBarDrawerOpen\`,
\`ButtonBarFavorites\`. No migration needed — nothing was ever written
to the old slash-containing keys, so there is no on-disk state to
convert.

## Testing

Verified locally on macOS (Apple Silicon, dev build off \`main\`):
- Before fix: \`ButtonBar/Layout\` and \`ButtonBar/DrawerOpen\` absent
from \`AetherSDR.settings\` after every save (silently discarded)
- After fix: \`ButtonBarLayout\` and \`ButtonBarDrawerOpen\` present in
\`AetherSDR.settings\` with correct JSON values
- Button bar order and drawer state survive app restart

Fixes aethersdr#3358
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.

Button bar and band plan settings persistence does not save across restarts

2 participants