feat: render into a caller-supplied IO sink (with_terminal/app io kwarg) #39
No reviewers
Labels
No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
windows
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
kahliburke/Tachikoma.jl!39
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/injectable-io"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
Adds an
iokeyword towith_terminalandapp, letting a Tachikoma apprender into a caller-supplied
IOsink — a socket, a pipe, anIOBuffer—instead of only a terminal this process is attached to.
Why
Terminalis already IO-polymorphic — every write goes throught.io— butwith_terminalcould only ever build that sink from a path, so the onlyreachable sinks were
/dev/tty, a tty path, orstdout. There was no way tohand it a sink you already hold. That makes it awkward to drive a Tachikoma UI
from anything that isn't a local terminal: streaming frames to a WebSocket,
piping to another process, or capturing to a buffer for a test.
This is a small extension of the existing
remote_ttymachinery rather than anew path: an injected sink is a remote terminal in every respect that matters —
it isn't the terminal this process owns — so it reuses that mode's handling
(no raw mode on the process's own stdin, none of the kitty/graphics probes that
would write to a terminal the frames aren't going to).
Two things an injected sink can't do that a tty can
tty_sizeis required alongsideio,and
check_resize!must not probe: it was callingterminal_size()wheneverremote_tty_pathwasnothing— which is the injected case — and under anapp's own stdout capture that reaches a pipe and returns the 80×24 default,
silently resizing every frame. A new
external_sizeflag stops that.set_size!(::Terminal, sz)lets the caller— a socket that just received a resize frame, say — declare the new size.
The caller's
iois the caller's to close: a socket outlives any one app, sowith_terminalnever closes a sink it didn't open.Compatibility
Purely additive. The default path (no
io) is unchanged. The pre-existing13-field positional
Terminalconstructor is preserved for callers that predatethe new
external_sizefield.Tests
test/test_injectable_io.jl(new, wired intoruntests.jl) covers the injectedpath end to end, including a real
Modeldriven throughapp()into anIOBufferwith no terminal involved, plus the size-required guard, theresize path, and the don't-close-the-caller's-sink guarantee. Full suite green
locally (5178 passed, 0 failures; +19 over baseline).
Developed with AI assistance.
Maintainer follow-ups (kahliburke)
leave_tui!branched onremote_tty_path, which an injectedioleavesnothing, so teardown calledset_raw_mode!(false)on a stdin thatenter_tui!had deliberately not touched — dropping an embedding REPL out ofraw mode on every app exit. Both paths now derive "not our terminal" from one
_is_remote_terminalpredicate so they cannot drift again.input=now takes precedence over anINPUT_IOa host alreadyinstalled, and that previous source is restored on exit. It was silently
ignored, leaving the app taking no keys with nothing to explain why.
demos/TachikomaDemos/Project.toml[sources]to../..; runningthe test suite rewrites it to an absolute local path, which had been committed.
tools/io_sink_demo.jl, a manual harness (frames and input over asocket,
set_size!over a second port,ncas the client). It covers the onething no automated test can:
set_raw_mode!is a no-op when stdin isn't atty, so the teardown regression is invisible headless.
main.Review — render into a caller-supplied IO sink
Well-motivated, well-commented, and genuinely additive. Treating an injected sink as a remote terminal (reusing the
remote_ttyhandling) is the right call, and thecheck_resize!fix addresses a real latent bug (probing a captured-stdout pipe returns the 80×24 default and silently resizes every frame). Two substantive issues before merge, plus a few smaller notes.1.
app(; io=...)still dups fd 0 — headless use throws (see inline)appunconditionally runs the fd-0 dup whenINPUT_IO[]is unset.Base.TTY(RawFD(saved_fd))throwsEINVALwhen fd 0 isn't a tty — exactly the headless/socket case this feature targets. The new test only passes because it manually setsT.INPUT_IO[], papering over the wart the real caller hits. There's no publicinputkwarg onapp. Suggest skipping the dup whenio !== nothingand/or adding aninputkwarg.2.
resize!doesn't trigger the clear its docstring promises (see inline)draw!decides to emitCLEAR_SCREENfromcheck_resize!'s return, which is now hard-wired tofalseforexternal_size.resize!'s return value goes to the caller, not intodraw!. So the next frame hasresized == false: noCLEAR_SCREEN.resize_buf!blanks both buffers so grown regions redraw, but on a shrink stale glyphs outside the new bounds are never cleared from the receiver — unlike the tty path, which does emit the clear. Suggest a pending-clear flagresize!sets anddraw!consumes (or havecheck_resize!returntrueonce after an external resize), and fix the docstring's causal claim.Smaller points
on_terminalis undocumented. It's added toapp's signature but not the docstring, yet it's the only way a driver gets theTerminalto callresize!on. Worth a line.Base.resize!(::Terminal, sz)semantics. Overloadingresize!where the 2nd arg is a(rows, cols)NamedTuple rather than a length stretches theresize!(collection, n)contract. A namedset_size!would read clearer; if kept, the docstring is good (modulo #2).t.size/frame area but never inspects emitted bytes for a clear, so it wouldn't catch #2.Done well
io === nothing && tty_io !== stdout) is correct and tested._remote = tty_out !== nothing || io !== nothingcleanly unifies the two off-the-local-terminal modes.tty_sizeguard fails loudly at the call site rather than visually later.Solid, mergeable direction — I'd want #1 fixed and #2 reconciled first. Both are contained changes.
Reviewed with Claude Code.
Headless
app(io=...)throws before it gets here. A few lines up (unchanged), whenINPUT_IO[]is unsetappdoessaved_fd = ccall(:dup, ...)thenBase.TTY(RawFD(saved_fd)), which throwsEINVALwhen fd 0 isn't a tty — the exact headless/socket-server caseiotargets. The new test only works because it pre-setsT.INPUT_IO[], so the wart is hidden. Since there's no publicinputkwarg, a socket-driven caller has to poke a global to avoid the crash. Suggest skipping the fd-0 dup whenio !== nothing, and/or adding aninputkwarg so the driver supplies its input source cleanly.This causal claim doesn't hold.
draw!emitsCLEAR_SCREENbased oncheck_resize!'s return, notresize!'s — andcheck_resize!now hard-returnsfalseforexternal_size. So after an out-of-bandresize!, the nextdraw!hasresized == falseand emits no clear.resize_buf!blanks both buffers so grown regions redraw via the diff, but on a shrink stale glyphs outside the new bounds persist on the receiver — the tty resize path clears, this one doesn't. Suggest a pending-clear flag thatresize!sets anddraw!/check_resize!consumes (or returntrueonce fromcheck_resize!after an external resize), then this docstring becomes accurate.