From 510f4b46997ea5f0b7e8e4f4382948fbdcf95afd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 12 Mar 2023 14:19:18 -0700 Subject: [PATCH 01/19] config supports clone() operation for a deep copy --- src/config.zig | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/src/config.zig b/src/config.zig index 711a69957..d10f8b58e 100644 --- a/src/config.zig +++ b/src/config.zig @@ -597,6 +597,76 @@ pub const Config = struct { self.@"click-repeat-interval" = internal_os.clickInterval() orelse 500; } } + + /// Create a copy of this configuration. This is useful as a starting + /// point for modifying a configuration since a config can NOT be + /// modified once it is in use by an app or surface. + pub fn clone(self: *const Config, alloc_gpa: Allocator) !Config { + // Start with an empty config with a new arena we're going + // to use for all our copies. + var result: Config = .{ + ._arena = ArenaAllocator.init(alloc_gpa), + }; + errdefer result.deinit(); + const alloc = result._arena.?.allocator(); + + inline for (@typeInfo(Config).Struct.fields) |field| { + // Ignore fields starting with "_" since they're internal and + // not copied ever. + if (field.name[0] == '_') continue; + + @field(result, field.name) = try cloneValue( + alloc, + field.type, + @field(self, field.name), + ); + } + + return result; + } + + fn cloneValue(alloc: Allocator, comptime T: type, src: T) !T { + // Do known named types first + switch (T) { + []const u8 => return try alloc.dupe(u8, src), + [:0]const u8 => return try alloc.dupeZ(u8, src), + + else => {}, + } + + // Back into types of types + switch (@typeInfo(T)) { + inline .Bool, + .Int, + => return src, + + .Optional => |info| return try cloneValue( + alloc, + info.child, + src orelse return null, + ), + + .Struct => return try src.clone(alloc), + + else => { + @compileLog(T); + @compileError("unsupported field type"); + }, + } + } + + test "clone default" { + const testing = std.testing; + const alloc = testing.allocator; + + var source = try Config.default(alloc); + defer source.deinit(); + var dest = try source.clone(alloc); + defer dest.deinit(); + + // I want to do this but this doesn't work (the API doesn't work) + // try testing.expectEqualDeep(dest, source); + } }; /// Color represents a color using RGB. @@ -618,6 +688,11 @@ pub const Color = struct { return fromHex(input orelse return error.ValueRequired); } + /// Deep copy of the struct. Required by Config. + pub fn clone(self: Color, _: Allocator) !Color { + return self; + } + /// fromHex parses a color from a hex value such as #RRGGBB. The "#" /// is optional. pub fn fromHex(input: []const u8) !Color { @@ -681,6 +756,11 @@ pub const Palette = struct { self.value[key] = .{ .r = rgb.r, .g = rgb.g, .b = rgb.b }; } + /// Deep copy of the struct. Required by Config. + pub fn clone(self: Self, _: Allocator) !Self { + return self; + } + test "parseCLI" { const testing = std.testing; @@ -714,6 +794,13 @@ pub const RepeatableString = struct { try self.list.append(alloc, value); } + /// Deep copy of the struct. Required by Config. + pub fn clone(self: *const Self, alloc: Allocator) !Self { + return .{ + .list = try self.list.clone(alloc), + }; + } + test "parseCLI" { const testing = std.testing; var arena = ArenaAllocator.init(testing.allocator); @@ -758,6 +845,15 @@ pub const Keybinds = struct { } } + /// Deep copy of the struct. Required by Config. + pub fn clone(self: *const Keybinds, alloc: Allocator) !Keybinds { + return .{ + .set = .{ + .bindings = try self.set.bindings.clone(alloc), + }, + }; + } + test "parseCLI" { const testing = std.testing; var arena = ArenaAllocator.init(testing.allocator); From 16166b629708d28f555191d39042c4e60930bdec Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 12 Mar 2023 15:07:23 -0700 Subject: [PATCH 02/19] config: implement change iterator (one todo) --- src/config.zig | 127 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/src/config.zig b/src/config.zig index d10f8b58e..310212176 100644 --- a/src/config.zig +++ b/src/config.zig @@ -166,6 +166,30 @@ pub const Config = struct { /// This is set by the CLI parser for deinit. _arena: ?ArenaAllocator = null, + /// Key is an enum of all the available configuration keys. This is used + /// when paired with diff to determine what fields have changed in a config, + /// amongst other things. + pub const Key = key: { + const field_infos = std.meta.fields(Config); + var enumFields: [field_infos.len]std.builtin.Type.EnumField = undefined; + var decls = [_]std.builtin.Type.Declaration{}; + inline for (field_infos, 0..) |field, i| { + enumFields[i] = .{ + .name = field.name, + .value = i, + }; + } + + break :key @Type(.{ + .Enum = .{ + .tag_type = std.math.IntFittingRange(0, field_infos.len - 1), + .fields = &enumFields, + .decls = &decls, + .is_exhaustive = true, + }, + }); + }; + pub fn deinit(self: *Config) void { if (self._arena) |arena| arena.deinit(); self.* = undefined; @@ -655,6 +679,77 @@ pub const Config = struct { } } + /// Returns an iterator that goes through each changed field from + /// old to new. + pub fn changeIterator(old: *const Config, new: *const Config) ChangeIterator { + return .{ + .old = old, + .new = new, + }; + } + + fn equal(comptime T: type, old: T, new: T) bool { + // Do known named types first + switch (T) { + inline []const u8, + [:0]const u8, + => return std.mem.eql(u8, old, new), + + else => {}, + } + + // Back into types of types + switch (@typeInfo(T)) { + inline .Bool, + .Int, + => return old == new, + + .Optional => |info| { + if (old == null and new == null) return true; + if (old == null or new == null) return false; + return equal(info.child, old.?, new.?); + }, + + .Struct => return old.equal(new), + + else => { + @compileLog(T); + @compileError("unsupported field type"); + }, + } + } + + pub const ChangeIterator = struct { + old: *const Config, + new: *const Config, + i: usize = 0, + + pub fn next(self: *ChangeIterator) ?Key { + const fields = comptime std.meta.fields(Config); + + while (self.i < fields.len) { + switch (self.i) { + inline 0...(fields.len - 1) => |i| { + const field = fields[i]; + self.i += 1; + + if (field.name[0] == '_') return self.next(); + + const old_value = @field(self.old, field.name); + const new_value = @field(self.new, field.name); + if (!equal(field.type, old_value, new_value)) { + return @field(Key, field.name); + } + }, + + else => unreachable, + } + } + + return null; + } + }; + test "clone default" { const testing = std.testing; const alloc = testing.allocator; @@ -664,6 +759,10 @@ pub const Config = struct { var dest = try source.clone(alloc); defer dest.deinit(); + // Should have no changes + var it = source.changeIterator(&dest); + try testing.expectEqual(@as(?Key, null), it.next()); + // I want to do this but this doesn't work (the API doesn't work) // try testing.expectEqualDeep(dest, source); } @@ -693,6 +792,11 @@ pub const Color = struct { return self; } + /// Compare if two of our value are requal. Required by Config. + pub fn equal(self: Color, other: Color) bool { + return std.meta.eql(self, other); + } + /// fromHex parses a color from a hex value such as #RRGGBB. The "#" /// is optional. pub fn fromHex(input: []const u8) !Color { @@ -761,6 +865,11 @@ pub const Palette = struct { return self; } + /// Compare if two of our value are requal. Required by Config. + pub fn equal(self: Self, other: Self) bool { + return std.meta.eql(self, other); + } + test "parseCLI" { const testing = std.testing; @@ -801,6 +910,16 @@ pub const RepeatableString = struct { }; } + /// Compare if two of our value are requal. Required by Config. + pub fn equal(self: Self, other: Self) bool { + const itemsA = self.list.items; + const itemsB = other.list.items; + if (itemsA.len != itemsB.len) return false; + for (itemsA, itemsB) |a, b| { + if (!std.mem.eql(u8, a, b)) return false; + } else return true; + } + test "parseCLI" { const testing = std.testing; var arena = ArenaAllocator.init(testing.allocator); @@ -854,6 +973,14 @@ pub const Keybinds = struct { }; } + /// Compare if two of our value are requal. Required by Config. + pub fn equal(self: Keybinds, other: Keybinds) bool { + // TODO + _ = self; + _ = other; + return true; + } + test "parseCLI" { const testing = std.testing; var arena = ArenaAllocator.init(testing.allocator); From 0d93da5f3055935345fb00fa8cf3c01d5e90052e Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 12 Mar 2023 21:34:06 -0700 Subject: [PATCH 03/19] config: changed() to test if a specific key has changed --- src/config.zig | 67 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 18 deletions(-) diff --git a/src/config.zig b/src/config.zig index 310212176..262a59a8b 100644 --- a/src/config.zig +++ b/src/config.zig @@ -172,18 +172,24 @@ pub const Config = struct { pub const Key = key: { const field_infos = std.meta.fields(Config); var enumFields: [field_infos.len]std.builtin.Type.EnumField = undefined; - var decls = [_]std.builtin.Type.Declaration{}; - inline for (field_infos, 0..) |field, i| { + var i: usize = 0; + inline for (field_infos) |field| { + // Ignore fields starting with "_" since they're internal and + // not copied ever. + if (field.name[0] == '_') continue; + enumFields[i] = .{ .name = field.name, .value = i, }; + i += 1; } + var decls = [_]std.builtin.Type.Declaration{}; break :key @Type(.{ .Enum = .{ .tag_type = std.math.IntFittingRange(0, field_infos.len - 1), - .fields = &enumFields, + .fields = enumFields[0..i], .decls = &decls, .is_exhaustive = true, }, @@ -635,10 +641,7 @@ pub const Config = struct { const alloc = result._arena.?.allocator(); inline for (@typeInfo(Config).Struct.fields) |field| { - // Ignore fields starting with "_" since they're internal and - // not copied ever. - if (field.name[0] == '_') continue; - + if (!@hasField(Key, field.name)) continue; @field(result, field.name) = try cloneValue( alloc, field.type, @@ -680,7 +683,7 @@ pub const Config = struct { } /// Returns an iterator that goes through each changed field from - /// old to new. + /// old to new. The order of old or new do not matter. pub fn changeIterator(old: *const Config, new: *const Config) ChangeIterator { return .{ .old = old, @@ -688,6 +691,26 @@ pub const Config = struct { }; } + /// Returns true if the given key has changed from old to new. This + /// requires the key to be comptime known to make this more efficient. + pub fn changed(self: *const Config, new: *const Config, comptime key: Key) bool { + // Get the field at comptime + const field = comptime field: { + const fields = std.meta.fields(Config); + for (fields) |field| { + if (@field(Key, field.name) == key) { + break :field field; + } + } + + unreachable; + }; + + const old_value = @field(self, field.name); + const new_value = @field(new, field.name); + return !equal(field.type, old_value, new_value); + } + fn equal(comptime T: type, old: T, new: T) bool { // Do known named types first switch (T) { @@ -719,27 +742,21 @@ pub const Config = struct { } } + /// This yields a key for every changed field between old and new. pub const ChangeIterator = struct { old: *const Config, new: *const Config, i: usize = 0, pub fn next(self: *ChangeIterator) ?Key { - const fields = comptime std.meta.fields(Config); - + const fields = comptime std.meta.fields(Key); while (self.i < fields.len) { switch (self.i) { inline 0...(fields.len - 1) => |i| { const field = fields[i]; + const key = @field(Key, field.name); self.i += 1; - - if (field.name[0] == '_') return self.next(); - - const old_value = @field(self.old, field.name); - const new_value = @field(self.new, field.name); - if (!equal(field.type, old_value, new_value)) { - return @field(Key, field.name); - } + if (self.old.changed(self.new, key)) return key; }, else => unreachable, @@ -766,6 +783,20 @@ pub const Config = struct { // I want to do this but this doesn't work (the API doesn't work) // try testing.expectEqualDeep(dest, source); } + + test "changed" { + const testing = std.testing; + const alloc = testing.allocator; + + var source = try Config.default(alloc); + defer source.deinit(); + var dest = try source.clone(alloc); + defer dest.deinit(); + dest.@"font-family" = "something else"; + + try testing.expect(source.changed(&dest, .@"font-family")); + try testing.expect(!source.changed(&dest, .@"font-size")); + } }; /// Color represents a color using RGB. From 11e4215f9fba4878d35239e4851fc721a3943019 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 12 Mar 2023 21:52:48 -0700 Subject: [PATCH 04/19] config: implement comparison for keybinding change --- src/config.zig | 122 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 88 insertions(+), 34 deletions(-) diff --git a/src/config.zig b/src/config.zig index 262a59a8b..06626b3e7 100644 --- a/src/config.zig +++ b/src/config.zig @@ -1,3 +1,4 @@ +const config = @This(); const std = @import("std"); const builtin = @import("builtin"); const Allocator = std.mem.Allocator; @@ -711,37 +712,6 @@ pub const Config = struct { return !equal(field.type, old_value, new_value); } - fn equal(comptime T: type, old: T, new: T) bool { - // Do known named types first - switch (T) { - inline []const u8, - [:0]const u8, - => return std.mem.eql(u8, old, new), - - else => {}, - } - - // Back into types of types - switch (@typeInfo(T)) { - inline .Bool, - .Int, - => return old == new, - - .Optional => |info| { - if (old == null and new == null) return true; - if (old == null or new == null) return false; - return equal(info.child, old.?, new.?); - }, - - .Struct => return old.equal(new), - - else => { - @compileLog(T); - @compileError("unsupported field type"); - }, - } - } - /// This yields a key for every changed field between old and new. pub const ChangeIterator = struct { old: *const Config, @@ -799,6 +769,78 @@ pub const Config = struct { } }; +/// A config-specific helper to determine if two values of the same +/// type are equal. This isn't the same as std.mem.eql or std.testing.equals +/// because we expect structs to implement their own equality. +/// +/// This also doesn't support ALL Zig types, because we only add to it +/// as we need types for the config. +fn equal(comptime T: type, old: T, new: T) bool { + // Do known named types first + switch (T) { + inline []const u8, + [:0]const u8, + => return std.mem.eql(u8, old, new), + + else => {}, + } + + // Back into types of types + switch (@typeInfo(T)) { + .Void => return true, + + inline .Bool, + .Int, + .Enum, + => return old == new, + + .Optional => |info| { + if (old == null and new == null) return true; + if (old == null or new == null) return false; + return equal(info.child, old.?, new.?); + }, + + .Struct => |info| { + if (@hasDecl(T, "equal")) return old.equal(new); + + // If a struct doesn't declare an "equal" function, we fall back + // to a recursive field-by-field compare. + inline for (info.fields) |field_info| { + if (!equal( + field_info.type, + @field(old, field_info.name), + @field(new, field_info.name), + )) return false; + } + return true; + }, + + .Union => |info| { + const tag_type = info.tag_type.?; + const old_tag = std.meta.activeTag(old); + const new_tag = std.meta.activeTag(new); + if (old_tag != new_tag) return false; + + inline for (info.fields) |field_info| { + if (@field(tag_type, field_info.name) == old_tag) { + return equal( + field_info.type, + @field(old, field_info.name), + @field(new, field_info.name), + ); + } + } + + unreachable; + }, + + else => { + @compileLog(T); + @compileError("unsupported field type"); + }, + } +} + /// Color represents a color using RGB. pub const Color = struct { r: u8, @@ -1006,9 +1048,21 @@ pub const Keybinds = struct { /// Compare if two of our value are requal. Required by Config. pub fn equal(self: Keybinds, other: Keybinds) bool { - // TODO - _ = self; - _ = other; + const self_map = self.set.bindings; + const other_map = other.set.bindings; + if (self_map.count() != other_map.count()) return false; + + var it = self_map.iterator(); + while (it.next()) |self_entry| { + const other_entry = other_map.getEntry(self_entry.key_ptr.*) orelse + return false; + if (!config.equal( + inputpkg.Binding.Action, + self_entry.value_ptr.*, + other_entry.value_ptr.*, + )) return false; + } + return true; } From 3ce7baf30e4540a8bfbc71b593638858ce9a797c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 12 Mar 2023 22:03:20 -0700 Subject: [PATCH 05/19] config: dedicated load func so we can reload --- src/config.zig | 28 ++++++++++++++++++++++++++++ src/main.zig | 16 +--------------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/config.zig b/src/config.zig index 06626b3e7..3789fe1a8 100644 --- a/src/config.zig +++ b/src/config.zig @@ -202,6 +202,34 @@ pub const Config = struct { self.* = undefined; } + /// Load the configuration according to the default rules: + /// + /// 1. Defaults + /// 2. XDG Config File + /// 3. CLI flags + /// 4. Recursively defined configuration files + /// + pub fn load(alloc_gpa: Allocator) !Config { + var result = try default(alloc_gpa); + errdefer result.deinit(); + + // 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 + { + var iter = try std.process.argsWithAllocator(alloc_gpa); + defer iter.deinit(); + try cli_args.parse(Config, alloc_gpa, &result, &iter); + } + + // Parse the config files that were added from our file and CLI args. + try result.loadRecursiveFiles(alloc_gpa); + try result.finalize(); + + return result; + } + pub fn default(alloc_gpa: Allocator) Allocator.Error!Config { // Build up our basic config var result: Config = .{ diff --git a/src/main.zig b/src/main.zig index abd9dfdef..899f0494a 100644 --- a/src/main.zig +++ b/src/main.zig @@ -30,22 +30,8 @@ pub fn main() !void { const alloc = state.alloc; // Try reading our config - var config = try Config.default(alloc); + var config = try Config.load(alloc); defer config.deinit(); - - // If we have a configuration file in our home directory, parse that first. - try config.loadDefaultFiles(alloc); - - // Parse the config from the CLI args - { - var iter = try std.process.argsWithAllocator(alloc); - defer iter.deinit(); - try cli_args.parse(Config, alloc, &config, &iter); - } - - // Parse the config files that were added from our file and CLI args. - try config.loadRecursiveFiles(alloc); - try config.finalize(); //std.log.debug("config={}", .{config}); // Create our app state From 9b10b5d71638e6c7eadc60da24639c8ff96f7ba7 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Mar 2023 21:13:20 -0700 Subject: [PATCH 06/19] surface doesn't store a pointer to Config anymore --- src/App.zig | 4 +++- src/Surface.zig | 57 +++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/App.zig b/src/App.zig index 77ee6c7d0..f06fe1443 100644 --- a/src/App.zig +++ b/src/App.zig @@ -30,7 +30,9 @@ alloc: Allocator, /// The list of surfaces that are currently active. surfaces: SurfaceList, -// The configuration for the app. +// The configuration for the app. This may change (app runtimes are notified +// via the callback), but the change will only ever happen during tick() +// so app runtimes can ensure there are no data races in reading this. config: *const Config, /// The mailbox that can be used to send this thread messages. Note diff --git a/src/Surface.zig b/src/Surface.zig index d29aead0c..806529af6 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -19,6 +19,7 @@ const std = @import("std"); const builtin = @import("builtin"); const assert = std.debug.assert; const Allocator = std.mem.Allocator; +const ArenaAllocator = std.heap.ArenaAllocator; const renderer = @import("renderer.zig"); const termio = @import("termio.zig"); const objc = @import("objc"); @@ -28,7 +29,7 @@ const font = @import("font/main.zig"); const Command = @import("Command.zig"); const trace = @import("tracy").trace; const terminal = @import("terminal/main.zig"); -const Config = @import("config.zig").Config; +const configpkg = @import("config.zig"); const input = @import("input.zig"); const DevMode = @import("DevMode.zig"); const App = @import("App.zig"); @@ -85,8 +86,10 @@ cell_size: renderer.CellSize, /// Explicit padding due to configuration padding: renderer.Padding, -/// The app configuration -config: *const Config, +/// The configuration derived from the main config. We "derive" it so that +/// we don't have a shared pointer hanging around that we need to worry about +/// the lifetime of. This makes updating config at runtime easier. +config: DerivedConfig, /// Set to true for a single GLFW key/char callback cycle to cause the /// char callback to ignore. GLFW seems to always do key followed by char @@ -123,13 +126,48 @@ const Mouse = struct { event_point: terminal.point.Viewport = .{}, }; +/// The configuration that a surface has, this is copied from the main +/// Config struct usually to prevent sharing a single value. +const DerivedConfig = struct { + arena: ArenaAllocator, + + /// For docs for these, see the associated config they are derived from. + original_font_size: u8, + keybind: configpkg.Keybinds, + clipboard_read: bool, + clipboard_write: bool, + clipboard_trim_trailing_spaces: bool, + + pub fn init(alloc_gpa: Allocator, config: *const configpkg.Config) !DerivedConfig { + var arena = ArenaAllocator.init(alloc_gpa); + errdefer arena.deinit(); + const alloc = arena.allocator(); + + return .{ + .original_font_size = config.@"font-size", + .keybind = try config.keybind.clone(alloc), + .clipboard_read = config.@"clipboard-read", + .clipboard_write = config.@"clipboard-write", + .clipboard_trim_trailing_spaces = config.@"clipboard-trim-trailing-spaces", + + // Assignments happen sequentially so we have to do this last + // so that the memory is captured from allocs above. + .arena = arena, + }; + } + + pub fn deinit(self: *DerivedConfig) void { + self.arena.deinit(); + } +}; + /// Create a new surface. This must be called from the main thread. The /// pointer to the memory for the surface must be provided and must be /// stable due to interfacing with various callbacks. pub fn init( self: *Surface, alloc: Allocator, - config: *const Config, + config: *const configpkg.Config, app_mailbox: App.Mailbox, rt_surface: *apprt.runtime.Surface, ) !void { @@ -369,7 +407,7 @@ pub fn init( .grid_size = grid_size, .cell_size = cell_size, .padding = padding, - .config = config, + .config = try DerivedConfig.init(alloc, config), .imgui_ctx = if (!DevMode.enabled) {} else try imgui.Context.create(), }; @@ -476,6 +514,7 @@ pub fn deinit(self: *Surface) void { self.alloc.destroy(self.font_group); self.alloc.destroy(self.renderer_state.mutex); + self.config.deinit(); log.info("surface closed addr={x}", .{@ptrToInt(self)}); } @@ -557,7 +596,7 @@ pub fn imePoint(self: *const Surface) apprt.IMEPos { } fn clipboardRead(self: *const Surface, kind: u8) !void { - if (!self.config.@"clipboard-read") { + if (!self.config.clipboard_read) { log.info("application attempted to read clipboard, but 'clipboard-read' setting is off", .{}); return; } @@ -593,7 +632,7 @@ fn clipboardRead(self: *const Surface, kind: u8) !void { } fn clipboardWrite(self: *const Surface, data: []const u8) !void { - if (!self.config.@"clipboard-write") { + if (!self.config.clipboard_write) { log.info("application attempted to write clipboard, but 'clipboard-write' setting is off", .{}); return; } @@ -833,7 +872,7 @@ pub fn keyCallback( var buf = self.io.terminal.screen.selectionString( self.alloc, sel, - self.config.@"clipboard-trim-trailing-spaces", + self.config.clipboard_trim_trailing_spaces, ) catch |err| { log.err("error reading selection string err={}", .{err}); return; @@ -901,7 +940,7 @@ pub fn keyCallback( log.debug("reset font size", .{}); var size = self.font_size; - size.points = self.config.@"font-size"; + size.points = self.config.original_font_size; self.setFontSize(size); }, From 8cb9ee5d597339ca9bf4c58b1bfe8626f169c6c8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Mar 2023 21:16:55 -0700 Subject: [PATCH 07/19] make it claer the config pointer is not stable after renderer/IO init --- src/renderer/Options.zig | 4 +++- src/termio/Options.zig | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/renderer/Options.zig b/src/renderer/Options.zig index c8b58c8c7..7450dd9f2 100644 --- a/src/renderer/Options.zig +++ b/src/renderer/Options.zig @@ -5,7 +5,9 @@ const font = @import("../font/main.zig"); const renderer = @import("../renderer.zig"); const Config = @import("../config.zig").Config; -/// The app configuration. +/// The app configuration. This must NOT be stored by any termio implementation. +/// The memory it points to is NOT stable after the init call so any values +/// in here must be copied. config: *const Config, /// The font group that should be used. diff --git a/src/termio/Options.zig b/src/termio/Options.zig index 06af6b4d0..28a10f1f4 100644 --- a/src/termio/Options.zig +++ b/src/termio/Options.zig @@ -11,7 +11,9 @@ grid_size: renderer.GridSize, /// The size of the viewport in pixels. screen_size: renderer.ScreenSize, -/// The app configuration. +/// The app configuration. This must NOT be stored by any termio implementation. +/// The memory it points to is NOT stable after the init call so any values +/// in here must be copied. config: *const Config, /// The render state. The IO implementation can modify anything here. The From 3e1f975551c2258b82dbc4bb747fef3a5d5d1910 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Mar 2023 21:44:45 -0700 Subject: [PATCH 08/19] move config loading into apprt to prep for reloading --- src/App.zig | 35 +++++++++++++++++++++++++++-------- src/Surface.zig | 20 ++++++++++++++++++++ src/apprt/glfw.zig | 15 +++++++++++---- src/apprt/gtk.zig | 13 +++++++++++-- src/apprt/surface.zig | 6 ++++++ src/main.zig | 9 +-------- 6 files changed, 76 insertions(+), 22 deletions(-) diff --git a/src/App.zig b/src/App.zig index f06fe1443..e9358b3a1 100644 --- a/src/App.zig +++ b/src/App.zig @@ -30,11 +30,6 @@ alloc: Allocator, /// The list of surfaces that are currently active. surfaces: SurfaceList, -// The configuration for the app. This may change (app runtimes are notified -// via the callback), but the change will only ever happen during tick() -// so app runtimes can ensure there are no data races in reading this. -config: *const Config, - /// The mailbox that can be used to send this thread messages. Note /// this is a blocking queue so if it is full you will get errors (or block). mailbox: Mailbox.Queue, @@ -47,17 +42,15 @@ quit: bool, /// "startup" logic. pub fn create( alloc: Allocator, - config: *const Config, ) !*App { // If we have DevMode on, store the config so we can show it - if (DevMode.enabled) DevMode.instance.config = config; + //if (DevMode.enabled) DevMode.instance.config = config; var app = try alloc.create(App); errdefer alloc.destroy(app); app.* = .{ .alloc = alloc, .surfaces = .{}, - .config = config, .mailbox = .{}, .quit = false, }; @@ -99,6 +92,21 @@ pub fn tick(self: *App, rt_app: *apprt.App) !bool { return self.quit or self.surfaces.items.len == 0; } +/// Update the configuration associated with the app. This can only be +/// called from the main thread. +/// +/// The caller owns the config memory. The prior config must not be freed +/// until this function returns successfully. +pub fn updateConfig(self: *App, config: *const Config) !void { + // Update our config + self.config = config; + + // Go through and update all of the surface configurations. + for (self.surfaces.items) |surface| { + try surface.handleMessage(.{ .change_config = config }); + } +} + /// Add an initialized surface. This is really only for the runtime /// implementations to call and should NOT be called by general app users. /// The surface must be from the pool. @@ -125,6 +133,7 @@ fn drainMailbox(self: *App, rt_app: *apprt.App) !void { while (self.mailbox.pop()) |message| { log.debug("mailbox message={s}", .{@tagName(message)}); switch (message) { + .reload_config => try self.reloadConfig(rt_app), .new_window => |msg| try self.newWindow(rt_app, msg), .close => |surface| try self.closeSurface(rt_app, surface), .quit => try self.setQuit(), @@ -134,6 +143,12 @@ fn drainMailbox(self: *App, rt_app: *apprt.App) !void { } } +fn reloadConfig(self: *App, rt_app: *apprt.App) !void { + _ = rt_app; + _ = self; + //try rt_app.reloadConfig(); +} + fn closeSurface(self: *App, rt_app: *apprt.App, surface: *Surface) !void { if (!self.hasSurface(surface)) return; rt_app.closeSurface(surface.rt_surface); @@ -195,6 +210,10 @@ fn hasSurface(self: *App, surface: *Surface) bool { /// The message types that can be sent to the app thread. pub const Message = union(enum) { + /// Reload the configuration for the entire app and propagate it to + /// all the active surfaces. + reload_config: void, + /// Create a new terminal window. new_window: NewWindow, diff --git a/src/Surface.zig b/src/Surface.zig index 806529af6..b34eabaa2 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -528,6 +528,8 @@ pub fn close(self: *Surface) void { /// surface. pub fn handleMessage(self: *Surface, msg: Message) !void { switch (msg) { + .change_config => |config| try self.changeConfig(config), + .set_title => |*v| { // The ptrCast just gets sliceTo to return the proper type. // We know that our title should end in 0. @@ -553,6 +555,24 @@ pub fn handleMessage(self: *Surface, msg: Message) !void { } } +/// Update our configuration at runtime. +fn changeConfig(self: *Surface, config: *const configpkg.Config) !void { + // Update our new derived config immediately + const derived = DerivedConfig.init(self.alloc, config) catch |err| { + // If the derivation fails then we just log and return. We don't + // hard fail in this case because we don't want to error the surface + // when config fails we just want to keep using the old config. + log.err("error updating configuration err={}", .{err}); + return; + }; + self.config.deinit(); + self.config = derived; + + // Update our derived configurations for the renderer and termio, + // then send them a message to update. + // TODO +} + /// Returns the x/y coordinate of where the IME (Input Method Editor) /// keyboard should be rendered. pub fn imePoint(self: *const Surface) apprt.IMEPos { diff --git a/src/apprt/glfw.zig b/src/apprt/glfw.zig index 1995f03cd..c86031602 100644 --- a/src/apprt/glfw.zig +++ b/src/apprt/glfw.zig @@ -19,6 +19,7 @@ const Renderer = renderer.Renderer; const apprt = @import("../apprt.zig"); const CoreApp = @import("../App.zig"); const CoreSurface = @import("../Surface.zig"); +const Config = @import("../config.zig").Config; // Get native API access on certain platforms so we can do more customization. const glfwNative = glfw.Native(.{ @@ -29,6 +30,7 @@ const log = std.log.scoped(.glfw); pub const App = struct { app: *CoreApp, + config: Config, /// Mac-specific state. darwin: if (Darwin.enabled) Darwin else void, @@ -53,14 +55,19 @@ pub const App = struct { var darwin = if (Darwin.enabled) try Darwin.init() else {}; errdefer if (Darwin.enabled) darwin.deinit(); + // Load our configuration + var config = try Config.load(core_app.alloc); + errdefer config.deinit(); + return .{ .app = core_app, + .config = config, .darwin = darwin, }; } - pub fn terminate(self: App) void { - _ = self; + pub fn terminate(self: *App) void { + self.config.deinit(); glfw.terminate(); } @@ -139,7 +146,7 @@ pub const App = struct { errdefer surface.deinit(); // If we have a parent, inherit some properties - if (self.app.config.@"window-inherit-font-size") { + if (self.config.@"window-inherit-font-size") { if (parent_) |parent| { surface.core_surface.setFontSize(parent.font_size); } @@ -313,7 +320,7 @@ pub const Surface = struct { // Initialize our surface now that we have the stable pointer. try self.core_surface.init( app.app.alloc, - app.app.config, + &app.config, .{ .rt_app = app, .mailbox = &app.app.mailbox }, self, ); diff --git a/src/apprt/gtk.zig b/src/apprt/gtk.zig index 218866a33..c6556e49d 100644 --- a/src/apprt/gtk.zig +++ b/src/apprt/gtk.zig @@ -9,6 +9,7 @@ const apprt = @import("../apprt.zig"); const input = @import("../input.zig"); const CoreApp = @import("../App.zig"); const CoreSurface = @import("../Surface.zig"); +const Config = @import("../config.zig").Config; pub const c = @cImport({ @cInclude("gtk/gtk.h"); @@ -37,6 +38,7 @@ pub const App = struct { }; core_app: *CoreApp, + config: Config, app: *c.GtkApplication, ctx: *c.GMainContext, @@ -53,6 +55,10 @@ pub const App = struct { // rid of this dep. if (!glfw.init(.{})) return error.GlfwInitFailed; + // Load our configuration + var config = try Config.load(core_app.alloc); + errdefer config.deinit(); + // Create our GTK Application which encapsulates our process. const app = @ptrCast(?*c.GtkApplication, c.gtk_application_new( null, @@ -108,6 +114,7 @@ pub const App = struct { return .{ .core_app = core_app, .app = app, + .config = config, .ctx = ctx, .cursor_default = cursor_default, .cursor_ibeam = cursor_ibeam, @@ -116,7 +123,7 @@ pub const App = struct { // Terminate the application. The application will not be restarted after // this so all global state can be cleaned up. - pub fn terminate(self: App) void { + pub fn terminate(self: *App) void { c.g_settings_sync(); while (c.g_main_context_iteration(self.ctx, 0) != 0) {} c.g_main_context_release(self.ctx); @@ -125,6 +132,8 @@ pub const App = struct { c.g_object_unref(self.cursor_ibeam); c.g_object_unref(self.cursor_default); + self.config.deinit(); + glfw.terminate(); } @@ -575,7 +584,7 @@ pub const Surface = struct { // Initialize our surface now that we have the stable pointer. try self.core_surface.init( self.app.core_app.alloc, - self.app.core_app.config, + &self.app.config, .{ .rt_app = self.app, .mailbox = &self.app.core_app.mailbox }, self, ); diff --git a/src/apprt/surface.zig b/src/apprt/surface.zig index 3f409efb7..d5ebc0083 100644 --- a/src/apprt/surface.zig +++ b/src/apprt/surface.zig @@ -2,6 +2,7 @@ const App = @import("../App.zig"); const Surface = @import("../Surface.zig"); const renderer = @import("../renderer.zig"); const termio = @import("../termio.zig"); +const Config = @import("../config.zig").Config; /// The message types that can be sent to a single surface. pub const Message = union(enum) { @@ -24,6 +25,11 @@ pub const Message = union(enum) { /// Write the clipboard contents. clipboard_write: WriteReq, + /// Change the configuration to the given configuration. The pointer is + /// not valid after receiving this message so any config must be used + /// and derived immediately. + change_config: *const Config, + /// Close the surface. This will only close the current surface that /// receives this, not the full application. close: void, diff --git a/src/main.zig b/src/main.zig index 899f0494a..9600a4a66 100644 --- a/src/main.zig +++ b/src/main.zig @@ -14,8 +14,6 @@ const xdg = @import("xdg.zig"); const apprt = @import("apprt.zig"); const App = @import("App.zig"); -const cli_args = @import("cli_args.zig"); -const Config = @import("config.zig").Config; const Ghostty = @import("main_c.zig").Ghostty; /// Global process state. This is initialized in main() for exe artifacts @@ -29,13 +27,8 @@ pub fn main() !void { defer state.deinit(); const alloc = state.alloc; - // Try reading our config - var config = try Config.load(alloc); - defer config.deinit(); - //std.log.debug("config={}", .{config}); - // Create our app state - var app = try App.create(alloc, &config); + var app = try App.create(alloc); defer app.destroy(); // Create our runtime app From a9928cfb90a70bb5a24aca2b9193a1c4c38d8d76 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Mar 2023 21:52:42 -0700 Subject: [PATCH 09/19] implement reload_config app message --- src/App.zig | 21 ++++++--------------- src/DevMode.zig | 2 +- src/apprt/glfw.zig | 23 +++++++++++++++++++++++ src/apprt/gtk.zig | 17 +++++++++++++++++ 4 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/App.zig b/src/App.zig index e9358b3a1..7563058b2 100644 --- a/src/App.zig +++ b/src/App.zig @@ -18,7 +18,6 @@ const renderer = @import("renderer.zig"); const font = @import("font/main.zig"); const macos = @import("macos"); const objc = @import("objc"); -const DevMode = @import("DevMode.zig"); const log = std.log.scoped(.app); @@ -43,9 +42,6 @@ quit: bool, pub fn create( alloc: Allocator, ) !*App { - // If we have DevMode on, store the config so we can show it - //if (DevMode.enabled) DevMode.instance.config = config; - var app = try alloc.create(App); errdefer alloc.destroy(app); app.* = .{ @@ -93,17 +89,12 @@ pub fn tick(self: *App, rt_app: *apprt.App) !bool { } /// Update the configuration associated with the app. This can only be -/// called from the main thread. -/// -/// The caller owns the config memory. The prior config must not be freed -/// until this function returns successfully. +/// called from the main thread. The caller owns the config memory. The +/// memory can be freed immediately when this returns. pub fn updateConfig(self: *App, config: *const Config) !void { - // Update our config - self.config = config; - // Go through and update all of the surface configurations. for (self.surfaces.items) |surface| { - try surface.handleMessage(.{ .change_config = config }); + try surface.core_surface.handleMessage(.{ .change_config = config }); } } @@ -144,9 +135,9 @@ fn drainMailbox(self: *App, rt_app: *apprt.App) !void { } fn reloadConfig(self: *App, rt_app: *apprt.App) !void { - _ = rt_app; - _ = self; - //try rt_app.reloadConfig(); + if (try rt_app.reloadConfig()) |new| { + try self.updateConfig(new); + } } fn closeSurface(self: *App, rt_app: *apprt.App, surface: *Surface) !void { diff --git a/src/DevMode.zig b/src/DevMode.zig index bd28366ca..29288046a 100644 --- a/src/DevMode.zig +++ b/src/DevMode.zig @@ -27,7 +27,7 @@ pub var instance: DevMode = .{}; visible: bool = false, /// Our app config -config: ?*const Config = null, +config: ?Config = null, /// The surface we're tracking. surface: ?*Surface = null, diff --git a/src/apprt/glfw.zig b/src/apprt/glfw.zig index c86031602..dfe758bf9 100644 --- a/src/apprt/glfw.zig +++ b/src/apprt/glfw.zig @@ -20,6 +20,7 @@ const apprt = @import("../apprt.zig"); const CoreApp = @import("../App.zig"); const CoreSurface = @import("../Surface.zig"); const Config = @import("../config.zig").Config; +const DevMode = @import("../DevMode.zig"); // Get native API access on certain platforms so we can do more customization. const glfwNative = glfw.Native(.{ @@ -59,6 +60,11 @@ pub const App = struct { var config = try Config.load(core_app.alloc); errdefer config.deinit(); + // If we have DevMode on, store the config so we can show it. This + // is messy because we're copying a thing here. We should clean this + // up when we take a pass at cleaning up the dev mode. + if (DevMode.enabled) DevMode.instance.config = config; + return .{ .app = core_app, .config = config, @@ -97,6 +103,23 @@ pub const App = struct { glfw.postEmptyEvent(); } + /// Reload the configuration. This should return the new configuration. + /// The old value can be freed immediately at this point assuming a + /// successful return. + /// + /// The returned pointer value is only valid for a stable self pointer. + pub fn reloadConfig(self: *App) !?*const Config { + // Load our configuration + var config = try Config.load(self.app.alloc); + errdefer config.deinit(); + + // Update the existing config, be sure to clean up the old one. + self.config.deinit(); + self.config = config; + + return &self.config; + } + /// Create a new window for the app. pub fn newWindow(self: *App, parent_: ?*CoreSurface) !void { _ = try self.newSurface(parent_); diff --git a/src/apprt/gtk.zig b/src/apprt/gtk.zig index c6556e49d..5d6937aa7 100644 --- a/src/apprt/gtk.zig +++ b/src/apprt/gtk.zig @@ -137,6 +137,23 @@ pub const App = struct { glfw.terminate(); } + /// Reload the configuration. This should return the new configuration. + /// The old value can be freed immediately at this point assuming a + /// successful return. + /// + /// The returned pointer value is only valid for a stable self pointer. + pub fn reloadConfig(self: *App) !?*const Config { + // Load our configuration + var config = try Config.load(self.core_app.alloc); + errdefer config.deinit(); + + // Update the existing config, be sure to clean up the old one. + self.config.deinit(); + self.config = config; + + return &self.config; + } + pub fn wakeup(self: App) void { _ = self; c.g_main_context_wakeup(null); From f5c1dfa37471e8ea2ceff494e9b09953a6b485e4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Mar 2023 22:00:10 -0700 Subject: [PATCH 10/19] reload_config keybinding (defaults to ctrl+alt+super+space) --- src/App.zig | 2 ++ src/Surface.zig | 6 ++++++ src/config.zig | 6 ++++++ src/input/Binding.zig | 6 ++++++ 4 files changed, 20 insertions(+) diff --git a/src/App.zig b/src/App.zig index 7563058b2..f2d18a7fd 100644 --- a/src/App.zig +++ b/src/App.zig @@ -135,7 +135,9 @@ fn drainMailbox(self: *App, rt_app: *apprt.App) !void { } fn reloadConfig(self: *App, rt_app: *apprt.App) !void { + log.debug("reloading configuration", .{}); if (try rt_app.reloadConfig()) |new| { + log.debug("new configuration received, applying", .{}); try self.updateConfig(new); } } diff --git a/src/Surface.zig b/src/Surface.zig index b34eabaa2..6e002091b 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -852,6 +852,12 @@ pub fn keyCallback( .unbind => unreachable, .ignore => {}, + .reload_config => { + _ = self.app_mailbox.push(.{ + .reload_config = {}, + }, .{ .instant = {} }); + }, + .csi => |data| { _ = self.io_thread.mailbox.push(.{ .write_stable = "\x1B[", diff --git a/src/config.zig b/src/config.zig index 3789fe1a8..c30355f55 100644 --- a/src/config.zig +++ b/src/config.zig @@ -239,6 +239,12 @@ pub const Config = struct { const alloc = result._arena.?.allocator(); // Add our default keybindings + try result.keybind.set.put( + alloc, + .{ .key = .space, .mods = .{ .super = true, .alt = true, .ctrl = true } }, + .{ .reload_config = {} }, + ); + { // On macOS we default to super but Linux ctrl+shift since // ctrl+c is to kill the process. diff --git a/src/input/Binding.zig b/src/input/Binding.zig index cac34826c..746ccbff7 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -187,6 +187,12 @@ pub const Action = union(enum) { /// Focus on a split in a given direction. goto_split: SplitFocusDirection, + /// Reload the configuration. The exact meaning depends on the app runtime + /// in use but this usually involves re-reading the configuration file + /// and applying any changes. Note that not all changes can be applied at + /// runtime. + reload_config: void, + /// Close the current "surface", whether that is a window, tab, split, /// etc. This only closes ONE surface. close_surface: void, From 8d3f40fa41d996a9a2af71da1fe5de82e6a84260 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 13 Mar 2023 22:08:35 -0700 Subject: [PATCH 11/19] apprt/embedded: reload config support --- macos/Sources/Ghostty/AppState.swift | 8 +++++++ src/apprt/embedded.zig | 32 ++++++++++++++++++++++------ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/macos/Sources/Ghostty/AppState.swift b/macos/Sources/Ghostty/AppState.swift index 892b41af5..2307aa082 100644 --- a/macos/Sources/Ghostty/AppState.swift +++ b/macos/Sources/Ghostty/AppState.swift @@ -54,6 +54,7 @@ extension Ghostty { var runtime_cfg = ghostty_runtime_config_s( userdata: Unmanaged.passUnretained(self).toOpaque(), wakeup_cb: { userdata in AppState.wakeup(userdata) }, + reload_config_cb: { userdata in AppState.reloadConfig(userdata) }, set_title_cb: { userdata, title in AppState.setTitle(userdata, title: title) }, read_clipboard_cb: { userdata in AppState.readClipboard(userdata) }, write_clipboard_cb: { userdata, str in AppState.writeClipboard(userdata, string: str) }, @@ -140,6 +141,13 @@ extension Ghostty { pb.setString(valueStr, forType: .string) } + static func reloadConfig(_ userdata: UnsafeMutableRawPointer?) -> ghostty_config_t? { + // TODO: implement config reloading in the mac app + let state = Unmanaged.fromOpaque(userdata!).takeUnretainedValue() + _ = state + return nil + } + static func wakeup(_ userdata: UnsafeMutableRawPointer?) { let state = Unmanaged.fromOpaque(userdata!).takeUnretainedValue() diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index b782ba8cf..08fd04a5d 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -13,6 +13,7 @@ const apprt = @import("../apprt.zig"); const input = @import("../input.zig"); const CoreApp = @import("../App.zig"); const CoreSurface = @import("../Surface.zig"); +const Config = @import("../config.zig").Config; const log = std.log.scoped(.embedded_window); @@ -35,6 +36,11 @@ pub const App = struct { /// a full tick of the app loop. wakeup: *const fn (AppUD) callconv(.C) void, + /// Reload the configuration and return the new configuration. + /// The old configuration can be freed immediately when this is + /// called. + reload_config: *const fn (AppUD) callconv(.C) ?*const Config, + /// Called to set the title of the window. set_title: *const fn (SurfaceUD, [*]const u8) callconv(.C) void, @@ -57,16 +63,31 @@ pub const App = struct { }; core_app: *CoreApp, + config: *const Config, opts: Options, - pub fn init(core_app: *CoreApp, opts: Options) !App { - return .{ .core_app = core_app, .opts = opts }; + pub fn init(core_app: *CoreApp, config: *const Config, opts: Options) !App { + return .{ + .core_app = core_app, + .config = config, + .opts = opts, + }; } pub fn terminate(self: App) void { _ = self; } + pub fn reloadConfig(self: *App) !?*const Config { + // Reload + if (self.opts.reload_config(self.opts.userdata)) |new| { + self.config = new; + return self.config; + } + + return null; + } + pub fn wakeup(self: App) void { self.opts.wakeup(self.opts.userdata); } @@ -143,7 +164,7 @@ pub const Surface = struct { // ready to use. try self.core_surface.init( app.core_app.alloc, - app.core_app.config, + app.config, .{ .rt_app = app, .mailbox = &app.core_app.mailbox }, self, ); @@ -338,7 +359,6 @@ pub const Surface = struct { // C API pub const CAPI = struct { const global = &@import("../main.zig").state; - const Config = @import("../config.zig").Config; /// Create a new app. export fn ghostty_app_new( @@ -355,13 +375,13 @@ pub const CAPI = struct { opts: *const apprt.runtime.App.Options, config: *const Config, ) !*App { - var core_app = try CoreApp.create(global.alloc, config); + var core_app = try CoreApp.create(global.alloc); errdefer core_app.destroy(); // Create our runtime app var app = try global.alloc.create(App); errdefer global.alloc.destroy(app); - app.* = try App.init(core_app, opts.*); + app.* = try App.init(core_app, config, opts.*); errdefer app.terminate(); return app; From a5cfd4b04b857bf65fb70513c1a56053aa4e5ffa Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 16 Mar 2023 15:07:44 -0700 Subject: [PATCH 12/19] ghostty.h: add missing reload callback --- include/ghostty.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/ghostty.h b/include/ghostty.h index 1f06ea2f5..00c43b532 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -216,6 +216,7 @@ typedef struct { } ghostty_surface_config_s; typedef void (*ghostty_runtime_wakeup_cb)(void *); +typedef const ghostty_config_t (*ghostty_runtime_reload_config_cb)(void *); typedef void (*ghostty_runtime_set_title_cb)(void *, const char *); typedef const char* (*ghostty_runtime_read_clipboard_cb)(void *); typedef void (*ghostty_runtime_write_clipboard_cb)(void *, const char *); @@ -226,6 +227,7 @@ typedef void (*ghostty_runtime_focus_split_cb)(void *, ghostty_split_focus_direc typedef struct { void *userdata; ghostty_runtime_wakeup_cb wakeup_cb; + ghostty_runtime_reload_config_cb reload_config_cb; ghostty_runtime_set_title_cb set_title_cb; ghostty_runtime_read_clipboard_cb read_clipboard_cb; ghostty_runtime_write_clipboard_cb write_clipboard_cb; From b26e51d2226db0b2123e78b337613457bfee5822 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 16 Mar 2023 15:29:46 -0700 Subject: [PATCH 13/19] macos: implement config reloading callback --- include/ghostty.h | 1 + macos/Sources/Ghostty/AppState.swift | 74 ++++++++++++++++++++-------- src/config.zig | 21 ++++++-- 3 files changed, 70 insertions(+), 26 deletions(-) diff --git a/include/ghostty.h b/include/ghostty.h index 00c43b532..96f7de272 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -243,6 +243,7 @@ int ghostty_init(void); ghostty_config_t ghostty_config_new(); void ghostty_config_free(ghostty_config_t); +void ghostty_config_load_cli_args(ghostty_config_t); void ghostty_config_load_string(ghostty_config_t, const char *, uintptr_t); void ghostty_config_load_default_files(ghostty_config_t); void ghostty_config_load_recursive_files(ghostty_config_t); diff --git a/macos/Sources/Ghostty/AppState.swift b/macos/Sources/Ghostty/AppState.swift index 2307aa082..ab2fa605d 100644 --- a/macos/Sources/Ghostty/AppState.swift +++ b/macos/Sources/Ghostty/AppState.swift @@ -12,12 +12,25 @@ extension Ghostty { /// The readiness value of the state. @Published var readiness: AppReadiness = .loading - /// The ghostty global configuration. - var config: ghostty_config_t? = nil + /// The ghostty global configuration. This should only be changed when it is definitely + /// safe to change. It is definite safe to change only when the embedded app runtime + /// in Ghostty says so (usually, only in a reload configuration callback). + var config: ghostty_config_t? = nil { + didSet { + // Free the old value whenever we change + guard let old = oldValue else { return } + ghostty_config_free(old) + } + } /// The ghostty app instance. We only have one of these for the entire app, although I guess /// in theory you can have multiple... I don't know why you would... - var app: ghostty_app_t? = nil + var app: ghostty_app_t? = nil { + didSet { + guard let old = oldValue else { return } + ghostty_app_free(old) + } + } /// Cached clipboard string for `read_clipboard` callback. private var cached_clipboard_string: String? = nil @@ -31,24 +44,12 @@ extension Ghostty { } // Initialize the global configuration. - guard let cfg = ghostty_config_new() else { - GhosttyApp.logger.critical("ghostty_config_new failed") + guard let cfg = Self.reloadConfig() else { readiness = .error return } self.config = cfg; - // Load our configuration files from the home directory. - ghostty_config_load_default_files(cfg); - ghostty_config_load_recursive_files(cfg); - - // TODO: we'd probably do some config loading here... for now we'd - // have to do this synchronously. When we support config updating we can do - // this async and update later. - - // Finalize will make our defaults available. - ghostty_config_finalize(cfg) - // Create our "runtime" config. The "runtime" is the configuration that ghostty // uses to interface with the application runtime environment. var runtime_cfg = ghostty_runtime_config_s( @@ -75,8 +76,32 @@ extension Ghostty { } deinit { - ghostty_app_free(app) - ghostty_config_free(config) + // This will force the didSet callbacks to run which free. + self.app = nil + self.config = nil + } + + /// Initializes a new configuration and loads all the values. + static func reloadConfig() -> ghostty_config_t? { + // Initialize the global configuration. + guard let cfg = ghostty_config_new() else { + GhosttyApp.logger.critical("ghostty_config_new failed") + return nil + } + + // Load our configuration files from the home directory. + ghostty_config_load_default_files(cfg); + ghostty_config_load_cli_args(cfg); + ghostty_config_load_recursive_files(cfg); + + // TODO: we'd probably do some config loading here... for now we'd + // have to do this synchronously. When we support config updating we can do + // this async and update later. + + // Finalize will make our defaults available. + ghostty_config_finalize(cfg) + + return cfg } func appTick() { @@ -142,10 +167,17 @@ extension Ghostty { } static func reloadConfig(_ userdata: UnsafeMutableRawPointer?) -> ghostty_config_t? { - // TODO: implement config reloading in the mac app + guard let newConfig = AppState.reloadConfig() else { + GhosttyApp.logger.warning("failed to reload configuration") + return nil + } + + // Assign the new config. This will automatically free the old config. + // It is safe to free the old config from within this function call. let state = Unmanaged.fromOpaque(userdata!).takeUnretainedValue() - _ = state - return nil + state.config = newConfig + + return newConfig } static func wakeup(_ userdata: UnsafeMutableRawPointer?) { diff --git a/src/config.zig b/src/config.zig index c30355f55..3f9e6edee 100644 --- a/src/config.zig +++ b/src/config.zig @@ -217,11 +217,7 @@ pub const Config = struct { try result.loadDefaultFiles(alloc_gpa); // Parse the config from the CLI args - { - var iter = try std.process.argsWithAllocator(alloc_gpa); - defer iter.deinit(); - try cli_args.parse(Config, alloc_gpa, &result, &iter); - } + try result.loadCliArgs(alloc_gpa); // Parse the config files that were added from our file and CLI args. try result.loadRecursiveFiles(alloc_gpa); @@ -564,6 +560,14 @@ pub const Config = struct { } } + /// Load and parse the CLI args. + pub fn loadCliArgs(self: *Config, alloc_gpa: Allocator) !void { + // Parse the config from the CLI args + var iter = try std.process.argsWithAllocator(alloc_gpa); + defer iter.deinit(); + try cli_args.parse(Config, alloc_gpa, self, &iter); + } + /// Load and parse the config files that were added in the "config-file" key. pub fn loadRecursiveFiles(self: *Config, alloc: Allocator) !void { // TODO(mitchellh): we should parse the files form the homedir first @@ -1190,6 +1194,13 @@ pub const CAPI = struct { } } + /// Load the configuration from the CLI args. + export fn ghostty_config_load_cli_args(self: *Config) void { + self.loadCliArgs(global.alloc) catch |err| { + log.err("error loading config err={}", .{err}); + }; + } + /// Load the configuration from a string in the same format as /// the file-based syntax for the desktop version of the terminal. export fn ghostty_config_load_string( From f34da17a118a98e6bbfa0e7cf6f32ee883254e58 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 16 Mar 2023 16:03:44 -0700 Subject: [PATCH 14/19] renderer: use a DerivedConfig to avoid the main Config pointer --- src/Surface.zig | 2 +- src/renderer/Metal.zig | 97 +++++++++++++++++++++++-------------- src/renderer/OpenGL.zig | 100 +++++++++++++++++++++++++-------------- src/renderer/Options.zig | 6 +-- 4 files changed, 129 insertions(+), 76 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 6e002091b..1e12fd075 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -322,7 +322,7 @@ pub fn init( // Create our terminal grid with the initial size var renderer_impl = try Renderer.init(alloc, .{ - .config = config, + .config = try Renderer.DerivedConfig.init(alloc, config), .font_group = font_group, .padding = .{ .explicit = padding, diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index 085a268eb..7159ff829 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -11,6 +11,7 @@ const objc = @import("objc"); const macos = @import("macos"); const imgui = @import("imgui"); const apprt = @import("../apprt.zig"); +const configpkg = @import("../config.zig"); const font = @import("../font/main.zig"); const terminal = @import("../terminal/main.zig"); const renderer = @import("../renderer.zig"); @@ -31,6 +32,9 @@ const log = std.log.scoped(.metal); /// Allocator that can be used alloc: std.mem.Allocator, +/// The configuration we need derived from the main config. +config: DerivedConfig, + /// The mailbox for communicating with the window. surface_mailbox: apprt.surface.Mailbox, @@ -51,17 +55,6 @@ focused: bool, /// blinking. cursor_visible: bool, cursor_style: renderer.CursorStyle, -cursor_color: ?terminal.color.RGB, - -/// Default foreground color -foreground: terminal.color.RGB, - -/// Default background color -background: terminal.color.RGB, - -/// Default selection color -selection_background: ?terminal.color.RGB, -selection_foreground: ?terminal.color.RGB, /// The current set of cells to render. This is rebuilt on every frame /// but we keep this around so that we don't reallocate. Each set of @@ -117,6 +110,48 @@ const GPUCellMode = enum(u8) { strikethrough = 8, }; +/// The configuration for this renderer that is derived from the main +/// configuration. This must be exported so that we don't need to +/// pass around Config pointers which makes memory management a pain. +pub const DerivedConfig = struct { + cursor_color: ?terminal.color.RGB, + background: terminal.color.RGB, + foreground: terminal.color.RGB, + selection_background: ?terminal.color.RGB, + selection_foreground: ?terminal.color.RGB, + + pub fn init( + alloc_gpa: Allocator, + config: *const configpkg.Config, + ) !DerivedConfig { + _ = alloc_gpa; + + return .{ + .cursor_color = if (config.@"cursor-color") |col| + col.toTerminalRGB() + else + null, + + .background = config.background.toTerminalRGB(), + .foreground = config.foreground.toTerminalRGB(), + + .selection_background = if (config.@"selection-background") |bg| + bg.toTerminalRGB() + else + null, + + .selection_foreground = if (config.@"selection-foreground") |bg| + bg.toTerminalRGB() + else + null, + }; + } + + pub fn deinit(self: *DerivedConfig) void { + _ = self; + } +}; + /// Returns the hints that we want for this pub fn glfwWindowHints() glfw.Window.Hints { return .{ @@ -233,6 +268,7 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal { return Metal{ .alloc = alloc, + .config = options.config, .surface_mailbox = options.surface_mailbox, .cell_size = .{ .width = metrics.cell_width, .height = metrics.cell_height }, .screen_size = null, @@ -240,17 +276,6 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal { .focused = true, .cursor_visible = true, .cursor_style = .box, - .cursor_color = if (options.config.@"cursor-color") |col| col.toTerminalRGB() else null, - .background = options.config.background.toTerminalRGB(), - .foreground = options.config.foreground.toTerminalRGB(), - .selection_background = if (options.config.@"selection-background") |bg| - bg.toTerminalRGB() - else - null, - .selection_foreground = if (options.config.@"selection-foreground") |bg| - bg.toTerminalRGB() - else - null, // Render state .cells_bg = .{}, @@ -286,6 +311,8 @@ pub fn deinit(self: *Metal) void { self.font_shaper.deinit(); self.alloc.free(self.font_shaper.cell_buf); + self.config.deinit(); + deinitMTLResource(self.buf_cells_bg); deinitMTLResource(self.buf_cells); deinitMTLResource(self.buf_instance); @@ -482,15 +509,15 @@ pub fn render( } // Swap bg/fg if the terminal is reversed - const bg = self.background; - const fg = self.foreground; + const bg = self.config.background; + const fg = self.config.foreground; defer { - self.background = bg; - self.foreground = fg; + self.config.background = bg; + self.config.foreground = fg; } if (state.terminal.modes.reverse_colors) { - self.background = fg; - self.foreground = bg; + self.config.background = fg; + self.config.foreground = bg; } // We used to share terminal state, but we've since learned through @@ -516,7 +543,7 @@ pub fn render( null; break :critical .{ - .bg = self.background, + .bg = self.config.background, .devmode = if (state.devmode) |dm| dm.visible else false, .selection = selection, .screen = screen_copy, @@ -878,8 +905,8 @@ pub fn updateCell( // If we are selected, we our colors are just inverted fg/bg if (sel.contains(screen_point)) { break :colors BgFg{ - .bg = self.selection_background orelse self.foreground, - .fg = self.selection_foreground orelse self.background, + .bg = self.config.selection_background orelse self.config.foreground, + .fg = self.config.selection_foreground orelse self.config.background, }; } } @@ -888,13 +915,13 @@ pub fn updateCell( // In normal mode, background and fg match the cell. We // un-optionalize the fg by defaulting to our fg color. .bg = if (cell.attrs.has_bg) cell.bg else null, - .fg = if (cell.attrs.has_fg) cell.fg else self.foreground, + .fg = if (cell.attrs.has_fg) cell.fg else self.config.foreground, } else .{ // In inverted mode, the background MUST be set to something // (is never null) so it is either the fg or default fg. The // fg is either the bg or default background. - .bg = if (cell.attrs.has_fg) cell.fg else self.foreground, - .fg = if (cell.attrs.has_bg) cell.bg else self.background, + .bg = if (cell.attrs.has_fg) cell.fg else self.config.foreground, + .fg = if (cell.attrs.has_bg) cell.bg else self.config.background, }; break :colors res; }; @@ -988,7 +1015,7 @@ fn addCursor(self: *Metal, screen: *terminal.Screen) void { screen.cursor.x, ); - const color = self.cursor_color orelse terminal.color.RGB{ + const color = self.config.cursor_color orelse terminal.color.RGB{ .r = 0xFF, .g = 0xFF, .b = 0xFF, diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index ccb6686bb..9e2a72303 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -8,6 +8,7 @@ const assert = std.debug.assert; const testing = std.testing; const Allocator = std.mem.Allocator; const apprt = @import("../apprt.zig"); +const configpkg = @import("../config.zig"); const font = @import("../font/main.zig"); const imgui = @import("imgui"); const renderer = @import("../renderer.zig"); @@ -43,6 +44,9 @@ const drawMutexZero = if (DrawMutex == void) void{} else .{}; alloc: std.mem.Allocator, +/// The configuration we need derived from the main config. +config: DerivedConfig, + /// Current cell dimensions for this grid. cell_size: renderer.CellSize, @@ -84,17 +88,6 @@ font_shaper: font.Shaper, /// blinking. cursor_visible: bool, cursor_style: renderer.CursorStyle, -cursor_color: ?terminal.color.RGB, - -/// Default foreground color -foreground: terminal.color.RGB, - -/// Default background color -background: terminal.color.RGB, - -/// Default selection color -selection_background: ?terminal.color.RGB, -selection_foreground: ?terminal.color.RGB, /// True if the window is focused focused: bool, @@ -232,6 +225,48 @@ const GPUCellMode = enum(u8) { } }; +/// The configuration for this renderer that is derived from the main +/// configuration. This must be exported so that we don't need to +/// pass around Config pointers which makes memory management a pain. +pub const DerivedConfig = struct { + cursor_color: ?terminal.color.RGB, + background: terminal.color.RGB, + foreground: terminal.color.RGB, + selection_background: ?terminal.color.RGB, + selection_foreground: ?terminal.color.RGB, + + pub fn init( + alloc_gpa: Allocator, + config: *const configpkg.Config, + ) !DerivedConfig { + _ = alloc_gpa; + + return .{ + .cursor_color = if (config.@"cursor-color") |col| + col.toTerminalRGB() + else + null, + + .background = config.background.toTerminalRGB(), + .foreground = config.foreground.toTerminalRGB(), + + .selection_background = if (config.@"selection-background") |bg| + bg.toTerminalRGB() + else + null, + + .selection_foreground = if (config.@"selection-foreground") |bg| + bg.toTerminalRGB() + else + null, + }; + } + + pub fn deinit(self: *DerivedConfig) void { + _ = self; + } +}; + pub fn init(alloc: Allocator, options: renderer.Options) !OpenGL { // Create the initial font shaper var shape_buf = try alloc.alloc(font.shape.Cell, 1); @@ -354,6 +389,7 @@ pub fn init(alloc: Allocator, options: renderer.Options) !OpenGL { return OpenGL{ .alloc = alloc, + .config = options.config, .cells_bg = .{}, .cells = .{}, .cells_lru = CellsLRU.init(0), @@ -369,18 +405,7 @@ pub fn init(alloc: Allocator, options: renderer.Options) !OpenGL { .font_shaper = shaper, .cursor_visible = true, .cursor_style = .box, - .cursor_color = if (options.config.@"cursor-color") |col| col.toTerminalRGB() else null, - .background = options.config.background.toTerminalRGB(), - .foreground = options.config.foreground.toTerminalRGB(), - .draw_background = options.config.background.toTerminalRGB(), - .selection_background = if (options.config.@"selection-background") |bg| - bg.toTerminalRGB() - else - null, - .selection_foreground = if (options.config.@"selection-foreground") |bg| - bg.toTerminalRGB() - else - null, + .draw_background = options.config.background, .focused = true, .padding = options.padding, .surface_mailbox = options.surface_mailbox, @@ -404,6 +429,9 @@ pub fn deinit(self: *OpenGL) void { self.cells.deinit(self.alloc); self.cells_bg.deinit(self.alloc); + + self.config.deinit(); + self.* = undefined; } @@ -673,15 +701,15 @@ pub fn render( } // Swap bg/fg if the terminal is reversed - const bg = self.background; - const fg = self.foreground; + const bg = self.config.background; + const fg = self.config.foreground; defer { - self.background = bg; - self.foreground = fg; + self.config.background = bg; + self.config.foreground = fg; } if (state.terminal.modes.reverse_colors) { - self.background = fg; - self.foreground = bg; + self.config.background = fg; + self.config.foreground = bg; } // Build our devmode draw data @@ -723,7 +751,7 @@ pub fn render( null; break :critical .{ - .gl_bg = self.background, + .gl_bg = self.config.background, .devmode_data = devmode_data, .active_screen = state.terminal.active_screen, .selection = selection, @@ -944,7 +972,7 @@ fn addCursor(self: *OpenGL, screen: *terminal.Screen) void { screen.cursor.x, ); - const color = self.cursor_color orelse terminal.color.RGB{ + const color = self.config.cursor_color orelse terminal.color.RGB{ .r = 0xFF, .g = 0xFF, .b = 0xFF, @@ -1031,8 +1059,8 @@ pub fn updateCell( // If we are selected, we use the selection colors if (sel.contains(screen_point)) { break :colors BgFg{ - .bg = self.selection_background orelse self.foreground, - .fg = self.selection_foreground orelse self.background, + .bg = self.config.selection_background orelse self.config.foreground, + .fg = self.config.selection_foreground orelse self.config.background, }; } } @@ -1041,13 +1069,13 @@ pub fn updateCell( // In normal mode, background and fg match the cell. We // un-optionalize the fg by defaulting to our fg color. .bg = if (cell.attrs.has_bg) cell.bg else null, - .fg = if (cell.attrs.has_fg) cell.fg else self.foreground, + .fg = if (cell.attrs.has_fg) cell.fg else self.config.foreground, } else .{ // In inverted mode, the background MUST be set to something // (is never null) so it is either the fg or default fg. The // fg is either the bg or default background. - .bg = if (cell.attrs.has_fg) cell.fg else self.foreground, - .fg = if (cell.attrs.has_bg) cell.bg else self.background, + .bg = if (cell.attrs.has_fg) cell.fg else self.config.foreground, + .fg = if (cell.attrs.has_bg) cell.bg else self.config.background, }; break :colors res; }; diff --git a/src/renderer/Options.zig b/src/renderer/Options.zig index 7450dd9f2..e7b647924 100644 --- a/src/renderer/Options.zig +++ b/src/renderer/Options.zig @@ -5,10 +5,8 @@ const font = @import("../font/main.zig"); const renderer = @import("../renderer.zig"); const Config = @import("../config.zig").Config; -/// The app configuration. This must NOT be stored by any termio implementation. -/// The memory it points to is NOT stable after the init call so any values -/// in here must be copied. -config: *const Config, +/// The derived configuration for this renderer implementation. +config: renderer.Renderer.DerivedConfig, /// The font group that should be used. font_group: *font.GroupCache, From 7eda21d5449305b4759c15a2aef25f62904fb6cb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 16 Mar 2023 17:03:57 -0700 Subject: [PATCH 15/19] surface propagates new config to renderer --- src/Surface.zig | 8 +++++++- src/renderer/Metal.zig | 5 +++++ src/renderer/OpenGL.zig | 5 +++++ src/renderer/Thread.zig | 4 ++++ src/renderer/message.zig | 3 +++ 5 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Surface.zig b/src/Surface.zig index 1e12fd075..f6307c696 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -570,7 +570,13 @@ fn changeConfig(self: *Surface, config: *const configpkg.Config) !void { // Update our derived configurations for the renderer and termio, // then send them a message to update. - // TODO + var renderer_config = try Renderer.DerivedConfig.init(self.alloc, config); + errdefer renderer_config.deinit(); + // TODO: termio config + + _ = self.renderer_thread.mailbox.push(.{ + .change_config = renderer_config, + }, .{ .forever = {} }); } /// Returns the x/y coordinate of where the IME (Input Method Editor) diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index 7159ff829..b5ee0a58c 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -724,6 +724,11 @@ fn drawCells( } } +/// Update the configuration. +pub fn changeConfig(self: *Metal, config: DerivedConfig) !void { + self.config = config; +} + /// Resize the screen. pub fn setScreenSize(self: *Metal, dim: renderer.ScreenSize) !void { // Store our screen size diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index 9e2a72303..77950fb99 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -1232,6 +1232,11 @@ fn gridSize(self: *const OpenGL, screen_size: renderer.ScreenSize) renderer.Grid ); } +/// Update the configuration. +pub fn changeConfig(self: *OpenGL, config: DerivedConfig) !void { + self.config = config; +} + /// Set the screen size for rendering. This will update the projection /// used for the shader so that the scaling of the grid is correct. pub fn setScreenSize(self: *OpenGL, dim: renderer.ScreenSize) !void { diff --git a/src/renderer/Thread.zig b/src/renderer/Thread.zig index 099c71cdf..ba956dda0 100644 --- a/src/renderer/Thread.zig +++ b/src/renderer/Thread.zig @@ -255,6 +255,10 @@ fn drainMailbox(self: *Thread) !void { .screen_size => |size| { try self.renderer.setScreenSize(size); }, + + .change_config => |config| { + try self.renderer.changeConfig(config); + }, } } } diff --git a/src/renderer/message.zig b/src/renderer/message.zig index e129d9fe9..80465a2e0 100644 --- a/src/renderer/message.zig +++ b/src/renderer/message.zig @@ -22,4 +22,7 @@ pub const Message = union(enum) { /// Change the screen size. screen_size: renderer.ScreenSize, + + /// The derived configuration to update the renderer with. + change_config: renderer.Renderer.DerivedConfig, }; From 8f0be3ad9efd289f0bc27422fcd2bebd1e7f5bdd Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 19 Mar 2023 10:09:17 -0700 Subject: [PATCH 16/19] termio: use DerivedConfig --- src/Surface.zig | 18 +++++++++++++--- src/termio/Exec.zig | 49 +++++++++++++++++++++++++++++++++++++++--- src/termio/Options.zig | 8 +++++-- src/termio/Thread.zig | 1 + src/termio/message.zig | 4 ++++ 5 files changed, 72 insertions(+), 8 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index f6307c696..cb8e1fe49 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -362,7 +362,8 @@ pub fn init( var io = try termio.Impl.init(alloc, .{ .grid_size = grid_size, .screen_size = screen_size, - .config = config, + .full_config = config, + .config = try termio.Impl.DerivedConfig.init(alloc, config), .renderer_state = &self.renderer_state, .renderer_wakeup = render_thread.wakeup, .renderer_mailbox = render_thread.mailbox, @@ -572,11 +573,22 @@ fn changeConfig(self: *Surface, config: *const configpkg.Config) !void { // then send them a message to update. var renderer_config = try Renderer.DerivedConfig.init(self.alloc, config); errdefer renderer_config.deinit(); - // TODO: termio config - + var termio_config = try termio.Impl.DerivedConfig.init(self.alloc, config); + errdefer termio_config.deinit(); _ = self.renderer_thread.mailbox.push(.{ .change_config = renderer_config, }, .{ .forever = {} }); + _ = self.io_thread.mailbox.push(.{ + .change_config = termio_config, + }, .{ .forever = {} }); + + // With mailbox messages sent, we have to wake them up so they process it. + self.queueRender() catch |err| { + log.warn("failed to notify renderer of config change err={}", .{err}); + }; + self.io_thread.wakeup.notify() catch |err| { + log.warn("failed to notify io thread of config change err={}", .{err}); + }; } /// Returns the x/y coordinate of where the IME (Input Method Editor) diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index c1a903b7f..515a66c25 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -6,6 +6,7 @@ const builtin = @import("builtin"); const build_config = @import("../build_config.zig"); const assert = std.debug.assert; const Allocator = std.mem.Allocator; +const ArenaAllocator = std.heap.ArenaAllocator; const EnvMap = std.process.EnvMap; const termio = @import("../termio.zig"); const Command = @import("../Command.zig"); @@ -19,6 +20,7 @@ const trace = tracy.trace; const apprt = @import("../apprt.zig"); const fastmem = @import("../fastmem.zig"); const internal_os = @import("../os/main.zig"); +const configpkg = @import("../config.zig"); const log = std.log.scoped(.io_exec); @@ -62,9 +64,35 @@ grid_size: renderer.GridSize, /// The data associated with the currently running thread. data: ?*EventData, +/// The configuration for this IO that is derived from the main +/// configuration. This must be exported so that we don't need to +/// pass around Config pointers which makes memory management a pain. +pub const DerivedConfig = struct { + palette: terminal.color.Palette, + + pub fn init( + alloc_gpa: Allocator, + config: *const configpkg.Config, + ) !DerivedConfig { + _ = alloc_gpa; + + return .{ + .palette = config.palette.value, + }; + } + + pub fn deinit(self: *DerivedConfig) void { + _ = self; + } +}; + /// Initialize the exec implementation. This will also start the child /// process. pub fn init(alloc: Allocator, opts: termio.Options) !Exec { + // Clean up our derived config because we don't need it after this. + var config = opts.config; + defer config.deinit(); + // Create our terminal var term = try terminal.Terminal.init( alloc, @@ -72,7 +100,7 @@ pub fn init(alloc: Allocator, opts: termio.Options) !Exec { opts.grid_size.rows, ); errdefer term.deinit(alloc); - term.color_palette = opts.config.palette.value; + term.color_palette = opts.config.palette; var subprocess = try Subprocess.init(alloc, opts); errdefer subprocess.deinit(); @@ -189,6 +217,14 @@ pub fn threadExit(self: *Exec, data: ThreadData) void { data.read_thread.join(); } +/// Update the configuration. +pub fn changeConfig(self: *Exec, config: DerivedConfig) !void { + var copy = config; + defer copy.deinit(); + + self.terminal.color_palette = config.palette; +} + /// Resize the terminal. pub fn resize( self: *Exec, @@ -413,7 +449,7 @@ const Subprocess = struct { const alloc = arena.allocator(); // Determine the path to the binary we're executing - const path = (try Command.expandPath(alloc, opts.config.command orelse "sh")) orelse + const path = (try Command.expandPath(alloc, opts.full_config.command orelse "sh")) orelse return error.CommandNotFound; // On macOS, we launch the program as a login shell. This is a Mac-specific @@ -487,10 +523,17 @@ const Subprocess = struct { break :args try args.toOwnedSlice(); }; + // We have to copy the cwd because there is no guarantee that + // pointers in full_config remain valid. + var cwd: ?[]u8 = if (opts.full_config.@"working-directory") |cwd| + try alloc.dupe(u8, cwd) + else + null; + return .{ .arena = arena, .env = env, - .cwd = opts.config.@"working-directory", + .cwd = cwd, .path = if (internal_os.isFlatpak()) args[0] else path, .args = args, .grid_size = opts.grid_size, diff --git a/src/termio/Options.zig b/src/termio/Options.zig index 28a10f1f4..5e88b1010 100644 --- a/src/termio/Options.zig +++ b/src/termio/Options.zig @@ -4,6 +4,7 @@ const xev = @import("xev"); const apprt = @import("../apprt.zig"); const renderer = @import("../renderer.zig"); const Config = @import("../config.zig").Config; +const termio = @import("../termio.zig"); /// The size of the terminal grid. grid_size: renderer.GridSize, @@ -11,10 +12,13 @@ grid_size: renderer.GridSize, /// The size of the viewport in pixels. screen_size: renderer.ScreenSize, -/// The app configuration. This must NOT be stored by any termio implementation. +/// The full app configuration. This is only available during initialization. /// The memory it points to is NOT stable after the init call so any values /// in here must be copied. -config: *const Config, +full_config: *const Config, + +/// The derived configuration for this termio implementation. +config: termio.Impl.DerivedConfig, /// The render state. The IO implementation can modify anything here. The /// surface thread will setup the initial "terminal" pointer but the IO impl diff --git a/src/termio/Thread.zig b/src/termio/Thread.zig index b1c03cc6e..72656f00e 100644 --- a/src/termio/Thread.zig +++ b/src/termio/Thread.zig @@ -150,6 +150,7 @@ fn drainMailbox(self: *Thread) !void { log.debug("mailbox message={}", .{message}); switch (message) { + .change_config => |config| try self.impl.changeConfig(config), .resize => |v| self.handleResize(v), .clear_screen => |v| try self.impl.clearScreen(v.history), .write_small => |v| try self.impl.queueWrite(v.data[0..v.len]), diff --git a/src/termio/message.zig b/src/termio/message.zig index d1a75bc01..5399b3d9d 100644 --- a/src/termio/message.zig +++ b/src/termio/message.zig @@ -3,6 +3,7 @@ const assert = std.debug.assert; const Allocator = std.mem.Allocator; const renderer = @import("../renderer.zig"); const terminal = @import("../terminal/main.zig"); +const termio = @import("../termio.zig"); /// The messages that can be sent to an IO thread. /// @@ -28,6 +29,9 @@ pub const Message = union(enum) { padding: renderer.Padding, }; + /// The derived configuration to update the implementation with. + change_config: termio.Impl.DerivedConfig, + /// Resize the window. resize: Resize, From 6f4e913182dec4aedbb05c72e0cb84875dc6561a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 19 Mar 2023 10:11:32 -0700 Subject: [PATCH 17/19] termio/exec: comment about what we're updating --- src/termio/Exec.zig | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 515a66c25..cb9b157e4 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -222,6 +222,14 @@ pub fn changeConfig(self: *Exec, config: DerivedConfig) !void { var copy = config; defer copy.deinit(); + // Update the configuration that we know about. + // + // Specific things we don't update: + // - command, working-directory: we never restart the underlying + // process so we don't care or need to know about these. + + // Update the palette. Note this will only apply to new colors drawn + // since we decode all palette colors to RGB on usage. self.terminal.color_palette = config.palette; } From e84fb55e2cc7db59d3db9d197567f4955ad18091 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 19 Mar 2023 10:14:28 -0700 Subject: [PATCH 18/19] surface mouse_interval is dynamically update-able --- src/Surface.zig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index cb8e1fe49..7a4710645 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -71,7 +71,6 @@ renderer_thr: std.Thread, /// Mouse state. mouse: Mouse, -mouse_interval: u64, /// The terminal IO handler. io: termio.Impl, @@ -137,6 +136,7 @@ const DerivedConfig = struct { clipboard_read: bool, clipboard_write: bool, clipboard_trim_trailing_spaces: bool, + mouse_interval: u64, pub fn init(alloc_gpa: Allocator, config: *const configpkg.Config) !DerivedConfig { var arena = ArenaAllocator.init(alloc_gpa); @@ -149,6 +149,7 @@ const DerivedConfig = struct { .clipboard_read = config.@"clipboard-read", .clipboard_write = config.@"clipboard-write", .clipboard_trim_trailing_spaces = config.@"clipboard-trim-trailing-spaces", + .mouse_interval = config.@"click-repeat-interval" * 1_000_000, // 500ms // Assignments happen sequentially so we have to do this last // so that the memory is captured from allocs above. @@ -400,7 +401,6 @@ pub fn init( }, .renderer_thr = undefined, .mouse = .{}, - .mouse_interval = config.@"click-repeat-interval" * 1_000_000, // 500ms .io = io, .io_thread = io_thread, .io_thr = undefined, @@ -1506,7 +1506,7 @@ pub fn mouseButtonCallback( // is less than and our interval and if so, increase the count. if (self.mouse.left_click_count > 0) { const since = now.since(self.mouse.left_click_time); - if (since > self.mouse_interval) { + if (since > self.config.mouse_interval) { self.mouse.left_click_count = 0; } } From b0b3b0af2d7b73c9c0cb0e1d54b125360495892f Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 19 Mar 2023 10:48:42 -0700 Subject: [PATCH 19/19] update config messages use pointers now to make messages small again --- src/Surface.zig | 25 +++++++++++++++++++------ src/renderer/Metal.zig | 4 ++-- src/renderer/OpenGL.zig | 4 ++-- src/renderer/Thread.zig | 3 ++- src/renderer/message.zig | 5 ++++- src/termio/Exec.zig | 5 ++--- src/termio/Thread.zig | 5 ++++- src/termio/message.zig | 8 ++++++-- 8 files changed, 41 insertions(+), 18 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 7a4710645..d19f61d11 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -569,17 +569,30 @@ fn changeConfig(self: *Surface, config: *const configpkg.Config) !void { self.config.deinit(); self.config = derived; + // We need to store our configs in a heap-allocated pointer so that + // our messages aren't huge. + var renderer_config_ptr = try self.alloc.create(Renderer.DerivedConfig); + errdefer self.alloc.destroy(renderer_config_ptr); + var termio_config_ptr = try self.alloc.create(termio.Impl.DerivedConfig); + errdefer self.alloc.destroy(termio_config_ptr); + // Update our derived configurations for the renderer and termio, // then send them a message to update. - var renderer_config = try Renderer.DerivedConfig.init(self.alloc, config); - errdefer renderer_config.deinit(); - var termio_config = try termio.Impl.DerivedConfig.init(self.alloc, config); - errdefer termio_config.deinit(); + renderer_config_ptr.* = try Renderer.DerivedConfig.init(self.alloc, config); + errdefer renderer_config_ptr.deinit(); + termio_config_ptr.* = try termio.Impl.DerivedConfig.init(self.alloc, config); + errdefer termio_config_ptr.deinit(); _ = self.renderer_thread.mailbox.push(.{ - .change_config = renderer_config, + .change_config = .{ + .alloc = self.alloc, + .ptr = renderer_config_ptr, + }, }, .{ .forever = {} }); _ = self.io_thread.mailbox.push(.{ - .change_config = termio_config, + .change_config = .{ + .alloc = self.alloc, + .ptr = termio_config_ptr, + }, }, .{ .forever = {} }); // With mailbox messages sent, we have to wake them up so they process it. diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index b5ee0a58c..ff55c1e59 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -725,8 +725,8 @@ fn drawCells( } /// Update the configuration. -pub fn changeConfig(self: *Metal, config: DerivedConfig) !void { - self.config = config; +pub fn changeConfig(self: *Metal, config: *DerivedConfig) !void { + self.config = config.*; } /// Resize the screen. diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index 77950fb99..e44b317e2 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -1233,8 +1233,8 @@ fn gridSize(self: *const OpenGL, screen_size: renderer.ScreenSize) renderer.Grid } /// Update the configuration. -pub fn changeConfig(self: *OpenGL, config: DerivedConfig) !void { - self.config = config; +pub fn changeConfig(self: *OpenGL, config: *DerivedConfig) !void { + self.config = config.*; } /// Set the screen size for rendering. This will update the projection diff --git a/src/renderer/Thread.zig b/src/renderer/Thread.zig index ba956dda0..a4eac3b1a 100644 --- a/src/renderer/Thread.zig +++ b/src/renderer/Thread.zig @@ -257,7 +257,8 @@ fn drainMailbox(self: *Thread) !void { }, .change_config => |config| { - try self.renderer.changeConfig(config); + defer config.alloc.destroy(config.ptr); + try self.renderer.changeConfig(config.ptr); }, } } diff --git a/src/renderer/message.zig b/src/renderer/message.zig index 80465a2e0..7441380da 100644 --- a/src/renderer/message.zig +++ b/src/renderer/message.zig @@ -24,5 +24,8 @@ pub const Message = union(enum) { screen_size: renderer.ScreenSize, /// The derived configuration to update the renderer with. - change_config: renderer.Renderer.DerivedConfig, + change_config: struct { + alloc: Allocator, + ptr: *renderer.Renderer.DerivedConfig, + }, }; diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index cb9b157e4..bc8582e77 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -218,9 +218,8 @@ pub fn threadExit(self: *Exec, data: ThreadData) void { } /// Update the configuration. -pub fn changeConfig(self: *Exec, config: DerivedConfig) !void { - var copy = config; - defer copy.deinit(); +pub fn changeConfig(self: *Exec, config: *DerivedConfig) !void { + defer config.deinit(); // Update the configuration that we know about. // diff --git a/src/termio/Thread.zig b/src/termio/Thread.zig index 72656f00e..d034ede3d 100644 --- a/src/termio/Thread.zig +++ b/src/termio/Thread.zig @@ -150,7 +150,10 @@ fn drainMailbox(self: *Thread) !void { log.debug("mailbox message={}", .{message}); switch (message) { - .change_config => |config| try self.impl.changeConfig(config), + .change_config => |config| { + defer config.alloc.destroy(config.ptr); + try self.impl.changeConfig(config.ptr); + }, .resize => |v| self.handleResize(v), .clear_screen => |v| try self.impl.clearScreen(v.history), .write_small => |v| try self.impl.queueWrite(v.data[0..v.len]), diff --git a/src/termio/message.zig b/src/termio/message.zig index 5399b3d9d..fe4fc535d 100644 --- a/src/termio/message.zig +++ b/src/termio/message.zig @@ -29,8 +29,12 @@ pub const Message = union(enum) { padding: renderer.Padding, }; - /// The derived configuration to update the implementation with. - change_config: termio.Impl.DerivedConfig, + /// The derived configuration to update the implementation with. This + /// is allocated via the allocator and is expected to be freed when done. + change_config: struct { + alloc: Allocator, + ptr: *termio.Impl.DerivedConfig, + }, /// Resize the window. resize: Resize,