From 24c0adc1762be08783e2af6305c0c6d2bb228391 Mon Sep 17 00:00:00 2001 From: Andrew de los Reyes Date: Sat, 24 May 2025 15:49:24 -0700 Subject: [PATCH] PageList.zig: Allow a Pin's x to be a column or negative. Changes Pin.x from u16 to a union that's either .col: u16 (common case) or .neg: void (newly introduced case). The .neg situation occurs when the mouse is dragging and it's at the very left edge of the terminal, or even further left. This also changes gtkMouseMotion to not clamp x motion to 0 or positive. This is to ensure that a slightly positive x value that transitions negative will trigger `!is_cursor_still`. Also, this adds a couple unit tests. The overall goal of the change is to fix this issue where an attempt to select a full line of text by dragging the mouse off the left hand side would inadvertantly select one additional character: https://github.com/ghostty-org/ghostty/discussions/5058 --- src/Surface.zig | 29 ++-- src/apprt/gtk/Surface.zig | 2 +- src/font/shaper/run.zig | 4 +- src/renderer/OpenGL.zig | 104 ++++++------ src/renderer/cell.zig | 8 +- src/terminal/PageList.zig | 208 ++++++++++++++++-------- src/terminal/Screen.zig | 107 +++++++++--- src/terminal/Selection.zig | 14 +- src/terminal/kitty/graphics_exec.zig | 2 +- src/terminal/kitty/graphics_storage.zig | 8 +- src/terminal/kitty/graphics_unicode.zig | 4 +- src/terminal/search.zig | 2 +- 12 files changed, 309 insertions(+), 183 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index f9e232340..88be5df59 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -3583,7 +3583,7 @@ pub fn cursorPosCallback( // Convert to points const screen = &self.renderer_state.terminal.screen; - const pin = screen.pages.pin(.{ + var pin = screen.pages.pin(.{ .viewport = .{ .x = pos_vp.x, .y = pos_vp.y, @@ -3592,6 +3592,9 @@ pub fn cursorPosCallback( if (comptime std.debug.runtime_safety) unreachable; return; }; + if (pos.x <= @as(f32, @floatFromInt(self.size.padding.left))) { + pin.x = .neg; + } // Handle dragging depending on click count switch (self.mouse.left_click_count) { @@ -3710,7 +3713,7 @@ fn dragLeftClickSingle( // first xpos of the clicked cell adjusted for padding const left_padding_f64: f64 = @as(f64, @floatFromInt(self.size.padding.left)); - const cell_xstart = @as(f64, @floatFromInt(click_pin.x)) * cell_width_f64; + const cell_xstart = @as(f64, @floatFromInt(click_pin.xInt())) * cell_width_f64; const cell_start_xpos = self.mouse.left_click_xpos - cell_xstart - left_padding_f64; // If this is the same cell, then we only start the selection if weve @@ -3746,16 +3749,16 @@ fn dragLeftClickSingle( self.mouse.mods, )) start: { if (cell_start_xpos >= cell_xboundary) break :start click_pin; - if (click_pin.x > 0) break :start click_pin.left(1); + if (click_pin.xInt() > 0) break :start click_pin.left(1); var start = click_pin.up(1) orelse click_pin; - start.x = self.io.terminal.screen.pages.cols - 1; + start.x = .{ .col = self.io.terminal.screen.pages.cols - 1 }; break :start start; } else start: { if (cell_start_xpos < cell_xboundary) break :start click_pin; - if (click_pin.x < self.io.terminal.screen.pages.cols - 1) + if (click_pin.xInt() < self.io.terminal.screen.pages.cols - 1) break :start click_pin.right(1); var start = click_pin.down(1) orelse click_pin; - start.x = 0; + start.x = .{ .col = 0 }; break :start start; }; @@ -3798,14 +3801,14 @@ fn checkResetSelSwitch( // the click point depending on the selection mode we're in, with // the exception of single-column selections, which we always reset // on if we drift. - if (sel_start.x == sel_end.x) { - reset = drag_pin.x != sel_start.x; + if (sel_start.x.eq(sel_end.x)) { + reset = !drag_pin.x.eq(sel_start.x); } else { reset = switch (sel.order(screen)) { - .forward => drag_pin.x < sel_start.x or drag_pin.before(sel_start), - .reverse => drag_pin.x > sel_start.x or sel_start.before(drag_pin), - .mirrored_forward => drag_pin.x > sel_start.x or drag_pin.before(sel_start), - .mirrored_reverse => drag_pin.x < sel_start.x or sel_start.before(drag_pin), + .forward => drag_pin.x.lessThan(sel_start.x) or drag_pin.before(sel_start), + .reverse => drag_pin.x.greaterThan(sel_start.x) or sel_start.before(drag_pin), + .mirrored_forward => drag_pin.x.greaterThan(sel_start.x) or drag_pin.before(sel_start), + .mirrored_reverse => drag_pin.x.lessThan(sel_start.x) or sel_start.before(drag_pin), }; } } else { @@ -3831,7 +3834,7 @@ fn dragLeftClickBefore( mods: input.Mods, ) bool { if (mods.ctrlOrSuper() and mods.alt) { - return drag_pin.x < click_pin.x; + return drag_pin.x.lessThan(click_pin.x); } return drag_pin.before(click_pin); diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig index bcb78e087..b9dcd30c3 100644 --- a/src/apprt/gtk/Surface.zig +++ b/src/apprt/gtk/Surface.zig @@ -1563,7 +1563,7 @@ fn gtkMouseMotion( const scaled = self.scaledCoordinates(x, y); const pos: apprt.CursorPos = .{ - .x = @floatCast(@max(0, scaled.x)), + .x = @floatCast(scaled.x), .y = @floatCast(scaled.y), }; diff --git a/src/font/shaper/run.zig b/src/font/shaper/run.zig index 18ddd4b56..b7246cacc 100644 --- a/src/font/shaper/run.zig +++ b/src/font/shaper/run.zig @@ -89,8 +89,8 @@ pub const RunIterator = struct { if (self.selection) |unordered_sel| { if (j > self.i) { const sel = unordered_sel.ordered(self.screen, .forward); - const start_x = sel.start().x; - const end_x = sel.end().x; + const start_x = sel.start().xInt(); + const end_x = sel.end().xInt(); if (start_x > 0 and j == start_x) break; diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index d0222a390..9196a3803 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -1035,7 +1035,7 @@ fn prepKittyVirtualPlacement( try self.prepKittyImage(&image); try self.image_placements.append(self.alloc, .{ .image_id = image.id, - .x = @intCast(rp.top_left.x), + .x = @intCast(rp.top_left.x.col), .y = @intCast(viewport.viewport.y), .z = -1, .width = rp.dest_width, @@ -1097,7 +1097,7 @@ fn prepKittyPlacement( if (dest_size.width > 0 and dest_size.height > 0) { try self.image_placements.append(self.alloc, .{ .image_id = image.id, - .x = @intCast(rect.top_left.x), + .x = @intCast(rect.top_left.x.col), .y = y_pos, .z = p.z, .width = dest_size.width, @@ -1366,29 +1366,29 @@ pub fn rebuildCells( // Try to read the cells from the shaping cache if we can. self.font_shaper_cache.get(run) orelse cache: { - // Otherwise we have to shape them. - const cells = try self.font_shaper.shape(run); + // Otherwise we have to shape them. + const cells = try self.font_shaper.shape(run); - // Try to cache them. If caching fails for any reason we - // continue because it is just a performance optimization, - // not a correctness issue. - self.font_shaper_cache.put( - self.alloc, - run, - cells, - ) catch |err| { - log.warn( - "error caching font shaping results err={}", - .{err}, - ); - }; - - // The cells we get from direct shaping are always owned - // by the shaper and valid until the next shaping call so - // we can safely use them. - break :cache cells; + // Try to cache them. If caching fails for any reason we + // continue because it is just a performance optimization, + // not a correctness issue. + self.font_shaper_cache.put( + self.alloc, + run, + cells, + ) catch |err| { + log.warn( + "error caching font shaping results err={}", + .{err}, + ); }; + // The cells we get from direct shaping are always owned + // by the shaper and valid until the next shaping call so + // we can safely use them. + break :cache cells; + }; + // Advance our index until we reach or pass // our current x position in the shaper cells. while (shaper_cells.?[shaper_cells_i].x < x) { @@ -1402,7 +1402,7 @@ pub fn rebuildCells( const cell_pin: terminal.Pin = cell: { var copy = row; - copy.x = @intCast(x); + copy.x = .{ .col = @intCast(x) }; break :cell copy; }; @@ -1411,14 +1411,16 @@ pub fn rebuildCells( sel.contains(screen, .{ .node = row.node, .y = row.y, - .x = @intCast( - // Spacer tails should show the selection - // state of the wide cell they belong to. - if (wide == .spacer_tail) - x -| 1 - else - x, - ), + .x = .{ + .col = @intCast( + // Spacer tails should show the selection + // state of the wide cell they belong to. + if (wide == .spacer_tail) + x -| 1 + else + x, + ), + }, }) else false; @@ -1611,29 +1613,29 @@ pub fn rebuildCells( // Try to read the cells from the shaping cache if we can. self.font_shaper_cache.get(run) orelse cache: { - // Otherwise we have to shape them. - const cells = try self.font_shaper.shape(run); + // Otherwise we have to shape them. + const cells = try self.font_shaper.shape(run); - // Try to cache them. If caching fails for any reason we - // continue because it is just a performance optimization, - // not a correctness issue. - self.font_shaper_cache.put( - self.alloc, - run, - cells, - ) catch |err| { - log.warn( - "error caching font shaping results err={}", - .{err}, - ); - }; - - // The cells we get from direct shaping are always owned - // by the shaper and valid until the next shaping call so - // we can safely use them. - break :cache cells; + // Try to cache them. If caching fails for any reason we + // continue because it is just a performance optimization, + // not a correctness issue. + self.font_shaper_cache.put( + self.alloc, + run, + cells, + ) catch |err| { + log.warn( + "error caching font shaping results err={}", + .{err}, + ); }; + // The cells we get from direct shaping are always owned + // by the shaper and valid until the next shaping call so + // we can safely use them. + break :cache cells; + }; + const cells = shaper_cells orelse break :glyphs; // If there are no shaper cells for this run, ignore it. diff --git a/src/renderer/cell.zig b/src/renderer/cell.zig index c84fbcc6f..750b05e87 100644 --- a/src/renderer/cell.zig +++ b/src/renderer/cell.zig @@ -70,16 +70,16 @@ pub fn fgMode( } // If we are at the end of the screen its definitely constrained - if (cell_pin.x == cell_pin.node.data.size.cols - 1) break :text .constrained; + if (cell_pin.xInt() == cell_pin.node.data.size.cols - 1) break :text .constrained; // If we have a previous cell and it was PUA then we need to // also constrain. This is so that multiple PUA glyphs align. // As an exception, we ignore powerline glyphs since they are // used for box drawing and we consider them whitespace. - if (cell_pin.x > 0) prev: { + if (cell_pin.xInt() > 0) prev: { const prev_cp = prev_cp: { var copy = cell_pin; - copy.x -= 1; + copy.x.col -= 1; const prev_cell = copy.rowAndCell().cell; break :prev_cp prev_cell.codepoint(); }; @@ -96,7 +96,7 @@ pub fn fgMode( // full glyph size. const next_cp = next_cp: { var copy = cell_pin; - copy.x += 1; + copy.x = .{ .col = copy.xInt() + 1 }; const next_cell = copy.rowAndCell().cell; break :next_cp next_cell.codepoint(); }; diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 95519fe99..9198e55e4 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -411,7 +411,7 @@ pub fn reset(self: *PageList) void { while (it.next()) |entry| { const p: *Pin = entry.key_ptr.*; p.node = self.pages.first.?; - p.x = 0; + p.x = .{ .col = 0 }; p.y = 0; } } @@ -846,19 +846,19 @@ const ReflowCursor = struct { const pin_keys = list.tracked_pins.keys(); for (pin_keys) |p| { if (&p.node.data != src_page or - p.y != src_y) continue; + p.y != src_y or p.x != .col) continue; // If this pin is in the blanks on the right and past the end // of the dst col width then we move it to the end of the dst // col width instead. - if (p.x >= cols_len) { - p.x = @min(p.x, cap.cols - 1 - self.x); + if (p.x.col >= cols_len) { + p.x.col = @min(p.x.col, cap.cols - 1 - self.x); } // We increase our col len to at least include this pin. // This ensures that blank rows with pins are processed, // so that the pins can be properly remapped. - cols_len = @max(cols_len, p.x + 1); + cols_len = @max(cols_len, p.x.col + 1); } } @@ -897,10 +897,11 @@ const ReflowCursor = struct { for (pin_keys) |p| { if (&p.node.data != src_page or p.y != src_y or - p.x != x) continue; + p.x != .col or + p.x.col != x) continue; p.node = self.node; - p.x = self.x; + p.x = .{ .col = self.x }; p.y = self.y; } } @@ -1303,7 +1304,12 @@ fn resizeWithoutReflow(self: *PageList, opts: Resize) !void { // beyond the edge, clamp it. const pin_keys = self.tracked_pins.keys(); for (pin_keys) |p| { - if (p.x >= cols) p.x = cols - 1; + switch (p.x) { + .col => |x| { + if (x >= cols) p.x.col = cols - 1; + }, + .neg => {}, + } } self.cols = cols; @@ -1828,7 +1834,7 @@ pub fn grow(self: *PageList) !?*List.Node { if (p.node != first) continue; p.node = self.pages.first.?; p.y = 0; - p.x = 0; + p.x = .{ .col = 0 }; } // In this case we do NOT need to update page_size because @@ -2161,7 +2167,7 @@ pub fn eraseRowBounded( p.y <= pn.y + limit) { if (p.y == 0) { - p.x = 0; + p.x = .{ .col = 0 }; } else { p.y -= 1; } @@ -2190,7 +2196,7 @@ pub fn eraseRowBounded( for (pin_keys) |p| { if (p.node == node and p.y >= pn.y) { if (p.y == 0) { - p.x = 0; + p.x = .{ .col = 0 }; } else { p.y -= 1; } @@ -2341,7 +2347,7 @@ pub fn eraseRows( p.y -= chunk.end; } else { p.y = 0; - p.x = 0; + p.x = .{ .col = 0 }; } } @@ -2397,7 +2403,7 @@ fn erasePage(self: *PageList, node: *List.Node) void { if (p.node != node) continue; p.node = node.next orelse node.prev orelse unreachable; p.y = 0; - p.x = 0; + p.x = .{ .col = 0 }; } // Remove the page from the linked list @@ -2419,7 +2425,7 @@ pub fn pin(self: *const PageList, pt: point.Point) ?Pin { // Grab the top left and move to the point. var p = self.getTopLeft(pt).down(pt.coord().y) orelse return null; - p.x = x; + p.x = .{ .col = x }; return p; } @@ -2466,7 +2472,7 @@ pub fn pinIsValid(self: *const PageList, p: Pin) bool { while (it) |node| : (it = node.next) { if (node != p.node) continue; return p.y < node.data.size.rows and - p.x < node.data.size.cols; + (p.x != .col or p.x.col < node.data.size.cols); } return false; @@ -2509,7 +2515,7 @@ pub fn pointFromPin(self: *const PageList, tag: point.Tag, p: Pin) ?point.Point const tl = self.getTopLeft(tag); // Count our first page which is special because it may be partial. - var coord: point.Coordinate = .{ .x = p.x }; + var coord: point.Coordinate = .{ .x = p.xInt() }; if (p.node == tl.node) { // If our top-left is after our y then we're outside the range. if (tl.y > p.y) return null; @@ -2545,13 +2551,13 @@ pub fn pointFromPin(self: *const PageList, tag: point.Tag, p: Pin) ?point.Point /// Warning: this is slow and should not be used in performance critical paths pub fn getCell(self: *const PageList, pt: point.Point) ?Cell { const pt_pin = self.pin(pt) orelse return null; - const rac = pt_pin.node.data.getRowAndCell(pt_pin.x, pt_pin.y); + const rac = pt_pin.node.data.getRowAndCell(pt_pin.xInt(), pt_pin.y); return .{ .node = pt_pin.node, .row = rac.row, .cell = rac.cell, .row_idx = pt_pin.y, - .col_idx = pt_pin.x, + .col_idx = pt_pin.xInt(), }; } @@ -2815,12 +2821,13 @@ pub const CellIterator = struct { pub fn next(self: *CellIterator) ?Pin { const cell = self.cell orelse return null; + const x: u16 = cell.xInt(); switch (self.row_it.page_it.direction) { .right_down => { - if (cell.x + 1 < cell.node.data.size.cols) { + if (x + 1 < cell.node.data.size.cols) { // We still have cells in this row, increase x. var copy = cell; - copy.x += 1; + copy.x = .{ .col = x + 1 }; self.cell = copy; } else { // We need to move to the next row. @@ -2829,16 +2836,16 @@ pub const CellIterator = struct { }, .left_up => { - if (cell.x > 0) { + if (x > 0) { // We still have cells in this row, decrease x. var copy = cell; - copy.x -= 1; + copy.x = .{ .col = x - 1 }; self.cell = copy; } else { // We need to move to the previous row and last col if (self.row_it.next()) |next_cell| { var copy = next_cell; - copy.x = next_cell.node.data.size.cols - 1; + copy.x = .{ .col = next_cell.node.data.size.cols - 1 }; self.cell = copy; } else { self.cell = null; @@ -3191,7 +3198,7 @@ pub fn getBottomRight(self: *const PageList, tag: point.Tag) ?Pin { break :last .{ .node = node, .y = node.data.size.rows - 1, - .x = node.data.size.cols - 1, + .x = .{ .col = node.data.size.cols - 1 }, }; }, @@ -3294,13 +3301,59 @@ fn markDirty(self: *PageList, pt: point.Point) void { pub const Pin = struct { node: *List.Node, y: size.CellCountInt = 0, - x: size.CellCountInt = 0, + x: union(enum) { + col: size.CellCountInt, + neg: void, + + pub fn lessThan(self: @This(), other: @This()) bool { + return switch (self) { + .col => |col| switch (other) { + .col => |other_col| col < other_col, + .neg => false, // col is always greater than negative + }, + .neg => switch (other) { + .col => true, + .neg => false, + }, + }; + } + pub fn eq(self: @This(), other: @This()) bool { + return switch (self) { + .col => |col| switch (other) { + .col => |other_col| col == other_col, + .neg => false, + }, + .neg => switch (other) { + .col => false, + .neg => true, + }, + }; + } + pub fn lessEq(self: @This(), other: @This()) bool { + return switch (self) { + .col => |col| switch (other) { + .col => |other_col| col <= other_col, + .neg => false, // col is always greater than negative + }, + .neg => true, + }; + } + pub fn greaterThan(self: @This(), other: @This()) bool { + return !self.lessEq(other); + } + } = .{ .col = 0 }, + pub fn xInt(self: Pin) size.CellCountInt { + return switch (self.x) { + .col => |x| x, + .neg => 0, + }; + } pub fn rowAndCell(self: Pin) struct { row: *pagepkg.Row, cell: *pagepkg.Cell, } { - const rac = self.node.data.getRowAndCell(self.x, self.y); + const rac = self.node.data.getRowAndCell(self.xInt(), self.y); return .{ .row = rac.row, .cell = rac.cell }; } @@ -3312,10 +3365,17 @@ pub const Pin = struct { pub fn cells(self: Pin, subset: CellSubset) []pagepkg.Cell { const rac = self.rowAndCell(); const all = self.node.data.getCells(rac.row); - return switch (subset) { - .all => all, - .left => all[0 .. self.x + 1], - .right => all[self.x..], + return switch (self.x) { + .col => |x| switch (subset) { + .all => all, + .left => all[0 .. x + 1], + .right => all[x..], + }, + .neg => switch (subset) { + .all => all, + .left => all[0..0], + .right => all, + }, }; } @@ -3469,7 +3529,7 @@ pub const Pin = struct { // If top is bottom, must be ordered. assert(top.y <= bottom.y); if (top.y == bottom.y) { - assert(top.x <= bottom.x); + assert(top.x.lessEq(bottom.x)); } } else { // If top is not bottom, top must be before bottom. @@ -3501,7 +3561,7 @@ pub const Pin = struct { // Otherwise our y is the same as the top y, so we need to // check the x coordinate. assert(self.y == top.y); - if (self.x < top.x) return false; + if (self.x.lessThan(top.x)) return false; } if (self.node == bottom.node) { // Our page is the bottom page so we're between the top and @@ -3512,7 +3572,7 @@ pub const Pin = struct { // If our y is the same, then we're between if we're before // or equal to the bottom x. assert(self.y == bottom.y); - return self.x <= bottom.x; + return self.x.lessEq(bottom.x); } // Our page isn't the top or bottom so we need to check if @@ -3538,7 +3598,7 @@ pub const Pin = struct { if (self.node == other.node) { if (self.y < other.y) return true; if (self.y > other.y) return false; - return self.x < other.x; + return self.x.lessThan(other.x); } var node_ = self.node.next; @@ -3552,22 +3612,28 @@ pub const Pin = struct { pub fn eql(self: Pin, other: Pin) bool { return self.node == other.node and self.y == other.y and - self.x == other.x; + self.x.eq(other.x); } /// Move the pin left n columns. n must fit within the size. pub fn left(self: Pin, n: usize) Pin { - assert(n <= self.x); + switch (self.x) { + .col => |x| assert(n <= x), + .neg => assert(false), + } var result = self; - result.x -= std.math.cast(size.CellCountInt, n) orelse result.x; + result.x.col -= std.math.cast(size.CellCountInt, n) orelse result.x.col; return result; } /// Move the pin right n columns. n must fit within the size. pub fn right(self: Pin, n: usize) Pin { - assert(self.x + n < self.node.data.size.cols); + switch (self.x) { + .col => |x| assert(x + n < self.node.data.size.cols), + .neg => assert(false), + } var result = self; - result.x +|= std.math.cast(size.CellCountInt, n) orelse + result.x.col +|= std.math.cast(size.CellCountInt, n) orelse std.math.maxInt(size.CellCountInt); return result; } @@ -3727,7 +3793,7 @@ test "PageList" { try testing.expectEqual(Pin{ .node = s.pages.first.?, .y = 0, - .x = 0, + .x = .{ .col = 0 }, }, s.getTopLeft(.active)); } @@ -3770,7 +3836,7 @@ test "PageList pointFromPin active no history" { }, s.pointFromPin(.active, .{ .node = s.pages.first.?, .y = 0, - .x = 0, + .x = .{ .col = 0 }, }).?); } { @@ -3782,7 +3848,7 @@ test "PageList pointFromPin active no history" { }, s.pointFromPin(.active, .{ .node = s.pages.first.?, .y = 2, - .x = 4, + .x = .{ .col = 4 }, }).?); } } @@ -3804,7 +3870,7 @@ test "PageList pointFromPin active with history" { }, s.pointFromPin(.active, .{ .node = s.pages.first.?, .y = 30, - .x = 2, + .x = .{ .col = 2 }, }).?); } @@ -3813,7 +3879,7 @@ test "PageList pointFromPin active with history" { try testing.expect(s.pointFromPin(.active, .{ .node = s.pages.first.?, .y = 21, - .x = 2, + .x = .{ .col = 2 }, }) == null); } } @@ -3846,7 +3912,7 @@ test "PageList pointFromPin active from prior page" { }, s.pointFromPin(.active, .{ .node = s.pages.last.?, .y = 0, - .x = 2, + .x = .{ .col = 2 }, }).?); } @@ -3855,7 +3921,7 @@ test "PageList pointFromPin active from prior page" { try testing.expect(s.pointFromPin(.active, .{ .node = s.pages.first.?, .y = 0, - .x = 0, + .x = .{ .col = 0 }, }) == null); } } @@ -3893,7 +3959,7 @@ test "PageList pointFromPin traverse pages" { }, s.pointFromPin(.screen, .{ .node = s.pages.last.?.prev.?, .y = 5, - .x = 2, + .x = .{ .col = 2 }, }).?); } @@ -3902,7 +3968,7 @@ test "PageList pointFromPin traverse pages" { try testing.expect(s.pointFromPin(.active, .{ .node = s.pages.first.?, .y = 0, - .x = 0, + .x = .{ .col = 0 }, }) == null); } } @@ -4402,7 +4468,7 @@ test "PageList grow prune scrollback" { // Our tracked pin should point to the top-left of the first page try testing.expect(p.node == s.pages.first.?); - try testing.expect(p.x == 0); + try testing.expect(p.x.col == 0); try testing.expect(p.y == 0); } @@ -4912,7 +4978,7 @@ test "PageList erase row with tracked pin resets to top-left" { // Our pin should move to the first page try testing.expectEqual(s.pages.first.?, p.node); try testing.expectEqual(@as(usize, 0), p.y); - try testing.expectEqual(@as(usize, 0), p.x); + try testing.expectEqual(@as(usize, 0), p.x.col); } test "PageList erase row with tracked pin shifts" { @@ -4933,7 +4999,7 @@ test "PageList erase row with tracked pin shifts" { // Our pin should move to the first page try testing.expectEqual(s.pages.first.?, p.node); try testing.expectEqual(@as(usize, 0), p.y); - try testing.expectEqual(@as(usize, 2), p.x); + try testing.expectEqual(@as(usize, 2), p.x.col); } test "PageList erase row with tracked pin is erased" { @@ -4954,7 +5020,7 @@ test "PageList erase row with tracked pin is erased" { // Our pin should move to the first page try testing.expectEqual(s.pages.first.?, p.node); try testing.expectEqual(@as(usize, 0), p.y); - try testing.expectEqual(@as(usize, 0), p.x); + try testing.expectEqual(@as(usize, 0), p.x.col); } test "PageList erase resets viewport to active if moves within active" { @@ -5113,15 +5179,15 @@ test "PageList eraseRowBounded less than full row" { try testing.expectEqual(s.pages.first.?, p_top.node); try testing.expectEqual(@as(usize, 4), p_top.y); - try testing.expectEqual(@as(usize, 0), p_top.x); + try testing.expectEqual(@as(usize, 0), p_top.x.col); try testing.expectEqual(s.pages.first.?, p_bot.node); try testing.expectEqual(@as(usize, 7), p_bot.y); - try testing.expectEqual(@as(usize, 0), p_bot.x); + try testing.expectEqual(@as(usize, 0), p_bot.x.col); try testing.expectEqual(s.pages.first.?, p_out.node); try testing.expectEqual(@as(usize, 9), p_out.y); - try testing.expectEqual(@as(usize, 0), p_out.x); + try testing.expectEqual(@as(usize, 0), p_out.x.col); } test "PageList eraseRowBounded with pin at top" { @@ -5147,7 +5213,7 @@ test "PageList eraseRowBounded with pin at top" { try testing.expectEqual(s.pages.first.?, p_top.node); try testing.expectEqual(@as(usize, 0), p_top.y); - try testing.expectEqual(@as(usize, 0), p_top.x); + try testing.expectEqual(@as(usize, 0), p_top.x.col); } test "PageList eraseRowBounded full rows single page" { @@ -5177,11 +5243,11 @@ test "PageList eraseRowBounded full rows single page" { // Our pin should move to the first page try testing.expectEqual(s.pages.first.?, p_in.node); try testing.expectEqual(@as(usize, 6), p_in.y); - try testing.expectEqual(@as(usize, 0), p_in.x); + try testing.expectEqual(@as(usize, 0), p_in.x.col); try testing.expectEqual(s.pages.first.?, p_out.node); try testing.expectEqual(@as(usize, 8), p_out.y); - try testing.expectEqual(@as(usize, 0), p_out.x); + try testing.expectEqual(@as(usize, 0), p_out.x.col); } test "PageList eraseRowBounded full rows two pages" { @@ -5215,19 +5281,19 @@ test "PageList eraseRowBounded full rows two pages" { { try testing.expectEqual(s.pages.last.?.prev.?, p_first.node); try testing.expectEqual(@as(usize, p_first.node.data.size.rows - 1), p_first.y); - try testing.expectEqual(@as(usize, 0), p_first.x); + try testing.expectEqual(@as(usize, 0), p_first.x.col); try testing.expectEqual(s.pages.last.?.prev.?, p_first_out.node); try testing.expectEqual(@as(usize, p_first_out.node.data.size.rows - 2), p_first_out.y); - try testing.expectEqual(@as(usize, 0), p_first_out.x); + try testing.expectEqual(@as(usize, 0), p_first_out.x.col); try testing.expectEqual(s.pages.last.?, p_in.node); try testing.expectEqual(@as(usize, 3), p_in.y); - try testing.expectEqual(@as(usize, 0), p_in.x); + try testing.expectEqual(@as(usize, 0), p_in.x.col); try testing.expectEqual(s.pages.last.?, p_out.node); try testing.expectEqual(@as(usize, 4), p_out.y); - try testing.expectEqual(@as(usize, 0), p_out.x); + try testing.expectEqual(@as(usize, 0), p_out.x.col); } // Erase only a few rows in our active @@ -5243,22 +5309,22 @@ test "PageList eraseRowBounded full rows two pages" { // In page in first page is shifted try testing.expectEqual(s.pages.last.?.prev.?, p_first.node); try testing.expectEqual(@as(usize, p_first.node.data.size.rows - 2), p_first.y); - try testing.expectEqual(@as(usize, 0), p_first.x); + try testing.expectEqual(@as(usize, 0), p_first.x.col); // Out page in first page should not be shifted try testing.expectEqual(s.pages.last.?.prev.?, p_first_out.node); try testing.expectEqual(@as(usize, p_first_out.node.data.size.rows - 2), p_first_out.y); - try testing.expectEqual(@as(usize, 0), p_first_out.x); + try testing.expectEqual(@as(usize, 0), p_first_out.x.col); // In page is shifted try testing.expectEqual(s.pages.last.?, p_in.node); try testing.expectEqual(@as(usize, 2), p_in.y); - try testing.expectEqual(@as(usize, 0), p_in.x); + try testing.expectEqual(@as(usize, 0), p_in.x.col); // Out page is not shifted try testing.expectEqual(s.pages.last.?, p_out.node); try testing.expectEqual(@as(usize, 4), p_out.y); - try testing.expectEqual(@as(usize, 0), p_out.x); + try testing.expectEqual(@as(usize, 0), p_out.x.col); } test "PageList clone" { @@ -7484,7 +7550,7 @@ test "PageList resize reflow less cols no wrapped rows" { while (it.next()) |offset| { for (0..4) |x| { var offset_copy = offset; - offset_copy.x = @intCast(x); + offset_copy.x = .{ .col = @intCast(x) }; const rac = offset_copy.rowAndCell(); const cells = offset.node.data.getCells(rac.row); try testing.expectEqual(@as(usize, 5), cells.len); @@ -8177,7 +8243,7 @@ test "PageList resize reflow less cols copy style" { while (it.next()) |offset| { for (0..s.cols - 1) |x| { var offset_copy = offset; - offset_copy.x = @intCast(x); + offset_copy.x = .{ .col = @intCast(x) }; const rac = offset_copy.rowAndCell(); const style_id = rac.cell.style_id; try testing.expect(style_id != 0); @@ -8337,7 +8403,7 @@ test "PageList resize reflow less cols copy kitty placeholder" { while (it.next()) |offset| { for (0..s.cols - 1) |x| { var offset_copy = offset; - offset_copy.x = @intCast(x); + offset_copy.x = .{ .col = @intCast(x) }; const rac = offset_copy.rowAndCell(); const row = rac.row; @@ -8441,7 +8507,7 @@ test "PageList reset" { try testing.expectEqual(Pin{ .node = s.pages.first.?, .y = 0, - .x = 0, + .x = .{ .col = 0 }, }, s.getTopLeft(.active)); } @@ -8485,6 +8551,6 @@ test "PageList clears history" { try testing.expectEqual(Pin{ .node = s.pages.first.?, .y = 0, - .x = 0, + .x = .{ .col = 0 }, }, s.getTopLeft(.active)); } diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 9ab4b23e2..659dfcb5c 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -268,7 +268,8 @@ pub fn reset(self: *Screen) void { // our cursor pin, which should be at the top-left already. const cursor_pin: *PageList.Pin = self.cursor.page_pin; assert(cursor_pin.node == self.pages.pages.first.?); - assert(cursor_pin.x == 0); + assert(cursor_pin.x == .col); + assert(cursor_pin.x.col == 0); assert(cursor_pin.y == 0); const cursor_rac = cursor_pin.rowAndCell(); self.cursor.deinit(self.alloc); @@ -536,7 +537,7 @@ pub fn cursorCellEndOfPrev(self: *Screen) *pagepkg.Cell { assert(self.cursor.y > 0); var page_pin = self.cursor.page_pin.up(1).?; - page_pin.x = self.pages.cols - 1; + page_pin.x = .{ .col = self.pages.cols - 1 }; const page_rac = page_pin.rowAndCell(); return page_rac.cell; } @@ -549,7 +550,7 @@ pub fn cursorRight(self: *Screen, n: size.CellCountInt) void { const cell: [*]pagepkg.Cell = @ptrCast(self.cursor.page_cell); self.cursor.page_cell = @ptrCast(cell + n); - self.cursor.page_pin.x += n; + self.cursor.page_pin.x = .{ .col = self.cursor.page_pin.xInt() + n }; self.cursor.x += n; } @@ -560,7 +561,7 @@ pub fn cursorLeft(self: *Screen, n: size.CellCountInt) void { const cell: [*]pagepkg.Cell = @ptrCast(self.cursor.page_cell); self.cursor.page_cell = @ptrCast(cell - n); - self.cursor.page_pin.x -= n; + self.cursor.page_pin.x = .{ .col = self.cursor.page_pin.xInt() - n }; self.cursor.x -= n; } @@ -611,7 +612,7 @@ pub fn cursorHorizontalAbsolute(self: *Screen, x: size.CellCountInt) void { assert(x < self.pages.cols); defer self.assertIntegrity(); - self.cursor.page_pin.x = x; + self.cursor.page_pin.x = .{ .col = x }; const page_rac = self.cursor.page_pin.rowAndCell(); self.cursor.page_cell = page_rac.cell; self.cursor.x = x; @@ -629,7 +630,7 @@ pub fn cursorAbsolute(self: *Screen, x: size.CellCountInt, y: size.CellCountInt) self.cursor.page_pin.down(y - self.cursor.y).? else self.cursor.page_pin.*; - page_pin.x = x; + page_pin.x = .{ .col = x }; self.cursor.x = x; // Must be set before cursorChangePin self.cursor.y = y; self.cursorChangePin(page_pin); @@ -746,7 +747,7 @@ pub fn cursorDownScroll(self: *Screen) !void { self.cursor.page_pin.down(1).? else reuse: { var pin = self.cursor.page_pin.*; - pin.x = self.cursor.x; + pin.x = .{ .col = self.cursor.x }; break :reuse pin; }; @@ -2161,7 +2162,7 @@ pub fn selectionString(self: *Screen, alloc: Allocator, opts: SelectionString) ! const sel_start: Pin = start: { var start: Pin = sel_ordered.start(); const cell = start.rowAndCell().cell; - if (cell.wide == .spacer_tail) start.x -= 1; + if (cell.wide == .spacer_tail) start.x = .{ .col = start.x.col - 1 }; break :start start; }; const sel_end: Pin = end: { @@ -2171,12 +2172,12 @@ pub fn selectionString(self: *Screen, alloc: Allocator, opts: SelectionString) ! .narrow, .wide => {}, // We can omit the tail - .spacer_tail => end.x -= 1, + .spacer_tail => end.x.col -= 1, // With the head we want to include the wrapped wide character. .spacer_head => if (end.down(1)) |p| { end = p; - end.x = 0; + end.x = .{ .col = 0 }; }, } break :end end; @@ -2190,12 +2191,15 @@ pub fn selectionString(self: *Screen, alloc: Allocator, opts: SelectionString) ! const start_x = if ((row_i == 0 or sel_ordered.rectangle) and sel_start.node == chunk.node) - sel_start.x + sel_start.xInt() else 0; const end_x = if ((row_i == rows.len - 1 or sel_ordered.rectangle) and sel_end.node == chunk.node) - sel_end.x + 1 + switch (sel_end.x) { + .col => |x| x + 1, + .neg => 0, + } else self.pages.cols; @@ -2217,7 +2221,7 @@ pub fn selectionString(self: *Screen, alloc: Allocator, opts: SelectionString) ! for (0..encode_len) |_| try b.append(.{ .node = chunk.node, .y = @intCast(y), - .x = @intCast(x), + .x = .{ .col = @intCast(x) }, }); } } @@ -2230,7 +2234,7 @@ pub fn selectionString(self: *Screen, alloc: Allocator, opts: SelectionString) ! for (0..encode_len) |_| try b.append(.{ .node = chunk.node, .y = @intCast(y), - .x = @intCast(x), + .x = .{ .col = @intCast(x) }, }); } } @@ -2246,7 +2250,7 @@ pub fn selectionString(self: *Screen, alloc: Allocator, opts: SelectionString) ! if (mapbuilder) |*b| try b.append(.{ .node = chunk.node, .y = @intCast(y), - .x = chunk.node.data.size.cols - 1, + .x = .{ .col = chunk.node.data.size.cols - 1 }, }); } } @@ -2341,7 +2345,7 @@ pub fn selectLine(self: *const Screen, opts: SelectLine) ?Selection { if (!row.wrap) { var copy = it_prev; - copy.x = 0; + copy.x = .{ .col = 0 }; break :start_pin copy; } @@ -2350,7 +2354,7 @@ pub fn selectLine(self: *const Screen, opts: SelectLine) ?Selection { const current_prompt = row.semantic_prompt.promptOrInput(); if (current_prompt != v) { var copy = it_prev; - copy.x = 0; + copy.x = .{ .col = 0 }; break :start_pin copy; } } @@ -2358,7 +2362,7 @@ pub fn selectLine(self: *const Screen, opts: SelectLine) ?Selection { it_prev = p; } else { var copy = it_prev; - copy.x = 0; + copy.x = .{ .col = 0 }; break :start_pin copy; } }; @@ -2374,14 +2378,14 @@ pub fn selectLine(self: *const Screen, opts: SelectLine) ?Selection { const current_prompt = row.semantic_prompt.promptOrInput(); if (current_prompt != v) { var prev = p.up(1).?; - prev.x = p.node.data.size.cols - 1; + prev.x = .{ .col = p.node.data.size.cols - 1 }; break :end_pin prev; } } if (!row.wrap) { var copy = p; - copy.x = p.node.data.size.cols - 1; + copy.x = .{ .col = p.node.data.size.cols - 1 }; break :end_pin copy; } } @@ -2583,7 +2587,7 @@ pub fn selectWord(self: *Screen, pin: Pin) ?Selection { // If we are going to the next row and it isn't wrapped, we // return the previous. - if (p.x == p.node.data.size.cols - 1 and !rac.row.wrap) { + if (p.xInt() == p.node.data.size.cols - 1 and !rac.row.wrap) { break :end p; } @@ -2603,7 +2607,7 @@ pub fn selectWord(self: *Screen, pin: Pin) ?Selection { // If we are going to the next row and it isn't wrapped, we // return the previous. - if (p.x == p.node.data.size.cols - 1 and !rac.row.wrap) { + if (p.xInt() == p.node.data.size.cols - 1 and !rac.row.wrap) { break :start prev; } @@ -2657,7 +2661,7 @@ pub fn selectOutput(self: *Screen, pin: Pin) ?Selection { switch (row.semantic_prompt) { .input, .prompt_continuation, .prompt => { var copy = it_prev; - copy.x = it_prev.node.data.size.cols - 1; + copy.x = .{ .col = it_prev.node.data.size.cols - 1 }; break :boundary copy; }, else => {}, @@ -2673,7 +2677,7 @@ pub fn selectOutput(self: *Screen, pin: Pin) ?Selection { const cells = p.node.data.getCells(row); if (Cell.hasTextAny(cells)) { var copy = p; - copy.x = p.node.data.size.cols - 1; + copy.x = .{ .col = p.node.data.size.cols - 1 }; break :boundary copy; } } @@ -2787,7 +2791,7 @@ pub fn selectPrompt(self: *Screen, pin: Pin) ?Selection { const end: Pin = end: { var it = pin.rowIterator(.right_down, null); var it_prev = it.next().?; - it_prev.x = it_prev.node.data.size.cols - 1; + it_prev.x = .{ .col = it_prev.node.data.size.cols - 1 }; while (it.next()) |p| { const row = p.rowAndCell().row; switch (row.semantic_prompt) { @@ -2799,7 +2803,7 @@ pub fn selectPrompt(self: *Screen, pin: Pin) ?Selection { } it_prev = p; - it_prev.x = it_prev.node.data.size.cols - 1; + it_prev.x = .{ .col = it_prev.node.data.size.cols - 1 }; } break :end it_prev; @@ -8362,6 +8366,57 @@ test "Screen: selectionString end outside of written area" { } } +test "Screen: selectionString end at left side of area" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 5, 10, 0); + defer s.deinit(); + const str = "1ABCD\n2EFGH\n3IJKL\n4MNOP\n5QRST\n6UVWX"; + try s.testWriteString(str); + + { + const sel = Selection.init( + s.pages.pin(.{ .screen = .{ .x = 0, .y = 2 } }).?, + s.pages.pin(.{ .screen = .{ .x = 0, .y = 4 } }).?, + false, + ); + const contents = try s.selectionString(alloc, .{ + .sel = sel, + .trim = true, + }); + defer alloc.free(contents); + const expected = "3IJKL\n4MNOP\n5"; + try testing.expectEqualStrings(expected, contents); + } +} + +test "Screen: selectionString end left of area" { + const testing = std.testing; + const alloc = testing.allocator; + + var s = try init(alloc, 5, 10, 0); + defer s.deinit(); + const str = "1ABCD\n2EFGH\n3IJKL\n4MNOP\n5QRST\n6UVWX"; + try s.testWriteString(str); + + { + var sel = Selection.init( + s.pages.pin(.{ .screen = .{ .x = 0, .y = 2 } }).?, + s.pages.pin(.{ .screen = .{ .x = 0, .y = 4 } }).?, + false, + ); + sel.endPtr().x = .neg; + const contents = try s.selectionString(alloc, .{ + .sel = sel, + .trim = true, + }); + defer alloc.free(contents); + const expected = "3IJKL\n4MNOP"; + try testing.expectEqualStrings(expected, contents); + } +} + test "Screen: selectionString trim space" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/Selection.zig b/src/terminal/Selection.zig index a90595d20..03ad35b37 100644 --- a/src/terminal/Selection.zig +++ b/src/terminal/Selection.zig @@ -217,7 +217,7 @@ pub fn order(self: Selection, s: *const Screen) Order { if (start_pt.y < end_pt.y) return .forward; if (start_pt.y > end_pt.y) return .reverse; - if (start_pt.x <= end_pt.x) return .forward; + if (self.start().x.lessEq(self.end().x)) return .forward; return .reverse; } @@ -266,13 +266,13 @@ pub fn contains(self: Selection, s: *const Screen, pin: Pin) bool { // If tl/br are same line if (tl.y == br.y) return p.y == tl.y and p.x >= tl.x and - p.x <= br.x; + pin.x.lessEq(br_pin.x); // If on top line, just has to be left of X - if (p.y == tl.y) return p.x >= tl.x; + if (p.y == tl.y) return tl_pin.x.lessEq(pin.x); // If on bottom line, just has to be right of X - if (p.y == br.y) return p.x <= br.x; + if (p.y == br.y) return pin.x.lessEq(br_pin.x); // If between the top/bottom, always good. return p.y > tl.y and p.y < br.y; @@ -437,15 +437,15 @@ pub fn adjust( const cells = next.node.data.getCells(rac.row); if (page.Cell.hasTextAny(cells)) { end_pin.* = next; - end_pin.x = @intCast(cells.len - 1); + end_pin.x = .{ .col = @intCast(cells.len - 1) }; break; } } }, - .beginning_of_line => end_pin.x = 0, + .beginning_of_line => end_pin.x = .{ .col = 0 }, - .end_of_line => end_pin.x = end_pin.node.data.size.cols - 1, + .end_of_line => end_pin.x = .{ .col = end_pin.node.data.size.cols - 1 }, } } diff --git a/src/terminal/kitty/graphics_exec.zig b/src/terminal/kitty/graphics_exec.zig index 25c819b10..78d161a51 100644 --- a/src/terminal/kitty/graphics_exec.zig +++ b/src/terminal/kitty/graphics_exec.zig @@ -272,7 +272,7 @@ fn display( terminal.setCursorPos( terminal.screen.cursor.y, - pin.x + size.cols + 1, + pin.x.col + size.cols + 1, ); }, }, diff --git a/src/terminal/kitty/graphics_storage.zig b/src/terminal/kitty/graphics_storage.zig index 0c3022e4a..3e8d87a6c 100644 --- a/src/terminal/kitty/graphics_storage.zig +++ b/src/terminal/kitty/graphics_storage.zig @@ -331,7 +331,7 @@ pub const ImageStorage = struct { while (it.next()) |entry| { const img = self.imageById(entry.key_ptr.image_id) orelse continue; const rect = entry.value_ptr.rect(img, t) orelse continue; - if (rect.top_left.x <= x and rect.bottom_right.x >= x) { + if (rect.top_left.x.col <= x and rect.bottom_right.x.col >= x) { entry.value_ptr.deinit(&t.screen); self.placements.removeByPtr(entry.key_ptr); if (v.delete) self.deleteIfUnused(alloc, img.id); @@ -790,13 +790,13 @@ pub const ImageStorage = struct { .offset => |v| v, .overflow => |v| v.end, }; - br.x = @min( + br.x = .{ .col = @min( // We need to sub one here because the x value is // one width already. So if the image is width "1" // then we add zero to X because X itself is width 1. - pin.x + (grid_size.cols - 1), + pin.x.col + (grid_size.cols - 1), t.cols - 1, - ); + ) }; return .{ .top_left = pin.*, diff --git a/src/terminal/kitty/graphics_unicode.zig b/src/terminal/kitty/graphics_unicode.zig index a4a25e751..652960509 100644 --- a/src/terminal/kitty/graphics_unicode.zig +++ b/src/terminal/kitty/graphics_unicode.zig @@ -48,12 +48,12 @@ pub const PlacementIterator = struct { // Iterate over our remaining cells and find one with a placeholder. const cells = row.cells(.right); - for (cells, row.x..) |*cell, x| { + for (cells, row.xInt()..) |*cell, x| { // "row" now points to the top-left pin of the placement. // We need this temporary state to build our incomplete // placement. assert(@intFromPtr(row) == @intFromPtr(&self.row)); - row.x = @intCast(x); + row.x = .{ .col = @intCast(x) }; // If this cell doesn't have the placeholder, then we // complete the run if we have it otherwise we just move diff --git a/src/terminal/search.zig b/src/terminal/search.zig index 56b181c48..2c5e653ac 100644 --- a/src/terminal/search.zig +++ b/src/terminal/search.zig @@ -397,7 +397,7 @@ const SlidingWindow = struct { return .{ .node = meta.node, .y = map.y, - .x = map.x, + .x = .{ .col = map.x }, }; }