diff --git a/src/os/env.zig b/src/os/env.zig index 1916053b3..8f01a14d0 100644 --- a/src/os/env.zig +++ b/src/os/env.zig @@ -95,6 +95,21 @@ pub fn getenv(alloc: Allocator, key: []const u8) Error!?GetEnvResult { }; } +/// Gets the value of an environment variable. Returns null if not found or the +/// value is empty. This will allocate on Windows but not on other platforms. +/// The returned value should have deinit called to do the proper cleanup no +/// matter what platform you are on. +pub fn getenvNotEmpty(alloc: Allocator, key: []const u8) !?GetEnvResult { + const result_ = try getenv(alloc, key); + if (result_) |result| { + if (result.value.len == 0) { + result.deinit(alloc); + return null; + } + } + return result_; +} + pub fn setenv(key: [:0]const u8, value: [:0]const u8) c_int { return switch (builtin.os.tag) { .windows => c._putenv_s(key.ptr, value.ptr), diff --git a/src/os/xdg.zig b/src/os/xdg.zig index 1383679fe..e120ed857 100644 --- a/src/os/xdg.zig +++ b/src/os/xdg.zig @@ -7,6 +7,7 @@ const assert = std.debug.assert; const Allocator = std.mem.Allocator; const posix = std.posix; const homedir = @import("homedir.zig"); +const env_os = @import("env.zig"); pub const Options = struct { /// Subdirectories to join to the base. This avoids extra allocations @@ -70,36 +71,22 @@ fn dir( // First check the env var. On Windows we have to allocate so this tracks // both whether we have the env var and whether we own it. // on Windows we treat `LOCALAPPDATA` as a fallback for `XDG_CONFIG_HOME` - const env_, const owned = switch (builtin.os.tag) { - else => .{ posix.getenv(internal_opts.env), false }, - .windows => windows: { - if (std.process.getEnvVarOwned(alloc, internal_opts.env)) |env| { - break :windows .{ env, true }; - } else |err| switch (err) { - error.EnvironmentVariableNotFound => { - if (std.process.getEnvVarOwned(alloc, internal_opts.windows_env)) |env| { - break :windows .{ env, true }; - } else |err2| switch (err2) { - error.EnvironmentVariableNotFound => break :windows .{ null, false }, - else => return err, - } - }, - else => return err, - } - }, + const env_ = try env_os.getenvNotEmpty(alloc, internal_opts.env) orelse switch (builtin.os.tag) { + else => null, + .windows => try env_os.getenvNotEmpty(alloc, internal_opts.windows_env), }; - defer if (owned) if (env_) |v| alloc.free(v); + defer if (env_) |env| env.deinit(alloc); if (env_) |env| { // If we have a subdir, then we use the env as-is to avoid a copy. if (opts.subdir) |subdir| { return try std.fs.path.join(alloc, &[_][]const u8{ - env, + env.value, subdir, }); } - return try alloc.dupe(u8, env); + return try alloc.dupe(u8, env.value); } // Get our home dir @@ -169,6 +156,133 @@ test "cache directory paths" { } } +test "fallback when xdg env empty" { + if (builtin.os.tag == .windows) return error.SkipZigTest; + + const alloc = std.testing.allocator; + + const saved_home = home: { + const home = std.posix.getenv("HOME") orelse break :home null; + break :home try alloc.dupeZ(u8, home); + }; + defer env: { + const home = saved_home orelse { + _ = env_os.unsetenv("HOME"); + break :env; + }; + _ = env_os.setenv("HOME", home); + std.testing.allocator.free(home); + } + const temp_home = "/tmp/ghostty-test-home"; + _ = env_os.setenv("HOME", temp_home); + + const DirCase = struct { + name: [:0]const u8, + func: fn (Allocator, Options) anyerror![]u8, + default_subdir: []const u8, + }; + + const cases = [_]DirCase{ + .{ .name = "XDG_CONFIG_HOME", .func = config, .default_subdir = ".config" }, + .{ .name = "XDG_CACHE_HOME", .func = cache, .default_subdir = ".cache" }, + .{ .name = "XDG_STATE_HOME", .func = state, .default_subdir = ".local/state" }, + }; + + inline for (cases) |case| { + // Save and restore each environment variable + const saved_env = blk: { + const value = std.posix.getenv(case.name) orelse break :blk null; + break :blk try alloc.dupeZ(u8, value); + }; + defer env: { + const value = saved_env orelse { + _ = env_os.unsetenv(case.name); + break :env; + }; + _ = env_os.setenv(case.name, value); + alloc.free(value); + } + + const expected = try std.fs.path.join(alloc, &[_][]const u8{ + temp_home, + case.default_subdir, + }); + defer alloc.free(expected); + + // Test with empty string - should fallback to home + _ = env_os.setenv(case.name, ""); + const actual = try case.func(alloc, .{}); + defer alloc.free(actual); + + try std.testing.expectEqualStrings(expected, actual); + } +} + +test "fallback when xdg env empty and subdir" { + if (builtin.os.tag == .windows) return error.SkipZigTest; + + const env = @import("env.zig"); + const alloc = std.testing.allocator; + + const saved_home = home: { + const home = std.posix.getenv("HOME") orelse break :home null; + break :home try alloc.dupeZ(u8, home); + }; + defer env: { + const home = saved_home orelse { + _ = env.unsetenv("HOME"); + break :env; + }; + _ = env.setenv("HOME", home); + std.testing.allocator.free(home); + } + + const temp_home = "/tmp/ghostty-test-home"; + _ = env.setenv("HOME", temp_home); + + const DirCase = struct { + name: [:0]const u8, + func: fn (Allocator, Options) anyerror![]u8, + default_subdir: []const u8, + }; + + const cases = [_]DirCase{ + .{ .name = "XDG_CONFIG_HOME", .func = config, .default_subdir = ".config" }, + .{ .name = "XDG_CACHE_HOME", .func = cache, .default_subdir = ".cache" }, + .{ .name = "XDG_STATE_HOME", .func = state, .default_subdir = ".local/state" }, + }; + + inline for (cases) |case| { + // Save and restore each environment variable + const saved_env = blk: { + const value = std.posix.getenv(case.name) orelse break :blk null; + break :blk try alloc.dupeZ(u8, value); + }; + defer env: { + const value = saved_env orelse { + _ = env.unsetenv(case.name); + break :env; + }; + _ = env.setenv(case.name, value); + alloc.free(value); + } + + const expected = try std.fs.path.join(alloc, &[_][]const u8{ + temp_home, + case.default_subdir, + "ghostty", + }); + defer alloc.free(expected); + + // Test with empty string - should fallback to home + _ = env.setenv(case.name, ""); + const actual = try case.func(alloc, .{ .subdir = "ghostty" }); + defer alloc.free(actual); + + try std.testing.expectEqualStrings(expected, actual); + } +} + test parseTerminalExec { const testing = std.testing;