6b990766

Clean up clipboard code after review: remove dead serial field and add guard comments

a73x   2026-04-09 18:58

- Remove `serial` field from `OwnedSelectionState` and the `serial` param from
  `replaceText`; the stored serial was never read in production code after being
  written — the value was already consumed by `data_device.setSelection` before
  `replaceText` was called. Update the test accordingly.
- Revert reviewer's `term` → `&term` suggestion: `Terminal.init` returns `*Terminal`,
  so `term` is already the correct type; `&term` would produce `**Terminal` (confirmed
  via build error).
- Add ordered-guard comment to `copySelectionText` explaining why the `text.len == 0`
  branch cannot be unit-tested without a live Wayland connection.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---

diff --git a/src/main.zig b/src/main.zig
index 67fface..cc94c5d 100644
--- a/src/main.zig
+++ b/src/main.zig
@@ -566,6 +566,12 @@ fn copySelectionText(
    selection: ?SelectionSpan,
    serial: u32,
) !bool {
    // Guards fire in order:
    //   1. clipboard == null  → false (no Wayland clipboard available)
    //   2. selection == null  → false (no active selection span)
    //   3. text.len == 0     → false (selection covers only blank cells)
    // Guard 3 can only be reached with a real clipboard; it cannot be unit-tested
    // without a live Wayland connection.
    const cb = clipboard orelse return false;
    const span = selection orelse return false;
    const text = try extractSelectedText(alloc, term.render_state.row_data.items(.cells), span);
diff --git a/src/wayland.zig b/src/wayland.zig
index 7a89ec9..31366aa 100644
--- a/src/wayland.zig
+++ b/src/wayland.zig
@@ -296,7 +296,6 @@ const ClipboardOffer = struct {

const OwnedSelectionState = struct {
    text: ?[]u8 = null,
    serial: u32 = 0,

    fn deinit(self: *OwnedSelectionState, alloc: std.mem.Allocator) void {
        self.clear(alloc);
@@ -306,18 +305,15 @@ const OwnedSelectionState = struct {
        self: *OwnedSelectionState,
        alloc: std.mem.Allocator,
        text: []const u8,
        serial: u32,
    ) !void {
        const dup = try alloc.dupe(u8, text);
        self.clear(alloc);
        self.text = dup;
        self.serial = serial;
    }

    fn clear(self: *OwnedSelectionState, alloc: std.mem.Allocator) void {
        if (self.text) |text| alloc.free(text);
        self.text = null;
        self.serial = 0;
    }

    fn canServeMime(self: *const OwnedSelectionState, mime: []const u8) bool {
@@ -480,7 +476,7 @@ pub const Clipboard = struct {
        const source = try self.data_device_manager.createDataSource();
        errdefer source.destroy();

        try self.owned_selection.replaceText(self.alloc, text, serial);
        try self.owned_selection.replaceText(self.alloc, text);

        source.setListener(*Clipboard, dataSourceListener, self);
        source.offer("text/plain;charset=utf-8");
@@ -940,20 +936,17 @@ test "OwnedSelectionState replaces text and recognizes offered mime types" {
    var state = OwnedSelectionState{};
    defer state.deinit(std.testing.allocator);

    try state.replaceText(std.testing.allocator, "first", 17);
    try state.replaceText(std.testing.allocator, "first");
    try std.testing.expectEqualStrings("first", state.text.?);
    try std.testing.expectEqual(@as(u32, 17), state.serial);
    try std.testing.expect(state.canServeMime("text/plain;charset=utf-8"));
    try std.testing.expect(state.canServeMime("text/plain"));
    try std.testing.expect(!state.canServeMime("text/html"));

    try state.replaceText(std.testing.allocator, "second", 21);
    try state.replaceText(std.testing.allocator, "second");
    try std.testing.expectEqualStrings("second", state.text.?);
    try std.testing.expectEqual(@as(u32, 21), state.serial);

    state.clear(std.testing.allocator);
    try std.testing.expect(state.text == null);
    try std.testing.expectEqual(@as(u32, 0), state.serial);
}

test "replaceSelectionSource returns prior source to destroy" {