Repository navigation
feat(qt): add global zoom shortcuts - #7216
PastaPastaPasta wants to merge 6 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00bdcf9910
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds global font scaling to the Qt GUI: introduces FONT_SCALE_MIN / FONT_SCALE_MAX / FONT_SCALE_SHORTCUT_STEP and three View actions (Zoom In, Zoom Out, Reset) wired to a new private BitcoinGUI::adjustFontScale(int delta) that clamps, applies, refreshes, and persists font scale when allowed. Expands appearance widget slider range. Updates GUIUtil::AddButtonShortcut to accept an explicit Qt::ShortcutContext (defaulting to WindowShortcut) and registers font-size shortcuts in RPCConsole using the new helper to avoid duplicate shortcuts on theme changes. Sequence DiagramsequenceDiagram
actor User
participant BitcoinGUI
participant GUIUtil
participant OptionsModel
User->>BitcoinGUI: Trigger Zoom In / Zoom Out / Reset
BitcoinGUI->>BitcoinGUI: adjustFontScale(delta)\ncompute & clamp new scale
BitcoinGUI->>GUIUtil: set global font scale / refresh fonts
GUIUtil->>GUIUtil: update font registry\napply fonts across UI
alt OptionsModel available
BitcoinGUI->>OptionsModel: save new font scale
OptionsModel-->>BitcoinGUI: acknowledge
end
GUIUtil-->>User: UI updated with new font scale
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Seems to be working as expected but
|
00bdcf9 to
04cb014
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04cb01468e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/qt/bitcoingui.cpp`:
- Around line 529-531: The zoom-out shortcut was set to "Ctrl+_" but should be
"Ctrl+-"; update the shortcut string passed to m_zoom_out_action->setShortcuts
to QKeySequence(tr("Ctrl+-")) so it matches QKeySequence::ZoomOut, and make the
same change wherever the alternative shortcut string is used (e.g., the rpc
console's QAction/QKeySequence setup) so both m_zoom_out_action and the rpc
console shortcut use "Ctrl+-" instead of "Ctrl+_".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 741c0042-bcc9-4234-a35f-f11a73b96fbb
📥 Commits
Reviewing files that changed from the base of the PR and between 00bdcf991097a8d73320a92bc0607ebe08b1201c and 04cb01468e99d09514a17603c102bb826eb01aab.
📒 Files selected for processing (3)
CLAUDE.mdsrc/qt/bitcoingui.cppsrc/qt/bitcoingui.h
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/qt/bitcoingui.h
036cbdb to
5743625
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5743625069
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
5743625 to
391ac6d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 391ac6d679
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/qt/bitcoingui.cpp`:
- Around line 779-784: After updating the font scale
(GUIUtil::g_font_registry.SetFontScale and GUIUtil::updateFonts()) and saving it
to the options model, call the window width recalculation so the tab toolbar
minimum width is recomputed; add a call to updateWidth() (the
BitcoinGUI::updateWidth() method) after the clientModel/optionsModel update to
recompute main-window width and avoid clipped labels.
- Around line 525-535: adjustFontScale currently returns early when
OptionsModel::isOptionOverridden("-font-scale") is true but leaves the UI
actions active; update adjustFontScale (or the same initialization path) to
query OptionsModel::isOptionOverridden("-font-scale") and call
m_zoom_in_action->setEnabled(false), m_zoom_out_action->setEnabled(false), and
m_zoom_reset_action->setEnabled(false) when overridden (and setEnabled(true)
when not) so the zoom QAction items reflect the override state and no longer
advertise shortcuts that perform no-ops.
In `@src/qt/forms/appearancewidget.ui`:
- Around line 185-189: AppearanceWidget::updateFontScale currently passes the
raw slider value to GUIUtil::g_font_registry.SetFontScale, which can produce an
invalid zero font size for extreme values; fix it by clamping the incoming
nScale to the valid range before calling SetFontScale (use the same bounds as
elsewhere, e.g. constexpr FONT_SCALE_MIN = -100 and FONT_SCALE_MAX = 100 and
apply std::clamp), then call GUIUtil::g_font_registry.SetFontScale(nScale) and
GUIUtil::updateFonts().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8de5e6e9-0f72-4a20-964e-9ad88469d59a
📥 Commits
Reviewing files that changed from the base of the PR and between 04cb01468e99d09514a17603c102bb826eb01aab and 5743625069e86aae1667e1c051e31e1f85065b96.
📒 Files selected for processing (4)
CLAUDE.mdsrc/qt/bitcoingui.cppsrc/qt/bitcoingui.hsrc/qt/forms/appearancewidget.ui
🚧 Files skipped from review as they are similar to previous changes (2)
- CLAUDE.md
- src/qt/bitcoingui.h
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/qt/bitcoingui.cpp (1)
765-767:⚠️ Potential issue | 🟡 MinorZoom actions still present as no-ops when
-font-scaleis overridden.At Line 765,
adjustFontScale()exits early, but the menu actions remain enabled and advertised. Users can trigger shortcuts that do nothing.💡 Proposed fix (sync action enabled state with override)
OptionsModel* optionsModel = _clientModel->getOptionsModel(); if (optionsModel) { + const bool font_scale_locked{optionsModel->isOptionOverridden("-font-scale")}; + m_zoom_in_action->setEnabled(!font_scale_locked); + m_zoom_out_action->setEnabled(!font_scale_locked); + m_zoom_reset_action->setEnabled(!font_scale_locked); unitDisplayControl->setOptionsModel(optionsModel); m_mask_values_action->setChecked(optionsModel->getOption(OptionsModel::OptionID::MaskValues).toBool());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/qt/bitcoingui.cpp` around lines 765 - 767, The early return in adjustFontScale() when clientModel->getOptionsModel()->isOptionOverridden("-font-scale") leaves zoom-related QAction objects enabled and responsive but doing nothing; update adjustFontScale() to explicitly set the enabled state (e.g., zoomInAction, zoomOutAction, resetZoomAction) to false when the option is overridden (and true otherwise), and update their tooltips/status hints to reflect that zooming is disabled when overridden so shortcuts/menu items are not advertised as functional; locate and modify adjustFontScale() and the QAction members (zoomInAction, zoomOutAction, resetZoomAction) to implement this toggle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/qt/rpcconsole.cpp`:
- Around line 1351-1359: The problem is duplicate QShortcut registrations for
ui->fontBiggerButton and ui->fontSmallerButton because
GUIUtil::AddButtonShortcut is called each time setButtonIcons() runs; fix by
registering these shortcuts only once (e.g. move the GUIUtil::AddButtonShortcut
calls out of setButtonIcons() into the dialog/class constructor or gate them
with a boolean like shortcutsInitialized), or check for existing shortcuts on
the button (via button->findChildren<QShortcut*>() or button->shortcuts())
before calling GUIUtil::AddButtonShortcut for ui->fontBiggerButton and
ui->fontSmallerButton so repeated icon/theme reloads do not create duplicate
QShortcut objects.
---
Duplicate comments:
In `@src/qt/bitcoingui.cpp`:
- Around line 765-767: The early return in adjustFontScale() when
clientModel->getOptionsModel()->isOptionOverridden("-font-scale") leaves
zoom-related QAction objects enabled and responsive but doing nothing; update
adjustFontScale() to explicitly set the enabled state (e.g., zoomInAction,
zoomOutAction, resetZoomAction) to false when the option is overridden (and true
otherwise), and update their tooltips/status hints to reflect that zooming is
disabled when overridden so shortcuts/menu items are not advertised as
functional; locate and modify adjustFontScale() and the QAction members
(zoomInAction, zoomOutAction, resetZoomAction) to implement this toggle.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0a0e7d5c-dbf5-436e-a024-370a66f37c6e
📥 Commits
Reviewing files that changed from the base of the PR and between 5743625069e86aae1667e1c051e31e1f85065b96 and 391ac6d679665c38d9a79296c36a7913317830cd.
📒 Files selected for processing (7)
CLAUDE.mdsrc/qt/bitcoingui.cppsrc/qt/bitcoingui.hsrc/qt/forms/appearancewidget.uisrc/qt/guiutil.cppsrc/qt/guiutil.hsrc/qt/rpcconsole.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- CLAUDE.md
- src/qt/forms/appearancewidget.ui
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/qt/bitcoingui.cpp`:
- Around line 525-535: When adjustFontScale() exits early because the
-font-scale option is overridden, disable the zoom actions so they aren't active
no-ops: in setClientModel() after retrieving optionsModel call
optionsModel->isOptionOverridden("-font-scale") (or similar) and use that
boolean to call setEnabled(false) on m_zoom_in_action, m_zoom_out_action and
m_zoom_reset_action when the option is locked; this keeps the View menu and
shortcut hints in sync with the actual behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 69b282b2-962b-44df-acdb-beea25e6e8ec
📥 Commits
Reviewing files that changed from the base of the PR and between 391ac6d679665c38d9a79296c36a7913317830cd and d414f1d007fb774a36127ce4f8cccb189ea8a201.
📒 Files selected for processing (2)
src/qt/bitcoingui.cppsrc/qt/rpcconsole.cpp
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
PR widens the font scale slider from -30..30 to -100..100 and adds Ctrl+/Ctrl- global zoom shortcuts. The most impactful verified issue is that the expanded range allows font scale to reach -100, producing zero-sized (invisible) fonts. Most other findings are valid but lower severity; two findings are false positives.
🔴 1 blocking | 🟡 3 suggestion(s) | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/qt/bitcoingui.cpp`:
- [BLOCKING] lines 90-91: FONT_SCALE_MIN of -100 produces zero-sized fonts
The formula in `guiutil_font.h:85` is `size * (1 + (m_font_scale * 0.01))`. At scale -100 this yields `size * 0 = 0`, making all text invisible. The PR widened the UI slider range from the previous -30..30 (confirmed via git diff of `appearancewidget.ui`) to -100..100 to match these new constants. The old minimum of -30 gave `size * 0.7` which was still legible. A reasonable minimum would be around -50 (giving `size * 0.5`, i.e. half-size text).
- [SUGGESTION] lines 765-768: Menu zoom items remain enabled when -font-scale CLI override is active
When `-font-scale` is overridden via CLI, `adjustFontScale()` silently returns at line 767 without any user feedback. The View > Zoom In / Zoom Out / Reset Zoom menu items remain visually enabled and clickable, but do nothing. Users have no way to know why the controls don't work.
- [SUGGESTION] lines 90-91: Font scale range defined in two places — drift risk
The constants `FONT_SCALE_MIN{-100}` and `FONT_SCALE_MAX{100}` are defined in an anonymous namespace in `bitcoingui.cpp:90-91`. The font scale slider in `appearancewidget.ui:185-189` has matching but independently hardcoded minimum/maximum values. If either is changed independently, the slider and keyboard shortcuts would enforce different ranges.
In `src/qt/rpcconsole.cpp`:
- [SUGGESTION] lines 564-570: Overlapping shortcuts between global zoom and RPC console font-size buttons
The new window-level zoom actions use `QKeySequence::ZoomIn` (Ctrl++) and `Ctrl+=` (line 526 of bitcoingui.cpp). The RPC console registers widget-level shortcuts for the same keys (`Ctrl++`, `Ctrl+=`, `Ctrl+-`, `Ctrl+_`) at `WidgetWithChildrenShortcut` context. When the RPC console has focus, Qt may raise an ambiguous-shortcut conflict since both a `WindowShortcut` and `WidgetWithChildrenShortcut` match the same key. The two actions also do different things: one changes the global app font scale, the other changes only the console font size.
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76bec14969
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
PR #7216 adds Ctrl+/- zoom shortcuts with font scale persistence. Verified 2 agent findings: 1 confirmed with reduced severity (shortcut scope matches existing pattern), 1 dropped (factually incorrect claim about startup clamping). Found 1 additional CLI range inconsistency.
Reviewed commit: 76bec149
💬 2 nitpick(s)
2 additional findings
💬 nitpick: Zoom shortcuts use Qt::WindowShortcut scope (consistent with existing pattern)
src/qt/bitcoingui.cpp (line 1)
The zoom QActions at bitcoingui.cpp:525-535 use the default Qt::WindowShortcut context (no setShortcutContext call found anywhere in src/qt/). This means shortcuts won't fire when a separate dialog (e.g., OptionsDialog) has focus on Linux/Windows. However: (1) this is consistent with ALL other shortcuts in BitcoinGUI (e.g., Ctrl+Shift+D, Ctrl+M), (2) on macOS, native menu bar shortcuts work regardless of active window, and (3) 'global' in the commit message likely refers to zoom affecting all fonts application-wide, not the shortcut scope. Downgraded from medium since it matches existing codebase conventions.
💬 nitpick: CLI -font-scale validation still accepts [-100, 100] while shortcut/slider clamp uses [-50, 100]
src/qt/bitcoin.cpp (line 1)
Commit 76bec14 changed FONT_SCALE_MIN to -50 'to prevent zero-sized fonts', but bitcoin.cpp:750 still validates the -font-scale CLI arg against [-100, 100]. At scale -100, GetScaledFontSize() computes size * (1 + (-100 * 0.01)) = 0, producing zero-sized fonts. The slider range was also updated from [-30,30] to [-50,100] in appearancewidget.ui. Pre-existing issue but made more visible by this PR's intent to prevent zero-sized fonts. Consider updating bitcoin.cpp:750 to use -50 as the minimum for consistency.
76bec14 to
bf517df
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf517dfbc0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Clean feature PR that adds global zoom shortcuts to Dash Qt by reusing existing font-scaling infrastructure and fixes a real duplicate-shortcut bug in the RPC console. The one substantive issue is that the CLI -font-scale validation still accepts -100 (producing zero-sized fonts), inconsistent with the PR's stated goal. The rest of the findings are either resolved at HEAD or false positives.
Reviewed commit: bf517df
🟡 2 suggestion(s) | 💬 1 nitpick(s)
2 additional findings
🟡 suggestion: CLI `-font-scale` validation still allows -100, producing zero-sized fonts
src/qt/bitcoin.cpp (lines 750-757)
The commit bf517df explicitly aims to "prevent zero-sized fonts" by clamping the GUI minimum to -50. The slider (appearancewidget.ui) and shortcut path (bitcoingui.cpp:773) both enforce -50..100. However, the CLI validation at bitcoin.cpp:750 still accepts -100..100, and the help text at line 504 still advertises that range.
With GetScaledFontSize(size) = size * (1 + (scale * 0.01)), a scale of -100 yields size * 0.0 = 0, giving zero-point-size fonts. A user with font-scale=-100 in dash.conf or on the command line would get invisible text.
This is pre-existing behavior, so not blocking for this PR, but since the PR explicitly targets this class of bug it would be consistent to update both the CLI bounds and the help text to match the GUI's -50..100 range.
💡 Suggested change
if (gArgs.IsArgSet("-font-scale")) {
const int nScaleMin = -50, nScaleMax = 100;
int nScale = gArgs.GetIntArg("-font-scale", GUIUtil::g_font_registry.GetFontScale());
if (nScale < nScaleMin || nScale > nScaleMax) {
QMessageBox::critical(nullptr, PACKAGE_NAME,
QObject::tr("Error: Specified font-scale invalid. Valid range %1 to %2.").arg(nScaleMin).arg(nScaleMax));
return EXIT_FAILURE;
}
GUIUtil::g_font_registry.SetFontScale(nScale);
}
🟡 suggestion: Help text for `-font-scale` advertises -100..100, should match actual bounds
src/qt/bitcoin.cpp (line 504)
If the CLI bounds are updated to -50..100 (per the previous finding), the help text at line 504 still says .arg(-100).arg(100) and would need to be updated to .arg(-50).arg(100) to stay accurate.
💡 Suggested change
argsman.AddArg("-font-scale", QObject::tr("Set a scale factor which gets applied to the base font size. Possible range %1 (smallest fonts) to %2 (largest fonts). (default: %3)").arg(-50).arg(100).arg(GUIUtil::FontRegistry::DEFAULT_FONT_SCALE).toStdString(), ArgsManager::ALLOW_ANY, OptionsCategory::GUI);
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/qt/bitcoin.cpp`:
- [SUGGESTION] lines 750-757: CLI `-font-scale` validation still allows -100, producing zero-sized fonts
The commit bf517dfbc0 explicitly aims to "prevent zero-sized fonts" by clamping the GUI minimum to -50. The slider (appearancewidget.ui) and shortcut path (bitcoingui.cpp:773) both enforce -50..100. However, the CLI validation at bitcoin.cpp:750 still accepts -100..100, and the help text at line 504 still advertises that range.
With `GetScaledFontSize(size) = size * (1 + (scale * 0.01))`, a scale of -100 yields `size * 0.0 = 0`, giving zero-point-size fonts. A user with `font-scale=-100` in `dash.conf` or on the command line would get invisible text.
This is pre-existing behavior, so not blocking for this PR, but since the PR explicitly targets this class of bug it would be consistent to update both the CLI bounds and the help text to match the GUI's -50..100 range.
- [SUGGESTION] line 504: Help text for `-font-scale` advertises -100..100, should match actual bounds
If the CLI bounds are updated to -50..100 (per the previous finding), the help text at line 504 still says `.arg(-100).arg(100)` and would need to be updated to `.arg(-50).arg(100)` to stay accurate.
thepastaclaw
added
the
pastaclaw:changes-requested
e000404 to
1062ca0
Compare
|
knst: I reshaped the series so no commit removes the console's own font-size shortcuts and a later commit restores them. The final tree is byte-for-byte the same as the previous head e000404 (
Every commit builds on its own with Your thread about the console shortcuts (#7216 (comment)) stays open. Please confirm on your setup that 🤖 Posted autonomously by Codex on behalf of pasta. |
thepastaclaw
removed
the
pastaclaw:changes-requested
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Verified the findings against exact head 1062ca0. The reshaped commit series resolves the console-shortcut history concern, but startup still rejects out-of-range persisted font scales after clamping them, and the zoom tests bypass keyboard routing. This was a static review; the supplied CI snapshot shows successful lint and formatting checks, with platform builds and tests still pending.
🔴 1 blocking | 🟡 1 suggestion(s)
2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — The change adds GUI behavior with platform-dependent shortcut routing, enabled-state handling, and font-scale persistence, requiring ordinary logic review but touching no critical surface. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort high); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/qt/optionsmodel.cpp`:
- [BLOCKING] src/qt/optionsmodel.cpp:270-272: Preserve the persisted-scale clamp through startup validation
(existing thread: https://github.com/dashpay/dash/pull/7216#discussion_r4175686802)
The clamp changes only the live font scale, not the setting subsequently validated during startup. After createOptionsModel(), GuiMain() calls gArgs.IsArgSet("-font-scale") and GetIntArg() at bitcoin.cpp:718–726. Both read merged settings, including settings.json: IsArgSet() checks whether GetSetting() is non-null, and GetIntArg() converts that same setting. With a saved "font-scale": "-80" and no explicit override, Init() applies -50, but GuiMain() rereads -80 and returns EXIT_FAILURE. This PR newly rejects a previously accepted saved value and therefore does not deliver the documented clamp-on-load behavior. Normalize the saved value or distinguish persisted settings from explicit overrides during startup validation, retaining rejection of invalid explicit arguments. The new OptionTests case checks only Init() and cannot detect this later failure.
In `src/qt/test/apptests.cpp`:
- [SUGGESTION] src/qt/test/apptests.cpp:69-75: Exercise keyboard routing rather than only activating actions
(existing thread: https://github.com/dashpay/dash/pull/7216#discussion_r4175686804)
QAction::activate() verifies the handlers and persistence but bypasses shortcut matching, focus context, and ambiguity handling. This test would still pass if the new actions had no key bindings or competed with the console shortcuts. Those behaviors are central to this feature: BitcoinGUI conditionally omits zoom-in/out bindings when the console is embedded and forwards action activation to the separate active console. Add focused keyboard-event coverage for the main window and the RPC console line edit, checking that one key press changes only the intended global scale or console font size. Include the embedded-console case to protect the no-duplicate-bindings behavior. The reported cocoa action-test pass does not exercise these paths; native macOS menu routing also remains unvalidated interactively according to the PR description.
1062ca0 to
5f9ea89
Compare
thepastaclaw
removed
the
pastaclaw:changes-requested
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Independently reviewed the complete diff and six-commit series at 5f9ea89; no actionable in-scope defects remain. The revised history, real keyboard-event coverage, and persisted-scale write-back address the three prior findings, while embedded-console and native macOS menu routing remain explicitly documented validation gaps. This was a static review with no builds or tests run; the supplied CI snapshot still has build jobs queued or in progress.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — The change spans Qt shortcut routing, enabled-state handling, font-scale validation and persistence, and regression tests, but does not alter a critical surface. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort high); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
5f9ea89 to
477effd
Compare
thepastaclaw
removed
the
pastaclaw:approved
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
The complete diff at 477effd fixes persisted-scale startup validation and preserves console shortcut registration throughout the reshaped series, but the new Qt tests fail in the supported no-wallet layout. Main-window zoom-out also lacks keyboard-event coverage. This verification was static only; the supplied CI snapshot shows successful lint and formatting checks, with platform builds still pending.
🔴 1 blocking | 🟡 1 suggestion(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — The change spans Qt shortcut routing, font-scale validation and persistence, platform-specific console behavior, and regression tests, but does not change a critical surface. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort high); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/qt/test/apptests.cpp`:
- [BLOCKING] src/qt/test/apptests.cpp:71-74: Adapt the zoom tests to the no-wallet window layout
AppTests is compiled and executed without an ENABLE_WALLET guard: src/Makefile.qttest.include includes apptests.cpp unconditionally, and test_main.cpp always runs AppTests. In a no-wallet build, BitcoinGUI embeds RPCConsole as its central widget and leaves walletFrame null, so createActions() intentionally does not register global ZoomIn/ZoomOut shortcuts. The key press here therefore cannot increase GUIUtil::fontScale(), and this assertion fails despite correct application behavior. TestConsoleZoomShortcuts also fails its unconditional console->isWindow() check at line 87 because that console is embedded. Adapt both helpers to the actual layout: retain global keyboard expectations for the wallet window, exercise global zoom through the menu action in the embedded layout, and activate console->window() while verifying that console keys resize only console text. This is established by the source paths, not attributed to an observed CI failure.
- [SUGGESTION] src/qt/test/apptests.cpp:71-78: Zoom-out binding has no key-press coverage
The main-window test presses ZoomIn and Ctrl+0, but never presses QKeySequence::ZoomOut. The console's Ctrl+- test checks a different binding and handler that change only console text size. Removing the main-window zoom-out shortcut or breaking its negative-delta connection would therefore leave these tests passing. Add a main-window ZoomOut key sequence from a known in-range scale and assert that both the live and persisted application scale decrease; keep the reset check starting from a non-default scale so it continues to detect a broken reset binding.
| const int initial_scale{GUIUtil::fontScale()}; | ||
| QTest::keySequence(window, QKeySequence::ZoomIn); | ||
| QVERIFY(GUIUtil::fontScale() > initial_scale); | ||
| QCOMPARE(persisted_scale(), int64_t{GUIUtil::fontScale()}); |
There was a problem hiding this comment.
✅ Resolved at
7a29ebb8; see the replies below.
🔴 Blocking: Adapt the zoom tests to the no-wallet window layout
AppTests is compiled and executed without an ENABLE_WALLET guard: src/Makefile.qttest.include includes apptests.cpp unconditionally, and test_main.cpp always runs AppTests. In a no-wallet build, BitcoinGUI embeds RPCConsole as its central widget and leaves walletFrame null, so createActions() intentionally does not register global ZoomIn/ZoomOut shortcuts. The key press here therefore cannot increase GUIUtil::fontScale(), and this assertion fails despite correct application behavior. TestConsoleZoomShortcuts also fails its unconditional console->isWindow() check at line 87 because that console is embedded. Adapt both helpers to the actual layout: retain global keyboard expectations for the wallet window, exercise global zoom through the menu action in the embedded layout, and activate console->window() while verifying that console keys resize only console text. This is established by the source paths, not attributed to an observed CI failure.
source: gpt-6.1-sol (phase2-reviewer: general)
There was a problem hiding this comment.
Resolved (re-reviewed at 7a29ebb8): Your current tests detect the embedded console, activate the global Zoom In menu action instead of its intentionally absent shortcut, and wait for console->window() rather than requiring a separate console window. These changes remove the no-wallet layout assumptions behind the failure.
| const int initial_scale{GUIUtil::fontScale()}; | ||
| QTest::keySequence(window, QKeySequence::ZoomIn); | ||
| QVERIFY(GUIUtil::fontScale() > initial_scale); | ||
| QCOMPARE(persisted_scale(), int64_t{GUIUtil::fontScale()}); | ||
|
|
||
| QTest::keySequence(window, QKeySequence{"Ctrl+0"}); | ||
| QCOMPARE(GUIUtil::fontScale(), GUIUtil::defaultFontScale()); | ||
| QCOMPARE(persisted_scale(), int64_t{GUIUtil::defaultFontScale()}); |
There was a problem hiding this comment.
🟡 Suggestion: Zoom-out binding has no key-press coverage
The main-window test presses ZoomIn and Ctrl+0, but never presses QKeySequence::ZoomOut. The console's Ctrl+- test checks a different binding and handler that change only console text size. Removing the main-window zoom-out shortcut or breaking its negative-delta connection would therefore leave these tests passing. Add a main-window ZoomOut key sequence from a known in-range scale and assert that both the live and persisted application scale decrease; keep the reset check starting from a non-default scale so it continues to detect a broken reset binding.
source: muse-spark-1.3-contributor (phase1-reviewer: general)
There was a problem hiding this comment.
Fixed in 713c318. TestZoomShortcuts now zooms in twice, then presses QKeySequence::ZoomOut and checks both the live scale and the persisted value step back. In the embedded-console layout it triggers the Zoom Out action, since that action has no key bindings there.
🤖 Posted autonomously by Codex on behalf of pasta.
thepastaclaw
added
the
pastaclaw:changes-requested
477effd to
7a29ebb
Compare
thepastaclaw
removed
the
pastaclaw:changes-requested
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Verified the complete diff at head 7a29ebb and revalidated all five prior findings. Four prior issues are fixed; the global zoom-out coverage suggestion remains, and the new zoom persistence path throws when dynamic settings are disabled with -nosettings. This was a static review: the supplied CI snapshot reports successful lint, several platform builds, and no-wallet tests, with other build/test jobs still pending; native macOS menu routing remains unverified.
🔴 1 blocking | 🟡 1 suggestion(s)
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — The change spans Qt shortcut routing, font-scale validation and persistence, UI bounds, and regression tests without changing a critical surface. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort high); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/qt/bitcoingui.cpp`:
- [BLOCKING] src/qt/bitcoingui.cpp:804-805: Avoid persisting zoom changes when -nosettings is active
With -nosettings and no command-line -font-scale override, setClientModel() enables the new zoom actions because it checks only whether font-scale is overridden. A main-window zoom change then calls setOption(FontScale), which reaches UpdateRwSetting(), NodeImpl::updateRwSetting(), and ArgsManager::WriteSettingsFile(). WriteSettingsFile() explicitly throws std::logic_error when GetSettingsPath() returns false, so pressing an ordinary zoom shortcut throws during Qt action handling instead of safely completing the change. Startup supports disabling dynamic settings, and ErrorSettingsWrite() even recommends -nosettings for an unwritable settings file. Disable the global zoom actions when dynamic settings are disabled, or implement session-only zoom that skips persistence, and add coverage for this configuration.
In `src/qt/test/apptests.cpp`:
- [SUGGESTION] src/qt/test/apptests.cpp:77-88: Zoom-out binding has no key-press coverage
(existing thread: https://github.com/dashpay/dash/pull/7216#discussion_r4176908396)
TestZoomShortcuts exercises the main-window Zoom In and Reset Zoom bindings, but never invokes global Zoom Out. Its shortcuts().isEmpty() check only verifies that a shortcut list exists: substituting an incorrect nonempty binding, disconnecting the zoom-out handler, or changing its delta to positive would escape these assertions. The console's Ctrl+- test exercises RPCConsole::fontSmaller(), not the global font-scale action. After zooming in, send QKeySequence::ZoomOut to the main window and assert that the live scale decreases and the persisted value matches. In the embedded-console layout, trigger the Zoom Out menu action instead, since its keyboard bindings are intentionally absent.
| if (clientModel && clientModel->getOptionsModel()) { | ||
| clientModel->getOptionsModel()->setOption(OptionsModel::FontScale, new_scale); |
There was a problem hiding this comment.
🔴 Blocking: Avoid persisting zoom changes when -nosettings is active
With -nosettings and no command-line -font-scale override, setClientModel() enables the new zoom actions because it checks only whether font-scale is overridden. A main-window zoom change then calls setOption(FontScale), which reaches UpdateRwSetting(), NodeImpl::updateRwSetting(), and ArgsManager::WriteSettingsFile(). WriteSettingsFile() explicitly throws std::logic_error when GetSettingsPath() returns false, so pressing an ordinary zoom shortcut throws during Qt action handling instead of safely completing the change. Startup supports disabling dynamic settings, and ErrorSettingsWrite() even recommends -nosettings for an unwritable settings file. Disable the global zoom actions when dynamic settings are disabled, or implement session-only zoom that skips persistence, and add coverage for this configuration.
source: gpt-6.1-sol (phase2-reviewer: general)
There was a problem hiding this comment.
Confirmed and fixed in d5f2e46. Without the guard, zooming with -nosettings aborts with std::logic_error: Attempt to write settings file when dynamic settings are disabled (reproduced in test_dash-qt). adjustFontScale() now persists only when gArgs.GetSettingsPath() succeeds, so the zoom lasts for the session. I kept the actions enabled because the zoom itself still works. TestZoomShortcuts forces -nosettings and checks that the scale changes but the setting is not written. It fails without the guard.
🤖 Posted autonomously by Codex on behalf of pasta.
thepastaclaw
added
the
pastaclaw:changes-requested
|
This pull request has conflicts, please rebase. |
setButtonIcons() re-runs on every theme change (and once more from the constructor via reloadThemedWidgets()), and each run created another QShortcut for the secondary Ctrl+= / Ctrl+_ keys through GUIUtil::AddButtonShortcut(). Duplicate shortcuts for the same key in the same window are ambiguous, so Qt stops delivering them. Register the console font-size shortcuts once in the constructor instead; the keys and their window context are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Add Zoom In, Zoom Out and Reset Zoom actions to the Window menu, bound to QKeySequence::ZoomIn / ZoomOut plus Ctrl+= / Ctrl+_, and Ctrl+0. They step the global font scale by 3 within GUIUtil::FONT_SCALE_MIN/MAX (-50..100; much lower shrinks text towards zero size), or reset it to the default, apply it live through GUIUtil::updateFonts(), recompute the toolbar width and persist it like the Appearance tab's font scale. With -nosettings there is no settings file to write to (writing would throw), so the zoom then lasts for the session only. The Appearance slider takes its range from the same bounds, widening it from -30..30 so it can show every scale the shortcuts can reach. The actions stay disabled until a client model is set (and again once it is cleared), and while -font-scale is given on the command line, matching the Appearance slider. The RPC console keeps its own font-size shortcuts: - Without a wallet the console is the main window's central widget and already binds the zoom-in/out keys, so the global zoom-in/out actions get no key bindings there; the menu entries still zoom the whole UI. - On macOS the main window's menu bar stays active while the separate console window has focus and would grab Cmd++, so the zoom-in/out actions resize the console text when the console window is active. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
-font-scale accepted -100..100, but -100 scales every font by 1 + (-100 * 0.01) = 0, making all text invisible, and the Appearance slider and zoom shortcuts stop at -50. Validate the option and describe it in its help text using GUIUtil::FONT_SCALE_MIN/MAX, so values below -50 are now rejected at startup. Move the startup validation into GUIUtil::setFontScaleFromArg() so tests can drive the same check GuiMain() runs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
OptionsModel::Init() applied the persisted font-scale setting unchecked, so a value outside -50..100 (e.g. from a hand-edited settings.json) could shrink text to nothing at -100, and it made the zoom shortcuts jump to the range boundary on their first step instead of moving by one step. Clamp it on load and, when the value came from settings.json, write the clamped value back. GuiMain() validates -font-scale again after OptionsModel::Init() through gArgs, which also sees settings.json, so without the write-back a saved -80 would still abort startup with "Specified font-scale invalid". Command-line and config-file values keep the strict validation: they are not written back, so a config value is never shadowed by settings.json and an out-of-range one is still rejected. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Zoom in, zoom out and press Reset Zoom on the real main window and check that the live font scale moves and that OptionsModel persists the new value. With dynamic settings disabled (-nosettings) zooming must still work without writing the setting. Then press the zoom keys in the RPC console and check that they resize only the console text and leave the application font scale alone. Cover both layouts. With a wallet the RPC console is a separate window and the Zoom In/Out actions own the zoom keys. Without one (built with --disable-wallet or run with -disablewallet) the console is embedded in the main window and keeps those keys for its own font size, so the actions must have no shortcuts and are triggered from the menu instead; Reset Zoom stays on Ctrl+0 in both. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
7a29ebb to
cbf65bb
Compare
thepastaclaw
removed
the
pastaclaw:changes-requested
Issue being fixed or feature implemented
Settings > Options > Appearance.Ctrl++,Ctrl+-, andCtrl+0behavior into the existing infrastructure.What was done?
Screen.Recording.2026-03-12.at.10.41.57.mov
(The recording predates moving the entries from a separate View menu into the Window menu.)
BitcoinGUI, listed in the Window menu.QKeySequence::ZoomIn/QKeySequence::ZoomOutplusCtrl+=/Ctrl+_, andCtrl+0for reset.BitcoinGUI::adjustFontScale(int delta)reuses the existing font scaling path: it adjusts (or resets)GUIUtil::fontScale()within the supported range, applies it live withGUIUtil::updateFonts(), recomputes the toolbar width and persists the value throughOptionsModel::FontScale. With-nosettingsthere is no settings file to write, so the zoom applies for the session only; writing would throwstd::logic_errorfromArgsManager::WriteSettingsFile().-font-scalehelp/validation, Appearance slider, shortcuts), defined once asGUIUtil::FONT_SCALE_MIN/MAX. -100 used to be accepted by-font-scaleand produced zero-sized fonts; the slider used to stop at -30..30. Command-line and config-file values are validated strictly (out of range aborts startup). A value saved insettings.jsonis never fatal:OptionsModelclamps it on load and writes the clamped value back, so the-font-scalecheckGuiMain()runs afterwards (viaGUIUtil::setFontScaleFromArg(), which also seessettings.json) accepts it. Nothing is written back when the effective value comes from the command line or config file, so a config value is never shadowed bysettings.json.-font-scaleis given on the command line (like the Appearance slider) and while there is no client model.Ctrl++/Ctrl+=/Ctrl+-/Ctrl+_font-size shortcuts keep working as before, but are now registered once in the constructor instead of insetButtonIcons(), which re-runs on every theme change and stacked duplicateQShortcuts.-disablewalletor no-wallet builds), the zoom in/out actions get no key bindings so they don't make the console's shortcuts ambiguous; the menu entries still zoom the whole UI.doc/release-notes-7216.md.Commits:
fix(qt): register RPC console font-size shortcuts only once(pre-existing bug:setButtonIcons()runs twice during construction and again on every theme change, stacking ambiguous duplicateCtrl+=/Ctrl+_shortcuts)feat(qt): add global zoom shortcuts(actions in the Window menu, -50..100 bounds inGUIUtil, wider Appearance slider, enabled-state handling, RPC console key routing)fix(qt): clamp CLI -font-scale minimum to -50 to match GUI bounds(startup validation moved intoGUIUtil::setFontScaleFromArg()so tests can run the same check)fix(qt): clamp the persisted font scale to the supported range(clamp + write-back of out-of-rangesettings.jsonvalues)test(qt): cover the zoom shortcuts in AppTestsdoc: add release notes for zoom shortcuts and font scale rangeHow Has This Been Tested?
OptionsModel::Init(), against cdeadac ("report malformed lang/font settings"). Thefont-scaleread stays inside that commit'stry/setting_errorhandler and now feeds the clamp.dash-qtandtest_dash-qton macOS arm64 (depends prefix,--enable-debug --enable-werror).src/qt/test/test_dash-qtwith the defaultminimalplatform: all tests pass. On macOS that platform skipsAppTestsandupdateFontsWithPixelSizedWidget;fontScaleSourcesruns there too. With the macOSminimalskip temporarily disabled,AppTestsincluding the key-press zoom tests also passed underminimal.QT_QPA_PLATFORM=cocoa src/qt/test/test_dash-qt:AppTests(including the newTestZoomShortcuts/TestConsoleZoomShortcuts, which press the real zoom-in, zoom-out and reset key sequences on the main window and on the separate RPC console's command line, and zoom under a forced-nosettingswithout writing the setting) and allOptionTests(including the newfontScaleSources) pass.SharedMnWalkthroughTests::walkthroughtimes out under cocoa on this headless machine with or without the new zoom test, and passes underminimal.fontScaleSourcesdrivesOptionsModel::Init()followed by theGuiMain()validation forsettings.json, command-line and config-file values (settings.json -80 starts, clamps to -50 and is rewritten; command-line/config -80 rejected; config 20 accepted and not copied intosettings.json; command line over settings.json leaves the file alone). Mutation checks: dropping the write-back, the settings.json source check, the command-line override check, or the range check insetFontScaleFromArg()each make it fail. Removing the zoom-in or reset key bindings, or the console'sCtrl++binding, makesAppTestsfail. Without the-nosettingsguard inadjustFontScale(),test_dash-qtaborts with thatstd::logic_error.dash-qt -regtestwithsettings.jsonholding"font-scale": "-80"starts and the file is rewritten to"-50".NSMenukey equivalents. The embedded-console (-disablewallet) layout is also not covered byAppTests, which runs with a wallet.Breaking Changes
-font-scalevalues below -50 on the command line or in the config file (previously accepted down to -100) are now rejected at startup. Covered in the release notes.Checklist:
🤖 Generated with Claude Code