config: edit opens AppSupport over XDG on macOS, prefers non-empty paths

Fixes #3953
Fixes #3284

This fixes two issues. In fixing one issue, the other became apparent so
I fixed both in this one commit.

The first issue is that on macOS, the `open` command should take the
`-t` flag to open text files in a text editor. To do this, the `os.open`
function now takes a type hint that is used to better do the right
thing.

Second, the order of the paths that we attempt to open when editing a
config on macOS is wrong. Our priority when loading configs is well documented:
https://ghostty.org/docs/config#macos-specific-path-(macos-only). But
open_config does the opposite. This makes it too easy for people to have
configs that are being overridden without them realizing it.

This commit changes the order of the paths to match the documented
order. If neither path exists, we prefer AppSupport.

src/Surface.zig

@@ -3195,7 +3195,7 @@ fn processLinks(self: *Surface, pos: apprt.CursorPos) !bool {
                 .trim = false,
             });
             defer self.alloc.free(str);
-            try internal_os.open(self.alloc, str);
+            try internal_os.open(self.alloc, .unknown, str);
         },
 
         ._open_osc8 => {
@@ -3203,7 +3203,7 @@ fn processLinks(self: *Surface, pos: apprt.CursorPos) !bool {
                 log.warn("failed to get URI for OSC8 hyperlink", .{});
                 return false;
             };
-            try internal_os.open(self.alloc, uri);
+            try internal_os.open(self.alloc, .unknown, uri);
         },
     }
 
@@ -4303,7 +4303,7 @@ fn writeScreenFile(
     const path = try tmp_dir.dir.realpath(filename, &path_buf);
 
     switch (write_action) {
-        .open => try internal_os.open(self.alloc, path),
+        .open => try internal_os.open(self.alloc, .text, path),
         .paste => self.io.queueMessage(try termio.Message.writeReq(
             self.alloc,
             path,

src/config/edit.zig

@@ -1,31 +1,29 @@
 const std = @import("std");
 const builtin = @import("builtin");
+const assert = std.debug.assert;
 const Allocator = std.mem.Allocator;
+const ArenaAllocator = std.heap.ArenaAllocator;
 const internal_os = @import("../os/main.zig");
 
 /// Open the configuration in the OS default editor according to the default
 /// paths the main config file could be in.
+///
+/// On Linux, this will open the file at the XDG config path. This is the
+/// only valid path for Linux so we don't need to check for other paths.
+///
+/// On macOS, both XDG and AppSupport paths are valid. Because Ghostty
+/// prioritizes AppSupport over XDG, we will open AppSupport if it exists,
+/// followed by XDG if it exists, and finally AppSupport if neither exist.
+/// For the existence check, we also prefer non-empty files over empty
+/// files.
 pub fn open(alloc_gpa: Allocator) !void {
-    // default location
-    const config_path = config_path: {
-        const xdg_config_path = try internal_os.xdg.config(alloc_gpa, .{ .subdir = "ghostty/config" });
+    // Use an arena to make memory management easier in here.
+    var arena = ArenaAllocator.init(alloc_gpa);
+    defer arena.deinit();
+    const alloc = arena.allocator();
 
-        if (comptime builtin.os.tag == .macos) macos: {
-            // On macOS, use the application support path if the XDG path doesn't exists.
-            if (std.fs.accessAbsolute(xdg_config_path, .{})) {
-                break :macos;
-            } else |err| switch (err) {
-                error.BadPathName, error.FileNotFound => {},
-                else => break :macos,
-            }
-
-            alloc_gpa.free(xdg_config_path);
-            break :config_path try internal_os.macos.appSupportDir(alloc_gpa, "config");
-        }
-
-        break :config_path xdg_config_path;
-    };
-    defer alloc_gpa.free(config_path);
+    // Get the path we should open
+    const config_path = try configPath(alloc);
 
     // Create config directory recursively.
     if (std.fs.path.dirname(config_path)) |config_dir| {
@@ -43,5 +41,73 @@ pub fn open(alloc_gpa: Allocator) !void {
         }
     };
 
-    try internal_os.open(alloc_gpa, config_path);
+    try internal_os.open(alloc, .text, config_path);
+}
+
+/// Returns the config path to use for open for the current OS.
+///
+/// The allocator must be an arena allocator. No memory is freed by this
+/// function and the resulting path is not all the memory that is allocated.
+///
+/// NOTE: WHY IS THIS INLINE? This is inline because when this is not
+/// inline then Zig 0.13 crashes [most of the time] when trying to compile
+/// this file. This is a workaround for that issue. This function is only
+/// called from one place that is not performance critical so it is fine
+/// to be inline.
+inline fn configPath(alloc_arena: Allocator) ![]const u8 {
+    const paths: []const []const u8 = try configPathCandidates(alloc_arena);
+    assert(paths.len > 0);
+
+    // Find the first path that exists and is non-empty. If no paths are
+    // non-empty but at least one exists, we will return the first path that
+    // exists.
+    var exists: ?[]const u8 = null;
+    for (paths) |path| {
+        const f = std.fs.openFileAbsolute(path, .{}) catch |err| {
+            switch (err) {
+                // File doesn't exist, continue.
+                error.BadPathName, error.FileNotFound => continue,
+
+                // Some other error, assume it exists and return it.
+                else => return err,
+            }
+        };
+        defer f.close();
+
+        // We expect stat to succeed because we just opened the file.
+        const stat = try f.stat();
+
+        // If the file is non-empty, return it.
+        if (stat.size > 0) return path;
+
+        // If the file is empty, remember it exists.
+        if (exists == null) exists = path;
+    }
+
+    // No paths are non-empty, return the first path that exists.
+    if (exists) |v| return v;
+
+    // No paths are non-empty or exist, return the first path.
+    return paths[0];
+}
+
+/// Returns a const list of possible paths the main config file could be
+/// in for the current OS.
+fn configPathCandidates(alloc_arena: Allocator) ![]const []const u8 {
+    var paths = try std.ArrayList([]const u8).initCapacity(alloc_arena, 2);
+    errdefer paths.deinit();
+
+    if (comptime builtin.os.tag == .macos) {
+        paths.appendAssumeCapacity(try internal_os.macos.appSupportDir(
+            alloc_arena,
+            "config",
+        ));
+    }
+
+    paths.appendAssumeCapacity(try internal_os.xdg.config(
+        alloc_arena,
+        .{ .subdir = "ghostty/config" },
+    ));
+
+    return paths.items;
 }

src/os/macos.zig

@@ -24,7 +24,7 @@ pub const AppSupportDirError = Allocator.Error || error{AppleAPIFailed};
 pub fn appSupportDir(
     alloc: Allocator,
     sub_path: []const u8,
-) AppSupportDirError![]u8 {
+) AppSupportDirError![]const u8 {
     comptime assert(builtin.target.isDarwin());
 
     const NSFileManager = objc.getClass("NSFileManager").?;

src/os/main.zig

@@ -41,5 +41,6 @@ pub const home = homedir.home;
 pub const ensureLocale = locale.ensureLocale;
 pub const clickInterval = mouse.clickInterval;
 pub const open = openpkg.open;
+pub const OpenType = openpkg.Type;
 pub const pipe = pipepkg.pipe;
 pub const resourcesDir = resourcesdir.resourcesDir;

src/os/open.zig

@@ -2,25 +2,26 @@ const std = @import("std");
 const builtin = @import("builtin");
 const Allocator = std.mem.Allocator;
 
+/// The type of the data at the URL to open. This is used as a hint
+/// to potentially open the URL in a different way.
+pub const Type = enum {
+    text,
+    unknown,
+};
+
 /// Open a URL in the default handling application.
 ///
 /// Any output on stderr is logged as a warning in the application logs.
 /// Output on stdout is ignored.
-pub fn open(alloc: Allocator, url: []const u8) !void {
-    // Some opener commands terminate after opening (macOS open) and some do not
-    // (xdg-open). For those which do not terminate, we do not want to wait for
-    // the process to exit to collect stderr.
-    const argv, const wait = switch (builtin.os.tag) {
-        .linux => .{ &.{ "xdg-open", url }, false },
-        .macos => .{ &.{ "open", url }, true },
-        .windows => .{ &.{ "rundll32", "url.dll,FileProtocolHandler", url }, false },
-        .ios => return error.Unimplemented,
-        else => @compileError("unsupported OS"),
-    };
+pub fn open(
+    alloc: Allocator,
+    typ: Type,
+    url: []const u8,
+) !void {
+    const cmd = try openCommand(alloc, typ, url);
 
-    var exe = std.process.Child.init(argv, alloc);
-
-    if (comptime wait) {
+    var exe = cmd.child;
+    if (cmd.wait) {
         // Pipe stdout/stderr so we can collect output from the command
         exe.stdout_behavior = .Pipe;
         exe.stderr_behavior = .Pipe;
@@ -28,7 +29,7 @@ pub fn open(alloc: Allocator, url: []const u8) !void {
 
     try exe.spawn();
 
-    if (comptime wait) {
+    if (cmd.wait) {
         // 50 KiB is the default value used by std.process.Child.run
         const output_max_size = 50 * 1024;
 
@@ -47,3 +48,36 @@ pub fn open(alloc: Allocator, url: []const u8) !void {
         if (stderr.items.len > 0) std.log.err("open stderr={s}", .{stderr.items});
     }
 }
+
+const OpenCommand = struct {
+    child: std.process.Child,
+    wait: bool = false,
+};
+
+fn openCommand(alloc: Allocator, typ: Type, url: []const u8) !OpenCommand {
+    return switch (builtin.os.tag) {
+        .linux => .{ .child = std.process.Child.init(
+            &.{ "xdg-open", url },
+            alloc,
+        ) },
+
+        .windows => .{ .child = std.process.Child.init(
+            &.{ "rundll32", "url.dll,FileProtocolHandler", url },
+            alloc,
+        ) },
+
+        .macos => .{
+            .child = std.process.Child.init(
+                switch (typ) {
+                    .text => &.{ "open", "-t", url },
+                    .unknown => &.{ "open", url },
+                },
+                alloc,
+            ),
+            .wait = true,
+        },
+
+        .ios => return error.Unimplemented,
+        else => @compileError("unsupported OS"),
+    };
+}