Include file/line context for config file errors

This commit is contained in:
Wilmer Paulino
2024-08-03 11:53:11 -07:00
parent 6dff92472a
commit fecf383d1c
6 changed files with 206 additions and 61 deletions

View File

@ -506,7 +506,7 @@ 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_string(ghostty_config_t, const char*, uintptr_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);

View File

@ -102,43 +102,111 @@ pub fn parse(comptime T: type, comptime Iter: type, alloc: Allocator, dst: *T, i
// 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};
var message: [:0]u8 = undefined;
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 => try dst._errors.add(arena_alloc, .{
.message = try std.fmt.allocPrintZ(
error.InvalidField => {
if (@hasDecl(Iter, "lineContext")) {
const line_context = iter.lineContext();
message = try std.fmt.allocPrintZ(
arena_alloc,
"{s}:{}:{}: unknown field {s}",
.{
iter.filepath,
line_context.line_number,
line_context.column_number_key,
key,
},
);
} else {
message = try std.fmt.allocPrintZ(
arena_alloc,
"{s}: unknown field",
.{key},
),
}),
);
}
},
error.ValueRequired => try dst._errors.add(arena_alloc, .{
.message = try std.fmt.allocPrintZ(
error.ValueRequired => {
if (@hasDecl(Iter, "lineContext")) {
const line_context = iter.lineContext();
message = try std.fmt.allocPrintZ(
arena_alloc,
"{s}:{}:{}: value required for {s}",
.{
iter.filepath,
line_context.line_number,
if (line_context.column_number_value) |column_number_value|
column_number_value
else
line_context.column_number_key,
key,
},
);
} else {
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 },
),
}),
);
}
},
error.InvalidValue => {
// The value must exist if it was invalid.
assert(value != null);
if (@hasDecl(Iter, "lineContext")) {
const line_context = iter.lineContext();
message = try std.fmt.allocPrintZ(
arena_alloc,
"{s}:{}:{}: invalid value {s} for {s}",
.{
iter.filepath,
line_context.line_number,
line_context.column_number_value.?,
value.?,
key,
},
);
} else {
message = try std.fmt.allocPrintZ(
arena_alloc,
"{s}: invalid value {s}",
.{ key, value.? },
);
}
},
else => {
// Any other errors will likely be an issue with the value since we've
// already handled errors for invalid keys and missing values above.
assert(value != null);
if (@hasDecl(Iter, "lineContext")) {
const line_context = iter.lineContext();
message = try std.fmt.allocPrintZ(
arena_alloc,
"{s}:{}:{}: {} for value {s}",
.{
iter.filepath,
line_context.line_number,
line_context.column_number_value.?,
err,
value.?,
},
);
} else {
message = try std.fmt.allocPrintZ(
arena_alloc,
"{s}: {} for value {s}",
.{ key, err, value.? },
);
}
},
}
try dst._errors.add(arena_alloc, .{ .message = message });
};
}
}
@ -748,6 +816,12 @@ test "parseIntoField: struct with parse func with unsupported error tracking" {
/// configuration files.
pub fn LineIterator(comptime ReaderType: type) type {
return struct {
const LineContext = struct {
line_number: usize,
column_number_key: usize,
column_number_value: ?usize,
};
const Self = @This();
/// The maximum size a single line can be. We don't expect any
@ -756,7 +830,13 @@ pub fn LineIterator(comptime ReaderType: type) type {
pub const MAX_LINE_SIZE = 4096;
r: ReaderType,
filepath: []const u8,
entry: [MAX_LINE_SIZE]u8 = [_]u8{ '-', '-' } ++ ([_]u8{0} ** (MAX_LINE_SIZE - 2)),
line_context: LineContext = LineContext{
.line_number = 0,
.column_number_key = 0,
.column_number_value = null,
},
pub fn next(self: *Self) ?[]const u8 {
// TODO: detect "--" prefixed lines and give a friendlier error
@ -768,6 +848,33 @@ pub fn LineIterator(comptime ReaderType: type) type {
unreachable;
} orelse return null;
self.line_context.line_number += 1;
if (mem.indexOfNone(u8, entry, whitespace)) |index_key| {
self.line_context.column_number_key = index_key + 1;
if (mem.indexOf(u8, entry, "=")) |index_separator| {
const entry_after_separator = entry[index_separator + 1 ..];
if (mem.indexOfNone(
u8,
entry_after_separator,
whitespace ++ "\"",
)) |index_value| {
// We add two because each index is 0-based and we need to represent
// them as 1-based.
self.line_context.column_number_value = index_separator + index_value + 2;
} else {
// No value found after a separator, point to it instead in case
// this is an error for a missing value.
self.line_context.column_number_value = index_separator + 1;
}
} else {
self.line_context.column_number_value = null;
}
} else {
// Empty line, we'll skip it below.
self.line_context.column_number_key = 0;
self.line_context.column_number_value = null;
}
// Trim any whitespace (including CR) around it
const trim = std.mem.trim(u8, entry, whitespace ++ "\r");
if (trim.len != entry.len) {
@ -815,12 +922,16 @@ pub fn LineIterator(comptime ReaderType: type) type {
// as CLI args.
return self.entry[0 .. buf.len + 2];
}
pub fn lineContext(self: *Self) LineContext {
return self.line_context;
}
};
}
// Constructs a LineIterator (see docs for that).
pub fn lineIterator(reader: anytype) LineIterator(@TypeOf(reader)) {
return .{ .r = reader };
pub fn lineIterator(reader: anytype, filepath: []const u8) LineIterator(@TypeOf(reader)) {
return .{ .r = reader, .filepath = filepath };
}
/// An iterator valid for arg parsing from a slice.
@ -853,28 +964,57 @@ test "LineIterator" {
\\D
\\
\\ # An indented comment
\\ E
\\ E = value
\\
\\# A quoted string with whitespace
\\F= "value "
\\
);
var iter = lineIterator(fbs.reader());
var iter = lineIterator(fbs.reader(), "");
try testing.expectEqualStrings("--A", iter.next().?);
try testing.expectEqual(1, iter.lineContext().line_number);
try testing.expectEqual(1, iter.lineContext().column_number_key);
try testing.expectEqual(null, iter.lineContext().column_number_value);
try testing.expectEqualStrings("--B=42", iter.next().?);
try testing.expectEqual(2, iter.lineContext().line_number);
try testing.expectEqual(1, iter.lineContext().column_number_key);
try testing.expectEqual(3, iter.lineContext().column_number_value.?);
try testing.expectEqualStrings("--C", iter.next().?);
try testing.expectEqual(3, iter.lineContext().line_number);
try testing.expectEqual(1, iter.lineContext().column_number_key);
try testing.expectEqual(null, iter.lineContext().column_number_value);
try testing.expectEqualStrings("--D", iter.next().?);
try testing.expectEqualStrings("--E", iter.next().?);
try testing.expectEqual(6, iter.lineContext().line_number);
try testing.expectEqual(1, iter.lineContext().column_number_key);
try testing.expectEqual(null, iter.lineContext().column_number_value);
try testing.expectEqualStrings("--E=value", iter.next().?);
try testing.expectEqual(9, iter.lineContext().line_number);
try testing.expectEqual(3, iter.lineContext().column_number_key);
try testing.expectEqual(7, iter.lineContext().column_number_value.?);
try testing.expectEqualStrings("--F=value ", iter.next().?);
try testing.expectEqual(12, iter.lineContext().line_number);
try testing.expectEqual(1, iter.lineContext().column_number_key);
try testing.expectEqual(6, iter.lineContext().column_number_value.?);
// No lines left, line context remains the same.
try testing.expectEqual(@as(?[]const u8, null), iter.next());
try testing.expectEqual(@as(?[]const u8, null), iter.next());
try testing.expectEqual(12, iter.lineContext().line_number);
try testing.expectEqual(1, iter.lineContext().column_number_key);
try testing.expectEqual(6, iter.lineContext().column_number_value);
}
test "LineIterator end in newline" {
const testing = std.testing;
var fbs = std.io.fixedBufferStream("A\n\n");
var iter = lineIterator(fbs.reader());
var iter = lineIterator(fbs.reader(), "");
try testing.expectEqualStrings("--A", iter.next().?);
try testing.expectEqual(@as(?[]const u8, null), iter.next());
try testing.expectEqual(@as(?[]const u8, null), iter.next());
@ -884,7 +1024,7 @@ test "LineIterator spaces around '='" {
const testing = std.testing;
var fbs = std.io.fixedBufferStream("A = B\n\n");
var iter = lineIterator(fbs.reader());
var iter = lineIterator(fbs.reader(), "");
try testing.expectEqualStrings("--A=B", iter.next().?);
try testing.expectEqual(@as(?[]const u8, null), iter.next());
try testing.expectEqual(@as(?[]const u8, null), iter.next());
@ -894,7 +1034,7 @@ test "LineIterator no value" {
const testing = std.testing;
var fbs = std.io.fixedBufferStream("A = \n\n");
var iter = lineIterator(fbs.reader());
var iter = lineIterator(fbs.reader(), "");
try testing.expectEqualStrings("--A=", iter.next().?);
try testing.expectEqual(@as(?[]const u8, null), iter.next());
}
@ -903,7 +1043,7 @@ test "LineIterator with CRLF line endings" {
const testing = std.testing;
var fbs = std.io.fixedBufferStream("A\r\nB = C\r\n");
var iter = lineIterator(fbs.reader());
var iter = lineIterator(fbs.reader(), "");
try testing.expectEqualStrings("--A", iter.next().?);
try testing.expectEqualStrings("--B=C", iter.next().?);
try testing.expectEqual(@as(?[]const u8, null), iter.next());

View File

@ -43,17 +43,19 @@ export fn ghostty_config_load_cli_args(self: *Config) void {
/// the file-based syntax for the desktop version of the terminal.
export fn ghostty_config_load_string(
self: *Config,
str: [*]const u8,
len: usize,
filepath: [*]const u8,
filepath_len: usize,
contents: [*]const u8,
contents_len: usize,
) void {
config_load_string_(self, str[0..len]) catch |err| {
config_load_string_(self, filepath[0..filepath_len], contents[0..contents_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());
fn config_load_string_(self: *Config, filepath: []const u8, contents: []const u8) !void {
var fbs = std.io.fixedBufferStream(contents);
var iter = cli.args.lineIterator(fbs.reader(), filepath);
try cli.args.parse(Config, @TypeOf(iter), global.alloc, self, &iter);
}

View File

@ -2077,7 +2077,7 @@ 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());
var iter = cli.args.lineIterator(buf_reader.reader(), path);
try self.loadIter(@TypeOf(iter), alloc, &iter);
try self.expandPaths(std.fs.path.dirname(path).?);
}
@ -2271,7 +2271,7 @@ 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());
var iter = cli.args.lineIterator(buf_reader.reader(), path);
try self.loadIter(@TypeOf(iter), alloc_gpa, &iter);
try self.expandPaths(std.fs.path.dirname(path).?);
}
@ -2302,11 +2302,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 file_with_path = (try themepkg.open(
self._arena.?.allocator(),
theme,
&self._errors,
)) orelse return;
const path = file_with_path.path;
const file = file_with_path.file;
defer file.close();
// From this point onwards, we load the theme and do a bit of a dance
@ -2332,7 +2334,7 @@ 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());
var iter = cli.args.lineIterator(buf_reader.reader(), path);
try new_config.loadIter(@TypeOf(iter), alloc_gpa, &iter);
// Replay our previous inputs so that we can override values

View File

@ -33,17 +33,19 @@ export fn config_free(ptr: ?*Config) void {
/// the file-based syntax for the desktop version of the terminal.
export fn config_load_string(
self: *Config,
str: [*]const u8,
len: usize,
filepath: [*]const u8,
filepath_len: usize,
contents: [*]const u8,
contents_len: usize,
) void {
config_load_string_(self, str[0..len]) catch |err| {
config_load_string_(self, filepath[0..filepath_len], contents[0..contents_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());
fn config_load_string_(self: *Config, filepath: []const u8, contents: []const u8) !void {
var fbs = std.io.fixedBufferStream(contents);
var iter = cli.args.lineIterator(fbs.reader(), filepath);
try cli.args.parse(Config, @TypeOf(iter), alloc, self, &iter);
}

View File

@ -108,14 +108,13 @@ pub fn open(
arena_alloc: Allocator,
theme: []const u8,
errors: *ErrorList,
) 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,
errors,
);
if (std.fs.path.isAbsolute(theme)) {
const file = try openAbsolute(arena_alloc, theme, errors) orelse return null;
return .{ .path = theme, .file = file };
}
const basename = std.fs.path.basename(theme);
if (!std.mem.eql(u8, theme, basename)) {
@ -136,7 +135,7 @@ pub fn open(
while (try it.next()) |loc| {
const path = try std.fs.path.join(arena_alloc, &.{ loc.dir, theme });
if (cwd.openFile(path, .{})) |file| {
return file;
return .{ .path = path, .file = file };
} else |err| switch (err) {
// Not an error, just continue to the next location.
error.FileNotFound => {},