config: more general purpose backwards compatibility handlers (#7717)

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.

In the future, I think this may continue to get _more_ general purpose
by moving the handlers from functions to structs so we can have more
metadata like descriptions and so on that we may use to generate docs or
other help strings.

src/cli.zig

@@ -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;

src/cli/args.zig

@@ -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.

src/config/Config.zig

@@ -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`