mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-05-24 05:40:15 +00:00
address review: Discover.init takes a Library across all backends
Per review feedback, drop the `if (Discover == Windows)` comptime branches in SharedGridSet and list_fonts by making every backend's `init` take a Library and ignore it when unused. Call sites just do `Discover.init(self.font_lib)` now. Also adds a discovery test for the Windows backend that looks up Arial and checks the returned face has the 'A' codepoint. Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -4,7 +4,6 @@ const ArenaAllocator = std.heap.ArenaAllocator;
|
||||
const Action = @import("ghostty.zig").Action;
|
||||
const args = @import("args.zig");
|
||||
const font = @import("../font/main.zig");
|
||||
const discovery = @import("../font/discovery.zig");
|
||||
|
||||
const log = std.log.scoped(.list_fonts);
|
||||
|
||||
@@ -101,18 +100,12 @@ fn runArgs(alloc_gpa: Allocator, argsIter: anytype) !u8 {
|
||||
var families: std.ArrayList([]const u8) = .empty;
|
||||
var map: std.StringHashMap(std.ArrayListUnmanaged([]const u8)) = .init(alloc);
|
||||
|
||||
// Look up all available fonts. The Windows backend needs a FreeType
|
||||
// library handle so it can open candidate font files while scanning
|
||||
// the system/user font directories.
|
||||
var font_lib = if (comptime font.Discover == discovery.Windows)
|
||||
try font.Library.init(alloc)
|
||||
else {};
|
||||
defer if (comptime font.Discover == discovery.Windows) font_lib.deinit();
|
||||
|
||||
var disco = if (comptime font.Discover == discovery.Windows)
|
||||
font.Discover.init(font_lib)
|
||||
else
|
||||
font.Discover.init();
|
||||
// Look up all available fonts. The library is only used by backends
|
||||
// that need it (the Windows backend opens candidate font files with
|
||||
// FreeType); other backends ignore it.
|
||||
var font_lib = try font.Library.init(alloc);
|
||||
defer font_lib.deinit();
|
||||
var disco = font.Discover.init(font_lib);
|
||||
defer disco.deinit();
|
||||
var disco_it = try disco.discover(alloc, .{
|
||||
.family = config.family,
|
||||
|
||||
@@ -442,10 +442,7 @@ fn discover(self: *SharedGridSet) !?*Discover {
|
||||
// If we initialized, use it
|
||||
if (self.font_discover) |*v| return v;
|
||||
|
||||
self.font_discover = if (comptime Discover == discovery.Windows)
|
||||
.init(self.font_lib)
|
||||
else
|
||||
.init();
|
||||
self.font_discover = .init(self.font_lib);
|
||||
return &self.font_discover.?;
|
||||
}
|
||||
|
||||
|
||||
@@ -247,7 +247,8 @@ pub const Descriptor = struct {
|
||||
pub const Fontconfig = struct {
|
||||
fc_config: *fontconfig.Config,
|
||||
|
||||
pub fn init() Fontconfig {
|
||||
pub fn init(lib: Library) Fontconfig {
|
||||
_ = lib;
|
||||
// safe to call multiple times and concurrently
|
||||
_ = fontconfig.init();
|
||||
return .{ .fc_config = fontconfig.initLoadConfigAndFonts() };
|
||||
@@ -338,7 +339,8 @@ pub const Fontconfig = struct {
|
||||
};
|
||||
|
||||
pub const CoreText = struct {
|
||||
pub fn init() CoreText {
|
||||
pub fn init(lib: Library) CoreText {
|
||||
_ = lib;
|
||||
// Required for the "interface" but does nothing for CoreText.
|
||||
return .{};
|
||||
}
|
||||
@@ -1162,7 +1164,10 @@ test "fontconfig" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var fc = Fontconfig.init();
|
||||
var lib = try Library.init(alloc);
|
||||
defer lib.deinit();
|
||||
|
||||
var fc = Fontconfig.init(lib);
|
||||
defer fc.deinit();
|
||||
var it = try fc.discover(alloc, .{ .family = "monospace", .size = 12 });
|
||||
defer it.deinit();
|
||||
@@ -1174,7 +1179,10 @@ test "fontconfig codepoint" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var fc = Fontconfig.init();
|
||||
var lib = try Library.init(alloc);
|
||||
defer lib.deinit();
|
||||
|
||||
var fc = Fontconfig.init(lib);
|
||||
defer fc.deinit();
|
||||
var it = try fc.discover(alloc, .{ .codepoint = 'A', .size = 12 });
|
||||
defer it.deinit();
|
||||
@@ -1196,7 +1204,10 @@ test "coretext" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var ct = CoreText.init();
|
||||
var lib = try Library.init(alloc);
|
||||
defer lib.deinit();
|
||||
|
||||
var ct = CoreText.init(lib);
|
||||
defer ct.deinit();
|
||||
var it = try ct.discover(alloc, .{ .family = "Monaco", .size = 12 });
|
||||
defer it.deinit();
|
||||
@@ -1214,7 +1225,10 @@ test "coretext codepoint" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var ct = CoreText.init();
|
||||
var lib = try Library.init(alloc);
|
||||
defer lib.deinit();
|
||||
|
||||
var ct = CoreText.init(lib);
|
||||
defer ct.deinit();
|
||||
var it = try ct.discover(alloc, .{ .codepoint = 'A', .size = 12 });
|
||||
defer it.deinit();
|
||||
@@ -1243,7 +1257,10 @@ test "coretext sorting" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var ct = CoreText.init();
|
||||
var lib = try Library.init(alloc);
|
||||
defer lib.deinit();
|
||||
|
||||
var ct = CoreText.init(lib);
|
||||
defer ct.deinit();
|
||||
|
||||
// We try to get a Regular, Italic, Bold, & Bold Italic version of SF Pro,
|
||||
@@ -1309,3 +1326,24 @@ test "coretext sorting" {
|
||||
try testing.expectEqualStrings("SF Pro Bold Italic", name);
|
||||
}
|
||||
}
|
||||
|
||||
test "windows" {
|
||||
if (options.backend != .freetype_windows) return error.SkipZigTest;
|
||||
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var lib = try Library.init(alloc);
|
||||
defer lib.deinit();
|
||||
|
||||
var win = Windows.init(lib);
|
||||
defer win.deinit();
|
||||
|
||||
// Arial ships on every stock Windows install.
|
||||
var it = try win.discover(alloc, .{ .family = "Arial", .size = 12 });
|
||||
defer it.deinit();
|
||||
|
||||
var face = (try it.next()) orelse return error.TestFontNotFound;
|
||||
defer face.deinit();
|
||||
try testing.expect(face.hasCodepoint('A', null));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user