Allow the window title to change, and the close to be cancelled - #341
Merged
Merged
Conversation
An application that edits documents needs two things this framework did not offer: a title that reflects the open document and whether it has unsaved changes, and a chance to prompt before the window goes away. ImGuiAppConfig.Title is init-only and read once when the window is created, so it cannot carry anything that changes. ImGuiApp.SetWindowTitle sets it while the application runs. It marshals through the Invoker so it is safe from any thread, and skips the write when the title is unchanged so a caller can call it every frame from OnRender without churning the window. ImGuiAppConfig.OnClosing is consulted before the window closes and cancels the close by returning false. Cancelling clears IWindow.IsClosing, which the windowing backend re-reads each iteration of the run loop, so this works on every platform rather than only where the native window can be subclassed - unlike HideOnClose, which intercepts WM_CLOSE and is Windows-only. The check runs before any teardown. That ordering is the point: the existing Closing handler frees the pinned font data, the ImGui controller, the input context and the GL context, none of which is reversible, so consulting the callback afterwards would leave a cancelled close rendering into a released context. It fires for every close, including ImGuiApp.Stop, which means a handler that always returns false makes the application impossible to exit. That is documented rather than special-cased: Stop is how an application confirms an exit it previously vetoed, so exempting it would break the flow the callback exists for. Also documented: a modal prompt cannot be drawn from the callback, since it returns before the next frame - set a flag, return false, draw the prompt from OnRender, and call Stop when the user confirms. Ten tests cover both, driving the mocked IWindow: the title changes and is reported back, an unchanged title is not written through, null is rejected, a call before the window exists is ignored and the configured title is reported instead; and a close proceeds with no handler or an allowing one, is cancelled with IsClosing cleared when declined, consults the handler once, and can be declined and then allowed - which is the prompt-then-confirm sequence an editor actually performs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WmzGm9XniSoqaiVGDT6qT
5 tasks
The net10.0-ios build failed with CS1574: the cref to ImGuiApp.SetWindowTitle(string) in ImGuiAppConfig's Title docs could not be resolved. ImGuiAppConfig.cs compiles for iOS but ImGuiApp.cs is excluded from it, so a doc reference to a desktop-only member has nothing to bind to. Fixed where the repository already handles this rather than by weakening the doc comment: Platform/iOS/ImGuiApp.iOS.NoOps.cs exists precisely to keep desktop window-management APIs on the iOS surface with matching signatures, "so cross-platform consumer code compiles unchanged", each logging a one-time warning instead of silently doing nothing. SetWindowTitle belongs in that set alongside Show, Hide and SetWindowIcon - iOS has no window title bar. WindowTitle is added there too, returning Config.Title. It does not warn: the title is never displayed on iOS, but reading back what was configured is an honest answer rather than a no-op. OnClosing needs no iOS gating. It is a Func<bool> with no Silk.NET coupling, and HideOnClose - equally desktop-only in effect - is ungated in the same file, so this matches the convention already there. Only the two members that name Silk.NET types sit behind #if !IOS. net10.0-ios cannot be built here: that target framework is only added when the build host is macOS. Desktop builds clean and all 340 tests still pass; the added code compiles only under #if IOS, so it cannot affect them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WmzGm9XniSoqaiVGDT6qT
SonarCloud failed the quality gate at 68.4% coverage on new code, and "Analyze & Release" failed only as a consequence - the scanner exits non-zero when the gate fails, so both checks had the same single cause. Measuring locally against the changed files showed two genuinely untested regions, both worth a test on their own merits rather than for the metric: The veto branch inside the Closing handler. ShouldClose was covered, but nothing exercised the handler that consults it, which is the part that actually decides whether teardown runs. Two tests now raise Closing through a new TestHelpers.SimulateClosing and assert on currentPinnedFontData: a cancelled close leaves it holding its handle, an allowed close frees it. That is the property that matters - a cancelled close must not release the font atlas, controller, input and GL context and then keep rendering. The dropped-write guard in the queued title update. This is reached only when SetWindowTitle is called off the window thread, since the Invoker runs work inline for its own thread. Staging that race in a test does not work: owning the Invoker from another thread makes Invoke block the caller until that thread pumps, leaving no window in which to null the field - the first attempt deadlocked. The applier is now internal and called directly under the same precondition, which tests the same code deterministically and says plainly what the guard is for. Its remarks record why the check is not redundant with the one in SetWindowTitle. Every new executable line in ImGuiApp.cs and ImGuiAppConfig.cs is now covered, verified with a cobertura run mapped onto the diff. 343 tests pass, up from 340. The two lines added to the iOS no-op surface stay uncovered and cannot be otherwise: that file compiles only for net10.0-ios, which is built only on a macOS host, so no test on the coverage platforms can execute it. Dropping the cref instead of adding those members would have avoided them, but would leave cross-platform code calling SetWindowTitle failing to compile on iOS - which is the exact hole that file exists to close. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WmzGm9XniSoqaiVGDT6qT
|
This was referenced Aug 28, 2026
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
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.



Two small additions to
ImGui.App, both needed by any application that edits documents. They came up building the Schema editor (ktsu-dev/Schema#116), where two acceptance criteria couldn't be met against 3.14.1 — but nothing about them is specific to that app.Three commits, each self-contained: the feature, an iOS build fix, and the tests that closed a coverage gap.
ImGuiApp.SetWindowTitle(string)ImGuiAppConfig.Titleisinit-only and read once when the window is created, so it can't carry anything that changes — the open document's name, or a dirty marker.SetWindowTitlesets it while the application runs.Invoker, so it's safe from any thread.OnRenderrather than requiring the caller to track the last value.ImGuiApp.WindowTitlereads it back, falling back toConfig.Titlebefore the window exists.ImGuiAppConfig.OnClosingA
Func<bool>consulted before the window closes; returningfalsecancels it.Cancelling clears
IWindow.IsClosing, which the windowing backend re-reads each iteration of the run loop — so this works on every platform, unlikeHideOnClose, which interceptsWM_CLOSEby subclassing the native window and is Windows-only.Three things worth a reviewer's attention:
The check runs before any teardown, and that ordering is the point. The existing
Closinghandler frees the pinned font data, the ImGui controller, the input context and the GL context. None of that is reversible, so consulting the callback afterwards would leave a cancelled close rendering into a released context.It fires for every close, including
ImGuiApp.Stop. So a handler that unconditionally returnsfalsemakes the application impossible to exit. I documented that rather than special-casingStop, becauseStopis precisely how an application confirms an exit it previously vetoed — exempting it would break the flow the callback exists for.A modal prompt can't be drawn from the callback, since it returns before the next frame is rendered. The documented pattern is: set a flag, return
false, draw the prompt fromOnRender, callStopwhen the user confirms. Worth stating explicitly — it's the first thing someone will try.When
HideOnCloseis also set, the close button is intercepted beforeClosingis reached and the window is hidden instead, soOnClosingisn't consulted on that path. Also documented.iOS
The first push failed
Build net10.0-ios (macOS)withCS1574: the<see cref>toSetWindowTitleinImGuiAppConfig's docs couldn't resolve, becauseImGuiAppConfig.cscompiles for iOS whileImGuiApp.csis excluded from that target.Fixed where the repo already handles this rather than by weakening the doc comment.
Platform/iOS/ImGuiApp.iOS.NoOps.csexists to keep desktop window APIs on the iOS surface with matching signatures — its own header says "so cross-platform consumer code compiles unchanged" — each logging a one-time warning.SetWindowTitlenow sits there alongsideShow,HideandSetWindowIcon; iOS has no title bar.WindowTitlegoes with it, returningConfig.Titlewithout warning, since reading back what was configured is an honest answer rather than a no-op.OnClosingneeds no iOS gating: it's aFunc<bool>with no Silk.NET coupling, andHideOnClose— just as desktop-only in effect — is ungated in the same file. Only the two members naming Silk.NET types sit behind#if !IOS.Verification
343 tests pass (330 before), and every new executable line in
ImGuiApp.csandImGuiAppConfig.csis covered — verified locally with a cobertura run mapped onto the diff, and SonarCloud reports 100% coverage on new code.The 13 new tests drive the mocked
IWindow:WindowTitlenullis rejectedfalseand clearsIsClosing— the two halves of the cancelcurrentPinnedFontData, which theClosinghandler clearsTwo notes on how those last ones are written, since both involved a false start:
The teardown pair needed a way to raise
Closingalone;SimulateWindowLifecycleruns the whole lifecycle and unregisters the handlers.TestHelpers.SimulateClosingdoes just the one event. (NamedSimulate…to match the existing helper —Raise…trips CA1030.)The dropped-write guard is reached only off the window thread, since the
Invokerruns work inline for its own thread. Staging that race in a test deadlocks: owning theInvokerfrom another thread makesInvokeblock the caller until that thread pumps, leaving no window in which to null the field.ApplyWindowTitleis thereforeinternaland called directly under the same precondition — same code, deterministic, and clearer about what the guard is for. Its remarks record why the check isn't redundant with the one inSetWindowTitle.TestHelpers.CreateMockWindowgains aSetupPropertyforTitle; it already had one forIsClosing.No demo change: this is application-lifecycle API rather than a widget, and I kept the diff minimal.
🤖 Generated with Claude Code
https://claude.ai/code/session_012WmzGm9XniSoqaiVGDT6qT