From 6ac13034fe9a9817dbebeb6f2907a0bc9ea98803 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 17 Jul 2024 11:59:15 -0700 Subject: [PATCH] config: font-family CLI flags will clear font families set in files This is a quality of life UX change. font-family is a repeatable configuration where each subsequent value will be added as a fallback font. This introduces a UX gotcha where if a font was set in a config file, the CLI args (which are loaded later) would _append_ to the font families. This has never once been the behavior I've wanted. Previously, you'd have to do `--font-family=""` which is clunky. This change makes it so that CLI font-family flags will automatically clear the families set in the configuration file. --- src/config/Config.zig | 47 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/src/config/Config.zig b/src/config/Config.zig index d712a56eb..e95a2ba8e 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -56,6 +56,13 @@ const c = @cImport({ /// font-family = "" /// font-family = "My Favorite Font" /// +/// Setting any of these as CLI arguments will automatically clear the +/// values set in configuration files so you don't need to specify +/// `--font-family=""` before setting a new value. You only need to specify +/// this within config files if you want to clear previously set values in +/// configuration files or on the CLI if you want to clear values set on the +/// CLI. +/// /// Changing this configuration at runtime will only affect new terminals, i.e. /// new windows, tabs, etc. @"font-family": RepeatableString = .{}, @@ -1198,8 +1205,44 @@ pub fn load(alloc_gpa: Allocator) !Config { // If we have a configuration file in our home directory, parse that first. try result.loadDefaultFiles(alloc_gpa); - // Parse the config from the CLI args - try result.loadCliArgs(alloc_gpa); + // Parse the config from the CLI args. If the font families change from + // the CLI, we clear the font families set in the configuration file. + // This avoids a UX oddity where on the CLI you have to specify + // `font-family=""` to clear the font families before setting a new one. + { + const fields = &[_][]const u8{ + "font-family", + "font-family-bold", + "font-family-italic", + "font-family-bold-italic", + }; + + // Build our initial counts + var counter: [fields.len]usize = undefined; + inline for (fields, 0..) |field, i| { + counter[i] = @field(result, field).list.items.len; + } + + try result.loadCliArgs(alloc_gpa); + + // If any of our font family settings were changed, then we + // replace the entire list with the new list. + inline for (fields, 0..) |field, i| { + const v = &@field(result, field); + const len = v.list.items.len - counter[i]; + if (len > 0) { + // Note: we don't have to worry about freeing the memory + // that we overwrite or cut off here because its all in + // an arena. + v.list.replaceRangeAssumeCapacity( + 0, + len, + v.list.items[counter[i]..], + ); + v.list.items.len = len; + } + } + } // Parse the config files that were added from our file and CLI args. try result.loadRecursiveFiles(alloc_gpa);