Skip to content

Commit

Permalink
Move app quit to apprt action (#4577)
Browse files Browse the repository at this point in the history
This changes quit signaling from a boolean return from core app `tick()`
to an apprt action. This simplifies the API and conceptually makes more
sense to me now.

This wasn't done just for that; this change was also needed so that
macOS can quit cleanly while fixing #4540 since we may no longer trigger
menu items. I wanted to split this out into a separate commit/PR because
it adds complexity making the diff harder to read.
  • Loading branch information
mitchellh authored Jan 4, 2025
2 parents 1baf892 + 6b30736 commit 8f5f432
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 54 deletions.
3 changes: 2 additions & 1 deletion include/ghostty.h
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ typedef struct {

// apprt.Action.Key
typedef enum {
GHOSTTY_ACTION_QUIT,
GHOSTTY_ACTION_NEW_WINDOW,
GHOSTTY_ACTION_NEW_TAB,
GHOSTTY_ACTION_NEW_SPLIT,
Expand Down Expand Up @@ -681,7 +682,7 @@ void ghostty_config_open();
ghostty_app_t ghostty_app_new(const ghostty_runtime_config_s*,
ghostty_config_t);
void ghostty_app_free(ghostty_app_t);
bool ghostty_app_tick(ghostty_app_t);
void ghostty_app_tick(ghostty_app_t);
void* ghostty_app_userdata(ghostty_app_t);
void ghostty_app_set_focus(ghostty_app_t, bool);
bool ghostty_app_key(ghostty_app_t, ghostty_input_key_s);
Expand Down
36 changes: 19 additions & 17 deletions macos/Sources/Ghostty/Ghostty.App.swift
Original file line number Diff line number Diff line change
Expand Up @@ -117,23 +117,7 @@ extension Ghostty {

func appTick() {
guard let app = self.app else { return }

// Tick our app, which lets us know if we want to quit
let exit = ghostty_app_tick(app)
if (!exit) { return }

// On iOS, applications do not terminate programmatically like they do
// on macOS. On iOS, applications are only terminated when a user physically
// closes the application (i.e. going to the home screen). If we request
// exit on iOS we ignore it.
#if os(iOS)
logger.info("quit request received, ignoring on iOS")
#endif

#if os(macOS)
// We want to quit, start that process
NSApplication.shared.terminate(nil)
#endif
ghostty_app_tick(app)
}

func openConfig() {
Expand Down Expand Up @@ -454,6 +438,9 @@ extension Ghostty {

// Action dispatch
switch (action.tag) {
case GHOSTTY_ACTION_QUIT:
quit(app)

case GHOSTTY_ACTION_NEW_WINDOW:
newWindow(app, target: target)

Expand Down Expand Up @@ -559,6 +546,21 @@ extension Ghostty {
}
}

private static func quit(_ app: ghostty_app_t) {
// On iOS, applications do not terminate programmatically like they do
// on macOS. On iOS, applications are only terminated when a user physically
// closes the application (i.e. going to the home screen). If we request
// exit on iOS we ignore it.
#if os(iOS)
logger.info("quit request received, ignoring on iOS")
#endif

#if os(macOS)
// We want to quit, start that process
NSApplication.shared.terminate(nil)
#endif
}

private static func newWindow(_ app: ghostty_app_t, target: ghostty_target_s) {
switch (target.tag) {
case GHOSTTY_TARGET_APP:
Expand Down
25 changes: 3 additions & 22 deletions src/App.zig
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ focused_surface: ?*Surface = null,
/// this is a blocking queue so if it is full you will get errors (or block).
mailbox: Mailbox.Queue,

/// Set to true once we're quitting. This never goes false again.
quit: bool,

/// The set of font GroupCache instances shared by surfaces with the
/// same font configuration.
font_grid_set: font.SharedGridSet,
Expand Down Expand Up @@ -98,7 +95,6 @@ pub fn create(
.alloc = alloc,
.surfaces = .{},
.mailbox = .{},
.quit = false,
.font_grid_set = font_grid_set,
.config_conditional_state = .{},
};
Expand All @@ -125,9 +121,7 @@ pub fn destroy(self: *App) void {
/// Tick ticks the app loop. This will drain our mailbox and process those
/// events. This should be called by the application runtime on every loop
/// tick.
///
/// This returns whether the app should quit or not.
pub fn tick(self: *App, rt_app: *apprt.App) !bool {
pub fn tick(self: *App, rt_app: *apprt.App) !void {
// If any surfaces are closing, destroy them
var i: usize = 0;
while (i < self.surfaces.items.len) {
Expand All @@ -142,13 +136,6 @@ pub fn tick(self: *App, rt_app: *apprt.App) !bool {

// Drain our mailbox
try self.drainMailbox(rt_app);

// No matter what, we reset the quit flag after a tick. If the apprt
// doesn't want to quit, then we can't force it to.
defer self.quit = false;

// We quit if our quit flag is on
return self.quit;
}

/// Update the configuration associated with the app. This can only be
Expand Down Expand Up @@ -272,7 +259,7 @@ fn drainMailbox(self: *App, rt_app: *apprt.App) !void {
// can try to quit as quickly as possible.
.quit => {
log.info("quit message received, short circuiting mailbox drain", .{});
self.setQuit();
try self.performAction(rt_app, .quit);
return;
},
}
Expand Down Expand Up @@ -314,12 +301,6 @@ pub fn newWindow(self: *App, rt_app: *apprt.App, msg: Message.NewWindow) !void {
);
}

/// Start quitting
pub fn setQuit(self: *App) void {
if (self.quit) return;
self.quit = true;
}

/// Handle an app-level focus event. This should be called whenever
/// the focus state of the entire app containing Ghostty changes.
/// This is separate from surface focus events. See the `focused`
Expand Down Expand Up @@ -437,7 +418,7 @@ pub fn performAction(
switch (action) {
.unbind => unreachable,
.ignore => {},
.quit => self.setQuit(),
.quit => try rt_app.performAction(.app, .quit, {}),
.new_window => try self.newWindow(rt_app, .{ .parent = null }),
.open_config => try rt_app.performAction(.app, .open_config, {}),
.reload_config => try rt_app.performAction(.app, .reload_config, .{}),
Expand Down
4 changes: 4 additions & 0 deletions src/apprt/action.zig
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ pub const Action = union(Key) {
// entry. If the value type is void then only the key needs to be
// added. Ensure the order matches exactly with the Zig code.

/// Quit the application.
quit,

/// Open a new window. The target determines whether properties such
/// as font size should be inherited.
new_window,
Expand Down Expand Up @@ -219,6 +222,7 @@ pub const Action = union(Key) {

/// Sync with: ghostty_action_tag_e
pub const Key = enum(c_int) {
quit,
new_window,
new_tab,
new_split,
Expand Down
5 changes: 2 additions & 3 deletions src/apprt/embedded.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1332,10 +1332,9 @@ pub const CAPI = struct {

/// Tick the event loop. This should be called whenever the "wakeup"
/// callback is invoked for the runtime.
export fn ghostty_app_tick(v: *App) bool {
return v.core_app.tick(v) catch |err| err: {
export fn ghostty_app_tick(v: *App) void {
v.core_app.tick(v) catch |err| {
log.err("error app tick err={}", .{err});
break :err false;
};
}

Expand Down
12 changes: 10 additions & 2 deletions src/apprt/glfw.zig
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ pub const App = struct {
app: *CoreApp,
config: Config,

/// Flips to true to quit on the next event loop tick. This
/// never goes false and forces the event loop to exit.
quit: bool = false,

/// Mac-specific state.
darwin: if (Darwin.enabled) Darwin else void,

Expand Down Expand Up @@ -124,8 +128,10 @@ pub const App = struct {
glfw.waitEvents();

// Tick the terminal app
const should_quit = try self.app.tick(self);
if (should_quit or self.app.surfaces.items.len == 0) {
try self.app.tick(self);

// If the tick caused us to quit, then we're done.
if (self.quit or self.app.surfaces.items.len == 0) {
for (self.app.surfaces.items) |surface| {
surface.close(false);
}
Expand All @@ -149,6 +155,8 @@ pub const App = struct {
value: apprt.Action.Value(action),
) !void {
switch (action) {
.quit => self.quit = true,

.new_window => _ = try self.newSurface(switch (target) {
.app => null,
.surface => |v| v,
Expand Down
18 changes: 9 additions & 9 deletions src/apprt/gtk/App.zig
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ pub fn performAction(
value: apprt.Action.Value(action),
) !void {
switch (action) {
.quit => self.quit(),
.new_window => _ = try self.newWindow(switch (target) {
.app => null,
.surface => |v| v,
Expand Down Expand Up @@ -1075,9 +1076,7 @@ fn loadCustomCss(self: *App) !void {
defer file.close();

log.info("loading gtk-custom-css path={s}", .{path});
const contents = try file.reader().readAllAlloc(
self.core_app.alloc,
5 * 1024 * 1024 // 5MB
const contents = try file.reader().readAllAlloc(self.core_app.alloc, 5 * 1024 * 1024 // 5MB
);
defer self.core_app.alloc.free(contents);

Expand Down Expand Up @@ -1174,14 +1173,10 @@ pub fn run(self: *App) !void {
_ = c.g_main_context_iteration(self.ctx, 1);

// Tick the terminal app and see if we should quit.
const should_quit = try self.core_app.tick(self);
try self.core_app.tick(self);

// Check if we must quit based on the current state.
const must_quit = q: {
// If we've been told by GTK that we should quit, do so regardless
// of any other setting.
if (should_quit) break :q true;

// If we are configured to always stay running, don't quit.
if (!self.config.@"quit-after-last-window-closed") break :q false;

Expand Down Expand Up @@ -1285,6 +1280,9 @@ fn newWindow(self: *App, parent_: ?*CoreSurface) !void {
}

fn quit(self: *App) void {
// If we're already not running, do nothing.
if (!self.running) return;

// If we have no toplevel windows, then we're done.
const list = c.gtk_window_list_toplevels();
if (list == null) {
Expand Down Expand Up @@ -1625,7 +1623,9 @@ fn gtkActionQuit(
ud: ?*anyopaque,
) callconv(.C) void {
const self: *App = @ptrCast(@alignCast(ud orelse return));
self.core_app.setQuit();
self.core_app.performAction(self, .quit) catch |err| {
log.err("error quitting err={}", .{err});
};
}

/// Action sent by the window manager asking us to present a specific surface to
Expand Down

0 comments on commit 8f5f432

Please sign in to comment.