From 83d1bdcfcba9c32d7f56daed530b1faa3f1068e3 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 14 Aug 2025 10:01:00 -0700 Subject: [PATCH] apprt/gtk-ng: clean up close handling of all types This cleans up our close handling of all types (surfaces, tabs, windows). Surfaces no longer emit their scope; their scope is always just the surface itself. For tab and window scope we use widget actions. This makes `close_tab` work properly (previously broken). --- src/apprt/gtk-ng/Surface.zig | 3 ++- src/apprt/gtk-ng/class/application.zig | 26 +++++++++++++++++-------- src/apprt/gtk-ng/class/split_tree.zig | 26 +++++++++++++++---------- src/apprt/gtk-ng/class/surface.zig | 27 ++++---------------------- src/apprt/gtk-ng/class/tab.zig | 22 ++++++++++++++++++--- src/apprt/gtk-ng/class/window.zig | 26 ------------------------- 6 files changed, 59 insertions(+), 71 deletions(-) diff --git a/src/apprt/gtk-ng/Surface.zig b/src/apprt/gtk-ng/Surface.zig index 1614d74d0..ac82f941b 100644 --- a/src/apprt/gtk-ng/Surface.zig +++ b/src/apprt/gtk-ng/Surface.zig @@ -32,7 +32,8 @@ pub fn rtApp(self: *Self) *ApprtApp { } pub fn close(self: *Self, process_active: bool) void { - self.surface.close(.{ .surface = process_active }); + _ = process_active; + self.surface.close(); } pub fn cgroup(self: *Self) ?[]const u8 { diff --git a/src/apprt/gtk-ng/class/application.zig b/src/apprt/gtk-ng/class/application.zig index 1fa61f791..511c1aa85 100644 --- a/src/apprt/gtk-ng/class/application.zig +++ b/src/apprt/gtk-ng/class/application.zig @@ -542,8 +542,8 @@ pub const Application = extern struct { value: apprt.Action.Value(action), ) !bool { switch (action) { - .close_tab => Action.close(target, .tab), - .close_window => Action.close(target, .window), + .close_tab => return Action.closeTab(target), + .close_window => return Action.closeWindow(target), .config_change => try Action.configChange( self, @@ -1582,13 +1582,23 @@ pub const Application = extern struct { /// All apprt action handlers const Action = struct { - pub fn close( - target: apprt.Target, - scope: Surface.CloseScope, - ) void { + pub fn closeTab(target: apprt.Target) bool { switch (target) { - .app => {}, - .surface => |v| v.rt_surface.surface.close(scope), + .app => return false, + .surface => |core| { + const surface = core.rt_surface.surface; + return surface.as(gtk.Widget).activateAction("tab.close", null) != 0; + }, + } + } + + pub fn closeWindow(target: apprt.Target) bool { + switch (target) { + .app => return false, + .surface => |core| { + const surface = core.rt_surface.surface; + return surface.as(gtk.Widget).activateAction("win.close", null) != 0; + }, } } diff --git a/src/apprt/gtk-ng/class/split_tree.zig b/src/apprt/gtk-ng/class/split_tree.zig index a5e823158..3e06bf983 100644 --- a/src/apprt/gtk-ng/class/split_tree.zig +++ b/src/apprt/gtk-ng/class/split_tree.zig @@ -407,6 +407,22 @@ pub const SplitTree = extern struct { //--------------------------------------------------------------- // Properties + /// Returns true if this split tree needs confirmation before quitting based + /// on the various Ghostty configurations. + pub fn getNeedsConfirmQuit(self: *Self) bool { + const tree = self.getTree() orelse return false; + var it = tree.iterator(); + while (it.next()) |entry| { + if (entry.view.core()) |core| { + if (core.needsConfirmQuit()) { + return true; + } + } + } + + return false; + } + /// Get the currently active surface. See the "active-surface" property. /// This does not ref the value. pub fn getActiveSurface(self: *Self) ?*Surface { @@ -636,18 +652,8 @@ pub const SplitTree = extern struct { fn surfaceCloseRequest( surface: *Surface, - scope: *const Surface.CloseScope, self: *Self, ) callconv(.c) void { - switch (scope.*) { - // Handled upstream... this will probably go away for widget - // actions eventually. - .window, .tab => return, - - // Remove the surface from the tree. - .surface => {}, - } - const core = surface.core() orelse return; // Reset our pending close state diff --git a/src/apprt/gtk-ng/class/surface.zig b/src/apprt/gtk-ng/class/surface.zig index 4b2d972bb..b8db91a2b 100644 --- a/src/apprt/gtk-ng/class/surface.zig +++ b/src/apprt/gtk-ng/class/surface.zig @@ -275,7 +275,7 @@ pub const Surface = extern struct { const impl = gobject.ext.defineSignal( name, Self, - &.{*const CloseScope}, + &.{}, void, ); }; @@ -1047,11 +1047,11 @@ pub const Surface = extern struct { //--------------------------------------------------------------- // Libghostty Callbacks - pub fn close(self: *Self, scope: CloseScope) void { + pub fn close(self: *Self) void { signals.@"close-request".impl.emit( self, null, - .{&scope}, + .{}, null, ); } @@ -1749,7 +1749,7 @@ pub const Surface = extern struct { self: *Self, ) callconv(.c) void { // This closes the surface with no confirmation. - self.close(.{ .surface = false }); + self.close(); } fn contextMenuClosed( @@ -2709,25 +2709,6 @@ pub const Surface = extern struct { pub const bindTemplateCallback = C.Class.bindTemplateCallback; }; - /// The scope of a close request. - pub const CloseScope = union(enum) { - /// Close the surface. The boolean determines if there is a - /// process active. - surface: bool, - - /// Close the tab. We can't know if there are processes active - /// for the entire tab scope so listeners must query the app. - tab, - - /// Close the window. - window, - - pub const getGObjectType = gobject.ext.defineBoxed( - CloseScope, - .{ .name = "GhosttySurfaceCloseScope" }, - ); - }; - /// Simple dimensions struct for the surface used by various properties. pub const Size = extern struct { width: u32, diff --git a/src/apprt/gtk-ng/class/tab.zig b/src/apprt/gtk-ng/class/tab.zig index 520050cb6..b24123fd8 100644 --- a/src/apprt/gtk-ng/class/tab.zig +++ b/src/apprt/gtk-ng/class/tab.zig @@ -208,6 +208,7 @@ pub const Tab = extern struct { // For action names: // https://docs.gtk.org/gio/type_func.Action.name_is_valid.html const actions = .{ + .{ "close", actionClose, null }, .{ "ring-bell", actionRingBell, null }, }; @@ -262,9 +263,8 @@ pub const Tab = extern struct { /// Returns true if this tab needs confirmation before quitting based /// on the various Ghostty configurations. pub fn getNeedsConfirmQuit(self: *Self) bool { - const surface = self.getActiveSurface() orelse return false; - const core_surface = surface.core() orelse return false; - return core_surface.needsConfirmQuit(); + const tree = self.getSplitTree(); + return tree.getNeedsConfirmQuit(); } /// Get the tab page holding this tab, if any. @@ -344,6 +344,22 @@ pub const Tab = extern struct { self.as(gobject.Object).notifyByPspec(properties.@"active-surface".impl.param_spec); } + fn actionClose( + _: *gio.SimpleAction, + _: ?*glib.Variant, + self: *Self, + ) callconv(.c) void { + const tab_view = ext.getAncestor( + adw.TabView, + self.as(gtk.Widget), + ) orelse return; + const page = tab_view.getPage(self.as(gtk.Widget)); + + // Delegate to our parent to handle this, since this will emit + // a close-page signal that the parent can intercept. + tab_view.closePage(page); + } + fn actionRingBell( _: *gio.SimpleAction, _: ?*glib.Variant, diff --git a/src/apprt/gtk-ng/class/window.zig b/src/apprt/gtk-ng/class/window.zig index 64aff1f9a..07801089e 100644 --- a/src/apprt/gtk-ng/class/window.zig +++ b/src/apprt/gtk-ng/class/window.zig @@ -683,13 +683,6 @@ pub const Window = extern struct { var it = tree.iterator(); while (it.next()) |entry| { const surface = entry.view; - _ = Surface.signals.@"close-request".connect( - surface, - *Self, - surfaceCloseRequest, - self, - .{}, - ); _ = Surface.signals.@"present-request".connect( surface, *Self, @@ -1458,25 +1451,6 @@ pub const Window = extern struct { self.addToast(i18n._("Cleared clipboard")); } - fn surfaceCloseRequest( - _: *Surface, - scope: *const Surface.CloseScope, - self: *Self, - ) callconv(.c) void { - switch (scope.*) { - // Handled directly by the tab. If the surface is the last - // surface then the tab will emit its own signal to request - // closing itself. - .surface => return, - - // Also handled directly by the tab. - .tab => return, - - // The only one we care about! - .window => self.as(gtk.Window).close(), - } - } - fn surfaceMenu( _: *Surface, self: *Self,