Repository navigation
Flush window title updates without waiting for a prompt - #1783
KennyMcSimpson wants to merge 4 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 6474 6506 +32
=======================================
+ Hits 6458 6490 +32
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. |
| output.set_title(title) | ||
| output.flush() | ||
|
|
||
| app = get_app_or_none() |
There was a problem hiding this comment.
select() still has the race the PR is meant to fix.
A plain background thread gets prompt-toolkit's shared default session, so it can see the main prompt while it's running. But select() runs choice() inside a new session created at cmd2/cmd2.py:4793, and only the main thread can see that one. So during a menu, a thread like the alerter in examples/async_printing.py gets no app back and writes directly to the same output object the menu is drawing with, from the wrong thread. choice() takes no input/output arguments, so that wrapper can't simply be removed.
One fix is for cmd2 to keep track of the app or loop it is running, instead of relying on the calling thread's context. A different fix would be for select() to handle the title itself.
| :param title: the new window title | ||
| """ | ||
| set_title(title) | ||
| output = get_app_session().output |
There was a problem hiding this comment.
The title is written to a different output from the app it's scheduled on.
But this isn't new: the old set_title() also wrote to the default session's output.
Now that the code has the running app, writing to app.output when one exists, and to get_app_session().output otherwise, is a one-line fix. It sends the title to the right terminal when stdout isn't sys.stdout.
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 |
| """ | ||
| set_title(title) | ||
| session = get_app_session() | ||
| with _select_app_sessions_lock: |
There was a problem hiding this comment.
Please comment this while loop to explain why you're doing this traversal.
There was a problem hiding this comment.
Added in c78994f8. The comment explains that a worker may still see the parent session, so the loop follows registered children to the innermost active select() session; [-1] selects the most recently registered active child when menus share a parent. make check, make test (1,879 passed / 159 skipped), and make docs-test pass locally.
set_window_title()now flushes terminal title updates immediately and keeps them on the output owned by the active prompt-toolkit application.What changed
Cmd.select()now records its temporary childAppSessionwhilechoice()is active.set_window_title()follows the parent-to-child route so an ordinary background thread reaches the menu session even though it does not inherit the child context.finally, supports nested menus and out-of-order completion, and never holds its lock while writing output or scheduling callbacks.app.output; before an application starts, the routed session output is used. Unrelated custom sessions remain isolated.Validation
mainate9c64db568404a6ec384f156656768d77a026c04; the 4.3.1 release section is preserved and this fix is listed under 4.3.2.2997f51), an independent reference using the publicCmd.select()andCmd.set_window_title()APIs reports 24 failures in 55 checks. Commite49ec13breports 0 failures in all 55 checks.app.outputdiffers from the ambient session output. Repository regressions also cover nested menus and out-of-order menu completion.make check: all hooks, Ruff, ty, and mypy pass.make teston Python 3.13: 1,879 passed / 159 skipped. Python 3.11: 1,878 passed / 160 skipped.make docs-test: no issues. Focused title/select tests: 27 passed; the completetests/test_cmd2.pymodule: 351 passed / 34 skipped.The current hosted head
c78994f8has 29 successful checks, including quality, type checks, docs, CodeQL, coverage, and 17 of 18 OS/Python test jobs. The macOS/Python 3.15t job again fails in the unchangedtest_pipeline_stops_with_cmd2_and_returns_terminal[wrapper-stop-and-interrupt-busy-shell-producer]: it times out attests/test_pipeline_job_control.py:335after a job-control signal, with 2,035 tests passed, 1 failed, and 2 skipped. The aggregate test check consequently fails as well. That case exercises a shell pipeline and does not callselect()orset_window_title(), and the test file matches upstreammain. The precise cause has not been established in a controlled macOS reproduction. GitHub denied the earlier attempted failed-job rerun with 403 because this account lacks the required upstream repository permissions.Commit
c78994f8adds the requested traversal comment: workers may still see the parent session, so registered children are followed to the innermost activeselect()session, with the latest active child selected when menus share a parent. This follow-up changes comments only.make check,make test(Python 3.13: 1,879 passed / 159 skipped), andmake docs-testpass locally.AI assistance was used to investigate, implement, and validate this pull request.
Closes #1782.