From 431c99313c13546052fb687108123f4a84abf954 Mon Sep 17 00:00:00 2001 From: Remi Gelinas Date: Wed, 17 Jul 2024 12:27:12 -0400 Subject: [PATCH 1/9] feat(cli): add initial validate-config action --- src/cli/action.zig | 5 ++++ src/cli/validate_config.zig | 59 +++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 src/cli/validate_config.zig diff --git a/src/cli/action.zig b/src/cli/action.zig index a7cbabf25..68bcf6448 100644 --- a/src/cli/action.zig +++ b/src/cli/action.zig @@ -9,6 +9,7 @@ const list_keybinds = @import("list_keybinds.zig"); const list_themes = @import("list_themes.zig"); const list_colors = @import("list_colors.zig"); const show_config = @import("show_config.zig"); +const validate_config = @import("validate_config.zig"); /// Special commands that can be invoked via CLI flags. These are all /// invoked by using `+` as a CLI flag. The only exception is @@ -35,6 +36,9 @@ pub const Action = enum { /// Dump the config to stdout @"show-config", + // Validate passed config file + @"validate-config", + pub const Error = error{ /// Multiple actions were detected. You can specify at most one /// action on the CLI otherwise the behavior desired is ambiguous. @@ -124,6 +128,7 @@ pub const Action = enum { .@"list-themes" => try list_themes.run(alloc), .@"list-colors" => try list_colors.run(alloc), .@"show-config" => try show_config.run(alloc), + .@"validate-config" => try validate_config.run(alloc), }; } diff --git a/src/cli/validate_config.zig b/src/cli/validate_config.zig new file mode 100644 index 000000000..7bd8e8032 --- /dev/null +++ b/src/cli/validate_config.zig @@ -0,0 +1,59 @@ +const std = @import("std"); +const Allocator = std.mem.Allocator; +const args = @import("args.zig"); +const Action = @import("action.zig").Action; +const Config = @import("../config.zig").Config; +const cli = @import("../cli.zig"); + +pub const Options = struct { + /// The path of the config file to validate + @"config-file": ?[:0]const u8 = null, + + pub fn deinit(self: Options) void { + _ = self; + } + + /// Enables "-h" and "--help" to work. + pub fn help(self: Options) !void { + _ = self; + return Action.help_error; + } +}; + +/// The `validate-config` command is used to validate a Ghostty config +pub fn run(alloc: std.mem.Allocator) !u8 { + var opts: Options = .{}; + defer opts.deinit(); + + { + var iter = try std.process.argsWithAllocator(alloc); + defer iter.deinit(); + try args.parse(Options, alloc, &opts, &iter); + } + + const stdout = std.io.getStdOut().writer(); + + // If a config path is passed, validate it, otherwise validate usual config options + if (opts.@"config-file") |config_path| { + const cwd = std.fs.cwd(); + + if (cwd.openFile(config_path, .{})) |file| { + defer file.close(); + + var cfg = try Config.default(alloc); + defer cfg.deinit(); + + var buf_reader = std.io.bufferedReader(file.reader()); + var iter = cli.args.lineIterator(buf_reader.reader()); + try cfg.loadIter(alloc, &iter); + try cfg.loadRecursiveFiles(alloc); + try cfg.finalize(); + } else |err| { + try stdout.print("{any}", .{err}); + } + } else { + _ = try Config.load(alloc); + } + + return 0; +} From 3350e3c848d95310d4b76a5ea0f2c92245e578e8 Mon Sep 17 00:00:00 2001 From: Remi Gelinas Date: Wed, 17 Jul 2024 15:02:03 -0400 Subject: [PATCH 2/9] refactor: move config file loading into loadFile --- src/cli/validate_config.zig | 25 +++++++++---------------- src/config/Config.zig | 27 ++++++++++++++++----------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/cli/validate_config.zig b/src/cli/validate_config.zig index 7bd8e8032..7f50e612b 100644 --- a/src/cli/validate_config.zig +++ b/src/cli/validate_config.zig @@ -33,26 +33,19 @@ pub fn run(alloc: std.mem.Allocator) !u8 { const stdout = std.io.getStdOut().writer(); - // If a config path is passed, validate it, otherwise validate usual config options + var cfg = try Config.default(alloc); + defer cfg.deinit(); + + // If a config path is passed, validate it, otherwise validate default configs if (opts.@"config-file") |config_path| { - const cwd = std.fs.cwd(); + try cfg.loadFile(alloc, config_path); - if (cwd.openFile(config_path, .{})) |file| { - defer file.close(); - - var cfg = try Config.default(alloc); - defer cfg.deinit(); - - var buf_reader = std.io.bufferedReader(file.reader()); - var iter = cli.args.lineIterator(buf_reader.reader()); - try cfg.loadIter(alloc, &iter); - try cfg.loadRecursiveFiles(alloc); - try cfg.finalize(); - } else |err| { - try stdout.print("{any}", .{err}); + if (!cfg._errors.empty()) { + try stdout.print("Config is not valid path={s}", .{config_path}); + return 1; } } else { - _ = try Config.load(alloc); + try cfg.loadDefaultFiles(alloc); } return 0; diff --git a/src/config/Config.zig b/src/config/Config.zig index 6f780bb33..6f27597fc 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1686,22 +1686,27 @@ pub fn loadIter( try cli.args.parse(Config, alloc, self, iter); } +pub fn loadFile(self: *Config, alloc: Allocator, path: []const u8) !void { + const cwd = std.fs.cwd(); + + var file = try cwd.openFile(path, .{}); + defer file.close(); + + 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()); + try self.loadIter(alloc, &iter); + try self.expandPaths(std.fs.path.dirname(path).?); +} + /// Load the configuration from the default configuration file. The default /// configuration file is at `$XDG_CONFIG_HOME/ghostty/config`. pub fn loadDefaultFiles(self: *Config, alloc: Allocator) !void { const config_path = try internal_os.xdg.config(alloc, .{ .subdir = "ghostty/config" }); defer alloc.free(config_path); - const cwd = std.fs.cwd(); - if (cwd.openFile(config_path, .{})) |file| { - defer file.close(); - std.log.info("reading configuration file path={s}", .{config_path}); - - var buf_reader = std.io.bufferedReader(file.reader()); - var iter = cli.args.lineIterator(buf_reader.reader()); - try self.loadIter(alloc, &iter); - try self.expandPaths(std.fs.path.dirname(config_path).?); - } else |err| switch (err) { + self.loadFile(alloc, config_path) catch |err| switch (err) { error.FileNotFound => std.log.info( "homedir config not found, not loading path={s}", .{config_path}, @@ -1711,7 +1716,7 @@ pub fn loadDefaultFiles(self: *Config, alloc: Allocator) !void { "error reading config file, not loading err={} path={s}", .{ err, config_path }, ), - } + }; } /// Load and parse the CLI args. From 368868f7129ee28e72f33f34a00abea79d5e64bc Mon Sep 17 00:00:00 2001 From: Remi Gelinas Date: Wed, 17 Jul 2024 16:51:16 -0400 Subject: [PATCH 3/9] fix: handle relative paths in CLI --- src/cli/validate_config.zig | 5 ++++- src/config/Config.zig | 4 +--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/cli/validate_config.zig b/src/cli/validate_config.zig index 7f50e612b..e82cca21d 100644 --- a/src/cli/validate_config.zig +++ b/src/cli/validate_config.zig @@ -38,7 +38,10 @@ pub fn run(alloc: std.mem.Allocator) !u8 { // If a config path is passed, validate it, otherwise validate default configs if (opts.@"config-file") |config_path| { - try cfg.loadFile(alloc, config_path); + var buf: [std.fs.MAX_PATH_BYTES]u8 = undefined; + const abs_path = try std.fs.cwd().realpath(config_path, &buf); + + try cfg.loadFile(alloc, abs_path); if (!cfg._errors.empty()) { try stdout.print("Config is not valid path={s}", .{config_path}); diff --git a/src/config/Config.zig b/src/config/Config.zig index 6f27597fc..ca185f2ed 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1687,9 +1687,7 @@ pub fn loadIter( } pub fn loadFile(self: *Config, alloc: Allocator, path: []const u8) !void { - const cwd = std.fs.cwd(); - - var file = try cwd.openFile(path, .{}); + var file = try std.fs.cwd().openFile(path, .{}); defer file.close(); std.log.info("reading configuration file path={s}", .{path}); From a546da04177668f0e857a5110f0dd1f955296df4 Mon Sep 17 00:00:00 2001 From: Remi Gelinas Date: Wed, 17 Jul 2024 17:03:09 -0400 Subject: [PATCH 4/9] feat: print every config error message --- src/cli/validate_config.zig | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/cli/validate_config.zig b/src/cli/validate_config.zig index e82cca21d..b799c6ccf 100644 --- a/src/cli/validate_config.zig +++ b/src/cli/validate_config.zig @@ -42,14 +42,17 @@ pub fn run(alloc: std.mem.Allocator) !u8 { const abs_path = try std.fs.cwd().realpath(config_path, &buf); try cfg.loadFile(alloc, abs_path); - - if (!cfg._errors.empty()) { - try stdout.print("Config is not valid path={s}", .{config_path}); - return 1; - } } else { try cfg.loadDefaultFiles(alloc); } - return 0; + if (!cfg._errors.empty()) { + for (cfg._errors.list.items) |err| { + try stdout.print("{s}\n", .{err.message}); + } + + return 1; + } + + return 1; } From 0197f6d15e29f9158866664ed34ac6443d8f8753 Mon Sep 17 00:00:00 2001 From: Remi Gelinas Date: Wed, 17 Jul 2024 17:27:31 -0400 Subject: [PATCH 5/9] fix: handle recursive config files --- src/cli/validate_config.zig | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/cli/validate_config.zig b/src/cli/validate_config.zig index b799c6ccf..4dcf7f72a 100644 --- a/src/cli/validate_config.zig +++ b/src/cli/validate_config.zig @@ -42,17 +42,20 @@ pub fn run(alloc: std.mem.Allocator) !u8 { const abs_path = try std.fs.cwd().realpath(config_path, &buf); try cfg.loadFile(alloc, abs_path); + try cfg.loadRecursiveFiles(alloc); } else { - try cfg.loadDefaultFiles(alloc); + cfg = try Config.load(alloc); } + try cfg.finalize(); + if (!cfg._errors.empty()) { for (cfg._errors.list.items) |err| { try stdout.print("{s}\n", .{err.message}); } - return 1; + return 65; } - return 1; + return 0; } From 699fce0ee543875ba4945f151a146321199ab69a Mon Sep 17 00:00:00 2001 From: Remi Gelinas Date: Thu, 18 Jul 2024 11:03:23 -0400 Subject: [PATCH 6/9] docs: add docstrings --- src/cli/validate_config.zig | 7 ++++++- src/config/Config.zig | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/cli/validate_config.zig b/src/cli/validate_config.zig index 4dcf7f72a..b61e0cdaf 100644 --- a/src/cli/validate_config.zig +++ b/src/cli/validate_config.zig @@ -20,7 +20,12 @@ pub const Options = struct { } }; -/// The `validate-config` command is used to validate a Ghostty config +/// The `validate-config` command is used to validate a Ghostty config file. +/// +/// When executed without any arguments, this will load the config from the default location. +/// +/// The `--config-file` argument can be passed to validate a specific target config +/// file in a non-default location. pub fn run(alloc: std.mem.Allocator) !u8 { var opts: Options = .{}; defer opts.deinit(); diff --git a/src/config/Config.zig b/src/config/Config.zig index ca185f2ed..090c8a77d 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1686,6 +1686,7 @@ pub fn loadIter( try cli.args.parse(Config, alloc, self, iter); } +/// Load configuration from the target config file at `path`. pub fn loadFile(self: *Config, alloc: Allocator, path: []const u8) !void { var file = try std.fs.cwd().openFile(path, .{}); defer file.close(); From c6cf13ac89965bd84279f5f44e7fedccd70bf1a7 Mon Sep 17 00:00:00 2001 From: Remi Gelinas Date: Sun, 21 Jul 2024 10:19:46 -0400 Subject: [PATCH 7/9] feat: add absolute path assertion --- src/config/Config.zig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/config/Config.zig b/src/config/Config.zig index 090c8a77d..65a0591da 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1688,6 +1688,8 @@ pub fn loadIter( /// Load configuration from the target config file at `path`. pub fn loadFile(self: *Config, alloc: Allocator, path: []const u8) !void { + assert(std.fs.path.isAbsolute(path)); + var file = try std.fs.cwd().openFile(path, .{}); defer file.close(); From 4f182c5578fc0cc221e1c5e7e17a960802660ec9 Mon Sep 17 00:00:00 2001 From: Remi Gelinas Date: Sun, 21 Jul 2024 11:36:43 -0400 Subject: [PATCH 8/9] docs: specify path must be absolute --- src/config/Config.zig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/config/Config.zig b/src/config/Config.zig index 65a0591da..a9bda5410 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1687,6 +1687,8 @@ pub fn loadIter( } /// Load configuration from the target config file at `path`. +/// +/// `path` must be resolved and absolute. pub fn loadFile(self: *Config, alloc: Allocator, path: []const u8) !void { assert(std.fs.path.isAbsolute(path)); From b3e1b2e02aa69edba656b766a7474ab786245b35 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Jul 2024 09:48:23 -0700 Subject: [PATCH 9/9] some tweaks --- src/cli/validate_config.zig | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/cli/validate_config.zig b/src/cli/validate_config.zig index b61e0cdaf..d6fedc544 100644 --- a/src/cli/validate_config.zig +++ b/src/cli/validate_config.zig @@ -6,7 +6,8 @@ const Config = @import("../config.zig").Config; const cli = @import("../cli.zig"); pub const Options = struct { - /// The path of the config file to validate + /// The path of the config file to validate. If this isn't specified, + /// then the default config file paths will be validated. @"config-file": ?[:0]const u8 = null, pub fn deinit(self: Options) void { @@ -43,7 +44,7 @@ pub fn run(alloc: std.mem.Allocator) !u8 { // If a config path is passed, validate it, otherwise validate default configs if (opts.@"config-file") |config_path| { - var buf: [std.fs.MAX_PATH_BYTES]u8 = undefined; + var buf: [std.fs.max_path_bytes]u8 = undefined; const abs_path = try std.fs.cwd().realpath(config_path, &buf); try cfg.loadFile(alloc, abs_path); @@ -59,7 +60,7 @@ pub fn run(alloc: std.mem.Allocator) !u8 { try stdout.print("{s}\n", .{err.message}); } - return 65; + return 1; } return 0;