Repository navigation
Show build-log and provider text in tooltips without printf formatting - #353
Merged
Merged
Conversation
…atting [patch] ImGui.SetTooltip(string) passes its argument to native igSetTooltip as a printf format with no arguments, so an error line containing %s or a %VAR% batch reference made vsnprintf read missing arguments: garbage text, or a native access violation that kills the process on hover. Both call sites that pass untrusted text (the Errors cell and the provider status bar) now go through Tooltip.Show, which draws the text with BeginTooltip/TextUnformatted/EndTooltip. Fixes #348 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0118yDY82foMMXSZ6XaQmc4c
The quality gate failed on coverage of new code: the two changed call sites in BuildMonitor.cs were never executed by a test. The errors cell renderer is now internal, and the per-provider part of the status bar is extracted into RenderProviderStatus, so the tests can render each in a headless ImGui frame with the mouse over it. Each test checks that the tooltip opens and that a "%%" message draws every character. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0118yDY82foMMXSZ6XaQmc4c
Moving the loop body into RenderProviderStatus re-indented lines the diff then counted as new, and the remaining loop in RenderProviderStatusBar was never run by a test, so coverage on new code fell. The extraction is undone; RenderProviderStatusBar is internal, and the test registers a provider on AppData and renders the bar itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0118yDY82foMMXSZ6XaQmc4c
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #348
Before: the Errors cell tooltip and the provider status-bar tooltip passed their text to
ImGui.SetTooltip(string). Hexa forwards that string to nativeigSetTooltipas a printf format with no arguments. Build-log errors often contain%s(GCC-Werror=format) or%VAR%(batch errors). Hovering one madevsnprintfread arguments that were never passed: at best garbage text, at worst a native access violation that kills the app.After: both call sites go through a new
Tooltip.Show(string), which draws the text withBeginTooltip/TextUnformatted/EndTooltip. The rest of the app already draws text this way. NoImGui.SetTooltipcall is left in the repository.RenderErrorsColumnandRenderProviderStatusBarare nowinternalinstead ofprivate, so tests can render them.Tests
TooltipTestsrender in a headless ImGui context and compare vertex counts. Each visible glyph is one quad, so two strings of the same length with no spaces draw the same vertex count only when every character of both is drawn. A format pass collapses%%to%and drops a glyph.Tooltip.Show:ab%%draws as many vertices asabcd, and'%%SIGNTOOL%%'as many as'xxSIGNTOOLxx'.ab%%draws as many vertices asabcd.AppDataand puts the mouse over its status text. The tooltip opens, and a status message containingab%%draws as many vertices as one containingabcd.The tests don't use a bare
%s. On the old code path it reads a missing argument and can crash the test host, which is the crash being fixed, and it can't be asserted on deterministically.Each fix was checked by reverting it:
Tooltip.ShowcallingImGui.SetTooltip, the twoTooltip.Showtests fail.ImGui.SetTooltipdirectly, its hover test fails.Full suite: 94/94 passing locally on Linux.
Not in this PR: the triage comment suggests checking the org's other ImGui apps for the same pattern. That is left for separate issues in those repositories.
🤖 Generated with Claude Code
https://claude.ai/code/session_0118yDY82foMMXSZ6XaQmc4c