From 2c0dbab7baa4fe6868a209664bb00fd1c7878851 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 25 Mar 2023 16:26:30 -0700 Subject: [PATCH 1/6] apprt/gtk: always confirm when surface is closed --- src/apprt/gtk.zig | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/apprt/gtk.zig b/src/apprt/gtk.zig index 5d6937aa7..ce5e7d60f 100644 --- a/src/apprt/gtk.zig +++ b/src/apprt/gtk.zig @@ -636,7 +636,29 @@ pub const Surface = struct { /// Close this surface. pub fn close(self: *Surface) void { - self.window.closeSurface(self); + // I'd like to make this prettier one day: + // - Make the "Yes" button red + // - Make the "No" button default + // + const alert = c.gtk_message_dialog_new( + self.window.window, + c.GTK_DIALOG_MODAL, + c.GTK_MESSAGE_QUESTION, + c.GTK_BUTTONS_YES_NO, + "Close this terminal?", + ); + c.gtk_message_dialog_format_secondary_text( + @ptrCast(*c.GtkMessageDialog, alert), + "There is still a running process in the terminal. " ++ + "Closing the terminal will kill this process. " ++ + "Are you sure you want to close the terminal?" ++ + "Click 'No' to cancel and return to your terminal.", + ); + + _ = c.g_signal_connect_data(alert, "response", c.G_CALLBACK(>kCloseConfirmation), self, null, G_CONNECT_DEFAULT); + + c.gtk_widget_show(alert); + //self.window.closeSurface(self); } pub fn newTab(self: *Surface) !void { @@ -1010,6 +1032,18 @@ pub const Surface = struct { }; } + fn gtkCloseConfirmation( + alert: *c.GtkMessageDialog, + response: c.gint, + ud: ?*anyopaque, + ) callconv(.C) void { + c.gtk_window_destroy(@ptrCast(*c.GtkWindow, alert)); + if (response == c.GTK_RESPONSE_YES) { + const self = userdataSelf(ud.?); + self.window.closeSurface(self); + } + } + fn userdataSelf(ud: *anyopaque) *Surface { return @ptrCast(*Surface, @alignCast(@alignOf(Surface), ud)); } From 3689f1fe390ad14651d3cc96042339b7c47cd3cb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 25 Mar 2023 16:36:12 -0700 Subject: [PATCH 2/6] apprt/gtk: only show exit confirmation if process is alive --- src/App.zig | 6 ++++-- src/Surface.zig | 12 +++++++++++- src/apprt/glfw.zig | 7 +++++-- src/apprt/gtk.zig | 13 ++++++------- src/apprt/surface.zig | 4 ++++ src/termio/Exec.zig | 2 +- 6 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/App.zig b/src/App.zig index f2d18a7fd..b5635c0da 100644 --- a/src/App.zig +++ b/src/App.zig @@ -74,7 +74,7 @@ pub fn tick(self: *App, rt_app: *apprt.App) !bool { while (i < self.surfaces.items.len) { const surface = self.surfaces.items[i]; if (surface.shouldClose()) { - rt_app.closeSurface(surface); + surface.close(false); continue; } @@ -143,8 +143,10 @@ fn reloadConfig(self: *App, rt_app: *apprt.App) !void { } fn closeSurface(self: *App, rt_app: *apprt.App, surface: *Surface) !void { + _ = rt_app; + if (!self.hasSurface(surface)) return; - rt_app.closeSurface(surface.rt_surface); + surface.close(); } fn redrawSurface(self: *App, rt_app: *apprt.App, surface: *apprt.Surface) !void { diff --git a/src/Surface.zig b/src/Surface.zig index 80fd87ac6..1b0a82cea 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -96,6 +96,10 @@ config: DerivedConfig, /// like such as "control-v" will write a "v" even if they're intercepted. ignore_char: bool = false, +/// This is set to true if our IO thread notifies us our child exited. +/// This is used to determine if we need to confirm, hold open, etc. +child_exited: bool = false, + /// Mouse state for the surface. const Mouse = struct { /// The last tracked mouse button state by button. @@ -522,7 +526,7 @@ pub fn deinit(self: *Surface) void { /// Close this surface. This will trigger the runtime to start the /// close process, which should ultimately deinitialize this surface. pub fn close(self: *Surface) void { - self.rt_surface.close(); + self.rt_surface.close(!self.child_exited); } /// Called from the app thread to handle mailbox messages to our specific @@ -553,6 +557,12 @@ pub fn handleMessage(self: *Surface, msg: Message) !void { }, .close => self.close(), + + // Close without confirmation. + .child_exited => { + self.child_exited = true; + self.close(); + }, } } diff --git a/src/apprt/glfw.zig b/src/apprt/glfw.zig index dfe758bf9..034ed31c1 100644 --- a/src/apprt/glfw.zig +++ b/src/apprt/glfw.zig @@ -409,8 +409,11 @@ pub const Surface = struct { } /// Close this surface. - pub fn close(self: *const Surface) void { - self.window.setShouldClose(true); + pub fn close(self: *Surface, processActive: bool) void { + _ = processActive; + self.setShouldClose(); + self.deinit(); + self.app.app.alloc.destroy(self); } /// Set the size limits of the window. diff --git a/src/apprt/gtk.zig b/src/apprt/gtk.zig index ce5e7d60f..2c1f653ab 100644 --- a/src/apprt/gtk.zig +++ b/src/apprt/gtk.zig @@ -171,11 +171,6 @@ pub const App = struct { } /// Close the given surface. - pub fn closeSurface(self: *App, surface: *Surface) void { - _ = self; - surface.close(); - } - pub fn redrawSurface(self: *App, surface: *Surface) void { _ = self; surface.invalidate(); @@ -635,7 +630,12 @@ pub const Surface = struct { } /// Close this surface. - pub fn close(self: *Surface) void { + pub fn close(self: *Surface, processActive: bool) void { + if (!processActive) { + self.window.closeSurface(self); + return; + } + // I'd like to make this prettier one day: // - Make the "Yes" button red // - Make the "No" button default @@ -658,7 +658,6 @@ pub const Surface = struct { _ = c.g_signal_connect_data(alert, "response", c.G_CALLBACK(>kCloseConfirmation), self, null, G_CONNECT_DEFAULT); c.gtk_widget_show(alert); - //self.window.closeSurface(self); } pub fn newTab(self: *Surface) !void { diff --git a/src/apprt/surface.zig b/src/apprt/surface.zig index d5ebc0083..859d694d5 100644 --- a/src/apprt/surface.zig +++ b/src/apprt/surface.zig @@ -33,6 +33,10 @@ pub const Message = union(enum) { /// Close the surface. This will only close the current surface that /// receives this, not the full application. close: void, + + /// The child process running in the surface has exited. This may trigger + /// a surface close, it may not. + child_exited: void, }; /// A surface mailbox. diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index a9632d799..4e908f72a 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -401,7 +401,7 @@ fn processExit( // Notify our surface we want to close _ = ev.surface_mailbox.push(.{ - .close = {}, + .child_exited = {}, }, .{ .forever = {} }); return .disarm; From 86c4a8ed7d11b0b2589c39aad03aa01a476bbd54 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 25 Mar 2023 16:41:18 -0700 Subject: [PATCH 3/6] apprt/embedded: support new process alive callback on close --- include/ghostty.h | 2 +- macos/Sources/Ghostty/AppState.swift | 6 ++++-- src/apprt/embedded.zig | 8 ++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/include/ghostty.h b/include/ghostty.h index 96f7de272..0d808373d 100644 --- a/include/ghostty.h +++ b/include/ghostty.h @@ -221,7 +221,7 @@ 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 *); typedef void (*ghostty_runtime_new_split_cb)(void *, ghostty_split_direction_e); -typedef void (*ghostty_runtime_close_surface_cb)(void *); +typedef void (*ghostty_runtime_close_surface_cb)(void *, bool); typedef void (*ghostty_runtime_focus_split_cb)(void *, ghostty_split_focus_direction_e); typedef struct { diff --git a/macos/Sources/Ghostty/AppState.swift b/macos/Sources/Ghostty/AppState.swift index ab2fa605d..90bc93785 100644 --- a/macos/Sources/Ghostty/AppState.swift +++ b/macos/Sources/Ghostty/AppState.swift @@ -60,7 +60,7 @@ extension Ghostty { read_clipboard_cb: { userdata in AppState.readClipboard(userdata) }, write_clipboard_cb: { userdata, str in AppState.writeClipboard(userdata, string: str) }, new_split_cb: { userdata, direction in AppState.newSplit(userdata, direction: direction) }, - close_surface_cb: { userdata in AppState.closeSurface(userdata) }, + close_surface_cb: { userdata, processAlive in AppState.closeSurface(userdata, processAlive: processAlive) }, focus_split_cb: { userdata, direction in AppState.focusSplit(userdata, direction: direction) } ) @@ -132,7 +132,9 @@ extension Ghostty { ]) } - static func closeSurface(_ userdata: UnsafeMutableRawPointer?) { + static func closeSurface(_ userdata: UnsafeMutableRawPointer?, processAlive: Bool) { + // TODO: handle processAlive to show confirmation dialog. We probably want to attach + // it to the notification and let downstream handle it. guard let surface = self.surfaceUserdata(from: userdata) else { return } NotificationCenter.default.post(name: Notification.ghosttyCloseSurface, object: surface) } diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 08fd04a5d..9dceddc8d 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -56,7 +56,7 @@ pub const App = struct { new_split: ?*const fn (SurfaceUD, input.SplitDirection) callconv(.C) void = null, /// Close the current surface given by this function. - close_surface: ?*const fn (SurfaceUD) callconv(.C) void = null, + close_surface: ?*const fn (SurfaceUD, bool) callconv(.C) void = null, /// Focus the previous/next split (if any). focus_split: ?*const fn (SurfaceUD, input.SplitFocusDirection) callconv(.C) void = null, @@ -188,13 +188,13 @@ pub const Surface = struct { func(self.opts.userdata, direction); } - pub fn close(self: *const Surface) void { + pub fn close(self: *const Surface, process_alive: bool) void { const func = self.app.opts.close_surface orelse { log.info("runtime embedder does not support closing a surface", .{}); return; }; - func(self.opts.userdata); + func(self.opts.userdata, process_alive); } pub fn gotoSplit(self: *const Surface, direction: input.SplitFocusDirection) void { @@ -506,7 +506,7 @@ pub const CAPI = struct { /// Request that the surface become closed. This will go through the /// normal trigger process that a close surface input binding would. export fn ghostty_surface_request_close(ptr: *Surface) void { - ptr.close(); + ptr.core_surface.close(); } /// Request that the surface split in the given direction. From bc9973d37fa592199cd8b9e105d8f9b2629b0b5c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 25 Mar 2023 16:45:17 -0700 Subject: [PATCH 4/6] apprt/gtk: set proper defaults to confirmation dialog --- src/apprt/gtk.zig | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/apprt/gtk.zig b/src/apprt/gtk.zig index 2c1f653ab..0fc39c91e 100644 --- a/src/apprt/gtk.zig +++ b/src/apprt/gtk.zig @@ -636,10 +636,7 @@ pub const Surface = struct { return; } - // I'd like to make this prettier one day: - // - Make the "Yes" button red - // - Make the "No" button default - // + // Setup our basic message const alert = c.gtk_message_dialog_new( self.window.window, c.GTK_DIALOG_MODAL, @@ -655,6 +652,19 @@ pub const Surface = struct { "Click 'No' to cancel and return to your terminal.", ); + // We want the "yes" to appear destructive. + const yes_widget = c.gtk_dialog_get_widget_for_response( + @ptrCast(*c.GtkDialog, alert), + c.GTK_RESPONSE_YES, + ); + c.gtk_widget_add_css_class(yes_widget, "destructive-action"); + + // We want the "no" to be the default action + c.gtk_dialog_set_default_response( + @ptrCast(*c.GtkDialog, alert), + c.GTK_RESPONSE_NO, + ); + _ = c.g_signal_connect_data(alert, "response", c.G_CALLBACK(>kCloseConfirmation), self, null, G_CONNECT_DEFAULT); c.gtk_widget_show(alert); From a1831ecacba3cfb682b538fa13345168c302fe2c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 26 Mar 2023 10:40:38 -0700 Subject: [PATCH 5/6] macos: show close confirmation if running process exists --- macos/Sources/Ghostty/AppState.swift | 6 ++-- macos/Sources/Ghostty/Ghostty.SplitView.swift | 35 ++++++++++++++++++- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/macos/Sources/Ghostty/AppState.swift b/macos/Sources/Ghostty/AppState.swift index 90bc93785..4463cce5c 100644 --- a/macos/Sources/Ghostty/AppState.swift +++ b/macos/Sources/Ghostty/AppState.swift @@ -133,10 +133,10 @@ extension Ghostty { } static func closeSurface(_ userdata: UnsafeMutableRawPointer?, processAlive: Bool) { - // TODO: handle processAlive to show confirmation dialog. We probably want to attach - // it to the notification and let downstream handle it. guard let surface = self.surfaceUserdata(from: userdata) else { return } - NotificationCenter.default.post(name: Notification.ghosttyCloseSurface, object: surface) + NotificationCenter.default.post(name: Notification.ghosttyCloseSurface, object: surface, userInfo: [ + "process_alive": processAlive, + ]) } static func focusSplit(_ userdata: UnsafeMutableRawPointer?, direction: ghostty_split_focus_direction_e) { diff --git a/macos/Sources/Ghostty/Ghostty.SplitView.swift b/macos/Sources/Ghostty/Ghostty.SplitView.swift index 52f8fdde6..6f87922e1 100644 --- a/macos/Sources/Ghostty/Ghostty.SplitView.swift +++ b/macos/Sources/Ghostty/Ghostty.SplitView.swift @@ -205,6 +205,9 @@ extension Ghostty { /// This will be set to true when the split requests that is become closed. @Binding var requestClose: Bool + /// This controls whether we're actively confirming if we want to close or not. + @State private var confirmClose: Bool = false + var body: some View { let center = NotificationCenter.default let pub = center.publisher(for: Notification.ghosttyNewSplit, object: leaf.surface) @@ -212,8 +215,38 @@ extension Ghostty { let pubFocus = center.publisher(for: Notification.ghosttyFocusSplit, object: leaf.surface) SurfaceWrapper(surfaceView: leaf.surface) .onReceive(pub) { onNewSplit(notification: $0) } - .onReceive(pubClose) { _ in requestClose = true } + .onReceive(pubClose) { onClose(notification: $0) } .onReceive(pubFocus) { onMoveFocus(notification: $0) } + .confirmationDialog( + "Close Terminal?", + isPresented: $confirmClose) { + Button("Close the Terminal", role: .destructive) { + confirmClose = false + requestClose = true + } + .keyboardShortcut(.defaultAction) + } message: { + Text("The terminal still has a running process. If you close the terminal " + + "the process will be killed.") + } + } + + private func onClose(notification: SwiftUI.Notification) { + var processAlive = false + if let valueAny = notification.userInfo?["process_alive"] { + if let value = valueAny as? Bool { + processAlive = value + } + } + + // If the child process is not alive, then we exit immediately + guard processAlive else { + requestClose = true + return + } + + // Child process is alive, so we want to show a confirmation. + confirmClose = true } private func onNewSplit(notification: SwiftUI.Notification) { From 8fa5a9d29983f3940b2049c9452453da1e7e18c9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sun, 26 Mar 2023 10:43:57 -0700 Subject: [PATCH 6/6] macos: Cmd+W closes settings window if focused --- macos/Sources/GhosttyApp.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/macos/Sources/GhosttyApp.swift b/macos/Sources/GhosttyApp.swift index 1e6fabc43..79f5c3521 100644 --- a/macos/Sources/GhosttyApp.swift +++ b/macos/Sources/GhosttyApp.swift @@ -82,7 +82,11 @@ struct GhosttyApp: App { } func close() { - guard let surfaceView = focusedSurface else { return } + guard let surfaceView = focusedSurface else { + Self.closeWindow() + return + } + guard let surface = surfaceView.surface else { return } ghostty.requestClose(surface: surface) }