Stage 3: ordinary lifecycle and mode selection - #1758
Open
tleonhardt wants to merge 27 commits into
Open
Conversation
Reserved rendering depends on things cmd2 does not control -- which backend prompt-toolkit selected, which version of it is installed, whether there is a terminal at all -- so one module decides those prerequisites before anything binds a bridge or writes a margin sequence. The two non-default modes answer the same question differently on purpose. 'auto' falls back to legacy rendering for anything it has not qualified, because a backend that looks close enough is still a guess and a wrong guess corrupts the screen the user is working in. 'reserved' refuses to start rather than fall back: a caller who asked for it and silently got legacy rendering has been given the behaviour they ruled out, and would learn that from a flickering toolbar rather than an error. Qualification is by exact prompt-toolkit version. The package requirement is what cmd2 installs against; the qualified set is what the mechanism has been tested against, and it grows only through the qualification gates. The default stays legacy and nothing selects a mode yet, so no behaviour changes. Documentation and the changelog wait for Stage 5, as the plan sequences them.
Binding is two assignments, not one. Application.output is what the application and its session report, but the renderer keeps its own reference to the output it was constructed with, so binding only the application leaves the renderer drawing through the unwrapped backend -- straight over the reserved row. Restoration is just as exact: the originals go back only where the adapter is still installed, since something else may have rebound them in between and a stale object is worse than a newer one. A backend left wrapped would report a terminal one row shorter than it is to whatever runs next. The toolbar's content is read through a callable rather than captured, so a caller assigning a new bottom_toolbar to the session still reaches the band. Nothing constructs this yet; the default mode is still legacy.
Reserved mode suppresses the window prompt-toolkit would draw the toolbar in, and paints the reserved rows itself. Both layouts share one container object, so one suppression covers the main prompt and the command display alike. The window is hidden by asking whether the reservation is live rather than by latching a False into its filter. A restoration that never runs -- a teardown that raised, a caller that dropped the object -- then leaves a filter that heals itself instead of a toolbar that is gone for the rest of the session. The content provider is left alone. Callers read bottom_toolbar to mean "a toolbar is configured", so suppressing by setting it to None would answer a different question than the one being asked. The band is painted when the reservation starts, so the toolbar is there from the first prompt rather than from the first refresh.
The mode is chosen when the loop starts rather than at construction, because the answer depends on the session in use then: a caller may have replaced main_session since, and the terminal it renders to is what decides whether a reservation is possible at all. The layout is one of those prerequisites, so an unrecognized one falls back under auto and is an error under forced reserved -- the same policy as every other prerequisite, rather than a second mechanism. The reservation is established after the intro has been printed. Installing it around output that belongs to the terminal's ordinary scrollback would put a scroll region over text the user expects to keep. A refused reservation is not a refused loop: below the two-row floor there is nothing to reserve and the toolbar renders natively, so the object is kept even while inactive because the terminal can grow back.
…still ours A caller using this as a context manager never reaches __exit__ when __enter__ raises, so a failure after the acquisition -- the first paint, say -- left the margins installed, both outputs wrapped and the native toolbar suppressed: a terminal nobody owned and nobody would release. Startup now unwinds itself, best-effort, and lets the original failure propagate. The application's output and the renderer's are saved separately. They are usually the same object, but nothing guarantees it, and restoring one over the other handed the renderer a terminal it never had. The toolbar filter is restored only while it is still the one this object installed, which is the rule the outputs already followed. A filter a caller installed while reserved rendering was live is theirs, not ours to discard.
The backend buffers a paint and flushes it as one write, so a failure part-way through that write leaves the terminal holding a prefix: cursor saved and moved into the band, autowrap off, some cells replaced and some not. Unwinding the Python call undoes none of it -- those sequences are already on the wire. The painter now restores wrap mode and cursor on that path, before anything else touches the terminal. Releasing the margins first would save a cursor still sitting in the band and put it back there afterwards, which is what the rollback was doing. It also discards its baseline: the band is showing something no frame describes, so the next paint has to be a full one rather than a diff against a frame that was never finished. Fixing this in the painter rather than in startup covers every caller. An ordinary refresh mid-session fails the same way and left the same state behind. The regression drives a real partial emission -- a stream that writes a prefix of one batch and then raises. The earlier tests replaced paint() outright, so nothing was ever emitted and they could not have seen this.
The paint's opening flush drains whatever another writer left buffered, so a failure there belongs to that output rather than to this paint. Nothing of the band has been emitted, and the terminal's saved position is still the one some earlier operation put there -- the margin change's, most likely. Restoring it would move the cursor backwards over output written since, and the next write would overwrite it. An old saved position is not a recovery origin, so cleanup now asks whether this paint got as far as saving one. Everything else about the path is unchanged: the baseline is still discarded either way, because a paint that raised anywhere leaves a band no frame describes.
…ack if it repeats The backend clears its buffer before writing it, so a failed flush cannot say whether the terminal received a prefix of the batch or none of it. Buffering the cursor save does not prove the terminal got it. Either way the cursor is somewhere this process no longer knows, which makes it the renderer's problem as much as the painter's -- the next frame would be drawn from a believed position that may not be where the cursor is. A failed refresh therefore leaves recovery owed before anything renders again. A failure never reaches the command that was running. The toolbar is cosmetic and the command is not its to interrupt, so the error is kept for the caller to report once, and an error the painter is still holding is taken before the painter is dropped -- one the user never sees is the same as no error handling. One failure is a bad moment; two in a row is a terminal that has gone away. The second gives the rows back and lets the native toolbar render again, because compatibility rendering starts only after the reservation is released, never alongside it. Establishing and refreshing differ deliberately: a band that cannot be painted at all is a reservation that cannot be established, so startup still rolls back and reports rather than running with rows nothing can draw in.
In legacy rendering, output written while a command runs goes through prompt-toolkit's stdout proxy, which erases the toolbar, prints, and draws it again. That erase-and-redraw is the flicker the reserved row exists to remove: with rows withheld from scrolling, output can go straight to the terminal and the toolbar stays where it is. Straight to the terminal, but not at any moment. The write and its flush happen inside the terminal transaction, so a command's output and a toolbar paint reach the terminal one after the other rather than interleaved. The bridge is told inside that same transaction. This is the contract Stage 2b could not enforce from where it sat: output moves the cursor and may scroll the screen, and telling the bridge after the lock is released would tell it about a terminal that may have changed again. Inside, "the terminal changed" and "the change was recorded" are one event. No prompt origin is claimed. Where the cursor ends up after arbitrary output -- wrapped lines, embedded control sequences, a resize mid-write -- is not something this layer knows, and a guess would put the next prompt over committed output. ToolbarStream picks its destination under the routing lock and then releases it before writing. Carrying a routing lock into the terminal transaction is the deadlock the ordering rule exists to prevent; the legacy proxy path keeps holding it, because there the write is what must not race a proxy being closed.
The stdout proxy exists to put output above a toolbar that scrolls with the screen: it erases the toolbar, prints, and draws it again. A reserved toolbar does not scroll, so none of that is needed, and doing it anyway would put back exactly the flicker the reservation removes. The command display therefore installs serialized writers instead of a proxy when a reservation is holding the toolbar, and takes them off again when it stops. Nothing is drained on the way out: a serialized write has reached the terminal before it returns, so there is no queued work to lose. Legacy rendering is untouched, including the routing lock held across a proxy write, which is what stops a write landing on a proxy being closed.
…nation once A write that raised may have emitted part of its text and may have scrolled the screen doing it, and a stream cannot say which. The bridge is therefore told whether or not the write succeeded, still inside the transaction: invalidating after output that never arrived costs a repaint, while not invalidating after output that did costs a prompt drawn over it. ToolbarStream now chooses its destination and acts in one routing-lock acquisition, or acts after a single one. Deciding under the lock, releasing it, and taking it again to act let the destination change in between -- a serializer installed in that gap was skipped, and its write reached the terminal outside any transaction. The serializer still runs after the release, because it takes the terminal lock and carrying a routing lock into that is the deadlock the ordering rule exists to prevent.
The bridge existed and was wired to the renderer, but nothing called it: prompt-toolkit still rendered directly whenever its event loop decided to, so a frame could interleave with a command's output or a toolbar paint. Bounded erases and virtual geometry meant it could not destroy the band, but the ordering the design asks for was not there. Renders, erases and clears now go through the bridge. Preparation runs off the lock, as before, and the recorded batch is replayed inside one transaction -- the same transaction command output and paints take, which is what puts all three writers in one queue. The interception is installed on the renderer instance and removed on the way out. Patching the class would change every renderer in the process, including ones cmd2 does not own. Preparation calls the saved original rather than the attribute, or the wrapper would call itself forever; a mutation that removes that indirection hangs, which is how it was checked. A render that finds recovery owed performs it first. It runs on the UI thread, which is where recovery's callbacks belong anyway, and a frame prepared before the terminal is resynchronized would be diffed against a screen nobody has seen. When recovery cannot finish -- no known origin, waiting on the terminal to say where the cursor is -- the frame is skipped rather than guessed at. An erase or a clear leaves recovery owed: both move the cursor and clear what was below it, so nothing may be diffed against what was there.
…iling paths Abandoning reserved emission did not release the rows, so rendering upstream directly from that branch wrote outside the transaction into a terminal that was still reserved. It now renders nothing and tells the owner, which is the only thing that can give the rows back and unbind the renderer. Compatibility rendering then follows that release rather than racing it: with the bridge unbound, upstream's own render is back on the renderer and nothing routes through here at all. An erase or clear that raised part-way has still moved the cursor and cleared some of what was below it, and a stream cannot say how much, so invalidation moved inside the transaction and onto both paths -- the rule the managed writer already follows. A clear also moves the prompt. Whatever row it started on it is not that row now, so the remembered origin is forgotten rather than carried across, and the cursor reports already in flight -- which describe the screen before the clear -- are discarded with it. Unbinding restores only the methods still holding this bridge's replacements. Another caller may have wrapped the renderer since, and putting the original back over theirs would silently undo it. That is the rule the outputs and the toolbar filter already followed.
…k stale requests Cleanup failure -- the one path that actually abandons reserved emission -- stopped emission by setting the flag itself, so the owner was never told: the rows stayed withheld and the renderer stayed bound. It goes through the same door as every other abandonment now. The later call that would have notified could not help, since it finds emission already stopped and returns. Emptying the pending cursor-report queue could not discard the replies: they are already in the terminal's hands. The next one to arrive was then matched against whatever request came after the clear -- the oldest reply answering the newest question, publishing an origin from a screen that no longer exists. The entries stay in the queue now, marked, so each reply is still consumed in order and each one is refused.
The reserved row makes a distinction that did not exist before. Stopping the renderer and the input reader is one thing; handing the physical terminal to something else is another, and until now one decorator meant both. Command finalization needs only the first. It runs at the end of every command to restore terminal input settings, and the toolbar has to still be there when the next prompt appears -- so it now quiesces the display and keeps the rows. Everything that hands the terminal to a program which knows nothing about a scroll region -- shell commands, editors, embedded interpreters, external pagers, and the public suspend_bottom_toolbar() callers do the same -- gives the rows back for the duration and takes them again after. The release happens inside the pause, not around it: the renderer has to be quiet before the margins go, or a frame could land in rows that are no longer reserved. The lease is kept across the loan, and the geometry is measured afresh on the way back, because the guest may have resized the window. What it left on the screen is unknown, so the band's contents and the renderer's beliefs are both discarded rather than trusted, and the band is repainted.
…changed hands Suspension asked whether a stdout proxy was installed, which in reserved mode is never -- serialized writers take its place. Both kinds of pause therefore left the command display running: its renderer drawing and its input reader reading, alongside a guest that had been given the terminal. What has to stop is the display, whichever way its output is routed. The prompt origin is forgotten as the terminal changes hands rather than after the guest has finished with it. From the moment of the handoff the remembered row describes a screen someone else is writing on, and anything rendering against it would paint over their output. Owning the display is not the same as holding a region, and one property was answering for both. A terminal below the two-row floor left the toolbar neither painted nor natively rendered: no band to paint in, and the native window still suppressed by an owner that called itself active. Activity now means rows are actually reserved, so the native toolbar renders whenever they are not. A handoff with no region installed still runs the protocol. There is nothing to give back, but the guest may resize the window, and the return path is where that is noticed and the rows are taken again.
Suspensions nest -- cmd2 suspends around its own external commands and callers suspend around theirs -- and running the handoff protocol for each one meant an inner block ending reinstalled the margins and repainted the band while the guest the outer block had handed the terminal to was still using it. Painting a band over someone else's screen is the same mistake as never releasing at all. This was a regression from making region-less handoffs run the protocol: before that, a nested suspension found no region installed and did nothing, so the nesting was safe by accident rather than by design. Depth is now tracked explicitly. The existing nested test asserted inside both contexts and inside neither of the intervals that matter; it now checks the one between the inner and outer exits, which is where the terminal is still the guest's.
…ted frames Upstream both asks for cursor reports and receives them: its own key binding calls report_absolute_cursor_row when the reply arrives, and its renderer asks whenever it needs the height. Neither went through the bridge, so in practice the band-row validation never ran on a real reply, and a request upstream made had its reply arrive uncorrelated and discarded -- leaving the prompt's height unknown, which is the failure the design names as the reason for the rule. Both are intercepted now. A request is recorded only if one actually went out: in full-screen mode, and on backends that answer natively, upstream fills in the height and returns, and recording those would leave entries in the queue that no reply will ever consume. The band is repainted after each frame the terminal actually received. That ties it to the refresh cadence the session already has -- its interval, its invalidations, its key presses -- rather than inventing a second timer, and it paints after the prompt rather than into the middle of it. Upstream's own after-render event cannot serve: it fires during preparation, when the frame exists only as a recording.
… after_render The main prompt went through the same physical suspension as an external program, so every prompt gave the rows back, showed the native toolbar and reset the margins -- which is the stable-toolbar requirement inverted. The prompt the reservation exists for now keeps it; any other session is one cmd2 has not bound to the reservation and still gets the terminal to itself. Upstream fires after_render once render() returns, whatever the wrapper decided, so a skipped frame told everything downstream that a frame was on the screen. It is gated on the emission actually reaching the terminal now. Gating it exposed that the command display used that same event to mean "the display has started" -- two different questions sharing one signal. Waiting for a committed frame made starting the display depend on a cursor-position round trip, and with a terminal that never answers it never started at all. Readiness now hangs on a render attempt, which is what it was really asking about. The readiness wait is also bounded. It had no timeout, so anything that stopped the signal arriving held the command thread forever -- which is how this was found, as a hung suite rather than a failing test. A display that cannot start now says so, and the command runs without it.
The readiness timeout only bounded the waiting. Cleanup then joined the display thread with no timeout, so a render callback blocked inside it held the command thread anyway -- the hang moved rather than went away. The join is bounded now. A thread that will not finish is reported and kept: it is a daemon and still owns the application, so clearing the reference would let a second display start over it, and one terminal cannot have two input readers. Resuming refuses for the same reason, and a suspension that cannot restore its display says so once and carries on without a toolbar -- the command that suspended it is not this failure's to end. The earlier test suppressed the readiness signal while leaving the worker responsive, so the join it performed always returned promptly and the case could not appear. The new tests block a real render callback.
…says so Bounding the join stopped the hang but left the caller believing the display had stopped. It had not: the thread was still inside the application, still rendering and still reading input. Suspension went on to release the margins and run the guest, so two programs shared one terminal; the borrowed layout and key bindings went back to the main prompt underneath a thread still using them; and the display object was dropped at the end of the command, taking the only record of the problem with it. A timed-out pause now raises. Nothing that follows a successful pause happens: no handoff, no restoration, no typeahead transfer. The caller is the only one who knows what it was about to do with the terminal, so the failure goes to it rather than being reported and swallowed. The refusal is recorded on the application rather than on the display object, which the next command replaces. The thread outlives it, and one terminal cannot have two input readers. Startup keeps propagating the failure that brought it there rather than a secondary cleanup failure -- the session is disabled either way, and "did not start" is the useful half. A stop that fails during ordinary teardown still propagates, as it did before. Guarding _resume() against a live thread became unreachable once _pause() refuses instead of returning, so it is gone rather than left as a branch no test can reach.
…op still holds it Disabling future command displays only stopped another one from starting. The thread that would not stop was still inside the application -- rendering, and reading the same input -- and everything else went on as though the terminal were free: startup reported its timeout, the command ran, and the next suspension released the margins and handed a guest a terminal cmd2 did not own. The application now keeps a reference to the display that did not let go, and refuses to pause, prompt or hand over until it does. That covers the paths that matter through the two context managers every caller already goes through: the main prompt, nested prompts, external commands, pagers and finalization. The refusal heals itself. A render callback that finally returns, a subprocess that finally exits, and the thread ends; the next check finds it gone and stops refusing, rather than leaving the session broken for a condition that has passed.
… refusal A pause that timed out left the application as the running thread expected to find it: its layout, its key bindings, its erase-on-done setting. That was the right thing to do while the thread was alive. Once the thread ended, that state belonged to nobody, and simply dropping the refusal handed the next prompt an application still dressed as the command display. The teardown the pause could not do is finished at the moment the refusal lifts, and the refusal lifts only if it could be. Both halves of the pause -- returning the borrowed application, and handing over the keys typed meanwhile -- are now one method, so the deferred path cannot drift from the ordinary one. The regression asserts the layout, bindings and erase setting are back, not just that the refusal is gone. Checking only the refusal is what let this through.
The reservation seen from outside: a real cmdloop, start to finish. It takes the rows, gives them back, and leaves the application rendering through its own backend again -- and a second loop in the same process does it all over. A finalization hook that raises does not cost the toolbar. Finalization runs at the end of every command, which is exactly when the toolbar has to still be there, and it is the site the design singles out for keeping the reservation while the display is quiesced.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## reserved_row_toolbar #1758 +/- ##
========================================================
+ Coverage 99.73% 99.74% +0.01%
========================================================
Files 32 35 +3
Lines 7428 7915 +487
========================================================
+ Hits 7408 7895 +487
Misses 20 20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Wedging the display inside a render callback only asked for a redraw, which schedules one rather than performing it. The pause that followed could reach the event loop first, in which case the display exited cleanly and there was no wedged thread to test against -- so the test asserted a refusal that never happened, or hit the resume path and waited out the startup timeout. It now waits until the callback has actually been entered. The reserved-lifecycle harness never bound an ambient application session, so prompt-toolkit built one on demand -- patch_stdout() in _read_raw_input() does -- and on Windows that means asking for a console the CI runner does not have. The existing toolbar fixture already binds one for this reason; the new harness now does the same. Both are test defects. Neither reproduced here: this machine runs the free-threaded build, which was among the configurations that passed.
…aking them Unbinding leaves a wrapper somebody else installed over ours in place, because it is not ours to remove -- and that wrapper goes on calling in here afterwards. Clearing the saved originals at the same time left those calls with nothing to delegate to: a render that emitted nothing, and a fired after-render event that raised on a None. The originals are kept now, and every intercepted method checks whether the bridge is still bound. Unbound it is not the terminal's owner, so it passes the call straight through to upstream rather than taking the lock, preparing a frame, or withholding a notification it has no business withholding. The earlier restoration tests used standalone replacements, which never called back in, so they could not see this. The new ones delegate.
The pass-through guard reached every intercepted method except this one. A retained delegating wrapper therefore had a retired bridge take its lock and invalidate its state on someone else's behalf -- and taking the lock is not harmless: the caller may hold a higher-level lock, and the ordering rule forbids that nesting, so acting as owner turned someone else's clear into an exception. The test could not have caught it. It asserted the delegated call returned, which it did whenever nothing else held a lock. It now asserts what the guard is actually for: no terminal transaction is taken, under a higher-level lock, and the retired bridge's own state is left alone.
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
1 participant
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.
Stage 3 wires the Stage 2a/2b machinery into cmd2's command loop. The reserved toolbar is now
something a user can turn on —
bottom_toolbar_mode="reserved"— rather than something onlythe tests can drive. The default stays
legacyand nothing changes for existing applications.What this adds
cmd2/toolbar_mode.pycmd2/reserved_toolbar.pycmd2/managed_output.pyplus the integration in
cmd2.py,command_toolbar.pyand the bridge.The decisions worth reviewing
autofalls back silently;reservedrefuses to start. A backend that looks close enoughis still a guess, and a wrong guess corrupts the screen the user is working in. But a caller
who asked for reserved rendering and silently got legacy has been given the behaviour they
ruled out, and would learn it from a flickering toolbar rather than an error.
Two kinds of pause, and each site says which it means. Stopping the renderer and the input
reader is one thing; handing the physical terminal to something else is another. Command
finalization needs only the first — the toolbar has to still be there at the next prompt — and
so does the main prompt, which is what the reservation exists for. Shell commands, editors,
embedded interpreters and external pagers get the rows back for the duration.
Command output goes straight to the terminal, inside the transaction. The stdout proxy
exists to put output above a toolbar that scrolls; a reserved toolbar does not, and proxying
anyway would put back exactly the flicker being removed. The bridge is told inside the same
transaction — the contract Stage 2b could not enforce from where it sat.
Renders go through the bridge. Prompt-toolkit rendered whenever its event loop decided to,
so a frame could interleave with command output or a paint. Renders, erases, clears and cursor
reports are intercepted on the renderer instance and restored on teardown.
Gate evidence
The observer gate runs against a real cmd2 application now, not a harness that drives the
adapter directly.
The same application in legacy mode is the control: 34 disappearances across 34 commands, 33
of them erase-caused, with the toolbar still present after every window and history complete.
Legacy remains functional and flickers; reserved does not. Same observer, same workload, only
the mode differs.
Mode selection verified from the raw captures:
reservedandautoinstallESC[1;23randreset exactly once;
legacyinstalls no margins at all. The dynamic gate passes at all threegeometries, with 0 of 35 cursor reports coming from inside the band — the first live evidence
that the band-row validation is exercised by a real terminal.
Repeated loops, finalization failure and shutdown are covered at the
cmdloop()level.Verification
Full suite 2514 passed / 6 skipped;
make checkandmake docs-testclean. 100% line coverageon the five new/rewritten modules, 99% on
command_toolbar.pyandcmd2.py. Ten review roundsand twenty-two findings were resolved on this branch, each with a failing test first and each
fix mutation-checked.
Documentation, the changelog, the real-terminal matrix and any change to the default belong to
Stage 5.