terminal: only apply VS15/16 to emoji

Fixes #1482
This commit is contained in:
Mitchell Hashimoto
2024-02-10 17:26:45 -08:00
parent 6d5e73fd75
commit 004405ccf9
2 changed files with 144 additions and 40 deletions

View File

@ -806,8 +806,12 @@ pub fn print(self: *Terminal, c: u21) !void {
// If this is an emoji variation selector then we need to modify // If this is an emoji variation selector then we need to modify
// the cell width accordingly. VS16 makes the character wide and // the cell width accordingly. VS16 makes the character wide and
// VS15 makes it narrow. // VS15 makes it narrow.
// if (c == 0xFE0F or c == 0xFE0E) {
// TODO: This should use the emoji-variation-selector.txt UCD file. // This only applies to emoji
const prev_props = unicode.getProperties(@intCast(prev.cell.char));
const emoji = prev_props.grapheme_boundary_class == .extended_pictographic;
if (!emoji) return;
switch (c) { switch (c) {
0xFE0F => wide: { 0xFE0F => wide: {
if (prev.cell.attrs.wide) break :wide; if (prev.cell.attrs.wide) break :wide;
@ -854,7 +858,8 @@ pub fn print(self: *Terminal, c: u21) !void {
break :narrow; break :narrow;
}, },
else => {}, else => unreachable,
}
} }
log.debug("c={x} grapheme attach to x={}", .{ c, prev.x }); log.debug("c={x} grapheme attach to x={}", .{ c, prev.x });
@ -900,6 +905,14 @@ pub fn print(self: *Terminal, c: u21) !void {
break :prev x - 1; break :prev x - 1;
}; };
// If this is a emoji variation selector, prev must be an emoji
if (c == 0xFE0F or c == 0xFE0E) {
const prev_cell = row.getCellPtr(prev);
const prev_props = unicode.getProperties(@intCast(prev_cell.char));
const emoji = prev_props.grapheme_boundary_class == .extended_pictographic;
if (!emoji) return;
}
try row.attachGrapheme(prev, c); try row.attachGrapheme(prev, c);
return; return;
} }
@ -2508,6 +2521,96 @@ test "Terminal: print multicodepoint grapheme, mode 2027" {
} }
} }
test "Terminal: print invalid VS16 non-grapheme" {
var t = try init(testing.allocator, 80, 80);
defer t.deinit(testing.allocator);
// https://github.com/mitchellh/ghostty/issues/1482
try t.print('x');
try t.print(0xFE0F);
// We should have 2 cells taken up. It is one character but "wide".
try testing.expectEqual(@as(usize, 0), t.screen.cursor.y);
try testing.expectEqual(@as(usize, 1), t.screen.cursor.x);
// Assert various properties about our screen to verify
// we have all expected cells.
const row = t.screen.getRow(.{ .screen = 0 });
{
const cell = row.getCell(0);
try testing.expectEqual(@as(u32, 'x'), cell.char);
try testing.expect(!cell.attrs.wide);
try testing.expectEqual(@as(usize, 1), row.codepointLen(0));
}
{
const cell = row.getCell(1);
try testing.expectEqual(@as(u32, 0), cell.char);
}
}
test "Terminal: print invalid VS16 grapheme" {
var t = try init(testing.allocator, 80, 80);
defer t.deinit(testing.allocator);
// Enable grapheme clustering
t.modes.set(.grapheme_cluster, true);
// https://github.com/mitchellh/ghostty/issues/1482
try t.print('x');
try t.print(0xFE0F);
// We should have 2 cells taken up. It is one character but "wide".
try testing.expectEqual(@as(usize, 0), t.screen.cursor.y);
try testing.expectEqual(@as(usize, 1), t.screen.cursor.x);
// Assert various properties about our screen to verify
// we have all expected cells.
const row = t.screen.getRow(.{ .screen = 0 });
{
const cell = row.getCell(0);
try testing.expectEqual(@as(u32, 'x'), cell.char);
try testing.expect(!cell.attrs.wide);
try testing.expectEqual(@as(usize, 1), row.codepointLen(0));
}
{
const cell = row.getCell(1);
try testing.expectEqual(@as(u32, 0), cell.char);
}
}
test "Terminal: print invalid VS16 with second char" {
var t = try init(testing.allocator, 80, 80);
defer t.deinit(testing.allocator);
// Enable grapheme clustering
t.modes.set(.grapheme_cluster, true);
// https://github.com/mitchellh/ghostty/issues/1482
try t.print('x');
try t.print(0xFE0F);
try t.print('y');
// We should have 2 cells taken up. It is one character but "wide".
try testing.expectEqual(@as(usize, 0), t.screen.cursor.y);
try testing.expectEqual(@as(usize, 2), t.screen.cursor.x);
// Assert various properties about our screen to verify
// we have all expected cells.
const row = t.screen.getRow(.{ .screen = 0 });
{
const cell = row.getCell(0);
try testing.expectEqual(@as(u32, 'x'), cell.char);
try testing.expect(!cell.attrs.wide);
try testing.expectEqual(@as(usize, 1), row.codepointLen(0));
}
{
const cell = row.getCell(1);
try testing.expectEqual(@as(u32, 'y'), cell.char);
try testing.expect(!cell.attrs.wide);
try testing.expectEqual(@as(usize, 1), row.codepointLen(0));
}
}
test "Terminal: soft wrap" { test "Terminal: soft wrap" {
var t = try init(testing.allocator, 3, 80); var t = try init(testing.allocator, 3, 80);
defer t.deinit(testing.allocator); defer t.deinit(testing.allocator);

View File

@ -4,6 +4,7 @@ const grapheme = @import("grapheme.zig");
const props = @import("props.zig"); const props = @import("props.zig");
pub const table = props.table; pub const table = props.table;
pub const Properties = props.Properties; pub const Properties = props.Properties;
pub const getProperties = props.get;
pub const graphemeBreak = grapheme.graphemeBreak; pub const graphemeBreak = grapheme.graphemeBreak;
pub const GraphemeBreakState = grapheme.BreakState; pub const GraphemeBreakState = grapheme.BreakState;