6b990766

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

a73x 2026-04-09 18:58

Commit message

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

- 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>

src/main.zig

Old		New
@@ -566,6 +566,12 @@ fn copySelectionText(
566	selection: ?SelectionSpan,	566	selection: ?SelectionSpan,
567	serial: u32,	567	serial: u32,
568	) !bool {	568	) !bool {
		569	// Guards fire in order:
		570	// 1. clipboard == null → false (no Wayland clipboard available)
		571	// 2. selection == null → false (no active selection span)
		572	// 3. text.len == 0 → false (selection covers only blank cells)
		573	// Guard 3 can only be reached with a real clipboard; it cannot be unit-tested
		574	// without a live Wayland connection.
569	const cb = clipboard orelse return false;	575	const cb = clipboard orelse return false;
570	const span = selection orelse return false;	576	const span = selection orelse return false;
571	const text = try extractSelectedText(alloc, term.render_state.row_data.items(.cells), span);	577	const text = try extractSelectedText(alloc, term.render_state.row_data.items(.cells), span);

src/wayland.zig

Old		New
@@ -296,7 +296,6 @@ const ClipboardOffer = struct {
296		296
297	const OwnedSelectionState = struct {	297	const OwnedSelectionState = struct {
298	text: ?[]u8 = null,	298	text: ?[]u8 = null,
299	serial: u32 = 0,
300		299
301	fn deinit(self: *OwnedSelectionState, alloc: std.mem.Allocator) void {	300	fn deinit(self: *OwnedSelectionState, alloc: std.mem.Allocator) void {
302	self.clear(alloc);	301	self.clear(alloc);
@@ -306,18 +305,15 @@ const OwnedSelectionState = struct {
306	self: *OwnedSelectionState,	305	self: *OwnedSelectionState,
307	alloc: std.mem.Allocator,	306	alloc: std.mem.Allocator,
308	text: []const u8,	307	text: []const u8,
309	serial: u32,
310	) !void {	308	) !void {
311	const dup = try alloc.dupe(u8, text);	309	const dup = try alloc.dupe(u8, text);
312	self.clear(alloc);	310	self.clear(alloc);
313	self.text = dup;	311	self.text = dup;
314	self.serial = serial;
315	}	312	}
316		313
317	fn clear(self: *OwnedSelectionState, alloc: std.mem.Allocator) void {	314	fn clear(self: *OwnedSelectionState, alloc: std.mem.Allocator) void {
318	if (self.text) \|text\| alloc.free(text);	315	if (self.text) \|text\| alloc.free(text);
319	self.text = null;	316	self.text = null;
320	self.serial = 0;
321	}	317	}
322		318
323	fn canServeMime(self: *const OwnedSelectionState, mime: []const u8) bool {	319	fn canServeMime(self: *const OwnedSelectionState, mime: []const u8) bool {
@@ -480,7 +476,7 @@ pub const Clipboard = struct {
480	const source = try self.data_device_manager.createDataSource();	476	const source = try self.data_device_manager.createDataSource();
481	errdefer source.destroy();	477	errdefer source.destroy();
482		478
483	try self.owned_selection.replaceText(self.alloc, text, serial);	479	try self.owned_selection.replaceText(self.alloc, text);
484		480
485	source.setListener(*Clipboard, dataSourceListener, self);	481	source.setListener(*Clipboard, dataSourceListener, self);
486	source.offer("text/plain;charset=utf-8");	482	source.offer("text/plain;charset=utf-8");
@@ -940,20 +936,17 @@ test "OwnedSelectionState replaces text and recognizes offered mime types" {
940	var state = OwnedSelectionState{};	936	var state = OwnedSelectionState{};
941	defer state.deinit(std.testing.allocator);	937	defer state.deinit(std.testing.allocator);
942		938
943	try state.replaceText(std.testing.allocator, "first", 17);	939	try state.replaceText(std.testing.allocator, "first");
944	try std.testing.expectEqualStrings("first", state.text.?);	940	try std.testing.expectEqualStrings("first", state.text.?);
945	try std.testing.expectEqual(@as(u32, 17), state.serial);
946	try std.testing.expect(state.canServeMime("text/plain;charset=utf-8"));	941	try std.testing.expect(state.canServeMime("text/plain;charset=utf-8"));
947	try std.testing.expect(state.canServeMime("text/plain"));	942	try std.testing.expect(state.canServeMime("text/plain"));
948	try std.testing.expect(!state.canServeMime("text/html"));	943	try std.testing.expect(!state.canServeMime("text/html"));
949		944
950	try state.replaceText(std.testing.allocator, "second", 21);	945	try state.replaceText(std.testing.allocator, "second");
951	try std.testing.expectEqualStrings("second", state.text.?);	946	try std.testing.expectEqualStrings("second", state.text.?);
952	try std.testing.expectEqual(@as(u32, 21), state.serial);
953		947
954	state.clear(std.testing.allocator);	948	state.clear(std.testing.allocator);
955	try std.testing.expect(state.text == null);	949	try std.testing.expect(state.text == null);
956	try std.testing.expectEqual(@as(u32, 0), state.serial);
957	}	950	}
958		951
959	test "replaceSelectionSource returns prior source to destroy" {	952	test "replaceSelectionSource returns prior source to destroy" {