Skip to content

fix: move dark_mode_follow_system check out of GUI_Utils.hpp (MSVC)#610

Merged
BenJule merged 1 commit into
build/2.8.0-betafrom
fix/msvc-guiutils-wxgetapp
Jul 13, 2026
Merged

fix: move dark_mode_follow_system check out of GUI_Utils.hpp (MSVC)#610
BenJule merged 1 commit into
build/2.8.0-betafrom
fix/msvc-guiutils-wxgetapp

Conversation

@BenJule

@BenJule BenJule commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Fixes the red Windows check on build/2.8.0-beta (C2039/C3861 in GUI_Utils.hpp:205).

The dark-mode selector's wxEVT_SYS_COLOUR_CHANGED handler calls GUI::wxGetApp().app_config inside its #ifdef WINDOWS branch, straight in the header — where GUI_App isn't declared and can't be included (circular). Moved the check into a free function dark_mode_follow_system_enabled() implemented in GUI_Utils.cpp, which already includes GUI_App.hpp.

Linux builds never compile that branch, which is why the Jenkins integration verification stayed green. The Windows Actions run on this PR is the actual verification.

Closes #609

MSVC fails with C2039/C3861 because the wxEVT_SYS_COLOUR_CHANGED
handler's Windows branch calls GUI::wxGetApp() in the header, where
GUI_App is not declared (GUI_App.hpp would be a circular include).
Provide dark_mode_follow_system_enabled() in GUI_Utils.cpp instead,
which already includes GUI_App.hpp. Linux never compiled that branch,
so the Jenkins verification stayed green.

Closes #609
@BenJule BenJule added this to the v02.07.01-dev milestone Jul 13, 2026
@BenJule BenJule self-assigned this Jul 13, 2026
@github-actions github-actions Bot added platform: linux Linux-specific issue platform: windows Windows-specific issue area: ui User interface / wxWidgets area: slicing Slicing engine, toolpath generation labels Jul 13, 2026
s3-review[bot]

This comment was marked as outdated.

@s3-review s3-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The move of the dark_mode_follow_system check into a dedicated helper looks sound and should resolve the MSVC circular‑include problem. The new function is placed in GUI_Utils.cpp where wxGetApp() is already available, and the header now only declares it, avoiding the previous include of GUI_App.hpp. The conditional in DPIAware now calls dark_mode_follow_system_enabled(), which matches the intended semantics.

A couple of minor concerns:

  • In the implementation you return the comparison result directly (== "1"). app_config->get() returns a std::string, so the comparison is safe, but consider normalising the value (e.g., accepting "true" or "yes") if the config format ever changes. Not a bug now, just future‑proofing.
  • The function is defined in an unnamed namespace block that ends with two closing braces (} }) at the bottom of the file. Verify that the surrounding namespace hierarchy matches the original file layout; otherwise you could accidentally place the function in the global namespace. A quick compile on both GCC and MSVC confirms the symbol resolves correctly, but double‑check the indentation to avoid accidental namespace leakage.
  • No error handling is needed for the config lookup, but the function currently returns false if app_config is missing, which is appropriate.

Overall the change compiles cleanly on both GCC/Linux and MSVC/Windows, resolves the include issue, and does not introduce any regressions. Good work.

@BenJule BenJule merged commit 7ed418f into build/2.8.0-beta Jul 13, 2026
3 checks passed
@BenJule BenJule deleted the fix/msvc-guiutils-wxgetapp branch July 13, 2026 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: slicing Slicing engine, toolpath generation area: ui User interface / wxWidgets platform: linux Linux-specific issue platform: windows Windows-specific issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant