Keep the bottom toolbar visible during nested prompts - #1750
Merged
Conversation
read_input() and read_secret() each build their own PromptSession and passed no bottom_toolbar, while _read_raw_input() suspends the main display. The toolbar therefore vanished entirely for as long as a nested prompt was up, which is a visible inconsistency for a bar documented as persistent. Pass the toolbar to both sessions when the application has one, gated on main_session.bottom_toolbar being set so an app without a toolbar does not sprout one. Also inherit refresh_interval, so a clock or status display keeps ticking while the nested prompt waits rather than freezing at its first render. select() is deliberately left alone: prompt-toolkit's choice() accepts a toolbar but has no refresh_interval, so its toolbar would go stale while the selection sits idle.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## consolidated_toolbar #1750 +/- ##
=====================================================
Coverage 99.67% 99.67%
=====================================================
Files 25 25
Lines 6464 6464
=====================================================
Hits 6443 6443
Misses 21 21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
read_input()andread_secret()each build their ownPromptSessionand passed nobottom_toolbar, while_read_raw_input()is decorated@suspend_toolbar. So the main display stops and the nested prompt has no bar of its own — the toolbar vanishes entirely for as long as the nested prompt is up. For a bar documented as persistent, that is a visible inconsistency.Driving a real app through a pty, with a toolbar that ticks once per 200ms:
Fix
Pass the toolbar to both temporary sessions when the application has one, gated on
main_session.bottom_toolbar is not Noneso an app without a toolbar does not sprout one at a nested prompt. Also inheritrefresh_interval, so a clock or status display keeps ticking rather than freezing at its first render.Production change is 4 lines.
Verified for both
read_input()andread_secret().select()is deliberately excludedprompt_toolkit'schoice()acceptsbottom_toolbarandstylebut notrefresh_interval, andChoiceInput._create_application()constructs itsApplicationwithout one (verified in 3.0.53). Input-driven redraws would still refresh it, so it would not freeze permanently — but it would go stale while the selection sits idle, and a clock that stops until the next keypress is worse than no clock. Left for a follow-up: either accept a stale-while-idle toolbar there, or upstream arefresh_intervalparameter tochoice().Tests
Five added, TDD throughout — each of the four positive tests was watched failing for the right reason before any production code:
read_inputsession shows the toolbarbottom_toolbarwasNoneread_inputsession inheritsrefresh_interval0read_secretsession shows the toolbarbottom_toolbarwasNoneread_secretsession inheritsrefresh_interval0The fifth,
test_nested_prompt_has_no_toolbar_when_the_app_has_none, is a guard that passes against correct code by construction. Rather than claim it was red-first, I mutation-checked it: making the assignment unconditional fails it for both methods.The tests capture the real
PromptSessioncmd2 constructs and assert on its attributes, rather than asserting on mock call kwargs.make checkclean (ruff, prettier, typos,ty,mypy --strict)make test: 1930 passed, 2 skippedmake docs-testcleanDocs
docs/features/prompt.mdsaid cmd2 "temporarily hides the toolbar for its input prompts", which this makes false. Corrected, including whyselect()is excluded.CHANGELOG.mdentry added under 4.3.0.Scope
This is stage 1 of the bottom-toolbar work and is independent of the flicker investigation — it needs none of that machinery and changes nothing about run boundaries. It removes the toolbar's absence during nested prompts, not the repaint:
_read_raw_inputstill genuinely suspends, so entering and leaving a nested prompt still repaints the bar.Opened primarily to get CI coverage on all supported platforms — particularly Windows and the free-threaded 3.14t jobs — before this merges to
consolidated_toolbar.