Repository navigation
Keep direct window title errors with their caller - #1793
Merged
tleonhardt merged 1 commit intoOct 29, 2026
Merged
tleonhardt merged 1 commit into
tleonhardt merged 1 commit into
Conversation
KennyMcSimpson
requested review from
kmvanbrunt and
tleonhardt
as code owners
October 29, 2026 04:58
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fix/window-title-flush-v2 #1793 +/- ##
==========================================================
Coverage 99.75% 99.75%
==========================================================
Files 23 23
Lines 6574 6574
==========================================================
Hits 6558 6558
Misses 16 16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
tleonhardt
approved these changes
Oct 29, 2026
tleonhardt
left a comment
tleonhardt
left a comment
Member
There was a problem hiding this comment.
Thanks for catching and fixing this edge case!!
tleonhardt
merged commit Oct 29, 2026
4472a0a
into
python-cmd2:fix/window-title-flush-v2
30 checks passed
tleonhardt
added a commit
that referenced
this pull request
Oct 30, 2026
* Flush window title updates on the prompt event loop * Fix window title routing during select * Explain select session traversal for title updates * Route synchronous title updates through each Cmd output * Write window titles on the running prompt's event loop instead of locking the output Replace the locked output proxy from #1783 with a smaller change confined to set_window_title(). The title is written and flushed on main_session.output. When another thread calls it while a prompt or select() menu is running, the write is scheduled on that prompt's event loop so it doesn't interleave with rendering. A pending title is written when the prompt finishes in case the loop stops before running the callback. This drops the _LockedOutput proxy, the main_session property setter, and the forced per-Cmd output driver, restoring prompt-toolkit's dumb-terminal fallback, the ambient AppSession output when stdout is sys.stdout, and isinstance checks on the renderer's output. * Don't assert when a worker's title becomes visible during a prompt The loop thread can write the title before the worker checks the stream, which happens on free-threaded Python. Only a call on the loop thread is guaranteed to have written it before returning. * Harden window title writes found in review - Read the select() AppSession and its app once in _get_prompt_loop(), since the main thread clears them when select() finishes. - Ignore output errors when a title is written on a prompt's event loop or during prompt and select() cleanup, so they can't disrupt the prompt or replace its result or exception. - Use a reentrant lock so a signal handler which sets the title while one is being written doesn't deadlock. - Write titles to a terminal stdout when stdin isn't a terminal and the main session uses a DummyOutput. - Document that a title set from another thread appears once the prompt's loop runs its next callback. * Fix window title edge cases found in second review - When a signal handler sets the title while this thread is writing one, only record it. The outer call writes pending titles until none remain, so the latest title wins and writes never nest in the same stream. - Ignore ValueError from a closed stream as well as OSError when writing a title quietly. - Write a title left pending by a prompt after patch_stdout() exits, so it can't race with its writer thread. - Create the fallback terminal output the first time a title needs it, from the stdout captured when the app was created. This also covers a main_session replaced later by one with a DummyOutput, and titles set while output is redirected. - Share the list of terminal initialization errors between the main session and the title fallback. * Schedule every title on a running prompt's loop and check stdout lazily - Treat a prompt as running while its application's loop is set, which covers prompt-toolkit's teardown after it clears is_running but can still write to the terminal. - Schedule the title on the prompt's loop even when called on that loop, so a signal handler can't nest a title write inside prompt-toolkit's own flush. - Check whether the fallback stdout is a terminal the first time a title needs it, so constructing a Cmd with a stdout that lacks isatty() or is closed works as it did before. - Describe the stdout fallback in the set_window_title() docstring and the prompt docs. - Run test workers through futures so their exceptions reach the test without a blind except and noqa. * Coalesce queued title callbacks and clarify when titles use the stdout fallback - While a title callback is queued on a prompt's loop, later titles only replace the pending title, so a burst of updates schedules one callback. The callback and prompt cleanup clear the queued loop. - Keep the stdout this app was created with in its own field, and track whether its fallback output has been checked with a flag instead of clearing that field. A cached_property doesn't work here, because inspect.getmembers() evaluates properties while a Cmd is constructed. - Describe all the cases where prompts use a dummy output, and note that a direct write from another thread isn't synchronized with other writers to the terminal. * Give the prompt cleanup test a VT100 output for patch_stdout() On a Windows CI runner the default app session output is a Win32Output, which raises NoConsoleScreenBufferError because there's no console. Run the test inside create_app_session() like the other prompt tests. * Keep synchronous window title errors with their caller (#1793) --------- Co-authored-by: KennyMcSimpson <70093673+KennyMcSimpson@users.noreply.github.com>
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.
When no prompt is running,
set_window_title()releases its lock after storing the pending title but before flushing it. Concurrent prompt cleanup can consume that request and suppress itsOSErrororValueError, so the synchronous caller returns successfully even though its write failed.Keep the existing reentrant lock through the direct flush, including the closed-loop fallback, so the caller retains its request and receives its output error. The existing reentrant-write guard still handles signal handlers.
This is a single-commit follow-up to #1790, targeting
fix/window-title-flush-v2ate72ceb558.The deterministic regression pauses the direct caller before flushing and runs concurrent quiet cleanup. All eight cases fail on the unchanged #1790 implementation and pass with this fix, covering absent/closed loops, both output operations, and both exception types. It also checks which thread receives the failure and that pending/write state is cleared.
Validation on Windows:
uv lock --locked,prek run -a,ty check, andmypy ..zensical build -spassed.