diff --git a/include/ghostty.h b/include/ghostty.h index 6cc288b8f..5f0ee0fe9 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -291,7 +291,7 @@ typedef struct { typedef struct { const char* message; -} ghostty_error_s; +} ghostty_diagnostic_s; typedef struct { double tl_px_x; @@ -615,8 +615,8 @@ bool ghostty_config_get(ghostty_config_t, void*, const char*, uintptr_t); ghostty_input_trigger_s ghostty_config_trigger(ghostty_config_t, const char*, uintptr_t); -uint32_t ghostty_config_errors_count(ghostty_config_t); -ghostty_error_s ghostty_config_get_error(ghostty_config_t, uint32_t); +uint32_t ghostty_config_diagnostics_count(ghostty_config_t); +ghostty_diagnostic_s ghostty_config_get_diagnostic(ghostty_config_t, uint32_t); void ghostty_config_open(); ghostty_app_t ghostty_app_new(const ghostty_runtime_config_s*, diff --git a/macos/Sources/Ghostty/Ghostty.Config.swift b/macos/Sources/Ghostty/Ghostty.Config.swift index 95f8ad734..69c9f992b 100644 --- a/macos/Sources/Ghostty/Ghostty.Config.swift +++ b/macos/Sources/Ghostty/Ghostty.Config.swift @@ -22,15 +22,15 @@ extension Ghostty { var errors: [String] { guard let cfg = self.config else { return [] } - var errors: [String] = []; - let errCount = ghostty_config_errors_count(cfg) - for i in 0.. 0 { - logger.warning("config error: \(errCount) configuration errors on reload") - var errors: [String] = []; - for i in 0.. 0 { + logger.warning("config error: \(diagsCount) configuration errors on reload") + var diags: [String] = []; + for i in 0.. return err, + error.InvalidField => "unknown field", + error.ValueRequired => "value required", + error.InvalidValue => "invalid value", + else => try std.fmt.allocPrintZ( + arena_alloc, + "unknown error {}", + .{err}, + ), + }; - error.InvalidField => try dst._errors.add(arena_alloc, .{ - .message = try std.fmt.allocPrintZ( - arena_alloc, - "{s}: unknown field", - .{key}, - ), - }), - - error.ValueRequired => try dst._errors.add(arena_alloc, .{ - .message = try std.fmt.allocPrintZ( - arena_alloc, - "{s}: value required", - .{key}, - ), - }), - - error.InvalidValue => try dst._errors.add(arena_alloc, .{ - .message = try std.fmt.allocPrintZ( - arena_alloc, - "{s}: invalid value", - .{key}, - ), - }), - - else => try dst._errors.add(arena_alloc, .{ - .message = try std.fmt.allocPrintZ( - arena_alloc, - "{s}: unknown error {}", - .{ key, err }, - ), - }), - } + // Add our diagnostic + try dst._diagnostics.append(arena_alloc, .{ + .key = try arena_alloc.dupeZ(u8, key), + .message = message, + .location = Diagnostic.Location.fromIter(iter), + }); }; } } } -/// Returns true if this type can track errors. -fn canTrackErrors(comptime T: type) bool { - return @hasField(T, "_errors"); +/// Returns true if this type can track diagnostics. +fn canTrackDiags(comptime T: type) bool { + return @hasField(T, "_diagnostics"); } /// Parse a single key/value pair into the destination type T. @@ -199,15 +192,6 @@ fn parseIntoField( // 3 arg = (self, alloc, input) => void 3 => try @field(dst, field.name).parseCLI(alloc, value), - // 4 arg = (self, alloc, errors, input) => void - 4 => if (comptime canTrackErrors(T)) { - try @field(dst, field.name).parseCLI(alloc, &dst._errors, value); - } else { - var list: ErrorList = .{}; - try @field(dst, field.name).parseCLI(alloc, &list, value); - if (!list.empty()) return error.InvalidValue; - }, - else => @compileError("parseCLI invalid argument count"), } @@ -468,7 +452,7 @@ test "parse: empty value resets to default" { try testing.expect(!data.b); } -test "parse: error tracking" { +test "parse: diagnostic tracking" { const testing = std.testing; var data: struct { @@ -476,7 +460,7 @@ test "parse: error tracking" { b: enum { one } = .one, _arena: ?ArenaAllocator = null, - _errors: ErrorList = .{}, + _diagnostics: DiagnosticList = .{}, } = .{}; defer if (data._arena) |arena| arena.deinit(); @@ -488,7 +472,48 @@ test "parse: error tracking" { try parse(@TypeOf(data), testing.allocator, &data, &iter); try testing.expect(data._arena != null); try testing.expectEqualStrings("42", data.a); - try testing.expect(!data._errors.empty()); + try testing.expect(data._diagnostics.items().len == 1); + { + const diag = data._diagnostics.items()[0]; + try testing.expectEqual(Diagnostic.Location.none, diag.location); + try testing.expectEqualStrings("what", diag.key); + try testing.expectEqualStrings("unknown field", diag.message); + } +} + +test "parse: diagnostic location" { + const testing = std.testing; + + var data: struct { + a: []const u8 = "", + b: enum { one, two } = .one, + + _arena: ?ArenaAllocator = null, + _diagnostics: DiagnosticList = .{}, + } = .{}; + defer if (data._arena) |arena| arena.deinit(); + + var fbs = std.io.fixedBufferStream( + \\a=42 + \\what + \\b=two + ); + const r = fbs.reader(); + + const Iter = LineIterator(@TypeOf(r)); + var iter: Iter = .{ .r = r, .filepath = "test" }; + try parse(@TypeOf(data), testing.allocator, &data, &iter); + try testing.expect(data._arena != null); + try testing.expectEqualStrings("42", data.a); + try testing.expect(data.b == .two); + try testing.expect(data._diagnostics.items().len == 1); + { + const diag = data._diagnostics.items()[0]; + try testing.expectEqualStrings("what", diag.key); + try testing.expectEqualStrings("unknown field", diag.message); + try testing.expectEqualStrings("test", diag.location.file.path); + try testing.expectEqual(2, diag.location.file.line); + } } test "parseIntoField: ignore underscore-prefixed fields" { @@ -738,62 +763,6 @@ test "parseIntoField: struct with parse func" { try testing.expectEqual(@as([]const u8, "HELLO!"), data.a.v); } -test "parseIntoField: struct with parse func with error tracking" { - const testing = std.testing; - var arena = ArenaAllocator.init(testing.allocator); - defer arena.deinit(); - const alloc = arena.allocator(); - - var data: struct { - a: struct { - const Self = @This(); - - pub fn parseCLI( - _: Self, - parse_alloc: Allocator, - errors: *ErrorList, - value: ?[]const u8, - ) !void { - _ = value; - try errors.add(parse_alloc, .{ .message = "OH NO!" }); - } - } = .{}, - - _errors: ErrorList = .{}, - } = .{}; - - try parseIntoField(@TypeOf(data), alloc, &data, "a", "42"); - try testing.expect(!data._errors.empty()); -} - -test "parseIntoField: struct with parse func with unsupported error tracking" { - const testing = std.testing; - var arena = ArenaAllocator.init(testing.allocator); - defer arena.deinit(); - const alloc = arena.allocator(); - - var data: struct { - a: struct { - const Self = @This(); - - pub fn parseCLI( - _: Self, - parse_alloc: Allocator, - errors: *ErrorList, - value: ?[]const u8, - ) !void { - _ = value; - try errors.add(parse_alloc, .{ .message = "OH NO!" }); - } - } = .{}, - } = .{}; - - try testing.expectError( - error.InvalidValue, - parseIntoField(@TypeOf(data), alloc, &data, "a", "42"), - ); -} - test "parseIntoField: tagged union" { const testing = std.testing; var arena = ArenaAllocator.init(testing.allocator); @@ -899,7 +868,21 @@ pub fn LineIterator(comptime ReaderType: type) type { /// like 4 years and be wrong about this. pub const MAX_LINE_SIZE = 4096; + /// Our stateful reader. r: ReaderType, + + /// Filepath that is used for diagnostics. This is only used for + /// diagnostic messages so it can be formatted however you want. + /// It is prefixed to the messages followed by the line number. + filepath: []const u8 = "", + + /// The current line that we're on. This is 1-indexed because + /// lines are generally 1-indexed in the real world. The value + /// can be zero if we haven't read any lines yet. + line: usize = 0, + + /// This is the buffer where we store the current entry that + /// is formatted to be compatible with the parse function. entry: [MAX_LINE_SIZE]u8 = [_]u8{ '-', '-' } ++ ([_]u8{0} ** (MAX_LINE_SIZE - 2)), pub fn next(self: *Self) ?[]const u8 { @@ -912,6 +895,9 @@ pub fn LineIterator(comptime ReaderType: type) type { unreachable; } orelse return null; + // Increment our line counter + self.line += 1; + // Trim any whitespace (including CR) around it const trim = std.mem.trim(u8, entry, whitespace ++ "\r"); if (trim.len != entry.len) { @@ -959,6 +945,18 @@ pub fn LineIterator(comptime ReaderType: type) type { // as CLI args. return self.entry[0 .. buf.len + 2]; } + + /// Modify the diagnostic so it includes richer context about + /// the location of the error. + pub fn location(self: *const Self) ?Diagnostic.Location { + // If we have no filepath then we have no location. + if (self.filepath.len == 0) return null; + + return .{ .file = .{ + .path = self.filepath, + .line = self.line, + } }; + } }; } diff --git a/src/cli/diagnostics.zig b/src/cli/diagnostics.zig new file mode 100644 index 000000000..80dc06b5a --- /dev/null +++ b/src/cli/diagnostics.zig @@ -0,0 +1,124 @@ +const std = @import("std"); +const builtin = @import("builtin"); +const assert = std.debug.assert; +const Allocator = std.mem.Allocator; +const build_config = @import("../build_config.zig"); + +/// A diagnostic message from parsing. This is used to provide additional +/// human-friendly warnings and errors about the parsed data. +/// +/// All of the memory for the diagnostic is allocated from the arena +/// associated with the config structure. If an arena isn't available +/// then diagnostics are not supported. +pub const Diagnostic = struct { + location: Location = .none, + key: [:0]const u8 = "", + message: [:0]const u8, + + /// The possible locations for a diagnostic message. This is used + /// to provide context for the message. + pub const Location = union(enum) { + none, + cli: usize, + file: struct { + path: []const u8, + line: usize, + }, + + pub fn fromIter(iter: anytype) Location { + const Iter = t: { + const T = @TypeOf(iter); + break :t switch (@typeInfo(T)) { + .Pointer => |v| v.child, + .Struct => T, + else => return .none, + }; + }; + + if (!@hasDecl(Iter, "location")) return .none; + return iter.location() orelse .none; + } + }; + + /// Write the full user-friendly diagnostic message to the writer. + pub fn write(self: *const Diagnostic, writer: anytype) !void { + switch (self.location) { + .none => {}, + .cli => |index| try writer.print("cli:{}:", .{index}), + .file => |file| try writer.print( + "{s}:{}:", + .{ file.path, file.line }, + ), + } + + if (self.key.len > 0) { + try writer.print("{s}: ", .{self.key}); + } else if (self.location != .none) { + try writer.print(" ", .{}); + } + + try writer.print("{s}", .{self.message}); + } +}; + +/// A list of diagnostics. The "_diagnostics" field must be this type +/// for diagnostics to be supported. If this field is an incorrect type +/// a compile-time error will be raised. +/// +/// This is implemented as a simple wrapper around an array list +/// so that we can inject some logic around adding diagnostics +/// and potentially in the future structure them differently. +pub const DiagnosticList = struct { + /// The list of diagnostics. + list: std.ArrayListUnmanaged(Diagnostic) = .{}, + + /// Precomputed data for diagnostics. This is used specifically + /// when we build libghostty so that we can precompute the messages + /// and return them via the C API without allocating memory at + /// call time. + precompute: Precompute = precompute_init, + + const precompute_enabled = switch (build_config.artifact) { + // We enable precompute for tests so that the logic is + // semantically analyzed and run. + .exe, .wasm_module => builtin.is_test, + + // We specifically want precompute for libghostty. + .lib => true, + }; + const Precompute = if (precompute_enabled) struct { + messages: std.ArrayListUnmanaged([:0]const u8) = .{}, + } else void; + const precompute_init: Precompute = if (precompute_enabled) .{} else {}; + + pub fn append( + self: *DiagnosticList, + alloc: Allocator, + diag: Diagnostic, + ) Allocator.Error!void { + try self.list.append(alloc, diag); + errdefer _ = self.list.pop(); + + if (comptime precompute_enabled) { + var buf = std.ArrayList(u8).init(alloc); + defer buf.deinit(); + try diag.write(buf.writer()); + + const owned: [:0]const u8 = try buf.toOwnedSliceSentinel(0); + errdefer alloc.free(owned); + + try self.precompute.messages.append(alloc, owned); + errdefer _ = self.precompute.messages.pop(); + + assert(self.precompute.messages.items.len == self.list.items.len); + } + } + + pub fn empty(self: *const DiagnosticList) bool { + return self.list.items.len == 0; + } + + pub fn items(self: *DiagnosticList) []Diagnostic { + return self.list.items; + } +}; diff --git a/src/cli/list_themes.zig b/src/cli/list_themes.zig index 99b419801..feec6fcb0 100644 --- a/src/cli/list_themes.zig +++ b/src/cli/list_themes.zig @@ -882,7 +882,7 @@ const Preview = struct { next_start += child.height; } - if (!config._errors.empty()) { + if (config._diagnostics.items().len > 0) { const child = win.child( .{ .x_off = x_off, @@ -891,7 +891,7 @@ const Preview = struct { .limit = width, }, .height = .{ - .limit = if (config._errors.empty()) 0 else 2 + config._errors.list.items.len, + .limit = if (config._diagnostics.items().len == 0) 0 else 2 + config._diagnostics.items().len, }, }, ); @@ -908,10 +908,14 @@ const Preview = struct { }, ); } - for (config._errors.list.items, 0..) |err, i| { + + var buf = std.ArrayList(u8).init(alloc); + defer buf.deinit(); + for (config._diagnostics.items(), 0..) |diag, i| { + try diag.write(buf.writer()); _ = try child.printSegment( .{ - .text = err.message, + .text = buf.items, .style = self.ui_err(), }, .{ @@ -919,6 +923,7 @@ const Preview = struct { .col_offset = 2, }, ); + buf.clearRetainingCapacity(); } next_start += child.height; } diff --git a/src/cli/validate_config.zig b/src/cli/validate_config.zig index d6fedc544..ef1dd3ecc 100644 --- a/src/cli/validate_config.zig +++ b/src/cli/validate_config.zig @@ -55,9 +55,14 @@ pub fn run(alloc: std.mem.Allocator) !u8 { try cfg.finalize(); - if (!cfg._errors.empty()) { - for (cfg._errors.list.items) |err| { - try stdout.print("{s}\n", .{err.message}); + if (cfg._diagnostics.items().len > 0) { + var buf = std.ArrayList(u8).init(alloc); + defer buf.deinit(); + + for (cfg._diagnostics.items()) |diag| { + try diag.write(buf.writer()); + try stdout.print("{s}\n", .{buf.items}); + buf.clearRetainingCapacity(); } return 1; diff --git a/src/config/CAPI.zig b/src/config/CAPI.zig index 1949d6e91..dc81a97ed 100644 --- a/src/config/CAPI.zig +++ b/src/config/CAPI.zig @@ -112,14 +112,15 @@ fn config_trigger_( return trigger.cval(); } -export fn ghostty_config_errors_count(self: *Config) u32 { - return @intCast(self._errors.list.items.len); +export fn ghostty_config_diagnostics_count(self: *Config) u32 { + return @intCast(self._diagnostics.items().len); } -export fn ghostty_config_get_error(self: *Config, idx: u32) Error { - if (idx >= self._errors.list.items.len) return .{}; - const err = self._errors.list.items[idx]; - return .{ .message = err.message.ptr }; +export fn ghostty_config_get_diagnostic(self: *Config, idx: u32) Diagnostic { + const items = self._diagnostics.items(); + if (idx >= items.len) return .{}; + const message = self._diagnostics.precompute.messages.items[idx]; + return .{ .message = message.ptr }; } export fn ghostty_config_open() void { @@ -128,7 +129,7 @@ export fn ghostty_config_open() void { }; } -/// Sync with ghostty_error_s -const Error = extern struct { +/// Sync with ghostty_diagnostic_s +const Diagnostic = extern struct { message: [*:0]const u8 = "", }; diff --git a/src/config/Config.zig b/src/config/Config.zig index a5c337fe6..666bfbdcb 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1662,10 +1662,9 @@ term: []const u8 = "xterm-ghostty", /// This is set by the CLI parser for deinit. _arena: ?ArenaAllocator = null, -/// List of errors that occurred while loading. This can be accessed directly -/// by callers. It is only underscore-prefixed so it can't be set by the -/// configuration file. -_errors: ErrorList = .{}, +/// List of diagnostics that were generated during the loading of +/// the configuration. +_diagnostics: cli.DiagnosticList = .{}, /// The steps we can use to reload the configuration after it has been loaded /// without reopening the files. This is used in very specific cases such @@ -2446,7 +2445,7 @@ pub fn loadRecursiveFiles(self: *Config, alloc_gpa: Allocator) !void { // We must only load a unique file once if (try loaded.fetchPut(path, {}) != null) { - try self._errors.add(arena_alloc, .{ + try self._diagnostics.append(arena_alloc, .{ .message = try std.fmt.allocPrintZ( arena_alloc, "config-file {s}: cycle detected", @@ -2458,7 +2457,7 @@ pub fn loadRecursiveFiles(self: *Config, alloc_gpa: Allocator) !void { var file = cwd.openFile(path, .{}) catch |err| { if (err != error.FileNotFound or !optional) { - try self._errors.add(arena_alloc, .{ + try self._diagnostics.append(arena_alloc, .{ .message = try std.fmt.allocPrintZ( arena_alloc, "error opening config-file {s}: {}", @@ -2495,7 +2494,7 @@ fn expandPaths(self: *Config, base: []const u8) !void { try @field(self, field.name).expand( arena_alloc, base, - &self._errors, + &self._diagnostics, ); } } @@ -2506,7 +2505,7 @@ fn loadTheme(self: *Config, theme: []const u8) !void { const file: std.fs.File = (try themepkg.open( self._arena.?.allocator(), theme, - &self._errors, + &self._diagnostics, )) orelse return; defer file.close(); @@ -2697,7 +2696,12 @@ pub fn finalize(self: *Config) !void { /// Callback for src/cli/args.zig to allow us to handle special cases /// like `--help` or `-e`. Returns "false" if the CLI parsing should halt. -pub fn parseManuallyHook(self: *Config, alloc: Allocator, arg: []const u8, iter: anytype) !bool { +pub fn parseManuallyHook( + self: *Config, + alloc: Allocator, + arg: []const u8, + iter: anytype, +) !bool { // Keep track of our input args no matter what.. try self._replay_steps.append(alloc, .{ .arg = try alloc.dupe(u8, arg) }); @@ -2714,7 +2718,8 @@ pub fn parseManuallyHook(self: *Config, alloc: Allocator, arg: []const u8, iter: } if (command.items.len == 0) { - try self._errors.add(alloc, .{ + try self._diagnostics.append(alloc, .{ + .location = cli.Diagnostic.Location.fromIter(iter), .message = try std.fmt.allocPrintZ( alloc, "missing command after {s}", @@ -3506,7 +3511,7 @@ pub const RepeatablePath = struct { self: *Self, alloc: Allocator, base: []const u8, - errors: *ErrorList, + diags: *cli.DiagnosticList, ) !void { assert(std.fs.path.isAbsolute(base)); var dir = try std.fs.cwd().openDir(base, .{}); @@ -3533,7 +3538,7 @@ pub const RepeatablePath = struct { break :abs buf[0..resolved.len]; } - try errors.add(alloc, .{ + try diags.append(alloc, .{ .message = try std.fmt.allocPrintZ( alloc, "error resolving file path {s}: {}", diff --git a/src/config/theme.zig b/src/config/theme.zig index fdb5dd08a..af2cab88d 100644 --- a/src/config/theme.zig +++ b/src/config/theme.zig @@ -4,7 +4,7 @@ const assert = std.debug.assert; const Allocator = std.mem.Allocator; const global_state = &@import("../global.zig").state; const internal_os = @import("../os/main.zig"); -const ErrorList = @import("ErrorList.zig"); +const cli = @import("../cli.zig"); /// Location of possible themes. The order of this enum matters because it /// defines the priority of theme search (from top to bottom). @@ -107,19 +107,19 @@ pub const LocationIterator = struct { pub fn open( arena_alloc: Allocator, theme: []const u8, - errors: *ErrorList, + diags: *cli.DiagnosticList, ) error{OutOfMemory}!?std.fs.File { // Absolute themes are loaded a different path. if (std.fs.path.isAbsolute(theme)) return try openAbsolute( arena_alloc, theme, - errors, + diags, ); const basename = std.fs.path.basename(theme); if (!std.mem.eql(u8, theme, basename)) { - try errors.add(arena_alloc, .{ + try diags.append(arena_alloc, .{ .message = try std.fmt.allocPrintZ( arena_alloc, "theme \"{s}\" cannot include path separators unless it is an absolute path", @@ -143,7 +143,7 @@ pub fn open( // Anything else is an error we log and give up on. else => { - try errors.add(arena_alloc, .{ + try diags.append(arena_alloc, .{ .message = try std.fmt.allocPrintZ( arena_alloc, "failed to load theme \"{s}\" from the file \"{s}\": {}", @@ -163,7 +163,7 @@ pub fn open( it.reset(); while (try it.next()) |loc| { const path = try std.fs.path.join(arena_alloc, &.{ loc.dir, theme }); - try errors.add(arena_alloc, .{ + try diags.append(arena_alloc, .{ .message = try std.fmt.allocPrintZ( arena_alloc, "theme \"{s}\" not found, tried path \"{s}\"", @@ -186,18 +186,18 @@ pub fn open( pub fn openAbsolute( arena_alloc: Allocator, theme: []const u8, - errors: *ErrorList, + diags: *cli.DiagnosticList, ) error{OutOfMemory}!?std.fs.File { return std.fs.openFileAbsolute(theme, .{}) catch |err| { switch (err) { - error.FileNotFound => try errors.add(arena_alloc, .{ + error.FileNotFound => try diags.append(arena_alloc, .{ .message = try std.fmt.allocPrintZ( arena_alloc, "failed to load theme from the path \"{s}\"", .{theme}, ), }), - else => try errors.add(arena_alloc, .{ + else => try diags.append(arena_alloc, .{ .message = try std.fmt.allocPrintZ( arena_alloc, "failed to load theme from the path \"{s}\": {}",