Repository navigation
Flush window title updates without waiting for a prompt - #1783
KennyMcSimpson wants to merge 8 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1783 +/- ##
=======================================
Coverage 99.75% 99.75%
=======================================
Files 23 23
Lines 6503 6536 +33
=======================================
+ Hits 6487 6520 +33
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. |
06f325a to
2997f51
Compare
|
@KennyMcSimpson Thanks for the PR! I left a couple comments for things I think likely need to be tweaked. I also just put out a 4.3.1 patch release, so you will need to pull latest and merge |
# Conflicts: # CHANGELOG.md
|
Thanks for the review. Commit
Validation after the follow-up: independent public-API reference 55/55 (previous implementation 31/55), Python 3.13 suite 1,879 passed / 159 skipped, Python 3.11 suite 1,878 passed / 160 skipped, Hosted follow-up: 29 checks pass, including quality, type checks, docs, CodeQL, coverage, and 17 of 18 OS/Python test jobs. The macOS/Python 3.15t job times out in the unchanged |
|
@KennyMcSimpson I took the liberty of merging changes from the |
| :param title: the new window title | ||
| """ | ||
| set_title(title) | ||
| output = self.main_session.output |
There was a problem hiding this comment.
I'm thinking there is a much simpler fix for this bug from an overall design perspective. The bug is that title output is buffered and never flushed.
If we write and flush on self.main_session.output, handing the write to app.loop while the app is running, or use a lock that only set_window_title uses I think that would avoid a lot of the edge cases that exist with this current approach.
There are a lot more edge cases with this current approach I could point out, but I think a simpler approach will eliminate essentially all of them.
|
Thank you very much for this contribution, @KennyMcSimpson! Your investigation and tests made the We're continuing this work in #1790. That branch merges your commits unchanged, so you'll keep full You're very welcome to review #1790 and share any feedback. Thanks again! |
* 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>
set_window_title()now flushes a title before returning and always targets the terminal owned by the callingCmdinstance. For example, a worker callingapp.set_window_title("Working…")updates the same output before, during, and after that app's main prompt, custom input prompts, orselect()menus.Each
Cmdcreates its own output driver. A private output proxy delegates terminal behavior and serializes output methods with one reentrant lock; the title write and flush hold that same lock together. Replacingmain_sessionwith a customPromptSessionalso binds the proxy to both the application and its renderer. This protects the VT100 render buffer during prompt setup and teardown and keeps errors synchronous in the calling thread.The process-wide session registry and event-loop callbacks have been removed.
set_window_title()is now an instance method; the 4.4.0 breaking-change entry documents migration toapp.set_window_title(title)orself.set_window_title(title). The prompt guide has been updated, and the Textual example no longer needs a manual flush.Validation:
f12523d4040b5afe384f5362035a7c47c1837f8d.main_sessionassignment. The native Win32 title backend is also exercised.make checkequivalents pass: locked dependency check, all prek hooks including Ruff, ty, and strict mypy.make docs-testequivalent (zensical build -s) reports no issues.AI assistance was used to investigate, implement, and validate this pull request.
Closes #1782.