Merge pull request #841 from pjz/cfgpath

config: make config-file names resolve relative to the config dir

src/config/Config.zig

@@ -503,7 +503,14 @@ keybind: Keybinds = .{},
 /// is determined by the OS settings. On every other platform it is 500ms.
 @"click-repeat-interval": u32 = 0,
 
-/// Additional configuration files to read.
+/// Additional configuration files to read. This configuration can be repeated
+/// to read multiple configuration files. Configuration files themselves can
+/// load more configuration files. Paths are relative to the file containing
+/// the `config-file` directive. For command-line arguments, paths are
+/// relative to the current working directory.
+///
+/// Cycles are not allowed. If a cycle is detected, an error will be logged
+/// and the configuration file will be ignored.
 @"config-file": RepeatableString = .{},
 
 /// Confirms that a surface should be closed before closing it. This defaults
@@ -1070,29 +1077,30 @@ fn ctrlOrSuper(mods: inputpkg.Mods) inputpkg.Mods {
     return copy;
 }
 
-/// Load the configuration from the default file locations. Currently,
-/// this loads from $XDG_CONFIG_HOME/ghostty/config.
+/// 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 home_config_path = try internal_os.xdg.config(alloc, .{ .subdir = "ghostty/config" });
-    defer alloc.free(home_config_path);
+    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(home_config_path, .{})) |file| {
+    if (cwd.openFile(config_path, .{})) |file| {
         defer file.close();
-        std.log.info("reading configuration file path={s}", .{home_config_path});
+        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 cli.args.parse(Config, alloc, self, &iter);
+        try self.expandConfigFiles(std.fs.path.dirname(config_path).?);
     } else |err| switch (err) {
         error.FileNotFound => std.log.info(
             "homedir config not found, not loading path={s}",
-            .{home_config_path},
+            .{config_path},
         ),
 
         else => std.log.warn(
-            "error reading homedir config file, not loading err={} path={s}",
-            .{ err, home_config_path },
+            "error reading config file, not loading err={} path={s}",
+            .{ err, config_path },
         ),
     }
 }
@@ -1110,19 +1118,47 @@ pub fn loadCliArgs(self: *Config, alloc_gpa: Allocator) !void {
     var iter = try std.process.argsWithAllocator(alloc_gpa);
     defer iter.deinit();
     try cli.args.parse(Config, alloc_gpa, self, &iter);
+
+    // Config files loaded from the CLI args are relative to pwd
+    if (self.@"config-file".list.items.len > 0) {
+        var buf: [std.fs.MAX_PATH_BYTES]u8 = undefined;
+        try self.expandConfigFiles(try std.fs.cwd().realpath(".", &buf));
+    }
 }
 
 /// Load and parse the config files that were added in the "config-file" key.
-pub fn loadRecursiveFiles(self: *Config, alloc: Allocator) !void {
-    // TODO(mitchellh): support nesting (config-file in a config file)
-    // TODO(mitchellh): detect cycles when nesting
-
+pub fn loadRecursiveFiles(self: *Config, alloc_gpa: Allocator) !void {
     if (self.@"config-file".list.items.len == 0) return;
-
     const arena_alloc = self._arena.?.allocator();
+
+    // Keeps track of loaded files to prevent cycles.
+    var loaded = std.StringHashMap(void).init(alloc_gpa);
+    defer loaded.deinit();
+
     const cwd = std.fs.cwd();
-    const len = self.@"config-file".list.items.len;
-    for (self.@"config-file".list.items) |path| {
+    var i: usize = 0;
+    while (i < self.@"config-file".list.items.len) : (i += 1) {
+        const path = self.@"config-file".list.items[i];
+
+        // Error paths
+        if (path.len == 0) continue;
+
+        // All paths should already be absolute at this point because
+        // they're fixed up after each load.
+        assert(std.fs.path.isAbsolute(path));
+
+        // We must only load a unique file once
+        if (try loaded.fetchPut(path, {}) != null) {
+            try self._errors.add(arena_alloc, .{
+                .message = try std.fmt.allocPrintZ(
+                    arena_alloc,
+                    "config-file {s}: cycle detected",
+                    .{path},
+                ),
+            });
+            continue;
+        }
+
         var file = cwd.openFile(path, .{}) catch |err| {
             try self._errors.add(arena_alloc, .{
                 .message = try std.fmt.allocPrintZ(
@@ -1135,22 +1171,41 @@ pub fn loadRecursiveFiles(self: *Config, alloc: Allocator) !void {
         };
         defer file.close();
 
+        log.info("loading config-file path={s}", .{path});
         var buf_reader = std.io.bufferedReader(file.reader());
         var iter = cli.args.lineIterator(buf_reader.reader());
-        try cli.args.parse(Config, alloc, self, &iter);
+        try cli.args.parse(Config, alloc_gpa, self, &iter);
+        try self.expandConfigFiles(std.fs.path.dirname(path).?);
+    }
+}
 
-        // We don't currently support adding more config files to load
-        // from within a loaded config file. This can be supported
-        // later.
-        if (self.@"config-file".list.items.len > len) {
+/// Expand the relative paths in config-files to be absolute paths
+/// relative to the base directory.
+fn expandConfigFiles(self: *Config, base: []const u8) !void {
+    assert(std.fs.path.isAbsolute(base));
+    var dir = try std.fs.cwd().openDir(base, .{});
+    defer dir.close();
+
+    const arena_alloc = self._arena.?.allocator();
+    for (self.@"config-file".list.items, 0..) |path, i| {
+        // If it is already absolute we can ignore it.
+        if (path.len == 0 or std.fs.path.isAbsolute(path)) continue;
+
+        // If it isn't absolute, we need to make it absolute relative to the base.
+        const abs = dir.realpathAlloc(arena_alloc, path) catch |err| {
             try self._errors.add(arena_alloc, .{
                 .message = try std.fmt.allocPrintZ(
                     arena_alloc,
-                    "config-file cannot be used in a config-file. Found in {s}",
-                    .{path},
+                    "error resolving config-file {s}: {}",
+                    .{ path, err },
                 ),
             });
-        }
+            self.@"config-file".list.items[i] = "";
+            continue;
+        };
+
+        log.debug("expanding config-file path relative={s} abs={s}", .{ path, abs });
+        self.@"config-file".list.items[i] = abs;
     }
 }