cli: allow renaming config fields to maintain backwards compatibility (#5329)

Fixes #4631

This introduces a mechanism by which parsed config fields can be renamed
to maintain backwards compatibility. This already has a use case --
implemented in this commit -- for `background-blur-radius` to be renamed
to `background-blur`.

The remapping is comptime-known which lets us do some comptime
validation. The remap check isn't done unless no fields match which
means for well-formed config files, there's no overhead.

For future improvements:

- We should update our config help generator to note renamed fields.
- We could offer automatic migration of config files be rewriting them.
- We can enrich the value type with more metadata to help with config
gen or other tooling.
This commit is contained in:
Mitchell Hashimoto
2025-01-23 14:25:26 -08:00
committed by GitHub
7 changed files with 73 additions and 12 deletions

View File

@ -339,7 +339,7 @@ extension Ghostty {
var backgroundBlurRadius: Int {
guard let config = self.config else { return 1 }
var v: Int = 0
let key = "background-blur-radius"
let key = "background-blur"
_ = ghostty_config_get(config, &v, key, UInt(key.count))
return v;
}

View File

@ -1958,7 +1958,7 @@ pub const CAPI = struct {
_ = CGSSetWindowBackgroundBlurRadius(
CGSDefaultConnectionForThread(),
nswindow.msgSend(usize, objc.sel("windowNumber"), .{}),
@intCast(config.@"background-blur-radius".cval()),
@intCast(config.@"background-blur".cval()),
);
}

View File

@ -176,7 +176,7 @@ pub const Window = struct {
pub fn init(config: *const Config) DerivedConfig {
return .{
.blur = config.@"background-blur-radius".enabled(),
.blur = config.@"background-blur".enabled(),
.window_decoration = config.@"window-decoration",
};
}

View File

@ -165,7 +165,7 @@ pub const Window = struct {
pub fn init(config: *const Config) DerivedConfig {
return .{
.blur = config.@"background-blur-radius".enabled(),
.blur = config.@"background-blur".enabled(),
.has_decoration = switch (config.@"window-decoration") {
.none => false,
.auto, .client, .server => true,

View File

@ -38,6 +38,12 @@ pub const Error = error{
/// "DiagnosticList" and any diagnostic messages will be added to that list.
/// When diagnostics are present, only allocation errors will be returned.
///
/// If the destination type has a decl "renamed", it must be of type
/// std.StaticStringMap([]const u8) and contains a mapping from the old
/// field name to the new field name. This is used to allow renaming fields
/// while still supporting the old name. If a renamed field is set, parsing
/// will automatically set the new field name.
///
/// Note: If the arena is already non-null, then it will be used. In this
/// case, in the case of an error some memory might be leaked into the arena.
pub fn parse(
@ -49,6 +55,24 @@ pub fn parse(
const info = @typeInfo(T);
assert(info == .Struct);
comptime {
// Verify all renamed fields are valid (source does not exist,
// destination does exist).
if (@hasDecl(T, "renamed")) {
for (T.renamed.keys(), T.renamed.values()) |key, value| {
if (@hasField(T, key)) {
@compileLog(key);
@compileError("renamed field source exists");
}
if (!@hasField(T, value)) {
@compileLog(value);
@compileError("renamed field destination does not exist");
}
}
}
}
// Make an arena for all our allocations if we support it. Otherwise,
// use an allocator that always fails. If the arena is already set on
// the config, then we reuse that. See memory note in parse docs.
@ -367,6 +391,16 @@ pub fn parseIntoField(
}
}
// Unknown field, is the field renamed?
if (@hasDecl(T, "renamed")) {
for (T.renamed.keys(), T.renamed.values()) |old, new| {
if (mem.eql(u8, old, key)) {
try parseIntoField(T, alloc, dst, new, value);
return;
}
}
}
return error.InvalidField;
}
@ -1104,6 +1138,24 @@ test "parseIntoField: tagged union missing tag" {
);
}
test "parseIntoField: renamed field" {
const testing = std.testing;
var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
var data: struct {
a: []const u8,
const renamed = std.StaticStringMap([]const u8).initComptime(&.{
.{ "old", "a" },
});
} = undefined;
try parseIntoField(@TypeOf(data), alloc, &data, "old", "42");
try testing.expectEqualStrings("42", data.a);
}
/// An iterator that considers its location to be CLI args. It
/// iterates through an underlying iterator and increments a counter
/// to track the current CLI arg index.

View File

@ -42,6 +42,15 @@ const c = @cImport({
@cInclude("unistd.h");
});
/// Renamed fields, used by cli.parse
pub const renamed = std.StaticStringMap([]const u8).initComptime(&.{
// Ghostty 1.1 introduced background-blur support for Linux which
// doesn't support a specific radius value. The renaming is to let
// one field be used for both platforms (macOS retained the ability
// to set a radius).
.{ "background-blur-radius", "background-blur" },
});
/// The font families to use.
///
/// You can generate the list of valid values using the CLI:
@ -649,7 +658,7 @@ palette: Palette = .{},
/// need to set environment-specific settings and/or install third-party plugins
/// in order to support background blur, as there isn't a unified interface for
/// doing so.
@"background-blur-radius": BackgroundBlur = .false,
@"background-blur": BackgroundBlur = .false,
/// The opacity level (opposite of transparency) of an unfocused split.
/// Unfocused splits by default are slightly faded out to make it easier to see
@ -5854,7 +5863,7 @@ pub const AutoUpdate = enum {
download,
};
/// See background-blur-radius
/// See background-blur
pub const BackgroundBlur = union(enum) {
false,
true,

View File

@ -192,21 +192,21 @@ test "c_get: background-blur" {
defer c.deinit();
{
c.@"background-blur-radius" = .false;
c.@"background-blur" = .false;
var cval: u8 = undefined;
try testing.expect(get(&c, .@"background-blur-radius", @ptrCast(&cval)));
try testing.expect(get(&c, .@"background-blur", @ptrCast(&cval)));
try testing.expectEqual(0, cval);
}
{
c.@"background-blur-radius" = .true;
c.@"background-blur" = .true;
var cval: u8 = undefined;
try testing.expect(get(&c, .@"background-blur-radius", @ptrCast(&cval)));
try testing.expect(get(&c, .@"background-blur", @ptrCast(&cval)));
try testing.expectEqual(20, cval);
}
{
c.@"background-blur-radius" = .{ .radius = 42 };
c.@"background-blur" = .{ .radius = 42 };
var cval: u8 = undefined;
try testing.expect(get(&c, .@"background-blur-radius", @ptrCast(&cval)));
try testing.expect(get(&c, .@"background-blur", @ptrCast(&cval)));
try testing.expectEqual(42, cval);
}
}