superrob1500
/
mirror-ghostty
mirror of https://github.com/ghostty-org/ghostty
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(font): Treat Powerline glyphs as normal characters for constraint width purposes (#8829) 

Powerline glyphs were treated as whitespace, giving the preceding cell a
constraint width of 2 and cutting off icons in people's prompts and
statuslines. It is however correct to not treat Powerline glyphs like
other Nerd Font symbols; they should simply be treated as normal
characters, just like their relatives in the block elements unicode
block.

This resolves
https://discord.com/channels/1005603569187160125/1417236683266592798
(never promoted to an issue, but real and easy to reproduce).

**Tip**
<img width="215" height="63" alt="Screenshot 2025-09-21 at 16 57 58"
src="https://github.com/user-attachments/assets/81e770c5-d688-4d8e-839c-1f4288703c06"
/>

**This PR**
<img width="215" height="63" alt="Screenshot 2025-09-21 at 16 58 42"
src="https://github.com/user-attachments/assets/5d2dd770-0314-46f6-99b5-237a0933998e"
/>

The constraint width logic was untested but contains some quite subtle
interactions, so I wrote a suite of tests covering the cases I'm aware
of.

While working on this code I also resolved a TODO comment to add all the
box drawing/block element type characters to the set of codepoints
excluded from the minimum contrast settings.
1.2.x
Mitchell Hashimoto 2025-09-29 12:16:23 -07:00
parent a905e14cc4
commit 7053f5a537
No known key found for this signature in database
GPG Key ID: 523D5DC389D273BC
2 changed files with 132 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
src/renderer/cell.zig
View File

@ -236,8 +236,8 @@ pub fn isCovering(cp: u21) bool {
}
/// Returns true of the codepoint is a "symbol-like" character, which
/// for now we define as anything in a private use area and anything
/// in several unicode blocks:
/// for now we define as anything in a private use area, except
/// the Powerline range, and anything in several unicode blocks:
/// - Dingbats
/// - Emoticons
/// - Miscellaneous Symbols
@ -249,11 +249,13 @@ pub fn isCovering(cp: u21) bool {
/// In the future it may be prudent to expand this to encompass more
/// symbol-like characters, and/or exclude some PUA sections.
pub fn isSymbol(cp: u21) bool {
return symbols.get(cp);
return symbols.get(cp) and !isPowerline(cp);
}
/// Returns the appropriate `constraint_width` for
/// the provided cell when rendering its glyph(s).
///
/// Tested as part of the Screen tests.
pub fn constraintWidth(cell_pin: terminal.Pin) u2 {
const cell = cell_pin.rowAndCell().cell;
const cp = cell.codepoint();
@ -274,9 +276,7 @@ pub fn constraintWidth(cell_pin: terminal.Pin) u2 {
// If we have a previous cell and it was a symbol 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.x > 0) {
const prev_cp = prev_cp: {
var copy = cell_pin;
copy.x -= 1;
@ -284,9 +284,6 @@ pub fn constraintWidth(cell_pin: terminal.Pin) u2 {
break :prev_cp prev_cell.codepoint();
};
// We consider powerline glyphs whitespace.
if (isPowerline(prev_cp)) break :prev;
if (isSymbol(prev_cp)) {
return 1;
}
@ -300,10 +297,7 @@ pub fn constraintWidth(cell_pin: terminal.Pin) u2 {
const next_cell = copy.rowAndCell().cell;
break :next_cp next_cell.codepoint();
};
if (next_cp == 0 or
isSpace(next_cp) or
isPowerline(next_cp))
{
if (next_cp == 0 or isSpace(next_cp)) {
return 2;
}
@ -312,9 +306,10 @@ pub fn constraintWidth(cell_pin: terminal.Pin) u2 {
}
/// Whether min contrast should be disabled for a given glyph.
/// True for glyphs used for terminal graphics, such as box
/// drawing characters, block elements, and Powerline glyphs.
pub fn noMinContrast(cp: u21) bool {
// TODO: We should disable for all box drawing type characters.
return isPowerline(cp);
return isBoxDrawing(cp) or isBlockElement(cp) or isLegacyComputing(cp) or isPowerline(cp);
}
// Some general spaces, others intentionally kept
@ -328,10 +323,36 @@ fn isSpace(char: u21) bool {
};
}
// Returns true if the codepoint is a box drawing character.
fn isBoxDrawing(char: u21) bool {
return switch (char) {
0x2500...0x257F => true,
else => false,
};
}
// Returns true if the codepoint is a block element.
fn isBlockElement(char: u21) bool {
return switch (char) {
0x2580...0x259F => true,
else => false,
};
}
// Returns true if the codepoint is in a Symbols for Legacy
// Computing block, including supplements.
fn isLegacyComputing(char: u21) bool {
return switch (char) {
0x1FB00...0x1FBFF => true,
0x1CC00...0x1CEBF => true, // Supplement introduced in Unicode 16.0
else => false,
};
}
// Returns true if the codepoint is a part of the Powerline range.
fn isPowerline(char: u21) bool {
return switch (char) {
0xE0B0...0xE0C8, 0xE0CA, 0xE0CC...0xE0D2, 0xE0D4 => true,
0xE0B0...0xE0D7 => true,
else => false,
};
}

95
src/terminal/Screen.zig
View File

@ -14,6 +14,7 @@ const Selection = @import("Selection.zig");
const PageList = @import("PageList.zig");
const StringMap = @import("StringMap.zig");
const pagepkg = @import("page.zig");
const cellpkg = @import("../renderer/cell.zig");
const point = @import("point.zig");
const size = @import("size.zig");
const style = @import("style.zig");
@ -9094,3 +9095,97 @@ test "Screen UTF8 cell map with blank prefix" {
.y = 1,
}, cell_map.items[3]);
}
test "Screen cell constraint widths" {
const testing = std.testing;
const alloc = testing.allocator;
var s = try Screen.init(alloc, 4, 1, 0);
defer s.deinit();
// for each case, the numbers in the comment denote expected
// constraint widths for the symbol-containing cells
// symbol->nothing: 2
{
try s.testWriteString("");
const p0 = s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?;
try testing.expectEqual(2, cellpkg.constraintWidth(p0));
s.reset();
}
// symbol->character: 1
{
try s.testWriteString("z");
const p0 = s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?;
try testing.expectEqual(1, cellpkg.constraintWidth(p0));
s.reset();
}
// symbol->space: 2
{
try s.testWriteString(" z");
const p0 = s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?;
try testing.expectEqual(2, cellpkg.constraintWidth(p0));
s.reset();
}
// symbol->no-break space: 1
{
try s.testWriteString("\u{00a0}z");
const p0 = s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?;
try testing.expectEqual(1, cellpkg.constraintWidth(p0));
s.reset();
}
// symbol->end of row: 1
{
try s.testWriteString("");
const p3 = s.pages.pin(.{ .screen = .{ .x = 3, .y = 0 } }).?;
try testing.expectEqual(1, cellpkg.constraintWidth(p3));
s.reset();
}
// character->symbol: 2
{
try s.testWriteString("z");
const p1 = s.pages.pin(.{ .screen = .{ .x = 1, .y = 0 } }).?;
try testing.expectEqual(2, cellpkg.constraintWidth(p1));
s.reset();
}
// symbol->symbol: 1,1
{
try s.testWriteString("");
const p0 = s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?;
const p1 = s.pages.pin(.{ .screen = .{ .x = 1, .y = 0 } }).?;
try testing.expectEqual(1, cellpkg.constraintWidth(p0));
try testing.expectEqual(1, cellpkg.constraintWidth(p1));
s.reset();
}
// symbol->space->symbol: 2,2
{
try s.testWriteString(" ");
const p0 = s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?;
const p2 = s.pages.pin(.{ .screen = .{ .x = 2, .y = 0 } }).?;
try testing.expectEqual(2, cellpkg.constraintWidth(p0));
try testing.expectEqual(2, cellpkg.constraintWidth(p2));
s.reset();
}
// symbol->powerline: 1 (dedicated test because powerline is special-cased in cellpkg)
{
try s.testWriteString("");
const p0 = s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?;
try testing.expectEqual(1, cellpkg.constraintWidth(p0));
s.reset();
}
// powerline->symbol: 2 (dedicated test because powerline is special-cased in cellpkg)
{
try s.testWriteString("");
const p1 = s.pages.pin(.{ .screen = .{ .x = 1, .y = 0 } }).?;
try testing.expectEqual(2, cellpkg.constraintWidth(p1));
s.reset();
}
}