From 326e55c8f8bd5bdf3d5883ecf19479668c65966a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 7 Aug 2025 08:35:18 -0700 Subject: [PATCH] apprt/gtk-ng: PR feedback --- src/apprt/gtk-ng/class/split_tree.zig | 38 ++++++++++++-------------- src/apprt/gtk-ng/class/tab.zig | 11 ++++---- src/apprt/gtk-ng/class/window.zig | 16 ++++++----- src/apprt/gtk-ng/ui/1.5/split-tree.blp | 37 ++++++++++++------------- src/apprt/gtk-ng/ui/1.5/tab.blp | 2 +- src/datastruct/split_tree.zig | 10 +++---- 6 files changed, 55 insertions(+), 59 deletions(-) diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index 7e3f7d92b..750ba670e 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -26,7 +26,7 @@ const log = std.log.scoped(.gtk_ghostty_split_tree); pub const SplitTree = extern struct { const Self = @This(); parent_instance: Parent, - pub const Parent = adw.Bin; + pub const Parent = gtk.Box; pub const getGObjectType = gobject.ext.defineClass(Self, .{ .name = "GhosttySplitTree", .instanceInit = &init, @@ -36,21 +36,21 @@ pub const SplitTree = extern struct { }); pub const properties = struct { - pub const @"is-empty" = struct { - pub const name = "is-empty"; + pub const @"has-surfaces" = struct { + pub const name = "has-surfaces"; const impl = gobject.ext.defineProperty( name, Self, bool, .{ - .nick = "Tree Is Empty", - .blurb = "True when the tree has no surfaces.", + .nick = "Has Surfaces", + .blurb = "Tree has surfaces.", .default = false, .accessor = gobject.ext.typedAccessor( Self, bool, .{ - .getter = getIsEmpty, + .getter = getHasSurfaces, }, ), }, @@ -76,17 +76,15 @@ pub const SplitTree = extern struct { }; pub const signals = struct { - /// Emitted whenever the tree property is about to change. - /// - /// The new value is given as the signal parameter. The old value - /// can still be retrieved from the tree property. - pub const @"tree-will-change" = struct { - pub const name = "tree-will-change"; + /// Emitted whenever the tree property has changed, with access + /// to the previous and new values. + pub const changed = struct { + pub const name = "changed"; pub const connect = impl.connect; const impl = gobject.ext.defineSignal( name, Self, - &.{?*const Surface.Tree}, + &.{ ?*const Surface.Tree, ?*const Surface.Tree }, void, ); }; @@ -109,9 +107,9 @@ pub const SplitTree = extern struct { //--------------------------------------------------------------- // Properties - pub fn getIsEmpty(self: *Self) bool { + pub fn getHasSurfaces(self: *Self) bool { const tree: *const Surface.Tree = self.private().tree orelse &.empty; - return tree.isEmpty(); + return !tree.isEmpty(); } /// Get the tree data model that we're showing in this widget. This @@ -127,10 +125,10 @@ pub const SplitTree = extern struct { // Emit the signal so that handlers can witness both the before and // after values of the tree. - signals.@"tree-will-change".impl.emit( + signals.changed.impl.emit( self, null, - .{tree}, + .{ priv.tree, tree }, null, ); @@ -206,7 +204,7 @@ pub const SplitTree = extern struct { } // Dependent properties - self.as(gobject.Object).notifyByPspec(properties.@"is-empty".impl.param_spec); + self.as(gobject.Object).notifyByPspec(properties.@"has-surfaces".impl.param_spec); } /// Builds the widget tree associated with a surface split tree. @@ -265,7 +263,7 @@ pub const SplitTree = extern struct { // Properties gobject.ext.registerProperties(class, &.{ - properties.@"is-empty".impl, + properties.@"has-surfaces".impl, properties.tree.impl, }); @@ -276,7 +274,7 @@ pub const SplitTree = extern struct { class.bindTemplateCallback("notify_tree", &propTree); // Signals - signals.@"tree-will-change".impl.register(.{}); + signals.changed.impl.register(.{}); // Virtual methods gobject.Object.virtual_methods.dispose.implement(class, &dispose); diff --git a/src/apprt/gtk-ng/class/tab.zig b/src/apprt/gtk-ng/class/tab.zig index c60b01bfa..5de4839ec 100644 --- a/src/apprt/gtk-ng/class/tab.zig +++ b/src/apprt/gtk-ng/class/tab.zig @@ -328,13 +328,14 @@ pub const Tab = extern struct { } } - fn splitTreeWillChange( - split_tree: *SplitTree, + fn splitTreeChanged( + _: *SplitTree, + old_tree: ?*const Surface.Tree, new_tree: ?*const Surface.Tree, self: *Self, ) callconv(.c) void { - if (split_tree.getTree()) |old_tree| { - self.disconnectSurfaceHandlers(old_tree); + if (old_tree) |tree| { + self.disconnectSurfaceHandlers(tree); } if (new_tree) |tree| { @@ -406,7 +407,7 @@ pub const Tab = extern struct { class.bindTemplateChildPrivate("split_tree", .{}); // Template Callbacks - class.bindTemplateCallback("tree_will_change", &splitTreeWillChange); + class.bindTemplateCallback("tree_changed", &splitTreeChanged); class.bindTemplateCallback("notify_active_surface", &propActiveSurface); class.bindTemplateCallback("notify_tree", &propSplitTree); diff --git a/src/apprt/gtk-ng/class/window.zig b/src/apprt/gtk-ng/class/window.zig index 3d7143953..bffa43bb1 100644 --- a/src/apprt/gtk-ng/class/window.zig +++ b/src/apprt/gtk-ng/class/window.zig @@ -411,18 +411,19 @@ pub const Window = extern struct { // Bind signals const split_tree = tab.getSplitTree(); - _ = SplitTree.signals.@"tree-will-change".connect( + _ = SplitTree.signals.changed.connect( split_tree, *Self, - tabSplitTreeWillChange, + tabSplitTreeChanged, self, .{}, ); // Run an initial notification for the surface tree so we can setup // initial state. - tabSplitTreeWillChange( + tabSplitTreeChanged( split_tree, + null, split_tree.getTree(), self, ); @@ -1507,13 +1508,14 @@ pub const Window = extern struct { } } - fn tabSplitTreeWillChange( - split_tree: *SplitTree, + fn tabSplitTreeChanged( + _: *SplitTree, + old_tree: ?*const Surface.Tree, new_tree: ?*const Surface.Tree, self: *Self, ) callconv(.c) void { - if (split_tree.getTree()) |old_tree| { - self.disconnectSurfaceHandlers(old_tree); + if (old_tree) |tree| { + self.disconnectSurfaceHandlers(tree); } if (new_tree) |tree| { diff --git a/src/apprt/gtk-ng/ui/1.5/split-tree.blp b/src/apprt/gtk-ng/ui/1.5/split-tree.blp index 2ce4b3f10..e8c53b607 100644 --- a/src/apprt/gtk-ng/ui/1.5/split-tree.blp +++ b/src/apprt/gtk-ng/ui/1.5/split-tree.blp @@ -1,28 +1,25 @@ using Gtk 4.0; using Adw 1; -template $GhosttySplitTree: Adw.Bin { +template $GhosttySplitTree: Box { notify::tree => $notify_tree(); + orientation: vertical; - Box { - orientation: vertical; + Adw.Bin tree_bin { + visible: bind template.has-surfaces; + hexpand: true; + vexpand: true; + } - Adw.Bin tree_bin { - visible: bind template.is-empty inverted; - hexpand: true; - vexpand: true; - } - - // This could be a lot more visually pleasing but in practice this doesn't - // ever happen at the time of writing this comment. A surface-less split - // tree always closes its parent. - Label { - visible: bind template.is-empty; - // Purposely not localized currently because this shouldn't really - // ever appear. When we have a situation it does appear, we may want - // to change the styling and text so I don't want to burden localizers - // to handle this yet. - label: "No surfaces."; - } + // This could be a lot more visually pleasing but in practice this doesn't + // ever happen at the time of writing this comment. A surface-less split + // tree always closes its parent. + Label { + visible: bind template.has-surfaces inverted; + // Purposely not localized currently because this shouldn't really + // ever appear. When we have a situation it does appear, we may want + // to change the styling and text so I don't want to burden localizers + // to handle this yet. + label: "No surfaces."; } } diff --git a/src/apprt/gtk-ng/ui/1.5/tab.blp b/src/apprt/gtk-ng/ui/1.5/tab.blp index f0d7f5f68..61f106ce1 100644 --- a/src/apprt/gtk-ng/ui/1.5/tab.blp +++ b/src/apprt/gtk-ng/ui/1.5/tab.blp @@ -12,6 +12,6 @@ template $GhosttyTab: Box { $GhosttySplitTree split_tree { notify::tree => $notify_tree(); - tree-will-change => $tree_will_change(); + changed => $tree_changed(); } } diff --git a/src/datastruct/split_tree.zig b/src/datastruct/split_tree.zig index 23e9eae0c..68a7c09e7 100644 --- a/src/datastruct/split_tree.zig +++ b/src/datastruct/split_tree.zig @@ -728,22 +728,20 @@ pub fn SplitTree(comptime V: type) type { }, .funcs = .{ - // The @ptrCast below is to workaround this bug: - // https://github.com/ianprime0509/zig-gobject/issues/115 - .copy = @ptrCast(&struct { + .copy = &struct { fn copy(self: *Self) callconv(.c) *Self { const ptr = @import("glib").ext.create(Self); const alloc = self.arena.child_allocator; ptr.* = self.clone(alloc) catch @panic("oom"); return ptr; } - }.copy), - .free = @ptrCast(&struct { + }.copy, + .free = &struct { fn free(self: *Self) callconv(.c) void { self.deinit(); @import("glib").ext.destroy(self); } - }.free), + }.free, }, }, ),