From c9371500c98c690070343d1a7e95b51cf990f92a Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Tue, 23 Jan 2024 18:57:33 -0800
Subject: [PATCH] empty cli or config args reset the value to the default

Fixes #1367

We previously special-cased optionals but we should do better and have
this reset ANY type to the defined default value on the struct.
---
 README.md        |  4 ++++
 src/cli/args.zig | 51 +++++++++++++++++++++++++++++++++++-------------
 2 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/README.md b/README.md
index be96b1a33..21e9f1e87 100644
--- a/README.md
+++ b/README.md
@@ -71,6 +71,10 @@ foreground= ffffff
 keybind = ctrl+z=close_surface
 keybind = ctrl+d=new_split:right
 
+# Empty values reset the configuration to the default value
+
+font-family =
+
 # Colors can be changed by setting the 16 colors of `palette`, which each color
 # being defined as regular and bold.
 #
diff --git a/src/cli/args.zig b/src/cli/args.zig
index abca087e3..61dbc509d 100644
--- a/src/cli/args.zig
+++ b/src/cli/args.zig
@@ -170,20 +170,14 @@ fn parseIntoField(
 
     inline for (info.Struct.fields) |field| {
         if (field.name[0] != '_' and mem.eql(u8, field.name, key)) {
-            // If the field is optional then consider scenarios we reset
-            // the value to being unset. We allow unsetting optionals
-            // whenever the value is "".
-            //
-            // At the time of writing this, empty string isn't a desirable
-            // value for any optional field under any realistic scenario.
-            //
-            // We don't allow unset values to set optional fields to
-            // null because unset value for booleans always means true.
-            if (@typeInfo(field.type) == .Optional) optional: {
-                if (std.mem.eql(u8, "", value orelse break :optional)) {
-                    @field(dst, field.name) = null;
-                    return;
-                }
+            // If the value is empty string (set but empty string),
+            // then we reset the value to the default.
+            if (value) |v| default: {
+                if (v.len != 0) break :default;
+                const raw = field.default_value orelse break :default;
+                const ptr: *const field.type = @alignCast(@ptrCast(raw));
+                @field(dst, field.name) = ptr.*;
+                return;
             }
 
             // For optional fields, we just treat it as the child type.
@@ -396,6 +390,26 @@ test "parse: quoted value" {
     try testing.expectEqualStrings("hello!", data.b);
 }
 
+test "parse: empty value resets to default" {
+    const testing = std.testing;
+
+    var data: struct {
+        a: u8 = 42,
+        b: bool = false,
+        _arena: ?ArenaAllocator = null,
+    } = .{};
+    defer if (data._arena) |arena| arena.deinit();
+
+    var iter = try std.process.ArgIteratorGeneral(.{}).init(
+        testing.allocator,
+        "--a= --b=",
+    );
+    defer iter.deinit();
+    try parse(@TypeOf(data), testing.allocator, &data, &iter);
+    try testing.expectEqual(@as(u8, 42), data.a);
+    try testing.expect(!data.b);
+}
+
 test "parse: error tracking" {
     const testing = std.testing;
 
@@ -865,6 +879,15 @@ test "LineIterator spaces around '='" {
     try testing.expectEqual(@as(?[]const u8, null), iter.next());
 }
 
+test "LineIterator no value" {
+    const testing = std.testing;
+    var fbs = std.io.fixedBufferStream("A = \n\n");
+
+    var iter = lineIterator(fbs.reader());
+    try testing.expectEqualStrings("--A=", iter.next().?);
+    try testing.expectEqual(@as(?[]const u8, null), iter.next());
+}
+
 test "LineIterator with CRLF line endings" {
     const testing = std.testing;
     var fbs = std.io.fixedBufferStream("A\r\nB = C\r\n");