Skip to content

Fixes usage of mixed enum flags.#693

Closed
dkosmari wants to merge 1 commit intoepezent:masterfrom
dkosmari:fix-mixed-enums
Closed

Fixes usage of mixed enum flags.#693
dkosmari wants to merge 1 commit intoepezent:masterfrom
dkosmari:fix-mixed-enums

Conversation

@dkosmari
Copy link
Copy Markdown
Contributor

@dkosmari dkosmari commented Apr 3, 2026

  • Undo warning suppression introduced in 9c34c33.
  • ImGui's enums are static_cast to eliminate the warning.
  • ImPlot's enums are extended with _NoLegend and _NoFit so there's never a need to mix them.

- Undo warning suppression introduced in 9c34c33
- ImGui's enums are `static_cast` to eliminate the warning.
- ImPlot's enums are extended with `_NoLegend` and `_NoFit` so there's never a need to mix them.
@brenocq
Copy link
Copy Markdown
Collaborator

brenocq commented Apr 4, 2026

I'm planning to keep the warning suppression since this was the solution implemented in ImGui, when ImGui implements a different solution I'm more than happy to implement the same here.

I can see the benefit of extending ImPlot's enums with _NoLegend/_NoFit, but this makes it a bit harder to maintain when we add new item flags. For now I'll keep as is since the suppression was the ImGui solution.

@brenocq brenocq closed this Apr 4, 2026
@dkosmari dkosmari deleted the fix-mixed-enums branch April 4, 2026 10:20
@dkosmari
Copy link
Copy Markdown
Contributor Author

dkosmari commented Apr 4, 2026

You have a _None flag repeated for every single one of these enums.

You already created maintenance hell because adding a new "generic" enum requires making sure it doesn't clash with any of the other "specialized" flags.

And in case somebody forgets to add the _NoLegend and _NoFit flags to a brand new enum, nothing will break, the user can still use the generic version (with an explicit cast if compiling with warnings.)

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.

2 participants