From 86d94f150a2c3a8e83c4dcab004e1a89af07b1c6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 18 Aug 2026 12:09:40 -0700 Subject: [PATCH] terminal/kitty: preserve chunked response identifiers Chunked image responses used the final command even though only the initial chunk carries the image and placement identifiers. Successful replies lost image numbers and placement IDs. Final validation errors could also be suppressed entirely. Save the initial response identifiers with the in-progress image and use them when the final chunk completes. Continue replacing the response ID with the generated image ID after a successful load. Cover successful image-number replies and invalid final payloads with unit tests. --- src/terminal/kitty/graphics_exec.zig | 80 +++++++++++++++++++++++++-- src/terminal/kitty/graphics_image.zig | 9 +++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/terminal/kitty/graphics_exec.zig b/src/terminal/kitty/graphics_exec.zig index 8a7cb645b..ca041a1fc 100644 --- a/src/terminal/kitty/graphics_exec.zig +++ b/src/terminal/kitty/graphics_exec.zig @@ -160,11 +160,15 @@ fn transmit( cmd: *const Command, ) Response { const t = cmd.transmission().?; - var result: Response = .{ - .id = t.image_id, - .image_number = t.image_number, - .placement_id = t.placement_id, - }; + const storage = &terminal.screens.active.kitty_images; + var result: Response = if (storage.loading) |loading| + loading.response + else + .{ + .id = t.image_id, + .image_number = t.image_number, + .placement_id = t.placement_id, + }; const load = loadAndAddImage(io, alloc, terminal, cmd) catch |err| { encodeError(&result, err); @@ -594,6 +598,72 @@ test "kittygfx conflicting identifiers are rejected before mutation" { } } +test "kittygfx chunked success response uses initial identifiers" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var t = try Terminal.init(io, alloc, .{ .rows = 5, .cols = 5 }); + defer t.deinit(alloc); + + { + const cmd = try command.Parser.parseString( + alloc, + "a=t,f=24,s=1,v=2,I=93,p=7,m=1;AAAA", + ); + defer cmd.deinit(alloc); + try testing.expect(execute(io, alloc, &t, &cmd) == null); + } + + { + const cmd = try command.Parser.parseString(alloc, "m=0;AAAA"); + defer cmd.deinit(alloc); + const resp = execute(io, alloc, &t, &cmd).?; + + try testing.expect(resp.ok()); + try testing.expectEqual(@as(u32, 2147483647), resp.id); + try testing.expectEqual(@as(u32, 93), resp.image_number); + try testing.expectEqual(@as(u32, 7), resp.placement_id); + + var buf: [128]u8 = undefined; + var writer: std.Io.Writer = .fixed(&buf); + try resp.encode(&writer); + try testing.expectEqualStrings( + "\x1b_Gi=2147483647,I=93,p=7;OK\x1b\\", + writer.buffered(), + ); + } +} + +test "kittygfx chunked error response uses initial identifiers" { + const testing = std.testing; + const alloc = testing.allocator; + const io = testing.io; + + var t = try Terminal.init(io, alloc, .{ .rows = 5, .cols = 5 }); + defer t.deinit(alloc); + + { + const cmd = try command.Parser.parseString( + alloc, + "a=t,f=24,s=1,v=1,i=41,p=7,m=1;AA==", + ); + defer cmd.deinit(alloc); + try testing.expect(execute(io, alloc, &t, &cmd) == null); + } + + { + const cmd = try command.Parser.parseString(alloc, "m=0;AA=="); + defer cmd.deinit(alloc); + const resp = execute(io, alloc, &t, &cmd).?; + + try testing.expect(!resp.ok()); + try testing.expectEqual(@as(u32, 41), resp.id); + try testing.expectEqual(@as(u32, 7), resp.placement_id); + try testing.expectEqualStrings("EINVAL: invalid data", resp.message); + } +} + test "kittygfx more chunks with q=1" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/kitty/graphics_image.zig b/src/terminal/kitty/graphics_image.zig index a6a8be014..aed853d00 100644 --- a/src/terminal/kitty/graphics_image.zig +++ b/src/terminal/kitty/graphics_image.zig @@ -39,6 +39,10 @@ pub const LoadingImage = struct { /// used if q isn't set on subsequent chunks. quiet: command.Command.Quiet, + /// Response identifiers from the initial load command. Subsequent chunks + /// omit these, so completion responses must use the saved values. + response: command.Response = .{}, + /// The temporary directory for file transmission (null means that /// temporary directory transmission is disabled). temporary_directory: ?[]const u8, @@ -104,6 +108,11 @@ pub const LoadingImage = struct { .display = cmd.display(), .quiet = cmd.quiet, + .response = .{ + .id = t.image_id, + .image_number = t.image_number, + .placement_id = t.placement_id, + }, .temporary_directory = switch (limits.temporary_file) { .enabled => |d| d.directory, .disabled => null,