Merge pull request #130 from mitchellh/bug-chungus

Bug fixes
This commit is contained in:
Mitchell Hashimoto
2023-03-23 10:59:19 -07:00
committed by GitHub
7 changed files with 325 additions and 35 deletions

View File

@ -43,12 +43,22 @@ pub const Shaper = struct {
/// Returns an iterator that returns one text run at a time for the
/// given terminal row. Note that text runs are are only valid one at a time
/// for a Shaper struct since they share state.
///
/// The selection must be a row-only selection (height = 1). See
/// Selection.containedRow. The run iterator will ONLY look at X values
/// and assume the y value matches.
pub fn runIterator(
self: *Shaper,
group: *GroupCache,
row: terminal.Screen.Row,
selection: ?terminal.Selection,
) font.shape.RunIterator {
return .{ .hooks = .{ .shaper = self }, .group = group, .row = row };
return .{
.hooks = .{ .shaper = self },
.group = group,
.row = row,
.selection = selection,
};
}
/// Shape the given text run. The text run must be the immediately previous
@ -136,7 +146,7 @@ test "run iterator" {
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |_| count += 1;
try testing.expectEqual(@as(usize, 1), count);
@ -149,7 +159,7 @@ test "run iterator" {
try screen.testWriteString("ABCD EFG");
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |_| count += 1;
try testing.expectEqual(@as(usize, 1), count);
@ -163,7 +173,7 @@ test "run iterator" {
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |_| {
count += 1;
@ -197,7 +207,7 @@ test "run iterator: empty cells with background set" {
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -211,6 +221,7 @@ test "run iterator: empty cells with background set" {
try testing.expectEqual(@as(usize, 1), count);
}
}
test "shape" {
const testing = std.testing;
const alloc = testing.allocator;
@ -231,7 +242,7 @@ test "shape" {
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -254,7 +265,7 @@ test "shape inconsolata ligs" {
try screen.testWriteString(">=");
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -271,7 +282,7 @@ test "shape inconsolata ligs" {
try screen.testWriteString("===");
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -296,7 +307,7 @@ test "shape emoji width" {
try screen.testWriteString("👍");
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -330,7 +341,7 @@ test "shape emoji width long" {
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -361,7 +372,7 @@ test "shape variation selector VS15" {
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -392,7 +403,7 @@ test "shape variation selector VS16" {
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -420,7 +431,7 @@ test "shape with empty cells in between" {
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -452,7 +463,7 @@ test "shape Chinese characters" {
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -493,7 +504,7 @@ test "shape box glyphs" {
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }));
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), null);
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
@ -508,6 +519,99 @@ test "shape box glyphs" {
try testing.expectEqual(@as(usize, 1), count);
}
test "shape selection boundary" {
const testing = std.testing;
const alloc = testing.allocator;
var testdata = try testShaper(alloc);
defer testdata.deinit();
// Make a screen with some data
var screen = try terminal.Screen.init(alloc, 3, 10, 0);
defer screen.deinit();
try screen.testWriteString("a1b2c3d4e5");
// Full line selection
{
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), .{
.start = .{ .x = 0, .y = 0 },
.end = .{ .x = screen.cols - 1, .y = 0 },
});
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
_ = try shaper.shape(run);
}
try testing.expectEqual(@as(usize, 1), count);
}
// Offset x, goes to end of line selection
{
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), .{
.start = .{ .x = 2, .y = 0 },
.end = .{ .x = screen.cols - 1, .y = 0 },
});
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
_ = try shaper.shape(run);
}
try testing.expectEqual(@as(usize, 2), count);
}
// Offset x, starts at beginning of line
{
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), .{
.start = .{ .x = 0, .y = 0 },
.end = .{ .x = 3, .y = 0 },
});
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
_ = try shaper.shape(run);
}
try testing.expectEqual(@as(usize, 2), count);
}
// Selection only subset of line
{
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), .{
.start = .{ .x = 1, .y = 0 },
.end = .{ .x = 3, .y = 0 },
});
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
_ = try shaper.shape(run);
}
try testing.expectEqual(@as(usize, 3), count);
}
// Selection only one character
{
// Get our run iterator
var shaper = testdata.shaper;
var it = shaper.runIterator(testdata.cache, screen.getRow(.{ .screen = 0 }), .{
.start = .{ .x = 1, .y = 0 },
.end = .{ .x = 1, .y = 0 },
});
var count: usize = 0;
while (try it.next(alloc)) |run| {
count += 1;
_ = try shaper.shape(run);
}
try testing.expectEqual(@as(usize, 3), count);
}
}
const TestShaper = struct {
alloc: Allocator,
shaper: Shaper,

View File

@ -28,6 +28,7 @@ pub const RunIterator = struct {
hooks: font.Shaper.RunIteratorHook,
group: *font.GroupCache,
row: terminal.Screen.Row,
selection: ?terminal.Selection = null,
i: usize = 0,
pub fn next(self: *RunIterator, alloc: Allocator) !?TextRun {
@ -56,6 +57,16 @@ pub const RunIterator = struct {
const cluster = j;
const cell = self.row.getCell(j);
// If we have a selection and we're at a boundary point, then
// we break the run here.
if (self.selection) |unordered_sel| {
if (j > self.i) {
const sel = unordered_sel.ordered(.forward);
if (sel.start.x > 0 and j == sel.start.x) break;
if (sel.end.x > 0 and j == sel.end.x + 1) break;
}
}
// If we're a spacer, then we ignore it
if (cell.attrs.wide_spacer_tail) continue;

View File

@ -57,8 +57,14 @@ pub const Shaper = struct {
self: *Shaper,
group: *font.GroupCache,
row: terminal.Screen.Row,
selection: ?terminal.Selection,
) font.shape.RunIterator {
return .{ .hooks = .{ .shaper = self }, .group = group, .row = row };
return .{
.hooks = .{ .shaper = self },
.group = group,
.row = row,
.selection = selection,
};
}
/// Shape the given text run. The text run must be the immediately
@ -281,7 +287,7 @@ pub const Wasm = struct {
while (rowIter.next()) |row| {
defer y += 1;
var iter = self.runIterator(group, row);
var iter = self.runIterator(group, row, null);
while (try iter.next(alloc)) |run| {
const cells = try self.shape(run);
log.info("y={} run={d} shape={any} idx={}", .{

View File

@ -844,8 +844,24 @@ fn rebuildCells(
}
};
// We need to get this row's selection if there is one for proper
// run splitting.
const row_selection = sel: {
if (term_selection) |sel| {
const screen_point = (terminal.point.Viewport{
.x = 0,
.y = y,
}).toScreen(screen);
if (sel.containedRow(screen, screen_point)) |row_sel| {
break :sel row_sel;
}
}
break :sel null;
};
// Split our row into runs and shape each one.
var iter = self.font_shaper.runIterator(self.font_group, row);
var iter = self.font_shaper.runIterator(self.font_group, row, row_selection);
while (try iter.next(self.alloc)) |run| {
for (try self.font_shaper.shape(run)) |shaper_cell| {
if (self.updateCell(

View File

@ -847,8 +847,9 @@ pub fn rebuildCells(
defer y += 1;
// Our selection value is only non-null if this selection happens
// to contain this row. If the selection changes for any reason,
// then we invalidate the cache.
// to contain this row. This selection value will be set to only be
// the selection that contains this row. This way, if the selection
// changes but not for this line, we don't invalidate the cache.
const selection = sel: {
if (term_selection) |sel| {
const screen_point = (terminal.point.Viewport{
@ -856,8 +857,10 @@ pub fn rebuildCells(
.y = y,
}).toScreen(screen);
// If we are selected, we our colors are just inverted fg/bg
if (sel.containsRow(screen_point)) break :sel sel;
// If we are selected, we our colors are just inverted fg/bg.
if (sel.containedRow(screen, screen_point)) |row_sel| {
break :sel row_sel;
}
}
break :sel null;
@ -901,7 +904,7 @@ pub fn rebuildCells(
const start = self.cells.items.len;
// Split our row into runs and shape each one.
var iter = self.font_shaper.runIterator(self.font_group, row);
var iter = self.font_shaper.runIterator(self.font_group, row, selection);
while (try iter.next(self.alloc)) |run| {
for (try self.font_shaper.shape(run)) |shaper_cell| {
if (self.updateCell(

View File

@ -65,9 +65,6 @@ const fastmem = @import("../fastmem.zig");
const log = std.log.scoped(.screen);
/// Whitespace characters for selection purposes
const whitespace = &[_]u32{ 0, ' ', '\t' };
/// Cursor represents the cursor state.
pub const Cursor = struct {
// x, y where the cursor currently exists (0-indexed). This x/y is
@ -1157,6 +1154,9 @@ pub fn clearHistory(self: *Screen) void {
/// over whitespace but the line has non-whitespace characters elsewhere, the
/// line will be selected.
pub fn selectLine(self: *Screen, pt: point.ScreenPoint) ?Selection {
// Whitespace characters for selection purposes
const whitespace = &[_]u32{ 0, ' ', '\t' };
// Impossible to select anything outside of the area we've written.
const y_max = self.rowsWritten() - 1;
if (pt.y > y_max or pt.x >= self.cols) return null;
@ -1258,6 +1258,9 @@ pub fn selectLine(self: *Screen, pt: point.ScreenPoint) ?Selection {
/// This will return null if a selection is impossible. The only scenario
/// this happens is if the point pt is outside of the written screen space.
pub fn selectWord(self: *Screen, pt: point.ScreenPoint) ?Selection {
// Boundary characters for selection purposes
const boundary = &[_]u32{ 0, ' ', '\t', '\'', '"' };
// Impossible to select anything outside of the area we've written.
const y_max = self.rowsWritten() - 1;
if (pt.y > y_max) return null;
@ -1270,8 +1273,8 @@ pub fn selectWord(self: *Screen, pt: point.ScreenPoint) ?Selection {
// areas where the screen is not yet written.
if (start_cell.empty()) return null;
// Determine if we are whitespace or not to determine what our boundary is.
const expect_whitespace = std.mem.indexOfAny(u32, whitespace, &[_]u32{start_cell.char}) != null;
// Determine if we are a boundary or not to determine what our boundary is.
const expect_boundary = std.mem.indexOfAny(u32, boundary, &[_]u32{start_cell.char}) != null;
// Go forwards to find our end boundary
const end: point.ScreenPoint = boundary: {
@ -1290,12 +1293,12 @@ pub fn selectWord(self: *Screen, pt: point.ScreenPoint) ?Selection {
if (cell.empty()) break :boundary prev;
// If we do not match our expected set, we hit a boundary
const this_whitespace = std.mem.indexOfAny(
const this_boundary = std.mem.indexOfAny(
u32,
whitespace,
boundary,
&[_]u32{cell.char},
) != null;
if (this_whitespace != expect_whitespace) break :boundary prev;
if (this_boundary != expect_boundary) break :boundary prev;
// Increase our prev
prev.x = x;
@ -1324,12 +1327,12 @@ pub fn selectWord(self: *Screen, pt: point.ScreenPoint) ?Selection {
// we reach a boundary condition.
while (x > 0) : (x -= 1) {
const cell = current_row.getCell(x - 1);
const this_whitespace = std.mem.indexOfAny(
const this_boundary = std.mem.indexOfAny(
u32,
whitespace,
boundary,
&[_]u32{cell.char},
) != null;
if (this_whitespace != expect_whitespace) break :boundary prev;
if (this_boundary != expect_boundary) break :boundary prev;
// Update our prev
prev.x = x - 1;
@ -3368,6 +3371,53 @@ test "Screen: selectWord whitespace across soft-wrap" {
}
}
test "Screen: selectWord with single quote boundary" {
const testing = std.testing;
const alloc = testing.allocator;
var s = try init(alloc, 10, 20, 0);
defer s.deinit();
try s.testWriteString(" 'abc' \n123");
// Inside quotes forward
{
const sel = s.selectWord(.{ .x = 2, .y = 0 }).?;
try testing.expectEqual(@as(usize, 2), sel.start.x);
try testing.expectEqual(@as(usize, 0), sel.start.y);
try testing.expectEqual(@as(usize, 4), sel.end.x);
try testing.expectEqual(@as(usize, 0), sel.end.y);
}
// Inside quotes backward
{
const sel = s.selectWord(.{ .x = 4, .y = 0 }).?;
try testing.expectEqual(@as(usize, 2), sel.start.x);
try testing.expectEqual(@as(usize, 0), sel.start.y);
try testing.expectEqual(@as(usize, 4), sel.end.x);
try testing.expectEqual(@as(usize, 0), sel.end.y);
}
// Inside quotes bidirectional
{
const sel = s.selectWord(.{ .x = 3, .y = 0 }).?;
try testing.expectEqual(@as(usize, 2), sel.start.x);
try testing.expectEqual(@as(usize, 0), sel.start.y);
try testing.expectEqual(@as(usize, 4), sel.end.x);
try testing.expectEqual(@as(usize, 0), sel.end.y);
}
// On quote
// NOTE: this behavior is not ideal, so we can change this one day,
// but I think its also not that important compared to the above.
{
const sel = s.selectWord(.{ .x = 1, .y = 0 }).?;
try testing.expectEqual(@as(usize, 0), sel.start.x);
try testing.expectEqual(@as(usize, 0), sel.start.y);
try testing.expectEqual(@as(usize, 1), sel.end.x);
try testing.expectEqual(@as(usize, 0), sel.end.y);
}
}
test "Screen: scrollRegionUp single" {
const testing = std.testing;
const alloc = testing.allocator;

View File

@ -3,6 +3,7 @@
const Selection = @This();
const std = @import("std");
const assert = std.debug.assert;
const point = @import("point.zig");
const Screen = @import("Screen.zig");
const ScreenPoint = point.ScreenPoint;
@ -93,6 +94,42 @@ pub fn containsRow(self: Selection, p: ScreenPoint) bool {
return p.y >= tl.y and p.y <= br.y;
}
/// Get a selection for a single row in the screen. This will return null
/// if the row is not included in the selection.
pub fn containedRow(self: Selection, screen: *const Screen, p: ScreenPoint) ?Selection {
const tl = self.topLeft();
const br = self.bottomRight();
if (p.y < tl.y or p.y > br.y) return null;
if (p.y == tl.y) {
// If the selection is JUST this line, return it as-is.
if (p.y == br.y) return self;
// Selection top-left line matches only.
return .{
.start = tl,
.end = .{ .y = tl.y, .x = screen.cols - 1 },
};
}
// Row is our bottom selection, so we return the selection from the
// beginning of the line to the br. We know our selection is more than
// one line (due to conditionals above)
if (p.y == br.y) {
assert(p.y != tl.y);
return .{
.start = .{ .y = br.y, .x = 0 },
.end = br,
};
}
// Row is somewhere between our selection lines so we return the full line.
return .{
.start = .{ .y = p.y, .x = 0 },
.end = .{ .y = p.y, .x = screen.cols - 1 },
};
}
/// Returns the top left point of the selection.
pub fn topLeft(self: Selection) ScreenPoint {
return switch (self.order()) {
@ -109,8 +146,19 @@ pub fn bottomRight(self: Selection) ScreenPoint {
};
}
/// Returns the selection in the given order.
pub fn ordered(self: Selection, desired: Order) Selection {
if (self.order() == desired) return self;
const tl = self.topLeft();
const br = self.bottomRight();
return switch (desired) {
.forward => .{ .start = tl, .end = br },
.reverse => .{ .start = br, .end = tl },
};
}
/// The order of the selection (whether it is selecting forward or back).
const Order = enum { forward, reverse };
pub const Order = enum { forward, reverse };
fn order(self: Selection) Order {
if (self.start.y < self.end.y) return .forward;
@ -162,6 +210,58 @@ test "Selection: contains" {
}
}
test "Selection: containedRow" {
const testing = std.testing;
var screen = try Screen.init(testing.allocator, 5, 10, 0);
defer screen.deinit();
{
const sel: Selection = .{
.start = .{ .x = 5, .y = 1 },
.end = .{ .x = 3, .y = 3 },
};
// Not contained
try testing.expect(sel.containedRow(&screen, .{ .x = 1, .y = 4 }) == null);
// Start line
try testing.expectEqual(Selection{
.start = sel.start,
.end = .{ .x = screen.cols - 1, .y = 1 },
}, sel.containedRow(&screen, .{ .x = 1, .y = 1 }).?);
// End line
try testing.expectEqual(Selection{
.start = .{ .x = 0, .y = 3 },
.end = sel.end,
}, sel.containedRow(&screen, .{ .x = 2, .y = 3 }).?);
// Middle line
try testing.expectEqual(Selection{
.start = .{ .x = 0, .y = 2 },
.end = .{ .x = screen.cols - 1, .y = 2 },
}, sel.containedRow(&screen, .{ .x = 2, .y = 2 }).?);
}
// Single-line selection
{
const sel: Selection = .{
.start = .{ .x = 2, .y = 1 },
.end = .{ .x = 6, .y = 1 },
};
// Not contained
try testing.expect(sel.containedRow(&screen, .{ .x = 1, .y = 0 }) == null);
try testing.expect(sel.containedRow(&screen, .{ .x = 1, .y = 2 }) == null);
// Contained
try testing.expectEqual(Selection{
.start = sel.start,
.end = sel.end,
}, sel.containedRow(&screen, .{ .x = 1, .y = 1 }).?);
}
}
test "Selection: within" {
const testing = std.testing;
{