fix: apply variation selectors to preceding codepoint (#12596)

This fixes a bug where the variation selectors (VS15 & VS16) were
checked against the first codepoint in a cell instead of the previous
codepoint in the cell's grapheme cluster, causing them to be dropped if
that first codepoint was not a valid base.
This commit is contained in:
Mitchell Hashimoto
2026-05-22 09:02:48 -07:00
committed by GitHub

View File

@@ -377,20 +377,20 @@ pub fn print(self: *Terminal, c: u21) !void {
// necessarily a grapheme break.
if (prev.cell.codepoint() == 0) break :grapheme;
var previous_codepoint: u21 = prev.cell.content.codepoint;
const grapheme_break = brk: {
var state: uucode.grapheme.BreakState = .default;
var cp1: u21 = prev.cell.content.codepoint;
if (prev.cell.hasGrapheme()) {
const cps = self.screens.active.cursor.page_pin.node.data.lookupGrapheme(prev.cell).?;
for (cps) |cp2| {
// log.debug("cp1={x} cp2={x}", .{ cp1, cp2 });
assert(!unicode.graphemeBreak(cp1, cp2, &state));
cp1 = cp2;
// log.debug("cp1={x} cp2={x}", .{ previous_codepoint, cp2 });
assert(!unicode.graphemeBreak(previous_codepoint, cp2, &state));
previous_codepoint = cp2;
}
}
// log.debug("cp1={x} cp2={x} end", .{ cp1, c });
break :brk unicode.graphemeBreak(cp1, c, &state);
// log.debug("cp1={x} cp2={x} end", .{ previous_codepoint, c });
break :brk unicode.graphemeBreak(previous_codepoint, c, &state);
};
// If we can NOT break, this means that "c" is part of a grapheme
@@ -402,7 +402,7 @@ pub fn print(self: *Terminal, c: u21) !void {
// the cell width accordingly. VS16 makes the character wide and
// VS15 makes it narrow.
if (c == 0xFE0F or c == 0xFE0E) {
const prev_props = unicode.table.get(prev.cell.content.codepoint);
const prev_props = unicode.table.get(previous_codepoint);
// Check if it is a valid variation sequence in
// emoji-variation-sequences.txt, and if not, ignore the char.
if (!prev_props.emoji_vs_base) return;
@@ -3318,7 +3318,7 @@ test "Terminal: zero-width character at start" {
try testing.expect(!t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } }));
}
// https://github.com/ghostty-org/ghostty/issues/12581
// https://github.com/ghostty-org/ghostty/pull/12581
test "Terminal: zero-width character attaches to pending wrap cell" {
var t = try init(testing.allocator, .{ .cols = 2, .rows = 2 });
defer t.deinit(testing.allocator);
@@ -3741,6 +3741,27 @@ test "Terminal: invalid VS16 doesn't mark dirty" {
try testing.expect(!t.isDirty(.{ .screen = .{ .x = 0, .y = 0 } }));
}
// https://github.com/ghostty-org/ghostty/pull/12596
test "Terminal: variation selectors apply to preceding codepoint" {
var t = try init(testing.allocator, .{ .cols = 5, .rows = 5 });
defer t.deinit(testing.allocator);
// Enable grapheme clustering
t.modes.set(.grapheme_cluster, true);
// Pirate flag: black flag + ZWJ + skull and crossbones + VS16.
try t.print(0x1F3F4);
try t.print(0x200D);
try t.print(0x2620);
try t.print(0xFE0F);
const list_cell = t.screens.active.pages.getCell(.{ .screen = .{ .x = 0, .y = 0 } }).?;
const cell = list_cell.cell;
try testing.expectEqual(@as(u21, 0x1F3F4), cell.content.codepoint);
try testing.expect(cell.hasGrapheme());
try testing.expectEqualSlices(u21, &.{ 0x200D, 0x2620, 0xFE0F }, list_cell.node.data.lookupGrapheme(cell).?);
}
test "Terminal: print multicodepoint grapheme, mode 2027" {
var t = try init(testing.allocator, .{ .cols = 80, .rows = 80 });
defer t.deinit(testing.allocator);