terminal/kitty: accept empty graphics delete ranges (#13932)

The `d=r`/`d=R` delete parser required a `y` key and enforced `x <= y`,
rejecting the entire command with `error.InvalidFormat` otherwise. Both
bounds now default to zero and neither is validated.

This matches upstream reference implementation.

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);
+}

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;