Repository navigation
Gate kitty image output on terminal support - #95
Merged
Merged
Conversation
Per the graphics protocol spec, q=1 suppresses OK responses and q=2 also suppresses failure responses. mdriver sets no image id, so successful transmissions already draw no acknowledgement, but a failed one replies on the tty where we would never read it and the shell would echo it after we exit. Cheap to make unconditional. Refs #91
Multiplexers interpret the stream themselves and mangle kitty graphics escapes rather than forwarding them, so --images kitty inside tmux drew garbage on the screen instead of an image. Fall back to alt text and say why under --verbose. Fixes #92
image_protocol was only gated behind use_color, so --color=always --images kitty wrote megabytes of base64 kitty escapes into whatever file or pipe stdout pointed at. Adds MDRIVER_FORCE_IMAGES=1 as an escape hatch, since the check is necessarily a heuristic and there is otherwise no way to exercise image diagnostics from a subprocess. Fixes #93
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.
Three related fixes to when and how kitty graphics escapes get emitted, from reading how pi handles terminal capabilities.
Set
q=2on kitty transmissions (#91)Per the graphics protocol spec,
q=1suppresses OK responses andq=2also suppresses failure responses.Worth being precise about the scope here, because the issue as I originally filed it overstated the problem: we never set an image id (
i=/I=), and kitty ties acknowledgements to those keys, so successful transmissions were already silent. This is hardening against the failure response, which would otherwise be written to a tty nobody reads and end up in the shell's input buffer. Cheap, so unconditional.Disable images under tmux and screen (#92)
Multiplexers interpret the stream themselves and mangle graphics sequences rather than forwarding them, so
--images kittyinside tmux drew garbage rather than an image. Now falls back to alt text, with the reason logged under--verbose.Don't emit image escapes when stdout isn't a terminal (#93)
image_protocolwas only gated behinduse_color, somdriver --color=always --images kitty README.md > out.mdwrote megabytes of base64 into the file. Colors are still meaningful when piped; graphics escapes never are.Note: this adds an environment variable
MDRIVER_FORCE_IMAGES=1bypasses both new checks. This wasn't in any of the issues, so flagging it explicitly for review.It's load-bearing for the tests: four existing tests in
tests/logging.rsrun--images kittyin a subprocess to exercise image-failure diagnostics, and once images require a tty there's no way to reach that code without a pty. It also gives users an out when the heuristic is wrong — tmux with graphics passthrough configured, say. The alternative is a pty-based test harness and no override, which I'm happy to switch to.The per-condition logic is a pure function (
mdriver::image_disable_reason) taking its inputs rather than reading the environment, so every case is unit tested directly;tests/images.rscovers the end-to-end piped behavior.Verification
Beyond the test suite, checked against the real binary:
\x1b_Gf=100,a=T,q=2,c=23,m=0;...TMUXset:images: disabled because tmux does not pass kitty graphics escapes through to the terminalCommits are unsigned
Signing failed from the sandbox (
Couldn't load public key ~/.ssh/id_ed25519.pub: No such file or directory), as CLAUDE.md anticipates. Needs re-signing before merge:Session transcript
Session transcript