Activity

1. tleonhardt

   self-assigned this

   on Sep 5, 2026
2. tleonhardt

   added

   enhancement

   on Sep 5, 2026
3. tleonhardt commented on Sep 5, 2026

   

   tleonhardt

   on Sep 5, 2026

   Last edited by tleonhardt

   MemberAuthor

   More actions

   Summary

   I have two different architectural approaches for solving this problem currently published as two separate PRs:

   1. Persistent toolbar: Persistent bottom toolbar during command execution #1744
   2. Consolidated toolbar: Consolidated toolbar during command execution #1745

   There are differences in approach that impact user experience, and developer experience. I don't think one of them is obviously the right choice. So I would like feedback from other developers on what they like or don't like about each approach.

   First, a framing correction

   These aren't two independent solutions. persistent-bottom-bar's tip (6cb419d9) is a git ancestor of consolidated_toolbar — the entire PBB implementation is present verbatim in the consolidated branch. The real difference is one commit, aac8a4e7, plus two merge-backs of PBB fixes.

   main ──► 76dcc784 ─► 1c491b1e ─► 17026549 ─► 6cb419d9   ← persistent-bottom-bar
                                                     │
                                                     └─► aac8a4e7 ─► merges  ← consolidated_toolbar
   

   So ~90% of the design is shared, and the comparison is really about the delta.

   The shared foundation (both branches)

   cmd2/command_toolbar.py — command execution stays on the main thread; a prompt-toolkit UI runs on a background thread (cmd2-toolbar) so the toolbar keeps refreshing. Around that:

   • ToolbarStream wrappers give cmd.stdout / sys.stdout / sys.stderr stable identities across suspensions and cmd2 redirection, swapping the underlying sink between a StdoutProxy (prints above the bar) and the raw terminal, under an RLock.
   • _ContextStdoutProxy overrides _start_write_thread so the flush worker inherits the toolbar's create_app_session contextvar.
   • _ToolbarBuffer incremental-decodes subprocess bytes so a split UTF-8 sequence doesn't corrupt output.
   • Input coordination: the UI owns the keyboard (needed for CPR), stashes typed keys and re-injects them as typeahead at the next prompt; Ctrl-C is forwarded via killpg/interrupt_main; Ctrl-Z suspends.
   • suspend_toolbar decorator + suspend_bottom_toolbar() context manager step aside for nested prompts, select(), do_shell, _run_python, do_ipy, and pipe processes (pipe_target() + RedirectionSavedState.toolbar_suspension).

   Where they diverge

   persistent-bottom-bar — two applications

   CommandToolbar.__init__ constructs a second, private Application with its own hand-built layout:

   self.app: Application[None] = Application(
       layout=Layout(HSplit([Window(height=0), Window(),
           ConditionalContainer(
               Window(FormattedTextControl(lambda: session.bottom_toolbar, ...),
                      style="class:bottom-toolbar", height=1, always_hide_cursor=True),
               filter=~is_done & renderer_height_is_known)])),
       input=session.input, output=session.output, style=session.style,
       color_depth=session.color_depth, refresh_interval=session.refresh_interval, ...)

   Paging stays external: ppaged is decorated with @suspend_toolbar and shells out to less/more, hiding the bar for the duration.

   consolidated_toolbar — one application, interchangeable layouts

   CommandToolbar borrows session.app and swaps its attributes for the duration of a command, restoring them via an ExitStack:

   self.app = session.app
   root = session.layout.container
   if not isinstance(root, HSplit): raise TypeError("Unsupported PromptSession layout")
   self.toolbar = root.children[-1]          # the session's real toolbar container
   ...
   for name, value in (("layout", self._layout), ("key_bindings", self._bindings),
                       ("erase_when_done", True)):
       stack.callback(setattr, self.app, name, getattr(self.app, name))
       setattr(self.app, name, value)

   Because there's now a single application to host views in, a third layout becomes possible: cmd2/pager.py (209 lines) — a TextArea + SearchToolbar + the same toolbar container. ppaged() measures with Pager.fits(), prints directly if it fits, otherwise swaps in the pager layout via _call_in_ui() (a Future-marshalled call onto the UI event loop) and blocks the command thread on pager.closed. New use_builtin_pager flag (defaults to enable_bottom_toolbar) opts back out to the external pager.

   Comparison

   	persistent-bottom-bar	consolidated_toolbar
   prompt-toolkit apps	2	1
   Toolbar container	reimplemented	the session's own object
   Style/color/refresh config	copied at construction	inherited automatically
   Paging	external less/more, toolbar hidden	embedded, toolbar stays live (+ opt-out)
   Chop on Windows	not supported (more wraps)	supported
   Non-shared code	355 + 497 test lines	463 + 209 + 715 test lines
   PT internals touched	get/store_typeahead, StdoutProxy._start_write_thread, app.loop	all of those plus session.layout.container shape, app.renderer.erase()/full_screen/request_absolute_cursor_position(), app.is_done, live mutation of layout/key_bindings/editing_mode/full_screen

   Advantages / disadvantages

   persistent-bottom-bar

   • ✅ Strong isolation — command-time UI can't corrupt PromptSession state (buffer, history, search state, vi state, editing_mode).
   • ✅ No dependency on the internal shape of PromptSession._create_layout().
   • ✅ Smaller: no _call_in_ui, no Future marshalling, no pager.
   • ❌ Duplicated display config drifts. Real, present bug: PBB hardcodes height=1, while PromptSession uses height=Dimension(min=1), dont_extend_height=True. A multiline bottom_toolbar renders fully at the prompt and is truncated to one row during commands. PBB's filter also drops the self.bottom_toolbar is not None clause. Every future upstream toolbar change has to be mirrored by hand.
   • ❌ Paging blanks the toolbar — for a status/progress bar (the feature's whole point), that's the moment users most want it.
   • ❌ No chopped-line paging on Windows.

   consolidated_toolbar

   • ✅ Zero display-config duplication. Reusing root.children[-1] means style, visibility filter, and multiline sizing are correct by construction, forever.
   • ✅ Follows prompt-toolkit's own documented model (one Application, interchangeable layouts).
   • ✅ Toolbar survives paging; consistent chop behavior cross-platform; use_builtin_pager=False preserves the old path.
   • ❌ One guarded structural coupling to PromptSession's layout (isinstance(root, HSplit) + children[-1] + style check). It fails loudly, and is verified against 3.0.52/3.0.53 — but it is a coupling.
   • ❌ Mutating a live shared Application is a wider blast radius. More restore paths to get right, and _call_in_ui + _check_running polling exist purely to serialize that. Latent trap: _CombinedRegistry._key_bindings caches merged bindings keyed only on (current_window, frozenset(controls)) and is not invalidated when app.key_bindings is reassigned — safe here only because each layout carries freshly constructed Windows. Worth a comment; it's the kind of thing a later refactor silently breaks.
   • ❌ Reaches into app.renderer internals for the full-screen transition.
   • ❌ +415 lines of new surface; the pager is a new feature riding along with an architecture change, which muddies review and blame.

   Which do I think is "better"

   This is a weak recommendation and I'm open to having my mind changed or to bring ideas from others:
   consolidated_toolbar — but for the architecture, not the pager

   The decisive argument isn't the pager; it's that PBB's second application must hand-copy the toolbar's construction, and that copy is already wrong (the height=1 multiline truncation). That's not a bug you fix once — it's a structural obligation to re-mirror upstream on every prompt-toolkit bump. Consolidated deletes the obligation by reusing the object. Both branches already pay the hard costs of this feature (thread, stdout proxy, stream identity, typeahead, suspension protocol); consolidated removes one of the two remaining duplications rather than adding a category of complexity.

   The counter-argument — isolation — is weaker than it looks. Both branches already run a prompt-toolkit app on the same input/output while the main thread is elsewhere; the coupling is inherent to the feature. Consolidated makes it explicit and reversible via ExitStack instead of implicit.

   Options for splitting into multiple PRs

   1. For the consolidated approach, the app-consolidation could be one reviewable change and the embedded pager could be a separate independent one that could go in first. They're independently valuable and independently riskier.
   2. For the consolidated approach, we could also have a follow-on PR to add a regression test pinning the layout-shape assumption against the prompt-toolkit versions in pyproject.toml, and a comment at the key_bindings swap noting the un-invalidated binding cache.
4. tleonhardt commented on Sep 6, 2026

   

   tleonhardt

   on Sep 6, 2026

   MemberAuthor

   More actions

   @kmvanbrunt @bambu I would particularly value feedback from both of you on the two architectural approaches present in the two separate PRs for this.

   If you have time, please look at the code and do some manual testing using the getting_started.py example app or whatever you prefer.

   I appreciate any and all feedback. I'm also very open to feedback from anyone else.
5. tleonhardt commented on Sep 6, 2026

   

   tleonhardt

   on Sep 6, 2026

   MemberAuthor

   More actions

   There is now a 3rd branch which is continuing evolution of the overall architecture which is trying to arrive at a cleaner architecture from a prompt-toolkit application perspective where we just have a single Prompt Toolkit application running continuously and we dynamically swap layouts at different stages. This is being done in attempt at eliminating an annoying flicker of the bottom toolbar.

   The WIP for this new continuous-toolbar-app branch is in PR #1748
6. tleonhardt commented on Sep 7, 2026

   

   tleonhardt

   on Sep 7, 2026

   MemberAuthor

   More actions

   The first attempt at eliminating flicker didn't pan out.

   PR #1745 does a good job of solving the persistence problem in most but not absolutely all situations.

   I am currently exploring options for eliminating visual flicker as well as truly solving the persistence problem in all situations.