Applied-Software/ghostty
5
0
Fork 0
mirror of https://github.com/ghostty-org/ghostty.git synced 2025-07-14 07:46:12 +03:00
Code Issues Packages Projects Releases Wiki Activity

config: "-e" arguments must stay at the end of replay steps

Browse Source 
Fixes #2908

When loading `config-file`, we need to ensure that all loaded
configuration is loaded _prior_ to any `-e` values from the CLI.

To do this, I inserted a new `-e` special tag type in our replay steps.
This can be used to find when `-e` starts and ensure it remains at the
end of replay steps when the replay steps are being modified.

This commit also found a similar (but not exercised) issue where this
could happen with light/dark themeing.
This commit is contained in:
Mitchell Hashimoto
2024-12-09 14:16:05 -08:00
parent efe13676e2
commit cac776a994
1 changed files with 90 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

129
src/config/Config.zig
View File

@ -2468,7 +2468,7 @@ pub fn loadCliArgs(self: *Config, alloc_gpa: Allocator) !void {
// First, we add an artificial "-e" so that if we
// replay the inputs to rebuild the config (i.e. if
// a theme is set) then we will get the same behavior.
try self._replay_steps.append(arena_alloc, .{ .arg = "-e" });
try self._replay_steps.append(arena_alloc, .@"-e");
// Next, take all remaining args and use that to build up
// a command to execute.
@ -2576,6 +2576,24 @@ pub fn loadRecursiveFiles(self: *Config, alloc_gpa: Allocator) !void {
const cwd = std.fs.cwd();
// We need to insert all of our loaded config-file values
// PRIOR to the "-e" in our replay steps, since everything
// after "-e" becomes an "initial-command". To do this, we
// dupe the values if we find it.
var replay_suffix = std.ArrayList(Replay.Step).init(alloc_gpa);
defer replay_suffix.deinit();
for (self._replay_steps.items, 0..) |step, i| if (step == .@"-e") {
// We don't need to clone the steps because they should
// all be allocated in our arena and we're keeping our
// arena.
try replay_suffix.appendSlice(self._replay_steps.items[i..]);
// Remove our old values. Again, don't need to free any
// memory here because its all part of our arena.
self._replay_steps.shrinkRetainingCapacity(i);
break;
};
// We must use a while below and not a for(items) because we
// may add items to the list while iterating for recursive
// config-file entries.
@ -2627,6 +2645,14 @@ pub fn loadRecursiveFiles(self: *Config, alloc_gpa: Allocator) !void {
try self.loadIter(alloc_gpa, &iter);
try self.expandPaths(std.fs.path.dirname(path).?);
}
// If we have a suffix, add that back.
if (replay_suffix.items.len > 0) {
try self._replay_steps.appendSlice(
arena_alloc,
replay_suffix.items,
);
}
}
/// Change the state of conditionals and reload the configuration
@ -2754,39 +2780,46 @@ fn loadTheme(self: *Config, theme: Theme) !void {
try new_config.loadIter(alloc_gpa, &iter);
// Setup our replay to be conditional.
for (new_config._replay_steps.items) |*item| switch (item.*) {
.expand => {},
conditional: for (new_config._replay_steps.items) |*item| {
switch (item.*) {
.expand => {},
// Change our arg to be conditional on our theme.
.arg => |v| {
const alloc_arena = new_config._arena.?.allocator();
const conds = try alloc_arena.alloc(Conditional, 1);
conds[0] = .{
.key = .theme,
.op = .eq,
.value = @tagName(self._conditional_state.theme),
};
item.* = .{ .conditional_arg = .{
.conditions = conds,
.arg = v,
} };
},
// If we see "-e" then we do NOT make the following arguments
// conditional since they are supposed to be part of the
// initial command.
.@"-e" => break :conditional,
.conditional_arg => |v| {
const alloc_arena = new_config._arena.?.allocator();
const conds = try alloc_arena.alloc(Conditional, v.conditions.len + 1);
conds[0] = .{
.key = .theme,
.op = .eq,
.value = @tagName(self._conditional_state.theme),
};
@memcpy(conds[1..], v.conditions);
item.* = .{ .conditional_arg = .{
.conditions = conds,
.arg = v.arg,
} };
},
};
// Change our arg to be conditional on our theme.
.arg => |v| {
const alloc_arena = new_config._arena.?.allocator();
const conds = try alloc_arena.alloc(Conditional, 1);
conds[0] = .{
.key = .theme,
.op = .eq,
.value = @tagName(self._conditional_state.theme),
};
item.* = .{ .conditional_arg = .{
.conditions = conds,
.arg = v,
} };
},
.conditional_arg => |v| {
const alloc_arena = new_config._arena.?.allocator();
const conds = try alloc_arena.alloc(Conditional, v.conditions.len + 1);
conds[0] = .{
.key = .theme,
.op = .eq,
.value = @tagName(self._conditional_state.theme),
};
@memcpy(conds[1..], v.conditions);
item.* = .{ .conditional_arg = .{
.conditions = conds,
.arg = v.arg,
} };
},
}
}
// Replay our previous inputs so that we can override values
// from the theme.
@ -2978,10 +3011,12 @@ pub fn parseManuallyHook(
arg: []const u8,
iter: anytype,
) !bool {
// Keep track of our input args no matter what..
try self._replay_steps.append(alloc, .{ .arg = try alloc.dupe(u8, arg) });
if (std.mem.eql(u8, arg, "-e")) {
// Add the special -e marker. This prevents:
// (1) config-file from adding args to the end (see #2908)
// (2) dark/light theme from making this conditional
try self._replay_steps.append(alloc, .@"-e");
// Build up the command. We don't clean this up because we take
// ownership in our allocator.
var command = std.ArrayList(u8).init(alloc);
@ -3017,6 +3052,12 @@ pub fn parseManuallyHook(
return false;
}
// Keep track of our input args for replay
try self._replay_steps.append(
alloc,
.{ .arg = try alloc.dupe(u8, arg) },
);
// If we didn't find a special case, continue parsing normally
return true;
}
@ -3284,11 +3325,22 @@ const Replay = struct {
arg: []const u8,
},
/// The start of a "-e" argument. This marks the end of
/// traditional configuration and the beginning of the
/// "-e" initial command magic. This is separate from "arg"
/// because there are some behaviors unique to this (i.e.
/// we want to keep this at the end for config-file).
///
/// Note: when "-e" is used, ONLY this is present and
/// not an additional "arg" with "-e" value.
@"-e",
fn clone(
self: Step,
alloc: Allocator,
) Allocator.Error!Step {
return switch (self) {
.@"-e" => self,
.arg => |v| .{ .arg = try alloc.dupe(u8, v) },
.expand => |v| .{ .expand = try alloc.dupe(u8, v) },
.conditional_arg => |v| conditional: {
@ -3324,10 +3376,6 @@ const Replay = struct {
log.warn("error expanding paths err={}", .{err});
},
.arg => |arg| {
return arg;
},
.conditional_arg => |v| conditional: {
// All conditions must match.
for (v.conditions) |cond| {
@ -3338,6 +3386,9 @@ const Replay = struct {
return v.arg;
},
.arg => |arg| return arg,
.@"-e" => return "-e",
}
}
}