utils/pty: add streaming spawn and terminal sizing primitives#13695
utils/pty: add streaming spawn and terminal sizing primitives#13695euroelessar wants to merge 1 commit intomainfrom
Conversation
euroelessar
force-pushed
the
ruslan/pty-and-streaming
branch
2 times, most recently
from
7c5bb76 to
6cfc6f8
Compare
Split stdout and stderr routing into reusable output sinks, expose explicit streaming spawn helpers, and thread terminal sizing through the shared PTY layer. This keeps the legacy spawn helpers intact so existing callers only need mechanical call-site updates while app-server gains the lower-level primitives it needs for interactive command execution.
euroelessar
force-pushed
the
ruslan/pty-and-streaming
branch
from
6cfc6f8 to
f59dcaf
Compare
| arg0: &Option<String>, | ||
| stdin_mode: PipeStdinMode, | ||
| ) -> Result<SpawnedProcess> { | ||
| output_sink: OutputSink, |
There was a problem hiding this comment.
why does the sink need to be passed in? Why not keep the same design as before where we return the output (even if we split output in two separate streams)
| } | ||
|
|
||
| #[derive(Clone, Debug)] | ||
| pub enum OutputSink { |
There was a problem hiding this comment.
This type feels complicated. Can we have the output always in a separate mode and a helper that creates a merged consumer?
There was a problem hiding this comment.
so, always return two mpsc receivers & have a helper which would spawn a task to merge them & return one broadcast receiver (to maintain behavior of existing call sites)?
| writer_tx: StdMutex<Option<mpsc::Sender<Vec<u8>>>>, | ||
| output_tx: Option<broadcast::Sender<Vec<u8>>>, | ||
| killer: StdMutex<Option<Box<dyn ChildTerminator>>>, | ||
| #[allow(dead_code)] |
There was a problem hiding this comment.
why did this become dead code?
| - `writer_sender()` → `mpsc::Sender<Vec<u8>>` (stdin) | ||
| - `output_receiver()` → `broadcast::Receiver<Vec<u8>>` (stdout/stderr merged) | ||
| - `resize(TerminalSize)` | ||
| - `close_stdin()` |
There was a problem hiding this comment.
Why do we need an explicit close_stdin?
pakrym-oai
left a comment
pakrym-oai
left a comment
There was a problem hiding this comment.
Looks like we've split how pty process work based on whether we want merged or separate output. I don't think we should push this distinction so deep into the process implementation. Instead the process should always operate in split mode and when needed we can combine streams into a merged one.
2 participants
Split stdout and stderr routing into reusable output sinks, expose explicit streaming spawn helpers, and thread terminal sizing through the shared PTY layer.
This keeps the spawn helpers intact so existing callers only need mechanical call-site updates while app-server gains the lower-level primitives it needs for interactive command execution.