Skip to content

Commit a715034

Browse files
committed
Merge persistent-bottom-bar fixes into consolidated_toolbar
2 parents aac8a4e + 1702654 commit a715034

6 files changed

Lines changed: 394 additions & 90 deletions

File tree

‎cmd2/cmd2.py‎

Lines changed: 71 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -2111,8 +2111,12 @@ def _command_toolbar_context(self) -> Iterator[None]:
21112111
yield
21122112
finally:
21132113
with self.sigint_protection:
2114-
toolbar.stop()
2115-
self._command_toolbar = None
2114+
try:
2115+
toolbar.stop()
2116+
finally:
2117+
# Always forget a toolbar that has been torn down. Keeping a failed
2118+
# one would disable the toolbar for the rest of the session.
2119+
self._command_toolbar = None
21162120

21172121
def get_rprompt(self) -> AnyFormattedText:
21182122
"""Provide text to populate the prompt-toolkit right prompt.
@@ -3374,37 +3378,50 @@ def _redirect_output(self, statement: Statement) -> utils.RedirectionSavedState:
33743378
if shell:
33753379
kwargs["executable"] = shell
33763380

3377-
# Capture subprocess output when it must pass through a Python stream,
3378-
# including the toolbar proxy which prints above the running display.
3379-
proc = subprocess.Popen( # noqa: S602
3380-
statement.redirect_to,
3381-
stdin=subproc_stdin,
3382-
stdout=subprocess.PIPE
3383-
if isinstance(self.stdout, (utils.StdSim, command_toolbar.ToolbarStream)) # type: ignore[unreachable]
3384-
else self.stdout,
3385-
stderr=subprocess.PIPE
3386-
if isinstance(sys.stderr, (utils.StdSim, command_toolbar.ToolbarStream))
3387-
else sys.stderr,
3388-
shell=True,
3389-
**kwargs,
3381+
# Hand the pipe process the real terminal when there is one to inherit, since it
3382+
# may be interactive. Otherwise capture its output so it can pass through a Python
3383+
# stream, including the toolbar proxy which prints above the running display.
3384+
pipe_stdout = (
3385+
None
3386+
if isinstance(self.stdout, utils.StdSim) # type: ignore[unreachable]
3387+
else command_toolbar.pipe_target(self.stdout)
33903388
)
3389+
pipe_stderr = None if isinstance(sys.stderr, utils.StdSim) else command_toolbar.pipe_target(sys.stderr)
3390+
3391+
with contextlib.ExitStack() as terminal_stack:
3392+
# The toolbar can neither draw nor hold the keyboard while a pipe process owns
3393+
# the terminal, so step aside until that process has finished.
3394+
if pipe_stdout is not None or pipe_stderr is not None:
3395+
terminal_stack.enter_context(self.suspend_bottom_toolbar())
3396+
3397+
proc = subprocess.Popen( # noqa: S602
3398+
statement.redirect_to,
3399+
stdin=subproc_stdin,
3400+
stdout=subprocess.PIPE if pipe_stdout is None else pipe_stdout,
3401+
stderr=subprocess.PIPE if pipe_stderr is None else pipe_stderr,
3402+
shell=True,
3403+
**kwargs,
3404+
)
33913405

3392-
# Popen was called with shell=True so the user can chain pipe commands and redirect their output
3393-
# like: !ls -l | grep user | wc -l > out.txt. But this makes it difficult to know if the pipe process
3394-
# started OK, since the shell itself always starts. Therefore, we will wait a short time and check
3395-
# if the pipe process is still running.
3396-
with contextlib.suppress(subprocess.TimeoutExpired):
3397-
proc.wait(0.2)
3406+
# Popen was called with shell=True so the user can chain pipe commands and redirect their output
3407+
# like: !ls -l | grep user | wc -l > out.txt. But this makes it difficult to know if the pipe process
3408+
# started OK, since the shell itself always starts. Therefore, we will wait a short time and check
3409+
# if the pipe process is still running.
3410+
with contextlib.suppress(subprocess.TimeoutExpired):
3411+
proc.wait(0.2)
3412+
3413+
# Check if the pipe process already exited
3414+
if proc.returncode is not None:
3415+
subproc_stdin.close()
3416+
new_stdout.close()
3417+
raise RedirectionError(f"Pipe process exited with code {proc.returncode} before command could run")
3418+
redir_saved_state.redirecting = True
3419+
cmd_pipe_proc_reader = utils.ProcReader(proc, self.stdout, sys.stderr)
33983420

3399-
# Check if the pipe process already exited
3400-
if proc.returncode is not None:
3401-
subproc_stdin.close()
3402-
new_stdout.close()
3403-
raise RedirectionError(f"Pipe process exited with code {proc.returncode} before command could run")
3404-
redir_saved_state.redirecting = True
3405-
cmd_pipe_proc_reader = utils.ProcReader(proc, self.stdout, sys.stderr)
3421+
self.stdout = new_stdout
34063422

3407-
self.stdout = new_stdout
3423+
# Hold the suspension open until _restore_output() reaps the pipe process.
3424+
redir_saved_state.toolbar_suspension = terminal_stack.pop_all()
34083425

34093426
elif statement.redirector in (constants.REDIRECTION_OVERWRITE, constants.REDIRECTION_APPEND):
34103427
if statement.redirect_to:
@@ -3456,29 +3473,35 @@ def _restore_output(self, statement: Statement, saved_redir_state: utils.Redirec
34563473
:param statement: Statement object which contains the parsed input from the user
34573474
:param saved_redir_state: contains information needed to restore state data
34583475
"""
3459-
if saved_redir_state.redirecting:
3460-
# If we redirected output to the clipboard
3461-
if (
3462-
statement.redirector in (constants.REDIRECTION_OVERWRITE, constants.REDIRECTION_APPEND)
3463-
and not statement.redirect_to
3464-
):
3465-
self.stdout.seek(0)
3466-
write_to_paste_buffer(self.stdout.read())
3476+
# The toolbar gets the terminal back once the pipe process is done with it.
3477+
with contextlib.ExitStack() as terminal_stack:
3478+
if saved_redir_state.toolbar_suspension is not None:
3479+
terminal_stack.callback(saved_redir_state.toolbar_suspension.close)
3480+
saved_redir_state.toolbar_suspension = None
3481+
3482+
if saved_redir_state.redirecting:
3483+
# If we redirected output to the clipboard
3484+
if (
3485+
statement.redirector in (constants.REDIRECTION_OVERWRITE, constants.REDIRECTION_APPEND)
3486+
and not statement.redirect_to
3487+
):
3488+
self.stdout.seek(0)
3489+
write_to_paste_buffer(self.stdout.read())
34673490

3468-
with contextlib.suppress(BrokenPipeError):
3469-
# Close the file or pipe that stdout was redirected to
3470-
self.stdout.close()
3491+
with contextlib.suppress(BrokenPipeError):
3492+
# Close the file or pipe that stdout was redirected to
3493+
self.stdout.close()
34713494

3472-
# Restore self.stdout
3473-
self.stdout = cast(TextIO, saved_redir_state.saved_self_stdout)
3495+
# Restore self.stdout
3496+
self.stdout = cast(TextIO, saved_redir_state.saved_self_stdout)
34743497

3475-
# Check if we need to wait for the process being piped to
3476-
if self._cur_pipe_proc_reader is not None:
3477-
self._cur_pipe_proc_reader.wait()
3498+
# Check if we need to wait for the process being piped to
3499+
if self._cur_pipe_proc_reader is not None:
3500+
self._cur_pipe_proc_reader.wait()
34783501

3479-
# These are restored regardless of whether the command redirected
3480-
self._cur_pipe_proc_reader = saved_redir_state.saved_pipe_proc_reader
3481-
self._redirecting = saved_redir_state.saved_redirecting
3502+
# These are restored regardless of whether the command redirected
3503+
self._cur_pipe_proc_reader = saved_redir_state.saved_pipe_proc_reader
3504+
self._redirecting = saved_redir_state.saved_redirecting
34823505

34833506
def get_command_func(self, command: str) -> BoundCommandFunc[...] | None:
34843507
"""Get the bound command function for a command.

‎cmd2/command_toolbar.py‎

Lines changed: 114 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,23 @@ def wrapped(self: "Cmd", *args: Any, **kwargs: Any) -> Any:
4444
return cast(_F, wrapped)
4545

4646

47+
def pipe_target(stream: Any) -> Any:
48+
"""Return the stream a pipe process can inherit, or ``None`` if its output must be captured.
49+
50+
A pipe process may be interactive, such as ``less`` or ``fzf``, so it needs the real
51+
terminal rather than a stream this process reads on its behalf. Look through a
52+
:class:`ToolbarStream` wrapper, but only hand back a stream owning a file descriptor.
53+
"""
54+
if isinstance(stream, ToolbarStream):
55+
stream = stream.original
56+
try:
57+
stream.fileno()
58+
except (AttributeError, OSError):
59+
# io.UnsupportedOperation, raised by streams like io.StringIO, subclasses OSError.
60+
return None
61+
return stream
62+
63+
4764
class _ContextStdoutProxy(StdoutProxy):
4865
"""Keep stdout's flush worker in the toolbar's isolated application session."""
4966

@@ -57,19 +74,24 @@ def _start_write_thread(self) -> threading.Thread:
5774
class ToolbarStream:
5875
"""Keep a stable stream identity across suspensions and cmd2 redirections."""
5976

60-
def __init__(self, original: TextIO) -> None:
77+
def __init__(self, original: TextIO, lock: "threading.RLock") -> None:
6178
"""Wrap a terminal stream while preserving its ordinary file attributes."""
6279
self.original = original
6380
self.proxy: StdoutProxy | None = None
81+
# Shared with the toolbar so a write from another thread cannot land on a proxy
82+
# that is being closed. Such a write is accepted by the dead proxy and discarded.
83+
self._lock = lock
6484
self.buffer = _ToolbarBuffer(self)
6585

6686
def write(self, data: str) -> int:
6787
"""Write above the toolbar, or directly while the toolbar is suspended."""
68-
return (self.proxy or self.original).write(data)
88+
with self._lock:
89+
return (self.proxy or self.original).write(data)
6990

7091
def flush(self) -> None:
7192
"""Flush the currently active output stream."""
72-
(self.proxy or self.original).flush()
93+
with self._lock:
94+
(self.proxy or self.original).flush()
7395

7496
def __getattr__(self, name: str) -> Any:
7597
"""Delegate file attributes to the original terminal stream."""
@@ -117,6 +139,8 @@ def __init__(self, cmd: "Cmd") -> None:
117139
self._thread: threading.Thread | None = None
118140
self._streams: list[ToolbarStream] = []
119141
self._proxy: StdoutProxy | None = None
142+
self._lock = threading.RLock()
143+
self._pausing = False
120144

121145
session = cmd.main_session
122146
self.app = session.app
@@ -149,12 +173,19 @@ def interrupt(event: KeyPressEvent) -> None: # noqa: ARG001
149173
self._keys.clear()
150174
if sys.platform == "win32":
151175
# os.kill(..., SIGINT) terminates the process on Windows instead
152-
# of dispatching Python's signal handler.
176+
# of dispatching Python's signal handler. This reaches only this
177+
# process, so a console subprocess started by a command keeps
178+
# running until it is waited on.
153179
import _thread
154180

155181
_thread.interrupt_main()
156182
else:
157-
os.kill(os.getpid(), signal.SIGINT)
183+
# Raw mode clears ISIG, so no signal is generated for us. Signal the
184+
# foreground process group the way the terminal driver would, since a
185+
# command may be waiting on a subprocess that shares this group. Pipe
186+
# processes are excluded because cmd2 starts them in their own session
187+
# and forwards to them from sigint_handler().
188+
os.killpg(os.getpgrp(), signal.SIGINT)
158189

159190
@bindings.add(
160191
"c-z",
@@ -184,7 +215,7 @@ def start(self) -> None:
184215
for obj, name in ((self.cmd, "stdout"), (sys, "stdout"), (sys, "stderr")):
185216
stream = getattr(obj, name)
186217
if stream.isatty():
187-
wrapper = ToolbarStream(stream)
218+
wrapper = ToolbarStream(stream, self._lock)
188219
self._streams.append(wrapper)
189220
setattr(obj, name, cast(TextIO, wrapper))
190221
stack.callback(self._restore_stream, obj, name, wrapper)
@@ -219,6 +250,7 @@ def run() -> None:
219250
self._error = exc
220251
finally:
221252
self._ready.set()
253+
self._app_exited()
222254

223255
self._thread = threading.Thread(target=context.run, args=(run,), name="cmd2-toolbar", daemon=True)
224256
self._thread.start()
@@ -227,32 +259,86 @@ def run() -> None:
227259
raise self._error
228260
# The worker already combines queued writes. A batching sleep would also
229261
# delay close(), which runs at each command finalization boundary.
230-
self._proxy = _ContextStdoutProxy(raw=True, sleep_between_writes=0)
231-
for stream in self._streams:
232-
stream.proxy = self._proxy
262+
proxy = _ContextStdoutProxy(raw=True, sleep_between_writes=0)
263+
with self._lock:
264+
self._proxy = proxy
265+
for stream in self._streams:
266+
stream.proxy = proxy
267+
268+
def _app_exited(self) -> None:
269+
"""Give the terminal back to the streams when the display stops on its own.
270+
271+
``_ready`` is set as soon as the first frame renders, so a failure after that is
272+
never seen by the command thread waiting in ``_resume()``. The display is gone at
273+
that point and its stdout proxy can no longer reach the terminal, so anything
274+
written through it would be discarded without a trace.
275+
"""
276+
if self._pausing:
277+
# A deliberate pause restores the streams itself, in the right order.
278+
return
279+
280+
with self._lock:
281+
# Leave self._proxy set so that the next _pause() still drains and closes
282+
# it. With the display gone, its worker writes to the terminal directly.
283+
started = self._proxy is not None
284+
for stream in self._streams:
285+
stream.proxy = None
286+
287+
# A proxy exists only once _resume() has handed startup failures to the command
288+
# thread, so reporting here does not duplicate the exception it raises.
289+
if started and self._error is not None:
290+
self.cmd.perror(f"Bottom toolbar stopped after an error: {self._error!r}")
291+
292+
def _exit(self) -> None:
293+
"""Stop the display unless it has already stopped on its own.
294+
295+
Application.exit() raises once the result is set, and this runs later than the
296+
check that scheduled it. Any exception here would reach the loop's default
297+
handler, which prints a traceback over the terminal.
298+
299+
Output queued before this does not need draining: Application.run_async() waits
300+
for cursor position reports and for run_in_terminal() calls still in flight
301+
before its loop closes.
302+
"""
303+
if self.app.is_running and not self.app.is_done:
304+
self.app.exit()
233305

234306
def _pause(self) -> None:
307+
self._pausing = True
235308
try:
236-
if self._proxy is not None:
237-
self._proxy.flush()
238-
self._proxy.close()
309+
try:
310+
# Hold off other threads while the proxy drains so their output is never
311+
# handed to a proxy whose worker has already stopped. Writes that arrive
312+
# after this go straight to the terminal, still in order.
313+
with self._lock:
314+
try:
315+
if self._proxy is not None:
316+
self._proxy.flush()
317+
self._proxy.close()
318+
finally:
319+
self._proxy = None
320+
for stream in self._streams:
321+
stream.proxy = None
322+
finally:
323+
# The lock is released before joining, since the toolbar thread may be
324+
# blocked writing through a stream that is waiting on it.
325+
if self.app.is_running and self.app.loop is not None:
326+
self.app.loop.call_soon_threadsafe(self._exit)
327+
if self._thread is not None:
328+
self._thread.join()
329+
self._thread = None
330+
# Return the borrowed application to the main prompt, including on
331+
# proxy failures. The upstream toolbar owned a separate application.
332+
if self._display_stack is not None:
333+
self._display_stack.close()
334+
self._display_stack = None
335+
# Application.run() saves its unprocessed queue before the thread
336+
# exits. Those keys arrived after the ones handled by save_key().
337+
pending_keys = get_typeahead(self.app.input)
338+
store_typeahead(self.app.input, self._keys + pending_keys)
339+
self._keys.clear()
239340
finally:
240-
self._proxy = None
241-
for stream in self._streams:
242-
stream.proxy = None
243-
if self.app.is_running and self.app.loop is not None:
244-
self.app.loop.call_soon_threadsafe(self.app.exit)
245-
if self._thread is not None:
246-
self._thread.join()
247-
self._thread = None
248-
if self._display_stack is not None:
249-
self._display_stack.close()
250-
self._display_stack = None
251-
# Application.run() saves its unprocessed queue before the thread
252-
# exits. Those keys arrived after the ones handled by save_key().
253-
pending_keys = get_typeahead(self.app.input)
254-
store_typeahead(self.app.input, self._keys + pending_keys)
255-
self._keys.clear()
341+
self._pausing = False
256342

257343
def stop(self) -> None:
258344
"""Flush output, stop rendering, and restore the terminal and its streams."""

‎cmd2/utils.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -690,6 +690,9 @@ def __init__(
690690
self.saved_pipe_proc_reader = pipe_proc_reader
691691
self.saved_redirecting = saved_redirecting
692692

693+
# Holds the bottom toolbar's suspension while a pipe process owns the terminal
694+
self.toolbar_suspension: contextlib.ExitStack | None = None
695+
693696

694697
def categorize(func: Callable[..., Any] | Iterable[Callable[..., Any]], category: str) -> None:
695698
"""Categorize a function.

‎docs/features/prompt.md‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,23 @@ The embedded pager provides basic navigation and search; applications that need
125125
features can use this opt-out.
126126

127127
cmd2 temporarily hides the toolbar for its input prompts, external pagers, Python environments, and
128-
shell commands. It restores the toolbar when those operations finish. For custom terminal UIs, calls
129-
to `input()`, or subprocesses that inherit the terminal, use [cmd2.Cmd.suspend_bottom_toolbar][]:
128+
shell commands. It also hides it while a command's output is piped to another process, since that
129+
process may be interactive, as `less` and `fzf` are. It restores the toolbar when those operations
130+
finish.
131+
132+
For custom terminal UIs, calls to `input()`, or subprocesses your own command code starts, use
133+
[cmd2.Cmd.suspend_bottom_toolbar][]:
130134

131135
```py
132136
with self.suspend_bottom_toolbar():
133137
answer = input("Continue? ")
134138
```
135139

140+
Suspending matters for subprocesses even when they only print. While the toolbar is displayed, the
141+
terminal is in raw mode, so the kernel generates no signals from keystrokes. cmd2 sends `Ctrl-C` on
142+
to its own process group, which reaches a subprocess your command started and waits on, but `Ctrl-\`
143+
(`SIGQUIT`) does nothing, exactly as at the main prompt.
144+
136145
The command toolbar is used by the interactive command loop, including startup commands and scripts
137146
launched from that loop. It is disabled for non-interactive input. Calls to `onecmd_plus_hooks()`
138147
outside the command loop do not start a toolbar.

0 commit comments

Comments
 (0)