diff --git a/src/terminal/kitty/graphics_command.zig b/src/terminal/kitty/graphics_command.zig index c7c4382ae..71a2bfc7b 100644 --- a/src/terminal/kitty/graphics_command.zig +++ b/src/terminal/kitty/graphics_command.zig @@ -1094,16 +1094,18 @@ pub const Delete = struct { }, 'r', 'R' => blk: { - const x = kv.get('x') orelse 0; - const y = kv.get('y') orelse return error.InvalidFormat; - if (x > y) return error.InvalidFormat; - break :blk .{ - .range = .{ - .delete = what == 'R', - .first = x, - .last = y, - }, - }; + // Both bounds default to zero when omitted and no + // validation is performed; an inverted or zero range + // simply matches no images. + var result: Action = .{ .range = .{ .delete = what == 'R' } }; + if (kv.get('x')) |v| { + result.range.first = v; + } + if (kv.get('y')) |v| { + result.range.last = v; + } + + break :blk result; }, 'x', 'X' => blk: { @@ -1565,6 +1567,9 @@ test "delete range command 2" { } test "delete range command 3" { + // An inverted range is accepted; it simply matches no images. This + // matches kitty, which does no validation and filters with + // `x <= image_id <= y`. const testing = std.testing; const alloc = testing.allocator; var p = Parser.init(alloc, 1024 * 1024); @@ -1572,10 +1577,21 @@ test "delete range command 3" { const input = "a=d,d=R,x=5,y=4"; for (input) |c| try p.feed(c); - try testing.expectError(error.InvalidFormat, p.complete(alloc)); + const command = try p.complete(alloc); + defer command.deinit(alloc); + + try testing.expect(command.control == .delete); + const v = command.control.delete.action; + try testing.expect(v == .range); + const range = v.range; + try testing.expect(range.delete); + try testing.expectEqual(@as(u32, 5), range.first); + try testing.expectEqual(@as(u32, 4), range.last); } test "delete range command 4" { + // An omitted upper bound defaults to zero, which matches no images + // since image IDs are always non-zero. const testing = std.testing; const alloc = testing.allocator; var p = Parser.init(alloc, 1024 * 1024); @@ -1583,7 +1599,16 @@ test "delete range command 4" { const input = "a=d,d=R,x=5"; for (input) |c| try p.feed(c); - try testing.expectError(error.InvalidFormat, p.complete(alloc)); + const command = try p.complete(alloc); + defer command.deinit(alloc); + + try testing.expect(command.control == .delete); + const v = command.control.delete.action; + try testing.expect(v == .range); + const range = v.range; + try testing.expect(range.delete); + try testing.expectEqual(@as(u32, 5), range.first); + try testing.expectEqual(@as(u32, 0), range.last); } test "delete range command 5" { @@ -1605,3 +1630,24 @@ test "delete range command 5" { try testing.expectEqual(@as(u32, 0), range.first); try testing.expectEqual(@as(u32, 5), range.last); } + +test "delete range command 6" { + // Both bounds omitted defaults to the empty range [0, 0]. + const testing = std.testing; + const alloc = testing.allocator; + var p = Parser.init(alloc, 1024 * 1024); + defer p.deinit(); + + const input = "a=d,d=r"; + for (input) |c| try p.feed(c); + const command = try p.complete(alloc); + defer command.deinit(alloc); + + try testing.expect(command.control == .delete); + const v = command.control.delete.action; + try testing.expect(v == .range); + const range = v.range; + try testing.expect(!range.delete); + try testing.expectEqual(@as(u32, 0), range.first); + try testing.expectEqual(@as(u32, 0), range.last); +} diff --git a/src/terminal/kitty/graphics_storage.zig b/src/terminal/kitty/graphics_storage.zig index b18eb5370..d7507496b 100644 --- a/src/terminal/kitty/graphics_storage.zig +++ b/src/terminal/kitty/graphics_storage.zig @@ -639,15 +639,10 @@ pub const ImageStorage = struct { }, .range => |v| range: { - // The lower bound defaults to zero when x is omitted. - if (v.last == 0) { - log.warn("delete range upper bound must be greater than zero", .{}); - break :range; - } - if (v.first > v.last) { - log.warn("delete range 'x' ({}) must be less than or equal to 'y' ({})", .{ v.first, v.last }); - break :range; - } + // Both bounds default to zero when omitted. An inverted range + // or a zero upper bound is not an error, it just selects + // nothing: image IDs are always non-zero. + if (v.last == 0 or v.first > v.last) break :range; // Remove matching placements in one pass. var placement_it = self.placements.iterator(); @@ -2033,6 +2028,42 @@ test "storage: uppercase range deletes unplaced image data" { try testing.expect(s.images.contains(3)); } +test "storage: delete images by empty range" { + // Ranges that select nothing are a silent no-op, matching kitty, which + // performs no validation and filters with `x <= image_id <= y`. + 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.addImage(io, alloc, t.screens.active, .{ .id = 2 }); + try s.addPlacement(io, alloc, t.screens.active, 1, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + try s.addPlacement(io, alloc, t.screens.active, 2, 1, .{ .location = .{ .pin = try trackPin(&t, .{ .x = 1, .y = 1 }) } }); + + // Inverted range. + s.delete(io, alloc, &t, .{ .range = .{ .delete = true, .first = 5, .last = 4 } }); + try testing.expectEqual(@as(usize, 2), s.images.count()); + try testing.expectEqual(@as(usize, 2), s.placements.count()); + + // Upper bound omitted, so it defaults to zero. + s.delete(io, alloc, &t, .{ .range = .{ .delete = true, .first = 5, .last = 0 } }); + try testing.expectEqual(@as(usize, 2), s.images.count()); + try testing.expectEqual(@as(usize, 2), s.placements.count()); + + // Both bounds omitted. Image IDs are never zero so this matches nothing. + s.delete(io, alloc, &t, .{ .range = .{ .delete = true, .first = 0, .last = 0 } }); + try testing.expectEqual(@as(usize, 2), s.images.count()); + try testing.expectEqual(@as(usize, 2), s.placements.count()); + + // Both placements survive, so both of their pins are still tracked. + try testing.expectEqual(tracked + 2, t.screens.active.pages.countTrackedPins()); +} + test "storage: erase display preserves scrollback and reclaims unplaced images" { const testing = std.testing; const alloc = testing.allocator;