terminal/kitty: preserve data on unmatched delete (#13898)

Make uppercase Kitty graphics deletes with a nonzero placement ID leave
the image intact when the named placement does not exist.

Previously, d=I,i=...,p=... could free unreferenced image data after
matching no placement. A later put then failed with ENOENT, diverging
from the protocol and Kitty.
This commit is contained in:
Mitchell Hashimoto
2026-08-18 12:20:45 -07:00
committed by GitHub
2 changed files with 112 additions and 3 deletions

View File

@@ -880,6 +880,38 @@ test "kittygfx delete aborts chunked image load" {
try testing.expectEqual(@as(usize, 6), storage.imageById(1).?.data.len());
}
test "kittygfx uppercase id delete preserves image when placement does not match" {
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 storage = &t.screens.active.kitty_images;
// Store an unplaced 1x1 RGB image.
{
const cmd = try command.Parser.parseString(
alloc,
"a=t,f=24,s=1,v=1,i=1;AAAA",
);
defer cmd.deinit(alloc);
try testing.expect(execute(io, alloc, &t, &cmd).?.ok());
}
// Uppercase deletion may free data only if the named placement matched.
{
const cmd = try command.Parser.parseString(
alloc,
"a=d,d=I,i=1,p=7",
);
defer cmd.deinit(alloc);
try testing.expect(execute(io, alloc, &t, &cmd) == null);
}
try testing.expect(storage.imageById(1) != null);
}
test "kittygfx default format is rgba" {
const testing = std.testing;
const alloc = testing.allocator;

View File

@@ -722,6 +722,8 @@ pub const ImageStorage = struct {
placement_id: u32,
delete_unused: bool,
) void {
var matched = placement_id == 0;
// If no placement, we delete all placements with the ID
if (placement_id == 0) self.removePlacementsByImageId(
s,
@@ -735,11 +737,13 @@ pub const ImageStorage = struct {
})) |entry| {
entry.value_ptr.deinit(s);
self.removePlacementByPtr(entry.key_ptr);
matched = true;
}
// If this is specified, then we also delete the image
// if it is no longer in use.
if (delete_unused) self.deleteIfUnused(alloc, image_id);
// A placement ID narrows the selection to that exact placement, so an
// unmatched selector must not free otherwise-unreferenced image data.
// https://sw.kovidgoyal.net/kitty/graphics-protocol/#deleting-images
if (delete_unused and matched) self.deleteIfUnused(alloc, image_id);
}
/// Delete an image if it is unused.
@@ -1525,6 +1529,79 @@ test "storage: delete placement by specific id" {
try testing.expectEqual(tracked + 2, t.screens.active.pages.countTrackedPins());
}
test "storage: uppercase id delete preserves image when placement does not match" {
const testing = std.testing;
const alloc = testing.allocator;
const io = testing.io;
var t = try terminal.Terminal.init(io, alloc, .{ .rows = 3, .cols = 3 });
defer t.deinit(alloc);
var s: ImageStorage = .{};
defer s.deinit(alloc, t.screens.active);
try s.addImage(io, alloc, t.screens.active, .{ .id = 1 });
s.dirty = false;
const generation = s.generation;
s.delete(io, alloc, &t, .{ .id = .{
.delete = true,
.image_id = 1,
.placement_id = 7,
} });
try testing.expect(s.imageById(1) != null);
try testing.expect(!s.dirty);
try testing.expectEqual(generation, s.generation);
}
test "storage: uppercase id delete frees image after placement matches" {
const testing = std.testing;
const alloc = testing.allocator;
const io = testing.io;
var t = try terminal.Terminal.init(io, alloc, .{ .rows = 3, .cols = 3 });
defer t.deinit(alloc);
const tracked = t.screens.active.pages.countTrackedPins();
var s: ImageStorage = .{};
defer s.deinit(alloc, t.screens.active);
try s.addImage(io, alloc, t.screens.active, .{ .id = 1 });
try s.addPlacement(io, alloc, t.screens.active, 1, 9, .{
.location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) },
});
s.dirty = false;
s.delete(io, alloc, &t, .{ .id = .{
.delete = true,
.image_id = 1,
.placement_id = 9,
} });
try testing.expectEqual(@as(usize, 0), s.placements.count());
try testing.expectEqual(@as(usize, 0), s.images.count());
try testing.expect(s.dirty);
try testing.expectEqual(tracked, t.screens.active.pages.countTrackedPins());
}
test "storage: uppercase id delete without placement frees unplaced image" {
const testing = std.testing;
const alloc = testing.allocator;
const io = testing.io;
var t = try terminal.Terminal.init(io, alloc, .{ .rows = 3, .cols = 3 });
defer t.deinit(alloc);
var s: ImageStorage = .{};
defer s.deinit(alloc, t.screens.active);
try s.addImage(io, alloc, t.screens.active, .{ .id = 1 });
s.dirty = false;
s.delete(io, alloc, &t, .{ .id = .{
.delete = true,
.image_id = 1,
} });
try testing.expectEqual(@as(usize, 0), s.images.count());
try testing.expect(s.dirty);
}
test "storage: delete intersecting cursor" {
const testing = std.testing;
const alloc = testing.allocator;