fix(font): Let powerline glyphs be wide (#8994)

#8829 fixed the interaction of Powerline glyphs with other symbols, but
regressed in the handling of the Powerline glyphs themselves by letting
them get caught in an early exit that imposes a constraint width of 1.
This PR fixes the regression and adds corresponding tests. Tried to be
somewhat principled about why the special treatment is warranted, hence
the new helper function `isGraphicsElement`.

**Before**
<img width="270" height="44" alt="Screenshot 2025-10-02 at 00 16 54"
src="https://github.com/user-attachments/assets/9e975434-114c-44d5-a4ed-ac6a954b9d00"
/>

**After**
<img width="270" height="44" alt="Screenshot 2025-10-02 at 00 16 11"
src="https://github.com/user-attachments/assets/20545e74-c9f9-4a6b-9bf0-a7cf1d38c3a0"
/>
pull/8999/head
Mitchell Hashimoto 2025-10-02 10:08:24 -07:00 committed by GitHub
commit 9c8d2e577e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 32 additions and 11 deletions

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, except
/// the Powerline range, and anything in several unicode blocks:
/// for now we define as anything in a private use area, and anything
/// in several unicode blocks:
/// - Dingbats
/// - Emoticons
/// - Miscellaneous Symbols
@ -249,13 +249,11 @@ 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) and !isPowerline(cp);
return symbols.get(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();
@ -276,6 +274,8 @@ 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.
// This does not apply if the previous symbol is a graphics
// element such as a block element or Powerline glyph.
if (cell_pin.x > 0) {
const prev_cp = prev_cp: {
var copy = cell_pin;
@ -284,7 +284,7 @@ pub fn constraintWidth(cell_pin: terminal.Pin) u2 {
break :prev_cp prev_cell.codepoint();
};
if (isSymbol(prev_cp)) {
if (isSymbol(prev_cp) and !isGraphicsElement(prev_cp)) {
return 1;
}
}
@ -305,11 +305,10 @@ pub fn constraintWidth(cell_pin: terminal.Pin) u2 {
return 1;
}
/// 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.
/// Whether min contrast should be disabled for a given glyph. True
/// for graphics elements such as blocks and Powerline glyphs.
pub fn noMinContrast(cp: u21) bool {
return isBoxDrawing(cp) or isBlockElement(cp) or isLegacyComputing(cp) or isPowerline(cp);
return isGraphicsElement(cp);
}
// Some general spaces, others intentionally kept
@ -323,6 +322,12 @@ fn isSpace(char: u21) bool {
};
}
/// Returns true if the codepoint is used for terminal graphics, such
/// as box drawing characters, block elements, and Powerline glyphs.
fn isGraphicsElement(char: u21) bool {
return isBoxDrawing(char) or isBlockElement(char) or isLegacyComputing(char) or isPowerline(char);
}
// Returns true if the codepoint is a box drawing character.
fn isBoxDrawing(char: u21) bool {
return switch (char) {
@ -514,7 +519,7 @@ test "Contents with zero-sized screen" {
try testing.expect(c.getCursorGlyph() == null);
}
test "Screen cell constraint widths" {
test "Cell constraint widths" {
const testing = std.testing;
const alloc = testing.allocator;
@ -606,4 +611,20 @@ test "Screen cell constraint widths" {
try testing.expectEqual(2, constraintWidth(p1));
s.reset();
}
// powerline->nothing: 2 (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(2, constraintWidth(p0));
s.reset();
}
// powerline->space: 2 (dedicated test because powerline is special-cased in cellpkg)
{
try s.testWriteString(" z");
const p0 = s.pages.pin(.{ .screen = .{ .x = 0, .y = 0 } }).?;
try testing.expectEqual(2, constraintWidth(p0));
s.reset();
}
}