Skip to content

Commit e1b04c3

Browse files
committed
Fix the reserved-to-legacy fallback during paging on a console-less terminal
The fallback abandoned a reservation by building the legacy stdout proxy on the display's loop through call_soon_threadsafe. That callback inherits the scheduling thread's context, which need not have entered the display's app session, so the proxy resolved its output by creating a new one -- and on a Windows CI runner with no real console that raises NoConsoleScreenBufferError, failing the pager-abandonment test there. The proxy is now built with the display's captured session active, whatever context the callback runs in. Two further problems the Windows failure exposed in that same path. The fallback installed the proxy and reset the renderer before checking whether the pager was open, so on a terminal where the pager was up it ran run_in_terminal under the alternate screen and could quit it. And it keyed "pager open" off the renderer's full_screen flag, which is not reliably set at the moment the loop callback runs. The fallback now defers its whole routing-and-layout switch while a deterministic paging flag is set -- showing only the unhidden native toolbar on the pager's bottom row -- and finishes the deferred switch when the pager closes, back on the main screen, so later command output keeps the toolbar. Tests: a focused test builds the proxy with no ambient session and checks it still uses the display's session output; the pager-abandonment test now checks the alternate screen is never quit while paging and that routing is restored after the pager closes. Validation: 2627 passed, 6 skipped with coverage, twice; the mutations that drop the session wrap, the paging defer and the deferred finish each fail a test; harness acceptance and dynamic gates PASS at 12, 24 and 40 rows, 23/23 observer controls. make check, make test and make docs-test passed.
1 parent 690c38e commit e1b04c3

3 files changed

Lines changed: 71 additions & 8 deletions

File tree

‎cmd2/command_toolbar.py‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from typing import TYPE_CHECKING, Any, TextIO, TypeVar, cast
1515

1616
from prompt_toolkit.application import Application, create_app_session
17+
from prompt_toolkit.application.current import _current_app_session, get_app_session
1718
from prompt_toolkit.enums import EditingMode
1819
from prompt_toolkit.filters import Condition, to_filter
1920
from prompt_toolkit.input.typeahead import get_typeahead, store_typeahead
@@ -204,6 +205,15 @@ def __init__(self, cmd: "Cmd") -> None:
204205
self._serialized = False
205206
self._lock = threading.RLock()
206207
self._pausing = False
208+
# The app session the display runs in, captured once it is entered. Output routed for
209+
# the legacy fallback is built against it, so a fallback scheduled onto the loop from a
210+
# context that never entered the session does not fall back to creating a new output --
211+
# which, on a Windows runner with no real console, raises.
212+
self._app_session: Any = None
213+
# Whether the built-in pager is on screen, and whether a reservation abandoned while it
214+
# was owe a legacy fallback once it closes. Both are read on the display's loop.
215+
self._paging = False
216+
self._legacy_fallback_pending = False
207217

208218
session = cmd.main_session
209219
self.app = session.app
@@ -308,6 +318,8 @@ def start(self) -> None:
308318
reserved.stopped_handler = self._reservation_stopped
309319
stack.callback(setattr, reserved, "stopped_handler", previous_handler)
310320
stack.enter_context(create_app_session(input=self.app.input, output=self.app.output))
321+
self._app_session = get_app_session()
322+
stack.callback(setattr, self, "_app_session", None)
311323
# Only replace terminal streams. In particular, preserve redirected stderr
312324
# and self.stdout when a nested command has redirected its output to a file.
313325
for obj, name in ((self.cmd, "stdout"), (sys, "stdout"), (sys, "stderr")):
@@ -389,7 +401,17 @@ def _install_legacy_proxy(self) -> None:
389401
"""Route output through the native toolbar's erase-and-redraw proxy."""
390402
# The worker already combines queued writes. A batching sleep would also
391403
# delay close(), which runs at each command finalization boundary.
392-
proxy = _ContextStdoutProxy(raw=True, sleep_between_writes=0)
404+
#
405+
# Built with the display's session active. The proxy resolves its output from the
406+
# current app session; a fallback can be scheduled onto the loop from a context that
407+
# never entered that session, where the proxy would otherwise create a fresh output
408+
# and, on a console-less Windows runner, raise.
409+
token = _current_app_session.set(self._app_session) if self._app_session is not None else None
410+
try:
411+
proxy = _ContextStdoutProxy(raw=True, sleep_between_writes=0)
412+
finally:
413+
if token is not None:
414+
_current_app_session.reset(token)
393415
with self._lock:
394416
if self._proxy is not None:
395417
proxy.close()
@@ -420,6 +442,7 @@ def _reservation_stopped(self) -> None:
420442
command. Routing and the redraw of a display that *is* running need its loop.
421443
"""
422444
self._layout = self._legacy_layout()
445+
self._legacy_fallback_pending = True
423446
if self.app.loop is not None and self.app.is_running:
424447
self.app.loop.call_soon_threadsafe(self._restore_legacy_display)
425448

@@ -437,15 +460,18 @@ def _restore_legacy_display(self) -> None:
437460
"""Switch a running display over to legacy routing and layout, on its own loop."""
438461
if self._pausing or not self.app.is_running or self.app.is_done:
439462
return
440-
self._install_legacy_proxy()
441-
self.app.erase_when_done = True
442-
if self.app.full_screen:
443-
# The pager is on screen. Its exit applies the display's layout and resets the
444-
# renderer; doing either here would quit the alternate screen under it and flash
445-
# the command output through. Routing is switched now, and a redraw shows the
446-
# native toolbar on the pager's bottom row; the rest waits for the pager's exit.
463+
if self._paging:
464+
# The pager is on screen. Installing the erase-and-redraw proxy or resetting the
465+
# renderer now would run run_in_terminal under the pager and quit its alternate
466+
# screen, flashing the command output through. The native toolbar is already
467+
# unhidden, so a redraw shows it on the pager's bottom row; the rest waits for the
468+
# pager's exit, which finishes the fallback with the terminal back on the main
469+
# screen.
447470
self.app.invalidate()
448471
return
472+
self._legacy_fallback_pending = False
473+
self._install_legacy_proxy()
474+
self.app.erase_when_done = True
449475
self._apply_display_layout()
450476
self.app.renderer.reset()
451477
self.app.renderer.request_absolute_cursor_position()
@@ -717,13 +743,15 @@ def restore() -> None:
717743
"""
718744
nonlocal restored
719745
restored = True
746+
self._paging = False
720747
self._apply_display_layout()
721748
self.app.key_bindings, self.app.editing_mode, self.app.full_screen = previous
722749
self.app.renderer.full_screen = self.app.full_screen
723750

724751
def enter() -> None:
725752
nonlocal entered
726753
entered = True
754+
self._paging = True
727755
# The pager has a full screen of its own to draw, so the render suppression that
728756
# keeps ordinary command frames from touching the terminal has to come off for the
729757
# duration -- otherwise the pager swaps in its layout and nothing is ever painted.
@@ -771,6 +799,10 @@ def close() -> None:
771799
# the full-screen flag and editing mode would carry into the next prompt.
772800
if not restored:
773801
restore()
802+
# A reservation abandoned while the pager was open deferred its legacy fallback until
803+
# the pager closed; now that it has, on the main screen, finish it.
804+
if self._legacy_fallback_pending and self.thread_is_alive:
805+
self._call_in_ui(self._restore_legacy_display)
774806

775807
@contextlib.contextmanager
776808
def suspend(self) -> Iterator[None]:

‎tests/test_command_toolbar.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,6 +1165,29 @@ def test_installing_the_legacy_proxy_twice_keeps_the_first(toolbar_app) -> None:
11651165
assert all(stream.proxy is proxy for stream in display._streams)
11661166

11671167

1168+
def test_legacy_proxy_uses_the_display_session_output_without_an_ambient_session(toolbar_app) -> None:
1169+
"""The fallback can be scheduled onto the loop from a context that never entered the
1170+
display's app session. The proxy must still resolve its output from that session -- not
1171+
create a fresh one, which on a console-less platform raises -- so it is built with the
1172+
session active regardless of the caller's context."""
1173+
from prompt_toolkit.application.current import AppSession, _current_app_session
1174+
1175+
app, _pipe, _output = toolbar_app
1176+
with app._command_toolbar_context():
1177+
display = app._command_toolbar
1178+
assert display is not None
1179+
session_output = display._app_session.output
1180+
display._pause() # drop the proxy so _install_legacy_proxy rebuilds it
1181+
# Stand in a context with no display session, the way an off-loop callback would.
1182+
token = _current_app_session.set(AppSession())
1183+
try:
1184+
display._install_legacy_proxy()
1185+
finally:
1186+
_current_app_session.reset(token)
1187+
assert display._proxy is not None
1188+
assert display._proxy._output is session_output
1189+
1190+
11681191
def test_restoring_the_legacy_display_on_a_stopped_display_changes_nothing(toolbar_app) -> None:
11691192
"""The fallback can be scheduled just before the display stops; by the time it runs
11701193
there is no display to switch, and it must not install routing into a closed one."""

‎tests/test_reserved_terminal.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,14 @@ def stop_mid_page() -> None:
686686
assert seen == {"toolbar": True, "pager": True, "stayed_in_pager": True}
687687
assert reserved.bridge is None
688688
assert len(display.app.layout.container.children) == 3
689+
# The fallback deferred during paging is finished once the pager closes: legacy
690+
# routing is in place -- the proxy installed, the reserved serializers gone -- so
691+
# later command output keeps the toolbar rather than scrolling it away.
692+
assert display._proxy is not None
693+
assert all(stream.serializer is None for stream in display._streams)
694+
harness.app.poutput("after the pager")
695+
assert wait_for(lambda: any("after the pager" in row for row in terminal.screen.display))
696+
assert terminal.screen.display[-1].startswith("STATUS")
689697
harness.app.main_session.bottom_toolbar = "RECOVERED"
690698
display.app.invalidate()
691699
assert wait_for(lambda: terminal.screen.display[-1].startswith("RECOVERED"))

0 commit comments

Comments
 (0)