From 8c4cfc3bbbfe9bc2f1010f0612d0824c2c6e6413 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Mon, 12 Aug 2024 11:58:57 -0500 Subject: [PATCH] themes: finish refactor --- src/cli/list_themes.zig | 84 ++++++++++++--------------- src/config/Config.zig | 7 +-- src/config/theme.zig | 126 ++++++++++++++++++++++++---------------- 3 files changed, 115 insertions(+), 102 deletions(-) diff --git a/src/cli/list_themes.zig b/src/cli/list_themes.zig index 5de9351b8..0509bffe9 100644 --- a/src/cli/list_themes.zig +++ b/src/cli/list_themes.zig @@ -2,13 +2,15 @@ const std = @import("std"); const inputpkg = @import("../input.zig"); const args = @import("args.zig"); const Action = @import("action.zig").Action; -const Arena = std.heap.ArenaAllocator; -const Allocator = std.mem.Allocator; const Config = @import("../config/Config.zig"); +const themepkg = @import("../config/theme.zig"); const internal_os = @import("../os/main.zig"); const global_state = &@import("../global.zig").state; pub const Options = struct { + /// If true, print the full path to the theme. + path: bool = false, + /// If true, show a small preview of the theme. preview: bool = false, @@ -45,17 +47,21 @@ pub const Options = struct { /// /// Flags: /// +/// * `--path`: Show the full path to the theme. /// * `--preview`: Show a short preview of the theme colors. -pub fn run(alloc: Allocator) !u8 { +pub fn run(gpa_alloc: std.mem.Allocator) !u8 { var opts: Options = .{}; defer opts.deinit(); { - var iter = try std.process.argsWithAllocator(alloc); + var iter = try std.process.argsWithAllocator(gpa_alloc); defer iter.deinit(); - try args.parse(Options, alloc, &opts, &iter); + try args.parse(Options, gpa_alloc, &opts, &iter); } + var arena = std.heap.ArenaAllocator.init(gpa_alloc); + const alloc = arena.allocator(); + const stderr = std.io.getStdErr().writer(); const stdout = std.io.getStdOut().writer(); @@ -63,29 +69,12 @@ pub fn run(alloc: Allocator) !u8 { try stderr.print("Could not find the Ghostty resources directory. Please ensure " ++ "that Ghostty is installed correctly.\n", .{}); - const paths: []const struct { - type: Config.ThemeDirType, - dir: ?[]const u8, - } = &.{ - .{ - .type = .user, - .dir = Config.themeDir(alloc, .user), - }, - .{ - .type = .system, - .dir = Config.themeDir(alloc, .system), - }, - }; - const ThemeListElement = struct { - type: Config.ThemeDirType, + location: themepkg.Location, path: []const u8, theme: []const u8, - fn deinit(self: *const @This(), alloc_: std.mem.Allocator) void { - alloc_.free(self.path); - alloc_.free(self.theme); - } fn lessThan(_: void, lhs: @This(), rhs: @This()) bool { + // TODO: use Unicode-aware comparison return std.ascii.orderIgnoreCase(lhs.theme, rhs.theme) == .lt; } }; @@ -93,40 +82,41 @@ pub fn run(alloc: Allocator) !u8 { var count: usize = 0; var themes = std.ArrayList(ThemeListElement).init(alloc); - defer { - for (themes.items) |v| v.deinit(alloc); - themes.deinit(); - } - for (paths) |path| { - if (path.dir) |p| { - defer alloc.free(p); + var it = themepkg.LocationIterator{ .arena = &arena }; - var dir = try std.fs.cwd().openDir(p, .{ .iterate = true }); - defer dir.close(); + while (try it.next()) |loc| { + var dir = std.fs.cwd().openDir(loc.dir, .{ .iterate = true }) catch |err| switch (err) { + error.FileNotFound => continue, + else => { + std.debug.print("err: {}\n", .{err}); + continue; + }, + }; + defer dir.close(); + var walker = dir.iterate(); - var walker = try dir.walk(alloc); - defer walker.deinit(); - - while (try walker.next()) |entry| { - if (entry.kind != .file) continue; - count += 1; - try themes.append(.{ - .type = path.type, - .path = try std.fs.path.join(alloc, &.{ p, entry.basename }), - .theme = try alloc.dupe(u8, entry.basename), - }); - } + while (try walker.next()) |entry| { + if (entry.kind != .file) continue; + count += 1; + try themes.append(.{ + .location = loc.location, + .path = try std.fs.path.join(alloc, &.{ loc.dir, entry.name }), + .theme = try alloc.dupe(u8, entry.name), + }); } } std.mem.sortUnstable(ThemeListElement, themes.items, {}, ThemeListElement.lessThan); for (themes.items) |theme| { - try stdout.print("{s} ({s})\n", .{ theme.theme, @tagName(theme.type) }); + if (opts.path) + try stdout.print("{s} ({s}) {s}\n", .{ theme.theme, @tagName(theme.location), theme.path }) + else + try stdout.print("{s} ({s})\n", .{ theme.theme, @tagName(theme.location) }); if (opts.preview) { - var config = try Config.default(alloc); + var config = try Config.default(gpa_alloc); defer config.deinit(); if (config.loadFile(config._arena.?.allocator(), theme.path)) |_| { if (!config._errors.empty()) { diff --git a/src/config/Config.zig b/src/config/Config.zig index 8901c1500..ee1b49983 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -247,7 +247,8 @@ const c = @cImport({ /// /// If the theme is not an absolute pathname, two different directories will be /// searched for a file name that matches the theme. This is case sensitive on -/// systems with case-sensitive filesystems. +/// systems with case-sensitive filesystems. It is an error for it to include +/// path separators. /// /// The first directory is the `themes` subdirectory of your Ghostty /// configuration directory. This is `$XDG_CONFIG_DIR/ghostty/themes` or @@ -2204,11 +2205,9 @@ pub fn themeDir(alloc: std.mem.Allocator, type_: ThemeDirType) ?[]const u8 { } fn loadTheme(self: *Config, theme: []const u8) !void { - const alloc = self._arena.?.allocator(); - // Find our theme file and open it. See the open function for details. const file: std.fs.File = (try themepkg.open( - alloc, + &self._arena.?, theme, &self._errors, )) orelse return; diff --git a/src/config/theme.zig b/src/config/theme.zig index 62b1468fa..e962324a6 100644 --- a/src/config/theme.zig +++ b/src/config/theme.zig @@ -1,48 +1,43 @@ const std = @import("std"); const builtin = @import("builtin"); const assert = std.debug.assert; -const Allocator = std.mem.Allocator; +const ArenaAllocator = std.heap.ArenaAllocator; const global_state = &@import("../main.zig").state; const internal_os = @import("../os/main.zig"); const ErrorList = @import("ErrorList.zig"); -/// Location of possible themes. The order of this enum matters because -/// it defines the priority of theme search (from top to bottom). +/// Location of possible themes. The order of this enum matters because it +/// defines the priority of theme search (from top to bottom). pub const Location = enum { - user, // xdg config dir + user, // XDG config dir resources, // Ghostty resources dir /// Returns the directory for the given theme based on this location type. /// - /// This will return null with no error if the directory type doesn't - /// exist or is invalid for any reason. For example, it is perfectly - /// valid to install and run Ghostty without the resources directory. + /// This will return null with no error if the directory type doesn't exist + /// or is invalid for any reason. For example, it is perfectly valid to + /// install and run Ghostty without the resources directory. /// - /// This may allocate memory but it isn't guaranteed so the allocator - /// should be something like an arena. It isn't safe to always free the - /// resulting pointer. + /// Due to the way allocations are handled, a pointer to an Arena allocator + /// must be used. pub fn dir( self: Location, - alloc_arena: Allocator, - theme: []const u8, + arena: *ArenaAllocator, ) error{OutOfMemory}!?[]const u8 { - if (comptime std.debug.runtime_safety) { - assert(!std.fs.path.isAbsolute(theme)); - } + const alloc = arena.allocator(); + + // if (comptime std.debug.runtime_safety) { + // assert(!std.fs.path.isAbsolute(theme)); + // } return switch (self) { .user => user: { - var buf: [std.fs.max_path_bytes]u8 = undefined; - const subdir = std.fmt.bufPrint( - &buf, - "ghostty/themes/{s}", - .{theme}, - ) catch |err| switch (err) { - error.NoSpaceLeft => return error.OutOfMemory, - }; + const subdir = std.fs.path.join(alloc, &.{ + "ghostty", "themes", + }) catch return error.OutOfMemory; break :user internal_os.xdg.config( - alloc_arena, + alloc, .{ .subdir = subdir }, ) catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, @@ -55,32 +50,37 @@ pub const Location = enum { }; }, - .resources => try std.fs.path.join(alloc_arena, &.{ + .resources => try std.fs.path.join(alloc, &.{ global_state.resources_dir orelse return null, "themes", - theme, }), }; } }; -/// An iterator that returns all possible locations for a theme in order -/// of priority. +/// An iterator that returns all possible directories for finding themes in +/// order of priority. pub const LocationIterator = struct { - alloc_arena: Allocator, - theme: []const u8, + arena: *ArenaAllocator, i: usize = 0, - pub fn next(self: *LocationIterator) !?[]const u8 { + pub fn next(self: *LocationIterator) !?struct { + location: Location, + dir: []const u8, + } { const max = @typeInfo(Location).Enum.fields.len; - while (true) { - if (self.i >= max) return null; - const loc: Location = @enumFromInt(self.i); + std.debug.print("a: {d} {d}\n", .{ self.i, max }); + while (self.i < max) { + std.debug.print("b: {d}\n", .{self.i}); + const location: Location = @enumFromInt(self.i); self.i += 1; - const dir_ = try loc.dir(self.alloc_arena, self.theme); - const dir = dir_ orelse continue; - return dir; + if (try location.dir(self.arena)) |dir| + return .{ + .location = location, + .dir = dir, + }; } + return null; } pub fn reset(self: *LocationIterator) void { @@ -94,23 +94,42 @@ pub const LocationIterator = struct { /// /// One error that is not recoverable and may be returned is OOM. This is /// always a critical error for configuration loading so it is returned. +/// +/// Due to the way allocations are handled, a pointer to an Arena allocator +/// must be used. pub fn open( - alloc_arena: Allocator, + arena: *ArenaAllocator, theme: []const u8, errors: *ErrorList, ) error{OutOfMemory}!?std.fs.File { + // Absolute themes are loaded a different path. if (std.fs.path.isAbsolute(theme)) return try openAbsolute( - alloc_arena, + arena, theme, errors, ); + const alloc = arena.allocator(); + + const basename = std.fs.path.basename(theme); + if (!std.mem.eql(u8, theme, basename)) { + try errors.add(alloc, .{ + .message = try std.fmt.allocPrintZ( + alloc, + "theme \"{s}\" cannot include path separators unless it is an absolute path", + .{theme}, + ), + }); + return null; + } + // Iterate over the possible locations to try to find the // one that exists. - var it: LocationIterator = .{ .alloc_arena = alloc_arena, .theme = theme }; + var it: LocationIterator = .{ .arena = arena }; const cwd = std.fs.cwd(); - while (try it.next()) |path| { + while (try it.next()) |loc| { + const path = try std.fs.path.join(alloc, &.{ loc.dir, theme }); if (cwd.openFile(path, .{})) |file| { return file; } else |err| switch (err) { @@ -119,9 +138,9 @@ pub fn open( // Anything else is an error we log and give up on. else => { - try errors.add(alloc_arena, .{ + try errors.add(alloc, .{ .message = try std.fmt.allocPrintZ( - alloc_arena, + alloc, "failed to load theme \"{s}\" from the file \"{s}\": {}", .{ theme, path, err }, ), @@ -137,10 +156,11 @@ pub fn open( // This does double allocate some memory but for errors I think thats // fine. it.reset(); - while (try it.next()) |path| { - try errors.add(alloc_arena, .{ + while (try it.next()) |loc| { + const path = try std.fs.path.join(alloc, &.{ loc.dir, theme }); + try errors.add(alloc, .{ .message = try std.fmt.allocPrintZ( - alloc_arena, + alloc, "theme \"{s}\" not found, tried path \"{s}\"", .{ theme, path }, ), @@ -154,23 +174,27 @@ pub fn open( /// then messages will be appended to the given error list and null is /// returned. If a non-null return value is returned, there are never any /// errors added. +/// +/// Due to the way allocations are handled, a pointer to an Arena allocator +/// must be used. pub fn openAbsolute( - alloc_arena: Allocator, + arena: *ArenaAllocator, theme: []const u8, errors: *ErrorList, ) error{OutOfMemory}!?std.fs.File { + const alloc = arena.allocator(); return std.fs.openFileAbsolute(theme, .{}) catch |err| { switch (err) { - error.FileNotFound => try errors.add(alloc_arena, .{ + error.FileNotFound => try errors.add(alloc, .{ .message = try std.fmt.allocPrintZ( - alloc_arena, + alloc, "failed to load theme from the path \"{s}\"", .{theme}, ), }), - else => try errors.add(alloc_arena, .{ + else => try errors.add(alloc, .{ .message = try std.fmt.allocPrintZ( - alloc_arena, + alloc, "failed to load theme from the path \"{s}\": {}", .{ theme, err }, ),