Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ImGuiIO docs and default values #5540

Merged
merged 2 commits into from
Aug 3, 2022

Conversation

Endilll
Copy link
Contributor

@Endilll Endilll commented Aug 3, 2022

ImGuiIO doc and constructor body seem to be out of sync a bit.
Maybe we should go one step further and eliminate the duplication by providing default initializers directly in struct declaration?

@ocornut
Copy link
Owner

ocornut commented Aug 3, 2022

Thanks for the PR.

On DisplaySize and Fonts, I think the existing comments are more in line with the information we want to convey in that header. A display size needs to be set, and atlas is auto-created on context creation if not provided. So there's only 1 miss-sync of value here, and the two false initializers are not strictly necessary but better to have them for consistency and searchability.

Maybe we should go one step further and eliminate the duplication by providing default initializers directly in struct declaration?

I agree but would need to carefully check with VS version does that limit us to. (see #4537)

@ocornut
Copy link
Owner

ocornut commented Aug 3, 2022

Based on https://docs.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?view=msvc-160

C++11 Everything else | VS 2015
N3653 Default member initializers for aggregates | VS 2017 15.0
P0683R1 Default member initializers for bit-fields | VS 2019 16.5

Based on that I don't think we should use them for aggregate nor bit-fields and I worry this would create quite a bunch of inconsistencies in some structures, but probably good enough for ImGuiIO only.

@ocornut ocornut merged commit c911901 into ocornut:master Aug 3, 2022
kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants