From d866fa4553850050ec54eefa9daf2c63a9f4f4e5 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 5 Aug 2026 07:26:40 -0700 Subject: [PATCH] terminal/kitty: fix graphics range deletion Use inclusive image ID bounds for the Kitty graphics protocol range delete operation. Range deletion previously joined the lower and upper bound checks with or, which matched every placement for any valid range. A targeted delete could therefore remove every graphics placement. Join the bounds with and and update the lowercase and uppercase range tests to keep placements below and above the selected interval. --- src/terminal/kitty/graphics_storage.zig | 38 ++++++++++++++++++------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/terminal/kitty/graphics_storage.zig b/src/terminal/kitty/graphics_storage.zig index d59de54d4..3db1cdde1 100644 --- a/src/terminal/kitty/graphics_storage.zig +++ b/src/terminal/kitty/graphics_storage.zig @@ -567,7 +567,7 @@ pub const ImageStorage = struct { var it = self.placements.iterator(); while (it.next()) |entry| { - if (entry.key_ptr.image_id >= v.first or entry.key_ptr.image_id <= v.last) { + if (entry.key_ptr.image_id >= v.first and entry.key_ptr.image_id <= v.last) { const image_id = entry.key_ptr.image_id; entry.value_ptr.deinit(t.screens.active); self.placements.removeByPtr(entry.key_ptr); @@ -1485,15 +1485,24 @@ test "storage: delete images by range 3" { try s.addImage(io, alloc, .{ .id = 3 }); try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, 3, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try testing.expectEqual(@as(usize, 3), s.images.count()); - try testing.expectEqual(@as(usize, 2), s.placements.count()); + try testing.expectEqual(@as(usize, 3), s.placements.count()); s.dirty = false; - s.delete(io, alloc, &t, .{ .range = .{ .delete = false, .first = 1, .last = 1 } }); + s.delete(io, alloc, &t, .{ .range = .{ .delete = false, .first = 2, .last = 2 } }); try testing.expect(s.dirty); try testing.expectEqual(@as(usize, 3), s.images.count()); - try testing.expectEqual(@as(usize, 0), s.placements.count()); - try testing.expectEqual(tracked, t.screens.active.pages.countTrackedPins()); + try testing.expectEqual(@as(usize, 2), s.placements.count()); + try testing.expect(s.placements.get(.{ + .image_id = 1, + .placement_id = .{ .tag = .external, .id = 1 }, + }) != null); + try testing.expect(s.placements.get(.{ + .image_id = 3, + .placement_id = .{ .tag = .external, .id = 1 }, + }) != null); + try testing.expectEqual(tracked + 2, t.screens.active.pages.countTrackedPins()); } test "storage: delete images by range 4" { @@ -1511,15 +1520,24 @@ test "storage: delete images by range 4" { try s.addImage(io, alloc, .{ .id = 3 }); try s.addPlacement(io, alloc, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try s.addPlacement(io, alloc, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, 3, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); try testing.expectEqual(@as(usize, 3), s.images.count()); - try testing.expectEqual(@as(usize, 2), s.placements.count()); + try testing.expectEqual(@as(usize, 3), s.placements.count()); s.dirty = false; - s.delete(io, alloc, &t, .{ .range = .{ .delete = true, .first = 1, .last = 1 } }); + s.delete(io, alloc, &t, .{ .range = .{ .delete = true, .first = 2, .last = 2 } }); try testing.expect(s.dirty); - try testing.expectEqual(@as(usize, 1), s.images.count()); - try testing.expectEqual(@as(usize, 0), s.placements.count()); - try testing.expectEqual(tracked, t.screens.active.pages.countTrackedPins()); + try testing.expectEqual(@as(usize, 2), s.images.count()); + try testing.expectEqual(@as(usize, 2), s.placements.count()); + try testing.expect(s.placements.get(.{ + .image_id = 1, + .placement_id = .{ .tag = .external, .id = 1 }, + }) != null); + try testing.expect(s.placements.get(.{ + .image_id = 3, + .placement_id = .{ .tag = .external, .id = 1 }, + }) != null); + try testing.expectEqual(tracked + 2, t.screens.active.pages.countTrackedPins()); } test "storage: aspect ratio calculation when only columns or rows specified" {