Render the built-in pager in reserved toolbar mode

The reserved-mode command display suppresses its renderer frames, since it
has nothing of its own to draw and a frame would scroll command output off
the screen. But the built-in pager borrows that same display and does have
a full screen to draw, so suppression left it painting nothing while its
keys still worked -- `cat` in the example app ran, showed no output, and
still quit on "q".

The pager now lifts render suppression while it is on screen and restores
it on the way out, so ordinary command output stays suppressed and the
pager renders for real. The reserved-mode pager path had no test -- the
existing pager tests run with the toolbar off -- so this adds one that
drives the pager over a reservation and one for output that fits without
paging.

Validation: 2599 passed, 6 skipped with coverage; the mutation that drops
the un-suppression fails the new test; make check, make test and
make docs-test passed.

Author: tleonhardt

CHANGELOG.md

@@ -13,6 +13,9 @@
     - Command output that does not end in a newline (for example a progress line updated with a
       carriage return) is no longer erased by a reserved toolbar's redraw. The line in progress is
       preserved and the next write continues it.
+    - The built-in pager (used by `Cmd.ppaged()`) again displays its content in reserved toolbar
+      mode. It had rendered nothing while still accepting its keys, because the command display's
+      renderer frames were being suppressed.
 
 - Breaking Changes
     - Replaced `enable_bottom_toolbar` with `bottom_toolbar_mode` in `Cmd.__init__()`. The default,

cmd2/command_toolbar.py

@@ -395,18 +395,19 @@ def _install_serializers(self) -> bool:
         # While the command display owns the terminal it has nothing of its own to draw, so its
         # renderer frames are suppressed: emitting one would reserve the usable height and
         # scroll command output off the screen. The toolbar is painted independently.
-        if reserved.bridge is not None:
-            reserved.bridge.set_render_suppressed(True)
+        self._set_render_suppressed(True)
         return True
 
-    def _resume_rendering(self) -> None:
-        """Let the renderer emit its frames again, now the command display has given up the terminal.
+    def _set_render_suppressed(self, suppressed: bool) -> None:
+        """Turn the renderer's own frames off while the command display owns the terminal.
 
-        The next frames belong to the main prompt, which must render for real.
+        Off for ordinary command output, which the empty command display must not repaint; on
+        again for the pager, whose full screen must render, and for the main prompt once the
+        display has given the terminal back.
         """
         bridge = self._reserved_bridge()
         if bridge is not None:
-            bridge.set_render_suppressed(False)
+            bridge.set_render_suppressed(suppressed)
 
     def _app_exited(self) -> None:
         """Give the terminal back to the streams when the display stops on its own.
@@ -420,7 +421,7 @@ def _app_exited(self) -> None:
             # A deliberate pause restores the streams itself, in the right order.
             return
 
-        self._resume_rendering()
+        self._set_render_suppressed(False)
         with self._lock:
             # Leave self._proxy set so that the next _pause() still drains and closes
             # it. With the display gone, its worker writes to the terminal directly.
@@ -450,7 +451,7 @@ def _exit(self) -> None:
 
     def _pause(self) -> None:
         self._pausing = True
-        self._resume_rendering()
+        self._set_render_suppressed(False)
         try:
             try:
                 # Hold off other threads while the proxy drains so their output is never
@@ -622,6 +623,10 @@ def page(self, text: str, *, chop: bool) -> None:
         def enter() -> None:
             nonlocal entered
             entered = True
+            # The pager has a full screen of its own to draw, so the render suppression that
+            # keeps ordinary command frames from touching the terminal has to come off for the
+            # duration -- otherwise the pager swaps in its layout and nothing is ever painted.
+            self._set_render_suppressed(False)
             self.app.renderer.erase()
             self.app.layout = layout
             self.app.key_bindings = pager.bindings
@@ -638,6 +643,9 @@ def leave() -> None:
             self.app.layout, self.app.key_bindings, self.app.editing_mode, self.app.full_screen = previous
             self.app.renderer.full_screen = self.app.full_screen
             self.app.renderer.request_absolute_cursor_position()
+            # Back to ordinary command output, whose frames are suppressed again so the toolbar
+            # stays put. Command finalization and the next prompt lift this in turn.
+            self._set_render_suppressed(True)
             self.app.invalidate()
 
         def close() -> None:

tests/test_reserved_terminal.py

@@ -4,6 +4,7 @@
 import sys
 import threading
 import time
+from concurrent.futures import ThreadPoolExecutor
 from types import SimpleNamespace
 
 import pyte
@@ -330,3 +331,42 @@ def test_a_carriage_return_progress_line_ends_on_its_final_value(self, terminal_
             harness.app.stdout.flush()
             assert terminal.screen.display[0].startswith("Progress: 100%")
             assert terminal.screen.display[-1].startswith("STATUS")
+
+
+class TestPager:
+    """The built-in pager renders a full screen of its own, so its frames must not be
+    suppressed the way an ordinary command's empty frames are."""
+
+    def test_the_pager_draws_its_content_over_the_reserved_toolbar(self, terminal_harness) -> None:
+        harness, terminal = terminal_harness
+        with harness.app._reserved_toolbar_context(), harness.app._command_toolbar_context():
+            display = harness.app._command_toolbar
+            body = "\n".join(f"row {index:03d}" for index in range(200))
+            shown = threading.Event()
+
+            def drive() -> None:
+                # Wait until the pager has painted its first screen, then quit it. Quit either
+                # way, so a pager that never draws fails the assertion instead of hanging the
+                # blocking page() call forever.
+                try:
+                    if wait_for(lambda: terminal.screen.display[0].startswith("row 000")):
+                        shown.set()
+                finally:
+                    harness.pipe.send_text("q")
+
+            with ThreadPoolExecutor() as executor:
+                future = executor.submit(drive)
+                display.page(body, chop=False)
+                future.result(timeout=5)
+
+            assert shown.is_set(), "the pager never drew its content"
+            # The toolbar is suppressed again for ordinary output once the pager has closed.
+            assert harness.app.reserved_toolbar.bridge._render_suppressed is True
+        assert terminal.screen.margins is None
+
+    def test_output_that_fits_is_printed_without_a_pager(self, terminal_harness) -> None:
+        harness, terminal = terminal_harness
+        with harness.app._reserved_toolbar_context(), harness.app._command_toolbar_context():
+            harness.app._command_toolbar.page("one short line", chop=False)
+            assert any(row.startswith("one short line") for row in terminal.screen.display)
+            assert terminal.screen.display[-1].startswith("STATUS")