From a4e14631ef6eb57382ff3c15134d90617c8fd264 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 16 Oct 2024 16:45:38 -0700 Subject: [PATCH 1/8] config: richer diagnostics for errors Rather than storing a list of errors we now store a list of "diagnostics." Each diagnostic has a richer set of structured information, including a message, a key, the location where it occurred. This lets us show more detailed messages, more human friendly messages, and also let's us filter by key or location. We don't take advantage of all of this capability in this initial commit, but we do use every field for something. --- include/ghostty.h | 6 +- macos/Sources/Ghostty/Ghostty.Config.swift | 30 +-- src/apprt/glfw.zig | 10 +- src/cli.zig | 5 + src/cli/args.zig | 222 ++++++++++----------- src/cli/diagnostics.zig | 124 ++++++++++++ src/cli/list_themes.zig | 13 +- src/cli/validate_config.zig | 11 +- src/config/CAPI.zig | 17 +- src/config/Config.zig | 29 +-- src/config/theme.zig | 18 +- 11 files changed, 316 insertions(+), 169 deletions(-) create mode 100644 src/cli/diagnostics.zig 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}\": {}", From f24098cbd8e757e2f73cd1ab07b46b5b4f85374d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 17 Oct 2024 07:32:54 -0700 Subject: [PATCH 2/8] config: show filepath and line numbers for config errors Fixes #1063 --- src/cli/args.zig | 2 +- src/config/Config.zig | 16 ++++++++++++---- src/config/theme.zig | 23 +++++++++++++++-------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/cli/args.zig b/src/cli/args.zig index c5355251e..b1fa9104d 100644 --- a/src/cli/args.zig +++ b/src/cli/args.zig @@ -961,7 +961,7 @@ pub fn LineIterator(comptime ReaderType: type) type { } // Constructs a LineIterator (see docs for that). -pub fn lineIterator(reader: anytype) LineIterator(@TypeOf(reader)) { +fn lineIterator(reader: anytype) LineIterator(@TypeOf(reader)) { return .{ .r = reader }; } diff --git a/src/config/Config.zig b/src/config/Config.zig index 666bfbdcb..fddea43a4 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2260,7 +2260,9 @@ pub fn loadFile(self: *Config, alloc: Allocator, path: []const u8) !void { std.log.info("reading configuration file path={s}", .{path}); var buf_reader = std.io.bufferedReader(file.reader()); - var iter = cli.args.lineIterator(buf_reader.reader()); + const reader = buf_reader.reader(); + const Iter = cli.args.LineIterator(@TypeOf(reader)); + var iter: Iter = .{ .r = reader, .filepath = path }; try self.loadIter(alloc, &iter); try self.expandPaths(std.fs.path.dirname(path).?); } @@ -2471,7 +2473,9 @@ pub fn loadRecursiveFiles(self: *Config, alloc_gpa: Allocator) !void { log.info("loading config-file path={s}", .{path}); var buf_reader = std.io.bufferedReader(file.reader()); - var iter = cli.args.lineIterator(buf_reader.reader()); + const reader = buf_reader.reader(); + const Iter = cli.args.LineIterator(@TypeOf(reader)); + var iter: Iter = .{ .r = reader, .filepath = path }; try self.loadIter(alloc_gpa, &iter); try self.expandPaths(std.fs.path.dirname(path).?); } @@ -2502,11 +2506,13 @@ fn expandPaths(self: *Config, base: []const u8) !void { fn loadTheme(self: *Config, theme: []const u8) !void { // Find our theme file and open it. See the open function for details. - const file: std.fs.File = (try themepkg.open( + const themefile = (try themepkg.open( self._arena.?.allocator(), theme, &self._diagnostics, )) orelse return; + const path = themefile.path; + const file = themefile.file; defer file.close(); // From this point onwards, we load the theme and do a bit of a dance @@ -2532,7 +2538,9 @@ fn loadTheme(self: *Config, theme: []const u8) !void { // Load our theme var buf_reader = std.io.bufferedReader(file.reader()); - var iter = cli.args.lineIterator(buf_reader.reader()); + const reader = buf_reader.reader(); + const Iter = cli.args.LineIterator(@TypeOf(reader)); + var iter: Iter = .{ .r = reader, .filepath = path }; try new_config.loadIter(alloc_gpa, &iter); // Replay our previous inputs so that we can override values diff --git a/src/config/theme.zig b/src/config/theme.zig index af2cab88d..4616d6363 100644 --- a/src/config/theme.zig +++ b/src/config/theme.zig @@ -108,14 +108,20 @@ pub fn open( arena_alloc: Allocator, theme: []const u8, diags: *cli.DiagnosticList, -) error{OutOfMemory}!?std.fs.File { +) error{OutOfMemory}!?struct { + path: []const u8, + file: std.fs.File, +} { // Absolute themes are loaded a different path. - if (std.fs.path.isAbsolute(theme)) return try openAbsolute( - arena_alloc, - theme, - diags, - ); + if (std.fs.path.isAbsolute(theme)) { + const file: std.fs.File = try openAbsolute( + arena_alloc, + theme, + diags, + ) orelse return null; + return .{ .path = theme, .file = file }; + } const basename = std.fs.path.basename(theme); if (!std.mem.eql(u8, theme, basename)) { @@ -135,8 +141,9 @@ pub fn open( const cwd = std.fs.cwd(); while (try it.next()) |loc| { const path = try std.fs.path.join(arena_alloc, &.{ loc.dir, theme }); - if (cwd.openFile(path, .{})) |file| { - return file; + if (cwd.openFile(path, .{})) |file| return .{ + .path = path, + .file = file, } else |err| switch (err) { // Not an error, just continue to the next location. error.FileNotFound => {}, From a12b33662cf3a51b06fb8cfd95920eca56d6d35f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 17 Oct 2024 07:55:02 -0700 Subject: [PATCH 3/8] config: track the location of CLI argument errors --- src/cli/args.zig | 31 +++++++++++++++++++++++++++++-- src/config/Config.zig | 6 +++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/cli/args.zig b/src/cli/args.zig index b1fa9104d..da8685e26 100644 --- a/src/cli/args.zig +++ b/src/cli/args.zig @@ -856,6 +856,34 @@ test "parseIntoField: tagged union missing tag" { ); } +/// An iterator that considers its location to be CLI args. It +/// iterates through an underlying iterator and increments a counter +/// to track the current CLI arg index. +pub fn ArgsIterator(comptime Iterator: type) type { + return struct { + const Self = @This(); + + /// The underlying args iterator. + iterator: Iterator, + + /// Our current index into the iterator. This is 1-indexed. + /// The 0 value is used to indicate that we haven't read any + /// values yet. + index: usize = 0, + + pub fn next(self: *Self) ?[]const u8 { + const value = self.iterator.next() orelse return null; + self.index += 1; + return value; + } + + /// Returns a location for a diagnostic message. + pub fn location(self: *const Self) ?Diagnostic.Location { + return .{ .cli = self.index }; + } + }; +} + /// Returns an iterator (implements "next") that reads CLI args by line. /// Each CLI arg is expected to be a single line. This is used to implement /// configuration files. @@ -946,8 +974,7 @@ pub fn LineIterator(comptime ReaderType: type) type { return self.entry[0 .. buf.len + 2]; } - /// Modify the diagnostic so it includes richer context about - /// the location of the error. + /// Returns a location for a diagnostic message. 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; diff --git a/src/config/Config.zig b/src/config/Config.zig index fddea43a4..8d33ef5b1 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2372,7 +2372,11 @@ pub fn loadCliArgs(self: *Config, alloc_gpa: Allocator) !void { if (iter.next()) |argv0| log.debug("skipping argv0 value={s}", .{argv0}); // Parse the config from the CLI args - try self.loadIter(alloc_gpa, &iter); + { + const ArgsIter = cli.args.ArgsIterator(@TypeOf(iter)); + var args_iter: ArgsIter = .{ .iterator = iter }; + try self.loadIter(alloc_gpa, &args_iter); + } // If we are not loading the default files, then we need to // replay the steps up to this point so that we can rebuild From 70c175e2a6fc47b612f764199920b9a0daf99d3a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 17 Oct 2024 08:02:28 -0700 Subject: [PATCH 4/8] c: remove the config load string API It was unused and doesn't match our diagnostic API. --- include/ghostty.h | 1 - src/cli.zig | 2 +- src/cli/args.zig | 8 +++---- src/cli/diagnostics.zig | 50 ++++++++++++++++++++--------------------- src/config/CAPI.zig | 18 --------------- src/config/Config.zig | 2 +- 6 files changed, 31 insertions(+), 50 deletions(-) diff --git a/include/ghostty.h b/include/ghostty.h index 5f0ee0fe9..0f4c65f56 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -607,7 +607,6 @@ ghostty_info_s ghostty_info(void); ghostty_config_t ghostty_config_new(); void ghostty_config_free(ghostty_config_t); void ghostty_config_load_cli_args(ghostty_config_t); -void ghostty_config_load_string(ghostty_config_t, const char*, uintptr_t); void ghostty_config_load_default_files(ghostty_config_t); void ghostty_config_load_recursive_files(ghostty_config_t); void ghostty_config_finalize(ghostty_config_t); diff --git a/src/cli.zig b/src/cli.zig index 52527e21b..4336501a8 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -4,7 +4,7 @@ pub const args = @import("cli/args.zig"); pub const Action = @import("cli/action.zig").Action; pub const DiagnosticList = diags.DiagnosticList; pub const Diagnostic = diags.Diagnostic; -pub const Location = diags.Diagnostic.Location; +pub const Location = diags.Location; test { @import("std").testing.refAllDecls(@This()); diff --git a/src/cli/args.zig b/src/cli/args.zig index da8685e26..ab66ba4fb 100644 --- a/src/cli/args.zig +++ b/src/cli/args.zig @@ -129,7 +129,7 @@ pub fn parse( try dst._diagnostics.append(arena_alloc, .{ .key = try arena_alloc.dupeZ(u8, key), .message = message, - .location = Diagnostic.Location.fromIter(iter), + .location = diags.Location.fromIter(iter), }); }; } @@ -475,7 +475,7 @@ test "parse: diagnostic tracking" { try testing.expect(data._diagnostics.items().len == 1); { const diag = data._diagnostics.items()[0]; - try testing.expectEqual(Diagnostic.Location.none, diag.location); + try testing.expectEqual(diags.Location.none, diag.location); try testing.expectEqualStrings("what", diag.key); try testing.expectEqualStrings("unknown field", diag.message); } @@ -878,7 +878,7 @@ pub fn ArgsIterator(comptime Iterator: type) type { } /// Returns a location for a diagnostic message. - pub fn location(self: *const Self) ?Diagnostic.Location { + pub fn location(self: *const Self) ?diags.Location { return .{ .cli = self.index }; } }; @@ -975,7 +975,7 @@ pub fn LineIterator(comptime ReaderType: type) type { } /// Returns a location for a diagnostic message. - pub fn location(self: *const Self) ?Diagnostic.Location { + pub fn location(self: *const Self) ?diags.Location { // If we have no filepath then we have no location. if (self.filepath.len == 0) return null; diff --git a/src/cli/diagnostics.zig b/src/cli/diagnostics.zig index 80dc06b5a..c808c104a 100644 --- a/src/cli/diagnostics.zig +++ b/src/cli/diagnostics.zig @@ -15,31 +15,6 @@ pub const Diagnostic = struct { 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) { @@ -61,6 +36,31 @@ pub const Diagnostic = struct { } }; +/// 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; + } +}; + /// 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. diff --git a/src/config/CAPI.zig b/src/config/CAPI.zig index dc81a97ed..bf86a0954 100644 --- a/src/config/CAPI.zig +++ b/src/config/CAPI.zig @@ -39,24 +39,6 @@ export fn ghostty_config_load_cli_args(self: *Config) void { }; } -/// Load the configuration from a string in the same format as -/// the file-based syntax for the desktop version of the terminal. -export fn ghostty_config_load_string( - self: *Config, - str: [*]const u8, - len: usize, -) void { - config_load_string_(self, str[0..len]) catch |err| { - log.err("error loading config err={}", .{err}); - }; -} - -fn config_load_string_(self: *Config, str: []const u8) !void { - var fbs = std.io.fixedBufferStream(str); - var iter = cli.args.lineIterator(fbs.reader()); - try cli.args.parse(Config, global.alloc, self, &iter); -} - /// Load the configuration from the default file locations. This /// is usually done first. The default file locations are locations /// such as the home directory. diff --git a/src/config/Config.zig b/src/config/Config.zig index 8d33ef5b1..6c21b6dfc 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2731,7 +2731,7 @@ pub fn parseManuallyHook( if (command.items.len == 0) { try self._diagnostics.append(alloc, .{ - .location = cli.Diagnostic.Location.fromIter(iter), + .location = cli.Location.fromIter(iter), .message = try std.fmt.allocPrintZ( alloc, "missing command after {s}", From 4e25840e08c9de6790924a515c54b6179179b198 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 17 Oct 2024 08:13:35 -0700 Subject: [PATCH 5/8] apprt/gtk: support new config diagnostics API --- src/apprt/gtk/App.zig | 12 ++++++++---- src/apprt/gtk/ConfigErrorsWindow.zig | 19 ++++++++++++++++--- src/cli/diagnostics.zig | 2 +- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index 67d08812e..6c17e3d3e 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -123,9 +123,13 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { errdefer config.deinit(); // If we had configuration errors, then log them. - if (!config._errors.empty()) { - for (config._errors.list.items) |err| { - log.warn("configuration error: {s}", .{err.message}); + if (!config._diagnostics.empty()) { + var buf = std.ArrayList(u8).init(core_app.alloc); + defer buf.deinit(); + for (config._diagnostics.items()) |diag| { + try diag.write(buf.writer()); + log.warn("configuration error: {s}", .{buf.items}); + buf.clearRetainingCapacity(); } } @@ -815,7 +819,7 @@ fn syncConfigChanges(self: *App) !void { /// there are new configuration errors and hide the window if the errors /// are resolved. fn updateConfigErrors(self: *App) !void { - if (!self.config._errors.empty()) { + if (!self.config._diagnostics.empty()) { if (self.config_errors_window == null) { try ConfigErrorsWindow.create(self); assert(self.config_errors_window != null); diff --git a/src/apprt/gtk/ConfigErrorsWindow.zig b/src/apprt/gtk/ConfigErrorsWindow.zig index 44fa83363..3f8ba3205 100644 --- a/src/apprt/gtk/ConfigErrorsWindow.zig +++ b/src/apprt/gtk/ConfigErrorsWindow.zig @@ -28,7 +28,7 @@ pub fn create(app: *App) !void { } pub fn update(self: *ConfigErrors) void { - if (self.app.config._errors.empty()) { + if (self.app.config._diagnostics.empty()) { c.gtk_window_destroy(@ptrCast(self.window)); return; } @@ -130,8 +130,21 @@ const PrimaryView = struct { const buf = c.gtk_text_buffer_new(null); errdefer c.g_object_unref(buf); - for (config._errors.list.items) |err| { - c.gtk_text_buffer_insert_at_cursor(buf, err.message, @intCast(err.message.len)); + var msg_buf: [4096]u8 = undefined; + var fbs = std.io.fixedBufferStream(&msg_buf); + + for (config._diagnostics.items()) |diag| { + fbs.reset(); + diag.write(fbs.writer()) catch |err| { + log.warn( + "error writing diagnostic to buffer err={}", + .{err}, + ); + continue; + }; + + const msg = fbs.getWritten(); + c.gtk_text_buffer_insert_at_cursor(buf, msg.ptr, @intCast(msg.len)); c.gtk_text_buffer_insert_at_cursor(buf, "\n", -1); } diff --git a/src/cli/diagnostics.zig b/src/cli/diagnostics.zig index c808c104a..1e56379c8 100644 --- a/src/cli/diagnostics.zig +++ b/src/cli/diagnostics.zig @@ -118,7 +118,7 @@ pub const DiagnosticList = struct { return self.list.items.len == 0; } - pub fn items(self: *DiagnosticList) []Diagnostic { + pub fn items(self: *const DiagnosticList) []const Diagnostic { return self.list.items; } }; From 940a46d41f8cc6a92f2f8f93b108d556410e22f2 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 17 Oct 2024 21:39:34 -0700 Subject: [PATCH 6/8] cli: positional arguments are invalid when parsing configuration --- src/cli/args.zig | 109 ++++++++++++++++++++++++++-------------- src/cli/diagnostics.zig | 15 ++++++ 2 files changed, 87 insertions(+), 37 deletions(-) diff --git a/src/cli/args.zig b/src/cli/args.zig index ab66ba4fb..3dcc08dac 100644 --- a/src/cli/args.zig +++ b/src/cli/args.zig @@ -93,46 +93,60 @@ pub fn parse( } } - if (mem.startsWith(u8, arg, "--")) { - var key: []const u8 = arg[2..]; - const value: ?[]const u8 = value: { - // If the arg has "=" then the value is after the "=". - if (mem.indexOf(u8, key, "=")) |idx| { - defer key = key[0..idx]; - break :value key[idx + 1 ..]; - } + // If this doesn't start with "--" then it isn't a config + // flag. We don't support positional arguments or configuration + // values set with spaces so this is an error. + if (!mem.startsWith(u8, arg, "--")) { + if (comptime !canTrackDiags(T)) return Error.InvalidField; - break :value null; - }; + // Add our diagnostic + try dst._diagnostics.append(arena_alloc, .{ + .key = try arena_alloc.dupeZ(u8, arg), + .message = "invalid field", + .location = diags.Location.fromIter(iter), + }); - parseIntoField(T, arena_alloc, dst, key, value) catch |err| { - if (comptime !canTrackDiags(T)) return err; - - // The error set is dependent on comptime T, so we always add - // an extra error so we can have the "else" below. - const ErrSet = @TypeOf(err) || error{Unknown}; - const message: [:0]const u8 = switch (@as(ErrSet, @errorCast(err))) { - // OOM is not recoverable since we need to allocate to - // track more error messages. - error.OutOfMemory => return err, - error.InvalidField => "unknown field", - error.ValueRequired => "value required", - error.InvalidValue => "invalid value", - else => try std.fmt.allocPrintZ( - arena_alloc, - "unknown error {}", - .{err}, - ), - }; - - // Add our diagnostic - try dst._diagnostics.append(arena_alloc, .{ - .key = try arena_alloc.dupeZ(u8, key), - .message = message, - .location = diags.Location.fromIter(iter), - }); - }; + continue; } + + var key: []const u8 = arg[2..]; + const value: ?[]const u8 = value: { + // If the arg has "=" then the value is after the "=". + if (mem.indexOf(u8, key, "=")) |idx| { + defer key = key[0..idx]; + break :value key[idx + 1 ..]; + } + + break :value null; + }; + + parseIntoField(T, arena_alloc, dst, key, value) catch |err| { + if (comptime !canTrackDiags(T)) return err; + + // The error set is dependent on comptime T, so we always add + // an extra error so we can have the "else" below. + const ErrSet = @TypeOf(err) || error{Unknown}; + const message: [:0]const u8 = switch (@as(ErrSet, @errorCast(err))) { + // OOM is not recoverable since we need to allocate to + // track more error messages. + error.OutOfMemory => return err, + error.InvalidField => "unknown field", + error.ValueRequired => "value required", + error.InvalidValue => "invalid value", + else => try std.fmt.allocPrintZ( + arena_alloc, + "unknown error {}", + .{err}, + ), + }; + + // Add our diagnostic + try dst._diagnostics.append(arena_alloc, .{ + .key = try arena_alloc.dupeZ(u8, key), + .message = message, + .location = diags.Location.fromIter(iter), + }); + }; } } @@ -452,6 +466,27 @@ test "parse: empty value resets to default" { try testing.expect(!data.b); } +test "parse: positional arguments are invalid" { + const testing = std.testing; + + var data: struct { + a: u8 = 42, + _arena: ?ArenaAllocator = null, + } = .{}; + defer if (data._arena) |arena| arena.deinit(); + + var iter = try std.process.ArgIteratorGeneral(.{}).init( + testing.allocator, + "--a=84 what", + ); + defer iter.deinit(); + try testing.expectError( + error.InvalidField, + parse(@TypeOf(data), testing.allocator, &data, &iter), + ); + try testing.expectEqual(@as(u8, 84), data.a); +} + test "parse: diagnostic tracking" { const testing = std.testing; diff --git a/src/cli/diagnostics.zig b/src/cli/diagnostics.zig index 1e56379c8..e4d390c03 100644 --- a/src/cli/diagnostics.zig +++ b/src/cli/diagnostics.zig @@ -46,6 +46,8 @@ pub const Location = union(enum) { line: usize, }, + pub const Key = @typeInfo(Location).Union.tag_type.?; + pub fn fromIter(iter: anytype) Location { const Iter = t: { const T = @TypeOf(iter); @@ -121,4 +123,17 @@ pub const DiagnosticList = struct { pub fn items(self: *const DiagnosticList) []const Diagnostic { return self.list.items; } + + /// Returns true if there are any diagnostics for the given + /// location type. + pub fn containsLocation( + self: *const DiagnosticList, + location: Location.Key, + ) bool { + for (self.list.items) |diag| { + if (diag.location == location) return true; + } + + return false; + } }; From 463f4afc0529f3992aecdf552cfe0f3e81796722 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 18 Oct 2024 08:14:40 -0700 Subject: [PATCH 7/8] apprt/glfw: exit with invalid CLI args --- src/App.zig | 11 ++++++++++- src/apprt/glfw.zig | 10 ++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/App.zig b/src/App.zig index b1ea2eb7b..0f9a0d89b 100644 --- a/src/App.zig +++ b/src/App.zig @@ -231,10 +231,19 @@ fn drainMailbox(self: *App, rt_app: *apprt.App) !void { .open_config => try self.performAction(rt_app, .open_config), .new_window => |msg| try self.newWindow(rt_app, msg), .close => |surface| self.closeSurface(surface), - .quit => self.setQuit(), .surface_message => |msg| try self.surfaceMessage(msg.surface, msg.message), .redraw_surface => |surface| self.redrawSurface(rt_app, surface), .redraw_inspector => |surface| self.redrawInspector(rt_app, surface), + + // If we're quitting, then we set the quit flag and stop + // draining the mailbox immediately. This lets us defer + // mailbox processing to the next tick so that the apprt + // can try to quick as quickly as possible. + .quit => { + log.info("quit message received, short circuiting mailbox drain", .{}); + self.setQuit(); + return; + }, } } } diff --git a/src/apprt/glfw.zig b/src/apprt/glfw.zig index 0f0be5480..668dd9143 100644 --- a/src/apprt/glfw.zig +++ b/src/apprt/glfw.zig @@ -11,6 +11,7 @@ const Allocator = std.mem.Allocator; const glfw = @import("glfw"); const macos = @import("macos"); const objc = @import("objc"); +const cli = @import("../cli.zig"); const input = @import("../input.zig"); const internal_os = @import("../os/main.zig"); const renderer = @import("../renderer.zig"); @@ -77,9 +78,18 @@ pub const App = struct { log.warn("configuration error: {s}", .{buf.items}); buf.clearRetainingCapacity(); } + + // If we have any CLI errors, exit. + if (config._diagnostics.containsLocation(.cli)) { + log.warn("CLI errors detected, exiting", .{}); + _ = core_app.mailbox.push(.{ + .quit = {}, + }, .{ .forever = {} }); + } } // Queue a single new window that starts on launch + // Note: above we may send a quit so this may never happen _ = core_app.mailbox.push(.{ .new_window = .{}, }, .{ .forever = {} }); From be3fc5c04a21109538adc7354bc6cbc571b4e6e3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 18 Oct 2024 08:26:17 -0700 Subject: [PATCH 8/8] apprt/gtk: exit if there are CLI errors --- src/apprt/gtk/App.zig | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig index 6c17e3d3e..10a2bdc02 100644 --- a/src/apprt/gtk/App.zig +++ b/src/apprt/gtk/App.zig @@ -131,6 +131,12 @@ pub fn init(core_app: *CoreApp, opts: Options) !App { log.warn("configuration error: {s}", .{buf.items}); buf.clearRetainingCapacity(); } + + // If we have any CLI errors, exit. + if (config._diagnostics.containsLocation(.cli)) { + log.warn("CLI errors detected, exiting", .{}); + std.posix.exit(1); + } } c.gtk_init(); @@ -1368,10 +1374,7 @@ fn gtkActionQuit( ud: ?*anyopaque, ) callconv(.C) void { const self: *App = @ptrCast(@alignCast(ud orelse return)); - self.core_app.setQuit() catch |err| { - log.warn("error setting quit err={}", .{err}); - return; - }; + self.core_app.setQuit(); } /// Action sent by the window manager asking us to present a specific surface to