gtk: add separate close_window apprt action

For *some* reason we have a binding for close_window but it merely closes the surface and not the entire window. That is not only misleading but also just wrong. Now we make a separate apprt action for close_window that would make it show a close confirmation prompt identical to as if the user had clicked the (X) button on the window titlebar.
ghostty-org · Feb 13, 2025 · 69fbec1 · 69fbec1
1 parent f375e90
commit 69fbec1
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 15 deletions.
diff --git a/include/ghostty.h b/include/ghostty.h
@@ -597,6 +597,7 @@ typedef enum {
   GHOSTTY_ACTION_COLOR_CHANGE,
   GHOSTTY_ACTION_RELOAD_CONFIG,
   GHOSTTY_ACTION_CONFIG_CHANGE,
+  GHOSTTY_ACTION_CLOSE_WINDOW,
 } ghostty_action_tag_e;
 
 typedef union {

diff --git a/src/Surface.zig b/src/Surface.zig
@@ -4242,7 +4242,11 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !bool
 
         .close_surface => self.close(),
 
-        .close_window => self.app.closeSurface(self),
+        .close_window => return try self.rt_app.performAction(
+            .{ .surface = self },
+            .close_window,
+            {},
+        ),
 
         .crash => |location| switch (location) {
             .main => @panic("crash binding action, crashing intentionally"),

diff --git a/src/apprt/action.zig b/src/apprt/action.zig
@@ -226,6 +226,9 @@ pub const Action = union(Key) {
     /// for changes.
     config_change: ConfigChange,
 
+    /// Closes the currently focused window.
+    close_window,
+
     /// Sync with: ghostty_action_tag_e
     pub const Key = enum(c_int) {
         quit,
@@ -266,6 +269,7 @@ pub const Action = union(Key) {
         color_change,
         reload_config,
         config_change,
+        close_window,
     };
 
     /// Sync with: ghostty_action_u

diff --git a/src/apprt/glfw.zig b/src/apprt/glfw.zig
@@ -219,6 +219,7 @@ pub const App = struct {
             .toggle_split_zoom,
             .present_terminal,
             .close_all_windows,
+            .close_window,
             .close_tab,
             .toggle_tab_overview,
             .toggle_window_decorations,

diff --git a/src/apprt/gtk/App.zig b/src/apprt/gtk/App.zig
@@ -522,11 +522,12 @@ pub fn performAction(
             .app => null,
             .surface => |v| v,
         }),
+        .close_window => return try self.closeWindow(target),
         .toggle_maximize => self.toggleMaximize(target),
         .toggle_fullscreen => self.toggleFullscreen(target, value),
 
         .new_tab => try self.newTab(target),
-        .close_tab => try self.closeTab(target),
+        .close_tab => return try self.closeTab(target),
         .goto_tab => return self.gotoTab(target, value),
         .move_tab => self.moveTab(target, value),
         .new_split => try self.newSplit(target, value),
@@ -589,19 +590,20 @@ fn newTab(_: *App, target: apprt.Target) !void {
     }
 }
 
-fn closeTab(_: *App, target: apprt.Target) !void {
+fn closeTab(_: *App, target: apprt.Target) !bool {
     switch (target) {
-        .app => {},
+        .app => return false,
         .surface => |v| {
             const tab = v.rt_surface.container.tab() orelse {
                 log.info(
                     "close_tab invalid for container={s}",
                     .{@tagName(v.rt_surface.container)},
                 );
-                return;
+                return false;
             };
 
             tab.closeWithConfirmation();
+            return true;
         },
     }
 }
@@ -1060,7 +1062,7 @@ fn syncActionAccelerators(self: *App) !void {
     try self.syncActionAccelerator("app.open-config", .{ .open_config = {} });
     try self.syncActionAccelerator("app.reload-config", .{ .reload_config = {} });
     try self.syncActionAccelerator("win.toggle_inspector", .{ .inspector = .toggle });
-    try self.syncActionAccelerator("win.close", .{ .close_surface = {} });
+    try self.syncActionAccelerator("win.close", .{ .close_window = {} });
     try self.syncActionAccelerator("win.new_window", .{ .new_window = {} });
     try self.syncActionAccelerator("win.new_tab", .{ .new_tab = {} });
     try self.syncActionAccelerator("win.split_right", .{ .new_split = .right });
@@ -1439,6 +1441,17 @@ fn newWindow(self: *App, parent_: ?*CoreSurface) !void {
     try window.newTab(parent_);
 }
 
+fn closeWindow(_: *App, target: apprt.action.Target) !bool {
+    switch (target) {
+        .app => return false,
+        .surface => |v| {
+            const window = v.rt_surface.container.window() orelse return false;
+            window.closeWithConfirmation();
+            return true;
+        },
+    }
+}
+
 fn quit(self: *App) void {
     // If we're already not running, do nothing.
     if (!self.running) return;
@@ -1517,6 +1530,7 @@ pub fn quitNow(self: *App) void {
         }
     }.callback, null);
 }
+
 fn gtkQuitConfirmation(
     alert: *c.GtkMessageDialog,
     response: c.gint,

diff --git a/src/apprt/gtk/Window.zig b/src/apprt/gtk/Window.zig
@@ -797,12 +797,15 @@ fn gtkRefocusTerm(v: *c.GtkWindow, ud: ?*anyopaque) callconv(.C) bool {
 
     return true;
 }
-
 fn gtkCloseRequest(v: *c.GtkWindow, ud: ?*anyopaque) callconv(.C) bool {
     _ = v;
     log.debug("window close request", .{});
     const self = userdataSelf(ud.?);
+    self.closeWithConfirmation();
+    return true;
+}
 
+pub fn closeWithConfirmation(self: *Window) void {
     // If none of our surfaces need confirmation, we can just exit.
     for (self.app.core_app.surfaces.items) |surface| {
         if (surface.container.window()) |window| {
@@ -811,15 +814,15 @@ fn gtkCloseRequest(v: *c.GtkWindow, ud: ?*anyopaque) callconv(.C) bool {
         }
     } else {
         c.gtk_window_destroy(self.window);
-        return true;
+        return;
     }
 
     const target: CloseDialog.Target = .{ .window = self };
 
     if (adwaita.supportsDialogs() and adwaita.enabled(&self.app.config)) {
         const dialog = CloseDialog.new();
         dialog.show(target);
-        return true;
+        return;
     }
 
     // Setup our basic message
@@ -851,7 +854,6 @@ fn gtkCloseRequest(v: *c.GtkWindow, ud: ?*anyopaque) callconv(.C) bool {
     _ = c.g_signal_connect_data(alert, "response", c.G_CALLBACK(&gtkCloseConfirmation), self, null, c.G_CONNECT_DEFAULT);
 
     c.gtk_widget_show(alert);
-    return true;
 }
 
 fn gtkCloseConfirmation(
@@ -966,11 +968,7 @@ fn gtkActionClose(
     ud: ?*anyopaque,
 ) callconv(.C) void {
     const self: *Window = @ptrCast(@alignCast(ud orelse return));
-    const surface = self.actionSurface() orelse return;
-    _ = surface.performBindingAction(.{ .close_surface = {} }) catch |err| {
-        log.warn("error performing binding action error={}", .{err});
-        return;
-    };
+    self.closeWithConfirmation();
 }
 
 fn gtkActionNewWindow(

diff --git a/src/apprt/gtk/close_dialog.zig b/src/apprt/gtk/close_dialog.zig
@@ -11,7 +11,15 @@ const gio = @import("gio");
 const adw = @import("adw");
 const gtk = @import("gtk");
 
+/// The dialog opened whenever the user requests to close a
+/// window/tab/split/etc. but there's still one or more running
+/// processes inside the target that cannot be closed automatically.
+/// We then ask the user whether they want to terminate existing processes.
+///
+/// Implemented as a simple subclass of Adw.AlertDialog that has
+/// styling and content specific to the target.
 pub const CloseDialog = extern struct {
+    // Mostly just GObject boilerplate
     parent: Parent,
 
     pub const Parent = adw.AlertDialog;
@@ -21,6 +29,7 @@ pub const CloseDialog = extern struct {
 
         var offset: c_int = 0;
     };
+
     pub const getGObjectType = gobject.ext.defineClass(CloseDialog, .{
         .instanceInit = init,
         .private = .{ .Type = Private, .offset = &Private.offset },
@@ -32,8 +41,13 @@ pub const CloseDialog = extern struct {
 
     pub fn show(self: *CloseDialog, target: Target) void {
         const super = self.as(Parent);
+
+        // If we don't have a possible window to ask the user,
+        // in most situations (e.g. when a split isn't attached to a window)
+        // we should just close unconditionally.
         const dialog_window = target.dialogWindow() orelse {
             target.close();
+            self.as(gtk.Widget).unref();
             return;
         };
 
@@ -72,6 +86,7 @@ pub const CloseDialog = extern struct {
         }
     }
 
+    // More GObject boilerplate
     pub fn as(self: *CloseDialog, comptime T: type) *T {
         return gobject.ext.as(T, self);
     }
@@ -86,6 +101,11 @@ pub const CloseDialog = extern struct {
         pub const Instance = CloseDialog;
     };
 
+    /// The target of this close dialog.
+    ///
+    /// This is here so that we can consolidate all logic related to
+    /// prompting the user and closing windows/tabs/surfaces/etc.
+    /// together into one struct that is the sole source of truth.
     pub const Target = union(enum) {
         app: *App,
         window: *Window,
@@ -113,6 +133,10 @@ pub const CloseDialog = extern struct {
         pub fn dialogWindow(self: Target) ?*gtk.Window {
             return switch (self) {
                 .app => {
+                    // Find the currently focused window. We don't store this
+                    // anywhere inside the App structure for some reason, so
+                    // we have to query every single open window and see which
+                    // one is active (focused and receiving keyboard input)
                     const list = gtk.Window.listToplevels();
                     defer list.free();