terminal/osc: honor iTerm2 OSC 1337 File= geometry hints#377
Merged
Conversation
Follow-up C to #376. The parser previously accepted File= but ignored the geometry options block; the helper produced a kitty graphics command with default Display (native size, cursor placement). This threads width / height / preserveAspectRatio through to the Kitty Display struct. Mapping: iTerm2 hint | Kitty Display -----------------------+---------------------------------------------- width=N (cells) | columns = N height=N (cells) | rows = N width=auto / height=auto | leave field 0 (native sizing) preserveAspectRatio=0 | implicit stretch when both dims set; | no-op when only one dim is given width=Npx, height=Npx | log.warn + leave 0 (no Kitty primitive | for pixel-scale) width=N%, height=N% | log.warn + leave 0 (no percent primitive) size, name | ignored per spec (unknown keys allowed) Shape change: Command.iterm2_image_transmit went from [:0]const u8 to a struct carrying the raw base64 payload plus a Iterm2ImageHints sub-struct. Both types are pub on osc.Command so stream.zig and stream_handler.zig can reference them without widening the parsers tree visibility. The single options-walk now picks up inline=1 alongside the geometry hints rather than running two separate passes. Tests (8 new parser + 3 new helper): - parser: width and height in cells populate hints - parser: width=auto leaves columns at 0 - parser: pixel-suffixed width leaves columns at 0 - parser: percent-suffixed width leaves columns at 0 - parser: case-insensitive Width and PreserveAspectRatio - parser: preserveAspectRatio=1 keeps default true - parser: non-numeric width is ignored - existing inline=1 tests updated to assert .payload + default hints - helper: hint columns and rows map to Display - helper: only columns set leaves rows at 0 for aspect preservation - helper: preserve_aspect_ratio=false with both dims allows stretch Verification: - Windows: zig build test -> 2889/2944 pass, 55 skipped, 0 failures - Linux: pending (ubuntinovm offline at commit time) - Mac: pending (macbookale offline)
- parseCellDim: width=0 / height=0 now log.warn instead of silently degrading. The iTerm2 grammar doesn't sanction zero, but some emitters send it; the warning surfaces what we couldn't honor while the return value still falls back to native sizing so downstream behavior is unchanged. - Remove "per spec" overclaim on the unknown-keys comment. The iTerm2 docs don't actually mandate that implementations ignore unknown options; reword to "iTerm2 and WezTerm do the same in practice." - synthKittyCommand: log.debug when preserveAspectRatio=false is received with only one dimension set, so layout bisectors can see that the hint was parsed but couldn't be honored (Kitty stretch mode is implicit when both columns and rows are supplied). No behavior changes for already-passing tests; 105/106 still green on Windows.
4 tasks
deblasis
added a commit
that referenced
this pull request
May 19, 2026
* terminal/osc: support iTerm2 OSC 1337 multipart File= transfers Follow-up B to #377. The single-shot File= path can only carry images that fit in a single OSC payload (~2 KiB after base64); larger images use the multipart split: OSC 1337;MultipartFile=options -- start, options only, no chunk OSC 1337;FilePart=base64_chunk -- continue, repeat as needed OSC 1337;FileEnd -- terminator iTerm2's spec has no session identifier, so transfers are strictly serialized: one in flight at a time. Wire-up: - New osc.Command.Iterm2MultipartEvent union { start: hints, chunk: bytes, end } emitted one-per-OSC by the parser. - New Iterm2MultipartAssembler in osc/parsers/iterm2.zig stitches events across OSCs: stashes hints + ArrayList(u8) on start, appends chunks, returns an Iterm2ImageTransmit on end. Owned by the stream handler via a multipart_iterm2 field with deinit on shutdown. - The MultipartFile arm rejects without inline=1, matching the single-shot File= path; the parser walks options through the same shared parseFileOptions helper so the geometry hints from #377 work on multipart too. - The single-shot iterm2ImageTransmit handler is reused unchanged. After FileEnd the multipart handler frees the assembled buffer immediately after the inner call returns, because synthKittyCommand copies the bytes through base64 decode into a fresh kitty Command buffer. The payload type on Iterm2ImageTransmit relaxed from [:0]const u8 to []const u8. The single-shot parser still produces a slice into the parser's null-terminated capture buffer, and the multipart assembler now produces a heap-allocated slice without an artificial sentinel. No consumer used the sentinel (simd.base64 takes []const u8). Error / edge cases: - FilePart with no active transfer: log.warn, drop chunk - FileEnd with no active transfer: log.warn, no-op - MultipartFile while one is in flight: log.warn, discard previous, start fresh - Accumulated payload above 64 MiB (matches APC kitty cap): log.warn, drop entire transfer - StreamHandler.deinit frees the in-flight payload via the assembler Tests: Parser (6): MultipartFile with inline+hints, MultipartFile without inline rejected, FilePart with chunk, FilePart with empty value, FileEnd, FileEnd= tolerated. Assembler (5): happy path stitches chunks + preserves hints, orphan chunk dropped, orphan end ignored, overlapping start discards previous, oversize transfer rejected. Verified: - Windows: zig build test -> 2900/2955 pass, 55 skipped, 0 failures - Mac (alessandros-macbook-air, arm64, Zig 0.15.2): zig build test-lib-vt -Dtest-filter=iterm2 -> 145/147, 2 skipped - Linux: ubuntinovm still offline at commit time * terminal/osc: fold reviewer feedback on multipart assembler - max_payload_bytes now references apc.Protocol.defaultMaxBytes(.kitty) instead of a hard-coded 64 * 1024 * 1024 literal. The literal had drifted by 1 MiB from the actual APC kitty cap (65 MiB), and the comment claimed "matches" when it did not. Sourcing the value keeps the two image-transport ceilings in lockstep automatically. - Drop the "we cheat" framing in the oversize test comment; the ArrayList.resize call does a real 65 MiB allocation, which is what we want so the boundary check exercises the production arithmetic. - Reword the FileEnd= tolerance comment so it stands on its own ("defensive because the key=value parse loop produces an empty value for the bare key") rather than citing phantom emitters that round-trip the key as `key=`. No behavior changes; 116/117 still green on Windows.
This was referenced May 19, 2026
deblasis
added a commit
that referenced
this pull request
May 20, 2026
PR #377 added Iterm2ImageTransmit + Iterm2ImageHints + Iterm2MultipartEvent as stream.zig Action variants. The stream.zig TaggedUnion meta-programming builds an extern union over all Action payload types (lib/union.zig:90), and Zig 0.15.2 rejects extern unions whose fields are not C-ABI compatible. Three specific incompatibilities: * Iterm2ImageTransmit.payload is `[]const u8` -- a Zig slice with a length field that has no stable C layout. * Iterm2MultipartEvent is a `union(enum)` with `.chunk: []const u8` -- a slice inside an unnamed Zig union tag, doubly non-extern. * Iterm2ImageHints is a plain Zig struct whose fields are individually extern-compatible but the struct itself carries no layout guarantee. Surfaced via `zig build` (the libghostty `combine_archives` step exercises the meta-programming); `zig build test` was happy because the test target doesn't pull lib/. CI on Linux/Mac didn't catch it because Windows is the only target that runs the full DX12 + iTerm2 path; #377 documented the "Linux+Mac unverified" caveat. Per the canonical precedent at apprt/action.zig `KeyTable` (lines 794-833), three localized changes: * Promote Iterm2ImageHints to `extern struct`. Field set, defaults and API are unchanged; runtime layout is identical (u32, u32, bool packs to 12 bytes either way), so the @sizeof(Command) == 64 assertion at osc.zig:368 still holds. * Add `Iterm2ImageTransmit.C` (`extern struct` with nested ptr+len payload and an embedded Iterm2ImageHints) plus `cval()`. Includes a borrowed-pointer LIFETIME comment so consumers know to copy the bytes before the dispatch callback returns. * Add `Iterm2MultipartEvent.{Tag, CValue, C}` matching KeyTable's `{Tag, CValue, C}` shape exactly: `Tag` is an `enum(c_int)`, `CValue` is the `extern union`, `C` wraps both. `cval()` uses `undefined` for `.end` (no payload) following the KeyTable convention. Wire-format mirror comments (`Sync with: ghostty_osc_iterm2_*`) make the eventual C header generation discoverable. After this, both `zig build` and `zig build test` are clean on Windows. Closes the pre-existing build break introduced by #377/#378. Co-Authored-By: Alessandro De Blasis <alex@deblasis.net>
deblasis
added a commit
that referenced
this pull request
May 20, 2026
PR #377 added Iterm2ImageTransmit + Iterm2ImageHints + Iterm2MultipartEvent as stream.zig Action variants. The stream.zig TaggedUnion meta-programming builds an extern union over all Action payload types (lib/union.zig:90), and Zig 0.15.2 rejects extern unions whose fields are not C-ABI compatible. Three specific incompatibilities: * Iterm2ImageTransmit.payload is `[]const u8` -- a Zig slice with a length field that has no stable C layout. * Iterm2MultipartEvent is a `union(enum)` with `.chunk: []const u8` -- a slice inside an unnamed Zig union tag, doubly non-extern. * Iterm2ImageHints is a plain Zig struct whose fields are individually extern-compatible but the struct itself carries no layout guarantee. Surfaced via `zig build` (the libghostty `combine_archives` step exercises the meta-programming); `zig build test` was happy because the test target doesn't pull lib/. CI on Linux/Mac didn't catch it because Windows is the only target that runs the full DX12 + iTerm2 path; #377 documented the "Linux+Mac unverified" caveat. Per the canonical precedent at apprt/action.zig `KeyTable` (lines 794-833), three localized changes: * Promote Iterm2ImageHints to `extern struct`. Field set, defaults and API are unchanged; runtime layout is identical (u32, u32, bool packs to 12 bytes either way), so the @sizeof(Command) == 64 assertion at osc.zig:368 still holds. * Add `Iterm2ImageTransmit.C` (`extern struct` with nested ptr+len payload and an embedded Iterm2ImageHints) plus `cval()`. Includes a borrowed-pointer LIFETIME comment so consumers know to copy the bytes before the dispatch callback returns. * Add `Iterm2MultipartEvent.{Tag, CValue, C}` matching KeyTable's `{Tag, CValue, C}` shape exactly: `Tag` is an `enum(c_int)`, `CValue` is the `extern union`, `C` wraps both. `cval()` uses `undefined` for `.end` (no payload) following the KeyTable convention. Wire-format mirror comments (`Sync with: ghostty_osc_iterm2_*`) make the eventual C header generation discoverable. After this, both `zig build` and `zig build test` are clean on Windows. Closes the pre-existing build break introduced by #377/#378.
deblasis
added a commit
that referenced
this pull request
May 21, 2026
* terminal/osc: honor iTerm2 OSC 1337 File= geometry hints Follow-up C to #376. The parser previously accepted File= but ignored the geometry options block; the helper produced a kitty graphics command with default Display (native size, cursor placement). This threads width / height / preserveAspectRatio through to the Kitty Display struct. Mapping: iTerm2 hint | Kitty Display -----------------------+---------------------------------------------- width=N (cells) | columns = N height=N (cells) | rows = N width=auto / height=auto | leave field 0 (native sizing) preserveAspectRatio=0 | implicit stretch when both dims set; | no-op when only one dim is given width=Npx, height=Npx | log.warn + leave 0 (no Kitty primitive | for pixel-scale) width=N%, height=N% | log.warn + leave 0 (no percent primitive) size, name | ignored per spec (unknown keys allowed) Shape change: Command.iterm2_image_transmit went from [:0]const u8 to a struct carrying the raw base64 payload plus a Iterm2ImageHints sub-struct. Both types are pub on osc.Command so stream.zig and stream_handler.zig can reference them without widening the parsers tree visibility. The single options-walk now picks up inline=1 alongside the geometry hints rather than running two separate passes. Tests (8 new parser + 3 new helper): - parser: width and height in cells populate hints - parser: width=auto leaves columns at 0 - parser: pixel-suffixed width leaves columns at 0 - parser: percent-suffixed width leaves columns at 0 - parser: case-insensitive Width and PreserveAspectRatio - parser: preserveAspectRatio=1 keeps default true - parser: non-numeric width is ignored - existing inline=1 tests updated to assert .payload + default hints - helper: hint columns and rows map to Display - helper: only columns set leaves rows at 0 for aspect preservation - helper: preserve_aspect_ratio=false with both dims allows stretch Verification: - Windows: zig build test -> 2889/2944 pass, 55 skipped, 0 failures - Linux: pending (ubuntinovm offline at commit time) - Mac: pending (macbookale offline) * terminal/osc: fold reviewer feedback on geometry hints - parseCellDim: width=0 / height=0 now log.warn instead of silently degrading. The iTerm2 grammar doesn't sanction zero, but some emitters send it; the warning surfaces what we couldn't honor while the return value still falls back to native sizing so downstream behavior is unchanged. - Remove "per spec" overclaim on the unknown-keys comment. The iTerm2 docs don't actually mandate that implementations ignore unknown options; reword to "iTerm2 and WezTerm do the same in practice." - synthKittyCommand: log.debug when preserveAspectRatio=false is received with only one dimension set, so layout bisectors can see that the hint was parsed but couldn't be honored (Kitty stretch mode is implicit when both columns and rows are supplied). No behavior changes for already-passing tests; 105/106 still green on Windows.
deblasis
added a commit
that referenced
this pull request
May 21, 2026
* terminal/osc: support iTerm2 OSC 1337 multipart File= transfers Follow-up B to #377. The single-shot File= path can only carry images that fit in a single OSC payload (~2 KiB after base64); larger images use the multipart split: OSC 1337;MultipartFile=options -- start, options only, no chunk OSC 1337;FilePart=base64_chunk -- continue, repeat as needed OSC 1337;FileEnd -- terminator iTerm2's spec has no session identifier, so transfers are strictly serialized: one in flight at a time. Wire-up: - New osc.Command.Iterm2MultipartEvent union { start: hints, chunk: bytes, end } emitted one-per-OSC by the parser. - New Iterm2MultipartAssembler in osc/parsers/iterm2.zig stitches events across OSCs: stashes hints + ArrayList(u8) on start, appends chunks, returns an Iterm2ImageTransmit on end. Owned by the stream handler via a multipart_iterm2 field with deinit on shutdown. - The MultipartFile arm rejects without inline=1, matching the single-shot File= path; the parser walks options through the same shared parseFileOptions helper so the geometry hints from #377 work on multipart too. - The single-shot iterm2ImageTransmit handler is reused unchanged. After FileEnd the multipart handler frees the assembled buffer immediately after the inner call returns, because synthKittyCommand copies the bytes through base64 decode into a fresh kitty Command buffer. The payload type on Iterm2ImageTransmit relaxed from [:0]const u8 to []const u8. The single-shot parser still produces a slice into the parser's null-terminated capture buffer, and the multipart assembler now produces a heap-allocated slice without an artificial sentinel. No consumer used the sentinel (simd.base64 takes []const u8). Error / edge cases: - FilePart with no active transfer: log.warn, drop chunk - FileEnd with no active transfer: log.warn, no-op - MultipartFile while one is in flight: log.warn, discard previous, start fresh - Accumulated payload above 64 MiB (matches APC kitty cap): log.warn, drop entire transfer - StreamHandler.deinit frees the in-flight payload via the assembler Tests: Parser (6): MultipartFile with inline+hints, MultipartFile without inline rejected, FilePart with chunk, FilePart with empty value, FileEnd, FileEnd= tolerated. Assembler (5): happy path stitches chunks + preserves hints, orphan chunk dropped, orphan end ignored, overlapping start discards previous, oversize transfer rejected. Verified: - Windows: zig build test -> 2900/2955 pass, 55 skipped, 0 failures - Mac (alessandros-macbook-air, arm64, Zig 0.15.2): zig build test-lib-vt -Dtest-filter=iterm2 -> 145/147, 2 skipped - Linux: ubuntinovm still offline at commit time * terminal/osc: fold reviewer feedback on multipart assembler - max_payload_bytes now references apc.Protocol.defaultMaxBytes(.kitty) instead of a hard-coded 64 * 1024 * 1024 literal. The literal had drifted by 1 MiB from the actual APC kitty cap (65 MiB), and the comment claimed "matches" when it did not. Sourcing the value keeps the two image-transport ceilings in lockstep automatically. - Drop the "we cheat" framing in the oversize test comment; the ArrayList.resize call does a real 65 MiB allocation, which is what we want so the boundary check exercises the production arithmetic. - Reword the FileEnd= tolerance comment so it stands on its own ("defensive because the key=value parse loop produces an empty value for the bare key") rather than citing phantom emitters that round-trip the key as `key=`. No behavior changes; 116/117 still green on Windows.
deblasis
added a commit
that referenced
this pull request
May 21, 2026
PR #377 added Iterm2ImageTransmit + Iterm2ImageHints + Iterm2MultipartEvent as stream.zig Action variants. The stream.zig TaggedUnion meta-programming builds an extern union over all Action payload types (lib/union.zig:90), and Zig 0.15.2 rejects extern unions whose fields are not C-ABI compatible. Three specific incompatibilities: * Iterm2ImageTransmit.payload is `[]const u8` -- a Zig slice with a length field that has no stable C layout. * Iterm2MultipartEvent is a `union(enum)` with `.chunk: []const u8` -- a slice inside an unnamed Zig union tag, doubly non-extern. * Iterm2ImageHints is a plain Zig struct whose fields are individually extern-compatible but the struct itself carries no layout guarantee. Surfaced via `zig build` (the libghostty `combine_archives` step exercises the meta-programming); `zig build test` was happy because the test target doesn't pull lib/. CI on Linux/Mac didn't catch it because Windows is the only target that runs the full DX12 + iTerm2 path; #377 documented the "Linux+Mac unverified" caveat. Per the canonical precedent at apprt/action.zig `KeyTable` (lines 794-833), three localized changes: * Promote Iterm2ImageHints to `extern struct`. Field set, defaults and API are unchanged; runtime layout is identical (u32, u32, bool packs to 12 bytes either way), so the @sizeof(Command) == 64 assertion at osc.zig:368 still holds. * Add `Iterm2ImageTransmit.C` (`extern struct` with nested ptr+len payload and an embedded Iterm2ImageHints) plus `cval()`. Includes a borrowed-pointer LIFETIME comment so consumers know to copy the bytes before the dispatch callback returns. * Add `Iterm2MultipartEvent.{Tag, CValue, C}` matching KeyTable's `{Tag, CValue, C}` shape exactly: `Tag` is an `enum(c_int)`, `CValue` is the `extern union`, `C` wraps both. `cval()` uses `undefined` for `.end` (no payload) following the KeyTable convention. Wire-format mirror comments (`Sync with: ghostty_osc_iterm2_*`) make the eventual C header generation discoverable. After this, both `zig build` and `zig build test` are clean on Windows. Closes the pre-existing build break introduced by #377/#378.
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.
Follow-up C to #376. The parser previously accepted File= but ignored the geometry options block; the helper produced a kitty graphics command with default Display (native size, cursor placement). This PR threads
width/height/preserveAspectRatiothrough to the Kitty Display struct.Mapping
width=N(cells)columns = Nheight=N(cells)rows = Nwidth=auto,height=autopreserveAspectRatio=0width=Npx,height=Npxlog.warn+ leave 0 (no Kitty primitive for pixel-scale)width=N%,height=N%log.warn+ leave 0 (no percent primitive)size,nameShape change
Command.iterm2_image_transmitwent from[:0]const u8to a struct carrying the raw base64 payload plus anIterm2ImageHintssub-struct. Both types arepubonosc.Commandsostream.zigandstream_handler.zigcan reference them without widening theparserstree visibility.The single options-walk now picks up
inline=1alongside the geometry hints rather than running two separate passes.Tests (8 new parser + 3 new helper)
Parser:
width=autoleaves columns at 0WidthandPreserveAspectRatiopreserveAspectRatio=1keeps default trueinline=1tests updated to assert.payload+ default hintsHelper:
preserve_aspect_ratio=falsewith both dims allows stretchVerification
zig build test-> 2889/2944 pass, 55 skipped, 0 failuresDeferred (still tracked from #375)
FilePart/FileEnd/MultipartFilechunked imagesTest plan