config: more general purpose backwards compatibility handlers

Fixes #7706

We previously had a very specific backwards compatibility handler for
handling renamed fields. We always knew that wouldn't scale but I wanted
to wait for a real case. Well, #7706 is a real case, so here we are.

This commit makes our backwards compatibility handler more general
purpose, and makes a special-case handler for renamed fields built on
top of this same general purpose system. The new system lets us do a lot
more with regards to backwards compatibility.

To start, this addresses #7706 by allowing us to handle a removed single
enum value of a still-existing field.
This commit is contained in:
Mitchell Hashimoto
2025-06-28 13:06:36 -07:00
parent 206d41844e
commit 84432a7beb
3 changed files with 174 additions and 55 deletions

View File

@@ -2,6 +2,8 @@ const diags = @import("cli/diagnostics.zig");
pub const args = @import("cli/args.zig");
pub const Action = @import("cli/action.zig").Action;
pub const CompatibilityHandler = args.CompatibilityHandler;
pub const compatibilityRenamed = args.compatibilityRenamed;
pub const DiagnosticList = diags.DiagnosticList;
pub const Diagnostic = diags.Diagnostic;
pub const Location = diags.Location;

View File

@@ -40,11 +40,14 @@ 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.
/// If the destination type has a decl "compatibility", it must be of type
/// std.StaticStringMap(CompatibilityHandler(T)), and it will be used to
/// handle backwards compatibility for fields with the given name. The
/// field name doesn't need to exist (so you can setup compatibility for
/// removed fields). The value is a function that will be called when
/// all other parsing fails for that field. If a field changes such that
/// the old values would NOT error, then the caller should handle that
/// downstream after parsing is done, not through this method.
///
/// 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.
@@ -57,24 +60,6 @@ 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.
@@ -148,6 +133,16 @@ pub fn parse(
};
parseIntoField(T, arena_alloc, dst, key, value) catch |err| {
// If we get an error parsing a field, then we try to fall
// back to compatibility handlers if able.
if (@hasDecl(T, "compatibility")) {
// If we have a compatibility handler for this key, then
// we call it and see if it handles the error.
if (T.compatibility.get(key)) |handler| {
if (handler(dst, arena_alloc, key, value)) return;
}
}
if (comptime !canTrackDiags(T)) return err;
// The error set is dependent on comptime T, so we always add
@@ -177,6 +172,58 @@ pub fn parse(
}
}
/// The function type for a compatibility handler. The compatibility
/// handler is documented in the `parse` function documentation.
///
/// The function type should return bool if the compatibility was
/// handled, and false otherwise. If false is returned then the
/// naturally occurring error will continue to be processed as if
/// this compatibility handler was not present.
///
/// Compatibility handlers aren't allowed to return errors because
/// they're generally only called in error cases, so we already have
/// an error message to show users. If there is an error in handling
/// the compatibility, then the handler should return false.
pub fn CompatibilityHandler(comptime T: type) type {
return *const fn (
dst: *T,
alloc: Allocator,
key: []const u8,
value: ?[]const u8,
) bool;
}
/// Convenience function to create a compatibility handler that
/// renames a field from `from` to `to`.
pub fn compatibilityRenamed(
comptime T: type,
comptime to: []const u8,
) CompatibilityHandler(T) {
comptime assert(@hasField(T, to));
return (struct {
fn compat(
dst: *T,
alloc: Allocator,
key: []const u8,
value: ?[]const u8,
) bool {
_ = key;
parseIntoField(T, alloc, dst, to, value) catch |err| {
log.warn("error parsing renamed field {s}: {}", .{
to,
err,
});
return false;
};
return true;
}
}).compat;
}
fn formatValueRequired(
comptime T: type,
arena_alloc: std.mem.Allocator,
@@ -401,16 +448,6 @@ 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;
}
@@ -752,6 +789,75 @@ test "parse: diagnostic location" {
}
}
test "parse: compatibility handler" {
const testing = std.testing;
var data: struct {
a: bool = false,
_arena: ?ArenaAllocator = null,
pub const compatibility: std.StaticStringMap(
CompatibilityHandler(@This()),
) = .initComptime(&.{
.{ "a", compat },
});
fn compat(
self: *@This(),
alloc: Allocator,
key: []const u8,
value: ?[]const u8,
) bool {
_ = alloc;
if (std.mem.eql(u8, key, "a")) {
if (value) |v| {
if (mem.eql(u8, v, "yuh")) {
self.a = true;
return true;
}
}
}
return false;
}
} = .{};
defer if (data._arena) |arena| arena.deinit();
var iter = try std.process.ArgIteratorGeneral(.{}).init(
testing.allocator,
"--a=yuh",
);
defer iter.deinit();
try parse(@TypeOf(data), testing.allocator, &data, &iter);
try testing.expect(data._arena != null);
try testing.expect(data.a);
}
test "parse: compatibility renamed" {
const testing = std.testing;
var data: struct {
a: bool = false,
_arena: ?ArenaAllocator = null,
pub const compatibility: std.StaticStringMap(
CompatibilityHandler(@This()),
) = .initComptime(&.{
.{ "old", compatibilityRenamed(@This(), "a") },
});
} = .{};
defer if (data._arena) |arena| arena.deinit();
var iter = try std.process.ArgIteratorGeneral(.{}).init(
testing.allocator,
"--old=true",
);
defer iter.deinit();
try parse(@TypeOf(data), testing.allocator, &data, &iter);
try testing.expect(data._arena != null);
try testing.expect(data.a);
}
test "parseIntoField: ignore underscore-prefixed fields" {
const testing = std.testing;
var arena = ArenaAllocator.init(testing.allocator);
@@ -1176,24 +1282,6 @@ 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

@@ -46,14 +46,22 @@ const c = @cImport({
@cInclude("unistd.h");
});
/// Renamed fields, used by cli.parse
pub const renamed = std.StaticStringMap([]const u8).initComptime(&.{
pub const compatibility = std.StaticStringMap(
cli.CompatibilityHandler(Config),
).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" },
.{ "adw-toolbar-style", "gtk-toolbar-style" },
.{ "background-blur-radius", cli.compatibilityRenamed(Config, "background-blur") },
// Ghostty 1.2 renamed all our adw options to gtk because we now have
// a hard dependency on libadwaita.
.{ "adw-toolbar-style", cli.compatibilityRenamed(Config, "gtk-toolbar-style") },
// Ghostty 1.2 removed the `hidden` value from `gtk-tabs-location` and
// moved it to `window-show-tab-bar`.
.{ "gtk-tabs-location", compatGtkTabsLocation },
});
/// The font families to use.
@@ -3792,6 +3800,27 @@ pub fn parseManuallyHook(
return true;
}
/// parseFieldManuallyFallback is a fallback called only when
/// parsing the field directly failed. It can be used to implement
/// backward compatibility. Since this is only called when parsing
/// fails, it doesn't impact happy-path performance.
fn compatGtkTabsLocation(
self: *Config,
alloc: Allocator,
key: []const u8,
value: ?[]const u8,
) bool {
_ = alloc;
assert(std.mem.eql(u8, key, "gtk-tabs-location"));
if (std.mem.eql(u8, value orelse "", "hidden")) {
self.@"window-show-tab-bar" = .never;
return true;
}
return false;
}
/// Create a shallow copy of this config. This will share all the memory
/// allocated with the previous config but will have a new arena for
/// any changes or new allocations. The config should have `deinit`